From c49da0a4c9696b204a930153df21650555ae9c47 Mon Sep 17 00:00:00 2001
From: Rolf Schmidt <rolf.schmidt@zammad.com>
Date: Tue, 8 Jun 2021 09:31:28 +0100
Subject: [PATCH] Fixes #3600 - wrong behaviour for the disabled option in
 session timeout.

---
 .../_plugin/session_timeout.coffee            |  9 ++--
 app/jobs/session_timeout_job.rb               |  3 +-
 spec/jobs/session_timeout_job_spec.rb         | 48 +++++++++++++++++++
 spec/system/dashboard_spec.rb                 | 24 ++++++++++
 4 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/app/assets/javascripts/app/controllers/_plugin/session_timeout.coffee b/app/assets/javascripts/app/controllers/_plugin/session_timeout.coffee
index 3b184e3a4..44987d8c1 100644
--- a/app/assets/javascripts/app/controllers/_plugin/session_timeout.coffee
+++ b/app/assets/javascripts/app/controllers/_plugin/session_timeout.coffee
@@ -35,7 +35,10 @@ class SessionTimeout extends App.Controller
   checkLogout: =>
     return if App.Session.get() is undefined
 
-    @timeTillLogout = @currentTime() - (@lastEvent + @getTimeout())
+    timeout = @getTimeout()
+    return if timeout < 1
+
+    @timeTillLogout = @currentTime() - (@lastEvent + timeout)
 
     # close logut warning
     if @timeTillLogout < @showLogoutWarningBefore
@@ -58,7 +61,7 @@ class SessionTimeout extends App.Controller
     return if App.Session.get() is undefined
 
     @logoutWarningClose()
-  
+
     App.Auth.logout(false, =>
       @navigate '#session_timeout'
     )
@@ -74,7 +77,7 @@ class SessionTimeout extends App.Controller
       continue if parseInt(value) < timeout
       timeout = parseInt(value)
 
-    if timeout is -1
+    if timeout < 1
       timeout = parseInt(config['default'])
 
     return timeout * 1000
diff --git a/app/jobs/session_timeout_job.rb b/app/jobs/session_timeout_job.rb
index 947313c09..da8f8f267 100644
--- a/app/jobs/session_timeout_job.rb
+++ b/app/jobs/session_timeout_job.rb
@@ -14,6 +14,7 @@ class SessionTimeoutJob < ApplicationJob
     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
     end
 
@@ -49,7 +50,7 @@ class SessionTimeoutJob < ApplicationJob
       timeout = value.to_i
     end
 
-    if timeout == -1
+    if timeout < 1
       timeout = config['default'].to_i
     end
 
diff --git a/spec/jobs/session_timeout_job_spec.rb b/spec/jobs/session_timeout_job_spec.rb
index c62f7a51d..ae42f8367 100644
--- a/spec/jobs/session_timeout_job_spec.rb
+++ b/spec/jobs/session_timeout_job_spec.rb
@@ -102,4 +102,52 @@ RSpec.describe SessionTimeoutJob, type: :job do
       expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(0)
     end
   end
+
+  context 'with timeout fallback from admin to default' do
+    let(:user) { create(:admin) }
+
+    before do
+      Setting.set('session_timeout', { admin: '0', default: 30.minutes.to_s })
+    end
+
+    it 'does kill the session' do
+      travel_to 1.hour.from_now
+      expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(-1)
+    end
+
+    it 'does also kill the session of deleted users' do
+      user.destroy
+      travel_to 1.hour.from_now
+      expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(-1)
+    end
+
+    it 'does not kill the session' do
+      travel_to 1.minute.from_now
+      expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(0)
+    end
+  end
+
+  context 'with timeouts all disabled' do
+    let(:user) { create(:admin) }
+
+    before do
+      Setting.set('session_timeout', { admin: '0', default: '0' })
+    end
+
+    it 'does not kill the session because all timeouts are disabled in 1 hour' do
+      travel_to 1.hour.from_now
+      expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(0)
+    end
+
+    it 'does also kill the session of deleted users' do
+      user.destroy
+      travel_to 1.hour.from_now
+      expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(-1)
+    end
+
+    it 'does not kill the session because all timeouts are disabled in 1 minute' do
+      travel_to 1.minute.from_now
+      expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(0)
+    end
+  end
 end
diff --git a/spec/system/dashboard_spec.rb b/spec/system/dashboard_spec.rb
index 9821c8a9e..0e5a50c45 100644
--- a/spec/system/dashboard_spec.rb
+++ b/spec/system/dashboard_spec.rb
@@ -115,5 +115,29 @@ RSpec.describe 'Dashboard', type: :system, authenticated_as: true do
         expect(page).to have_text('Due to inactivity you are automatically logged out.', wait: 20)
       end
     end
+
+    context 'Logout by frontend plugin - Fallback from admin to default', authenticated_as: :authenticate do
+      def authenticate
+        Setting.set('session_timeout', { admin: '0', default: '1000' })
+        admin
+      end
+
+      it 'does not logout user', authenticated_as: :admin do
+        sleep 1.5
+        expect(page).to have_no_text('Due to inactivity you are automatically logged out.', wait: 0)
+      end
+    end
+
+    context 'Logout by frontend plugin - No logout because timeouts are disabled', authenticated_as: :authenticate do
+      def authenticate
+        Setting.set('session_timeout', { admin: '0', default: '0' })
+        admin
+      end
+
+      it 'does not logout user', authenticated_as: :admin do
+        sleep 1.5
+        expect(page).to have_no_text('Due to inactivity you are automatically logged out.', wait: 0)
+      end
+    end
   end
 end