Implement DB backed sessions for candidates

In testing the one login integration, we discovered that having session backed
login is not ideal. There is a risk that the session gets too big, over 5000
bytes. To fix this, we are switching to Database backed sessions.

This is inspired by the rails 8 new db backed sessions. Most of the logic is in
this concern `Authentication` using this and before filters in controllers we
control the one login mechanism.

We will use DB backed session for the normal candidate flow. Impersonation will
still use cookie session.

This means that deleting the session from the DB logs out the user.

Switching between one login and magic link auth system will be seamless as we
always check `current_candidate`.

We will still save some data in the session but it will be very little,
just ids rather than big tokens.

The session size decreased from roughly 3224 to 600

app/controllers/candidate_interface/candidate_interface_controller.rb

@@ -1,9 +1,10 @@
 module CandidateInterface
   class CandidateInterfaceController < ApplicationController
     include BackLinks
+    include Authentication
 
     before_action :protect_with_basic_auth
-    before_action :authenticate_candidate!
+    before_action :authenticate_candidate!, unless: -> { one_login_enabled? }
     before_action :set_user_context
     before_action :check_cookie_preferences
     before_action :check_account_locked
@@ -12,6 +13,10 @@ class CandidateInterfaceController < ApplicationController
     alias audit_user current_candidate
     alias current_user current_candidate
 
+    def current_candidate
+      super || Current.session&.candidate
+    end
+
     def set_user_context(candidate_id = current_candidate&.id)
       Sentry.set_user(id: "candidate_#{candidate_id}")
 

app/controllers/candidate_interface/start_page_controller.rb

@@ -3,6 +3,7 @@ class StartPageController < CandidateInterfaceController
     before_action :redirect_to_application_if_signed_in
     before_action :redirect_to_post_offer_dashboard_if_accepted_deferred_or_recruited, if: :candidate_signed_in?
     skip_before_action :authenticate_candidate!
+    allow_unauthenticated_access only: %i[create_account_or_sign_in create_account_or_sign_in_handler]
 
     def create_account_or_sign_in
       @create_account_or_sign_in_form = CreateAccountOrSignInForm.new

app/controllers/concerns/authentication.rb

@@ -0,0 +1,63 @@
+module Authentication
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :require_authentication, if: -> { one_login_enabled? }
+    helper_method :authenticated?
+  end
+
+  class_methods do
+    def allow_unauthenticated_access(**options)
+      skip_before_action :require_authentication, **options
+    end
+  end
+
+private
+
+  def authenticated?
+    resume_session
+  end
+
+  def require_authentication
+    current_candidate || resume_session || request_authentication
+  end
+
+  def resume_session
+    Current.session ||= find_session_by_cookie
+  end
+
+  def find_session_by_cookie
+    Session.find_by(id: cookies.signed[:session_id]) if cookies.signed[:session_id]
+  end
+
+  def request_authentication
+    redirect_to candidate_interface_create_account_or_sign_in_path
+  end
+
+  def start_new_session_for(candidate:, id_token_hint: nil)
+    ActiveRecord::Base.transaction do
+      unless authenticated?
+        candidate.sessions.create!(
+          user_agent: request.user_agent,
+          ip_address: request.remote_ip,
+          id_token_hint:,
+        ).tap do |session|
+          Current.session = session
+          cookies.signed.permanent[:session_id] = { value: session.id, httponly: true, same_site: :lax }
+        end
+
+        candidate.update!(last_signed_in_at: Time.zone.now)
+      end
+    end
+  end
+
+  def terminate_session
+    Current.session&.destroy
+    cookies.delete(:session_id)
+    reset_session
+  end
+
+  def one_login_enabled?
+    FeatureFlag.active?(:one_login_candidate_sign_in)
+  end
+end

app/controllers/one_login_controller.rb

@@ -1,17 +1,39 @@
 class OneLoginController < ApplicationController
+  include Authentication
+
   before_action :redirect_to_candidate_sign_in_unless_one_login_enabled
+  allow_unauthenticated_access
 
   def callback
     auth = request.env['omniauth.auth']
-    session[:one_login_id_token] = auth&.credentials&.id_token
+    id_token_hint = auth&.credentials&.id_token
     candidate = OneLoginUser.authenticate_or_create_by(auth)
 
-    sign_in_candidate(candidate)
+    start_new_session_for(
+      candidate:,
+      id_token_hint:,
+    )
 
     redirect_to candidate_interface_interstitial_path
-  rescue OneLoginUser::Error => e
-    session[:one_login_error] = e.message
-    redirect_to auth_one_login_sign_out_path
+  rescue StandardError => e
+    session_error = SessionError.create!(
+      candidate: OneLoginUser.find_candidate(auth),
+      id_token_hint:,
+      body: e.message,
+      omniauth_hash: auth&.to_h,
+    )
+
+    if e.is_a?(OneLoginUser::Error)
+      session[:session_error_id] = session_error.id
+      Sentry.capture_message(
+        "One login session error, check session_error record #{session_error.id}",
+        level: :error,
+      )
+
+      redirect_to auth_one_login_sign_out_path
+    else
+      redirect_to internal_server_error_path
+    end
   end
 
   def bypass_callback
@@ -21,7 +43,7 @@ def bypass_callback
     candidate = one_login_user_bypass.authenticate
 
     if candidate.present?
-      sign_in_candidate(candidate)
+      start_new_session_for(candidate:)
 
       redirect_to candidate_interface_interstitial_path
     else
@@ -31,32 +53,43 @@ def bypass_callback
   end
 
   def sign_out
-    id_token = session[:one_login_id_token]
-    one_login_error = session[:one_login_error]
-    reset_session
+    session_error = SessionError.find_by(id: session[:session_error_id])
+    id_token_hint = if authenticated?
+                      Current.session&.id_token_hint
+                    else
+                      session_error&.id_token_hint
+                    end
+
+    terminate_session
 
-    session[:one_login_error] = one_login_error
-    if OneLogin.bypass? || id_token.nil?
+    session[:session_error_id] = session_error.id if session_error.present?
+    if OneLogin.bypass? || id_token_hint.nil?
       redirect_to candidate_interface_create_account_or_sign_in_path
     else
       # Go back to one login to sign out the user on their end as well
-      redirect_to logout_one_login(id_token), allow_other_host: true
+      redirect_to logout_one_login(id_token_hint), allow_other_host: true
     end
   end
 
   def sign_out_complete
-    if session[:one_login_error].present?
-      Sentry.capture_message(session[:one_login_error], level: :error)
+    if session[:session_error_id].present?
+      reset_session
       redirect_to internal_server_error_path
     else
       redirect_to candidate_interface_create_account_or_sign_in_path
     end
   end
 
   def failure
-    session[:one_login_error] = "One login failure with #{params[:message]} " \
-                                "for one_login_id_token: #{session[:one_login_id_token]}"
+    session_error = SessionError.create!(
+      body: "One login failure with #{params[:message]}",
+    )
+    Sentry.capture_message(
+      "#{session_error.body}, check session_error record #{session_error.id}",
+      level: :error,
+    )
 
+    session[:session_error_id] = session_error.id if session_error.present?
     redirect_to auth_one_login_sign_out_path
   end
 
@@ -68,11 +101,6 @@ def redirect_to_candidate_sign_in_unless_one_login_enabled
     end
   end
 
-  def sign_in_candidate(candidate)
-    sign_in(candidate, scope: :candidate)
-    candidate.update!(last_signed_in_at: Time.zone.now)
-  end
-
   def logout_one_login(id_token_hint)
     params = {
       post_logout_redirect_uri: URI(auth_one_login_sign_out_complete_url),

app/models/current.rb

@@ -0,0 +1,4 @@
+class Current < ActiveSupport::CurrentAttributes
+  attribute :session
+  delegate :candidate, to: :session, allow_nil: true
+end

app/models/one_login_user.rb

@@ -21,6 +21,14 @@ def authenticate_or_create_by
     create_candidate!
   end
 
+  def self.find_candidate(omniauth_auth)
+    new(omniauth_auth).find_candidate
+  end
+
+  def find_candidate
+    OneLoginAuth.find_by(token:)&.candidate || Candidate.find_by(email_address:)
+  end
+
 private
 
   def candidate_with_one_login(one_login_auth)

config/brakeman.ignore

@@ -115,6 +115,29 @@
       ],
       "note": ""
     },
+    {
+      "warning_type": "Redirect",
+      "warning_code": 18,
+      "fingerprint": "783f66b9e6119c7b65c794a57f29b291e256d49267c511bff375d031ef83ea96",
+      "check_name": "Redirect",
+      "message": "Possible unprotected redirect",
+      "file": "app/controllers/one_login_controller.rb",
+      "line": 71,
+      "link": "https://brakemanscanner.org/docs/warning_types/redirect/",
+      "code": "redirect_to(logout_one_login((Current.session.id_token_hint or SessionError.find_by(:id => session[:session_error_id]).id_token_hint)), :allow_other_host => true)",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "OneLoginController",
+        "method": "sign_out"
+      },
+      "user_input": "Current.session.id_token_hint",
+      "confidence": "Weak",
+      "cwe_id": [
+        601
+      ],
+      "note": ""
+    },
     {
       "warning_type": "SQL Injection",
       "warning_code": 0,
@@ -276,6 +299,29 @@
       ],
       "note": ""
     },
+    {
+      "warning_type": "Unscoped Find",
+      "warning_code": 82,
+      "fingerprint": "bb99c47602bf4e5e8c658f10645d3d0af3f1ac7e78e590db5e66d8986ba1642a",
+      "check_name": "UnscopedFind",
+      "message": "Unscoped call to `Session#find_by`",
+      "file": "app/controllers/concerns/authentication.rb",
+      "line": 32,
+      "link": "https://brakemanscanner.org/docs/warning_types/unscoped_find/",
+      "code": "Session.find_by(:id => cookies.signed[:session_id])",
+      "render_path": null,
+      "location": {
+        "type": "method",
+        "class": "Authentication",
+        "method": "find_session_by_cookie"
+      },
+      "user_input": "cookies.signed[:session_id]",
+      "confidence": "Weak",
+      "cwe_id": [
+        285
+      ],
+      "note": ""
+    },
     {
       "warning_type": "SQL Injection",
       "warning_code": 0,
@@ -346,6 +392,6 @@
       "note": ""
     }
   ],
-  "updated": "2024-08-27 15:35:00 +0100",
-  "brakeman_version": "6.1.2"
+  "updated": "2025-01-02 12:10:42 +0000",
+  "brakeman_version": "6.2.2"
 }