diff --git a/.travis.yml b/.travis.yml index b5f0baaa2..8a60a1a2d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -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" diff --git a/Appraisals b/Appraisals index 3e85e1ec1..f70c6284e 100644 --- a/Appraisals +++ b/Appraisals @@ -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 diff --git a/Gemfile b/Gemfile index 463f11973..241285669 100644 --- a/Gemfile +++ b/Gemfile @@ -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' diff --git a/Gemfile.lock b/Gemfile.lock index 598a893cb..e0c60208e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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/ @@ -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) @@ -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) diff --git a/NEWS.md b/NEWS.md index e8cc34c9c..0f6581768 100644 --- a/NEWS.md +++ b/NEWS.md @@ -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 diff --git a/README.md b/README.md index c14253cbc..03c42d564 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,8 @@ Clearance.configure do |config| config.routes = true config.httponly = false config.mailer_sender = "reply@example.com" + config.message_verifier = ActiveSupport::MessageVerifier.new(secret_key_base) + config.password_reset_time_limit = 15.minutes config.password_strategy = Clearance::PasswordStrategies::BCrypt config.redirect_url = "/" config.secure_cookie = false @@ -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 diff --git a/app/controllers/clearance/passwords_controller.rb b/app/controllers/clearance/passwords_controller.rb index 897065011..fb8205735 100644 --- a/app/controllers/clearance/passwords_controller.rb +++ b/app/controllers/clearance/passwords_controller.rb @@ -1,79 +1,46 @@ 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) 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 @@ -81,35 +48,25 @@ def find_user_for_create 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 diff --git a/app/controllers/clearance/sessions_controller.rb b/app/controllers/clearance/sessions_controller.rb index 7243ec08d..90b5dd187 100644 --- a/app/controllers/clearance/sessions_controller.rb +++ b/app/controllers/clearance/sessions_controller.rb @@ -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) @@ -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 diff --git a/app/controllers/clearance/users_controller.rb b/app/controllers/clearance/users_controller.rb index 72be00a26..373ef7dde 100644 --- a/app/controllers/clearance/users_controller.rb +++ b/app/controllers/clearance/users_controller.rb @@ -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 @@ -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 diff --git a/app/mailers/clearance_mailer.rb b/app/mailers/clearance_mailer.rb index ffd91aa01..65adbe123 100644 --- a/app/mailers/clearance_mailer.rb +++ b/app/mailers/clearance_mailer.rb @@ -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, @@ -10,4 +12,10 @@ def change_password(user) ), ) end + + private + + def generate_password_reset_token(user) + Clearance::PasswordResetToken.generate_for(user) + end end diff --git a/app/views/clearance_mailer/change_password.html.erb b/app/views/clearance_mailer/change_password.html.erb index bc315ecba..ed96e1b4c 100644 --- a/app/views/clearance_mailer/change_password.html.erb +++ b/app/views/clearance_mailer/change_password.html.erb @@ -2,7 +2,7 @@

<%= 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) %>

<%= raw t(".closing") %>

diff --git a/app/views/clearance_mailer/change_password.text.erb b/app/views/clearance_mailer/change_password.text.erb index ed01f74b5..39231c289 100644 --- a/app/views/clearance_mailer/change_password.text.erb +++ b/app/views/clearance_mailer/change_password.text.erb @@ -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") %> diff --git a/app/views/passwords/edit.html.erb b/app/views/passwords/edit.html.erb index 6761ab321..d924a83ac 100644 --- a/app/views/passwords/edit.html.erb +++ b/app/views/passwords/edit.html.erb @@ -4,8 +4,8 @@

<%= t(".description") %>

<%= 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| %>
<%= form.label :password %> <%= form.password_field :password %> diff --git a/clearance.gemspec b/clearance.gemspec index a842493ac..8b9c5c742 100644 --- a/clearance.gemspec +++ b/clearance.gemspec @@ -5,7 +5,7 @@ require 'date' Gem::Specification.new do |s| s.add_dependency 'bcrypt' s.add_dependency 'email_validator', '~> 1.4' - s.add_dependency 'rails', '>= 3.1' + s.add_dependency 'rails', '>= 4.2' s.authors = [ 'Dan Croak', 'Eugene Bolshakov', @@ -35,7 +35,7 @@ Gem::Specification.new do |s| s.name = %q{clearance} s.rdoc_options = ['--charset=UTF-8'] s.require_paths = ['lib'] - s.required_ruby_version = Gem::Requirement.new('>= 1.9.2') + s.required_ruby_version = Gem::Requirement.new('>= 2.2.0') s.summary = 'Rails authentication & authorization with email & password.' s.test_files = `git ls-files -- {spec}/*`.split("\n") s.version = Clearance::VERSION diff --git a/config/locales/clearance.en.yml b/config/locales/clearance.en.yml index da8de0daa..2068c0c9d 100644 --- a/config/locales/clearance.en.yml +++ b/config/locales/clearance.en.yml @@ -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: diff --git a/db/migrate/20110111224543_create_clearance_users.rb b/db/migrate/20110111224543_create_clearance_users.rb index 75e22bfee..11106ec5c 100644 --- a/db/migrate/20110111224543_create_clearance_users.rb +++ b/db/migrate/20110111224543_create_clearance_users.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index bf7e2f009..ff34b8c90 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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 diff --git a/gemfiles/rails32.gemfile b/gemfiles/rails32.gemfile deleted file mode 100644 index 6c4115354..000000000 --- a/gemfiles/rails32.gemfile +++ /dev/null @@ -1,18 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "appraisal", "~> 1.0" -gem "ammeter" -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 "shoulda-matchers", "~> 2.8" -gem "sqlite3", "~> 1.3" -gem "timecop", "~> 0.6" -gem "pry", :require => false -gem "rails", "~> 3.2.21" - -gemspec :path => "../" diff --git a/gemfiles/rails40.gemfile b/gemfiles/rails40.gemfile deleted file mode 100644 index 6468184b5..000000000 --- a/gemfiles/rails40.gemfile +++ /dev/null @@ -1,20 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "appraisal" -gem "ammeter" -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 "shoulda-matchers", "~> 2.8" -gem "sqlite3", "~> 1.3" -gem "timecop", "~> 0.6" -gem "pry", :require => false -gem "rails", "~> 4.0.13" -gem "test-unit" -gem "mime-types", "~> 2.99" - -gemspec :path => "../" diff --git a/gemfiles/rails41.gemfile b/gemfiles/rails41.gemfile deleted file mode 100644 index e30f73d8b..000000000 --- a/gemfiles/rails41.gemfile +++ /dev/null @@ -1,19 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "appraisal" -gem "ammeter" -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 "shoulda-matchers", "~> 2.8" -gem "sqlite3", "~> 1.3" -gem "timecop", "~> 0.6" -gem "pry", :require => false -gem "rails", "~> 4.1.9" -gem "mime-types", "~> 2.99" - -gemspec :path => "../" diff --git a/gemfiles/rails42.gemfile b/gemfiles/rails42.gemfile index 8b8a99ac6..533dd767e 100644 --- a/gemfiles/rails42.gemfile +++ b/gemfiles/rails42.gemfile @@ -8,12 +8,11 @@ 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" gem "pry", :require => false gem "rails", "~> 4.2.0" -gem "mime-types", "~> 2.99" gemspec :path => "../" diff --git a/gemfiles/rails50.gemfile b/gemfiles/rails50.gemfile index db992a3f6..3b55babd5 100644 --- a/gemfiles/rails50.gemfile +++ b/gemfiles/rails50.gemfile @@ -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.5.0.beta1" +gem "rspec-rails", "~> 3.5.0.beta3" gem "shoulda-matchers", "~> 2.8" gem "sqlite3", "~> 1.3" gem "timecop", "~> 0.6" diff --git a/lib/clearance.rb b/lib/clearance.rb index c72680443..62d03a508 100644 --- a/lib/clearance.rb +++ b/lib/clearance.rb @@ -8,14 +8,7 @@ require 'clearance/engine' require 'clearance/password_strategies' require 'clearance/constraints' +require "clearance/password_reset_token" module Clearance - # @deprecated Use `Gem::Specification` API if you need to access Clearance's - # Gem root. - def self.root - warn "#{Kernel.caller.first}: [DEPRECATION] `Clearance.root` is " + - "deprecated and will be removed in the next major release. If you need " + - "to find Clearance's root, you can use the `Gem::Specification` API." - File.expand_path('../..', __FILE__) - end end diff --git a/lib/clearance/authentication.rb b/lib/clearance/authentication.rb index 4541db7de..2027fb869 100644 --- a/lib/clearance/authentication.rb +++ b/lib/clearance/authentication.rb @@ -7,7 +7,6 @@ module Authentication private( :authenticate, :current_user, - :current_user=, :handle_unverified_request, :sign_in, :sign_out, @@ -37,13 +36,6 @@ def current_user clearance_session.current_user end - # @deprecated Use the {#sign_in} method instead. - def current_user=(user) - warn "#{Kernel.caller.first}: [DEPRECATION] " + - 'Assigning the current_user has been deprecated. Use the sign_in method instead.' - clearance_session.sign_in user - end - # Sign in the provided user. # @param [User] user # diff --git a/lib/clearance/authorization.rb b/lib/clearance/authorization.rb index 8628b8408..e6c9337fd 100644 --- a/lib/clearance/authorization.rb +++ b/lib/clearance/authorization.rb @@ -3,7 +3,7 @@ module Authorization extend ActiveSupport::Concern included do - private :authorize, :deny_access, :require_login + private :deny_access, :require_login end # Use as a `before_action` to require a user be signed in to proceed. @@ -23,15 +23,6 @@ def require_login end end - # @deprecated use {#require_login} - def authorize - warn "[DEPRECATION] Clearance's `authorize` before_action is " + - "deprecated. Use `require_login` instead. Be sure to update any " + - "instances of `skip_before_action :authorize` or " + - "`skip_before_action :authorize` as well" - require_login - end - # Responds to unauthorized requests in a manner fitting the request format. # `js`, `json`, and `xml` requests will receive a 401 with no body. All # other formats will be redirected appropriately and can optionally have the @@ -63,7 +54,7 @@ def redirect_request(flash_message) store_location if flash_message - flash[:notice] = flash_message + flash[:alert] = flash_message end if signed_in? diff --git a/lib/clearance/configuration.rb b/lib/clearance/configuration.rb index 32c5bfbd2..53b5a9cbc 100644 --- a/lib/clearance/configuration.rb +++ b/lib/clearance/configuration.rb @@ -47,6 +47,18 @@ class Configuration # @return [String] attr_accessor :mailer_sender + # Used to generate and verify password reset tokens + # Defaults to an `ActiveSupport::MessageVerifier` instance, initialized + # in concert with your application's `secret_key_base` via + # `Rails.application.message_verifier`. + # @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=] @@ -93,6 +105,8 @@ def initialize @cookie_name = "remember_token" @httponly = false @mailer_sender = 'reply@example.com' + @message_verifier = Rails.application.message_verifier("clearance") + @password_reset_time_limit = 15.minutes @redirect_url = '/' @routes = true @secure_cookie = false diff --git a/lib/clearance/password_reset_token.rb b/lib/clearance/password_reset_token.rb new file mode 100644 index 000000000..1adb65cfe --- /dev/null +++ b/lib/clearance/password_reset_token.rb @@ -0,0 +1,43 @@ +module Clearance + class PasswordResetToken + def self.generate_for(user) + token = Clearance.configuration.message_verifier.generate([ + user.id, + Digest::MD5.hexdigest(user.encrypted_password), + Clearance.configuration.password_reset_time_limit.from_now + ]) + + new(token) + end + + def initialize(token) + @token = token.to_s + end + + def user + user_id, digested_secret, expires_at = verify + + if expires_at.future? + user = Clearance.configuration.user_model.find_by(id: user_id) + + if digested_secret == Digest::MD5.hexdigest(user.encrypted_password) + user + end + end + end + + def to_s + token + end + + private + + attr_reader :token + + def verify + Clearance.configuration.message_verifier.verify(token) + rescue ActiveSupport::MessageVerifier::InvalidSignature + [nil, nil, 1.hour.ago] + end + end +end diff --git a/lib/clearance/password_strategies/bcrypt_migration_from_sha1.rb b/lib/clearance/password_strategies/bcrypt_migration_from_sha1.rb deleted file mode 100644 index f4c718ca1..000000000 --- a/lib/clearance/password_strategies/bcrypt_migration_from_sha1.rb +++ /dev/null @@ -1,77 +0,0 @@ -module Clearance - module PasswordStrategies - # @deprecated Use {BCrypt} or `clearance-deprecated_password_strategies` gem - module BCryptMigrationFromSHA1 - DEPRECATION_MESSAGE = "[DEPRECATION] The BCryptMigrationFromSha1 " \ - "password strategy has been deprecated and will be removed from " \ - "Clearance 2.0. BCrypt is the only officially supported strategy, " \ - "though you are free to provide your own. To continue using this " \ - "strategy, add clearance-deprecated_password_strategies to your " \ - "Gemfile." - - # @api private - class BCryptUser - include Clearance::PasswordStrategies::BCrypt - - def initialize(user) - @user = user - end - - delegate :encrypted_password, :encrypted_password=, to: :@user - end - - # @api private - class SHA1User - include Clearance::PasswordStrategies::SHA1 - - def initialize(user) - @user = user - end - - delegate :salt, :salt=, :encrypted_password, :encrypted_password=, to: :@user - end - - # @deprecated Use {BCrypt} or `clearance-deprecated_password_strategies` - # gem - def authenticated?(password) - warn "#{Kernel.caller.first}: #{DEPRECATION_MESSAGE}" - authenticated_with_sha1?(password) || authenticated_with_bcrypt?(password) - end - - # @deprecated Use {BCrypt} or `clearance-deprecated_password_strategies` - # gem - def password=(new_password) - warn "#{Kernel.caller.first}: #{DEPRECATION_MESSAGE}" - @password = new_password - BCryptUser.new(self).password = new_password - end - - private - - # @api private - def authenticated_with_bcrypt?(password) - begin - BCryptUser.new(self).authenticated? password - rescue ::BCrypt::Errors::InvalidHash - false - end - end - - # @api private - def authenticated_with_sha1?(password) - if sha1_password? - if SHA1User.new(self).authenticated? password - self.password = password - self.save - true - end - end - end - - # @api private - def sha1_password? - self.encrypted_password =~ /^[a-f0-9]{40}$/ - end - end - end -end diff --git a/lib/clearance/password_strategies/blowfish.rb b/lib/clearance/password_strategies/blowfish.rb deleted file mode 100644 index 9dbc8dabc..000000000 --- a/lib/clearance/password_strategies/blowfish.rb +++ /dev/null @@ -1,61 +0,0 @@ -require 'openssl' -require 'base64' - -module Clearance - module PasswordStrategies - # @deprecated Use {BCrypt} or `clearance-deprecated_password_strategies` gem - module Blowfish - DEPRECATION_MESSAGE = "[DEPRECATION] The Blowfish password strategy " \ - "has been deprecated and will be removed from Clearance 2.0. BCrypt " \ - "is the only officially supported strategy, though you are free to " \ - "provide your own. To continue using this strategy add " \ - "clearance-deprecated_password_strategies to your Gemfile." - - # @deprecated Use {BCrypt} or `clearance-deprecated_password_strategies` - # gem - def authenticated?(password) - warn "#{Kernel.caller.first}: #{DEPRECATION_MESSAGE}" - encrypted_password == encrypt(password) - end - - # @deprecated Use {BCrypt} or `clearance-deprecated_password_strategies` - # gem - def password=(new_password) - warn "#{Kernel.caller.first}: #{DEPRECATION_MESSAGE}" - @password = new_password - initialize_salt_if_necessary - - if new_password.present? - self.encrypted_password = encrypt(new_password) - end - end - - protected - - # @api private - def encrypt(string) - generate_hash("--#{salt}--#{string}--") - end - - # @api private - def generate_hash(string) - cipher = OpenSSL::Cipher::Cipher.new('bf-cbc').encrypt - cipher.key = Digest::SHA256.digest(salt) - hash = cipher.update(string) << cipher.final - Base64.encode64(hash).encode('utf-8') - end - - # @api private - def initialize_salt_if_necessary - if salt.blank? - self.salt = generate_salt - end - end - - # @api private - def generate_salt - Base64.encode64(SecureRandom.hex(20)).encode('utf-8') - end - end - end -end diff --git a/lib/clearance/password_strategies/sha1.rb b/lib/clearance/password_strategies/sha1.rb deleted file mode 100644 index 7a8564bb8..000000000 --- a/lib/clearance/password_strategies/sha1.rb +++ /dev/null @@ -1,59 +0,0 @@ -module Clearance - module PasswordStrategies - # @deprecated Use {BCrypt} or `clearance-deprecated_password_strategies` gem - module SHA1 - require 'digest/sha1' - - DEPRECATION_MESSAGE = "[DEPRECATION] The SHA1 password strategy " \ - "has been deprecated and will be removed from Clearance 2.0. BCrypt " \ - "is the only officially supported strategy, though you are free to " \ - "provide your own. To continue using this strategy add " \ - "clearance-deprecated_password_strategies to your Gemfile." - - extend ActiveSupport::Concern - - # @deprecated Use {BCrypt} or `clearance-deprecated_password_strategies` - # gem - def authenticated?(password) - warn "#{Kernel.caller.first}: #{DEPRECATION_MESSAGE}" - encrypted_password == encrypt(password) - end - - # @deprecated Use {BCrypt} or `clearance-deprecated_password_strategies` - # gem - def password=(new_password) - warn "#{Kernel.caller.first}: #{DEPRECATION_MESSAGE}" - @password = new_password - initialize_salt_if_necessary - - if new_password.present? - self.encrypted_password = encrypt(new_password) - end - end - - private - - # @api private - def encrypt(string) - generate_hash "--#{salt}--#{string}--" - end - - # @api private - def generate_hash(string) - Digest::SHA1.hexdigest(string).encode 'UTF-8' - end - - # @api private - def initialize_salt_if_necessary - if salt.blank? - self.salt = generate_salt - end - end - - # @api private - def generate_salt - SecureRandom.hex(20).encode('UTF-8') - end - end - end -end diff --git a/lib/clearance/session.rb b/lib/clearance/session.rb index ba77ea81a..87aef3380 100644 --- a/lib/clearance/session.rb +++ b/lib/clearance/session.rb @@ -108,15 +108,7 @@ def remember_token # @api private def remember_token_expires - if expires_configuration.arity == 1 - expires_configuration.call(cookies) - else - warn "#{Kernel.caller.first}: [DEPRECATION] " + - 'Clearance.configuration.cookie_expiration lambda with no parameters ' + - 'has been deprecated and will be removed from a future release. The ' + - 'lambda should accept the collection of previously set cookies.' - expires_configuration.call - end + expires_configuration.call(cookies) end # @api private diff --git a/lib/clearance/testing.rb b/lib/clearance/testing.rb deleted file mode 100644 index 93ef3f6c6..000000000 --- a/lib/clearance/testing.rb +++ /dev/null @@ -1,11 +0,0 @@ -if defined?(RSpec) - require 'clearance/rspec' -elsif defined?(ActionController::TestCase) - require 'clearance/test_unit' -end - -warn( - "#{Kernel.caller.first} [DEPRECATION] Requiring `clearance/testing` in " + - '`spec/spec_helper.rb` (or in `test/test_helper.rb`) is deprecated. ' + - 'Require `clearance/rspec` or `clearance/test_unit` instead.' -) diff --git a/lib/clearance/testing/deny_access_matcher.rb b/lib/clearance/testing/deny_access_matcher.rb index 7f19a0155..3e657f0e4 100644 --- a/lib/clearance/testing/deny_access_matcher.rb +++ b/lib/clearance/testing/deny_access_matcher.rb @@ -8,7 +8,7 @@ module Testing module Matchers # The `deny_access` matcher is used to assert that a # request is denied access by clearance. - # @option opts [String] :flash The expected flash notice message. Defaults + # @option opts [String] :flash The expected flash alert message. Defaults # to nil, which means the flash will not be checked. # @option opts [String] :redirect The expected redirect url. Defaults to # `'/'` if signed in or the `sign_in_url` if signed out. @@ -78,16 +78,12 @@ def clearance_session @controller.request.env[:clearance] end - def flash_notice - @controller.flash[:notice] + def flash_alert + @controller.flash[:alert] end - def flash_notice_value - if flash_notice.respond_to?(:values) - flash_notice.values.first - else - flash_notice - end + def flash_alert_value + flash_alert.values.first end def redirects_to_url? @@ -107,16 +103,14 @@ def redirects_to_url? def sets_the_flash? if @flash.blank? true + elsif flash_alert_value == @flash + @failure_message_when_negated << + "Didn't expect to set the flash to #{@flash}" + true else - if flash_notice_value == @flash - @failure_message_when_negated << - "Didn't expect to set the flash to #{@flash}" - true - else - @failure_message << "Expected the flash to be set to #{@flash} "\ - "but was #{flash_notice_value}" - false - end + @failure_message << "Expected the flash to be set to #{@flash} "\ + "but was #{flash_alert_value}" + false end end end diff --git a/lib/clearance/testing/helpers.rb b/lib/clearance/testing/helpers.rb deleted file mode 100644 index 719d2270a..000000000 --- a/lib/clearance/testing/helpers.rb +++ /dev/null @@ -1,15 +0,0 @@ -require "clerance/testing/controller_helpers" - -module Clearance - module Testing - # @deprecated Use Clearance::Testing::ControllerHelpers - module Helpers - warn( - "#{Kernel.caller.first} [DEPRECATION] Clearance::Testing::Helpers is "\ - "deprecated and has been replaced with " \ - "Clearance::Testing::ControllerHelpers. Require " \ - "clearance/testing/controller_helpers instead." - ) - end - end -end diff --git a/lib/clearance/user.rb b/lib/clearance/user.rb index a51ea27dc..efd3126ea 100644 --- a/lib/clearance/user.rb +++ b/lib/clearance/user.rb @@ -42,13 +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 password_changing - # @deprecated Dirty tracking is now handled automatically. - # # @!attribute [r] password # @return [String] Transient (non-persisted) attribute that is set when # updating a user's password. Only the {#encrypted_password} is persisted. @@ -110,24 +103,6 @@ def password=(value) encrypted_password_will_change! super end - - def password_changing - warn "#{Kernel.caller.first}: [DEPRECATION] " \ - "The `password_changing` attribute is deprecated. Clearance uses " \ - "the dirty state of the `encrypted_password` field to track this " \ - "automatically." - - @password_changing - end - - def password_changing=(value) - warn "#{Kernel.caller.first}: [DEPRECATION] " \ - "The `password_changing` attribute is deprecated. Clearance uses " \ - "the dirty state of the `encrypted_password` field to track this " \ - "automatically." - - @password_changing = value - end end # @api private @@ -180,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}. @@ -213,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 @@ -225,7 +185,6 @@ def update_password(new_password) self.password = new_password if valid? - self.confirmation_token = nil generate_remember_token end @@ -267,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 diff --git a/lib/clearance/version.rb b/lib/clearance/version.rb index 1e1c052be..808424c63 100644 --- a/lib/clearance/version.rb +++ b/lib/clearance/version.rb @@ -1,3 +1,3 @@ module Clearance - VERSION = "1.14.1".freeze + VERSION = "2.0.0.alpha".freeze end diff --git a/lib/generators/clearance/install/install_generator.rb b/lib/generators/clearance/install/install_generator.rb index d233a11f9..ef3e186b4 100644 --- a/lib/generators/clearance/install/install_generator.rb +++ b/lib/generators/clearance/install/install_generator.rb @@ -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 @@ -102,11 +101,7 @@ def migration_name_without_timestamp(file) end def users_table_exists? - if ActiveRecord::Base.connection.respond_to?(:data_source_exists?) - ActiveRecord::Base.connection.data_source_exists?(:users) - else - ActiveRecord::Base.connection.table_exists?(:users) - end + ActiveRecord::Base.connection.data_source_exists?(:users) end def existing_users_columns diff --git a/lib/generators/clearance/specs/templates/features/clearance/visitor_resets_password_spec.rb.tt b/lib/generators/clearance/specs/templates/features/clearance/visitor_resets_password_spec.rb.tt index 80deeba13..224619eba 100644 --- a/lib/generators/clearance/specs/templates/features/clearance/visitor_resets_password_spec.rb.tt +++ b/lib/generators/clearance/specs/templates/features/clearance/visitor_resets_password_spec.rb.tt @@ -3,7 +3,6 @@ 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 @@ -11,7 +10,6 @@ RSpec.feature "Visitor resets password" do example.run ActiveJob::Base.queue_adapter = original_adapter end -<% end -%> scenario "by navigating to the page" do visit sign_in_path @@ -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 @@ -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 diff --git a/lib/generators/clearance/specs/templates/features/clearance/visitor_updates_password_spec.rb.tt b/lib/generators/clearance/specs/templates/features/clearance/visitor_updates_password_spec.rb.tt index 4c7504436..9fd956c78 100644 --- a/lib/generators/clearance/specs/templates/features/clearance/visitor_updates_password_spec.rb.tt +++ b/lib/generators/clearance/specs/templates/features/clearance/visitor_updates_password_spec.rb.tt @@ -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" @@ -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 "" @@ -37,10 +37,14 @@ 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::PasswordResetToken.generate_for(user) + end + def change_password_to(password) fill_in "password_reset_password", with: password click_button I18n.t("helpers.submit.password_reset.submit") diff --git a/lib/generators/clearance/specs/templates/support/features/clearance_helpers.rb b/lib/generators/clearance/specs/templates/support/features/clearance_helpers.rb index 13540614c..5b1a2714a 100644 --- a/lib/generators/clearance/specs/templates/support/features/clearance_helpers.rb +++ b/lib/generators/clearance/specs/templates/support/features/clearance_helpers.rb @@ -38,12 +38,6 @@ def expect_user_to_be_signed_in def expect_user_to_be_signed_out expect(page).to have_content I18n.t("layouts.application.sign_in") end - - def user_with_reset_password - user = FactoryGirl.create(:user) - reset_password_for user.email - user.reload - end end end diff --git a/lib/generators/clearance/upgrade/USAGE b/lib/generators/clearance/upgrade/USAGE new file mode 100644 index 000000000..82497fca8 --- /dev/null +++ b/lib/generators/clearance/upgrade/USAGE @@ -0,0 +1,2 @@ +Description: + Prepare database for upgrade to Clearance 2.0 diff --git a/lib/generators/clearance/upgrade/templates/db/migrate/remove_confirmation_token_from_users.rb.tt b/lib/generators/clearance/upgrade/templates/db/migrate/remove_confirmation_token_from_users.rb.tt new file mode 100644 index 000000000..19675856c --- /dev/null +++ b/lib/generators/clearance/upgrade/templates/db/migrate/remove_confirmation_token_from_users.rb.tt @@ -0,0 +1,5 @@ +class RemoveConfirmationTokenFromUsers < ActiveRecord::Migration<%= migration_version %> + def change + remove_column :<%= Clearance.configuration.user_model.table_name %>, :confirmation_token, type: :string, limit: 128 + end +end diff --git a/lib/generators/clearance/upgrade/upgrade_generator.rb b/lib/generators/clearance/upgrade/upgrade_generator.rb new file mode 100644 index 000000000..6c2ec88bb --- /dev/null +++ b/lib/generators/clearance/upgrade/upgrade_generator.rb @@ -0,0 +1,34 @@ +require "rails/generators/base" +require "rails/generators/active_record" + +module Clearance + module Generators + class UpgradeGenerator < Rails::Generators::Base + include Rails::Generators::Migration + source_root File.expand_path("../templates", __FILE__) + + # for generating a timestamp when using `create_migration` + def self.next_migration_number(dir) + ActiveRecord::Generators::Base.next_migration_number(dir) + end + + def change_users_table + migration_name = "remove_confirmation_token_from_users.rb" + + migration_template( + "db/migrate/#{migration_name}.tt", + "db/migrate/#{migration_name}", + migration_version: migration_version, + ) + end + + private + + def migration_version + if Rails.version >= "5.0.0" + "[#{Rails::VERSION::MAJOR}.#{Rails::VERSION::MINOR}]" + end + end + end + end +end diff --git a/spec/clearance/password_reset_token_spec.rb b/spec/clearance/password_reset_token_spec.rb new file mode 100644 index 000000000..298c467f6 --- /dev/null +++ b/spec/clearance/password_reset_token_spec.rb @@ -0,0 +1,51 @@ +require "spec_helper" + +describe Clearance::PasswordResetToken do + describe ".generate_for" do + it "uses the configured message verifier to generate a token" do + Timecop.freeze do + user = double("User", id: 1, encrypted_password: "foo") + time_limit = Clearance.configuration.password_reset_time_limit + allow(Clearance.configuration.message_verifier).to receive(:generate). + with([1, Digest::MD5.hexdigest("foo"), time_limit.from_now]). + and_return("SEKRET") + + token = Clearance::PasswordResetToken.generate_for(user) + + expect(token.to_s).to eq "SEKRET" + end + end + end + + describe "#user" do + it "is the user if signature is valid and token is not expired" do + user = create(:user) + token = Clearance::PasswordResetToken.generate_for(user) + + expect(token.user).to eq user + end + + it "is nil if the signature is invalid" do + token = Clearance::PasswordResetToken.new("FOO") + + expect(token.user).to be nil + end + + it "is nil if the signature is valid but expired" do + user = double("User", id: 1, encrypted_password: "foo") + token = Clearance::PasswordResetToken.generate_for(user) + + Timecop.freeze(1.day.from_now) do + expect(token.user).to be nil + end + end + + it "is nil if the signature is valid an unexpired by password changed" do + user = create(:user, encrypted_password: "something") + token = Clearance::PasswordResetToken.generate_for(user) + user.update!(encrypted_password: "something_else") + + expect(token.user).to be nil + end + end +end diff --git a/spec/clearance/session_spec.rb b/spec/clearance/session_spec.rb index d77d32ec4..fca85e466 100644 --- a/spec/clearance/session_spec.rb +++ b/spec/clearance/session_spec.rb @@ -113,7 +113,6 @@ def stub_callable expect(session.current_user).to be_nil end - def stub_sign_in_guard(options) session_status = stub_status(options.fetch(:succeed)) @@ -191,36 +190,6 @@ def stub_status(success) end end - context 'configured with lambda taking no arguments' do - it 'logs a deprecation warning' do - expiration = -> { Time.now } - with_custom_expiration expiration do - session = Clearance::Session.new(env_without_remember_token) - allow(session).to receive(:warn) - session.add_cookie_to_headers headers - - expect(session).to have_received(:warn).once - end - end - - it 'is set to the value of the evaluated lambda' do - expires_at = -> { 1.day.from_now } - with_custom_expiration expires_at do - user = double("User", remember_token: "123abc") - headers = {} - session = Clearance::Session.new(env_without_remember_token) - session.sign_in user - allow(session).to receive(:warn) - session.add_cookie_to_headers headers - - expect(headers).to set_cookie( - 'remember_token', - user.remember_token, expires_at.call - ) - end - end - end - context 'configured with lambda taking one argument' do it 'it can use other cookies to set the value of the expires token' do remembered_expires = 12.hours.from_now diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 79ad9d89e..3026943f2 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -146,4 +146,35 @@ expect(Clearance.configuration.reload_user_model).to be_nil end end + + describe "#message_verifier" do + it "returns the configured verifier if one has been configured" do + verifier = Class.new.new + Clearance.configure { |config| config.message_verifier = verifier } + + expect(Clearance.configuration.message_verifier).to be verifier + end + + it "returns an active support message verifier instance by default" do + Clearance.configuration = Clearance::Configuration.new + + expect(Clearance.configuration.message_verifier).to be_an_instance_of( + ActiveSupport::MessageVerifier, + ) + end + end + + describe "#password_reset_time_limit" do + it "is the configured amount if overridden" do + Clearance.configure { |config| config.password_reset_time_limit = 1.hour } + + expect(Clearance.configuration.password_reset_time_limit).to eq 1.hour + end + + it "is 15 minutes by default" do + Clearance.configuration = Clearance::Configuration.new + + expect(Clearance.configuration.password_reset_time_limit).to eq 15.minutes + end + end end diff --git a/spec/controllers/apis_controller_spec.rb b/spec/controllers/apis_controller_spec.rb index 9afa2f2ac..abd254ff5 100644 --- a/spec/controllers/apis_controller_spec.rb +++ b/spec/controllers/apis_controller_spec.rb @@ -3,11 +3,7 @@ class ApisController < ActionController::Base include Clearance::Controller - if respond_to?(:before_action) - before_action :require_login - else - before_filter :require_login - end + before_action :require_login def show head :ok diff --git a/spec/controllers/forgeries_controller_spec.rb b/spec/controllers/forgeries_controller_spec.rb index abb8e5458..3252ebd86 100644 --- a/spec/controllers/forgeries_controller_spec.rb +++ b/spec/controllers/forgeries_controller_spec.rb @@ -5,11 +5,7 @@ class ForgeriesController < ActionController::Base protect_from_forgery - if respond_to?(:before_action) - before_action :require_login - else - before_filter :require_login - end + before_action :require_login # This is off in test by default, but we need it for this test self.allow_forgery_protection = true diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb index fd946dc4e..5b67fbf7e 100644 --- a/spec/controllers/passwords_controller_spec.rb +++ b/spec/controllers/passwords_controller_spec.rb @@ -16,14 +16,6 @@ describe "#create" do context "email corresponds to an existing user" do - it "generates a password change token" do - user = create(:user) - - post :create, password: { email: user.email.upcase } - - expect(user.reload.confirmation_token).not_to be_nil - end - it "sends the password reset email" do ActionMailer::Base.deliveries.clear user = create(:user) @@ -59,9 +51,9 @@ describe "#edit" do context "valid id and token are supplied" do it "renders the password form for the user" do - user = create(:user, :with_forgotten_password) + user = create(:user) - get :edit, user_id: user, token: user.confirmation_token + get :edit, user_id: user, token: token_for(user) expect(response).to be_success expect(response).to render_template(:edit) @@ -69,23 +61,12 @@ end end - context "blank token is supplied" do - it "renders the new password reset form with a flash notice" do - get :edit, user_id: 1, token: "" - - expect(response).to render_template(:new) - expect(flash.now[:notice]).to match(/double check the URL/i) - end - end - context "invalid token is supplied" do - it "renders the new password reset form with a flash notice" do - user = create(:user, :with_forgotten_password) - - get :edit, user_id: 1, token: user.confirmation_token + "a" + it "renders the new password reset form with a flash alert" do + get :edit, user_id: 1, token: "a" expect(response).to render_template(:new) - expect(flash.now[:notice]).to match(/double check the URL/i) + expect(flash.now[:alert]).to match(/expired or is invalid/i) end end end @@ -93,7 +74,7 @@ describe "#update" do context "valid id, token, and new password provided" do it "updates the user's password" do - user = create(:user, :with_forgotten_password) + user = create(:user) old_encrypted_password = user.encrypted_password put :update, update_parameters(user, new_password: "my_new_password") @@ -102,7 +83,7 @@ end it "signs the user in and redirects" do - user = create(:user, :with_forgotten_password) + user = create(:user) put :update, update_parameters(user, new_password: "my_new_password") @@ -113,22 +94,21 @@ context "password update fails" do it "does not update the password" do - user = create(:user, :with_forgotten_password) + user = create(:user) old_encrypted_password = user.encrypted_password put :update, update_parameters(user, new_password: "") user.reload expect(user.encrypted_password).to eq old_encrypted_password - expect(user.confirmation_token).to be_present end it "re-renders the password edit form" do - user = create(:user, :with_forgotten_password) + user = create(:user) put :update, update_parameters(user, new_password: "") - expect(flash.now[:notice]).to match(/password can't be blank/i) + expect(flash.now[:alert]).to match(/password can't be blank/i) expect(response).to render_template(:edit) expect(cookies[:remember_token]).to be_nil end @@ -140,8 +120,12 @@ def update_parameters(user, options = {}) { user_id: user, - token: user.confirmation_token, - password_reset: { password: new_password } + token: token_for(user), + password_reset: { password: new_password }, } end + + def token_for(user) + Clearance::PasswordResetToken.generate_for(user) + end end diff --git a/spec/controllers/permissions_controller_spec.rb b/spec/controllers/permissions_controller_spec.rb index 0791db4e0..736d07ad4 100644 --- a/spec/controllers/permissions_controller_spec.rb +++ b/spec/controllers/permissions_controller_spec.rb @@ -3,11 +3,7 @@ class PermissionsController < ActionController::Base include Clearance::Controller - if respond_to?(:before_action) - before_action :require_login, only: :show - else - before_filter :require_login, only: :show - end + before_action :require_login, only: :show def new head :ok @@ -62,7 +58,7 @@ def show it "denies access to show and display a flash message" do get :show - expect(flash[:notice]).to match(/^Please sign in to continue/) + expect(flash[:alert]).to match(/^Please sign in to continue/) end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 4db8bd7bc..302d9ef18 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -31,7 +31,7 @@ post :create, session: { email: user.email, password: user.password } expect(response).to render_template(:new) - expect(flash[:notice]).to match(/^Bad email or password/) + expect(flash[:alert]).to match(/^Bad email or password/) end end diff --git a/spec/dummy/application.rb b/spec/dummy/application.rb index fc8b3be96..278ee4604 100644 --- a/spec/dummy/application.rb +++ b/spec/dummy/application.rb @@ -3,11 +3,6 @@ module Dummy APP_ROOT = File.expand_path("..", __FILE__).freeze - - def self.rails4? - Rails::VERSION::MAJOR >= 4 - end - I18n.enforce_available_locales = true class Application < Rails::Application @@ -29,17 +24,9 @@ class Application < Rails::Application config.paths["config/database"] = "#{APP_ROOT}/config/database.yml" config.paths["log"] = "tmp/log/development.log" config.secret_token = "SECRET_TOKEN_IS_MIN_30_CHARS_LONG" - - if Dummy.rails4? - config.paths.add "config/routes.rb", with: "#{APP_ROOT}/config/routes.rb" - config.secret_key_base = "SECRET_KEY_BASE" - else - config.paths.add "config/routes", with: "#{APP_ROOT}/config/routes.rb" - end - - if config.respond_to?(:active_job) - config.active_job.queue_adapter = :inline - end + config.paths.add "config/routes.rb", with: "#{APP_ROOT}/config/routes.rb" + config.secret_key_base = "SECRET_KEY_BASE" + config.active_job.queue_adapter = :inline def require_environment! initialize! diff --git a/spec/factories.rb b/spec/factories.rb index 204620e6a..9ccab6cc1 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -7,10 +7,6 @@ email password 'password' - trait :with_forgotten_password do - confirmation_token Clearance::Token.new - end - factory :user_with_optional_password, class: 'UserWithOptionalPassword' do password nil encrypted_password '' diff --git a/spec/generators/clearance/install/install_generator_spec.rb b/spec/generators/clearance/install/install_generator_spec.rb index feecee590..9a9643240 100644 --- a/spec/generators/clearance/install/install_generator_spec.rb +++ b/spec/generators/clearance/install/install_generator_spec.rb @@ -118,16 +118,9 @@ def table_does_not_exist(name) connection = ActiveRecord::Base.connection - - if connection.respond_to?(:data_source_exists?) - allow(connection).to receive(:data_source_exists?). - with(name). - and_return(false) - else - allow(connection).to receive(:table_exists?). - with(name). - and_return(false) - end + allow(connection).to receive(:data_source_exists?). + with(name). + and_return(false) end def contain_models_inherit_from diff --git a/spec/generators/clearance/upgrade/upgrade_generator_spec.rb b/spec/generators/clearance/upgrade/upgrade_generator_spec.rb new file mode 100644 index 000000000..674092779 --- /dev/null +++ b/spec/generators/clearance/upgrade/upgrade_generator_spec.rb @@ -0,0 +1,16 @@ +require "spec_helper" +require "generators/clearance/upgrade/upgrade_generator" + +describe Clearance::Generators::UpgradeGenerator, :generator do + it "copies a database migration to the host application" do + run_generator + + migration = migration_file( + "db/migrate/remove_confirmation_token_from_users.rb", + ) + + expect(migration).to exist + expect(migration).to have_correct_syntax + expect(migration).to contain("remove_column :users, :confirmation_token") + end +end diff --git a/spec/mailers/clearance_mailer_spec.rb b/spec/mailers/clearance_mailer_spec.rb index 81fae81fd..c18d4cdbc 100644 --- a/spec/mailers/clearance_mailer_spec.rb +++ b/spec/mailers/clearance_mailer_spec.rb @@ -3,7 +3,6 @@ describe ClearanceMailer do it "is from DO_NOT_REPLY" do user = create(:user) - user.forgot_password! email = ClearanceMailer.change_password(user) @@ -12,7 +11,6 @@ it "is sent to user" do user = create(:user) - user.forgot_password! email = ClearanceMailer.change_password(user) @@ -21,7 +19,6 @@ it "sets its subject" do user = create(:user) - user.forgot_password! email = ClearanceMailer.change_password(user) @@ -30,7 +27,6 @@ it "has html and plain text parts" do user = create(:user) - user.forgot_password! email = ClearanceMailer.change_password(user) @@ -40,19 +36,23 @@ end it "contains a link to edit the password" do - user = create(:user) - user.forgot_password! - host = ActionMailer::Base.default_url_options[:host] - link = "http://#{host}/users/#{user.id}/password/edit" \ - "?token=#{user.confirmation_token}" - - email = ClearanceMailer.change_password(user) - - expect(email.text_part.body).to include(link) - expect(email.html_part.body).to include(link) - expect(email.html_part.body).to have_css( - "a", - text: I18n.t("clearance_mailer.change_password.link_text") - ) + Timecop.freeze do + user = create(:user) + host = ActionMailer::Base.default_url_options[:host] + allow(Clearance::PasswordResetToken).to receive(:generate_for). + with(user). + and_return("THIS_IS_THE_TOKEN") + link = "http://#{host}/users/#{user.id}/password/edit" \ + "?token=THIS_IS_THE_TOKEN" + + email = ClearanceMailer.change_password(user) + + expect(email.text_part.body).to include(link) + expect(email.html_part.body).to include(link) + expect(email.html_part.body).to have_css( + "a", + text: I18n.t("clearance_mailer.change_password.link_text"), + ) + end end end diff --git a/spec/password_strategies/bcrypt_migration_from_sha1_spec.rb b/spec/password_strategies/bcrypt_migration_from_sha1_spec.rb deleted file mode 100644 index 652708a2f..000000000 --- a/spec/password_strategies/bcrypt_migration_from_sha1_spec.rb +++ /dev/null @@ -1,122 +0,0 @@ -require "spec_helper" -include FakeModelWithPasswordStrategy - -describe Clearance::PasswordStrategies::BCryptMigrationFromSHA1 do - around do |example| - silence_warnings do - example.run - end - end - - describe "#password=" do - it "encrypts the password into a BCrypt-encrypted encrypted_password" do - stub_bcrypt_password - - expect(model_instance.encrypted_password).to eq encrypted_password - end - - it "encrypts with BCrypt" do - stub_bcrypt_password - - expect(BCrypt::Password).to have_received(:create). - with(password, anything) - end - - it "sets the pasword on the subject" do - stub_bcrypt_password - - expect(model_instance.password).to be_present - end - - def stub_bcrypt_password - model_instance.salt = salt - digestable = "--#{salt}--#{password}--" - model_instance.encrypted_password = Digest::SHA1.hexdigest(digestable) - allow(BCrypt::Password).to receive(:create).and_return(encrypted_password) - model_instance.password = password - end - - def encrypted_password - @encrypted_password ||= double("encrypted password") - end - end - - describe "#authenticated?" do - context "with a SHA1-encrypted password" do - it "is authenticated" do - model_instance.salt = salt - model_instance.encrypted_password = sha1_hash - allow(model_instance).to receive(:save) - - expect(model_instance).to be_authenticated(password) - end - - it "changes the hash into a BCrypt-encrypted one" do - model_instance.salt = salt - model_instance.encrypted_password = sha1_hash - allow(model_instance).to receive(:save) - - model_instance.authenticated? password - - expect(model_instance.encrypted_password).not_to eq sha1_hash - end - - it "does not raise a BCrypt error for invalid passwords" do - model_instance.salt = salt - model_instance.encrypted_password = sha1_hash - - expect do - model_instance.authenticated? "bad" + password - end.not_to raise_error - end - - it "saves the subject to database" do - model_instance.salt = salt - model_instance.encrypted_password = sha1_hash - allow(model_instance).to receive(:save) - - model_instance.authenticated? password - - expect(model_instance).to have_received(:save) - end - - def sha1_hash - Digest::SHA1.hexdigest("--#{salt}--#{password}--") - end - end - - context "with a BCrypt-encrypted password" do - it "is authenticated" do - model_instance.encrypted_password = bcrypt_hash - - expect(model_instance).to be_authenticated(password) - end - - it "does not change the hash" do - model_instance.encrypted_password = bcrypt_hash - - model_instance.authenticated? password - - expect(model_instance.encrypted_password.to_s).to eq bcrypt_hash.to_s - end - - def bcrypt_hash - @bcrypt_hash ||= ::BCrypt::Password.create(password) - end - end - end - - def model_instance - @model_instance ||= fake_model_with_password_strategy( - Clearance::PasswordStrategies::BCryptMigrationFromSHA1 - ) - end - - def salt - "salt" - end - - def password - "password" - end -end diff --git a/spec/password_strategies/blowfish_spec.rb b/spec/password_strategies/blowfish_spec.rb deleted file mode 100644 index 2cc98f7ae..000000000 --- a/spec/password_strategies/blowfish_spec.rb +++ /dev/null @@ -1,61 +0,0 @@ -require "spec_helper" -include FakeModelWithPasswordStrategy - -describe Clearance::PasswordStrategies::Blowfish do - around do |example| - silence_warnings do - example.run - end - end - - describe "#password=" do - context "when the password is set" do - it "does not initialize the salt" do - model_instance = fake_model_with_blowfish_strategy - model_instance.salt = salt - model_instance.password = password - - expect(model_instance.salt).to eq salt - end - - it "encrypts the password using Blowfish and the existing salt" do - model_instance = fake_model_with_blowfish_strategy - model_instance.salt = salt - model_instance.salt = salt - model_instance.password = password - cipher = OpenSSL::Cipher::Cipher.new("bf-cbc").encrypt - cipher.key = Digest::SHA256.digest(salt) - expected = cipher.update("--#{salt}--#{password}--") << cipher.final - - encrypted_password = Base64.decode64(model_instance.encrypted_password) - - expect(encrypted_password).to eq expected - end - end - - context "when the salt is not set" do - it "should initialize the salt" do - model_instance = fake_model_with_blowfish_strategy - model_instance.salt = salt - model_instance.salt = nil - model_instance.password = password - - expect(model_instance.salt).not_to be_nil - end - end - end - - def fake_model_with_blowfish_strategy - @model_instance ||= fake_model_with_password_strategy( - Clearance::PasswordStrategies::Blowfish - ) - end - - def salt - "salt" - end - - def password - "password" - end -end diff --git a/spec/password_strategies/sha1_spec.rb b/spec/password_strategies/sha1_spec.rb deleted file mode 100644 index 0ac4c5956..000000000 --- a/spec/password_strategies/sha1_spec.rb +++ /dev/null @@ -1,59 +0,0 @@ -require "spec_helper" -include FakeModelWithPasswordStrategy - -describe Clearance::PasswordStrategies::SHA1 do - around do |example| - silence_warnings do - example.run - end - end - - describe "#password=" do - context "when the salt is set" do - it "does not initialize the salt when assigned" do - model_instance = fake_model_with_sha1_strategy - - model_instance.salt = salt - - expect(model_instance.salt).to eq salt - end - - it "encrypts the password using SHA1 and the existing salt" do - model_instance = fake_model_with_sha1_strategy - model_instance.salt = salt - model_instance.password = password - - expected = Digest::SHA1.hexdigest("--#{salt}--#{password}--") - - expect(model_instance.encrypted_password).to eq expected - end - end - - context "when the salt is set" do - it "generates the salt" do - model_instance = fake_model_with_sha1_strategy - model_instance.password = "" - - expect(model_instance.salt).not_to be_nil - end - - it "doesn't encrypt the password" do - model_instance = fake_model_with_sha1_strategy - - expect(model_instance.encrypted_password).to be_nil - end - end - end - - def fake_model_with_sha1_strategy - fake_model_with_password_strategy(Clearance::PasswordStrategies::SHA1) - end - - def salt - "salt" - end - - def password - "password" - end -end diff --git a/spec/user_spec.rb b/spec/user_spec.rb index e3a1238e7..07bae51dc 100644 --- a/spec/user_spec.rb +++ b/spec/user_spec.rb @@ -70,7 +70,7 @@ describe "#update_password" do context "with a valid password" do it "changes the encrypted password" do - user = create(:user, :with_forgotten_password) + user = create(:user) old_encrypted_password = user.encrypted_password user.update_password("new_password") @@ -78,16 +78,8 @@ expect(user.encrypted_password).not_to eq old_encrypted_password end - it "clears the confirmation token" do - user = create(:user, :with_forgotten_password) - - user.update_password("new_password") - - expect(user.confirmation_token).to be_nil - end - it "sets the remember token" do - user = create(:user, :with_forgotten_password) + user = create(:user) user.update_password("my_new_password") @@ -98,21 +90,13 @@ context "with blank password" do it "does not change the encrypted password" do - user = create(:user, :with_forgotten_password) + user = create(:user) old_encrypted_password = user.encrypted_password user.update_password("") expect(user.encrypted_password.to_s).to eq old_encrypted_password end - - it "does not clear the confirmation token" do - user = create(:user, :with_forgotten_password) - - user.update_password("") - - expect(user.confirmation_token).not_to be_nil - end end end @@ -129,16 +113,6 @@ end end - describe "#forgot_password!" do - it "generates the confirmation token" do - user = create(:user, confirmation_token: nil) - - user.forgot_password! - - expect(user.confirmation_token).not_to be_nil - end - end - describe "a user with an optional email" do subject { user }