fix: spend the claim challenge once across the widget's history moves - #1278
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Contributor
🦋 Changeset detectedLatest commit: f83f8dd The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The changeset omits the required @zitadel/server entry, and ADR 022 still documents the old sentinel-retirement behavior.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
Fixes claim challenge double-spending caused by widget history traversal and console loader revalidation.
Changes:
- Retires terminal sentinels with
replaceState. - Prevents claim loader reloads during widget history moves.
- Adds regression tests and a components changeset.
| File | Description |
|---|---|
packages/components/src/orchestrator/zitadel-login.ts |
Updates terminal sentinel retirement. |
packages/components/src/orchestrator/zitadel-login.spec.ts |
Tests navigation without host popstate. |
apps/console/src/routes/claim/index.tsx |
Disables loader reloads during same-document navigation. |
apps/console/src/routes/claim/claim.spec.tsx |
Tests claim protection during history moves. |
.changeset/login-terminal-sentinel.md |
Documents the components fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…y moves The claim page completed its single-use challenge twice: once in the document hosting the sign-in widget, once in the document the widget navigated to, so the second answer was 409 and the page told the developer their project was already claimed (cli-journey-e2e claim.spec). The trace shows why. On the terminal step the widget retires its back-gesture sentinel (ADR 022) with `history.back()`. That popstate is a navigation to the console router, which reloads the claim route's loader; by then the handoff exchange has set the session cookie, so the loader finds a session, the page swaps the widget for CompleteClaim and spends the challenge — two milliseconds after `location.assign` requested the next document. The browser aborts the request on unload, the server has already processed it, and the next document spends again. Two independent guards: - `<zitadel-login>` retires the sentinel with `history.replaceState` when the terminal step is about to navigate, so the host sees no popstate. A step that stays on screen traverses as before. - The claim route sets `shouldReload: false`: the session is read once per document, and no navigation the router sees while the widget is up can hand the page to CompleteClaim. The intended way back is a full-document navigation. Each guard has a unit test that fails without it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BapEwS3cXLhZStpkF8xe28
peintnermax
force-pushed
the
fix/claim-page-double-spend
branch
from
September 22, 2026 13:26
d283a66 to
887ce4d
Compare
ADR 022 said an armed sentinel without a back action is always retired with history.back(); the implementation now retires it in place with replaceState on a terminal step that navigates away. Record the exception, why it exists, the resulting stack shape, and the host-side rule that keeps the claim page reading the session once per document. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BapEwS3cXLhZStpkF8xe28
peintnermax
changed the title
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BapEwS3cXLhZStpkF8xe28
This branch was successfully deployed
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
claim/completeanswered 409 and the page told the developer their project was "Already claimed". This is what failedcli-journey-e2eclaim.spec.tson feat(console): branding settings beside a live login preview #1247 (run 35725532364), and it is the same symptom feat: key cache #1212 saw; feat: key cache #1212'sglobalThisgate covers remounts within one document, and this one is two documents.<zitadel-login>retires its back-gesture sentinel (ADR 022) withhistory.back(). Thatpopstateis a navigation to the console router, which reloads the claim route's loader; by thensessions/exchangehas set the cookie, so the loader finds a session, the page swaps the widget forCompleteClaimand POSTsclaim/complete— 2 ms afterlocation.assignhad already requested the next document. The browser aborts the request on unload, the server has already completed the claim, and the next document spends again. It's a race against the next document's HTML response, which is why it shows up on a loaded CI runner (--concurrency 5) and not locally.packages/components— when the terminal step is about to navigate topost-sign-in-url(this.completing), the sentinel is retired in place withhistory.replaceState({ ..., zl: false })instead of a traversal, so the host never sees apopstate. Steps that stay on screen traverse exactly as before. The retired entry stays under the destination (a same-URL destination like the claim page replaces it), which is the same one-hop stale-sentinel tolerance the code already accepts.apps/console— the/claim/route setsshouldReload: false: the session is read once per document, and no navigation the router observes while the widget is up can hand the page toCompleteClaim. The intended way back is a full-document navigation, so the initial read is the only one that matters;router.invalidate()still reloads.Validation
moon run components:test—zitadel-login.spec.ts48/48; the new "retires the sentinel in place when the terminal step navigates away" case fails on the unpatched orchestrator (at thehistory.backcall count) and passes with the fix.moon run console:test—claim.spec.tsx18/18; the new "does not spend the challenge when the widget's history sentinel moves the router" case renders on a realcreateBrowserHistory, replays the widget'spushState({ zl: true })→history.back()with a session now available, and asserts the widget stays up, the loader ran once andclaim/completewas never called. WithoutshouldReload: falseit fails with the page on "Project claimed".tsc --noEmit -p apps/console/tsconfig.spec.jsonclean;oxlintclean on the four touched files.cli-journey-e2eclaim journey itself (needs the release snapshot); thefull-prjob on this PR covers it.Release notes / changeset
.changeset/login-terminal-sentinel.md—@zitadel/componentspatch:<zitadel-login>no longer fires a hostpopstatewhen retiring its sentinel on a navigating terminal step. The console change ships with the server binary and needs no separate entry.Notes
skipPasskeyUpsellIfVisibleinclaim.spec.tsstill races on a "Skip for now" button that fix(login): land claims on sign-up, show the claim window, drop default-flow passkeys #1169 removed from the default flow, so a claim failure surfaces as a bare 30 s timeout; asserting on the outcome heading and failing fast on "Already claimed" would have named this bug directly.post-sign-in-url(e.g./login→/), the in-place retirement leaves one extra history entry for the login URL under the destination; a back press lands on the login page signed in, which redirects. The previoushistory.back()popped it first. Judged acceptable given the alternative; happy to discuss.🤖 Generated with Claude Code