Skip to content

fix(sso): keep the OIDC authorize request when Kerberos SSO fails - #73

Open
alghanim wants to merge 1 commit into
bodaay:masterfrom
alghanim:fix/sso-failure-keeps-oidc-request
Open

alghanim wants to merge 1 commit into
bodaay:masterfrom
alghanim:fix/sso-failure-keeps-oidc-request

Conversation

@alghanim

@alghanim alghanim commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator

When Kerberos SSO fails during an OIDC login, the browser ends up at the client's redirect_uri with ?error=<message> and no state. Every OIDC client treats a callback without its state as a CSRF attempt, so the user sees the client's state/CSRF error instead of the real reason, and never gets the chance to fall back to their AD password. A failed password attempt behaves correctly: it returns to SimpleAuth's login page with the whole request (H13).

This is the follow-up named in #72. The two PRs touch different functions and merge cleanly in either order.

Two hops dropped the authorize request

oidcAuthzRequest documents the Kerberos SSO link as a hop that must carry the request. The way back out of /login/sso did not:

  1. The Negotiate fallback. A browser that can't answer the 401 challenge (a machine not on the domain, or a browser not allowed to negotiate with the host) follows the page's meta-refresh. That retry URL kept only redirect_uri. client_id, oidc, state, nonce and the PKCE challenge were lost, so the retry resolved the default app:
    • an app whose redirect_uri is only in its own allowlist got a dead-end 400 redirect_uri not allowed;
    • an OIDC attempt quietly continued as a non-OIDC one.
  2. The failure redirect. All 15 failure exits in handleSSOLogin called redirectToLoginError, which bounces to the client's redirect_uri?error=… without state.

Change

  • The retry URL is built from the same typed oidcAuthzRequest allowlist, plus oidc and sso_attempt. Every value is percent-encoded by url.Values.Encode, so the meta-refresh still contains no raw request text, and the existing html.EscapeString stays as defence in depth.
  • A new ssoLoginError routes failures. An OIDC attempt (oidc=1) goes through renderOIDCLoginError, exactly like a failed password: back to SimpleAuth's authorize page with the error banner and the whole request. The __sso_attempted cookie and the error banner already stop auto-SSO from looping. Non-OIDC (hosted) flows keep redirectToLoginError and its ?error= redirect, now for the right app.

Verification

Unit tests (sso_failure_test.go). All four fail on master and pass here:

  • the Negotiate retry keeps every authorize parameter, plus oidc and sso_attempt=1;
  • an OIDC failure returns to the authorize page with state, nonce and PKCE, never to the client's callback;
  • the Kerberos-not-configured failure does the same;
  • the hosted flow still redirects an app-allowlisted redirect_uri with ?error=. Before, it got 400 from the default app.

The existing OIDC/PKCE/H13 tests are unchanged and pass. The full go test ./... is green, and go vet and gofmt are clean.

End to end: a Samba AD DC behind nginx with TLS, browser flows driven with one cookie jar:

flow before after
browser without Kerberos clicks SSO app callback ?error=SSO authentication failed…, no state SimpleAuth login page showing the reason → AD password → callback with code + state
Kerberos OK, user not assigned to the app app callback ?error=Access denied…, no state login page shows "Access denied: not assigned to this app"
Kerberos OK, assigned callback with code + state unchanged

Assignment is still enforced where it was: a code issued after the password fallback to an unassigned user is refused at the token endpoint (issueOIDCTokens → resolveTokenRoles → 403 access_denied).

🤖 Generated with Claude Code

Two hops of the /login/sso flow dropped the authorize request that
oidcAuthzRequest exists to carry:

- The Negotiate fallback. A browser that cannot answer the 401
  challenge follows the page's meta-refresh, whose URL kept only
  redirect_uri. client_id, oidc, state, nonce and the PKCE challenge
  were lost, so the retry resolved the DEFAULT app: an app whose
  redirect_uri is only in its own allowlist got 400 "redirect_uri not
  allowed", and an OIDC attempt continued as a non-OIDC one.

- The failure redirect. Every /login/sso failure called
  redirectToLoginError, which sends the browser to the client's
  redirect_uri with only ?error=<message>. That callback has no state,
  so OIDC clients reject it as a CSRF/state mismatch and the user
  never sees the real reason or gets a chance to use their password.

The retry URL is now built from the same typed oidcAuthzRequest
allowlist (plus oidc and sso_attempt), every value percent-encoded --
still no raw request text in the meta-refresh. Failures in an OIDC
attempt go through renderOIDCLoginError, exactly like a failed
password: back to SimpleAuth's authorize page with the error banner
and the whole request, so the user can fall back to their password and
the client still receives state, nonce and the PKCE challenge.
Non-OIDC (hosted) flows keep redirecting to the app with ?error=, now
for the right app.

Tests fail on master and pass here: the retry keeps every authorize
parameter, an OIDC failure returns to the authorize page with state
and PKCE, a Kerberos-not-configured failure does the same, and the
hosted flow still redirects an app-allowlisted redirect_uri with
?error=. Verified against a Samba AD DC behind nginx: a browser without
Kerberos lands on the login page with the reason shown, and the AD
password then completes the flow with code and state.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant