Add admin-configurable session timeout and security hardening

app/controllers/users_controller.rb

@@ -65,16 +65,19 @@ def create
 
   def change_role_to_admin
     @user.update role: 'admin'
+    @user.force_logout unless @user == current_user
     render 'edit'
   end
 
   def change_role_to_data_volunteer
     @user.update role: 'data_volunteer' if user_not_demoting_themself?(@user)
+    @user.force_logout unless @user == current_user
     render 'edit'
   end
 
   def change_role_to_cm
     @user.update role: 'cm' if user_not_demoting_themself?(@user)
+    @user.force_logout unless @user == current_user
     render 'edit'
   end
 

app/models/config.rb

@@ -27,7 +27,8 @@ class Config < ApplicationRecord
     show_patient_identifier: 'Enter "yes" to show the patient\' Daria Identifier on the patient information tab.',
     display_practical_support_attachment_url: 'CAUTION: Whether or not to allow people to enter attachment URLs for practical support entries; for example, a link to a file in Google Drive. Please ensure that any system storing these is properly secured by your fund!',
     display_practical_support_waiver: 'For funds that use waivers for practical support recipients. Enables the display of a checkbox for indicating if a patient has signed a practical support waiver. ',
-    display_consent_to_survey: 'For funds that do followup surveys, this displays a Consent to Survey checkbox under the Patient Information tab.'
+    display_consent_to_survey: 'For funds that do followup surveys, this displays a Consent to Survey checkbox under the Patient Information tab.',
+    session_timeout: 'Session inactivity timeout in minutes. Options: 15, 30 (default), 60, 120, 180.'
   }.freeze
 
   # Whether a config should show a current options dropdown to the right
@@ -65,7 +66,8 @@ class Config < ApplicationRecord
     aggregate_statistics: false,
     hide_standard_dropdown_values: false,
     county: nil,
-    time_zone: "Eastern"
+    time_zone: "Eastern",
+    session_timeout: 30
   }.freeze
 
   enum :config_key, {
@@ -94,7 +96,8 @@ class Config < ApplicationRecord
     show_patient_identifier: 22,
     display_practical_support_attachment_url: 23,
     display_practical_support_waiver: 24,
-    display_consent_to_survey: 25
+    display_consent_to_survey: 25,
+    session_timeout: 26
   }
 
   # which fields are URLs (run special validation only on those)
@@ -165,6 +168,8 @@ class Config < ApplicationRecord
       [:validate_singleton, :validate_yes_or_no],
     display_consent_to_survey:
       [:validate_singleton, :validate_yes_or_no],
+    session_timeout:
+      [:validate_singleton, :validate_session_timeout],
   }.freeze
 
   before_validation :clean_config_value
@@ -268,6 +273,17 @@ def self.display_consent_to_survey
     config_to_bool('display_consent_to_survey')
   end
 
+  SESSION_TIMEOUT_OPTIONS = [15, 30, 60, 120, 180].freeze
+
+  def self.session_timeout
+    return DEFAULTS[:session_timeout].minutes if ActsAsTenant.current_tenant.nil?
+
+    timeout = Config.find_or_create_by(config_key: 'session_timeout').options.try :last
+    timeout = timeout.to_i if timeout.present?
+    timeout = DEFAULTS[:session_timeout] unless SESSION_TIMEOUT_OPTIONS.include?(timeout)
+    timeout.minutes
+  end
+
   private
     ### Generic Functions
 
@@ -413,6 +429,14 @@ def validate_shared_reset_days
       end
     end
 
+    ### Session timeout
+
+    def validate_session_timeout
+      unless SESSION_TIMEOUT_OPTIONS.include?(options.last.to_i)
+        "Must be one of: #{SESSION_TIMEOUT_OPTIONS.join(', ')} minutes"
+      end
+    end
+
     def validate_length
       total_length = 0
       options.each do |option|

app/models/user.rb

@@ -127,6 +127,11 @@ def self.search(name_or_email_str)
         .limit(SEARCH_LIMIT)
   end
 
+  # Dynamic session timeout — reads from fund's Config, falls back to Devise default
+  def timeout_in
+    Config.session_timeout
+  end
+
   def force_logout
     update_attribute(:session_validity_token, nil)
   end

config/initializers/devise.rb

@@ -196,7 +196,8 @@ def route(scope)
   # ==> Configuration for :timeoutable
   # The time you want to timeout the user session without activity. After this
   # time the user will be asked for credentials again. Default is 30 minutes.
-  config.timeout_in = 2.hours
+  # Base default — overridden dynamically per-fund via Config.session_timeout
+  config.timeout_in = 30.minutes
 
   # ==> Configuration for :lockable
   # Defines which strategy will be used to lock an account.

config/initializers/session_store.rb

@@ -2,4 +2,6 @@
 
 Rails.application.config.session_store :active_record_store,
                                        key: '_my_app_session',
-                                       secure: Rails.env.production? || Rails.env.staging?
+                                       secure: Rails.env.production? || Rails.env.staging?,
+                                       httponly: true,
+                                       same_site: :lax

test/controllers/users_controller_test.rb

@@ -208,6 +208,32 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
         assert_equal 'admin', @user.role
       end
     end
+
+    describe 'force_logout on role change' do
+      it 'should clear session_validity_token when admin changes another user role' do
+        @user_2.update_column(:session_validity_token, 'original-token')
+        patch change_role_to_admin_path(@user_2)
+        @user_2.reload
+        assert_nil @user_2.session_validity_token
+      end
+
+      it 'should not clear own session_validity_token when changing own role' do
+        original_token = @user.session_validity_token
+        # Admin changing self to admin (no-op role wise, but force_logout guarded)
+        patch change_role_to_admin_path(@user)
+        @user.reload
+        assert_equal original_token, @user.session_validity_token
+      end
+
+      it 'should clear session_validity_token on demotion to cm' do
+        @user_2.update role: 'admin'
+        @user_2.update_column(:session_validity_token, 'active-session-token')
+        patch change_role_to_cm_path(@user_2)
+        @user_2.reload
+        assert_equal 'cm', @user_2.role
+        assert_nil @user_2.session_validity_token
+      end
+    end
   end
 
   describe 'toggle_lock method' do

test/models/config_test.rb

@@ -363,4 +363,128 @@ class ConfigTest < ActiveSupport::TestCase
     end
 
   end
+
+  describe 'session_timeout' do
+    it 'should have enum value 26' do
+      assert_equal 26, Config.config_keys['session_timeout']
+    end
+
+    it 'should return default 30 minutes when no tenant is set' do
+      ActsAsTenant.without_tenant do
+        assert_equal 30.minutes, Config.session_timeout
+      end
+    end
+
+    it 'should return configured timeout value when tenant is set' do
+      c = Config.find_or_create_by(config_key: 'session_timeout')
+      c.config_value = { options: ["60"] }
+      c.save!
+      assert_equal 60.minutes, Config.session_timeout
+    end
+
+    it 'should fall back to default for invalid timeout values' do
+      c = Config.find_or_create_by(config_key: 'session_timeout')
+      c.config_value = { options: ["999"] }
+      c.save!(validate: false)
+      assert_equal 30.minutes, Config.session_timeout
+    end
+
+    it 'should accept all valid timeout options' do
+      [15, 30, 60, 120, 180].each do |minutes|
+        c = Config.find_or_create_by(config_key: 'session_timeout')
+        c.config_value = { options: [minutes.to_s] }
+        c.save!
+        assert_equal minutes.minutes, Config.session_timeout
+      end
+    end
+
+    it 'should validate session_timeout values' do
+      c = Config.find_or_create_by(config_key: 'session_timeout')
+
+      c.config_value = { options: ["30"] }
+      assert c.valid?
+
+      c.config_value = { options: ["60"] }
+      assert c.valid?
+
+      c.config_value = { options: ["45"] }
+      refute c.valid?
+
+      c.config_value = { options: ["0"] }
+      refute c.valid?
+    end
+
+    it 'should have default value of 30' do
+      assert_equal 30, Config::DEFAULTS[:session_timeout]
+    end
+
+    it 'should be used by User#timeout_in for dynamic session timeout' do
+      user = create :user
+      c = Config.find_or_create_by(config_key: 'session_timeout')
+      c.config_value = { options: ["120"] }
+      c.save!
+      assert_equal 120.minutes, user.timeout_in
+    end
+
+    it 'should have session store configured with security attributes' do
+      store_config = Rails.application.config.session_options
+      assert_equal true, store_config[:httponly]
+      assert_equal :lax, store_config[:same_site]
+    end
+
+    describe 'cross-fund isolation' do
+      it 'should not leak timeout config from one fund to another' do
+        # Configure timeout for current tenant
+        c = Config.find_or_create_by(config_key: 'session_timeout')
+        c.config_value = { options: ["120"] }
+        c.save!
+        assert_equal 120.minutes, Config.session_timeout
+
+        # Switch to a different tenant
+        other_fund = create :fund, name: 'OtherFund', full_name: 'Other Fund'
+        ActsAsTenant.current_tenant = other_fund
+        ActsAsTenant.test_tenant = other_fund
+
+        # The other fund should get the default, not fund 1's value
+        assert_equal 30.minutes, Config.session_timeout
+      end
+    end
+
+    describe 'missing config record' do
+      it 'should return default timeout when config record does not exist' do
+        Config.where(config_key: 'session_timeout').destroy_all
+        assert_equal 30.minutes, Config.session_timeout
+      end
+
+      it 'should return default timeout when config record has nil options' do
+        c = Config.find_or_create_by(config_key: 'session_timeout')
+        c.update_columns(config_value: { 'options' => [nil] })
+        assert_equal 30.minutes, Config.session_timeout
+      end
+    end
+
+    describe 'User#timeout_in' do
+      it 'should return an ActiveSupport::Duration' do
+        user = create :user
+        result = user.timeout_in
+        assert_kind_of ActiveSupport::Duration, result
+      end
+
+      it 'should reflect the configured session timeout' do
+        c = Config.find_or_create_by(config_key: 'session_timeout')
+        c.config_value = { options: ["60"] }
+        c.save!
+
+        user = create :user
+        assert_equal 60.minutes, user.timeout_in
+        assert_equal 3600, user.timeout_in.to_i
+      end
+
+      it 'should return default 30 minutes when unconfigured' do
+        Config.where(config_key: 'session_timeout').destroy_all
+        user = create :user
+        assert_equal 30.minutes, user.timeout_in
+      end
+    end
+  end
 end