feat(auth): restart login on auth_error callback bounces, loop-guarded (release-2026.07.1) - #241
Merged
cyberantonz merged 3 commits intoJul 30, 2026
Conversation
A failed OIDC callback used to dead-end the browser on the authenticator's problem+json — nothing was loaded at /auth/callback, so an expired login state (the 300 s Redis TTL, insight#2032) simply stopped the login. The authenticator now 302s such failures back to the SPA with a fixed auth_error=<reason> query parameter. Boot consumes the parameter before the session probe: a stale bounce on a live session is ignored, retryable reasons (state_expired, idp_error, invalid_callback, exchange_failed) restart the login once, and a sessionStorage attempt counter halts persistent failures on a new full-page error screen with a manual retry instead of looping browser -> IdP. access_denied never auto-retries — a silent SSO hop would just reproduce it. Safe to deploy ahead of the authenticator change: without the new parameter the boot path is unchanged. Part of insight#2032 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
- signIn() with no arg on auto-retry and the error screen's button: return to the current URL (already stripped of auth_error) instead of hardcoding "/", so a non-root default_return_to survives the bounce. - Strip an empty ?auth_error= without counting it against the retry budget (the authenticator always sends a reason; empty means a hand-crafted URL) so it can't ride into return_to and stick. - Reset the signIn redirect guard on bfcache restores (pageshow with persisted) — browser Back from the IdP used to leave the module's redirecting flag true, turning the "Try again" button into a silent no-op. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
- Do not auto-retry an attempt that could not be persisted (getItem works, setItem throws): the next bounce would read zero again and the loop guard would never trip. writeAttempts now reports success and autoRetry requires it. - Preserve the URL hash in signIn's default return_to (covers both the auto-retry and the error screen's Try-again path). - Semantic markup on the login-error screen: main landmark, h1 title, role=alert on the message so screen readers announce it on mount. 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
a4f4ea8
into
constructorfabric:release-2026.07.1
6 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
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 #239 to
release-2026.07.1— cherry-picks offf4e199,aafbc4c,384ab72(clean, no conflicts). FE half of constructorfabric/insight#2032.The SPA now consumes the authenticator's
?auth_error=<reason>callback-failure bounce: a live session ignores it, retryable reasons (state_expired,idp_error,invalid_callback,exchange_failed) restart the login once, and a sessionStorage attempt counter (fail-closed when storage is unavailable or writes fail) halts persistent failures on a full-page error screen with a manual "Try again" instead of looping browser → IdP.access_deniednever auto-retries. Also resets thesignInredirect guard on bfcache restores and preserves the URL hash in the defaultreturn_to.Deploy order
Safe ahead of the authenticator change (boot path unchanged without the parameter); the authenticator's redirect change must not ship first.
Test plan
On this branch (release base):
npm run test— 703 passed;tsc -bandeslint src/clean.Part of constructorfabric/insight#2032
🤖 Generated with Claude Code