Skip to content

Conversation

@theiris6
Copy link

@theiris6 theiris6 commented Jul 22, 2025

Description

Upstream PR :
thoth-tech#61
This PR addresses two critical security vulnerabilities in the authentication system:

Session Binding Vulnerability: The previous implementation used overly strict session binding that locked out legitimate users when their IP/User-Agent changed while still being vulnerable to session hijacking across different user accounts.

Session Fixation Vulnerability: The current authentication system allows session tokens to remain valid after logout if the logout request is intercepted, creating a security risk where stolen tokens can be reused.

Dependencies

Database migration required to add new columns to auth_tokens table
Configuration settings in initializers
Fixes # Session Binding/Fixation
Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update

How Has This Been Tested?

  • Burp Suite
  • Postman

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation if appropriate
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have created or extended unit tests to address my new additions
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

If you have any questions, please contact @macite or @jakerenzella.

Replaced strict IP/UA binding with flexible multi-level approach
Added IP history tracking with JSON serialization
Implemented two-phase token invalidation for secure logouts
Added maximum token lifetime checks
Added activity timestamp tracking
Added config-based security settings

config/initializers/session_security.rb:

Added configuration for security settings (binding strictness, timeouts)
Configurable settings for IP changes, suspicious activity detection
Set defaults for token lifetime and enforcement windows

config/initializers/reload_authentication.rb:

Added initializer to ensure proper module loading
Force reload of authentication helpers during application startup

db/migrate/20250428135250_add_session_binding_columns_to_auth_tokens.rb:

Added columns for IP history (last_seen_ip, ip_history)
Added suspicious_activity_detected_at for grace period tracking
Added columns for flexible IP binding implementation

db/migrate/20250429074259_add_timestamps_to_auth_tokens.rb:

Added invalidation_requested_at column for two-phase logout
Added last_activity_at for token activity tracking
Added index on invalidation_requested_at for performance

AuthToken.column_names:

Added documentation for new column structure and usage

db/schema.rb:

Updated schema with new auth_tokens table structure

Security validation test results confirm both issues are fixed:

Session tokens cannot be used with different usernames (binding fix)
Session tokens are properly invalidated after logout (fixation fix)
-Added the missing documentation for new column structure and usage

auth_token.rb
- Added `destroy_invalidated_tokens` to clean up tokens marked for invalidation
- Implemented `initialize_session_binding` to record IP and User-Agent at token creation
- Introduced `ip_history_array` and `add_ip_to_history` to track and update IP usage
- Added `invalidate` method to mark tokens for invalidation via timestamp
- These additions support robust session binding enforcement and replay prevention

20241025050957_add_scorm_feat.rb
- Change `unless` to `if` in the migration file
- Wrapped `add_column`, `remove_column`, and `rename_column` operations with `column_exists?` checks to avoid migration crashes
- Guarded index operations with `index_exists?` to prevent duplication errors during re-runs
- Migrated SCORM-related columns to be added only if missing in `tasks` and `task_definitions`
- Applied safer handling for polymorphic column renaming and indexing in `task_comments`
- Added corresponding `down` method for reversibility with conditional checks

20250419030255_add_session_binding_to_auth_tokens.rb
- Added migration to include `session_ip` and `session_user_agent` columns in auth_tokens table
- Created `destroy_invalidated_tokens` method to clean up tokens marked for invalidation
- Implemented `initialize_session_binding` to store IP/User-Agent on token creation
- Added helpers: `ip_history_array`, `add_ip_to_history`, and `invalidate` for session tracking
- These changes enable session binding enforcement and support mitigation of session hijacking/fixation risks

session_security_fix_verification.md
- Documentation for verifying session security fix
- Added test cases for session binding and fixation prevention
- Included instructions for running tests and verifying the fix

test-session-binding.sh
- Test script for session binding
- Validates that the session binding feature works as intended
- Ensures that tokens are invalidated when the session binding is broken
- Provides feedback on the success or failure of the test

test-session-fixation.sh
- Test script for session fixation
- Validates that the session fixation vulnerability is mitigated
- Ensures that tokens are not reused across different sessions
- Provides feedback on the success or failure of the test
db/migrate/20250419030255_add_session_binding_to_auth_tokens.rb
- Disgard changes in 20241025050957 add_scorm_feat
- Add new migration file for consolidated security and features
- Consolidate security and feature migrations into one
- Ensure that the new migration file is properly formatted and includes all necessary changes
@b0ink b0ink changed the base branch from 9.x to 10.0.x July 24, 2025 10:35
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