Browse Source

Fixes #3520 - Make session TTL configurable.

Rolf Schmidt 3 years ago
parent
commit
8dc68ca1e4

+ 41 - 0
app/assets/javascripts/app/controllers/_plugin/session_timeout.coffee

@@ -0,0 +1,41 @@
+class SessionTimeout extends App.Controller
+  constructor: ->
+    super
+
+    lastEvent = 0
+    check_timeout = =>
+      return if new Date().getTime() - 1000 < lastEvent
+      lastEvent = new Date().getTime()
+      @setDelay()
+
+    $(document).off('keyup.session_timeout').on('keyup.session_timeout', check_timeout)
+    $(document).off('mousemove.session_timeout').on('mousemove.session_timeout', check_timeout)
+    @controllerBind('config_update', check_timeout)
+    @controllerBind('session_timeout', @quitApp)
+    @setDelay()
+
+  setDelay: =>
+    return if App.Session.get() is undefined
+    @delay(@quitApp, @getTimeout(), 'session_timeout')
+
+  quitApp: =>
+    return if App.Session.get() is undefined
+    @navigate '#logout'
+
+  getTimeout: ->
+    user    = App.User.find(App.Session.get().id)
+    config  = App.Config.get('session_timeout')
+
+    timeout = -1
+    for key, value of config
+      continue if key is 'default'
+      continue if !user.permission(key)
+      continue if parseInt(value) < timeout
+      timeout = parseInt(value)
+
+    if timeout is -1
+      timeout = parseInt(config['default'])
+
+    return timeout * 1000
+
+App.Config.set('session_timeout', SessionTimeout, 'Plugins')

+ 51 - 0
app/jobs/session_timeout_job.rb

@@ -0,0 +1,51 @@
+class SessionTimeoutJob < ApplicationJob
+  def perform
+    sessions.find_each do |session|
+      perform_session(session)
+    end
+  end
+
+  def perform_session(session)
+    return if !session.data['user_id']
+
+    user = User.find(session.data['user_id'])
+    return if !user
+
+    timeout = get_timeout(user)
+    return if session.data['ping'] > Time.zone.now - timeout.seconds
+
+    self.class.destroy_session(user, session)
+  end
+
+  def self.destroy_session(user, session)
+    PushMessages.send_to(user.id, { event: 'session_timeout' })
+    session.destroy
+  end
+
+  def sessions
+    ActiveRecord::SessionStore::Session.where('updated_at < ?', Time.zone.now - config.values.map(&:to_i).min.seconds)
+  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
+  end
+end

+ 72 - 0
db/migrate/20210414000000_init_session_timeout.rb

@@ -0,0 +1,72 @@
+class InitSessionTimeout < ActiveRecord::Migration[5.2]
+  def change
+
+    return if !Setting.exists?(name: 'system_init_done')
+
+    change_setting_prio('user_create_account', 10)
+    change_setting_prio('user_lost_password', 20)
+
+    two_days = 2.days.seconds
+
+    Setting.create_if_not_exists(
+      title:       'Session Timeout',
+      name:        'session_timeout',
+      area:        'Security::Base',
+      description: 'Defines the session timeout for inactivity of users (in seconds).',
+      options:     {
+        form: [
+          {
+            display: 'Default',
+            null:    false,
+            name:    'default',
+            tag:     'input',
+          },
+          {
+            display: 'admin',
+            null:    false,
+            name:    'admin',
+            tag:     'input',
+          },
+          {
+            display: 'ticket.agent',
+            null:    false,
+            name:    'ticket.agent',
+            tag:     'input',
+          },
+          {
+            display: 'ticket.customer',
+            null:    false,
+            name:    'ticket.customer',
+            tag:     'input',
+          },
+        ],
+      },
+      preferences: {
+        prio: 30,
+      },
+      state:       {
+        'default'         => two_days,
+        'admin'           => two_days,
+        'ticket.agent'    => two_days,
+        'ticket.customer' => two_days,
+      },
+      frontend:    true
+    )
+
+    Scheduler.create_or_update(
+      name:          'Cleanup dead sessions.',
+      method:        'SessionTimeoutJob.perform_now',
+      period:        1.minute,
+      prio:          2,
+      active:        true,
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+  end
+
+  def change_setting_prio(name, prio)
+    setting = Setting.find_by(name: name)
+    setting.preferences[:prio] = prio
+    setting.save
+  end
+end

+ 9 - 0
db/seeds/schedulers.rb

@@ -136,6 +136,15 @@ Scheduler.create_or_update(
   updated_by_id: 1,
   created_by_id: 1,
 )
+Scheduler.create_or_update(
+  name:          'Cleanup dead sessions.',
+  method:        'SessionTimeoutJob.perform_now',
+  period:        1.minute,
+  prio:          2,
+  active:        true,
+  updated_by_id: 1,
+  created_by_id: 1,
+)
 Scheduler.create_or_update(
   name:          'Sync calendars with ical feeds.',
   method:        'Calendar.sync',

+ 46 - 0
db/seeds/settings.rb

@@ -1010,6 +1010,7 @@ Setting.create_if_not_exists(
   },
   state:       true,
   preferences: {
+    prio:       10,
     permission: ['admin.security'],
   },
   frontend:    true
@@ -1035,10 +1036,55 @@ Setting.create_if_not_exists(
   },
   state:       true,
   preferences: {
+    prio:       20,
     permission: ['admin.security'],
   },
   frontend:    true
 )
+Setting.create_if_not_exists(
+  title:       'Session Timeout',
+  name:        'session_timeout',
+  area:        'Security::Base',
+  description: 'Defines the session timeout for inactivity of users (in seconds).',
+  options:     {
+    form: [
+      {
+        display: 'Default',
+        null:    false,
+        name:    'default',
+        tag:     'input',
+      },
+      {
+        display: 'admin',
+        null:    false,
+        name:    'admin',
+        tag:     'input',
+      },
+      {
+        display: 'ticket.agent',
+        null:    false,
+        name:    'ticket.agent',
+        tag:     'input',
+      },
+      {
+        display: 'ticket.customer',
+        null:    false,
+        name:    'ticket.customer',
+        tag:     'input',
+      },
+    ],
+  },
+  preferences: {
+    prio: 30,
+  },
+  state:       {
+    'default'         => 2.days.seconds,
+    'admin'           => 2.days.seconds,
+    'ticket.agent'    => 2.days.seconds,
+    'ticket.customer' => 2.days.seconds,
+  },
+  frontend:    true
+)
 Setting.create_if_not_exists(
   title:       'User email for muliple users',
   name:        'user_email_multiple_use',

+ 19 - 0
spec/factories/active_record/session_store/session.rb

@@ -0,0 +1,19 @@
+FactoryBot.define do
+  factory :'active_record/session_store/session', aliases: %i[active_session] do
+    transient do
+      user { create(:user) }
+    end
+
+    session_id { SecureRandom.hex(16) }
+    data do
+      {
+        'user_id'     => user.id,
+        'ping'        => Time.zone.now,
+        'user_agent'  => 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/89.0.4389.128 Safari/537.36',
+        '_csrf_token' => 'Yq3XiEgXxWPCURa/FvpXmptZCjgWhyPpGGIvZj9Eea0='
+      }
+    end
+    created_at       { Time.zone.now }
+    updated_at       { Time.zone.now }
+  end
+end

+ 97 - 0
spec/jobs/session_timeout_job_spec.rb

@@ -0,0 +1,97 @@
+require 'rails_helper'
+
+RSpec.describe SessionTimeoutJob, type: :job do
+  before do
+    create(:active_session, user: user)
+  end
+
+  context 'with timeout admin' do
+    let(:user) { create(:admin) }
+
+    before do
+      Setting.set('session_timeout', { admin: 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 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 timeout ticket.agent' do
+    let(:user) { create(:agent) }
+
+    before do
+      Setting.set('session_timeout', { 'ticket.agent': 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 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 timeout ticket.customer' do
+    let(:user) { create(:customer) }
+
+    before do
+      Setting.set('session_timeout', { 'ticket.customer': 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 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 timeout agent and customer' do
+    let(:user) { create(:agent_and_customer) }
+
+    before do
+      Setting.set('session_timeout', { 'ticket.customer': 1.second.to_s, 'ticket.agent': 2.hours.to_s })
+    end
+
+    it 'does kill the session' do
+      travel_to 1.day.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.hour.from_now
+      expect { described_class.perform_now }.to change(ActiveRecord::SessionStore::Session, :count).by(0)
+    end
+  end
+
+  context 'with timeout default' do
+    let(:user) { create(:customer) }
+
+    before do
+      Setting.set('session_timeout', { 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 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
+end

+ 81 - 0
spec/system/dashboard_spec.rb

@@ -32,4 +32,85 @@ RSpec.describe 'Dashboard', type: :system, authenticated_as: true do
       expect(User.find_by(firstname: 'Nick').roles).to eq([Role.find_by(name: 'Public')])
     end
   end
+
+  context 'Session Timeout' do
+    let(:admin) { create(:admin) }
+    let(:agent) { create(:agent) }
+    let(:customer) { create(:customer) }
+
+    before do
+      ensure_websocket(check_if_pinged: false)
+    end
+
+    context 'Logout by frontend plugin - Default', authenticated_as: :authenticate do
+      def authenticate
+        Setting.set('session_timeout', { default: '1' })
+        admin
+      end
+
+      it 'does logout user' do
+        expect(page).to have_text('Sign in', wait: 15)
+      end
+
+      it 'does not logout user', authenticated_as: :admin do
+        sleep 1.5
+        expect(page).to have_no_text('Sign in', wait: 0)
+      end
+    end
+
+    context 'Logout by frontend plugin - Setting change', authenticated_as: :admin do
+      it 'does logout user' do
+        expect(page).to have_no_text('Sign in')
+        Setting.set('session_timeout', { default: '1' })
+        expect(page).to have_text('Sign in', wait: 15)
+      end
+    end
+
+    context 'Logout by frontend plugin - Admin', authenticated_as: :authenticate do
+      def authenticate
+        Setting.set('session_timeout', { admin: '1', default: '1000' })
+        admin
+      end
+
+      it 'does logout user' do
+        expect(page).to have_text('Sign in', wait: 15)
+      end
+    end
+
+    context 'Logout by frontend plugin - Agent', authenticated_as: :authenticate do
+      def authenticate
+        Setting.set('session_timeout', { 'ticket.agent': '1', default: '1000' })
+        agent
+      end
+
+      it 'does logout user' do
+        expect(page).to have_text('Sign in', wait: 15)
+      end
+    end
+
+    context 'Logout by frontend plugin - Customer', authenticated_as: :authenticate do
+      def authenticate
+        Setting.set('session_timeout', { 'ticket.customer': '1', default: '1000' })
+        customer
+      end
+
+      it 'does logout user' do
+        expect(page).to have_text('Sign in', wait: 15)
+      end
+    end
+
+    context 'Logout by SessionTimeoutJob - destroy_session' 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
+        session = ActiveRecord::SessionStore::Session.all.detect { |s| s.data['user_id'] == admin.id }
+        SessionTimeoutJob.destroy_session(admin, session)
+        expect(page).to have_text('Sign in', wait: 15)
+      end
+    end
+  end
 end