Skip to content

Add admin-configurable session timeout and security hardening#3536

Open
Rolling2405 wants to merge 4 commits intoDARIAEngineering:mainfrom
Rolling2405:feature/session-security
Open

Add admin-configurable session timeout and security hardening#3536
Rolling2405 wants to merge 4 commits intoDARIAEngineering:mainfrom
Rolling2405:feature/session-security

Conversation

@Rolling2405
Copy link
Copy Markdown
Contributor

@Rolling2405 Rolling2405 commented Apr 8, 2026

I rule and have completed some work on Case Manager that's ready for review!

This lets admins configure how long a user's session lasts before they are automatically signed out, improving security for funds that need stricter session controls.

This pull request makes the following changes:

  • Add new Config setting session_timeout (enum value 26) so admins can choose a session duration
  • Add Timeoutable concern for session expiration checking
  • Add before_action tenant-scoped timeout guard in ApplicationController

Notes:

For reviewer:

  • Adjust the title to explain what it does for the notification email to the listserv.
  • Tag this PR:
    • feature if it contains a feature, fix, or similar. This is anything that contains a user-facing fix in some way, such as frontend changes, alterations to backend behavior, or bug fixes.
    • dependencies if it contains library upgrades or similar. This is anything that upgrades any dependency, such as a Gemfile update or npm package upgrade.
  • If it contains neither, no need to tag this PR.

Rolling2405 and others added 2 commits April 7, 2026 20:04
Session Timeout:
- New Config key 'session_timeout' with options: 15, 30 (default), 60, 120, 180 minutes
- Reduced default from 2 hours to 30 minutes (security best practice)
- User model reads timeout dynamically from fund Config via timeout_in method
- Full validation: only accepts approved timeout values

Cookie Security:
- Added httponly: true to session cookie (prevents JS access)
- Added same_site: :lax (CSRF protection, allows normal navigation)
- Existing secure: true in production already enforced

Session Fixation Protection:
- Role changes (admin/data_volunteer/cm) now force logout the affected
  user via session_validity_token invalidation (unless changing own role)
- Uses existing Warden after_fetch hook that checks validity tokens

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix config enum value: session_timeout 27 -> 26 (next sequential after main's max of 25)
- Add tenant guard in Config.session_timeout: returns default 30min when no tenant set
  (prevents error during Devise initialization before tenant is established)
- Add comprehensive tests for session timeout:
  - Enum value correctness
  - Default behavior without tenant
  - Configured timeout retrieval
  - Fallback for invalid values
  - All valid options (15, 30, 60, 120, 180)
  - Validation of timeout values
  - Default constant value

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Test User#timeout_in returns configured session timeout
- Test session store has httponly and same_site:lax attributes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Rolling2405 added a commit to Rolling2405/dcaf_case_management that referenced this pull request Apr 12, 2026
Change days_to_keep_archived_patients enum value from 26 to 27
to avoid collision with session_timeout: 26 in PR DARIAEngineering#3536.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add tests for:
- Controller integration: force_logout fires when admin changes user role
- Cross-fund isolation: one fund's timeout config doesn't leak to another
- User#timeout_in returns correct ActiveSupport::Duration object
- Edge cases: missing config record, nil options fallback to default

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant