Keep track of valid user sessions (#21058).

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

4 files changed, 39 insertions, 36 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index f11c04536..98a22463b 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -51,7 +51,7 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  before_filter :session_expiration, :user_setup, :force_logout_if_password_changed, :check_if_login_required, :check_password_change, :set_localization
+  before_filter :session_expiration, :user_setup, :check_if_login_required, :check_password_change, :set_localization
 
   rescue_from ::Unauthorized, :with => :deny_access
   rescue_from ::ActionView::MissingTemplate, :with => :missing_template
@@ -63,36 +63,23 @@ class ApplicationController < ActionController::Base
   include Redmine::SudoMode::Controller
 
   def session_expiration
-    if session[:user_id]
+    if session[:user_id] && Rails.application.config.redmine_verify_sessions != false
       if session_expired? && !try_to_autologin
         set_localization(User.active.find_by_id(session[:user_id]))
         self.logged_user = nil
         flash[:error] = l(:error_session_expired)
         require_login
-      else
-        session[:atime] = Time.now.utc.to_i
       end
     end
   end
 
   def session_expired?
-    if Setting.session_lifetime?
-      unless session[:ctime] && (Time.now.utc.to_i - session[:ctime].to_i <= Setting.session_lifetime.to_i * 60)
-        return true
-      end
-    end
-    if Setting.session_timeout?
-      unless session[:atime] && (Time.now.utc.to_i - session[:atime].to_i <= Setting.session_timeout.to_i * 60)
-        return true
-      end
-    end
-    false
+    ! User.verify_session_token(session[:user_id], session[:tk])
   end
 
   def start_user_session(user)
     session[:user_id] = user.id
-    session[:ctime] = Time.now.utc.to_i
-    session[:atime] = Time.now.utc.to_i
+    session[:tk] = user.generate_session_token
     if user.must_change_password?
       session[:pwd] = '1'
     end
@@ -149,18 +136,6 @@ 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
@@ -193,6 +168,7 @@ class ApplicationController < ActionController::Base
     if User.current.logged?
       cookies.delete(autologin_cookie_name)
       Token.delete_all(["user_id = ? AND action = ?", User.current.id, 'autologin'])
+      Token.delete_all(["user_id = ? AND action = ? AND value = ?", User.current.id, 'session', session[:tk]])
       self.logged_user = nil
     end
   end
diff --git a/app/controllers/my_controller.rb b/app/controllers/my_controller.rb
index 1f744a936..9fdc14314 100644
--- a/app/controllers/my_controller.rb
+++ b/app/controllers/my_controller.rb
@@ -103,9 +103,8 @@ 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] = User.current.passwd_changed_on.utc.to_i
+          # The session token was destroyed by the password change, generate a new one
+          session[:tk] = @user.generate_session_token
           flash[:notice] = l(:notice_account_password_updated)
           redirect_to my_account_path
         end
diff --git a/app/models/token.rb b/app/models/token.rb
index 84e7fc353..e458a92a7 100644
--- a/app/models/token.rb
+++ b/app/models/token.rb
@@ -36,7 +36,7 @@ class Token < ActiveRecord::Base
 
   # Delete all expired tokens
   def self.destroy_expired
-    Token.where("action NOT IN (?) AND created_on < ?", ['feeds', 'api'], Time.now - validity_time).delete_all
+    Token.where("action NOT IN (?) AND created_on < ?", ['feeds', 'api', 'session'], Time.now - validity_time).delete_all
   end
 
   # Returns the active user who owns the key for the given action
@@ -79,7 +79,15 @@ class Token < ActiveRecord::Base
   # Removes obsolete tokens (same user and action)
   def delete_previous_tokens
     if user
-      Token.where(:user_id => user.id, :action => action).delete_all
+      scope = Token.where(:user_id => user.id, :action => action)
+      if action == 'session'
+        ids = scope.order(:updated_on => :desc).offset(9).ids
+        if ids.any?
+          Token.delete(ids)
+        end
+      else
+        scope.delete_all
+      end
     end
   end
 end
diff --git a/app/models/user.rb b/app/models/user.rb
index e133cd02e..4a6109f7d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -394,6 +394,26 @@ class User < Principal
     api_token.value
   end
 
+  # Generates a new session token and returns its value
+  def generate_session_token
+    token = Token.create!(:user_id => id, :action => 'session')
+    token.value
+  end
+
+  # Returns true if token is a valid session token for the user whose id is user_id
+  def self.verify_session_token(user_id, token)
+    return false if user_id.blank? || token.blank?
+
+    scope = Token.where(:user_id => user_id, :value => token.to_s, :action => 'session')
+    if Setting.session_lifetime?
+      scope = scope.where("created_on > ?", Setting.session_lifetime.to_i.minutes.ago)
+    end
+    if Setting.session_timeout?
+      scope = scope.where("updated_on > ?", Setting.session_timeout.to_i.minutes.ago)
+    end
+    scope.update_all(:updated_on => Time.now) == 1
+  end
+
   # Return an array of project ids for which the user has explicitly turned mail notifications on
   def notified_projects_ids
     @notified_projects_ids ||= memberships.select {|m| m.mail_notification?}.collect(&:project_id)
@@ -764,8 +784,8 @@ class User < Principal
   # This helps to keep the account secure in case the associated email account
   # was compromised.
   def destroy_tokens
-    if hashed_password_changed?
-      tokens = ['recovery', 'autologin']
+    if hashed_password_changed? || (status_changed? && !active?)
+      tokens = ['recovery', 'autologin', 'session']
       Token.where(:user_id => id, :action => tokens).delete_all
     end
   end