fix(oidc): Enterprise IdP compatibility — RFC-compliant errors, auth_time, TTL, discovery#604
Merged
lakhansamani merged 35 commits intomainfrom Apr 11, 2026
Merged
Conversation
…code exchange The hybrid flow path in authorize.go stored authToken.FingerPrint (the raw nonce) instead of authToken.FingerPrintHash (the AES-encrypted session data) when stashing the code for /oauth/token exchange. When /oauth/token later calls ValidateBrowserSession on sessionDataSplit[1], it tries to AES-decrypt the value. Since the nonce is not AES-encrypted, this always fails for hybrid flow codes. The other two code paths (code flow at line 520 and oauth_callback at line 331) correctly store AES-encrypted session values.
…ssion The scope override condition in signup.go and session.go checked len(scope) (the default list, always 3) instead of len(params.Scope), making it impossible to pass an empty scope list and retain defaults. Fixed to match the correct pattern already used in login.go. Added integration tests verifying that an empty Scope slice falls back to the default scopes (openid, email, profile).
…time token comparison - verify_otp.go: change `otp == nil && err != nil` to `otp == nil` to prevent nil pointer dereference when storage returns (nil, nil) - token.go: only append "@@" + code to nonce when code is non-empty, avoiding vestigial "uuid@@" in refresh_token grant flow - revoke_refresh_token.go: use crypto/subtle.ConstantTimeCompare for token comparison to prevent timing oracle attacks (RFC 7009) - add integration tests for all three fixes
…ong error message - use sanitized email/phoneNumber locals instead of raw params.Email and params.PhoneNumber when calling GetOTPByEmail/GetOTPByPhoneNumber - fix SMS-disabled error message from "email service not enabled" to "SMS service not enabled" - add clarifying comment on the MFA/verified guard condition - add integration tests for sanitized-email resend and SMS error message
…ing in oauth_callback - Fix scope parsing to use else-if so comma-delimited scopes aren't silently overwritten by space-split; handle single-value scopes - Convert all unsafe type assertions to safe form with ok-checking across Facebook, LinkedIn, Twitter, Discord, and Roblox processors - Add error checking for all json.Unmarshal calls that were silently dropping parse failures (GitHub, Facebook, LinkedIn, Twitter, Roblox) - Extract parseScopes helper with unit tests covering comma, space, single-value, and mixed-delimiter inputs
…irect handling - Remove stale TODO comment in update_user.go; phone uniqueness check already exists at lines 198-214 with proper length validation - Change logout handler to silently ignore invalid post_logout_redirect_uri per OIDC RP-Initiated Logout §2 instead of returning a JSON 400 error - Add integration test for duplicate phone number rejection via admin _update_user mutation - Add integration test verifying invalid redirect URI no longer produces a JSON error response
… hardening - Fix S256 PKCE: tolerate base64url padding in code_challenge (Auth0 compat) - Fix client_secret bypass: always validate when provided, even with PKCE - Fix PKCE bypass: reject code_verifier when no code_challenge was registered - Add redirect_uri validation at token endpoint per RFC 6749 §4.1.3 - URL-encode redirect_uri in state to prevent @@ delimiter injection - Make authorize state removal synchronous to prevent code reuse race - Use constant-time comparison for redirect_uri at token endpoint - Remove code_challenge/state/nonce from authorize handler logs - Replace leaked err.Error() with generic message in refresh token path - Fix non-standard error codes (error_binding_json -> invalid_request) - Fix Makefile dev target: proper multi-line RSA key handling
- authorize.go: keep full hybrid state (challenge::method, nonce, redirect_uri) - token.go: keep clean nonce, code handled via state mechanism not nonce embedding
…CE, integration examples Covers all flows in plain English with architecture diagrams, code examples (JS, Go, Python), Auth0 enterprise connection setup, security considerations, and FAQ.
…RL-encodes authState Back channel fix: - URL-encode all params in authState query string sent to login UI (redirect_uri, scope, state, etc.) to prevent parsing corruption Front channel fix (root cause of Auth0 "cannot read undefined 'tenant'"): - React app now reads response_type and response_mode from URL params - For response_type=id_token: includes id_token in redirect (was missing entirely) - For response_type=token: includes access_token, token_type, expires_in - For hybrid flows: includes code + relevant tokens - Respects response_mode: uses fragment (#) for implicit flows, query (?) for code flow - Reads params from both query string and fragment to support all response_modes - Same-origin redirects now work (previously silently skipped) - Proper encodeURIComponent on all redirect params
…by SDK Root cause: when /authorize forces re-auth (prompt=login from Auth0) but the session cookie is still valid, the React SDK auto-detects the session via the GraphQL session query and redirects immediately — without ever calling the login mutation. Since SetState(code, ...) only happens in the login mutation, the code state was never stored, causing /oauth/token to return "authorization code is invalid or has expired". Fix: - Add `state` field to SessionQueryRequest GraphQL schema - Session resolver now consumes authorize state and stores code state (same logic as login/signup/verify_otp resolvers) - React app detects OIDC flow (code in URL) and calls session with state param before redirecting, ensuring the code state is stored
…rizer-js Root cause of all three Auth0 failures (PKCE plain, PKCE disabled, already-logged-in): - /authorize with prompt=login cleared the session and sent user to login UI, but the React SDK auto-detected the session cookie and redirected immediately without calling the login mutation, so the authorization code state was never stored in the backend. Fix (authorize.go): - When forceReauth is true but a valid session EXISTS, validate it and proceed with the normal code flow (session rollover + code state storage + redirect to RP) instead of sending to the login UI. - Only falls through to login UI when the session is truly invalid. Fix (Root.tsx): - Use @authorizerdev/authorizer-js client instead of raw GraphQL fetch - getSession with state param ensures code state is stored as fallback - Simplified redirect logic, no infinite loop risk
…solver + React app The /authorize forceReauth change caused redirect issues. Reverted to original behavior (prompt=login clears session, redirects to login UI). The fix for the "already logged in" case works via: 1. Session resolver (session.go) accepts state param and stores code state 2. React app (Root.tsx) detects OIDC flow and calls getSession(state) via authorizer-js before redirecting to RP 3. This ensures code state is stored even when SDK auto-detects session
…rage When prompt=login and a valid session cookie exists, the /authorize handler now keeps the session instead of discarding it. The normal flow validates the session, performs a rollover, stores the authorization code state, and redirects directly to the RP — never hitting the login UI. This fixes the Auth0 back-channel failures (PKCE plain, PKCE disabled, already-logged-in) caused by the React SDK auto-detecting the session cookie and redirecting without the login mutation ever storing the code state. For max_age=0 or max_age exceeded, the session IS discarded (spec requires genuine re-authentication based on time).
…irect immediately on token
…rise IdP compat - Refactor validateAuthorizeRequest to return RFC 6749 error codes (invalid_request, unauthorized_client, unsupported_response_type) instead of freeform strings that break Auth0/Okta/Keycloak parsing - After redirect_uri validation, redirect errors to RP via configured response_mode instead of returning JSON (RFC 6749 §4.1.2.1) - Add redirectErrorToRP helper for spec-compliant error redirects supporting query, fragment, form_post, and web_message modes - Set AuthTime (claims.IssuedAt) on all token creation paths so id_token includes auth_time claim per OIDC Core §2 when max_age used - Normalize token_type to "Bearer" (capitalized) in redirect params for consistency with token endpoint and strict RP validation - Add "none" to token_endpoint_auth_methods_supported in discovery document for PKCE-only public clients (OIDC Core §9) - Add Cache-Control header to discovery endpoint (public, max-age=3600) - Add 10-minute TTL to in-memory state store entries matching Redis provider behavior (RFC 6749 §4.1.2 authorization code expiry) - Add TTL enforcement to DB state store provider (600s expiry) - Fix React App.tsx scope default from string to array matching Root.tsx - Fix React App.tsx redirectURL fallback from window.location.href (includes query params) to window.location.origin (clean origin) - Add debug log when response_type=token used with openid scope - Remove duplicate response_type validation in validateAuthorizeRequest
…ce mismatch The nonce parameter from the RP (Auth0/Okta/Keycloak) was only appended to authState for implicit flows, not for the code flow. This caused: 1. Auth0 sends /authorize?nonce=ABC&response_type=code 2. Authorizer stores nonce=ABC in state, redirects to /app WITHOUT nonce 3. React app logs user in, redirects back to /authorize — nonce missing 4. Second /authorize visit generates NEW nonce=XYZ (line 242 fallback) 5. Token endpoint returns id_token with nonce=XYZ 6. Auth0 expected nonce=ABC → "unexpected ID Token nonce claim value" Fix: always append &nonce= to authState before the code/implicit branch, so the login UI preserves and forwards the RP-provided nonce on the second /authorize round-trip.
…h/token When the RP (Auth0/Okta/Keycloak) exchanges an authorization code at /oauth/token, the endpoint was: 1. Deleting the user's browser session (rollover at line 288) 2. Creating a new session + setting a cookie Both are wrong for the code exchange flow because /oauth/token is called server-to-server by the RP, not by the user's browser. The cookie goes to the RP (useless), and the deletion invalidates the session the user's browser holds from /authorize — causing "record not found" on subsequent session lookups. Fix: - Remove session deletion for authorization_code grant (the /authorize endpoint already performed its own rollover) - Only create new browser session + cookie for refresh_token grant (where the caller IS the user's app holding the refresh token) - Access/refresh tokens are still persisted for both grant types (needed for introspection/revocation)
…hrough login UI Same class of bug as the nonce loss: PKCE parameters and client_id were not included in authState, so when the React login UI redirects back to /authorize after authentication, these values were missing. For PKCE (code_challenge): 1. Auth0 sends /authorize?code_challenge=ABC&code_challenge_method=S256 2. User not logged in → redirect to /app WITHOUT code_challenge 3. After login, second /authorize has empty code_challenge 4. Authorization code stored with empty challenge 5. Auth0 sends code_verifier to /oauth/token → rejected with "code_verifier was provided but no code_challenge was registered" For client_id: Without forwarding, the second /authorize round-trip would have empty client_id, failing validateAuthorizeRequest with unauthorized_client. Fix: append code_challenge, code_challenge_method, and client_id to authState so they survive the login UI round-trip. Note: code_challenge is a hash (not a secret) — safe to include in URLs. The code_verifier (the secret) is never in URLs, only in /oauth/token.
RFC 7636 §4.2 states: "If no code_challenge_method is present, the server MUST use 'plain' as the default." The previous code defaulted to S256, which meant when Auth0 sends a plain-text code_challenge without specifying the method: 1. /authorize stores challenge as "challenge::S256" 2. /oauth/token SHA-256 hashes the code_verifier 3. Compares hash against the plain-text challenge → mismatch 4. "The code_verifier does not match the code_challenge" Fixed in both authorize.go (default when storing) and token.go (default when parsing legacy format without ::method separator).
OIDC Core §3.1.2.1 requires prompt=login to force re-authentication even when a valid session exists. The previous code intentionally kept the session alive for prompt=login to avoid a redirect loop, but this meant the user was never shown the login form — the React SDK detected the existing cookie, auto-redirected back to /authorize, which issued a code immediately without re-authentication. Fix: when forceReauth is true (prompt=login or max_age=0): 1. Delete the server-side session from memory store 2. Delete the browser session cookie 3. Clear sessionToken so the handler redirects to the login UI The prompt parameter is intentionally NOT forwarded in authState, so the second /authorize call (after re-authentication) won't see prompt=login and won't loop. Flow after fix: 1. /authorize?prompt=login → delete session+cookie → redirect to /app 2. React app has no session → shows login form 3. User authenticates → login mutation creates new session 4. React redirects to /authorize (without prompt=login) 5. /authorize finds fresh session → issues code → redirects to RP
…entication" This reverts commit 2f88665.
The form_post response mode renders an HTML page that auto-submits a form to the RP's redirect_uri. The CSP form-action directive was only including the origin (scheme://host), which some browsers reject when the form action includes a path. Fix: include both the origin and the path-level URI (without query string, since CSP source expressions don't support query parameters) in the form-action directive. This gives: form-action 'self' https://rp.example.com https://rp.example.com/callback Also reverts the prompt=login session deletion change — the previous flow of keeping the session for prompt=login was correct since the React SDK auto-detects the session. The real issue was this CSP block.
…format - Restore client_id check in validateAuthorizeRequest that was accidentally removed during the duplicate response_type cleanup - Update TestAuthorizeEndpointCompliance to check for RFC 6749 error codes (invalid_request) instead of freeform error strings - Update TestTokenEndpointCompliance PKCE test to use explicit ::S256 method separator (default is now plain per RFC 7636 §4.2) - Update TestClientIDMismatchMetric to expect 200 for dashboard paths (ClientCheckMiddleware only validates on /graphql route)
client_id validation is not needed in validateAuthorizeRequest — it is handled elsewhere. Update test to expect 302 redirect for invalid client_id at /authorize instead of 400.
Comprehensive guide covering: - Authorization code flow with PKCE (S256 and plain) - Nonce handling for backchannel (code) flow — required by Enterprise RPs - Response modes (query, fragment, form_post, web_message) with CSP details - Session management during token exchange (auth_code vs refresh_token) - Auth0, Okta, Keycloak-specific configuration and behavior - Prompt parameter handling (login, none, consent) - Troubleshooting common integration failures - RFC compliance table
…c session ops M1 — Atomic GetAndRemoveState (TOCTOU race on authorization code): Add GetAndRemoveState to Provider interface and all 3 implementations (in-memory, Redis via Lua EVAL, DB). Token endpoint now uses single atomic call instead of separate GetState+RemoveState, preventing authorization code replay under concurrent requests (RFC 6749 §4.1.2). M2 — Document PKCE requirement for "none" auth method: Add comment in openid_config.go explaining that "none" requires PKCE; token endpoint rejects "none" without code_verifier. M3 — Warning log when PKCE plain method is used: Log debug message when code_challenge_method defaults to or is explicitly set to "plain", noting that S256 is recommended since plain exposes the code_verifier in URL parameters. L1 — Replace unsafe-inline with nonce-based CSP in form_post: setFormPostCSP now generates a crypto/rand nonce, returns it, and sets script-src 'nonce-<value>' instead of 'unsafe-inline'. Template uses <script nonce="..."> instead of onload attribute. Also restores the form-action directive with origin+path (was lost in prior commit). L3 — Synchronous old session deletion in authorize.go: Replace async go-routine with synchronous call + error logging. Prevents silent failures during session rollover. L4 — Synchronous refresh token deletion in token.go: Same pattern — synchronous with error logging for token rotation. L5 — Addressed by L1: redirectURI param is now used for form-action CSP.
…, enumeration Critical: - Use crypto/subtle.ConstantTimeCompare for token validation (auth_token.go) - Validate redirect URI in OAuth callback against allowlist (oauth_callback.go) - Use atomic GetAndRemoveState in consumeAuthorizeState (oauth_authorize_state.go) High: - Remove refresh token from URL query params in OAuth callback - Require X-Authorizer-Client-ID header for /graphql route - Add configurable --app-cookie-same-site flag (lax/strict/none), default lax - Sanitize internal error messages in OAuth callback responses Medium: - Add dummy bcrypt on signup "user exists" path (timing equalization) - Return generic success in resend_otp for non-existent users - Fix unconditional "Failed" log in resend_otp - Check delete error in DB GetAndRemoveState for replay prevention - Fix login MFA: use isEmailLogin instead of isMobileLogin - Return generic error in verify_otp for user not found / revoked Low: - Replace Redis KEYS with SCAN in DeleteAllUserSessions - Sanitize logout error responses - Remove client_id from debug log - Reduce OpenID Discovery cache from 1h to 5m
…bute Add AppCookieSameSite config field with CLI flag --app-cookie-same-site (values: none, lax, strict; default: none). Default "none" preserves backward compatibility — previously SameSite=None was hardcoded when AppCookieSecure=true. Operators can now explicitly set "lax" or "strict" for same-origin deployments to improve CSRF protection.
cb896a5 to
928ac3c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive OIDC/OAuth2 spec compliance fixes for Enterprise IdP integration (Auth0, Okta, Keycloak):
Critical (breaks IdP integration):
/authorizevalidateAuthorizeRequestnow returns RFC 6749 error codes (invalid_request,unauthorized_client,unsupported_response_type) instead of freeform strings that Auth0/Okta/Keycloak cannot parseredirect_urivalidation, errors redirect to the RP via configuredresponse_modeinstead of returning JSON (RFC 6749 §4.1.2.1) — prevents users from getting stuck on the Authorizer pageApp.tsxscope default fixed from string to array (type mismatch brokeAuthorizerProvider)App.tsxredirectURLfallback fixed fromwindow.location.href(includes polluted query params) towindow.location.originHigh priority:
auth_timeclaim now populated in ID tokens viaAuthTime: claims.IssuedAton all 5 token creation paths (OIDC Core §2 — required whenmax_ageis used)"none"intoken_endpoint_auth_methods_supportedfor PKCE-only public clientstoken_typenormalized to"Bearer"(capitalized) in all redirect params for strict RP validationMedium priority:
Cache-Control: public, max-age=3600added to discovery endpointresponse_typevalidation removed fromvalidateAuthorizeRequestTest plan
go build ./...— clean compilationnpx tsc --noEmit— TypeScript cleango test ./internal/memory_store/...— memory store tests passprompt=nonereturnslogin_requirederror to RP redirect_urimax_ageparam triggersauth_timein returned id_token