Allow "stay logged in" from multiple browsers (#10840).

Patch by Gregor Schmidt.

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

diff --git a/app/models/token.rb b/app/models/token.rb
index a5ca18aa419264b4ba3d7cb993bd44609318c684..7966220530ce72c7374957f8293c46b9cea78a84 100644 (file)
--- a/app/models/token.rb
+++ b/app/models/token.rb
@@ -25,18 +25,70 @@ class Token < ActiveRecord::Base
   cattr_accessor :validity_time
   self.validity_time = 1.day
 
+  class << self
+    attr_reader :actions
+
+    def add_action(name, options)
+      options.assert_valid_keys(:max_instances, :validity_time)
+      @actions ||= {}
+      @actions[name.to_s] = options
+    end
+  end
+
+  add_action :api,       max_instances: 1,  validity_time: nil
+  add_action :autologin, max_instances: 10, validity_time: Proc.new { Setting.autologin.to_i.days }
+  add_action :feeds,     max_instances: 1,  validity_time: nil
+  add_action :recovery,  max_instances: 1,  validity_time: Proc.new { Token.validity_time }
+  add_action :register,  max_instances: 1,  validity_time: Proc.new { Token.validity_time }
+  add_action :session,   max_instances: 10, validity_time: nil
+
   def generate_new_token
     self.value = Token.generate_token_value
   end
 
   # Return true if token has expired
   def expired?
-    return Time.now > self.created_on + self.class.validity_time
+    return created_on < self.class.invalid_when_created_before(action)
+  end
+
+  def max_instances
+    Token.actions.has_key?(action) ? Token.actions[action][:max_instances] : 1
+  end
+
+  def self.invalid_when_created_before(action = nil)
+    if Token.actions.has_key?(action)
+      validity_time = Token.actions[action][:validity_time]
+      validity_time = validity_time.call(action) if validity_time.respond_to? :call
+    else
+      validity_time = self.validity_time
+    end
+
+    if validity_time.nil?
+      0
+    else
+      Time.now - validity_time
+    end
   end
 
   # Delete all expired tokens
   def self.destroy_expired
-    Token.where("action NOT IN (?) AND created_on < ?", ['feeds', 'api', 'session'], Time.now - validity_time).delete_all
+    t = Token.arel_table
+
+    # Unknown actions have default validity_time
+    condition = t[:action].not_in(self.actions.keys).and(t[:created_on].lt(invalid_when_created_before))
+
+    self.actions.each do |action, options|
+      validity_time = invalid_when_created_before(action)
+
+      # Do not delete tokens, which don't become invalid
+      next if validity_time.nil?
+
+      condition = condition.or(
+        t[:action].eq(action).and(t[:created_on].lt(validity_time))
+      )
+    end
+
+    Token.where(condition).delete_all
   end
 
   # Returns the active user who owns the key for the given action
@@ -80,8 +132,8 @@ class Token < ActiveRecord::Base
   def delete_previous_tokens
     if user
       scope = Token.where(:user_id => user.id, :action => action)
-      if action == 'session'
-        ids = scope.order(:updated_on => :desc).offset(9).ids
+      if max_instances > 1
+        ids = scope.order(:updated_on => :desc).offset(max_instances - 1).ids
         if ids.any?
           Token.delete(ids)
         end

diff --git a/test/unit/token_test.rb b/test/unit/token_test.rb
index 92a7f12f79f385a40b1e49d63ad0aebd5b798645..f25fcfd8cb92fedb9fbd76f3318fade2cc46306f 100644 (file)
--- a/test/unit/token_test.rb
+++ b/test/unit/token_test.rb
@@ -29,31 +29,34 @@ class TokenTest < ActiveSupport::TestCase
 
   def test_create_should_remove_existing_tokens
     user = User.find(1)
-    t1 = Token.create(:user => user, :action => 'autologin')
-    t2 = Token.create(:user => user, :action => 'autologin')
+    t1 = Token.create(:user => user, :action => 'register')
+    t2 = Token.create(:user => user, :action => 'register')
     assert_not_equal t1.value, t2.value
     assert !Token.exists?(t1.id)
     assert  Token.exists?(t2.id)
   end
 
-  def test_create_session_token_should_keep_last_10_tokens
+  def test_create_session_or_autologin_token_should_keep_last_10_tokens
     Token.delete_all
     user = User.find(1)
 
-    assert_difference 'Token.count', 10 do
-      10.times { Token.create!(:user => user, :action => 'session') }
-    end
+    ["autologin", "session"].each do |action|
+      assert_difference 'Token.count', 10 do
+        10.times { Token.create!(:user => user, :action => action) }
+      end
 
-    assert_no_difference 'Token.count' do
-      Token.create!(:user => user, :action => 'session')
+      assert_no_difference 'Token.count' do
+        Token.create!(:user => user, :action => action)
+      end
     end
   end
 
-  def test_destroy_expired_should_not_destroy_feeds_and_api_tokens
+  def test_destroy_expired_should_not_destroy_session_feeds_and_api_tokens
     Token.delete_all
 
     Token.create!(:user_id => 1, :action => 'api', :created_on => 7.days.ago)
     Token.create!(:user_id => 1, :action => 'feeds', :created_on => 7.days.ago)
+    Token.create!(:user_id => 1, :action => 'session', :created_on => 7.days.ago)
 
     assert_no_difference 'Token.count' do
       assert_equal 0, Token.destroy_expired
@@ -63,12 +66,24 @@ class TokenTest < ActiveSupport::TestCase
   def test_destroy_expired_should_destroy_expired_tokens
     Token.delete_all
 
-    Token.create!(:user_id => 1, :action => 'autologin', :created_on => 7.days.ago)
-    Token.create!(:user_id => 2, :action => 'autologin', :created_on => 3.days.ago)
-    Token.create!(:user_id => 3, :action => 'autologin', :created_on => 1.hour.ago)
+    # Expiration of autologin tokens is determined by Setting.autologin
+    Setting.autologin = "7"
+    Token.create!(:user_id => 2, :action => 'autologin', :created_on => 3.weeks.ago)
+    Token.create!(:user_id => 3, :action => 'autologin', :created_on => 3.days.ago)
+
+    # Expiration of register and recovery tokens is determined by Token.validity_time
+    Token.create!(:user_id => 1, :action => 'register', :created_on => 7.days.ago)
+    Token.create!(:user_id => 3, :action => 'register', :created_on => 7.hours.ago)
+
+    Token.create!(:user_id => 2, :action => 'recovery', :created_on => 3.days.ago)
+    Token.create!(:user_id => 3, :action => 'recovery', :created_on => 3.hours.ago)
+
+    # Expiration of tokens with unknown action is determined by Token.validity_time
+    Token.create!(:user_id => 2, :action => 'unknown_action', :created_on => 2.days.ago)
+    Token.create!(:user_id => 3, :action => 'unknown_action', :created_on => 2.hours.ago)
 
-    assert_difference 'Token.count', -2 do
-      assert_equal 2, Token.destroy_expired
+    assert_difference 'Token.count', -4 do
+      assert_equal 4, Token.destroy_expired
     end
   end