Skip to content

fix(auth): preserve the OIDC authorize continuation across every login path#112

Merged
windischb merged 1 commit into
developfrom
fix/oidc-login-continuation
Jul 11, 2026
Merged

fix(auth): preserve the OIDC authorize continuation across every login path#112
windischb merged 1 commit into
developfrom
fix/oidc-login-continuation

Conversation

@windischb

Copy link
Copy Markdown
Contributor

Problem

Starting a login from a client app (GET /connect/authorize) challenges the browser to /login?redirect=<authorize-url>. Several login paths authenticated the user without honoring that continuation, so the user landed on the Modgud dashboard while the client app never received its callback — silently breaking SSO for the affected methods.

Fixes

Login SPA

  • Extracted finishLogin + the same-origin guard into a shared useLoginRedirect composable; every session-establishing flow (password, TOTP, email-OTP, passkey, magic-link, external IdP) finishes through it.
  • External-IdP buttons now read the continuation from ?redirect= (previously the never-set ?returnUrl=, so the continuation was always lost) and apply the same-origin guard.
  • Magic-link, password-reset and self-registration thread the continuation through their e-mail round trips as a server-validated ?redirect= on the emailed URL.
  • ?redirect= is forwarded across every mid-flow detour (forgot-password, register, reset, verify-email, magic-login, back-to-login links).

Consent deny / dead-ends

  • Deny re-enters /connect/authorize with a subject-bound deny marker, so OpenIddict emits the RFC 6749 access_denied error to the client — honoring the client's response_mode (query/fragment/form_post) and RFC 9207 iss, symmetric with the approve re-entry. (This also removed a window.location.assign sink: the SPA now only navigates a same-origin URL.)
  • Expired/consumed tickets return a retryUrl; the subject-binding check runs before the consumed/expired branches so that URL only ever reaches the ticket's own subject.

Open-redirect hardening

  • Shared LoginRedirectGuard for all server-side continuations (external returnUrl, SAML RelayState, magic-link/reset/self-reg ReturnUrl). Rejects //, /\ and control characters — a browser strips TAB/CR/LF while resolving a redirect, collapsing /\t/evil.com to //evil.com (protocol-relative → external host). The frontend guard mirrors this and type-guards repeated (array) ?redirect= params. Also tightens a pre-existing SAML RelayState open redirect.

Review

This branch was reviewed by three independent no-prior-context agents (security, frontend correctness, backend correctness); their findings — the control-char smuggling, a javascript:/data: consent-deny sink, and an array-param crash — are all addressed here.

Tests

  • LoginRedirectGuardTests pins the accept/reject cases incl. control-char smuggling.
  • e2e/07-oidc-continuation.spec.ts covers deny-returns-to-client, magic-link and password-reset continuations end to end (production-image rig).
  • Unit 1283 green; targeted integration (Cimd/Dcr approve, magic-link, self-reg, OWASP/auth-enforcement/PKCE) green; smoke + login golden-path green.

Note: e2e/60-dcr fails at its admin-UI DCR toggle step (getByRole('checkbox', …) timeout) — a pre-existing selector issue unrelated to this change; it never reaches the consent portion.

🤖 Generated with Claude Code

…n path

Starting a login from a client app (GET /connect/authorize) challenges to
/login?redirect=<authorize-url>. Several paths authenticated the user without
honoring that continuation, stranding them on the IdP dashboard while the
client never received its callback.

Login SPA:
- Extract finishLogin + the same-origin guard into a shared useLoginRedirect
  composable; every session-establishing flow finishes through it.
- External-IdP buttons read the continuation from ?redirect= (was the
  never-set ?returnUrl=) and apply the same-origin guard.
- Magic-link, password-reset and self-registration thread the continuation
  through their e-mail round trips as a validated ?redirect= on the emailed URL.
- Forward ?redirect= across every mid-flow detour (forgot-password, register,
  reset, verify-email, magic-login, back-to-login links).

Consent deny/dead-ends:
- Deny re-enters /connect/authorize with a subject-bound deny marker so
  OpenIddict emits the RFC 6749 access_denied error to the client, honoring
  response_mode + RFC 9207 iss (symmetric with the approve re-entry).
- Expired/consumed tickets return a retryUrl; the subject-binding check runs
  before the consumed/expired branches so that URL only reaches its own subject.

Open-redirect hardening:
- Shared LoginRedirectGuard for all server-side continuations (external
  returnUrl, SAML RelayState, magic-link/reset/self-reg ReturnUrl); rejects
  //, /\ and control chars (a browser strips TAB/CR/LF, collapsing /\t/evil.com
  to //evil.com). Frontend guard mirrors it and type-guards array params.

Tests:
- LoginRedirectGuardTests pins the accept/reject cases incl. control-char smuggling.
- e2e 07-oidc-continuation covers deny-returns-to-client, magic-link and
  password-reset continuations end to end.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@windischb windischb force-pushed the fix/oidc-login-continuation branch from ebe0101 to 968d9cf Compare July 11, 2026 21:44
@windischb windischb merged commit d1a64aa into develop Jul 11, 2026
8 checks passed
@windischb windischb deleted the fix/oidc-login-continuation branch July 11, 2026 21:59
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