Fix broken invitation email text template

The text part used non-existent helpers (`invite_url`,
`@user.invitation_login_token`) and Ruby string interpolation in an ERB
file, so multipart delivery failed at render time and no invite mail
went out. Mirror the HTML template instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

app/assets/tailwind/application.css

@@ -3,6 +3,12 @@
 @custom-variant dark (&:where(.dark, .dark *));
 
 @layer base {
+  input:where([type="text"], [type="email"], [type="password"], [type="number"], [type="url"], [type="tel"], [type="search"]),
+  textarea,
+  select {
+    padding: 0.5rem 0.75rem;
+  }
+
   .dark input:where([type="text"], [type="email"], [type="password"], [type="number"], [type="url"], [type="tel"], [type="search"]),
   .dark textarea,
   .dark select {

app/controllers/admin/users_controller.rb

@@ -122,15 +122,14 @@ module Admin
     end
 
     def user_params
-      # Base attributes that all admins can modify
-      base_params = params.require(:user).permit(:email_address, :username, :name, :password, :status, :totp_required, :custom_claims)
+      permitted = [:email_address, :username, :name, :password, :status, :totp_required, :custom_claims]
 
       # Only allow modifying admin status when editing other users (prevent self-demotion)
       if params[:id] != Current.session.user.id.to_s
-        base_params[:admin] = params[:user][:admin] if params[:user][:admin].present?
+        permitted << :admin
       end
 
-      base_params
+      params.require(:user).permit(*permitted)
     end
   end
 end

app/controllers/api/forward_auth_controller.rb

@@ -163,16 +163,25 @@ module Api
 
     def check_forward_auth_token
       token = params[:fa_token]
-      return nil unless token.present?
+      return nil if token.blank?
 
-      session_id = Rails.cache.read("forward_auth_token:#{token}")
-      return nil unless session_id
+      cached = Rails.cache.read("forward_auth_token:#{token}")
+      return nil unless cached.is_a?(Hash)
 
-      session = Session.find_by(id: session_id)
+      # The token is bound to the host that created it. If the request is
+      # arriving at a different host, refuse — and do NOT burn the cache
+      # entry, so that the legitimate destination can still redeem within
+      # the 60s TTL.
+      request_host = (request.headers["X-Forwarded-Host"] || request.headers["Host"])
+        .to_s.sub(/:\d+\z/, "").downcase
+      return nil if request_host.blank?
+      return nil unless cached[:host] == request_host
+
+      session = Session.find_by(id: cached[:session_id])
       return nil unless session && !session.expired?
 
       Rails.cache.delete("forward_auth_token:#{token}")
-      session_id
+      cached[:session_id]
     end
 
     def extract_session_id

app/controllers/concerns/authentication.rb

@@ -31,7 +31,7 @@ module Authentication
   end
 
   def find_session_by_cookie
-    Session.find_by(id: cookies.signed[:session_id]) if cookies.signed[:session_id]
+    Session.active.find_by(id: cookies.signed[:session_id]) if cookies.signed[:session_id]
   end
 
   def request_authentication
@@ -43,9 +43,9 @@ module Authentication
     session.delete(:return_to_after_authenticating) || root_url
   end
 
-  def start_new_session_for(user, acr: "1")
+  def start_new_session_for(user, acr: "1", remember_me: false)
     user.update!(last_sign_in_at: Time.current)
-    user.sessions.create!(user_agent: request.user_agent, ip_address: request.remote_ip, acr: acr).tap do |session|
+    user.sessions.create!(user_agent: request.user_agent, ip_address: request.remote_ip, acr: acr, remember_me: remember_me).tap do |session|
       Current.session = session
 
       # Extract root domain for cross-subdomain cookies (required for forward auth)
@@ -58,8 +58,8 @@ module Authentication
         {
           value: session.id,
           httponly: true,
-          same_site: :none,  # Allow cross-site cookies for OIDC testing
-          secure: true        # Required for SameSite=None
+          same_site: :lax,
+          secure: true
         }
       else
         {
@@ -130,35 +130,35 @@ module Authentication
   end
 
   # Create a one-time token for forward auth to handle the race condition
-  # where the browser hasn't processed the session cookie yet
+  # where the browser hasn't processed the session cookie yet.
+  #
+  # The token is bound to the destination host so that anyone who observes
+  # the token (Referer leaks, access logs, JS monitors) cannot redeem it for
+  # a different application within the 60-second TTL.
   def create_forward_auth_token(session_obj)
-    # Generate a secure random token
-    token = SecureRandom.urlsafe_base64(32)
+    controller_session = session
+    return unless controller_session[:return_to_after_authenticating].present?
 
-    # Store it with an expiry of 60 seconds
+    uri = URI.parse(controller_session[:return_to_after_authenticating])
+
+    # OAuth flow handles its own session propagation — no fa_token needed.
+    return if uri.path&.start_with?("/oauth/")
+
+    # Path-only URLs are same-origin on Clinch; the cookie race doesn't apply
+    # and we have no destination host to bind against.
+    bound_host = uri.hostname&.downcase
+    return if bound_host.blank?
+
+    token = SecureRandom.urlsafe_base64(32)
     Rails.cache.write(
       "forward_auth_token:#{token}",
-      session_obj.id,
+      { session_id: session_obj.id, host: bound_host },
       expires_in: 60.seconds
     )
 
-    # Set the token as a query parameter on the redirect URL
-    # We need to store this in the controller's session
-    controller_session = session
-    if controller_session[:return_to_after_authenticating].present?
-      original_url = controller_session[:return_to_after_authenticating]
-      uri = URI.parse(original_url)
-
-      # Skip adding fa_token for OAuth URLs (OAuth flow should not have forward auth tokens)
-      unless uri.path&.start_with?("/oauth/")
-        # Add token as query parameter
     query_params = URI.decode_www_form(uri.query || "").to_h
     query_params["fa_token"] = token
     uri.query = URI.encode_www_form(query_params)
-
-        # Update the session with the tokenized URL
     controller_session[:return_to_after_authenticating] = uri.to_s
   end
 end
-  end
-end

app/controllers/oidc_controller.rb

@@ -1,9 +1,16 @@
 class OidcController < ApplicationController
+  SUPPORTED_SCOPES = %w[openid profile email groups offline_access].freeze
+
   # Discovery and JWKS endpoints are public
   # authorize is also unauthenticated to handle prompt=none and prompt=login specially
   allow_unauthenticated_access only: [:discovery, :jwks, :token, :revoke, :userinfo, :logout, :authorize]
   skip_before_action :verify_authenticity_token, only: [:token, :revoke, :userinfo, :logout, :authorize, :consent]
 
+  # RFC 6749 §4.1.2.1: client_id and redirect_uri must be validated *before* any
+  # other error can be reported via redirect. Failures here render a plain page.
+  before_action :set_application, only: :authorize
+  before_action :validate_redirect_uri, only: :authorize
+
   # Rate limiting to prevent brute force and abuse
   rate_limit to: 60, within: 1.minute, only: [:token, :revoke], with: -> {
     render json: {error: "too_many_requests", error_description: "Rate limit exceeded. Try again later."}, status: :too_many_requests
@@ -29,7 +36,7 @@ class OidcController < ApplicationController
       grant_types_supported: ["authorization_code", "refresh_token"],
       subject_types_supported: ["pairwise"],
       id_token_signing_alg_values_supported: ["RS256"],
-      scopes_supported: ["openid", "profile", "email", "groups", "offline_access"],
+      scopes_supported: SUPPORTED_SCOPES,
       token_endpoint_auth_methods_supported: ["client_secret_post", "client_secret_basic"],
       claims_supported: [
         "sub",              # Always included
@@ -42,7 +49,7 @@ class OidcController < ApplicationController
         # Note: Custom claims are also supported but not listed here
         # ID-token-only claims (auth_time, acr, azp, at_hash, nonce) are not listed
       ],
-      code_challenge_methods_supported: ["plain", "S256"],
+      code_challenge_methods_supported: ["S256"],
       backchannel_logout_supported: true,
       backchannel_logout_session_supported: true,
       request_parameter_supported: false,
@@ -59,7 +66,8 @@ class OidcController < ApplicationController
 
   # GET /oauth/authorize
   def authorize
-    # Get parameters (ignore forward auth tokens and other unknown params)
+    # @application and a validated redirect_uri are guaranteed by the before_actions.
+    # Read the remaining parameters (ignore forward auth tokens and other unknown params).
     client_id = params[:client_id]
     redirect_uri = params[:redirect_uri]
     state = params[:state]
@@ -67,57 +75,10 @@ class OidcController < ApplicationController
     scope = params[:scope] || "openid"
     response_type = params[:response_type]
     code_challenge = params[:code_challenge]
-    code_challenge_method = params[:code_challenge_method] || "plain"
-
-    # Validate client_id first (required before we can look up the application)
-    # OAuth2 RFC 6749 Section 4.1.2.1: If client_id is missing/invalid, show error page (don't redirect)
-    unless client_id.present?
-      render plain: "Invalid request: client_id is required", status: :bad_request
-      return
-    end
-
-    # Find the application by client_id
-    @application = Application.find_by(client_id: client_id, app_type: "oidc")
-    unless @application
-      # Log all OIDC applications for debugging
-      all_oidc_apps = Application.where(app_type: "oidc")
-      Rails.logger.error "OAuth: Invalid request - application not found for client_id: #{client_id}"
-      Rails.logger.error "OAuth: Available OIDC applications: #{all_oidc_apps.pluck(:id, :client_id, :name)}"
-
-      error_msg = if Rails.env.development?
-        "Invalid request: Application not found for client_id '#{client_id}'. Available OIDC applications: #{all_oidc_apps.pluck(:name, :client_id).map { |name, id| "#{name} (#{id})" }.join(", ")}"
-      else
-        "Invalid request: Application not found"
-      end
-
-      render plain: error_msg, status: :bad_request
-      return
-    end
-
-    # Validate redirect_uri presence and format
-    # OAuth2 RFC 6749 Section 4.1.2.1: If redirect_uri is missing/invalid, show error page (don't redirect)
-    unless redirect_uri.present?
-      render plain: "Invalid request: redirect_uri is required", status: :bad_request
-      return
-    end
-
-    # Validate redirect URI matches one of the registered URIs
-    unless @application.parsed_redirect_uris.include?(redirect_uri)
-      Rails.logger.error "OAuth: Invalid request - redirect URI mismatch. Expected: #{@application.parsed_redirect_uris}, Got: #{redirect_uri}"
-
-      # For development, show detailed error
-      error_msg = if Rails.env.development?
-        "Invalid request: Redirect URI mismatch. Application is configured for: #{@application.parsed_redirect_uris.join(", ")}, but received: #{redirect_uri}"
-      else
-        "Invalid request: Redirect URI not registered for this application"
-      end
-
-      render plain: error_msg, status: :bad_request
-      return
-    end
+    code_challenge_method = params[:code_challenge_method] || "S256"
 
     # ============================================================================
-    # At this point we have a valid client_id and redirect_uri
+    # client_id and redirect_uri are already validated (see before_actions).
     # All subsequent errors should redirect back to the client with error parameters
     # per OAuth2 RFC 6749 Section 4.1.2.1
     # ============================================================================
@@ -146,10 +107,10 @@ class OidcController < ApplicationController
 
     # Validate PKCE parameters if present (now we can safely redirect with error)
     if code_challenge.present?
-      unless %w[plain S256].include?(code_challenge_method)
+      unless code_challenge_method == "S256"
         Rails.logger.error "OAuth: Invalid code_challenge_method: #{code_challenge_method}"
         error_uri = "#{redirect_uri}?error=invalid_request"
-        error_uri += "&error_description=#{CGI.escape("Invalid code_challenge_method: must be 'plain' or 'S256'")}"
+        error_uri += "&error_description=#{CGI.escape("Invalid code_challenge_method: only 'S256' is supported")}"
         error_uri += "&state=#{CGI.escape(state)}" if state.present?
         redirect_to error_uri, allow_other_host: true
         return
@@ -166,6 +127,12 @@ class OidcController < ApplicationController
       end
     end
 
+    # Normalize requested scopes to the set we support. Needed here so claims
+    # validation below can check claim→scope coverage against what will actually
+    # be granted.
+    requested_scopes = scope.split(" ") & SUPPORTED_SCOPES
+    scope = requested_scopes.join(" ")
+
     # Parse claims parameter (JSON string) for OIDC claims request
     # Per OIDC Core §5.5: The claims parameter is a JSON object that requests
     # specific claims to be returned in the id_token and/or userinfo
@@ -289,7 +256,12 @@ class OidcController < ApplicationController
       return
     end
 
-    requested_scopes = scope.split(" ")
+    unless requested_scopes.include?("openid")
+      error_uri = "#{redirect_uri}?error=invalid_scope&error_description=#{CGI.escape("The 'openid' scope is required")}"
+      error_uri += "&state=#{CGI.escape(state)}" if state.present?
+      redirect_to error_uri, allow_other_host: true
+      return
+    end
 
     # Check if application is configured to skip consent
     # If so, automatically create consent and proceed without showing consent screen
@@ -420,8 +392,7 @@ class OidcController < ApplicationController
 
     user = Current.session.user
 
-    # Record user consent
-    requested_scopes = oauth_params["scope"].split(" ")
+    requested_scopes = oauth_params["scope"].split(" ") & SUPPORTED_SCOPES
     parsed_claims = begin
       JSON.parse(oauth_params["claims_requests"])
     rescue
@@ -539,15 +510,12 @@ class OidcController < ApplicationController
 
         # Check if code has already been used (CRITICAL: check AFTER locking)
         if auth_code.used?
-          # Per OAuth 2.0 spec, if an auth code is reused, revoke all tokens issued from it
+          # Per OAuth 2.0 spec, if an auth code is reused, revoke every token
+          # descended from it (both generations across any rotations).
           Rails.logger.warn "OAuth Security: Authorization code reuse detected for code #{auth_code.id}"
-
-          # Revoke all access tokens issued from this authorization code
-          OidcAccessToken.where(
-            application: application,
-            user: auth_code.user,
-            created_at: auth_code.created_at..Time.current
-          ).update_all(expires_at: Time.current)
+          now = Time.current
+          auth_code.oidc_access_tokens.where(revoked_at: nil).update_all(revoked_at: now)
+          auth_code.oidc_refresh_tokens.where(revoked_at: nil).update_all(revoked_at: now)
 
           render json: {
             error: "invalid_grant",
@@ -588,7 +556,8 @@ class OidcController < ApplicationController
         access_token_record = OidcAccessToken.create!(
           application: application,
           user: user,
-          scope: auth_code.scope
+          scope: auth_code.scope,
+          oidc_authorization_code: auth_code
         )
 
         # Generate refresh token (opaque, with hashing)
@@ -596,6 +565,7 @@ class OidcController < ApplicationController
           application: application,
           user: user,
           oidc_access_token: access_token_record,
+          oidc_authorization_code: auth_code,
           scope: auth_code.scope,
           auth_time: auth_code.auth_time,
           acr: auth_code.acr
@@ -720,10 +690,15 @@ class OidcController < ApplicationController
     refresh_token_record.revoke!
 
     # Generate new access token record (opaque token with BCrypt hashing)
+    # Carry the authorization-code FK forward across rotations so replay
+    # revocation reaches every descendant token in the chain.
+    issuing_auth_code = refresh_token_record.oidc_authorization_code
+
     new_access_token = OidcAccessToken.create!(
       application: application,
       user: user,
-      scope: refresh_token_record.scope
+      scope: refresh_token_record.scope,
+      oidc_authorization_code: issuing_auth_code
     )
 
     # Generate new refresh token (token rotation)
@@ -731,6 +706,7 @@ class OidcController < ApplicationController
       application: application,
       user: user,
       oidc_access_token: new_access_token,
+      oidc_authorization_code: issuing_auth_code,
       scope: refresh_token_record.scope,
       token_family_id: refresh_token_record.token_family_id,  # Keep same family for rotation tracking
       auth_time: refresh_token_record.auth_time,  # Carry over original auth_time
@@ -1000,6 +976,55 @@ class OidcController < ApplicationController
 
   private
 
+  # Look up @application from client_id. RFC 6749 §4.1.2.1 requires that an
+  # invalid client_id be reported on-page, not via redirect.
+  def set_application
+    client_id = params[:client_id]
+
+    unless client_id.present?
+      render plain: "Invalid request: client_id is required", status: :bad_request
+      return
+    end
+
+    @application = Application.find_by(client_id: client_id, app_type: "oidc")
+    return if @application
+
+    Rails.logger.error "OAuth: Invalid request - application not found for client_id: #{client_id}"
+
+    error_msg = if Rails.env.development?
+      all_oidc_apps = Application.where(app_type: "oidc")
+      Rails.logger.error "OAuth: Available OIDC applications: #{all_oidc_apps.pluck(:id, :client_id, :name)}"
+      "Invalid request: Application not found for client_id '#{client_id}'. Available OIDC applications: #{all_oidc_apps.pluck(:name, :client_id).map { |name, id| "#{name} (#{id})" }.join(", ")}"
+    else
+      "Invalid request: Application not found"
+    end
+
+    render plain: error_msg, status: :bad_request
+  end
+
+  # Confirm the redirect_uri param is present and registered on @application.
+  # Must run after set_application. Errors render on-page per RFC 6749 §4.1.2.1.
+  def validate_redirect_uri
+    redirect_uri = params[:redirect_uri]
+
+    unless redirect_uri.present?
+      render plain: "Invalid request: redirect_uri is required", status: :bad_request
+      return
+    end
+
+    return if @application.parsed_redirect_uris.include?(redirect_uri)
+
+    Rails.logger.error "OAuth: Invalid request - redirect URI mismatch. Expected: #{@application.parsed_redirect_uris}, Got: #{redirect_uri}"
+
+    error_msg = if Rails.env.development?
+      "Invalid request: Redirect URI mismatch. Application is configured for: #{@application.parsed_redirect_uris.join(", ")}, but received: #{redirect_uri}"
+    else
+      "Invalid request: Redirect URI not registered for this application"
+    end
+
+    render plain: error_msg, status: :bad_request
+  end
+
   def validate_pkce(application, auth_code, code_verifier)
     # Check if PKCE is required for this application
     pkce_required = application.requires_pkce?
@@ -1041,16 +1066,14 @@ class OidcController < ApplicationController
 
     # Recreate code challenge based on method
     expected_challenge = case auth_code.code_challenge_method
-    when "plain"
-      code_verifier
     when "S256"
       Base64.urlsafe_encode64(Digest::SHA256.digest(code_verifier), padding: false)
     else
       return {
         valid: false,
-        error: "server_error",
-        error_description: "Unsupported code challenge method",
-        status: :internal_server_error
+        error: "invalid_request",
+        error_description: "Unsupported code challenge method: only 'S256' is supported",
+        status: :bad_request
       }
     end
 
@@ -1156,6 +1179,7 @@ class OidcController < ApplicationController
   # id_token and/or userinfo keys, each mapping to claim requests
   def parse_claims_parameter(claims_string)
     return {} if claims_string.blank?
+    return nil if claims_string.length > 4096
 
     parsed = JSON.parse(claims_string)
     return nil unless parsed.is_a?(Hash)

app/controllers/sessions_controller.rb

@@ -76,6 +76,7 @@ class SessionsController < ApplicationController
       # TOTP is enabled, proceed to verification
       # Store user ID in session temporarily for TOTP verification
       session[:pending_totp_user_id] = user.id
+      session[:pending_remember_me] = remember_me?
       # Preserve the redirect URL through TOTP verification (after validation)
       if params[:rd].present?
         validated_url = validate_redirect_url(params[:rd])
@@ -86,7 +87,7 @@ class SessionsController < ApplicationController
     end
 
     # Sign in successful (password only)
-    start_new_session_for user, acr: "1"
+    start_new_session_for user, acr: "1", remember_me: remember_me?
 
     # Use status: :see_other to ensure browser makes a GET request
     # This prevents Turbo from converting it to a TURBO_STREAM request
@@ -118,6 +119,8 @@ class SessionsController < ApplicationController
         return
       end
 
+      remember_me = session.delete(:pending_remember_me) || false
+
       # Try TOTP verification first (password + TOTP = 2FA)
       if user.verify_totp(code)
         session.delete(:pending_totp_user_id)
@@ -125,7 +128,7 @@ class SessionsController < ApplicationController
         if session[:totp_redirect_url].present?
           session[:return_to_after_authenticating] = session.delete(:totp_redirect_url)
         end
-        start_new_session_for user, acr: "2"
+        start_new_session_for user, acr: "2", remember_me: remember_me
         redirect_to after_authentication_url, notice: "Signed in successfully.", allow_other_host: true
         return
       end
@@ -137,7 +140,7 @@ class SessionsController < ApplicationController
         if session[:totp_redirect_url].present?
           session[:return_to_after_authenticating] = session.delete(:totp_redirect_url)
         end
-        start_new_session_for user, acr: "2"
+        start_new_session_for user, acr: "2", remember_me: remember_me
         redirect_to after_authentication_url, notice: "Signed in successfully using backup code.", allow_other_host: true
         return
       end
@@ -189,6 +192,7 @@ class SessionsController < ApplicationController
 
     # Store user ID in session for verification
     session[:pending_webauthn_user_id] = user.id
+    session[:pending_remember_me] = remember_me?
 
     # Store redirect URL if present
     if params[:rd].present?
@@ -284,12 +288,13 @@ class SessionsController < ApplicationController
 
       # Clean up session
       session.delete(:pending_webauthn_user_id)
+      remember_me = session.delete(:pending_remember_me) || false
       if session[:webauthn_redirect_url].present?
         session[:return_to_after_authenticating] = session.delete(:webauthn_redirect_url)
       end
 
       # Create session (WebAuthn/passkey = phishing-resistant, ACR = "2")
-      start_new_session_for user, acr: "2"
+      start_new_session_for user, acr: "2", remember_me: remember_me
 
       render json: {
         success: true,
@@ -310,6 +315,10 @@ class SessionsController < ApplicationController
 
   private
 
+  def remember_me?
+    ActiveModel::Type::Boolean.new.cast(params[:remember_me]) || false
+  end
+
   def validate_redirect_url(url)
     return nil unless url.present?
 

app/controllers/totp_controller.rb

@@ -12,6 +12,10 @@ class TotpController < ApplicationController
     @totp_secret = ROTP::Base32.random
     @provisioning_uri = ROTP::TOTP.new(@totp_secret, issuer: "Clinch").provisioning_uri(@user.email_address)
 
+    # Hold the secret server-side until the user confirms it with a valid code,
+    # so an attacker with session access cannot substitute one they control.
+    session[:pending_totp_secret] = @totp_secret
+
     # Generate QR code
     require "rqrcode"
     @qr_code = RQRCode::QRCode.new(@provisioning_uri)
@@ -19,9 +23,14 @@ class TotpController < ApplicationController
 
   # POST /totp - Verify TOTP code and enable 2FA
   def create
-    totp_secret = params[:totp_secret]
+    totp_secret = session[:pending_totp_secret]
     code = params[:code]
 
+    unless totp_secret
+      redirect_to new_totp_path, alert: "Your TOTP setup session expired. Please start again."
+      return
+    end
+
     # Verify the code works
     totp = ROTP::TOTP.new(totp_secret)
     if totp.verify(code, drift_behind: 30, drift_ahead: 30)
@@ -30,6 +39,9 @@ class TotpController < ApplicationController
       plain_codes = @user.send(:generate_backup_codes) # Use private method from User model
       @user.save!
 
+      session.delete(:pending_totp_secret)
+      TotpMailer.enabled(@user).deliver_later
+
       # Store plain codes temporarily in session for display after redirect
       session[:temp_backup_codes] = plain_codes
 

app/javascript/controllers/webauthn_controller.js

@@ -180,7 +180,8 @@ export default class extends Controller {
           "X-CSRF-Token": this.getCSRFToken()
         },
         body: JSON.stringify({
-          email: this.getUserEmail()
+          email: this.getUserEmail(),
+          remember_me: this.getRememberMe()
         })
       });
 
@@ -295,6 +296,11 @@ export default class extends Controller {
     return emailInput ? emailInput.value.trim() : "";
   }
 
+  getRememberMe() {
+    const checkbox = document.querySelector('input[name="remember_me"][type="checkbox"]');
+    return checkbox ? checkbox.checked : false;
+  }
+
   isValidEmail(email) {
     return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email);
   }

app/mailers/totp_mailer.rb

@@ -0,0 +1,7 @@
+class TotpMailer < ApplicationMailer
+  def enabled(user)
+    @user = user
+    mail subject: "Two-factor authentication enabled on your account",
+         to: user.email_address
+  end
+end

app/models/application.rb

@@ -26,7 +26,7 @@ class Application < ApplicationRecord
 
   has_one_attached :icon
 
-  # Fix SVG content type after attachment
+  before_validation :sanitize_svg_icon, if: -> { attachment_changes["icon"].present? }
   after_save :fix_icon_content_type, if: -> { icon.attached? && saved_change_to_attribute?(:id) == false }
 
   has_many :application_groups, dependent: :destroy
@@ -283,6 +283,21 @@ class Application < ApplicationRecord
     end
   end
 
+  def sanitize_svg_icon
+    return unless icon.content_type == "image/svg+xml"
+
+    raw_svg = icon.download
+    doc = Loofah.xml_document(raw_svg)
+    doc.scrub!(SvgScrubber.new)
+    clean_svg = doc.to_xml
+
+    icon.attach(
+      io: StringIO.new(clean_svg),
+      filename: icon.filename.to_s,
+      content_type: "image/svg+xml"
+    )
+  end
+
   def icon_validation
     return unless icon.attached?
 

app/models/oidc_access_token.rb

@@ -1,6 +1,7 @@
 class OidcAccessToken < ApplicationRecord
   belongs_to :application
   belongs_to :user
+  belongs_to :oidc_authorization_code, optional: true
   has_many :oidc_refresh_tokens, dependent: :destroy
 
   before_validation :generate_token, on: :create

app/models/oidc_authorization_code.rb

@@ -1,6 +1,8 @@
 class OidcAuthorizationCode < ApplicationRecord
   belongs_to :application
   belongs_to :user
+  has_many :oidc_access_tokens
+  has_many :oidc_refresh_tokens
 
   attr_accessor :plaintext_code
 
@@ -9,7 +11,7 @@ class OidcAuthorizationCode < ApplicationRecord
 
   validates :code_hmac, presence: true, uniqueness: true
   validates :redirect_uri, presence: true
-  validates :code_challenge_method, inclusion: {in: %w[plain S256], allow_nil: true}
+  validates :code_challenge_method, inclusion: {in: %w[S256], allow_nil: true}
   validate :validate_code_challenge_format, if: -> { code_challenge.present? }
 
   scope :valid, -> { where(used: false).where("expires_at > ?", Time.current) }

app/models/oidc_refresh_token.rb

@@ -2,6 +2,7 @@ class OidcRefreshToken < ApplicationRecord
   belongs_to :application
   belongs_to :user
   belongs_to :oidc_access_token
+  belongs_to :oidc_authorization_code, optional: true
 
   before_validation :generate_token, on: :create
   before_validation :set_expiry, on: :create

app/models/user.rb

@@ -107,12 +107,12 @@ class User < ApplicationRecord
       save! # Save the updated array
 
       # Log successful backup code usage for security monitoring
-      Rails.logger.info "Backup code used successfully - User ID: #{id}, IP: #{Current.session&.client_ip}"
+      Rails.logger.info "Backup code used successfully - User ID: #{id}, IP: #{Current.session&.ip_address}"
       true
     else
       # Increment failed attempt counter and log for security monitoring
       increment_backup_code_failed_attempts
-      Rails.logger.warn "Failed backup code attempt - User ID: #{id}, IP: #{Current.session&.client_ip}"
+      Rails.logger.warn "Failed backup code attempt - User ID: #{id}, IP: #{Current.session&.ip_address}"
       false
     end
   end

app/services/oidc_jwt_service.rb

@@ -1,6 +1,8 @@
 class OidcJwtService
   extend ClaimsMerger
 
+  RESERVED_CLAIMS = %i[iss sub aud exp iat nbf jti nonce azp].freeze
+
   class << self
     # Generate an ID token (JWT) for the user
     def generate_id_token(user, application, consent: nil, nonce: nil, access_token: nil, auth_time: nil, acr: nil, scopes: "openid", claims_requests: {})
@@ -79,15 +81,16 @@ class OidcJwtService
 
       # Merge custom claims from groups (arrays are combined, not overwritten)
       # Note: Custom claims from groups are always merged (not scope-dependent)
+      # Reserved claims are stripped as defense-in-depth (also validated at model layer)
       user.groups.each do |group|
-        payload = deep_merge_claims(payload, group.parsed_custom_claims)
+        payload = deep_merge_claims(payload, group.parsed_custom_claims.except(*RESERVED_CLAIMS))
       end
 
       # Merge custom claims from user (arrays are combined, other values override)
-      payload = deep_merge_claims(payload, user.parsed_custom_claims)
+      payload = deep_merge_claims(payload, user.parsed_custom_claims.except(*RESERVED_CLAIMS))
 
       # Merge app-specific custom claims (highest priority, arrays are combined)
-      payload = deep_merge_claims(payload, application.custom_claims_for_user(user))
+      payload = deep_merge_claims(payload, application.custom_claims_for_user(user).except(*RESERVED_CLAIMS))
 
       # Filter custom claims based on claims parameter
       # If claims parameter is present, only include requested custom claims

app/views/invitations_mailer/invite_user.text.erb

@@ -1,8 +1,8 @@
 You've been invited to join Clinch!
 
 To set up your account and create your password, please visit:
-#{invite_url(@user.invitation_login_token)}
+<%= invitation_url(@user.generate_token_for(:invitation_login)) %>
 
-This invitation link will expire in #{distance_of_time_in_words(0, @user.invitation_login_token_expires_in)}.
+This invitation link will expire in 24 hours.
 
 If you didn't expect this invitation, you can safely ignore this email.

app/views/sessions/new.html.erb

@@ -1,6 +1,6 @@
-<div class="mx-auto md:w-2/3 w-full" data-controller="webauthn login-form" data-webauthn-check-url-value="/webauthn/check">
+<div class="mx-auto max-w-md w-full" data-controller="webauthn login-form" data-webauthn-check-url-value="/webauthn/check">
   <div class="mb-8">
-    <h1 class="font-bold text-4xl">Sign in to Clinch</h1>
+    <h1 class="font-bold text-4xl text-center">Sign in to Clinch</h1>
   </div>
 
   <%= form_with url: signin_path, class: "contents", data: { controller: "form-errors" } do |form| %>
@@ -53,6 +53,11 @@
             class: "block shadow-sm rounded-md border border-gray-400 focus:outline-blue-600 px-3 py-2 mt-2 w-full dark:border-gray-600 dark:bg-gray-800 dark:text-gray-100" %>
       </div>
 
+      <div class="my-5 flex items-center">
+        <%= form.check_box :remember_me, { class: "rounded border-gray-400 text-blue-600 focus:ring-blue-500 dark:border-gray-600 dark:bg-gray-800" }, "1", "0" %>
+        <%= form.label :remember_me, "Remember me for 30 days", class: "ml-2 text-sm text-gray-600 dark:text-gray-400" %>
+      </div>
+
       <div class="my-5">
         <%= form.submit "Sign in",
             class: "w-full rounded-md px-3.5 py-2.5 bg-blue-600 hover:bg-blue-500 text-white font-medium cursor-pointer" %>

app/views/totp/new.html.erb

@@ -35,8 +35,6 @@
       <div>
         <h3 class="text-lg font-medium text-gray-900 dark:text-gray-100 mb-4">Step 2: Verify</h3>
         <%= form_with url: totp_path, method: :post, class: "space-y-4" do |form| %>
-          <%= hidden_field_tag :totp_secret, @totp_secret %>
-
           <div>
             <%= label_tag :code, "Verification Code", class: "block text-sm font-medium text-gray-700 dark:text-gray-300" %>
             <%= text_field_tag :code,

app/views/totp_mailer/enabled.html.erb

@@ -0,0 +1,16 @@
+<p>Hello,</p>
+
+<p>
+  Two-factor authentication was just enabled on the Clinch account for
+  <strong><%= @user.email_address %></strong>.
+</p>
+
+<p>
+  If you did this, you can ignore this email.
+</p>
+
+<p>
+  If you did <strong>not</strong> do this, your account may have been
+  accessed by someone else. Reset your password immediately and contact
+  your administrator.
+</p>

app/views/totp_mailer/enabled.text.erb

@@ -0,0 +1,9 @@
+Hello,
+
+Two-factor authentication was just enabled on the Clinch account for
+<%= @user.email_address %>.
+
+If you did this, you can ignore this email.
+
+If you did NOT do this, your account may have been accessed by someone
+else. Reset your password immediately and contact your administrator.

config/initializers/content_security_policy.rb

@@ -11,7 +11,7 @@ Rails.application.configure do
 
     # Scripts: Allow self, importmaps, unsafe-inline for Turbo/StimulusJS, and blob: for downloads
     # Note: unsafe_inline is needed for Stimulus controllers and Turbo navigation
-    policy.script_src :self, :unsafe_inline, :unsafe_eval, "blob:"
+    policy.script_src :self, :unsafe_inline, "blob:"
 
     # Styles: Allow self and unsafe_inline for TailwindCSS dynamic classes
     # and Stimulus controller style manipulations

db/migrate/20260420073319_add_oidc_authorization_code_id_to_tokens.rb

@@ -0,0 +1,8 @@
+class AddOidcAuthorizationCodeIdToTokens < ActiveRecord::Migration[8.1]
+  def change
+    add_reference :oidc_access_tokens, :oidc_authorization_code,
+                  null: true, foreign_key: true, index: true
+    add_reference :oidc_refresh_tokens, :oidc_authorization_code,
+                  null: true, foreign_key: true, index: true
+  end
+end

db/migrate/20260420080000_nullify_auth_code_fk_on_delete.rb

@@ -0,0 +1,20 @@
+class NullifyAuthCodeFkOnDelete < ActiveRecord::Migration[8.1]
+  # When an OidcAuthorizationCode is deleted (e.g. by OidcTokenCleanupJob),
+  # null out the FK on any descendant tokens instead of blocking the delete
+  # on the default RESTRICT. Token rows survive for the audit trail.
+  def up
+    remove_foreign_key :oidc_access_tokens, :oidc_authorization_codes
+    add_foreign_key :oidc_access_tokens, :oidc_authorization_codes, on_delete: :nullify
+
+    remove_foreign_key :oidc_refresh_tokens, :oidc_authorization_codes
+    add_foreign_key :oidc_refresh_tokens, :oidc_authorization_codes, on_delete: :nullify
+  end
+
+  def down
+    remove_foreign_key :oidc_access_tokens, :oidc_authorization_codes
+    add_foreign_key :oidc_access_tokens, :oidc_authorization_codes
+
+    remove_foreign_key :oidc_refresh_tokens, :oidc_authorization_codes
+    add_foreign_key :oidc_refresh_tokens, :oidc_authorization_codes
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
+ActiveRecord::Schema[8.1].define(version: 2026_04_20_080000) do
   create_table "active_storage_attachments", force: :cascade do |t|
     t.bigint "blob_id", null: false
     t.datetime "created_at", null: false
@@ -118,6 +118,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
     t.integer "application_id", null: false
     t.datetime "created_at", null: false
     t.datetime "expires_at", null: false
+    t.integer "oidc_authorization_code_id"
     t.datetime "revoked_at"
     t.string "scope"
     t.string "token_hmac"
@@ -126,6 +127,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
     t.index ["application_id", "user_id"], name: "index_oidc_access_tokens_on_application_id_and_user_id"
     t.index ["application_id"], name: "index_oidc_access_tokens_on_application_id"
     t.index ["expires_at"], name: "index_oidc_access_tokens_on_expires_at"
+    t.index ["oidc_authorization_code_id"], name: "index_oidc_access_tokens_on_oidc_authorization_code_id"
     t.index ["revoked_at"], name: "index_oidc_access_tokens_on_revoked_at"
     t.index ["token_hmac"], name: "index_oidc_access_tokens_on_token_hmac", unique: true
     t.index ["user_id"], name: "index_oidc_access_tokens_on_user_id"
@@ -162,6 +164,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
     t.datetime "created_at", null: false
     t.datetime "expires_at", null: false
     t.integer "oidc_access_token_id", null: false
+    t.integer "oidc_authorization_code_id"
     t.datetime "revoked_at"
     t.string "scope"
     t.integer "token_family_id"
@@ -172,6 +175,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
     t.index ["application_id"], name: "index_oidc_refresh_tokens_on_application_id"
     t.index ["expires_at"], name: "index_oidc_refresh_tokens_on_expires_at"
     t.index ["oidc_access_token_id"], name: "index_oidc_refresh_tokens_on_oidc_access_token_id"
+    t.index ["oidc_authorization_code_id"], name: "index_oidc_refresh_tokens_on_oidc_authorization_code_id"
     t.index ["revoked_at"], name: "index_oidc_refresh_tokens_on_revoked_at"
     t.index ["token_family_id"], name: "index_oidc_refresh_tokens_on_token_family_id"
     t.index ["token_hmac"], name: "index_oidc_refresh_tokens_on_token_hmac", unique: true
@@ -274,11 +278,13 @@ ActiveRecord::Schema[8.1].define(version: 2026_03_05_000001) do
   add_foreign_key "application_user_claims", "applications", on_delete: :cascade
   add_foreign_key "application_user_claims", "users", on_delete: :cascade
   add_foreign_key "oidc_access_tokens", "applications"
+  add_foreign_key "oidc_access_tokens", "oidc_authorization_codes", on_delete: :nullify
   add_foreign_key "oidc_access_tokens", "users"
   add_foreign_key "oidc_authorization_codes", "applications"
   add_foreign_key "oidc_authorization_codes", "users"
   add_foreign_key "oidc_refresh_tokens", "applications"
   add_foreign_key "oidc_refresh_tokens", "oidc_access_tokens"
+  add_foreign_key "oidc_refresh_tokens", "oidc_authorization_codes", on_delete: :nullify
   add_foreign_key "oidc_refresh_tokens", "users"
   add_foreign_key "oidc_user_consents", "applications"
   add_foreign_key "oidc_user_consents", "users"

test/controllers/api/forward_auth_controller_test.rb

@@ -698,6 +698,131 @@ module Api
       assert_equal 30, count, "Successful request should not reset or decrement failure counter"
     end
 
+    # fa_token Host-Binding Tests (H-2)
+    #
+    # Rails.cache is a :null_store in test, so these cases swap in a
+    # MemoryStore for the duration of each test and restore it after.
+    class FaTokenHostBindingTest < ActionDispatch::IntegrationTest
+      setup do
+        @user = users(:bob)
+        Application.create!(name: "Bound App", slug: "bound-app", app_type: "forward_auth", domain_pattern: "app.example.com", active: true)
+
+        @original_cache = Rails.cache
+        Rails.cache = ActiveSupport::Cache::MemoryStore.new
+
+        @session = Session.create!(user: @user, ip_address: "127.0.0.1", user_agent: "test")
+        @token = "test-fa-token-123"
+        Rails.cache.write(
+          "forward_auth_token:#{@token}",
+          {session_id: @session.id, host: "app.example.com"},
+          expires_in: 60.seconds
+        )
+      end
+
+      teardown do
+        Rails.cache = @original_cache
+      end
+
+      test "matching X-Forwarded-Host allows redemption" do
+        get "/api/verify", params: {fa_token: @token},
+          headers: {"X-Forwarded-Host" => "app.example.com"}
+
+        assert_response 200
+        assert_nil Rails.cache.read("forward_auth_token:#{@token}"),
+          "cache entry should be burned on successful redemption"
+      end
+
+      test "mismatched X-Forwarded-Host is rejected and cache entry survives" do
+        get "/api/verify", params: {fa_token: @token},
+          headers: {"X-Forwarded-Host" => "evil.example.com"}
+
+        # Falls through to session-cookie auth; no cookie in this test -> 302 unauth redirect
+        assert_response 302
+        assert_equal "No session cookie", response.headers["x-auth-reason"]
+
+        cached = Rails.cache.read("forward_auth_token:#{@token}")
+        assert cached.is_a?(Hash), "cache entry must NOT be burned on host mismatch"
+        assert_equal "app.example.com", cached[:host]
+      end
+
+      test "port in X-Forwarded-Host is ignored for host binding" do
+        # Note: the subsequent Application domain-pattern match uses the raw
+        # X-Forwarded-Host (with port) and would 403, but that's orthogonal to
+        # the fa_token check. Successful binding is proven by the cache entry
+        # being burned.
+        get "/api/verify", params: {fa_token: @token},
+          headers: {"X-Forwarded-Host" => "APP.example.com:8443"}
+
+        assert_nil Rails.cache.read("forward_auth_token:#{@token}"),
+          "port + case variation should still match the bound host and burn the token"
+      end
+
+      test "falls back to Host header when X-Forwarded-Host is missing" do
+        get "/api/verify", params: {fa_token: @token},
+          headers: {"Host" => "app.example.com"}
+
+        assert_response 200
+      end
+
+      test "rejects when neither X-Forwarded-Host nor Host match" do
+        get "/api/verify", params: {fa_token: @token},
+          headers: {"Host" => "unknown.example.com"}
+
+        assert_response 302
+        cached = Rails.cache.read("forward_auth_token:#{@token}")
+        assert cached.is_a?(Hash), "cache entry must survive mismatched Host"
+      end
+    end
+
+    # fa_token Creation Tests (H-2)
+    #
+    # The URL-rewriting half of the H-2 fix: tokens are only created when the
+    # return URL has a host. Path-only URLs must not produce an fa_token
+    # (no cookie race exists for same-origin redirects, and there is no
+    # host to bind against).
+    class FaTokenCreationTest < ActionDispatch::IntegrationTest
+      setup do
+        @user = users(:bob)
+        Application.create!(name: "Create App", slug: "create-app", app_type: "forward_auth", domain_pattern: "app.example.com", active: true)
+
+        @original_cache = Rails.cache
+        Rails.cache = ActiveSupport::Cache::MemoryStore.new
+      end
+
+      teardown do
+        Rails.cache = @original_cache
+      end
+
+      test "path-only return_to does not produce an fa_token or cache entry" do
+        # Path-only rd (no host) — signin should not append fa_token.
+        post "/signin",
+          params: {email_address: @user.email_address, password: "password", rd: "/profile"}
+
+        assert_response 303
+        refute_match(/fa_token=/, response.location, "no fa_token for path-only return_to")
+      end
+
+      test "cross-origin return_to produces an fa_token bound to that host" do
+        # First bounce through /api/verify to populate session[:return_to_after_authenticating]
+        # with a full URL, then sign in.
+        get "/api/verify", headers: {"X-Forwarded-Host" => "app.example.com"}
+        assert_response 302
+
+        post "/signin",
+          params: {email_address: @user.email_address, password: "password"}
+        assert_response 303
+
+        # Extract the fa_token that was appended.
+        assert_match(/fa_token=([^&]+)/, response.location)
+        token = response.location[/fa_token=([^&]+)/, 1]
+
+        cached = Rails.cache.read("forward_auth_token:#{token}")
+        assert cached.is_a?(Hash), "cache entry should be a Hash, not legacy integer"
+        assert_equal "app.example.com", cached[:host]
+        assert cached[:session_id].present?
+      end
+    end
+
     # Performance and Load Tests
     test "should handle requests efficiently under load" do
       sign_in_as(@user)

test/controllers/oidc_authorization_code_security_test.rb

@@ -846,4 +846,62 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest
     old_token_record = OidcRefreshToken.find(refresh_token.id)
     assert old_token_record.revoked?
   end
+
+  test "code replay revokes the full token chain including rotated descendants" do
+    OidcUserConsent.create!(
+      user: @user,
+      application: @application,
+      scopes_granted: "openid profile",
+      granted_at: Time.current,
+      sid: "test-sid-chain"
+    )
+
+    auth_code = OidcAuthorizationCode.create!(
+      application: @application,
+      user: @user,
+      redirect_uri: "http://localhost:4000/callback",
+      scope: "openid profile",
+      expires_at: 10.minutes.from_now
+    )
+
+    basic = "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@plain_client_secret}")
+
+    # Initial exchange -> A1 + R1
+    post "/oauth/token", params: {
+      grant_type: "authorization_code",
+      code: auth_code.plaintext_code,
+      redirect_uri: "http://localhost:4000/callback"
+    }, headers: {"Authorization" => basic}
+    assert_response :success
+    first_refresh = JSON.parse(@response.body)["refresh_token"]
+
+    # Rotate once -> A2 + R2 (same auth_code FK carried forward)
+    post "/oauth/token", params: {
+      grant_type: "refresh_token",
+      refresh_token: first_refresh
+    }, headers: {"Authorization" => basic}
+    assert_response :success
+
+    # Sanity: the full chain is now linked to the auth_code
+    assert_equal 2, auth_code.oidc_access_tokens.count
+    assert_equal 2, auth_code.oidc_refresh_tokens.count
+
+    # Replay the original code
+    post "/oauth/token", params: {
+      grant_type: "authorization_code",
+      code: auth_code.plaintext_code,
+      redirect_uri: "http://localhost:4000/callback"
+    }, headers: {"Authorization" => basic}
+    assert_response :bad_request
+
+    # Every descendant token must now have revoked_at set
+    auth_code.oidc_access_tokens.each do |token|
+      assert_not_nil token.reload.revoked_at,
+        "access token #{token.id} should have revoked_at set after replay"
+    end
+    auth_code.oidc_refresh_tokens.each do |token|
+      assert_not_nil token.reload.revoked_at,
+        "refresh token #{token.id} should have revoked_at set after replay"
+    end
+  end
 end

test/controllers/oidc_pkce_controller_test.rb

@@ -33,8 +33,7 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
     config = JSON.parse(@response.body)
 
     assert config.key?("code_challenge_methods_supported")
-    assert_includes config["code_challenge_methods_supported"], "S256"
-    assert_includes config["code_challenge_methods_supported"], "plain"
+    assert_equal ["S256"], config["code_challenge_methods_supported"]
   end
 
   test "authorization endpoint accepts PKCE parameters (S256)" do
@@ -58,7 +57,7 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
     assert_match(/consent/, @response.body.downcase)
   end
 
-  test "authorization endpoint accepts PKCE parameters (plain)" do
+  test "authorization endpoint rejects PKCE plain method" do
     code_challenge = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
 
     auth_params = {
@@ -74,9 +73,9 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
 
     get "/oauth/authorize", params: auth_params
 
-    # Should show consent page (user is already authenticated)
-    assert_response :success
-    assert_match(/consent/, @response.body.downcase)
+    assert_response :redirect
+    assert_match(/error=invalid_request/, @response.location)
+    assert_match(/S256/, @response.location)
   end
 
   test "authorization endpoint rejects invalid code_challenge_method" do
@@ -153,7 +152,7 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
     assert_match(/code_verifier is required/, error["error_description"])
   end
 
-  test "token endpoint requires code_verifier when PKCE was used (plain)" do
+  test "token endpoint requires code_verifier when PKCE was used" do
     # Create consent for token endpoint
     OidcUserConsent.create!(
       user: @user,
@@ -163,14 +162,16 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
       sid: "test-sid-123"
     )
 
-    # Create authorization code with PKCE plain
+    code_verifier = "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk"
+    code_challenge = Base64.urlsafe_encode64(Digest::SHA256.digest(code_verifier), padding: false)
+
     auth_code = OidcAuthorizationCode.create!(
       application: @application,
       user: @user,
       redirect_uri: "http://localhost:4000/callback",
       scope: "openid profile",
-      code_challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM",
-      code_challenge_method: "plain",
+      code_challenge: code_challenge,
+      code_challenge_method: "S256",
       expires_at: 10.minutes.from_now
     )
 
@@ -274,28 +275,24 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
     assert_equal "Bearer", tokens["token_type"]
   end
 
-  test "token endpoint accepts valid code_verifier (plain)" do
-    # Create consent for token endpoint
-    OidcUserConsent.create!(
-      user: @user,
-      application: @application,
-      scopes_granted: "openid profile",
-      granted_at: Time.current,
-      sid: "test-sid-123"
-    )
-
+  test "token endpoint rejects code_verifier with plain challenge method" do
     code_verifier = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
 
-    # Create authorization code with PKCE plain
-    auth_code = OidcAuthorizationCode.create!(
+    # Directly insert a plain auth code to simulate legacy data
+    # Generate code HMAC manually since save!(validate: false) skips before_validation
+    plaintext_code = SecureRandom.urlsafe_base64(32)
+    auth_code = OidcAuthorizationCode.new(
       application: @application,
       user: @user,
       redirect_uri: "http://localhost:4000/callback",
       scope: "openid profile",
-      code_challenge: code_verifier, # Same as verifier for plain method
+      code_challenge: code_verifier,
       code_challenge_method: "plain",
+      code_hmac: OidcAuthorizationCode.compute_code_hmac(plaintext_code),
       expires_at: 10.minutes.from_now
     )
+    auth_code.plaintext_code = plaintext_code
+    auth_code.save!(validate: false)
 
     token_params = {
       grant_type: "authorization_code",
@@ -308,11 +305,9 @@ class OidcPkceControllerTest < ActionDispatch::IntegrationTest
       "Authorization" => "Basic " + Base64.strict_encode64("#{@application.client_id}:#{@application.client_secret}")
     }
 
-    assert_response :success
-    tokens = JSON.parse(@response.body)
-    assert tokens.key?("access_token")
-    assert tokens.key?("id_token")
-    assert_equal "Bearer", tokens["token_type"]
+    assert_response :bad_request
+    body = JSON.parse(@response.body)
+    assert_equal "invalid_request", body["error"]
   end
 
   test "token endpoint works without PKCE (backward compatibility)" do

test/controllers/totp_security_test.rb

@@ -257,6 +257,79 @@ class TotpSecurityTest < ActionDispatch::IntegrationTest
   # TOTP RECOVERY FLOW TESTS
   # ====================
 
+  # ====================
+  # TOTP ENROLLMENT — SECRET BINDING TESTS (H-1)
+  # ====================
+
+  test "enrollment uses the server-issued secret, not any client-submitted one" do
+    user = User.create!(email_address: "totp_enroll_binding@example.com", password: "password123")
+    post signin_path, params: {email_address: "totp_enroll_binding@example.com", password: "password123"}
+    assert_response :redirect
+
+    # Start enrollment: server generates a secret and stores it in the session
+    get new_totp_path
+    assert_response :success
+
+    # Attacker-supplied secret + a code that's valid for THAT secret must not
+    # succeed. The server should only ever consider its own session-stored secret.
+    attacker_secret = ROTP::Base32.random
+    attacker_code = ROTP::TOTP.new(attacker_secret).now
+
+    assert_no_enqueued_emails do
+      post totp_path, params: {totp_secret: attacker_secret, code: attacker_code}
+    end
+
+    user.reload
+    assert_nil user.totp_secret, "attacker-chosen secret must not be saved"
+    assert_not user.totp_enabled?
+
+    user.sessions.delete_all
+    user.destroy
+  end
+
+  test "enrollment succeeds, clears pending secret, and notifies the user by email" do
+    user = User.create!(email_address: "totp_enroll_success@example.com", password: "password123")
+    post signin_path, params: {email_address: "totp_enroll_success@example.com", password: "password123"}
+    assert_response :redirect
+
+    get new_totp_path
+    assert_response :success
+
+    # Pull the session-stored secret and produce a valid code for it
+    pending_secret = session[:pending_totp_secret]
+    assert pending_secret.present?, "new action should stash the secret in session"
+    valid_code = ROTP::TOTP.new(pending_secret).now
+
+    assert_enqueued_email_with TotpMailer, :enabled, args: [user] do
+      post totp_path, params: {code: valid_code}
+    end
+
+    user.reload
+    assert_equal pending_secret, user.totp_secret
+    assert user.totp_enabled?
+    assert_nil session[:pending_totp_secret], "pending secret must be cleared after enrollment"
+
+    user.sessions.delete_all
+    user.destroy
+  end
+
+  test "enrollment without a prior GET new is rejected" do
+    user = User.create!(email_address: "totp_enroll_no_session@example.com", password: "password123")
+    post signin_path, params: {email_address: "totp_enroll_no_session@example.com", password: "password123"}
+    assert_response :redirect
+
+    # Skip GET /totp/new — no session[:pending_totp_secret] is set
+    post totp_path, params: {code: "123456"}
+    assert_redirected_to new_totp_path
+
+    user.reload
+    assert_nil user.totp_secret
+    assert_not user.totp_enabled?
+
+    user.sessions.delete_all
+    user.destroy
+  end
+
   test "user can sign in with backup code when TOTP device is lost" do
     user = User.create!(email_address: "totp_recovery_test@example.com", password: "password123")
 

test/jobs/oidc_token_cleanup_job_test.rb

@@ -1,7 +1,51 @@
 require "test_helper"
 
 class OidcTokenCleanupJobTest < ActiveJob::TestCase
-  # test "the truth" do
-  #   assert true
-  # end
+  include ActiveSupport::Testing::TimeHelpers
+
+  # Regression: deleting an old authorization code while a descendant token
+  # still references it must not blow up on the FK. We rely on ON DELETE
+  # SET NULL so the token row survives (audit trail) with a NULL FK.
+  test "deletes old authorization codes whose descendant tokens still reference them" do
+    user = User.create!(email_address: "cleanup_test@example.com", password: "password123")
+    application = Application.create!(
+      name: "Cleanup Test App",
+      slug: "cleanup-test-app",
+      app_type: "oidc",
+      redirect_uris: ["http://localhost/cb"].to_json,
+      active: true
+    )
+
+    auth_code = nil
+    travel_to(10.days.ago) do
+      auth_code = OidcAuthorizationCode.create!(
+        application: application,
+        user: user,
+        redirect_uri: "http://localhost/cb",
+        scope: "openid"
+      )
+    end
+
+    token = OidcAccessToken.create!(
+      application: application,
+      user: user,
+      scope: "openid",
+      oidc_authorization_code: auth_code
+    )
+
+    OidcTokenCleanupJob.new.perform
+
+    assert_not OidcAuthorizationCode.exists?(auth_code.id),
+      "old authorization code should be deleted"
+    assert OidcAccessToken.exists?(token.id),
+      "token row should survive for audit trail"
+    assert_nil token.reload.oidc_authorization_code_id,
+      "token FK should be nullified by ON DELETE SET NULL"
+  ensure
+    OidcRefreshToken.where(application: application).delete_all if application
+    OidcAccessToken.where(application: application).delete_all if application
+    OidcAuthorizationCode.where(application: application).delete_all if application
+    user&.destroy
+    application&.destroy
+  end
 end

test/mailers/totp_mailer_test.rb

@@ -0,0 +1,19 @@
+require "test_helper"
+
+class TotpMailerTest < ActionMailer::TestCase
+  test "enabled email addresses the user and names the event" do
+    user = User.create!(email_address: "totp_mailer_test@example.com", password: "password123")
+
+    email = TotpMailer.enabled(user)
+
+    assert_equal ["totp_mailer_test@example.com"], email.to
+    assert_equal "Two-factor authentication enabled on your account", email.subject
+    text_body = email.text_part.body.to_s
+    html_body = email.html_part.body.to_s
+    assert_match "totp_mailer_test@example.com", text_body
+    assert_match "totp_mailer_test@example.com", html_body
+    assert_match(/Reset your password/i, text_body)
+
+    user.destroy
+  end
+end

test/models/pkce_authorization_code_test.rb

@@ -38,23 +38,18 @@ class PkceAuthorizationCodeTest < ActiveSupport::TestCase
     assert auth_code.uses_pkce?
   end
 
-  test "authorization code can store PKCE challenge with plain method" do
-    code_challenge = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"
-    code_challenge_method = "plain"
-
-    auth_code = OidcAuthorizationCode.create!(
+  test "authorization code rejects plain PKCE method" do
+    assert_raises(ActiveRecord::RecordInvalid) do
+      OidcAuthorizationCode.create!(
         application: @application,
         user: @user,
         redirect_uri: "http://localhost:4000/callback",
         scope: "openid profile",
-      code_challenge: code_challenge,
-      code_challenge_method: code_challenge_method,
+        code_challenge: "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM",
+        code_challenge_method: "plain",
         expires_at: 10.minutes.from_now
       )
-
-    assert_equal code_challenge, auth_code.code_challenge
-    assert_equal code_challenge_method, auth_code.code_challenge_method
-    assert auth_code.uses_pkce?
+    end
   end
 
   test "authorization code works without PKCE (backward compatibility)" do