Skip to content

Commit

Permalink
Add expiring, databaseless password reset tokens
Browse files Browse the repository at this point in the history
It is a best security practice for password reset token to expire after
some amount of time. In Clearance 1.x, this was not the case. A password
reset email could be used months after it was originally sent so long as
no other password reset was ever completed.

In this change, password resets expire after 15 minutes (configurable)
or after the user successfully changes their password in any manner
(whichever comes first).

The token, confusingly called `confirmation_token`, is no longer stored
in the database. Instead, we use `ActiveSupport::MessageVerifier` to
sign the token and validate it when it is used. The message verifier is
configurable in case developers want to use something else.

In a future refactoring, I'd like to introduce a layer between Clearance
and `ActiveSupport::MessageVerifier` to make the API a bit more pleasant
to use, but this is an exercise for a future PR. For instance, I'd
prefer that the Clearance abstraction generate and validate tokens only
by taking a user object (and using the Clearance configuration).
  • Loading branch information
Derek Prior & Melissa Xie authored and derekprior committed May 15, 2016
1 parent 64047ca commit 9a6185f
Show file tree
Hide file tree
Showing 24 changed files with 241 additions and 155 deletions.
10 changes: 10 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,21 @@ complete changelog, see the git history for each version via the version links.
- Removed support for Ruby versions older than 2.2
- Removed support for Rails versions older than 4.2
- Removed all deprecated code from Clearance 1.x
- Removed `User#confirmation_token`, `User#forgot_password!`, and
`User#generate_confirmation_token` as part of the change to expiring,
databaseless password reset tokens.

### Changed
- Password resets now use expiring signed tokens that do not require persistence
to the `users` table. By default, the tokens are generated with
`ActiveSupport::MessageVerifier` and expire in 15 minutes.
- Flash messages now use `flash[:alert]` rather than `flash[:notice]` as they
were used as errors more than notices.

### Added
- `rails generate clearance:upgrade` generator to prepare your Clearance 1.x
project for Clearance 2.0

[2.0.0]: https://github.com/thoughtbot/clearance/compare/v1.14.1...2.0

## [1.14.1] - May 12, 2016
Expand Down
46 changes: 25 additions & 21 deletions app/controllers/clearance/passwords_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ def new

def create
if user = find_user_for_create
user.forgot_password!
deliver_email(user)
end

Expand All @@ -26,9 +25,9 @@ def edit
def update
@user = find_user_for_update

if @user.update_password password_reset_params
sign_in @user
redirect_to url_after_update
if @user.update_password(password_reset_params)
sign_in(@user)
redirect_to(url_after_update)
else
flash_failure_after_update
render template: "passwords/edit"
Expand All @@ -38,45 +37,50 @@ def update
private

def deliver_email(user)
mail = ::ClearanceMailer.change_password(user)
mail.deliver_later
::ClearanceMailer.change_password(user).deliver_later
end

def password_reset_params
params[:password_reset][:password]
end

def find_user_by_id_and_confirmation_token
user_param = Clearance.configuration.user_id_parameter

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

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

def find_user_by_password_reset_token(token)
verifier = Clearance.configuration.message_verifier
user_model = Clearance.configuration.user_model

begin
user_id, encrypted_password, expiration = verifier.verify(token)
rescue
expiration = 1.day.ago
end

if expiration.future?
user_model.find_by(id: user_id, encrypted_password: encrypted_password)
end
end

def find_user_for_edit
find_user_by_id_and_confirmation_token
find_user_by_password_reset_token(params[:token])
end

def find_user_for_update
find_user_by_id_and_confirmation_token
find_user_by_password_reset_token(params[:token])
end

def ensure_existing_user
unless find_user_by_id_and_confirmation_token
flash_failure_when_forbidden
unless find_user_by_password_reset_token(params[:token])
flash_failure_when_invalid
render template: "passwords/new"
end
end

def flash_failure_when_forbidden
flash.now[:alert] = translate(:forbidden,
scope: [:clearance, :controllers, :passwords],
default: t("flashes.failure_when_forbidden"))
def flash_failure_when_invalid
flash.now[:alert] = translate("flashes.failure_when_password_reset_invalid")
end

def flash_failure_after_update
Expand Down
12 changes: 12 additions & 0 deletions app/mailers/clearance_mailer.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
class ClearanceMailer < ActionMailer::Base
def change_password(user)
@user = user
@token = generate_password_reset_token(@user)

mail(
from: Clearance.configuration.mailer_sender,
to: @user.email,
Expand All @@ -10,4 +12,14 @@ def change_password(user)
),
)
end

private

def generate_password_reset_token(user)
Clearance.configuration.message_verifier.generate([
user.id,
user.encrypted_password,
Clearance.configuration.password_reset_time_limit.from_now,
])
end
end
2 changes: 1 addition & 1 deletion app/views/clearance_mailer/change_password.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<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(@user, token: @token) %>
</p>

<p><%= raw t(".closing") %></p>
2 changes: 1 addition & 1 deletion app/views/clearance_mailer/change_password.text.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<%= t(".opening") %>
<%= edit_user_password_url(@user, token: @user.confirmation_token.html_safe) %>
<%= edit_user_password_url(@user, token: @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(@user, token: params[:token]),
html: { method: :patch } do |form| %>
<div class="password-field">
<%= form.label :password %>
<%= form.password_field :password %>
Expand Down
2 changes: 2 additions & 0 deletions config/locales/clearance.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ en:
failure_when_forbidden: Please double check the URL or try submitting
the form again.
failure_when_not_signed_in: Please sign in to continue.
failure_when_password_reset_invalid: Your password reset token has expired
or is invalid.
helpers:
label:
password:
Expand Down
1 change: 0 additions & 1 deletion db/migrate/20110111224543_create_clearance_users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ def self.up
t.timestamps 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
3 changes: 1 addition & 2 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@

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

create_table "users", force: true do |t|
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
15 changes: 15 additions & 0 deletions lib/clearance/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,17 @@ class Configuration
# @return [String]
attr_accessor :mailer_sender

# Used to generate and verify password reset tokens
# Defaults to an `ActiveSupport::MessageVerifier` instance, initialized
# with `secret_key_base`.
# @return [#generate #verify]
attr_accessor :message_verifier

# Determines how long password reset emails are valid for
# Defaults to 15 minutes
# @return [Integer]
attr_accessor :password_reset_time_limit

# The password strategy to use when authenticating and setting passwords.
# Defaults to {Clearance::PasswordStrategies::BCrypt}.
# @return [Module #authenticated? #password=]
Expand Down Expand Up @@ -93,6 +104,10 @@ def initialize
@cookie_name = "remember_token"
@httponly = false
@mailer_sender = '[email protected]'
@message_verifier = ActiveSupport::MessageVerifier.new(
Rails.application.secrets.secret_key_base,
)
@password_reset_time_limit = 15.minutes
@redirect_url = '/'
@routes = true
@secure_cookie = false
Expand Down
31 changes: 1 addition & 30 deletions lib/clearance/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@ module Clearance
# @return [String] The value used to identify this user in their {Session}
# cookie.
#
# @!attribute confirmation_token
# @return [String] The value used to identify this user in the password
# reset link.
#
# @!attribute [r] password
# @return [String] Transient (non-persisted) attribute that is set when
# updating a user's password. Only the {#encrypted_password} is persisted.
Expand Down Expand Up @@ -159,20 +155,6 @@ module Callbacks
end
end

# Generates a {#confirmation_token} for the user, which allows them to reset
# their password via an email link.
#
# Calling `forgot_password!` will cause the user model to be saved without
# validations. Any other changes you made to this user instance will also
# be persisted, without validation. It is inteded to be called on an
# instance with no changes (`dirty? == false`).
#
# @return [Boolean] Was the save successful?
def forgot_password!
generate_confirmation_token
save validate: false
end

# Generates a new {#remember_token} for the user, which will have the effect
# of signing all of the user's current sessions out. This is called
# internally by {Session#sign_out}.
Expand All @@ -192,8 +174,7 @@ def reset_remember_token!
# the configured password strategy. By default, this is
# {PasswordStrategies::BCrypt#password=}.
#
# This also has the side-effect of blanking the {#confirmation_token} and
# rotating the `#remember_token`.
# This also has the side-effect of rotating the `#remember_token`.
#
# Validations will be run as part of this update. If the user instance is
# not valid, the password change will not be persisted, and this method will
Expand All @@ -204,7 +185,6 @@ def update_password(new_password)
self.password = new_password

if valid?
self.confirmation_token = nil
generate_remember_token
end

Expand Down Expand Up @@ -246,15 +226,6 @@ def skip_password_validation?
(encrypted_password.present? && !encrypted_password_changed?)
end

# Sets the {#confirmation_token} on the instance to a new value generated by
# {Token.new}. The change is not automatically persisted. If you would like
# to generate and save in a single method call, use {#forgot_password!}.
#
# @return [String] The new confirmation token
def generate_confirmation_token
self.confirmation_token = Clearance::Token.new
end

# Sets the {#remember_token} on the instance to a new value generated by
# {Token.new}. The change is not automatically persisted. If you would like
# to generate and save in a single method call, use
Expand Down
1 change: 0 additions & 1 deletion lib/generators/clearance/install/install_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ def new_columns
@new_columns ||= {
email: 't.string :email',
encrypted_password: 't.string :encrypted_password, limit: 128',
confirmation_token: 't.string :confirmation_token, limit: 128',
remember_token: 't.string :remember_token, limit: 128'
}.reject { |column| existing_users_columns.include?(column.to_s) }
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@ require "support/features/clearance_helpers"

RSpec.feature "Visitor resets password" do
before { ActionMailer::Base.deliveries.clear }
<% if defined?(ActiveJob) -%>

around do |example|
original_adapter = ActiveJob::Base.queue_adapter
ActiveJob::Base.queue_adapter = :inline
example.run
ActiveJob::Base.queue_adapter = original_adapter
end
<% end -%>

scenario "by navigating to the page" do
visit sign_in_path
Expand All @@ -22,7 +20,8 @@ RSpec.feature "Visitor resets password" do
end

scenario "with valid email" do
user = user_with_reset_password
user = FactoryGirl.create(:user)
reset_password_for(user.email)

expect_page_to_display_change_password_message
expect_reset_notification_to_be_sent_to user
Expand All @@ -38,26 +37,18 @@ RSpec.feature "Visitor resets password" do
private

def expect_reset_notification_to_be_sent_to(user)
expect(user.confirmation_token).not_to be_blank
expect_mailer_to_have_delivery(
user.email,
"password",
user.confirmation_token
)
expect_mailer_to_have_delivery(user.email, "password")
end

def expect_page_to_display_change_password_message
expect(page).to have_content I18n.t("passwords.create.description")
end

def expect_mailer_to_have_delivery(recipient, subject, body)
def expect_mailer_to_have_delivery(recipient, subject)
expect(ActionMailer::Base.deliveries).not_to be_empty

message = ActionMailer::Base.deliveries.any? do |email|
email.to == [recipient] &&
email.subject =~ /#{subject}/i &&
email.html_part.body =~ /#{body}/ &&
email.text_part.body =~ /#{body}/
email.to == [recipient] && email.subject =~ /#{subject}/i
end

expect(message).to be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ require "support/features/clearance_helpers"

RSpec.feature "Visitor updates password" do
scenario "with valid password" do
user = user_with_reset_password
user = FactoryGirl.create(:user)
update_password user, "newpassword"

expect_user_to_be_signed_in
end

scenario "signs in with new password" do
user = user_with_reset_password
user = FactoryGirl.create(:user)
update_password user, "newpassword"
sign_out
sign_in_with user.email, "newpassword"
Expand All @@ -19,7 +19,7 @@ RSpec.feature "Visitor updates password" do
end

scenario "tries with a blank password" do
user = user_with_reset_password
user = FactoryGirl.create(:user)
visit_password_reset_page_for user
change_password_to ""

Expand All @@ -37,10 +37,18 @@ RSpec.feature "Visitor updates password" do
def visit_password_reset_page_for(user)
visit edit_user_password_path(
user_id: user,
token: user.confirmation_token
token: token_for(user)
)
end

def token_for(user)
Clearance.configuration.message_verifier.generate([
user.id,
user.encrypted_password,
Clearance.configuration.password_reset_time_limit.from_now,
])
end

def change_password_to(password)
fill_in "password_reset_password", with: password
click_button I18n.t("helpers.submit.password_reset.submit")
Expand Down
Loading

0 comments on commit 9a6185f

Please sign in to comment.