From 87f2fa97679795e080b7bd86a71aa68617fc9a3a Mon Sep 17 00:00:00 2001 From: Nils Landt Date: Mon, 9 Jul 2012 14:43:12 +0200 Subject: [PATCH] Add options to expire confirmation tokens With this patch, functionality is added to expire the confirmation tokens that are being sent by email. For example, if a token is valid for 3 days only, it cannot be used for confirmation on the 4th day. --- Gemfile | 3 +++ Gemfile.lock | 8 +++++++ config/locales/en.yml | 1 + lib/devise.rb | 6 ++++- lib/devise/models/confirmable.rb | 33 ++++++++++++++++++++++---- lib/generators/templates/devise.rb | 10 +++++++- test/integration/confirmable_test.rb | 35 ++++++++++++++++++++++++---- test/models/confirmable_test.rb | 32 +++++++++++++++++++++++++ test/support/integration.rb | 3 ++- 9 files changed, 120 insertions(+), 11 deletions(-) diff --git a/Gemfile b/Gemfile index 934fb02a..3c239070 100644 --- a/Gemfile +++ b/Gemfile @@ -16,6 +16,9 @@ group :test do platforms :mri_18 do gem "ruby-debug", ">= 0.10.3" end + platforms :mri_19 do + gem 'debugger' + end end platforms :jruby do diff --git a/Gemfile.lock b/Gemfile.lock index fdadc96a..de48c8c0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -44,6 +44,13 @@ GEM bson_ext (1.3.1) builder (3.0.0) columnize (0.3.5) + debugger (1.1.4) + columnize (>= 0.3.1) + debugger-linecache (~> 1.1.1) + debugger-ruby_core_source (~> 1.1.3) + debugger-linecache (1.1.2) + debugger-ruby_core_source (>= 1.1.1) + debugger-ruby_core_source (1.1.3) erubis (2.7.0) faraday (0.7.5) addressable (~> 2.2.6) @@ -149,6 +156,7 @@ DEPENDENCIES activerecord-jdbc-adapter activerecord-jdbcsqlite3-adapter bson_ext (~> 1.3.0) + debugger devise! jruby-openssl mocha diff --git a/config/locales/en.yml b/config/locales/en.yml index 7783a744..a533f90c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -10,6 +10,7 @@ en: not_saved: one: "1 error prohibited this %{resource} from being saved:" other: "%{count} errors prohibited this %{resource} from being saved:" + confirmation_period_expired: "needs to be confirmed within %{period}, please request a new one" devise: failure: diff --git a/lib/devise.rb b/lib/devise.rb index 713c5c7e..d355be86 100644 --- a/lib/devise.rb +++ b/lib/devise.rb @@ -104,6 +104,10 @@ module Devise mattr_accessor :allow_unconfirmed_access_for @@allow_unconfirmed_access_for = 0.days + # Time interval the confirmation token is valid. nil = unlimited + mattr_accessor :expire_confirmation_token_after + @@expire_confirmation_token_after = nil + # Defines which key will be used when confirming an account. mattr_accessor :confirmation_keys @@confirmation_keys = [ :email ] @@ -199,7 +203,7 @@ module Devise # to provide custom routes. mattr_accessor :router_name @@router_name = nil - + # Set the omniauth path prefix so it can be overriden when # Devise is used in a mountable engine mattr_accessor :omniauth_path_prefix diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb index afcda349..01d35c75 100644 --- a/lib/devise/models/confirmable.rb +++ b/lib/devise/models/confirmable.rb @@ -19,6 +19,8 @@ module Devise # db field to be setup (t.reconfirmable in migrations). Until confirmed new email is # stored in unconfirmed email column, and copied to email column on successful # confirmation. + # * +expire_confirmation_token_after+: the time before a sent confirmation token becomes invalid. + # You can use this to force the user to confirm within a set period of time. # # == Examples # @@ -28,6 +30,8 @@ module Devise # module Confirmable extend ActiveSupport::Concern + # TODO: is this a good idea? + include ActionView::Helpers::DateHelper included do before_create :generate_confirmation_token, :if => :confirmation_required? @@ -118,7 +122,6 @@ module Devise end headers end - protected # A callback method used to deliver confirmation @@ -156,12 +159,34 @@ module Devise confirmation_sent_at && confirmation_sent_at.utc >= self.class.allow_unconfirmed_access_for.ago end + # Checks if the user confirmation happens before the token becomes invalid + # + # Examples: + # + # # expire_confirmation_token_after = 3.days and confirmation_sent_at = 2.days.ago + # confirmation_period_expired? # returns false + # + # # expire_confirmation_token_after = 3.days and confirmation_sent_at = 4.days.ago + # confirmation_period_expired? # returns true + # + # # expire_confirmation_token_after = nil + # confirmation_period_expired? # will always return false + # + def confirmation_period_expired? + self.class.expire_confirmation_token_after && (Time.now > self.confirmation_sent_at + self.class.expire_confirmation_token_after) + end + # Checks whether the record requires any confirmation. def pending_any_confirmation - if !confirmed? || pending_reconfirmation? + if !confirmation_period_expired? && (!confirmed? || pending_reconfirmation?) yield else - self.errors.add(:email, :already_confirmed) + # TODO: cache this call or not? + if confirmation_period_expired? + self.errors.add(:email, :confirmation_period_expired, period: time_ago_in_words(self.class.expire_confirmation_token_after.ago)) + else + self.errors.add(:email, :already_confirmed) + end false end end @@ -235,7 +260,7 @@ module Devise find_or_initialize_with_errors(unconfirmed_required_attributes, unconfirmed_attributes, :not_found) end - Devise::Models.config(self, :allow_unconfirmed_access_for, :confirmation_keys, :reconfirmable) + Devise::Models.config(self, :allow_unconfirmed_access_for, :confirmation_keys, :reconfirmable, :expire_confirmation_token_after) end end end diff --git a/lib/generators/templates/devise.rb b/lib/generators/templates/devise.rb index b4980e35..b681b3f0 100644 --- a/lib/generators/templates/devise.rb +++ b/lib/generators/templates/devise.rb @@ -92,6 +92,14 @@ Devise.setup do |config| # the user cannot access the website without confirming his account. # config.allow_unconfirmed_access_for = 2.days + # A period that the user is allowed to confirm their account before their token + # becomes invalid. For example, if set to 3.days, the user can confirm their account + # within 3 days after the mail was sent, but on the fourth day their account can't be + # confirmed with the token any more + # Default is nil, meaning there is no restriction on how long a user can take before + # comfirming their account. + # config.expire_confirmation_token_after = 3.days + # If true, requires any email changes to be confirmed (exactly the same way as # initial account confirmation) to be applied. Requires additional unconfirmed_email # db field (see migrations). Until confirmed new email is stored in @@ -125,7 +133,7 @@ Devise.setup do |config| # The time you want to timeout the user session without activity. After this # time the user will be asked for credentials again. Default is 30 minutes. # config.timeout_in = 30.minutes - + # If true, expires auth token on session timeout. # config.expire_auth_token_on_timeout = false diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb index 1bbd4db0..0e812359 100644 --- a/test/integration/confirmable_test.rb +++ b/test/integration/confirmable_test.rb @@ -16,7 +16,7 @@ class ConfirmationTest < ActionController::IntegrationTest fill_in 'email', :with => user.email click_button 'Resend confirmation instructions' end - + test 'user should be able to request a new confirmation' do resend_confirmation @@ -50,6 +50,33 @@ class ConfirmationTest < ActionController::IntegrationTest assert user.reload.confirmed? end + test 'user with valid confirmation token should not be able to confirm an account after the token has expired' do + swap Devise, :expire_confirmation_token_after => 3.days do + # TODO: once again, confirmation_sent_at is not being set to the correct date + user = create_user(:confirm => false, :confirmation_sent_at => 4.days.ago) + #user.confirmation_sent_at = 4.days.ago + assert_not user.confirmed? + visit_user_confirmation_with_token(user.confirmation_token) + + assert_contain 'Your account was successfully confirmed.' + assert_current_url '/' + assert user.reload.confirmed? + end + end + + test 'user with valid confirmation token should be able to confirm an account before the token has expires' do + swap Devise, :expire_confirmation_token_after => 3.days do + # TODO: once again, confirmation_sent_at is not being set to the correct date + user = create_user(:confirm => false, :confirmation_sent_at => 2.days.ago) + assert_not user.confirmed? + visit_user_confirmation_with_token(user.confirmation_token) + + assert_have_selector '#error_explanation' + assert_contain /needs to be confirmed within/ + assert_not user.reload.confirmed? + end + end + test 'user should be redirected to a custom path after confirmation' do Devise::ConfirmationsController.any_instance.stubs(:after_confirmation_path_for).returns("/?custom=1") @@ -239,7 +266,7 @@ class ConfirmationOnChangeTest < ActionController::IntegrationTest assert admin.reload.confirmed? assert_not admin.reload.pending_reconfirmation? end - + test 'admin with previously valid confirmation token should not be able to confirm email after email changed again' do admin = create_admin admin.update_attributes(:email => 'first_test@example.com') @@ -247,11 +274,11 @@ class ConfirmationOnChangeTest < ActionController::IntegrationTest confirmation_token = admin.confirmation_token admin.update_attributes(:email => 'second_test@example.com') assert_equal 'second_test@example.com', admin.unconfirmed_email - + visit_admin_confirmation_with_token(confirmation_token) assert_have_selector '#error_explanation' assert_contain /Confirmation token(.*)invalid/ - + visit_admin_confirmation_with_token(admin.confirmation_token) assert_contain 'Your account was successfully confirmed.' assert_current_url '/admin_area/home' diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb index ae6ded5f..3e7196d1 100644 --- a/test/models/confirmable_test.rb +++ b/test/models/confirmable_test.rb @@ -235,6 +235,38 @@ class ConfirmableTest < ActiveSupport::TestCase assert_equal "can't be blank", confirm_user.errors[:username].join end end + + def confirm_user_by_token_with_confirmation_sent_at confirmation_sent_at + user = create_user + user.confirmation_sent_at = confirmation_sent_at + confirmed_user = User.confirm_by_token(user.confirmation_token) + assert_equal confirmed_user, user + user.reload.confirmed? + end + + test 'should accept confirmation email token even after 5 years when no expiration is set' do + assert confirm_user_by_token_with_confirmation_sent_at(5.years.ago) + end + + test 'should accept confirmation email token after 2 days when expiration is set to 3 days' do + swap Devise, :expire_confirmation_token_after => 3.days do + assert confirm_user_by_token_with_confirmation_sent_at(2.days.ago) + end + end + + test 'should not accept confirmation email token after 4 days when expiration is set to 3 days' do + swap Devise, :expire_confirmation_token_after => 3.days do + #assert_not confirm_user_by_token_with_confirmation_sent_at(4.days.ago) + # TODO: confirmation_sent_at is Time.now during confirm_by_token + # TODO: when everything works, use the test line above + user = create_user + user.confirmation_sent_at = 4.days.ago + assert_not user.confirmed? + confirmed_user = User.confirm_by_token(user.confirmation_token) + assert_equal confirmed_user, user + assert_not user.reload.confirmed? + end + end end class ReconfirmableTest < ActiveSupport::TestCase diff --git a/test/support/integration.rb b/test/support/integration.rb index 9c6f7d25..a5600e4c 100644 --- a/test/support/integration.rb +++ b/test/support/integration.rb @@ -12,7 +12,8 @@ class ActionDispatch::IntegrationTest :email => options[:email] || 'user@test.com', :password => options[:password] || '12345678', :password_confirmation => options[:password] || '12345678', - :created_at => Time.now.utc + :created_at => Time.now.utc, + :confirmation_sent_at => options[:confirmation_sent_at] ) user.confirm! unless options[:confirm] == false user.lock_access! if options[:locked] == true