Fixes #3605 - session timeout does regulary logout active users before timeout because of dead old sessions.

app/jobs/session_timeout_job.rb

@@ -2,58 +2,36 @@
 
 class SessionTimeoutJob < ApplicationJob
   def perform
-    sessions.find_each do |session|
+    sessions.each do |session|
       perform_session(session)
     end
   end
 
   def perform_session(session)
-    return if !session.data['user_id']
 
     # user is optional because it can be deleted already
-    user = User.find_by(id: session.data['user_id'])
-    if user
-      timeout = get_timeout(user)
-      return if timeout < 1
-      return if session.data['ping'] > timeout.seconds.ago
+    if session.user?
+      return if session.active?
+
+      # if the user has no active sessions then we
+      # make sure to definitely log him out if there
+      # is any frontends opened
+      if !active_session(session.user)
+        session.frontend_timeout
+      end
     end
 
-    self.class.destroy_session(user, session)
-  end
-
-  def self.destroy_session(user, session)
-
-    # user is optional because it can be deleted already
-    if user
-      PushMessages.send_to(user.id, { event: 'session_timeout' })
-    end
     session.destroy
   end
 
+  def active_session(user)
+    @active_session ||= {}
+    return @active_session[user.id] if @active_session[user.id].present?
+
+    @active_session[user.id] = sessions.detect { |session| session.active? && session.user.id == user.id }
+  end
+
   def sessions
-    ActiveRecord::SessionStore::Session.where('updated_at < ?', config.values.map(&:to_i).min.seconds.ago)
-  end
-
-  def config
-    Setting.get('session_timeout')
-  end
-
-  def get_timeout(user)
-    permissions = Permission.where(id: user.permissions_with_child_ids).pluck(:name)
-
-    timeout = -1
-    config.each do |key, value|
-      next if key == 'default'
-      next if permissions.exclude?(key)
-      next if value.to_i < timeout
-
-      timeout = value.to_i
-    end
-
-    if timeout < 1
-      timeout = config['default'].to_i
-    end
-
-    timeout
+    @sessions ||= ActiveRecord::SessionStore::Session.order(updated_at: :desc).limit(10_000).map { |session| SessionTimeoutJob::Session.new(session) }
   end
 end

app/jobs/session_timeout_job/session.rb

@@ -0,0 +1,53 @@
+# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/
+
+class SessionTimeoutJob::Session
+  attr_accessor :session, :user
+
+  delegate :destroy, to: :session
+
+  def initialize(session)
+    @session = session
+    @user    = User.find_by(id: session.data['user_id'])
+  end
+
+  def user?
+    user.present?
+  end
+
+  def active?
+    return true if !user?
+    return true if timeout < 1
+    return true if session.data['ping'] > timeout.seconds.ago
+  end
+
+  def frontend_timeout
+    return if !user?
+
+    PushMessages.send_to(user.id, { event: 'session_timeout' })
+  end
+
+  def timeout
+    @timeout ||= begin
+      permissions = Permission.where(id: user.permissions_with_child_ids).pluck(:name)
+
+      result = -1
+      config.each do |key, value|
+        next if key == 'default'
+        next if permissions.exclude?(key)
+        next if value.to_i < result
+
+        result = value.to_i
+      end
+
+      if result < 1
+        result = config['default'].to_i
+      end
+
+      result
+    end
+  end
+
+  def config
+    Setting.get('session_timeout')
+  end
+end

spec/jobs/session_timeout_job_spec.rb

@@ -150,4 +150,37 @@ RSpec.describe SessionTimeoutJob, type: :job do
       expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(0)
     end
   end
+
+  context 'with timeout and a dead session in the past' do
+    let(:user) { create(:admin) }
+
+    before do
+      Setting.set('session_timeout', { admin: 30.minutes.to_s })
+      travel_to 10.hours.ago
+      create(:active_session, user: user)
+      travel_to 10.hours.from_now
+    end
+
+    it 'does a frontend logout for the user' do
+      allow(PushMessages).to receive(:send_to)
+      travel_to 1.hour.from_now
+      described_class.perform_now
+      expect(PushMessages).to have_received(:send_to).with(user.id, { event: 'session_timeout' }).twice
+    end
+
+    it 'does not init a frontend logout for the user because he does not exist anymore' do
+      allow(PushMessages).to receive(:send_to)
+      user.destroy
+      travel_to 1.hour.from_now
+      described_class.perform_now
+      expect(PushMessages).not_to have_received(:send_to).with(user.id, { event: 'session_timeout' })
+    end
+
+    it 'does not init a frontend logout for the user because of an active session' do
+      allow(PushMessages).to receive(:send_to)
+      travel_to 1.minute.from_now
+      described_class.perform_now
+      expect(PushMessages).not_to have_received(:send_to).with(user.id, { event: 'session_timeout' })
+    end
+  end
 end

spec/system/dashboard_spec.rb

@@ -102,16 +102,16 @@ RSpec.describe 'Dashboard', type: :system, authenticated_as: true do
       end
     end
 
-    context 'Logout by SessionTimeoutJob - destroy_session' do
+    context 'Logout by SessionTimeoutJob - frontend_timeout' do
       it 'does logout user', authenticated_as: :admin do
 
         # because of the websocket server running in the same
         # process and the checks in the frontend it is really
         # hard test the SessionTimeoutJob.perform_now here
         # so we only check the session killing code and use
-        # backend tests for the rest
+        # backend tests for the reset
         session = ActiveRecord::SessionStore::Session.all.detect { |s| s.data['user_id'] == admin.id }
-        SessionTimeoutJob.destroy_session(admin, session)
+        SessionTimeoutJob::Session.new(session).frontend_timeout
         expect(page).to have_text('Due to inactivity you are automatically logged out.', wait: 20)
       end
     end