From b519aba63ee0043ffd60f9002fc236f717d9f172 Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Barth <jeanbaptiste.barth@gmail.com>
Date: Tue, 16 Sep 2014 21:38:54 +0000
Subject: [PATCH] Expire other sessions on password change (#17796).

Contributed by Jan Schulz-Hofen.


git-svn-id: http://svn.redmine.org/redmine/trunk@13412 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/controllers/application_controller.rb          | 14 +++++++++++++-
 app/controllers/my_controller.rb                   |  3 +++
 app/models/user.rb                                 |  1 +
 ...140903143914_add_password_changed_at_to_user.rb |  5 +++++
 test/functional/my_controller_test.rb              | 12 ++++++++++++
 5 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 db/migrate/20140903143914_add_password_changed_at_to_user.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index e70f1602c..e8f3565ee 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -49,7 +49,7 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  before_filter :session_expiration, :user_setup, :check_if_login_required, :check_password_change, :set_localization
+  before_filter :session_expiration, :user_setup, :force_logout_if_password_changed, :check_if_login_required, :check_password_change, :set_localization
 
   rescue_from ::Unauthorized, :with => :deny_access
   rescue_from ::ActionView::MissingTemplate, :with => :missing_template
@@ -145,6 +145,18 @@ class ApplicationController < ActionController::Base
     user
   end
 
+  def force_logout_if_password_changed
+    passwd_changed_on = User.current.passwd_changed_on || Time.at(0)
+    # Make sure we force logout only for web browser sessions, not API calls
+    # if the password was changed after the session creation.
+    if session[:user_id] && passwd_changed_on.utc.to_i > session[:ctime].to_i
+      reset_session
+      set_localization
+      flash[:error] = l(:error_session_expired)
+      redirect_to signin_url
+    end
+  end
+
   def autologin_cookie_name
     Redmine::Configuration['autologin_cookie_name'].presence || 'autologin'
   end
diff --git a/app/controllers/my_controller.rb b/app/controllers/my_controller.rb
index 59d1d2d0f..714b23857 100644
--- a/app/controllers/my_controller.rb
+++ b/app/controllers/my_controller.rb
@@ -100,6 +100,9 @@ class MyController < ApplicationController
         @user.password, @user.password_confirmation = params[:new_password], params[:new_password_confirmation]
         @user.must_change_passwd = false
         if @user.save
+          # Reset the session creation time to not log out this session on next
+          # request due to ApplicationController#force_logout_if_password_changed
+          session[:ctime] = Time.now.utc.to_i
           flash[:notice] = l(:notice_account_password_updated)
           redirect_to my_account_path
         end
diff --git a/app/models/user.rb b/app/models/user.rb
index 8405fa4b6..c79c674a9 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -279,6 +279,7 @@ class User < Principal
   def salt_password(clear_password)
     self.salt = User.generate_salt
     self.hashed_password = User.hash_password("#{salt}#{User.hash_password clear_password}")
+    self.passwd_changed_on = Time.now
   end
 
   # Does the backend storage allow this user to change their password?
diff --git a/db/migrate/20140903143914_add_password_changed_at_to_user.rb b/db/migrate/20140903143914_add_password_changed_at_to_user.rb
new file mode 100644
index 000000000..508b9d809
--- /dev/null
+++ b/db/migrate/20140903143914_add_password_changed_at_to_user.rb
@@ -0,0 +1,5 @@
+class AddPasswordChangedAtToUser < ActiveRecord::Migration
+  def change
+    add_column :users, :passwd_changed_on, :datetime
+  end
+end
diff --git a/test/functional/my_controller_test.rb b/test/functional/my_controller_test.rb
index 6577aee6a..53c73341a 100644
--- a/test/functional/my_controller_test.rb
+++ b/test/functional/my_controller_test.rb
@@ -185,6 +185,18 @@ class MyControllerTest < ActionController::TestCase
     assert User.try_to_login('jsmith', 'secret123')
   end
 
+  def test_change_password_kills_other_sessions
+    @request.session[:ctime] = (Time.now - 30.minutes).utc.to_i
+
+    jsmith = User.find(2)
+    jsmith.passwd_changed_on = Time.now
+    jsmith.save!
+
+    get 'account'
+    assert_response 302
+    assert flash[:error].match(/Your session has expired/)
+  end
+
   def test_change_password_should_redirect_if_user_cannot_change_its_password
     User.find(2).update_attribute(:auth_source_id, 1)
 
-- 
2.39.5