12 Commits

Author SHA1 Message Date
Dan Milne
e39721c7e6 Fix broken invitation email text template
Some checks failed
CI / scan_ruby (push) Has been cancelled
CI / scan_js (push) Has been cancelled
CI / scan_container (push) Has been cancelled
CI / lint (push) Has been cancelled
CI / test (push) Has been cancelled
CI / system-test (push) Has been cancelled
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>
2026-05-02 23:39:29 +10:00
Dan Milne
5178cf3d81 Drop redundant MemoryStore internals peek from fa_token creation test
The refute_match on response.location already proves create_forward_auth_token
did nothing: the cache.write and the URL rewrite are back-to-back with no
branch between them, so the URL lacking fa_token= implies no cache entry
was written. The instance_variable_get(:@data) inspection was both redundant
and coupled to MemoryStore's private layout.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
2026-04-20 20:28:28 +10:00
Dan Milne
2d5650e620 Bind forward-auth fa_token to its destination host
An observed fa_token (via Referer leaks, access logs, JS monitors)
could previously be redeemed against a different reverse-proxied app
within the 60s TTL. The token now stores the destination host at
creation and the verifier rejects mismatches without burning the cache
entry, so legitimate destinations can still redeem.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
2026-04-20 19:04:53 +10:00
Dan Milne
7f0d3d3900 Tighten TOTP enrollment comments to explain the threat, not the change
Replace the changelog-flavored "view no longer round-trips" line with a
one-liner naming the actual threat (session-holder substituting a secret
they control). Drop the narration comment above session.delete +
deliver_later -- the identifiers already say what the two lines do.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
2026-04-20 18:58:39 +10:00
Dan Milne
b876e02c3a Hold TOTP enrollment secret server-side and email user on activation
TOTP enrollment previously round-tripped the generated secret through a
hidden form field and saved whatever the client submitted, letting an
attacker with session access enroll a 2FA device they control by posting
their own secret plus a matching code. Stash the secret in the session
at GET /totp/new, read it only from the session at POST /totp, and drop
the hidden field from the view. Notify the user by email on successful
enrollment so unauthorized activations are visible even if a new vector
appears later.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
2026-04-20 18:17:50 +10:00
Dan Milne
93d8381214 Nullify token to auth-code FK on delete so cleanup job can purge codes
The FK added in b7fa499 defaulted to ON DELETE RESTRICT, which means
OidcTokenCleanupJob#perform would fail when deleting auth codes older
than 7 days if any refresh token (whose expiry is days-to-weeks) still
referenced them. Switch both token FKs to ON DELETE SET NULL so token
rows survive the code deletion with a NULL FK, preserving the audit
trail the cleanup job deliberately keeps.

Add a regression test covering the exact scenario: a 10-day-old auth
code with a token still pointing at it -> cleanup deletes the code,
token survives, token FK is nulled.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
2026-04-20 18:12:03 +10:00
Dan Milne
2068675173 Collapse auth-code replay revocation to two update_all queries
The previous implementation iterated find_each(&:revoke!) on both the
access-token and refresh-token associations. OidcAccessToken#revoke!
also cascades to its refresh tokens, so a chain of N access tokens with
their refresh tokens produced ~3N UPDATEs (outer loop + cascade +
outer refresh loop double-writing) all while holding a pessimistic
lock on the auth_code row. Replace with scoped update_all on each
association -- 2 UPDATEs total, no behavior change.

Also hoist the repeated refresh_token_record.oidc_authorization_code
lookup in the rotation path to a named local and drop the duplicated
inline comment.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
2026-04-20 18:11:54 +10:00
Dan Milne
b7fa49953c Revoke full token chain on OIDC authorization-code replay
The replay handler previously used a created_at time-range filter to
target access tokens and called update_all(expires_at:), which left
revoked_at nil, skipped refresh tokens entirely, and could miss or
falsely catch tokens from concurrent flows. Add an oidc_authorization_code
FK on both token tables, carry it through refresh-token rotation, and
use the association to revoke every descendant via revoke! (which sets
revoked_at and cascades access -> refresh).

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
2026-04-20 17:39:08 +10:00
Dan Milne
b7dd3c02e7 Extract client_id and redirect_uri validation into before_actions
The authorize action opened with ~55 lines of parameter validation that
ran before any business logic. Move the two RFC 6749 §4.1.2.1 checks
(client_id lookup, redirect_uri registration) into set_application and
validate_redirect_uri before_actions. The action body now starts at the
point where errors may legitimately redirect back to the client.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
2026-04-20 17:29:36 +10:00
Dan Milne
17a464fd15 Fix OIDC claims validation against undefined scopes variable
The authorize action called validate_claims_against_scopes with
requested_scopes before that local was assigned (assignment was ~100
lines later), raising NameError whenever a client passed a claims=
parameter. Move the scope normalization above the claims validation.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
2026-04-20 17:26:46 +10:00
Dan Milne
9197524c88 Add remember me checkbox, center and narrow sign-in form
- Add "Remember me for 30 days" checkbox (30-day vs 24-hour session expiry)
- Center heading and constrain form width to max-w-md
- Preserve remember_me preference through TOTP and WebAuthn auth flows

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-11 11:22:51 +10:00
Dan Milne
2235924f37 Harden OIDC, add SVG sanitization, improve form UX and security defaults
Remove PKCE plain method support (S256 only), enforce openid scope requirement,
filter to supported scopes, strip reserved claims from custom claims as
defense-in-depth, sanitize SVG icons with Loofah, add global input padding,
switch session cookies to SameSite=Lax, use Session.active scope, and remove
unsafe-eval from CSP.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-06 21:06:51 +10:00
31 changed files with 643 additions and 178 deletions

View File

@@ -3,6 +3,12 @@
@custom-variant dark (&:where(.dark, .dark *)); @custom-variant dark (&:where(.dark, .dark *));
@layer base { @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 input:where([type="text"], [type="email"], [type="password"], [type="number"], [type="url"], [type="tel"], [type="search"]),
.dark textarea, .dark textarea,
.dark select { .dark select {

View File

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

View File

@@ -163,16 +163,25 @@ module Api
def check_forward_auth_token def check_forward_auth_token
token = params[:fa_token] token = params[:fa_token]
return nil unless token.present? return nil if token.blank?
session_id = Rails.cache.read("forward_auth_token:#{token}") cached = Rails.cache.read("forward_auth_token:#{token}")
return nil unless session_id 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? return nil unless session && !session.expired?
Rails.cache.delete("forward_auth_token:#{token}") Rails.cache.delete("forward_auth_token:#{token}")
session_id cached[:session_id]
end end
def extract_session_id def extract_session_id

View File

@@ -31,7 +31,7 @@ module Authentication
end end
def find_session_by_cookie 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 end
def request_authentication def request_authentication
@@ -43,9 +43,9 @@ module Authentication
session.delete(:return_to_after_authenticating) || root_url session.delete(:return_to_after_authenticating) || root_url
end 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.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 Current.session = session
# Extract root domain for cross-subdomain cookies (required for forward auth) # Extract root domain for cross-subdomain cookies (required for forward auth)
@@ -58,8 +58,8 @@ module Authentication
{ {
value: session.id, value: session.id,
httponly: true, httponly: true,
same_site: :none, # Allow cross-site cookies for OIDC testing same_site: :lax,
secure: true # Required for SameSite=None secure: true
} }
else else
{ {
@@ -130,35 +130,35 @@ module Authentication
end end
# Create a one-time token for forward auth to handle the race condition # 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) def create_forward_auth_token(session_obj)
# Generate a secure random token controller_session = session
token = SecureRandom.urlsafe_base64(32) 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( Rails.cache.write(
"forward_auth_token:#{token}", "forward_auth_token:#{token}",
session_obj.id, { session_id: session_obj.id, host: bound_host },
expires_in: 60.seconds 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 = URI.decode_www_form(uri.query || "").to_h
query_params["fa_token"] = token query_params["fa_token"] = token
uri.query = URI.encode_www_form(query_params) uri.query = URI.encode_www_form(query_params)
# Update the session with the tokenized URL
controller_session[:return_to_after_authenticating] = uri.to_s controller_session[:return_to_after_authenticating] = uri.to_s
end end
end
end
end end

View File

@@ -1,9 +1,16 @@
class OidcController < ApplicationController class OidcController < ApplicationController
SUPPORTED_SCOPES = %w[openid profile email groups offline_access].freeze
# Discovery and JWKS endpoints are public # Discovery and JWKS endpoints are public
# authorize is also unauthenticated to handle prompt=none and prompt=login specially # authorize is also unauthenticated to handle prompt=none and prompt=login specially
allow_unauthenticated_access only: [:discovery, :jwks, :token, :revoke, :userinfo, :logout, :authorize] allow_unauthenticated_access only: [:discovery, :jwks, :token, :revoke, :userinfo, :logout, :authorize]
skip_before_action :verify_authenticity_token, only: [:token, :revoke, :userinfo, :logout, :authorize, :consent] 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 limiting to prevent brute force and abuse
rate_limit to: 60, within: 1.minute, only: [:token, :revoke], with: -> { 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 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"], grant_types_supported: ["authorization_code", "refresh_token"],
subject_types_supported: ["pairwise"], subject_types_supported: ["pairwise"],
id_token_signing_alg_values_supported: ["RS256"], 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"], token_endpoint_auth_methods_supported: ["client_secret_post", "client_secret_basic"],
claims_supported: [ claims_supported: [
"sub", # Always included "sub", # Always included
@@ -42,7 +49,7 @@ class OidcController < ApplicationController
# Note: Custom claims are also supported but not listed here # Note: Custom claims are also supported but not listed here
# ID-token-only claims (auth_time, acr, azp, at_hash, nonce) are not listed # 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_supported: true,
backchannel_logout_session_supported: true, backchannel_logout_session_supported: true,
request_parameter_supported: false, request_parameter_supported: false,
@@ -59,7 +66,8 @@ class OidcController < ApplicationController
# GET /oauth/authorize # GET /oauth/authorize
def 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] client_id = params[:client_id]
redirect_uri = params[:redirect_uri] redirect_uri = params[:redirect_uri]
state = params[:state] state = params[:state]
@@ -67,57 +75,10 @@ class OidcController < ApplicationController
scope = params[:scope] || "openid" scope = params[:scope] || "openid"
response_type = params[:response_type] response_type = params[:response_type]
code_challenge = params[:code_challenge] code_challenge = params[:code_challenge]
code_challenge_method = params[:code_challenge_method] || "plain" code_challenge_method = params[:code_challenge_method] || "S256"
# 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
# ============================================================================ # ============================================================================
# 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 # All subsequent errors should redirect back to the client with error parameters
# per OAuth2 RFC 6749 Section 4.1.2.1 # 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) # Validate PKCE parameters if present (now we can safely redirect with error)
if code_challenge.present? 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}" Rails.logger.error "OAuth: Invalid code_challenge_method: #{code_challenge_method}"
error_uri = "#{redirect_uri}?error=invalid_request" 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? error_uri += "&state=#{CGI.escape(state)}" if state.present?
redirect_to error_uri, allow_other_host: true redirect_to error_uri, allow_other_host: true
return return
@@ -166,6 +127,12 @@ class OidcController < ApplicationController
end end
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 # Parse claims parameter (JSON string) for OIDC claims request
# Per OIDC Core §5.5: The claims parameter is a JSON object that requests # 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 # specific claims to be returned in the id_token and/or userinfo
@@ -289,7 +256,12 @@ class OidcController < ApplicationController
return return
end 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 # Check if application is configured to skip consent
# If so, automatically create consent and proceed without showing consent screen # If so, automatically create consent and proceed without showing consent screen
@@ -420,8 +392,7 @@ class OidcController < ApplicationController
user = Current.session.user user = Current.session.user
# Record user consent requested_scopes = oauth_params["scope"].split(" ") & SUPPORTED_SCOPES
requested_scopes = oauth_params["scope"].split(" ")
parsed_claims = begin parsed_claims = begin
JSON.parse(oauth_params["claims_requests"]) JSON.parse(oauth_params["claims_requests"])
rescue rescue
@@ -539,15 +510,12 @@ class OidcController < ApplicationController
# Check if code has already been used (CRITICAL: check AFTER locking) # Check if code has already been used (CRITICAL: check AFTER locking)
if auth_code.used? 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}" Rails.logger.warn "OAuth Security: Authorization code reuse detected for code #{auth_code.id}"
now = Time.current
# Revoke all access tokens issued from this authorization code auth_code.oidc_access_tokens.where(revoked_at: nil).update_all(revoked_at: now)
OidcAccessToken.where( auth_code.oidc_refresh_tokens.where(revoked_at: nil).update_all(revoked_at: now)
application: application,
user: auth_code.user,
created_at: auth_code.created_at..Time.current
).update_all(expires_at: Time.current)
render json: { render json: {
error: "invalid_grant", error: "invalid_grant",
@@ -588,7 +556,8 @@ class OidcController < ApplicationController
access_token_record = OidcAccessToken.create!( access_token_record = OidcAccessToken.create!(
application: application, application: application,
user: user, user: user,
scope: auth_code.scope scope: auth_code.scope,
oidc_authorization_code: auth_code
) )
# Generate refresh token (opaque, with hashing) # Generate refresh token (opaque, with hashing)
@@ -596,6 +565,7 @@ class OidcController < ApplicationController
application: application, application: application,
user: user, user: user,
oidc_access_token: access_token_record, oidc_access_token: access_token_record,
oidc_authorization_code: auth_code,
scope: auth_code.scope, scope: auth_code.scope,
auth_time: auth_code.auth_time, auth_time: auth_code.auth_time,
acr: auth_code.acr acr: auth_code.acr
@@ -720,10 +690,15 @@ class OidcController < ApplicationController
refresh_token_record.revoke! refresh_token_record.revoke!
# Generate new access token record (opaque token with BCrypt hashing) # 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!( new_access_token = OidcAccessToken.create!(
application: application, application: application,
user: user, user: user,
scope: refresh_token_record.scope scope: refresh_token_record.scope,
oidc_authorization_code: issuing_auth_code
) )
# Generate new refresh token (token rotation) # Generate new refresh token (token rotation)
@@ -731,6 +706,7 @@ class OidcController < ApplicationController
application: application, application: application,
user: user, user: user,
oidc_access_token: new_access_token, oidc_access_token: new_access_token,
oidc_authorization_code: issuing_auth_code,
scope: refresh_token_record.scope, scope: refresh_token_record.scope,
token_family_id: refresh_token_record.token_family_id, # Keep same family for rotation tracking 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 auth_time: refresh_token_record.auth_time, # Carry over original auth_time
@@ -1000,6 +976,55 @@ class OidcController < ApplicationController
private 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) def validate_pkce(application, auth_code, code_verifier)
# Check if PKCE is required for this application # Check if PKCE is required for this application
pkce_required = application.requires_pkce? pkce_required = application.requires_pkce?
@@ -1041,16 +1066,14 @@ class OidcController < ApplicationController
# Recreate code challenge based on method # Recreate code challenge based on method
expected_challenge = case auth_code.code_challenge_method expected_challenge = case auth_code.code_challenge_method
when "plain"
code_verifier
when "S256" when "S256"
Base64.urlsafe_encode64(Digest::SHA256.digest(code_verifier), padding: false) Base64.urlsafe_encode64(Digest::SHA256.digest(code_verifier), padding: false)
else else
return { return {
valid: false, valid: false,
error: "server_error", error: "invalid_request",
error_description: "Unsupported code challenge method", error_description: "Unsupported code challenge method: only 'S256' is supported",
status: :internal_server_error status: :bad_request
} }
end end
@@ -1156,6 +1179,7 @@ class OidcController < ApplicationController
# id_token and/or userinfo keys, each mapping to claim requests # id_token and/or userinfo keys, each mapping to claim requests
def parse_claims_parameter(claims_string) def parse_claims_parameter(claims_string)
return {} if claims_string.blank? return {} if claims_string.blank?
return nil if claims_string.length > 4096
parsed = JSON.parse(claims_string) parsed = JSON.parse(claims_string)
return nil unless parsed.is_a?(Hash) return nil unless parsed.is_a?(Hash)

View File

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

View File

@@ -12,6 +12,10 @@ class TotpController < ApplicationController
@totp_secret = ROTP::Base32.random @totp_secret = ROTP::Base32.random
@provisioning_uri = ROTP::TOTP.new(@totp_secret, issuer: "Clinch").provisioning_uri(@user.email_address) @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 # Generate QR code
require "rqrcode" require "rqrcode"
@qr_code = RQRCode::QRCode.new(@provisioning_uri) @qr_code = RQRCode::QRCode.new(@provisioning_uri)
@@ -19,9 +23,14 @@ class TotpController < ApplicationController
# POST /totp - Verify TOTP code and enable 2FA # POST /totp - Verify TOTP code and enable 2FA
def create def create
totp_secret = params[:totp_secret] totp_secret = session[:pending_totp_secret]
code = params[:code] 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 # Verify the code works
totp = ROTP::TOTP.new(totp_secret) totp = ROTP::TOTP.new(totp_secret)
if totp.verify(code, drift_behind: 30, drift_ahead: 30) 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 plain_codes = @user.send(:generate_backup_codes) # Use private method from User model
@user.save! @user.save!
session.delete(:pending_totp_secret)
TotpMailer.enabled(@user).deliver_later
# Store plain codes temporarily in session for display after redirect # Store plain codes temporarily in session for display after redirect
session[:temp_backup_codes] = plain_codes session[:temp_backup_codes] = plain_codes

View File

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

View File

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

View File

@@ -26,7 +26,7 @@ class Application < ApplicationRecord
has_one_attached :icon 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 } after_save :fix_icon_content_type, if: -> { icon.attached? && saved_change_to_attribute?(:id) == false }
has_many :application_groups, dependent: :destroy has_many :application_groups, dependent: :destroy
@@ -283,6 +283,21 @@ class Application < ApplicationRecord
end end
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 def icon_validation
return unless icon.attached? return unless icon.attached?

View File

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

View File

@@ -1,6 +1,8 @@
class OidcAuthorizationCode < ApplicationRecord class OidcAuthorizationCode < ApplicationRecord
belongs_to :application belongs_to :application
belongs_to :user belongs_to :user
has_many :oidc_access_tokens
has_many :oidc_refresh_tokens
attr_accessor :plaintext_code attr_accessor :plaintext_code
@@ -9,7 +11,7 @@ class OidcAuthorizationCode < ApplicationRecord
validates :code_hmac, presence: true, uniqueness: true validates :code_hmac, presence: true, uniqueness: true
validates :redirect_uri, presence: 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? } validate :validate_code_challenge_format, if: -> { code_challenge.present? }
scope :valid, -> { where(used: false).where("expires_at > ?", Time.current) } scope :valid, -> { where(used: false).where("expires_at > ?", Time.current) }

View File

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

View File

@@ -107,12 +107,12 @@ class User < ApplicationRecord
save! # Save the updated array save! # Save the updated array
# Log successful backup code usage for security monitoring # 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 true
else else
# Increment failed attempt counter and log for security monitoring # Increment failed attempt counter and log for security monitoring
increment_backup_code_failed_attempts 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 false
end end
end end

View File

@@ -1,6 +1,8 @@
class OidcJwtService class OidcJwtService
extend ClaimsMerger extend ClaimsMerger
RESERVED_CLAIMS = %i[iss sub aud exp iat nbf jti nonce azp].freeze
class << self class << self
# Generate an ID token (JWT) for the user # 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: {}) 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) # Merge custom claims from groups (arrays are combined, not overwritten)
# Note: Custom claims from groups are always merged (not scope-dependent) # 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| 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 end
# Merge custom claims from user (arrays are combined, other values override) # 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) # 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 # Filter custom claims based on claims parameter
# If claims parameter is present, only include requested custom claims # If claims parameter is present, only include requested custom claims

View File

@@ -1,8 +1,8 @@
You've been invited to join Clinch! You've been invited to join Clinch!
To set up your account and create your password, please visit: 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. If you didn't expect this invitation, you can safely ignore this email.

View File

@@ -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"> <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> </div>
<%= form_with url: signin_path, class: "contents", data: { controller: "form-errors" } do |form| %> <%= 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" %> 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>
<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"> <div class="my-5">
<%= form.submit "Sign in", <%= 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" %> class: "w-full rounded-md px-3.5 py-2.5 bg-blue-600 hover:bg-blue-500 text-white font-medium cursor-pointer" %>

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

8
db/schema.rb generated
View File

@@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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| create_table "active_storage_attachments", force: :cascade do |t|
t.bigint "blob_id", null: false t.bigint "blob_id", null: false
t.datetime "created_at", 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.integer "application_id", null: false
t.datetime "created_at", null: false t.datetime "created_at", null: false
t.datetime "expires_at", null: false t.datetime "expires_at", null: false
t.integer "oidc_authorization_code_id"
t.datetime "revoked_at" t.datetime "revoked_at"
t.string "scope" t.string "scope"
t.string "token_hmac" 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", "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 ["application_id"], name: "index_oidc_access_tokens_on_application_id"
t.index ["expires_at"], name: "index_oidc_access_tokens_on_expires_at" 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 ["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 ["token_hmac"], name: "index_oidc_access_tokens_on_token_hmac", unique: true
t.index ["user_id"], name: "index_oidc_access_tokens_on_user_id" 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 "created_at", null: false
t.datetime "expires_at", null: false t.datetime "expires_at", null: false
t.integer "oidc_access_token_id", null: false t.integer "oidc_access_token_id", null: false
t.integer "oidc_authorization_code_id"
t.datetime "revoked_at" t.datetime "revoked_at"
t.string "scope" t.string "scope"
t.integer "token_family_id" 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 ["application_id"], name: "index_oidc_refresh_tokens_on_application_id"
t.index ["expires_at"], name: "index_oidc_refresh_tokens_on_expires_at" 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_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 ["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_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 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", "applications", on_delete: :cascade
add_foreign_key "application_user_claims", "users", 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", "applications"
add_foreign_key "oidc_access_tokens", "oidc_authorization_codes", on_delete: :nullify
add_foreign_key "oidc_access_tokens", "users" add_foreign_key "oidc_access_tokens", "users"
add_foreign_key "oidc_authorization_codes", "applications" add_foreign_key "oidc_authorization_codes", "applications"
add_foreign_key "oidc_authorization_codes", "users" add_foreign_key "oidc_authorization_codes", "users"
add_foreign_key "oidc_refresh_tokens", "applications" add_foreign_key "oidc_refresh_tokens", "applications"
add_foreign_key "oidc_refresh_tokens", "oidc_access_tokens" 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_refresh_tokens", "users"
add_foreign_key "oidc_user_consents", "applications" add_foreign_key "oidc_user_consents", "applications"
add_foreign_key "oidc_user_consents", "users" add_foreign_key "oidc_user_consents", "users"

View File

@@ -698,6 +698,131 @@ module Api
assert_equal 30, count, "Successful request should not reset or decrement failure counter" assert_equal 30, count, "Successful request should not reset or decrement failure counter"
end 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 # Performance and Load Tests
test "should handle requests efficiently under load" do test "should handle requests efficiently under load" do
sign_in_as(@user) sign_in_as(@user)

View File

@@ -846,4 +846,62 @@ class OidcAuthorizationCodeSecurityTest < ActionDispatch::IntegrationTest
old_token_record = OidcRefreshToken.find(refresh_token.id) old_token_record = OidcRefreshToken.find(refresh_token.id)
assert old_token_record.revoked? assert old_token_record.revoked?
end 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 end

View File

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

View File

@@ -257,6 +257,79 @@ class TotpSecurityTest < ActionDispatch::IntegrationTest
# TOTP RECOVERY FLOW TESTS # 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 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") user = User.create!(email_address: "totp_recovery_test@example.com", password: "password123")

View File

@@ -1,7 +1,51 @@
require "test_helper" require "test_helper"
class OidcTokenCleanupJobTest < ActiveJob::TestCase class OidcTokenCleanupJobTest < ActiveJob::TestCase
# test "the truth" do include ActiveSupport::Testing::TimeHelpers
# assert true
# end # 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 end

View File

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

View File

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