Merge pull request #3661 from vincentwoo/plain_confirmation

Do not use digests for confirmation tokens

lib/devise/models/confirmable.rb

@@ -7,7 +7,7 @@ module Models
     #
     # 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 Models
     #     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 @@ def pending_any_confirmation
         # 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 @@ def postpone_email_change_until_confirmation_and_regenerate_confirmation_token
           @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 @@ def send_confirmation_instructions(attributes={})
         # 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
 

test/mailers/confirmation_instructions_test.rb

@@ -86,7 +86,7 @@ def mail
     host, port = ActionMailer::Base.default_url_options.values_at :host, :port
 
     if mail.body.encoded =~ %r{<a href=\"http://#{host}:#{port}/users/confirmation\?confirmation_token=([^"]+)">}
-      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

test/models/confirmable_test.rb

@@ -291,12 +291,23 @@ def confirm_user_by_token_with_confirmation_sent_at(confirmation_sent_at)
     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