diff options
| -rw-r--r-- | app/models/user.rb | 14 | ||||
| -rw-r--r-- | test/unit/user_test.rb | 36 |
2 files changed, 49 insertions, 1 deletions
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 |