fix(authenticator): redirect failed /auth/callback into the SPA with auth_error (release-2026.07.1) - #2042
Merged
cyberantonz merged 1 commit intoJul 30, 2026
Conversation
…auth_error (constructorfabric#2040) * fix(authenticator): redirect failed /auth/callback into the SPA with auth_error A failed OIDC callback answered problem+json straight to the browser — no page is loaded at that point, so an expired login state (the 300 s Redis TTL, constructorfabric#2032), an IdP-reported error, a replayed callback, or a denied person dead-ended the login on raw JSON. Every browser-facing callback failure now 302s to default_return_to with a fixed auth_error=<reason> query parameter (state_expired, idp_error, invalid_callback, exchange_failed, access_denied) so the SPA can restart the login from scratch. The reason vocabulary is fixed — nothing IdP- or caller-supplied reaches the Location header. Rate-limit (429) and internal (5xx) responses stay problem+json. The SPA counterpart (consume auth_error, auto-retry once behind a loop guard, error screen for access_denied/repeated failures) must be deployed first: without it a persistent failure such as an unknown person would bounce between the SPA's auto-login and the IdP. Closes constructorfabric#2032 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech> * fix(authenticator): review follow-ups for the auth_error redirect - The view-as override deny (unknown target) is a browser-facing callback failure too — bounce it as access_denied instead of 403 problem+json (still a denial, never a fallback to the caller). - PersonError became unused with that — drop the type. - Log the IdP's error_description alongside error (sanitized): the only place the failure cause survives now that the browser gets a redirect. - Validate default_return_to (site-relative, no fragment, no control chars): it lands verbatim in Location headers, and a fragment would hide auth_error= from the SPA's loop guard. - Make the new e2e states unique per run so suite re-runs within the per-state rate-limit window don't flake. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech> * test(authenticator): unknown override target now bounces as access_denied The e2e pinned the old 403 problem+json; the denial is a 302 auth_error=access_denied redirect since the constructorfabric#2032 change (still no session minted, never a fallback to the caller). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech> --------- Signed-off-by: Anton Zelenov <antonz@constructor.tech> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cyberantonz
merged commit Jul 30, 2026
59cd8eb
into
constructorfabric:release-2026.07.1
2 checks passed
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
1 participant
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
Backport of #2040 to
release-2026.07.1— clean cherry-pick of the squash commit70650df9. Fixes #2032 on the release line.Every browser-facing
/auth/callbackfailure now 302s todefault_return_towith a fixedauth_error=<reason>(state_expired,idp_error,invalid_callback,exchange_failed,access_denied) instead of dead-ending the browser on problem+json. Rate-limit (429) and internal (5xx) responses stay problem+json; state consumption, session-fixation guard, and audit emissions unchanged. Includes the PRD/DESIGN/openapi updates.Deploy order
Merge/deploy the FE backport insight-front#241 first — without the SPA loop guard a persistent failure (e.g. unknown person) would bounce between the SPA's auto-login and the IdP.
Test plan
On this branch (release base):
run-e2e.shfull suite green (12 tests + endpoint-coverage gate);cargo test -p authenticator52 unit tests; clippy clean.🤖 Generated with Claude Code