From: Jean-Baptiste Barth <jeanbaptiste.barth@gmail.com>
Date: Sun, 14 Sep 2014 08:22:25 +0000 (+0000)
Subject: Invalidate security tokens on password or email changes (#17717).
X-Git-Tag: 2.6.0~42
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=2eb95f41b41d4cf7fef6773f98100e6a449f9192;p=redmine.git

Invalidate security tokens on password or email changes (#17717).

Contributed by Jan Schulz-Hofen.


git-svn-id: http://svn.redmine.org/redmine/trunk@13396 e93f8b46-1217-0410-a6f0-8f06a7374b81
---

diff --git a/app/models/user.rb b/app/models/user.rb
index 5f9674d2d..36594137d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -112,7 +112,7 @@ class User < Principal
   before_create :set_mail_notification
   before_save   :generate_password_if_needed, :update_hashed_password
   before_destroy :remove_references_before_destroy
-  after_save :update_notified_project_ids
+  after_save :update_notified_project_ids, :destroy_tokens
 
   scope :in_group, lambda {|group|
     group_id = group.is_a?(Group) ? group.id : group.to_i
@@ -681,6 +681,18 @@ class User < Principal
     end
   end
 
+  # Delete all outstanding password reset tokens on password or email change.
+  # Delete the autologin tokens on password change to prohibit session leakage.
+  # This helps to keep the account secure in case the associated email account
+  # was compromised.
+  def destroy_tokens
+    tokens  = []
+    tokens |= ['recovery', 'autologin'] if changes.has_key?('hashed_password')
+    tokens |= ['recovery'] if changes.has_key?('mail')
+
+    Token.delete_all(['user_id = ? AND action IN (?)', self.id, tokens]) if tokens.any?
+  end
+
   # Removes references that are not handled by associations
   # Things that are not deleted are reassociated with the anonymous user
   def remove_references_before_destroy
diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb
index 847c25c3b..69753960a 100644
--- a/test/unit/user_test.rb
+++ b/test/unit/user_test.rb
@@ -403,6 +403,42 @@ class UserTest < ActiveSupport::TestCase
     end
   end
 
+  def test_password_change_should_destroy_tokens
+    recovery_token = Token.create!(:user_id => 2, :action => 'recovery')
+    autologin_token = Token.create!(:user_id => 2, :action => 'autologin')
+
+    user = User.find(2)
+    user.password, user.password_confirmation = "a new password", "a new password"
+    assert user.save
+
+    assert_nil Token.find_by_id(recovery_token.id)
+    assert_nil Token.find_by_id(autologin_token.id)
+  end
+
+  def test_mail_change_should_destroy_tokens
+    recovery_token = Token.create!(:user_id => 2, :action => 'recovery')
+    autologin_token = Token.create!(:user_id => 2, :action => 'autologin')
+
+    user = User.find(2)
+    user.mail = "user@somwehere.com"
+    assert user.save
+
+    assert_nil Token.find_by_id(recovery_token.id)
+    assert_equal autologin_token, Token.find_by_id(autologin_token.id)
+  end
+
+  def test_change_on_other_fields_should_not_destroy_tokens
+    recovery_token = Token.create!(:user_id => 2, :action => 'recovery')
+    autologin_token = Token.create!(:user_id => 2, :action => 'autologin')
+
+    user = User.find(2)
+    user.firstname = "Bobby"
+    assert user.save
+
+    assert_equal recovery_token, Token.find_by_id(recovery_token.id)
+    assert_equal autologin_token, Token.find_by_id(autologin_token.id)
+  end
+
   def test_validate_login_presence
     @admin.login = ""
     assert !@admin.save