diff --git a/README.md b/README.md index 3aa46e39..2c4dd03c 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,7 @@ Clearance.configure do |config| config.routes = true config.httponly = true config.mailer_sender = "reply@example.com" + config.password_reset_token_expiration_in = nil config.password_strategy = Clearance::PasswordStrategies::BCrypt config.redirect_url = "/" config.url_after_destroy = nil @@ -131,6 +132,15 @@ Clearance.configure do |config| end ``` +By default, password reset tokens do not expire. It is recommended to set an expiration time for password reset tokens by changing the `password_reset_token_expiration_in` configuration option: +```ruby +Clearance.configure do |config| + config.password_reset_token_expiration_in = 1.hour +end +``` + +**Important:** The reset token expiration feature requires the `confirmation_token_created_at` column to exist in your user model. Run `rails generate clearance:install` to generate the appropriate migration file if upgrading from a version of Clearance before 2.12.0. + ### Multiple Domain Support You can support multiple domains, or other special domain configurations by diff --git a/app/controllers/clearance/passwords_controller.rb b/app/controllers/clearance/passwords_controller.rb index 686929f9..f56e978d 100644 --- a/app/controllers/clearance/passwords_controller.rb +++ b/app/controllers/clearance/passwords_controller.rb @@ -56,8 +56,10 @@ def find_user_by_id_and_confirmation_token user_param = Clearance.configuration.user_id_parameter token = params[:token] || session[:password_reset_token] - Clearance.configuration.user_model + user = Clearance.configuration.user_model .find_by(id: params[user_param], confirmation_token: token.to_s) + + user unless user&.password_reset_token_expired? end def email_from_password_params diff --git a/lib/clearance/configuration.rb b/lib/clearance/configuration.rb index 755f77af..349282ac 100644 --- a/lib/clearance/configuration.rb +++ b/lib/clearance/configuration.rb @@ -69,6 +69,18 @@ class Configuration # @return [Module #authenticated? #password=] attr_accessor :password_strategy + # How long a password reset token is valid. Defaults to `nil` (no + # expiration). Set to an ActiveSupport::Duration to enable expiration, e.g. + # + # config.password_reset_token_expiration_in = 2.hours + # + # When set, `forgot_password!` records the time the token was issued. + # Tokens with no recorded issue time (e.g. issued before upgrading or + # before running the migration) are treated as expired, forcing users to + # re-request a reset. + # @return [ActiveSupport::Duration, nil] + attr_accessor :password_reset_token_expiration_in + # The default path Clearance will redirect signed in users to. # Defaults to `"/"`. This can often be overridden for specific scenarios by # overriding controller methods that rely on it. @@ -161,6 +173,7 @@ def initialize @httponly = true @same_site = nil @mailer_sender = "reply@example.com" + @password_reset_token_expiration_in = nil @redirect_url = "/" @url_after_destroy = nil @url_after_denied_access_when_signed_out = nil diff --git a/lib/clearance/user.rb b/lib/clearance/user.rb index 4debf776..a1cae82d 100644 --- a/lib/clearance/user.rb +++ b/lib/clearance/user.rb @@ -9,7 +9,6 @@ module Clearance # # class User # include Clearance::User - # # # ... # end # @@ -47,6 +46,11 @@ module Clearance # @return [String] The value used to identify this user in the password # reset link. # + # @!attribute confirmation_token_created_at + # @return [Time, nil] When the current confirmation_token was issued. + # Used to enforce {Configuration#password_reset_token_expiration_in}. + # Nil for tokens issued before this column was added. + # # @!attribute [r] password # @return [String] Transient (non-persisted) attribute that is set when # updating a user's password. Only the {#encrypted_password} is persisted. @@ -215,12 +219,40 @@ def update_password(new_password) if valid? self.confirmation_token = nil + clear_confirmation_token_created_at generate_remember_token end save end + # Returns true if the password reset token has expired according to the + # configured {Configuration#password_reset_token_expiration_in}. + # + # Always returns false when expiration is not configured (the default), + # preserving existing behaviour. + # + # Returns true — treating the token as expired — when expiration is + # configured but the column does not yet exist (pre-migration) or the + # timestamp is nil (token issued before the column was added). This is + # intentional: if an operator has opted in to expiration, silently skipping + # it would undermine the security intent. + # + # @return [Boolean] + def password_reset_token_expired? + expiration = Clearance.configuration.password_reset_token_expiration_in + return false unless expiration + + unless self.class.column_names.include?("confirmation_token_created_at") + raise "The `confirmation_token_created_at` column is required to " \ + "check for expired password reset tokens. Please run " \ + "`rails generate clearance:install` to add the necessary migration." + end + + confirmation_token_created_at.nil? || + confirmation_token_created_at < expiration.ago + end + private # Sets the email on this instance to the value returned by @@ -257,12 +289,25 @@ def skip_password_validation? 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 + # {Token.new}, and stamps {#confirmation_token_created_at} if the column + # exists. 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 + if self.class.column_names.include?("confirmation_token_created_at") + self.confirmation_token_created_at = Time.current + end + end + + # Clears {#confirmation_token_created_at} if the column exists. + # + # @return [void] + def clear_confirmation_token_created_at + if self.class.column_names.include?("confirmation_token_created_at") + self.confirmation_token_created_at = nil + end end # Sets the {#remember_token} on the instance to a new value generated by diff --git a/lib/generators/clearance/install/install_generator.rb b/lib/generators/clearance/install/install_generator.rb index 7155342c..264f7b68 100644 --- a/lib/generators/clearance/install/install_generator.rb +++ b/lib/generators/clearance/install/install_generator.rb @@ -82,6 +82,7 @@ def new_columns email: "t.string :email", encrypted_password: "t.string :encrypted_password, limit: 128", confirmation_token: "t.string :confirmation_token, limit: 128", + confirmation_token_created_at: "t.datetime :confirmation_token_created_at", remember_token: "t.string :remember_token, limit: 128" }.reject { |column| existing_users_columns.include?(column.to_s) } end diff --git a/lib/generators/clearance/install/templates/db/migrate/create_users.rb.erb b/lib/generators/clearance/install/templates/db/migrate/create_users.rb.erb index 7fdd9876..90e51736 100644 --- a/lib/generators/clearance/install/templates/db/migrate/create_users.rb.erb +++ b/lib/generators/clearance/install/templates/db/migrate/create_users.rb.erb @@ -5,6 +5,7 @@ class CreateUsers < ActiveRecord::Migration<%= migration_version %> t.string :email, null: false t.string :encrypted_password, limit: 128, null: false t.string :confirmation_token, limit: 128 + t.datetime :confirmation_token_created_at t.string :remember_token, limit: 128, null: false end diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 1421e6db..68790e6d 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -266,4 +266,18 @@ expect(Clearance.configuration.rotate_csrf_on_sign_in?).to be false end end + + describe "#password_reset_token_expiration_in" do + it "returns nil when unset" do + expect(Clearance.configuration.password_reset_token_expiration_in).to be_nil + end + + it "returns 2.hours when set to 2.hours" do + Clearance.configure do |config| + config.password_reset_token_expiration_in = 2.hours + end + + expect(Clearance.configuration.password_reset_token_expiration_in).to eq 2.hours + end + end end diff --git a/spec/controllers/passwords_controller_spec.rb b/spec/controllers/passwords_controller_spec.rb index d94a422e..a6c3f581 100644 --- a/spec/controllers/passwords_controller_spec.rb +++ b/spec/controllers/passwords_controller_spec.rb @@ -197,6 +197,32 @@ expect(session[:password_reset_token]).to eq(user.confirmation_token) end end + + context "token has expired" do + before do + Clearance.configure do |config| + config.password_reset_token_expiration_in = 2.hours + end + end + + after do + Clearance.configure do |config| + config.password_reset_token_expiration_in = nil + end + end + + it "renders the new password reset form with a flash alert" do + user = create(:user, :with_forgotten_password, confirmation_token_created_at: 3.hours.ago) + + get :edit, params: { + user_id: user, + token: user.confirmation_token + } + + expect(response).to render_template(:new) + expect(flash.now[:alert]).to match(I18n.t("flashes.failure_when_forbidden")) + end + end end describe "#update" do @@ -294,6 +320,55 @@ expect(current_user).to be_nil end end + + context "token has expired" do + before do + Clearance.configure do |config| + config.password_reset_token_expiration_in = 2.hours + end + end + + after do + Clearance.configure do |config| + config.password_reset_token_expiration_in = nil + end + end + + it "re-renders the password edit form" do + user = create(:user, :with_forgotten_password, confirmation_token_created_at: 3.hours.ago) + + put :update, params: update_parameters( + user, + new_password: "new_password" + ) + + expect(response).to render_template(:new) + expect(flash.now[:alert]).to match(I18n.t("flashes.failure_when_forbidden")) + end + + it "does not update the password" do + user = create(:user, :with_forgotten_password, confirmation_token_created_at: 3.hours.ago) + old_encrypted_password = user.encrypted_password + + put :update, params: update_parameters( + user, + new_password: "new_password" + ) + + expect(user.reload.encrypted_password).to eq old_encrypted_password + end + + it "does not sign in user" do + user = create(:user, :with_forgotten_password, confirmation_token_created_at: 3.hours.ago) + + put :update, params: update_parameters( + user, + new_password: "new_password" + ) + + expect(current_user).to be_nil + end + end end def update_parameters(user, options = {}) diff --git a/spec/dummy/db/schema.rb b/spec/dummy/db/schema.rb index cad6f33e..25343c7d 100644 --- a/spec/dummy/db/schema.rb +++ b/spec/dummy/db/schema.rb @@ -17,6 +17,7 @@ t.string "email", null: false t.string "encrypted_password", limit: 128, null: false t.string "confirmation_token", limit: 128 + t.datetime "confirmation_token_created_at" t.string "remember_token", limit: 128, null: false t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["email"], name: "index_users_on_email" diff --git a/spec/factories/users.rb b/spec/factories/users.rb index dfb09205..5f3889a6 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -9,6 +9,7 @@ trait :with_forgotten_password do confirmation_token { Clearance::Token.new } + confirmation_token_created_at { Time.current } end factory :user_with_optional_password, class: "UserWithOptionalPassword" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f4615526..36510159 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -116,6 +116,14 @@ expect(user.confirmation_token).to be_nil end + it "clears the confirmation_token_created_at timestamp" do + user = create(:user, :with_forgotten_password) + + user.update_password("new_password") + + expect(user.confirmation_token_created_at).to be_nil + end + it "sets the remember token" do user = create(:user, :with_forgotten_password) @@ -143,6 +151,15 @@ expect(user.confirmation_token).not_to be_nil end + + it "does not clear the confirmation_token_created_at timestamp" do + user = create(:user, :with_forgotten_password) + original_timestamp = user.confirmation_token_created_at + + user.update_password("") + + expect(user.confirmation_token_created_at).to eq original_timestamp + end end end @@ -167,6 +184,88 @@ expect(user.confirmation_token).not_to be_nil end + + it "sets the confirmation_token_created_at timestamp" do + freeze_time do + user = create(:user) + + user.forgot_password! + + expect(user.confirmation_token_created_at).to eq Time.current + end + end + + it "updates the existing timestamp when called again" do + user = create(:user) + user.forgot_password! + first_timestamp = user.confirmation_token_created_at + + travel 1.hour do + user.forgot_password! + expect(user.confirmation_token_created_at).to be > first_timestamp + end + end + end + + describe "#password_reset_token_expired?" do + context "when expiration is not configured" do + it "returns false for any token" do + user = create(:user, :with_forgotten_password) + + expect(user.password_reset_token_expired?).to be false + end + + it "returns false even if confirmation_token_created_at is old" do + user = create(:user, :with_forgotten_password, confirmation_token_created_at: 1.year.ago) + + expect(user.password_reset_token_expired?).to be false + end + end + + context "when expiration is configured" do + before do + Clearance.configure do |config| + config.password_reset_token_expiration_in = 2.hours + end + end + + after do + Clearance.configure do |config| + config.password_reset_token_expiration_in = nil + end + end + + it "returns false for a token issued within the expiration window" do + freeze_time do + user = create(:user, :with_forgotten_password, confirmation_token_created_at: 2.hours.ago + 5.minutes) + + expect(user.password_reset_token_expired?).to be false + end + end + + it "returns true for a token issued beyond the expiration window" do + user = create(:user, :with_forgotten_password, confirmation_token_created_at: 3.hours.ago) + + expect(user.password_reset_token_expired?).to be true + end + + it "returns true when confirmation_token_created_at is nil" do + user = create(:user, :with_forgotten_password, confirmation_token_created_at: nil) + + expect(user.password_reset_token_expired?).to be true + end + + it "raises exception when confirmation_token_created_at column doesn't exist" do + user = create(:user, :with_forgotten_password) + allow(user.class).to receive(:column_names).and_return( + user.class.column_names - ["confirmation_token_created_at"] + ) + + expect { user.password_reset_token_expired? }.to raise_error( + /`confirmation_token_created_at` column is required/ + ) + end + end end describe "a user with an optional email" do