fix(guest): move guest token from URL query to sessionStorage (#21) #28

Open
hermes wants to merge 1 commit from fix/guest-token-session-storage into master
Collaborator

Why

Issue #21: The guest token (the customer's only credential) was passed as ?token=... in the URL query string on the payment page. Query strings land in browser history, access logs, and can leak via Referer headers.

Changes

  • GuestCheckoutPage.vue: store token in sessionStorage before navigating to payment; token removed from query string
  • GuestOrderPage.vue: replace RouterLink with click handler that stores token in sessionStorage before navigating
  • GuestPaymentRedirect.vue: read token from sessionStorage with fallback to query.token for backward compatibility
  • Tests: updated assertions in GuestCheckoutPage.spec.ts and GuestPaymentRedirect.spec.ts

sessionStorage persists across page refreshes within the same tab. The magic-link /gast-order/:token remains the canonical re-entry point (accepted pattern).

Test plan

  • npx vitest run — 318/318 pass
  • npm run lint — clean

Closes #21

## Why Issue #21: The guest token (the customer's only credential) was passed as `?token=...` in the URL query string on the payment page. Query strings land in browser history, access logs, and can leak via Referer headers. ## Changes - **GuestCheckoutPage.vue**: store token in `sessionStorage` before navigating to payment; token removed from query string - **GuestOrderPage.vue**: replace `RouterLink` with click handler that stores token in `sessionStorage` before navigating - **GuestPaymentRedirect.vue**: read token from `sessionStorage` with fallback to `query.token` for backward compatibility - Tests: updated assertions in GuestCheckoutPage.spec.ts and GuestPaymentRedirect.spec.ts `sessionStorage` persists across page refreshes within the same tab. The magic-link `/gast-order/:token` remains the canonical re-entry point (accepted pattern). ## Test plan - [x] `npx vitest run` — 318/318 pass - [x] `npm run lint` — clean Closes #21
hermes added 1 commit 2026-07-18 11:42:13 +00:00
fix(guest): move guest token from URL query to sessionStorage
Some checks failed
CI / Lint, type check, unit tests, coverage (pull_request) Successful in 3m38s
CI / E2E browser tests (pull_request) Failing after 1m16s
120251867c
The guest token (the customer's only credential) was passed as
?token=... in the URL query string on the payment page. Query strings
land in browser history, nginx/reverse-proxy access logs, and can leak
via Referer headers.

The magic-link landing /gast-order/:token is inherently URL-based
(accepted Notion/Stripe pattern), but the payment page does not need to
expose the token in the URL.

Changes:
  - GuestCheckoutPage.vue: store token in sessionStorage before
    navigating to payment page; remove token from query string
  - GuestOrderPage.vue: replace RouterLink with click handler that
    stores token in sessionStorage before navigating to payment;
    add useRouter import
  - GuestPaymentRedirect.vue: read token from sessionStorage with
    fallback to query.token for backward compatibility with existing
    magic links
  - GuestCheckoutPage.spec.ts: assert token is NOT in query, IS in
    sessionStorage after navigation
  - GuestPaymentRedirect.spec.ts: set sessionStorage in mount helper;
    clear sessionStorage in beforeEach

sessionStorage persists across page refreshes within the same tab, so
the payment page still survives refresh. If the user opens the payment
URL in a new tab (from history), sessionStorage is lost — but the
canonical re-entry point is the magic link /gast-order/:token, from
which they can navigate to payment again.

Closes #21
hermes reviewed 2026-07-18 11:43:34 +00:00
hermes left a comment
Author
Collaborator

Verdict: Solid security improvement, correct implementation. Minor suggestions below.

Critical

None.

Warnings

None.

Suggestions

  1. Clean up token after consumption (GuestPaymentRedirect.vue:82) - After payGuestOrder(token) succeeds, sessionStorage.removeItem('guestToken') before navigating away. While sessionStorage is tab-scoped and cleared on tab close, removing the credential immediately after use reduces the attack window if the user lingers on subsequent pages in the same tab.

  2. No test for the backward-compatibility fallback (GuestPaymentRedirect.spec.ts) - The route.query.token fallback path is untested. All existing tests either pass a valid token (stored in sessionStorage) or pass an empty string (both sessionStorage AND query empty). Consider adding a test where sessionStorage is empty but route.query.token is present, to verify the fallback actually works for users with old bookmarked payment URLs.

  3. <a href="#"> semantics (GuestOrderPage.vue:94) - The click handler replaces <RouterLink> with <a href="#" @click.prevent>, which gives screen readers a link to nowhere (#). A <button> styled as a link (or <RouterLink> with a beforeRouteLeave equivalent) would be more semantically correct. The existing .guest-order__hint a CSS selector still applies, so visual parity is maintained either way.

Looks Good

  • Security rationale is sound - moving the guest token out of the URL query string prevents leakage via browser history, server access logs, and Referer headers. This is the right fix for issue #21.
  • Backward compatibility fallback (GuestPaymentRedirect.vue:15-16) - the sessionStorage.getItem('guestToken') || route.query.token chain gracefully handles users who may have bookmarked the old payment URL format. Good transitional measure.
  • Tests properly updated - assertions verify token is absent from router.currentRoute.value.query.token and present in sessionStorage. sessionStorage.clear() in beforeEach prevents cross-test pollution.
  • Both write-sites covered - token is stored in sessionStorage in both GuestCheckoutPage.vue (new order flow) and GuestOrderPage.vue (magic-link re-entry flow). No path leaves the payment page without setting the token.
  • Comments are accurate and well-written in all three changed source files.
**Verdict: Solid security improvement, correct implementation. Minor suggestions below.** ### Critical None. ### Warnings None. ### Suggestions 1. **Clean up token after consumption** (`GuestPaymentRedirect.vue:82`) - After `payGuestOrder(token)` succeeds, `sessionStorage.removeItem('guestToken')` before navigating away. While sessionStorage is tab-scoped and cleared on tab close, removing the credential immediately after use reduces the attack window if the user lingers on subsequent pages in the same tab. 2. **No test for the backward-compatibility fallback** (`GuestPaymentRedirect.spec.ts`) - The `route.query.token` fallback path is untested. All existing tests either pass a valid token (stored in sessionStorage) or pass an empty string (both sessionStorage AND query empty). Consider adding a test where `sessionStorage` is empty but `route.query.token` is present, to verify the fallback actually works for users with old bookmarked payment URLs. 3. **`<a href="#">` semantics** (`GuestOrderPage.vue:94`) - The click handler replaces `<RouterLink>` with `<a href="#" @click.prevent>`, which gives screen readers a link to nowhere (`#`). A `<button>` styled as a link (or `<RouterLink>` with a `beforeRouteLeave` equivalent) would be more semantically correct. The existing `.guest-order__hint a` CSS selector still applies, so visual parity is maintained either way. ### Looks Good - **Security rationale is sound** - moving the guest token out of the URL query string prevents leakage via browser history, server access logs, and `Referer` headers. This is the right fix for issue #21. - **Backward compatibility fallback** (`GuestPaymentRedirect.vue:15-16`) - the `sessionStorage.getItem('guestToken') || route.query.token` chain gracefully handles users who may have bookmarked the old payment URL format. Good transitional measure. - **Tests properly updated** - assertions verify token is absent from `router.currentRoute.value.query.token` and present in `sessionStorage`. `sessionStorage.clear()` in `beforeEach` prevents cross-test pollution. - **Both write-sites covered** - token is stored in sessionStorage in both `GuestCheckoutPage.vue` (new order flow) and `GuestOrderPage.vue` (magic-link re-entry flow). No path leaves the payment page without setting the token. - **Comments are accurate and well-written** in all three changed source files.
Author
Collaborator

💡 Suggestion: The backward-compat fallback (sessionStorage.getItem('guestToken') || route.query.token) is untested. Consider adding a test case where mountPage is called with the token in the route query but sessionStorage cleared, to verify the fallback works for old bookmarked URLs.

💡 **Suggestion**: The backward-compat fallback (`sessionStorage.getItem('guestToken') || route.query.token`) is untested. Consider adding a test case where `mountPage` is called with the token in the route query but sessionStorage cleared, to verify the fallback works for old bookmarked URLs.
Author
Collaborator

💡 Suggestion: <a href="#" @click.prevent> degrades screen-reader semantics (a link to # / nowhere). A <button> styled to match, or keeping <RouterLink> with a custom navigation guard, would preserve accessibility without changing the visual result.

💡 **Suggestion**: `<a href="#" @click.prevent>` degrades screen-reader semantics (a link to `#` / nowhere). A `<button>` styled to match, or keeping `<RouterLink>` with a custom navigation guard, would preserve accessibility without changing the visual result.
Author
Collaborator

💡 Suggestion: Consider sessionStorage.removeItem('guestToken') right after payGuestOrder(token) resolves, before the router.push. The token has been consumed at this point and lingering in sessionStorage only extends the credential's lifetime unnecessarily.

💡 **Suggestion**: Consider `sessionStorage.removeItem('guestToken')` right after `payGuestOrder(token)` resolves, before the `router.push`. The token has been consumed at this point and lingering in sessionStorage only extends the credential's lifetime unnecessarily.
Some checks failed
CI / Lint, type check, unit tests, coverage (pull_request) Successful in 3m38s
CI / E2E browser tests (pull_request) Failing after 1m16s
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/guest-token-session-storage:fix/guest-token-session-storage
git checkout fix/guest-token-session-storage

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout master
git merge --no-ff fix/guest-token-session-storage
git checkout fix/guest-token-session-storage
git rebase master
git checkout master
git merge --ff-only fix/guest-token-session-storage
git checkout fix/guest-token-session-storage
git rebase master
git checkout master
git merge --no-ff fix/guest-token-session-storage
git checkout master
git merge --squash fix/guest-token-session-storage
git checkout master
git merge --ff-only fix/guest-token-session-storage
git checkout master
git merge fix/guest-token-session-storage
git push origin master
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: jocke/bilhej#28
No description provided.