From: Jean-Philippe Lang Date: Thu, 12 Jan 2017 20:34:08 +0000 (+0000) Subject: Allow "stay logged in" from multiple browsers (#10840). X-Git-Tag: 3.4.0~435 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=5d4b5fd1f68ab7aef4ef43d82d3cb378079aed31;p=redmine.git 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 a5ca18aa4..796622053 100644 --- 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 92a7f12f7..f25fcfd8c 100644 --- 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