Fix/session binding validation

AuthToken.column_names

@@ -0,0 +1 @@
+

app/helpers/authentication_helpers.rb

@@ -1,4 +1,5 @@
 require 'onelogin/ruby-saml'
+require 'json'
 
 #
 # The AuthenticationHelpers include functions to check if the user
@@ -9,11 +10,44 @@
 module AuthenticationHelpers
   module_function
 
+  # Configuration getters for security settings
+  def security_config
+    Doubtfire::Application.config.session_security || {
+      binding_enabled: true,
+      ip_binding_strictness: :flexible,
+      max_allowed_ip_changes: 3,
+      suspicious_change_timeout: 5.minutes,
+      token_max_lifetime: 8.hours,
+      auth_enforcement_window: 15.seconds
+    }
+  end
+
+  #
+  # Helper method to handle ip_history JSON serialization (for MariaDB)
+  #
+  def ip_history_array(token)
+    return [] if token.ip_history.nil?
+    token.ip_history.present? ? JSON.parse(token.ip_history) : []
+  rescue JSON::ParserError
+    logger.error("Error parsing IP history for token #{token.id}")
+    []
+  end
+
+  #
+  # Helper method to update ip_history with JSON serialization
+  #
+  def update_ip_history(token, current_ip)
+    history = ip_history_array(token)
+    history << current_ip unless history.include?(current_ip)
+    token.update(ip_history: history.to_json)
+  end
+
   #
   # Checks if the requested user is authenticated.
   # Reads details from the params fetched from the caller context.
   #
   def authenticated?(token_type = :general)
+    Rails.logger.info "AUTH DEBUG: Method called for #{headers['Username'] || headers['username']} with token_type #{token_type}"
     auth_param = headers['auth-token'] || headers['Auth-Token'] || params['authToken'] || headers['Auth_Token'] || headers['auth_token'] || params['auth_token'] || params['Auth_Token']
     user_param = headers['username'] || headers['Username'] || params['username']
 
@@ -28,7 +62,55 @@ def authenticated?(token_type = :general)
 
     # Check user by token
     if user.present? && token.present?
+      # Verify the token hasn't been marked for invalidation (logout in progress)
+      if token.invalidation_requested_at.present?
+        elapsed_time = Time.zone.now - token.invalidation_requested_at
+        config = security_config
+        if elapsed_time > config[:auth_enforcement_window]
+          # The token was marked for invalidation more than AUTH_ENFORCEMENT_WINDOW ago
+          # This means a logout was triggered but the request might have been dropped
+          logger.warn("Blocked attempted use of token that was marked for invalidation #{elapsed_time.round(2)} seconds ago")
+          token.destroy!
+          error!({ error: 'Session has been terminated. Please log in again.' }, 401)
+        end
+      end
+
+      # Verify the token hasn't exceeded its maximum lifetime
+      config = security_config
+      if token.created_at.present? && token.created_at + config[:token_max_lifetime] < Time.zone.now
+        logger.info("Token exceeded maximum lifetime for #{user.username} from #{request.ip}")
+        token.destroy!
+        error!({ error: 'Session has exceeded maximum allowed duration. Please log in again.' }, 419)
+      end
+
       if token.auth_token_expiry > Time.zone.now
+        if token.auth_token_expiry < 5.minutes.from_now
+          # Refresh the token expiry time
+          token.update(auth_token_expiry: 1.hour.from_now)
+          logger.info("Token refreshed for #{user.username}")
+        end
+        logger.info "DEBUG: Entered token expiry check for #{user.username}"
+
+        current_ip = request.ip
+        current_ua = request.user_agent
+
+        logger.info "DEBUG: Current IP: #{current_ip}, Current UA: #{current_ua}"
+        logger.info "DEBUG: Token IP: #{token.session_ip}, Token UA: #{token.session_user_agent}"
+
+        # Handle session binding based on configured security level
+        config = security_config
+        if config[:binding_enabled]
+          session_binding_result = verify_session_binding(token, user, current_ip, current_ua)
+          return false unless session_binding_result
+        else
+          # If binding is disabled, just update the last seen values
+          token.update(
+            last_seen_ip: current_ip,
+            last_seen_ua: current_ua,
+            last_activity_at: Time.zone.now
+          )
+        end
+
         logger.info("Authenticated #{user.username} from #{request.ip}")
         return true
       end
@@ -48,6 +130,128 @@ def authenticated?(token_type = :general)
     end
   end
 
+  #
+  # Verifies session binding based on configured security levels
+  # Returns true if session is valid, false otherwise
+  #
+  def verify_session_binding(token, user, current_ip, current_ua)
+    config = security_config
+
+    # Initialize token binding data if not present
+    if token.session_ip.nil? && token.session_user_agent.nil?
+      # For new sessions, set the initial binding data
+      token.update(
+        session_ip: current_ip,
+        session_user_agent: current_ua,
+        last_seen_ip: current_ip,
+        last_seen_ua: current_ua,
+        ip_history: [current_ip].to_json,
+        last_activity_at: Time.zone.now,
+        suspicious_activity_detected_at: nil
+      )
+      logger.info("New session bound for #{user.username} from #{current_ip}")
+      return true
+    end
+
+    # Check if there are any suspicious changes
+    ip_changed = token.session_ip != current_ip
+    ua_changed = token.session_user_agent != current_ua
+
+    # Update most recent IP/UA and activity timestamp
+    token.update(
+      last_seen_ip: current_ip,
+      last_seen_ua: current_ua,
+      last_activity_at: Time.zone.now
+    )
+
+    # No changes detected, everything is normal
+    return true unless ip_changed || ua_changed
+
+    # If strict IP binding is enabled and IP changed, handle accordingly
+    if ip_changed && config[:ip_binding_strictness] == :strict
+      logger.warn("Session hijacking attempt detected for #{user.username} from #{current_ip} - strict mode")
+      token.destroy!
+      error!({ error: 'Security alert: Your session has been invalidated due to a location change. Please log in again.' }, 403)
+      return false
+    end
+
+    # If flexible binding is enabled, check if this is the first suspicious change
+    if config[:ip_binding_strictness] == :flexible
+      # Track IP history for analysis
+      ip_history = ip_history_array(token)
+
+      # Add IP to history if not already present
+      ip_history << current_ip unless ip_history.include?(current_ip)
+      token.update(ip_history: ip_history.to_json)
+
+      # If too many IPs are associated with this token, it's suspicious
+      if ip_history.length > config[:max_allowed_ip_changes]
+        logger.warn("Too many IP changes for #{user.username}, current IP: #{current_ip}")
+        token.destroy!
+        error!({ error: 'Security alert: Unusual account activity detected. Please log in again.' }, 403)
+        return false
+      end
+
+      # If this is the first suspicious change, mark it
+      if token.suspicious_activity_detected_at.nil?
+        token.update(suspicious_activity_detected_at: Time.zone.now)
+        logger.info("Suspicious change detected for #{user.username} from #{current_ip}, monitoring for #{config[:suspicious_change_timeout]}")
+        return true
+      end
+
+      # If suspicious change was detected recently, check timeout
+      if token.suspicious_activity_detected_at + config[:suspicious_change_timeout] < Time.zone.now
+        # Grace period expired, require re-authentication
+        logger.warn("Grace period expired for #{user.username} after suspicious changes")
+        token.destroy!
+        error!({ error: 'For your security, please log in again to verify your identity.' }, 403)
+        return false
+      end
+
+      # Within grace period, allow access but log it
+      logger.info("Allowing access during grace period for #{user.username} from #{current_ip}")
+      return true
+    end
+    # IP binding disabled or passing all other checks
+    true
+  end
+  #
+  # Securely invalidates a user session/token
+  # This method should be called at the beginning of the logout process
+  #
+
+  def invalidate_session(user, token_text = nil)
+    if user.nil?
+      logger.warn("Attempted to invalidate session for nil user")
+      return
+    end
+
+    # Find the specific token or all tokens for the user
+    tokens = if token_text.present?
+               [user.token_for_text?(token_text)]
+             else
+               user.auth_tokens
+             end
+    config = security_config
+    tokens.compact.each do |token|
+      # Mark token for invalidation first (will be enforced by authenticated? method)
+      token.update(invalidation_requested_at: Time.zone.now)
+
+      # Then destroy it after a short delay
+      # In production, this should be handled by a background job
+      Thread.new do
+        sleep(config[:auth_enforcement_window] * 1.5) # Wait slightly longer than the enforcement window
+        token.destroy! if token.persisted?
+      rescue StandardError => e
+        logger.error("Error in background token destruction: #{e.message}")
+      ensure
+        ActiveRecord::Base.connection_pool.release_connection
+      end
+    end
+
+    logger.info("Session invalidation initiated for #{user.username}")
+  end
+
   #
   # Get the current user either from warden or from the header
   #
@@ -131,4 +335,10 @@ def ldap_auth?
   def db_auth?
     Doubtfire::Application.config.auth_method == :database
   end
+  # Explicitly declare these functions as module functions
+  module_function :security_config
+  module_function :ip_history_array
+  module_function :update_ip_history
+  module_function :verify_session_binding
+  module_function :invalidate_session
 end

config/initializers/reload_authentication.rb

@@ -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

config/initializers/session_security.rb

@@ -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}"

db/migrate/20240920052508_convert_task_def_filenames.rb

@@ -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

db/migrate/20241025050957_add_scorm_feat.rb

@@ -1,24 +1,46 @@
 class AddScormFeat < ActiveRecord::Migration[7.1]
-  def change
-    # Record scorm extensions added to a task
-    add_column :tasks, :scorm_extensions, :integer, null: false, default: 0
+  def up
+    # Add scorm_extensions column if it doesn't exist
+    unless column_exists?(:tasks, :scorm_extensions)
+      add_column :tasks, :scorm_extensions, :integer, null: false, default: 0
+    else
+      Rails.logger.info "Column 'scorm_extensions' already exists in 'tasks' table. Skipping..."
+    end
 
+    # Add columns to task_definitions if they don't exist
     change_table :task_definitions do |t|
-      t.boolean :scorm_enabled, default: false
-      t.boolean :scorm_allow_review, default: false
-      t.boolean :scorm_bypass_test, default: false
-      t.boolean :scorm_time_delay_enabled, default: false
-      t.integer :scorm_attempt_limit, default: 0
+      t.boolean :scorm_enabled, default: false unless column_exists?(:task_definitions, :scorm_enabled)
+      t.boolean :scorm_allow_review, default: false unless column_exists?(:task_definitions, :scorm_allow_review)
+      t.boolean :scorm_bypass_test, default: false unless column_exists?(:task_definitions, :scorm_bypass_test)
+      t.boolean :scorm_time_delay_enabled, default: false unless column_exists?(:task_definitions, :scorm_time_delay_enabled)
+      t.integer :scorm_attempt_limit, default: 0 unless column_exists?(:task_definitions, :scorm_attempt_limit)
     end
 
     # Enable polymorphic relationships for task comments
-    remove_index :task_comments, :overseer_assessment_id
+    remove_index :task_comments, :overseer_assessment_id if index_exists?(:task_comments, :overseer_assessment_id)
+
+    add_column :task_comments, :commentable_type, :string unless column_exists?(:task_comments, :commentable_type)
+    rename_column :task_comments, :overseer_assessment_id, :commentable_id if column_exists?(:task_comments, :overseer_assessment_id)
+
+    TaskComment.where.not(commentable_id: nil).in_batches.update_all(commentable_type: 'OverseerAssessment')
+
+    add_index :task_comments, [:commentable_type, :commentable_id] unless index_exists?(:task_comments, [:commentable_type, :commentable_id])
+  end
+
+  def down
+    # Remove scorm_extensions column if it exists
+    remove_column :tasks, :scorm_extensions if column_exists?(:tasks, :scorm_extensions)
 
-    add_column :task_comments, :commentable_type, :string
-    rename_column :task_comments, :overseer_assessment_id, :commentable_id
+    # Remove columns from task_definitions if they exist
+    change_table :task_definitions do |t|
+      t.remove :scorm_enabled, :scorm_allow_review, :scorm_bypass_test, :scorm_time_delay_enabled, :scorm_attempt_limit if column_exists?(:task_definitions, :scorm_enabled)
+    end
 
-    TaskComment.where('NOT commentable_id IS NULL').in_batches.update_all(commentable_type: 'OverseerAssessment')
+    # Revert polymorphic relationships for task comments
+    remove_index :task_comments, [:commentable_type, :commentable_id] if index_exists?(:task_comments, [:commentable_type, :commentable_id])
+    rename_column :task_comments, :commentable_id, :overseer_assessment_id if column_exists?(:task_comments, :commentable_id)
+    remove_column :task_comments, :commentable_type if column_exists?(:task_comments, :commentable_type)
 
-    add_index :task_comments, [:commentable_type, :commentable_id]
+    add_index :task_comments, :overseer_assessment_id unless index_exists?(:task_comments, :overseer_assessment_id)
   end
 end

db/migrate/20250419030255_add_session_binding_to_auth_tokens.rb

@@ -0,0 +1,6 @@
+class AddSessionBindingToAuthTokens < ActiveRecord::Migration[7.1]
+  def change
+    add_column :auth_tokens, :session_ip, :string
+    add_column :auth_tokens, :session_user_agent, :string
+  end
+end

db/migrate/20250428135250_add_session_binding_columns_to_auth_tokens.rb

@@ -0,0 +1,26 @@
+class AddSessionBindingColumnsToAuthTokens < ActiveRecord::Migration[7.1]
+  def change
+    # Columns for session binding improvements
+    # Only add columns if they don't already exist
+    add_column :auth_tokens, :last_seen_ip, :string unless column_exists?(:auth_tokens, :last_seen_ip)
+    add_column :auth_tokens, :last_seen_ua, :string unless column_exists?(:auth_tokens, :last_seen_ua)
+
+    # For arrays, use JSON in MySQL/MariaDB since they don't support native arrays
+    # Detect database type and use appropriate column type
+    if ActiveRecord::Base.connection.adapter_name.downcase.include?('mysql')
+      add_column :auth_tokens, :ip_history, :text unless column_exists?(:auth_tokens, :ip_history)
+    else
+      # PostgreSQL supports arrays
+      add_column :auth_tokens, :ip_history, :string, array: true, default: [] unless column_exists?(:auth_tokens, :ip_history)
+    end
+
+    # Columns for session fixation/hijacking prevention
+    add_column :auth_tokens, :suspicious_activity_detected_at, :datetime unless column_exists?(:auth_tokens, :suspicious_activity_detected_at)
+    add_column :auth_tokens, :invalidation_requested_at, :datetime unless column_exists?(:auth_tokens, :invalidation_requested_at)
+    add_column :auth_tokens, :last_activity_at, :datetime unless column_exists?(:auth_tokens, :last_activity_at)
+
+    # Add index to improve query performance for token validation
+    # Add index if it doesn't exist
+    add_index :auth_tokens, :invalidation_requested_at unless index_exists?(:auth_tokens, :invalidation_requested_at)
+  end
+end

db/migrate/20250429074259_add_timestamps_to_auth_tokens.rb

@@ -0,0 +1,5 @@
+class AddTimestampsToAuthTokens < ActiveRecord::Migration[7.1]
+  def change
+    add_timestamps :auth_tokens, default: -> { 'CURRENT_TIMESTAMP' }, null: false
+  end
+end