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

Add expiring, databaseless password reset tokens #682

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 2 additions & 21 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,13 @@ language:
- ruby

rvm:
- 1.9.3
- 2.0.0
- 2.1.8
- 2.2.4
- 2.3.0
- 2.2.5
- 2.3.1

gemfile:
- gemfiles/rails32.gemfile
- gemfiles/rails40.gemfile
- gemfiles/rails41.gemfile
- gemfiles/rails42.gemfile
- gemfiles/rails50.gemfile

matrix:
exclude:
- rvm: 1.9.3
gemfile: gemfiles/rails50.gemfile
- rvm: 2.0.0
gemfile: gemfiles/rails50.gemfile
- rvm: 2.1.8
gemfile: gemfiles/rails50.gemfile
- rvm: 2.2.4
gemfile: gemfiles/rails32.gemfile
- rvm: 2.3.0
gemfile: gemfiles/rails32.gemfile

install:
- "bin/setup"

Expand Down
27 changes: 3 additions & 24 deletions Appraisals
Original file line number Diff line number Diff line change
@@ -1,29 +1,8 @@
if RUBY_VERSION < "2.2.0"
appraise 'rails32' do
gem 'rails', '~> 3.2.21'
end
end

appraise 'rails40' do
gem 'rails', '~> 4.0.13'
gem 'test-unit'
gem 'mime-types', '~> 2.99'
end

appraise 'rails41' do
gem 'rails', '~> 4.1.9'
gem 'mime-types', '~> 2.99'
end

appraise 'rails42' do
gem 'rails', '~> 4.2.0'
gem 'mime-types', '~> 2.99'
end

if RUBY_VERSION >= "2.2.0"
appraise "rails50" do
gem "rails", "~> 5.0.0.beta3"
gem "rails-controller-testing"
gem "rspec-rails", "~> 3.5.0.beta1"
end
appraise "rails50" do
gem "rails", "~> 5.0.0.beta3"
gem "rails-controller-testing"
end
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ gem 'bundler', '~> 1.3'
gem 'capybara', '>= 2.6.2'
gem 'database_cleaner', '~> 1.0'
gem 'factory_girl_rails', '~> 4.2'
gem 'rspec-rails', '~> 3.1'
gem 'rspec-rails', '~> 3.5.0.beta3'
gem 'shoulda-matchers', '~> 2.8'
gem 'sqlite3', '~> 1.3'
gem 'timecop', '~> 0.6'
Expand Down
36 changes: 18 additions & 18 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
PATH
remote: .
specs:
clearance (1.14.1)
clearance (2.0.0.alpha)
bcrypt
email_validator (~> 1.4)
rails (>= 3.1)
rails (>= 4.2)

GEM
remote: https://rubygems.org/
Expand Down Expand Up @@ -123,23 +123,23 @@ GEM
rake (>= 0.8.7)
thor (>= 0.18.1, < 2.0)
rake (11.1.2)
rspec-core (3.4.4)
rspec-support (~> 3.4.0)
rspec-expectations (3.4.0)
rspec-core (3.5.0.beta3)
rspec-support (= 3.5.0.beta3)
rspec-expectations (3.5.0.beta3)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.4.0)
rspec-mocks (3.4.1)
rspec-support (= 3.5.0.beta3)
rspec-mocks (3.5.0.beta3)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.4.0)
rspec-rails (3.4.2)
actionpack (>= 3.0, < 4.3)
activesupport (>= 3.0, < 4.3)
railties (>= 3.0, < 4.3)
rspec-core (~> 3.4.0)
rspec-expectations (~> 3.4.0)
rspec-mocks (~> 3.4.0)
rspec-support (~> 3.4.0)
rspec-support (3.4.1)
rspec-support (= 3.5.0.beta3)
rspec-rails (3.5.0.beta3)
actionpack (>= 3.0)
activesupport (>= 3.0)
railties (>= 3.0)
rspec-core (= 3.5.0.beta3)
rspec-expectations (= 3.5.0.beta3)
rspec-mocks (= 3.5.0.beta3)
rspec-support (= 3.5.0.beta3)
rspec-support (3.5.0.beta3)
shoulda-matchers (2.8.0)
activesupport (>= 3.0.0)
slop (3.6.0)
Expand Down Expand Up @@ -171,7 +171,7 @@ DEPENDENCIES
database_cleaner (~> 1.0)
factory_girl_rails (~> 4.2)
pry
rspec-rails (~> 3.1)
rspec-rails (~> 3.5.0.beta3)
shoulda-matchers (~> 2.8)
sqlite3 (~> 1.3)
timecop (~> 0.6)
Expand Down
23 changes: 23 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,29 @@
The noteworthy changes for each Clearance version are included here. For a
complete changelog, see the git history for each version via the version links.

## [2.0.0] - Unreleased

### Removed
- 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

### Fixed
Expand Down
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ Clearance.configure do |config|
config.routes = true
config.httponly = false
config.mailer_sender = "[email protected]"
config.message_verifier = ActiveSupport::MessageVerifier.new(secret_key_base)
config.password_reset_time_limit = 15.minutes
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice.

config.password_strategy = Clearance::PasswordStrategies::BCrypt
config.redirect_url = "/"
config.secure_cookie = false
Expand Down Expand Up @@ -124,6 +126,15 @@ Clearance.configure do |config|
end
```

The password reset link contained in the email is configured to expire in 15
minutes. You can change this with the `password_reset_time_limit` configuration.

```ruby
Clearance.configure do |config|
config.password_reset_time_limit = 1.hour
end
```

### Integrating with Rack Applications

Clearance adds its session to the Rack environment hash so middleware and other
Expand Down
91 changes: 24 additions & 67 deletions app/controllers/clearance/passwords_controller.rb
Original file line number Diff line number Diff line change
@@ -1,115 +1,72 @@
require 'active_support/deprecation'

class Clearance::PasswordsController < Clearance::BaseController
if respond_to?(:before_action)
skip_before_action :require_login,
only: [:create, :edit, :new, :update],
raise: false
skip_before_action :authorize,
only: [:create, :edit, :new, :update],
raise: false
before_action :ensure_existing_user, only: [:edit, :update]
else
skip_before_filter :require_login,
only: [:create, :edit, :new, :update],
raise: false
skip_before_filter :authorize,
only: [:create, :edit, :new, :update],
raise: false
before_filter :ensure_existing_user, only: [:edit, :update]
before_action :ensure_existing_user, only: [:edit, :update]
skip_before_action :require_login, only: [:create, :edit, :new, :update], raise: false

def new
render template: "passwords/new"
end

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

def edit
@user = find_user_for_edit
render template: 'passwords/edit'
render template: "passwords/create"
end

def new
render template: 'passwords/new'
def edit
@user = find_user_by_password_reset_token(params[:token])
render template: "passwords/edit"
end

def update
@user = find_user_for_update
@user = find_user_by_password_reset_token(params[:token])

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes!

else
flash_failure_after_update
render template: 'passwords/edit'
render template: "passwords/edit"
end
end

private

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

if mail.respond_to?(:deliver_later)
mail.deliver_later
else
mail.deliver
end
::ClearanceMailer.change_password(user).deliver_later
end

def password_reset_params
if params.has_key? :user
ActiveSupport::Deprecation.warn %{Since locales functionality was added, accessing params[:user] is no longer supported.}
params[:user][:password]
else
params[:password_reset][:password]
end
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
params[:password_reset][:password]
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
def find_user_by_password_reset_token(token)
@user ||= Clearance::PasswordResetToken.new(token).user
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[:notice] = 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
flash.now[:notice] = translate(:blank_password,
flash.now[:alert] = translate(:blank_password,
scope: [:clearance, :controllers, :passwords],
default: t('flashes.failure_after_update'))
end

def url_after_create
sign_in_url
default: t("flashes.failure_after_update"))
end

def url_after_update
Expand Down
21 changes: 3 additions & 18 deletions app/controllers/clearance/sessions_controller.rb
Original file line number Diff line number Diff line change
@@ -1,21 +1,6 @@
class Clearance::SessionsController < Clearance::BaseController
if respond_to?(:before_action)
before_action :redirect_signed_in_users, only: [:new]
skip_before_action :require_login,
only: [:create, :new, :destroy],
raise: false
skip_before_action :authorize,
only: [:create, :new, :destroy],
raise: false
else
before_filter :redirect_signed_in_users, only: [:new]
skip_before_filter :require_login,
only: [:create, :new, :destroy],
raise: false
skip_before_filter :authorize,
only: [:create, :new, :destroy],
raise: false
end
before_action :redirect_signed_in_users, only: [:new]
skip_before_action :require_login, only: [:create, :new, :destroy], raise: false

def create
@user = authenticate(params)
Expand All @@ -24,7 +9,7 @@ def create
if status.success?
redirect_back_or url_after_create
else
flash.now.notice = status.failure_message
flash.now.alert = status.failure_message
render template: "sessions/new", status: :unauthorized
end
end
Expand Down
19 changes: 2 additions & 17 deletions app/controllers/clearance/users_controller.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,6 @@
class Clearance::UsersController < Clearance::BaseController
if respond_to?(:before_action)
before_action :redirect_signed_in_users, only: [:create, :new]
skip_before_action :require_login, only: [:create, :new], raise: false
skip_before_action :authorize, only: [:create, :new], raise: false
else
before_filter :redirect_signed_in_users, only: [:create, :new]
skip_before_filter :require_login, only: [:create, :new], raise: false
skip_before_filter :authorize, only: [:create, :new], raise: false
end
before_action :redirect_signed_in_users, only: [:create, :new]
skip_before_action :require_login, only: [:create, :new], raise: false

def new
@user = user_from_params
Expand All @@ -27,14 +20,6 @@ def create

private

def avoid_sign_in
warn "[DEPRECATION] Clearance's `avoid_sign_in` before_filter is " +
"deprecated. Use `redirect_signed_in_users` instead. " +
"Be sure to update any instances of `skip_before_filter :avoid_sign_in`" +
" or `skip_before_action :avoid_sign_in` as well"
redirect_signed_in_users
end

def redirect_signed_in_users
if signed_in?
redirect_to Clearance.configuration.redirect_url
Expand Down
Loading