From eb640ed344fb5e227e82b1f3a914ca9fabb938e0 Mon Sep 17 00:00:00 2001 From: Vincent Woo Date: Tue, 30 Jun 2015 15:22:09 -0700 Subject: [PATCH] Do not use digests for confirmation tokens --- lib/devise/models/confirmable.rb | 29 +++++++++++++------ .../mailers/confirmation_instructions_test.rb | 2 +- test/models/confirmable_test.rb | 15 ++++++++-- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb index fa3b63fb..5a05e0f9 100644 --- a/lib/devise/models/confirmable.rb +++ b/lib/devise/models/confirmable.rb @@ -7,7 +7,7 @@ module Devise # # Confirmable tracks the following columns: # - # * confirmation_token - An OpenSSL::HMAC.hexdigest of @raw_confirmation_token + # * confirmation_token - A unique random token # * confirmed_at - A timestamp when the user clicked the confirmation link # * confirmation_sent_at - A timestamp when the confirmation_token was generated (not sent) # * unconfirmed_email - An email address copied from the email attr. After confirmation @@ -29,6 +29,8 @@ module Devise # confirmation. # * +confirm_within+: 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. + # Confirmable will not generate a new token if a repeat confirmation is requested + # during this time frame, unless the user's email changed too. # # == Examples # @@ -230,10 +232,13 @@ module Devise # Generates a new random token for confirmation, and stores # the time this token is being generated in confirmation_sent_at def generate_confirmation_token - raw, enc = Devise.token_generator.generate(self.class, :confirmation_token) - @raw_confirmation_token = raw - self.confirmation_token = enc - self.confirmation_sent_at = Time.now.utc + if self.confirmation_token && !confirmation_period_expired? + @raw_confirmation_token = self.confirmation_token + else + raw, _ = Devise.token_generator.generate(self.class, :confirmation_token) + self.confirmation_token = @raw_confirmation_token = raw + self.confirmation_sent_at = Time.now.utc + end end def generate_confirmation_token! @@ -244,6 +249,7 @@ module Devise @reconfirmation_required = true self.unconfirmed_email = self.email self.email = self.email_was + self.confirmation_token = nil generate_confirmation_token end @@ -293,12 +299,17 @@ module Devise # If the user is already confirmed, create an error for the user # Options must have the confirmation_token def confirm_by_token(confirmation_token) - original_token = confirmation_token - confirmation_token = Devise.token_generator.digest(self, :confirmation_token, confirmation_token) + confirmable = find_first_by_auth_conditions(confirmation_token: confirmation_token) + unless confirmable + confirmation_digest = Devise.token_generator.digest(self, :confirmation_token, confirmation_token) + confirmable = find_or_initialize_with_error_by(:confirmation_token, confirmation_digest) + end + + # TODO: replace above lines with + # confirmable = find_or_initialize_with_error_by(:confirmation_token, confirmation_token) + # after enough time has passed that Devise clients do not use digested tokens - confirmable = find_or_initialize_with_error_by(:confirmation_token, confirmation_token) confirmable.confirm if confirmable.persisted? - confirmable.confirmation_token = original_token confirmable end diff --git a/test/mailers/confirmation_instructions_test.rb b/test/mailers/confirmation_instructions_test.rb index 8eb9358c..ef8d4962 100644 --- a/test/mailers/confirmation_instructions_test.rb +++ b/test/mailers/confirmation_instructions_test.rb @@ -86,7 +86,7 @@ class ConfirmationInstructionsTest < ActionMailer::TestCase host, port = ActionMailer::Base.default_url_options.values_at :host, :port if mail.body.encoded =~ %r{} - assert_equal Devise.token_generator.digest(user.class, :confirmation_token, $1), user.confirmation_token + assert_equal $1, user.confirmation_token else flunk "expected confirmation url regex to match" end diff --git a/test/models/confirmable_test.rb b/test/models/confirmable_test.rb index fa3a226b..008996c0 100644 --- a/test/models/confirmable_test.rb +++ b/test/models/confirmable_test.rb @@ -291,12 +291,23 @@ class ConfirmableTest < ActiveSupport::TestCase end end - test 'always generate a new token on resend' do + test 'do not generate a new token on resend' do user = create_user old = user.confirmation_token user = User.find(user.id) user.resend_confirmation_instructions - assert_not_equal user.confirmation_token, old + assert_equal user.confirmation_token, old + end + + test 'generate a new token after first has expired' do + swap Devise, confirm_within: 3.days do + user = create_user + old = user.confirmation_token + user.update_attribute(:confirmation_sent_at, 4.days.ago) + user = User.find(user.id) + user.resend_confirmation_instructions + assert_not_equal user.confirmation_token, old + end end test 'should call after_confirmation if confirmed' do