-
Notifications
You must be signed in to change notification settings - Fork 129
Fix/session binding validation #61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: app-attack-fixes
Are you sure you want to change the base?
Changes from 5 commits
4ca2475
83662ee
c3c91d1
845d331
eb3def3
c1577c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| # AuthToken Column Documentation | ||
| # This file documents the new columns added to auth_tokens table for security enhancements | ||
|
|
||
| # Original columns: | ||
| # id: primary key | ||
| # auth_token_expiry: datetime when token expires | ||
| # user_id: user who owns this token | ||
| # authentication_token: the actual token string | ||
| # token_type: type of token (general, api, etc.) | ||
| # session_ip: original IP address that created this token | ||
| # session_user_agent: original User-Agent that created this token | ||
|
|
||
| # New columns for session binding: | ||
| # last_seen_ip: most recent IP address that used this token | ||
| # last_seen_ua: most recent User-Agent that used this token | ||
| # ip_history: JSON array of all IPs that have used this token | ||
| # suspicious_activity_detected_at: timestamp when first suspicious change was detected | ||
|
|
||
| # New columns for session fixation/hijacking prevention: | ||
| # invalidation_requested_at: timestamp when logout was requested | ||
| # last_activity_at: timestamp of most recent activity with this token | ||
|
|
||
| # Security implementation notes: | ||
| # - When validating tokens, check invalidation_requested_at to prevent session fixation | ||
| # - Use ip_history to track and limit the number of different IPs that can use a token | ||
| # - suspicious_activity_detected_at starts a grace period for user to reauthenticate | ||
| # - The combined approach provides flexible session binding while preventing session stealing across users |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Rails.application.reloader.to_prepare do | ||
| load Rails.root.join("app/helpers/authentication_helpers.rb") | ||
| Rails.logger.info "Reloaded AuthenticationHelpers at #{Time.zone.now}" | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| # Be sure to restart your server when you modify this file. | ||
|
|
||
| # Configuration for authentication session security | ||
| Doubtfire::Application.config.session_security = { | ||
| binding_enabled: true, # Enable/disable session binding completely | ||
| ip_binding_strictness: :flexible, # :strict, :flexible, or :disabled | ||
| max_allowed_ip_changes: 3, # Maximum number of different IPs allowed per token | ||
| suspicious_change_timeout: 5.minutes, # Period to allow suspicious changes before requiring re-auth | ||
| token_max_lifetime: 8.hours, # Maximum lifetime of a token, regardless of activity | ||
| auth_enforcement_window: 15.seconds # Time window to check for forced session persistence | ||
| } | ||
| Rails.logger.info "Loading session security config at #{Time.zone.now}" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,16 @@ | ||
| class ConvertTaskDefFilenames < ActiveRecord::Migration[7.1] | ||
|
|
||
| # Check filenames in the upload requirements for each task definition | ||
| # and replace any invalid characters using sanitize filename | ||
| def change | ||
|
||
| unless column_exists?(:task_definitions, :new_column_name) | ||
| add_column :task_definitions, :new_column_name, :string | ||
| end | ||
|
|
||
| TaskDefinition.find_in_batches do |group| | ||
| group.each do |task_def| | ||
| next if task_def.valid? | ||
|
|
||
| upload_req = task_def.upload_requirements | ||
|
|
||
| change = false | ||
|
|
||
| upload_req.each do |req| | ||
| unless req['name'].match?(/^[a-zA-Z0-9_\- \.]+$/) | ||
| req['name'] = FileHelper.sanitized_filename(req['name']) | ||
|
|
@@ -30,3 +31,10 @@ def change | |
| end | ||
| end | ||
| end | ||
|
|
||
| class AddAuthTokenType < ActiveRecord::Migration[7.1] | ||
| def change | ||
| add_column :auth_tokens, :token_type, :integer, null: false, default: 0 | ||
| add_index :auth_tokens, :token_type | ||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed user_agent does not include PII or raise any data protection concerns - that is great