Skip to content

feat: forward the complete mail-link query to the confirm API call#21

Merged
TaprootFreak merged 2 commits into
developfrom
feat/confirm-forward-full-query
Jul 12, 2026
Merged

feat: forward the complete mail-link query to the confirm API call#21
TaprootFreak merged 2 commits into
developfrom
feat/confirm-forward-full-query

Conversation

@TaprootFreak

Copy link
Copy Markdown
Contributor

What

Forward the complete mail-link query to the API confirm call so no param is dropped before the API can audit it.

buildConfirmUrl previously hardcoded ?email=&code=&user=. The confirm flow now sends every param present in window.location.search:

  • The three known params keep their exact handling — email lowercased, code / user untouched.
  • Every other param (e.g. a wallet address / connection id, or lang) is appended verbatim, URL-encoded.
  • Only the web's own control knobs — api (local-preview API-base override) and mock (preview hook) — are stripped, as they are not part of the Aktionariat mail-link data.

hasRequiredParams still gates the flow on email / code / user.

Why

This is the counterpart to the API's raw-request logging: without it, any extra param the Aktionariat link carries never reaches the API and can't be audited.

Not changed

  • No visual change and no CSP change — same API host, no new connect-src.
  • Additive behaviour: the existing three-param path is preserved exactly.

Verification (on the build host)

  • format:check, validate:html, check:site: clean.
  • Vitest unit coverage: 31 passed, 100% line/branch/function/statement on public/js/lib/** (incl. the new buildConfirmUrl branches).
  • Playwright functional suite (site.spec.mjs + behavior.spec.mjs): 31 passed, 0 failed. Adds one confirm-flow test asserting an extra param reaches the API while the web-only api knob is not forwarded.
  • No screenshot baseline change (logic-only, page render unchanged).

The confirm page built the API URL from only email/code/user, dropping any
other param the Aktionariat mail link carries before it could reach the API's
audit log. Forward the full incoming query instead: keep the existing handling
of the three known params (email lowercased, code/user untouched) and append
every other param verbatim, stripping only the web's own control knobs (api,
mock). hasRequiredParams still gates on email/code/user.

Counterpart to the API raw-request logging; no visual or CSP change (same API
host, no new connect-src).
…ation gate

hasRequiredParams gates the confirm request on the first-occurrence
email/code/user (URLSearchParams.get), but the forwarded query was built from a
forEach map that keeps the LAST value of a duplicated key. A crafted link
repeating a modelled key (e.g. ?email=a&...&email=b) therefore validated one
value but forwarded the other.

Pin the three modelled params in the forwarded object back to the already
validated first-occurrence locals. Every other, unmodelled param keeps being
forwarded verbatim, and the web-only api/mock knobs are still stripped; the
single-valued link is byte-identical. Adds a Playwright case proving the
duplicate-key link forwards the first occurrence, not the last.
@TaprootFreak

Copy link
Copy Markdown
Contributor Author

Reviews complete — conformance + logic, adversarial (each finding independently verified):

  • Round 1: 1 minor finding — a first-wins→last-wins divergence for duplicated query keys (validation gated on params.get = first occurrence, but the forwarded URL was built from params.forEach = last occurrence).
  • Fix in 638ff62: after building allParams, pin the three modelled keys back to the validated first-occurrence locals (allParams.email/code/user = email/code/user); all other params still forwarded verbatim; api/mock still stripped; email lowercased exactly once in buildConfirmUrl. Added a page-level test for the duplicate-key scenario.
  • Round 2: 0 findings — fix resolves round 1, introduces nothing new; single-valued 3-param link byte-identical to before.

CI green: Quality (incl. coverage gate, 100% on public/js/lib/**) + Screenshots (no baseline change). Marking ready.

@TaprootFreak TaprootFreak marked this pull request as ready for review July 12, 2026 15:45
@TaprootFreak TaprootFreak merged commit 36ae0cf into develop Jul 12, 2026
2 checks passed
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