Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set an expiration for password reset tokens #532

Closed
wants to merge 70 commits into from
Closed
Show file tree
Hide file tree
Changes from 69 commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
d073537
Add allow_password_reset_expiration as a configurable option
mxie Jan 16, 2015
19035fd
Generate password resets migration
mxie Jan 19, 2015
11a6917
Create a PasswordResetMigrationGenerator
mxie Jan 21, 2015
041f77a
Add migration for removing confirmation_token column from users
mxie Jan 22, 2015
18b94f5
Set up PasswordReset model
mxie Jan 22, 2015
83c2c4c
Get rid of user association for password resets
mxie Jan 27, 2015
54c0880
Rename PasswordsController to PasswordResetsController
mxie Jan 27, 2015
597b156
keep the user-facing routes the same
mxie Jan 27, 2015
0103fcc
validate on user_id for password resets
mxie Jan 27, 2015
7b6fa2a
update PasswordResetsController#create to generate a new PasswordReset
mxie Jan 27, 2015
ee54dda
find matching password reset before returning the user
mxie Feb 2, 2015
7583ba9
Add missing password_reset_time_limit accessor + docs
mxie Feb 2, 2015
1d39226
user as an association
mxie Feb 2, 2015
a6e9f2e
rename method for finding user from password_reset
mxie Feb 2, 2015
e9eb533
check for expired token
mxie Feb 3, 2015
98f009f
wip: invalidate other reset tokens
mxie Feb 3, 2015
63a7043
add PasswordReset#active_for(user)
mxie Feb 3, 2015
e0b035a
add PasswordReset#deactivate_all
mxie Feb 3, 2015
ad444c3
implemeneted PasswordResetDeactivator
mxie Feb 3, 2015
393be4c
fix "no password" context in password resets controller spec
mxie Feb 3, 2015
e25e13d
just pass the password_reset object to mail delivery
mxie Feb 3, 2015
7c9a250
mention 15 minute expiration
mxie Feb 3, 2015
f8ec321
fix mailer specs
mxie Feb 3, 2015
ea1630f
remove confirmation_token checkin in user specs
mxie Feb 3, 2015
4e12f79
fix timestamp issues in migrations
mxie Feb 3, 2015
3ef2427
wip: check if confirmation_token column exists in the first place
mxie Feb 5, 2015
8cb3007
also check if the users table exists when checking for conf_token
mxie Feb 5, 2015
9867298
rename back to PasswordsController
mxie Feb 5, 2015
15d0be8
interpolate the time limit in the email template
mxie Feb 5, 2015
0348bcf
Remove with_forgotten_password trait for user
mxie Feb 6, 2015
c39d235
add confirmation_token datatype and change UPDATE statement
mxie Feb 6, 2015
f36db0e
fix mailer spec to include interpolation
mxie Feb 6, 2015
5f233fc
fix feature spec for visitor resetting password
mxie Feb 6, 2015
efb85c5
assign @password_reset in update when it fails to update user password
mxie Feb 6, 2015
8bc26b9
remove User#forgot_password!
mxie Feb 6, 2015
78ca920
README edit #1: new model/migration info + config option
mxie Feb 6, 2015
363954f
README edit #2: change deliver_email(user) reference
mxie Feb 6, 2015
58ad25d
README edit #3: add info about standalone PR migration generator
mxie Feb 6, 2015
8cb433c
Hound fixes: quotes, line lengths, alignment
mxie Feb 6, 2015
1986551
Rails 3.2 fixes
mxie Feb 6, 2015
5b9d26a
add users table checks to Clearance::Generators::Migration
mxie Feb 6, 2015
9024ac2
remove weird db/test-sqlite3-journal file
mxie Feb 6, 2015
fa3af5f
PasswordReset's time_limit should be a class method, not instance
mxie Feb 6, 2015
d7ee951
extract password reset generation
mxie Feb 10, 2015
fdcede4
run validations when deactivating password resets
mxie Feb 10, 2015
d27f72a
compound index with user_id + expires_at
mxie Feb 12, 2015
c360398
Module#include is a private method in Ruby 1.9.3.
mxie Feb 12, 2015
dfb10c3
bring find_user_from_password_reset closer to the callback
mxie Feb 12, 2015
1ac223a
simplify method call for deactiving password resets
mxie Mar 2, 2015
1e4870b
extract expiration timestamp as method
mxie Mar 2, 2015
2c2a317
Clean up instructions about customizable settings for emails
mxie Mar 21, 2015
42153c0
delivers->sends email and add "Existing users" as a prefix
mxie Jun 5, 2015
91b0bab
Simplify associating a user to new password reset
mxie Jun 5, 2015
dbf0acb
shorten password reset finding method
mxie Jun 5, 2015
caccebf
Bump limit to 20 minutes
mxie Jun 5, 2015
c41a02a
Rename deactivator to be plural
mxie Jun 5, 2015
498488e
rename deactivate! and add complete
mxie Jun 5, 2015
84ddd55
move time limit test
mxie Jun 5, 2015
d2cfb56
Change pw reset URL in text formatted email and mailer spec
mxie Jun 5, 2015
cc0858b
Remove character limit on token
mxie Jun 5, 2015
cf0c985
wip
mxie Jun 12, 2015
8492fcc
remove deactivation check
mxie Jul 17, 2015
2962f39
remove leftover controller callback
mxie Jul 17, 2015
70bce4a
find active resets
mxie Jul 17, 2015
0ec2fa5
change update failure message to be more generic
mxie Jul 17, 2015
d49f107
clean up password reset spec
mxie Jul 17, 2015
d5ad597
handle completed reset
mxie Jul 17, 2015
a3c73c7
just have #complete return true or false
mxie Jul 17, 2015
e8358da
forgot to remove :with_forgotten_password trait in spec
mxie Jul 17, 2015
3bd868e
long lines
mxie Jul 17, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ The generator:
* Creates an initializer to allow further configuration.
* Creates a migration that either creates a users table or adds any necessary
columns to the existing table.
* Creates a `PasswordReset` model.
* Creates a migration to remove the `confirmation_token` column from the users
table if it exists.

## Configure

Expand All @@ -47,6 +50,7 @@ Clearance.configure do |config|
config.routes = true
config.httponly = false
config.mailer_sender = '[email protected]'
config.password_reset_time_limit = 20.minutes
config.password_strategy = Clearance::PasswordStrategies::BCrypt
config.redirect_url = '/'
config.secure_cookie = false
Expand Down Expand Up @@ -106,15 +110,34 @@ helpers. For example:

### Password Resets

When a user resets their password, Clearance delivers them an email. You
should change the `mailer_sender` default, used in the email's "from" header:
When a user resets their password, Clearance sends them an email.

By default, the password reset token expires in 15 minutes. You can change the
time limit by passing in an `ActiveSupport::Duration` to
`config.password_reset_time_limit`.

You should also change the `mailer_sender` default, which used in the email's
"from" header:

```ruby
Clearance.configure do |config|
config.mailer_sender = '[email protected]'
config.password_reset_time_limit = 1.hour
end
```

*Existing users*: If your app is already using Clearance but it does not have
the token expiration feature, you can generate and run the migrations:

```shell
rails generate clearance:password_reset_migration
```

This will create a migration for the new password_resets table. If there are any
existing password resets (i.e. any user with a confirmation token), those will
be migrated over to the new table with its expiration set to the time limit
configured.

### Integrating with Rack Applications

Clearance adds its session to the Rack environment hash so middleware and other
Expand Down Expand Up @@ -241,8 +264,8 @@ If you are using an earlier version of Rails, you can override the

```ruby
class PasswordsController < Clearance::PasswordsController
def deliver_email(user)
ClearanceMailer.delay.change_password(user)
def deliver_email(password_reset)
ClearanceMailer.delay.change_password(password_reset)
end
end
```
Expand Down
41 changes: 18 additions & 23 deletions app/controllers/clearance/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@
class Clearance::PasswordsController < Clearance::BaseController
skip_before_filter :require_login, only: [:create, :edit, :new, :update]
skip_before_filter :authorize, only: [:create, :edit, :new, :update]
before_filter :ensure_existing_user, only: [:edit, :update]
before_filter :ensure_valid_password_reset, only: [:edit, :update]

def create
if user = find_user_for_create
user.forgot_password!
deliver_email(user)
password_reset = PasswordReset.create!(user: user)
deliver_email(password_reset)
end
render template: 'passwords/create'
end

def edit
@user = find_user_for_edit
@password_reset = find_active_password_reset
render template: 'passwords/edit'
end

Expand All @@ -23,10 +23,10 @@ def new
end

def update
@user = find_user_for_update
@password_reset = find_active_password_reset

if @user.update_password password_reset_params
sign_in @user
if @password_reset.complete(password_reset_params)
sign_in @password_reset.user
redirect_to url_after_update
else
flash_failure_after_update
Expand All @@ -36,8 +36,8 @@ def update

private

def deliver_email(user)
mail = ::ClearanceMailer.change_password(user)
def deliver_email(password_reset)
mail = ::ClearanceMailer.change_password(password_reset)

if Gem::Version.new(Rails::VERSION::STRING) >= Gem::Version.new("4.2.0")
mail.deliver_later
Expand All @@ -55,28 +55,23 @@ def password_reset_params
end
end

def find_user_by_id_and_confirmation_token
user_param = Clearance.configuration.user_id_parameter
def find_active_password_reset
@find_active_password_reset ||= PasswordReset.
active_for(params[user_param]).
find_by_token(params[:token])
end

Clearance.configuration.user_model.
find_by_id_and_confirmation_token params[user_param], params[:token].to_s
def user_param
Clearance.configuration.user_id_parameter
end

def find_user_for_create
Clearance.configuration.user_model.
find_by_normalized_email params[:password][:email]
end

def find_user_for_edit
find_user_by_id_and_confirmation_token
end

def find_user_for_update
find_user_by_id_and_confirmation_token
end

def ensure_existing_user
unless find_user_by_id_and_confirmation_token
def ensure_valid_password_reset
unless find_active_password_reset
flash_failure_when_forbidden
render template: "passwords/new"
end
Expand Down
8 changes: 5 additions & 3 deletions app/mailers/clearance_mailer.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
class ClearanceMailer < ActionMailer::Base
def change_password(user)
@user = user
def change_password(password_reset)
@password_reset = password_reset
@time_limit = PasswordReset.time_limit

mail(
from: Clearance.configuration.mailer_sender,
to: @user.email,
to: @password_reset.user_email,
subject: I18n.t(
:change_password,
scope: [:clearance, :models, :clearance_mailer],
Expand Down
58 changes: 58 additions & 0 deletions app/models/password_reset.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
class PasswordReset < ActiveRecord::Base
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Clearance models are internal to clearance and not generate. User is a template because we expect you already have a user class of some sort.

before_create :generate_token, :generate_expiration_timestamp

belongs_to :user

validates :user_id, presence: true

delegate :email, to: :user, prefix: true

def self.active_for(user_id)
where("#{Clearance.configuration.user_id_parameter} = ? AND expires_at > ?", user_id, Time.zone.now)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [104/80]

end

def self.time_limit
Clearance.configuration.password_reset_time_limit
end

def complete(new_password)
reset_successful = false

transaction do
unless user.update_password(new_password)
raise ActiveRecord::Rollback
end

deactivate_user_resets
reset_successful = true
end

reset_successful
end

def deactivate
update_attributes(expires_at: Time.zone.now)
end

def expired?
expires_at <= Time.zone.now
end

private

def active?
!expired?
end

def deactivate_user_resets
Clearance::PasswordResetsDeactivator.new(user).run
end

def generate_token
self.token = Clearance::Token.new
end

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra empty line detected at body end.

def generate_expiration_timestamp
self.expires_at = self.class.time_limit.from_now
end
end
4 changes: 2 additions & 2 deletions app/views/clearance_mailer/change_password.html.erb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
<p><%= t(".opening") %></p>
<p><%= t(".opening", time_limit: distance_of_time_in_words(@time_limit)) %></p>

<p>
<%= link_to t(".link_text", default: "Change my password"),
edit_user_password_url(@user, token: @user.confirmation_token.html_safe) %>
edit_user_password_url(@password_reset.user, token: @password_reset.token) %>
</p>

<p><%= raw t(".closing") %></p>
4 changes: 2 additions & 2 deletions app/views/clearance_mailer/change_password.text.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<%= t(".opening") %></p>
<%= t(".opening", time_limit: distance_of_time_in_words(@time_limit)) %>

<%= edit_user_password_url(@user, token: @user.confirmation_token.html_safe) %>
<%= edit_user_password_url(@password_reset.user, token: @password_reset.token) %>

<%= raw t(".closing") %>
4 changes: 2 additions & 2 deletions app/views/passwords/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
<p><%= t(".description") %></p>

<%= form_for :password_reset,
url: user_password_path(@user, token: @user.confirmation_token),
html: { method: :put } do |form| %>
url: user_password_path(@password_reset.user, token: @password_reset.token),
html: { method: :put } do |form| %>
<div class="password-field">
<%= form.label :password %>
<%= form.password_field :password %>
Expand Down
4 changes: 2 additions & 2 deletions config/locales/clearance.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ en:
not been changed.
link_text: Change my password
opening: "Someone, hopefully you, requested we send you a link to change
your password:"
your password. This link expires in %{time_limit}:"
flashes:
failure_after_create: Bad email or password.
failure_after_update: Password can't be blank.
failure_after_update: Something went wrong with the password you provided.
failure_when_forbidden: Please double check the URL or try submitting
the form again.
helpers:
Expand Down
12 changes: 12 additions & 0 deletions db/migrate/20150122204353_create_password_resets.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class CreatePasswordResets < ActiveRecord::Migration
def change
create_table :password_resets do |t|
t.integer :user_id, null: false
t.string :token, null: false
t.datetime :expires_at, null: false
t.timestamps null: false
end

add_index :password_resets, [:user_id, :expires_at]
end
end
36 changes: 36 additions & 0 deletions db/migrate/20150127232904_remove_confirmation_token_from_users.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
class RemoveConfirmationTokenFromUsers < ActiveRecord::Migration
def up
execute <<-SQL
INSERT INTO password_resets
(user_id, token, expires_at, created_at, updated_at)
SELECT
users.id,
users.confirmation_token,
'#{expiration_timestamp}',
CURRENT_TIMESTAMP,
CURRENT_TIMESTAMP
FROM users
WHERE users.confirmation_token IS NOT NULL
SQL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good thinking!


remove_column :users, :confirmation_token
end

def expiration_timestamp
Clearance.
configuration.
password_reset_time_limit.
from_now
end

def down
add_column :users, :confirmation_token, :string, limit: 128

execute <<-SQL
UPDATE users
SET confirmation_token =
(SELECT token FROM password_resets
WHERE users.id = password_resets.user_id)
SQL
end
end
15 changes: 12 additions & 3 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,23 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20110111224543) do
ActiveRecord::Schema.define(version: 20150127232904) do

create_table "users", force: true do |t|
create_table "password_resets", force: :cascade do |t|
t.integer "user_id", null: false
t.string "token", null: false
t.datetime "expires_at", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
end

add_index "password_resets", ["user_id", "expires_at"], name: "index_password_resets_on_user_id_and_expires_at"

create_table "users", force: :cascade do |t|
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.string "email", null: false
t.string "encrypted_password", limit: 128, null: false
t.string "confirmation_token", limit: 128
t.string "remember_token", limit: 128, null: false
end

Expand Down
1 change: 1 addition & 0 deletions lib/clearance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require 'clearance/controller'
require 'clearance/user'
require 'clearance/engine'
require "clearance/password_resets_deactivator"
require 'clearance/password_strategies'
require 'clearance/constraints'

Expand Down
6 changes: 6 additions & 0 deletions lib/clearance/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ class Configuration
# @return [String]
attr_accessor :mailer_sender

# The time limit given to the user before their password reset token expires
# Defaults to 20.minutes (900 seconds)
# @return [ActiveSupport::Duration]
attr_accessor :password_reset_time_limit

# The password strategy to use when authenticating and setting passwords.
# Defaults to `Clearance::PasswordStrategies::BCrypt`.
# @return [Class #authenticated? #password=]
Expand Down Expand Up @@ -93,6 +98,7 @@ def initialize
@cookie_name = "remember_token"
@httponly = false
@mailer_sender = '[email protected]'
@password_reset_time_limit = 20.minutes
@redirect_url = '/'
@routes = true
@secure_cookie = false
Expand Down
19 changes: 19 additions & 0 deletions lib/clearance/password_resets_deactivator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
module Clearance
class PasswordResetsDeactivator
def initialize(user)
@user = user
end

def run
active_password_resets.each(&:deactivate)
end

private

attr_reader :user

def active_password_resets
PasswordReset.active_for(user)
end
end
end
Loading