fix(auth): add safe browser sign-in recovery

[seonghobae]

Summary

• replace raw browser/OIDC callback errors with a customer-readable recovery action
• preserve requested same-origin return paths while stripping one-time authorization response artifacts at every admission/storage boundary
• verify Authorization Code + PKCE S256, protected /api/me access, return-path restoration, and password-token absence
• keep the recovery surface horizontally contained at narrow mobile widths

Stack authority — 2026-09-22 KST

• direct parent: security(auth): migrate public frontend off direct grants #1120 exact 73ab7ea24517bfcb1edfc83676f9fb56ea524bc0
• exact head: c49aff397736db601fd1d2cb428c8006d0090d26
• state: open / Draft / mechanically mergeable
• fresh compare from current security(auth): migrate public frontend off direct grants #1120 has exact merge-base 73ab7ea..., behind_by=0; effective child delta remains the same 13 ADR/Storybook/frontend paths
• predecessor local/browser/frontend receipts do not transfer
• no open PR uses this branch as its base

Keep this child on #1120. Do not retarget it to main or transfer child evidence upward.

Review findings and causal repairs

The existing browser-auth repair lineage remains unchanged: correlated callback artifact stripping and return-path precedence, WHATWG same-origin admission, fallback sanitization, and the 390×844 login-card overflow repair are all preserved. The latest movement is ancestry-only: ordinary two-parent/non-force convergence c49aff39... adopts current #1120 after its parent stack moved, without altering any of the 13 child-owned paths.

Validation authority

Predecessor 5e734e3040... had local/component lint, 538 frontend tests, production build, Storybook, and inspected 1440×1000 / 390×844 recovery rendering. Those remain predecessor receipts only. Current c49aff39... must reacquire applicable repository/frontend/Storybook/browser/security/static-analysis evidence.

Localization is still not product-complete. Canonical PostgreSQL-authoritative eight-locale copy belongs to Draft #929. This auth PR does not manufacture ES/DE/FR authority; promotion requires the canonical ledger/ACL consumption path plus KO/EN/JA/ZH/VI/ES/DE/FR recovery-screen evidence, including CJK/text expansion/font fallback.

Keep Draft until one unchanged exact head has applicable repository/frontend/Storybook/browser/security/static-analysis/localization evidence, no unresolved valid review finding, and qualifying independent approval after the prerequisite stack is promotable.

No self-approval, Admin bypass, force push, destructive rebase, gate weakening, predecessor-receipt transfer, new no-op wake commit, protected-main merge, or release is claimed.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[seonghobae]

Current-head review found and repaired one auth-recovery defect. The retry path is built from window.location, but the callback scrubber covered only successful-code/OIDC metadata. A standards-shaped failed authorization response can include error, error_description, and error_uri; preserving those in the remembered return URL can replay callback failure state after a successful retry and retain provider detail in a customer-controlled path. RED d19da523bcf952d8fdb27cceff5ab38f458fdfd4 pins stripping those fields while preserving the product deep link. Causal fix 81f885d322d09253786eb48622deaf941743aa2d extends the existing single callback-parameter boundary; it does not change IdP behavior, PKCE, grant topology, or external-navigation admission. Exact-head hosted Tests are Draft-policy skipped, so this is source-repaired rather than accepted.

[seonghobae]

Current-head follow-up: 05ea958ccaa2f1662a12665841f2fe448645e8c3 preserves the callback-error scrub repair and makes ADR 0109 code-current by explicitly classifying success/error authorization response fields as one-time protocol artifacts that must not be re-minted into product return URLs. One promotion gap remains intentionally outside this child: current static frontend locale support is only en/ko/zh/ja/vi, while the canonical DB-versioned eight-locale ledger is owned by Draft #929 and is not a released dependency. Do not copy ES/DE/FR static authority into this auth PR to manufacture parity. Keep localization delivery FAIL/pending until the canonical ledger is released/consumable and this recovery screen has KO/EN/JA/ZH/VI/ES/DE/FR plus expansion/font-fallback evidence. Exact-head Tests are Draft-policy skipped; no acceptance claim.

[seonghobae]

Fresh exact-head review found a second return-path admission defect after the browser-location scrub repair: rememberOidcReturnUrl, stateReturnUrl, and storage fallback accepted already-persisted/state return paths after same-origin/length validation but without removing one-time OAuth/OIDC response fields. That allowed stale code/state/session_state/iss/error* values from an older client or state payload to survive into a later product return path. RED 3bf1c370... covers write-time storage, state.returnUrl, and legacy storage recovery. Causal fix 0a59cb3c... centralizes same-origin bounded sanitization and applies it at every admission/write boundary. ADR 0109 is aligned at b536b11d.... No self-approval; current-head hosted checks/review remain required.

[seonghobae]

Current-head review found a retry-state regression in the browser recovery path. A failed OIDC callback is rooted at redirect_uri; App rebuilt the retry target from that callback and then rewrote the dedicated return-path storage. When the pre-redirect path was /?post=…, a callback containing only error/state sanitized to /, so retry replaced the buyer's remembered deep link with root. RED ccdde2ae235ef4e291ce5dc75fa0b9dd5e54350c pins the remembered-path case. Causal fix 192c78da6eb9e015f74d2c2aa07b00382fbf592e makes callback-derived retry prefer the already validated session/local return path while ordinary product navigation still derives from current location. ADR 0109 is aligned at eddfed0d505b60a36050b501472ce0c28f1f94e9. This is a COMMENT review only, not approval; exact-head hosted acceptance is still required.

[seonghobae]

Follow-up current-head review caught one regression in the first deep-link-preservation fix: callback detection treated a lone state query as sufficient callback evidence. With stale return-path storage present, an ordinary product URL such as /?post=current&state=... could therefore be replaced by the stale stored path. RED 5c648e2e48e16928484b72a7400262108e98fb00 pins that storage must not override current navigation for lone state. Causal fix caa77b62d05d34b715b7f20b48206b85d953be50 keeps state in the scrub set but removes it from the callback-signal set; code, OAuth error*, session_state, or iss still activate remembered-path precedence. ADR 0109 is aligned at fa819f29a6019d9f90d33ee15cadda4238eaec54. COMMENT only; no approval and no hosted-GREEN claim.

[seonghobae]

Fresh current-head review found a return-path admission defect in the new centralized sanitizer. The lexical startsWith("/") && !startsWith("//") check runs before WHATWG parsing, but special-scheme parsing treats backslashes as authority separators. A candidate such as /\\evil.example/forged?post=attacker therefore passed the lexical gate, parsed to another origin, and was then silently host-stripped into a different local path (/forged?...) when only pathname/search/hash were reconstructed. RED ba951dde63eacbe51553f67d0a3292fd948e3fb5 pins both current-location and OIDC-state admission to reject that external-shaped value. Causal fix d310514cd2e62e1e97c6578644aec9bdb43fc0f7 validates the parsed origin against the sentinel product origin before callback-field scrubbing/reconstruction. ADR 0109 is code-current at 2e7bd6ca9a2bff66d06abf2b4bf0c8938c8ea15a. COMMENT only; no self-approval and no hosted-GREEN claim.

[seonghobae]

Fresh current-head review found one remaining storage-precedence ambiguity. The prior fix correctly rejected a lone state, but any response-looking query name (code, error*, session_state, or iss) still activated remembered-path precedence by itself. Because the SPA root accepts product query state, stale auth storage could therefore replace current navigation such as /?post=current&error=validation_failed#workspace even though no correlated OIDC state was present. RFC 6749 §§4.1.2/4.1.2.1 and OpenID Connect Core require the response to return state when the authorization request supplied it, which this client does. RED 80816667c2023a6a0c3f204d2563ec7d77573cbe pins lone error and lone code against stale-storage takeover. Causal fix 3873ef4d15900d14b9b716fe968876a747e570a7 requires state plus a response signal before remembered-path precedence, while still scrubbing all reserved callback fields from the local path. ADR 0109 is code-current at 395742312682011624cd9a03d2fd8a5486249f89. COMMENT only; no self-approval and no hosted-GREEN claim.

[seonghobae]

Fresh exact-head review found that the prior callback-correlation repair was still too broad. state + session_state, state + iss, state + error_description, or state + error_uri activated remembered-path precedence even though RFC 6749 Authorization Code success requires code and an error response requires error; those other fields are ancillary metadata and cannot establish an authorization outcome by themselves. With stale auth storage present, an ordinary product URL carrying those names could still be replaced by the stale path. RED f82b38498bca83254ce6bcab0ac4174fa52a0d18 pins all four ancillary-only shapes. Causal fix d0d2026c71ce39c595dab2267f4ecfede19d25d3 narrows precedence to state + code or state + error while preserving the full callback scrub set. ADR 0109 is code-current at exact head fca1b35669f1deac5a780058ca478a7866bafbe6. COMMENT only; no self-approval or hosted-GREEN claim.

[seonghobae]

Current-head review finding: the final restoreOidcReturnUrl no-state/no-storage branch returned raw window.location.pathname, bypassing the same bounded same-origin sanitizer used by every other return-path source. A legitimate same-origin document URL can carry a pathname beginning //; handing that value to onSigninCallback → history.replaceState makes it a protocol-relative target and can raise SecurityError, leaving callback cleanup broken. RED bc782925149274f81c4c4bdda46b75d0fa3ef86d pins that current-path case. Causal fix 42614d44b69a558716741b79de06918a08f4d790 sanitizes the final fallback and uses / when inadmissible; ADR 0109 is aligned at exact head a5281e85fb5765c9b0d1ccf3831233eb2627a9cb. This is a COMMENT review only, not self-approval. Exact-head hosted acceptance remains unclaimed because Draft Tests 35601788860 was skipped and no exact-head Security/SAST/CodeQL receipt exists.

[seonghobae]

Current-head stack review at 1f1d904c3122c348402b19700a9d578a97c738b0: #1120 advanced test-only to 0808b081e7ac500c0181ed395743bd46b4c8b5b0. This child now adopts that exact parent via ordinary two-parent, non-force convergence. Fresh compare has merge-base 0808b081..., behind_by=0, and exactly the same 13 child-owned ADR/Storybook/frontend paths; no child delta was dropped or duplicated. Pre-convergence local/browser evidence remains predecessor-only and must be reacquired on this exact head before promotion.

[seonghobae]

Current-head stack review at 6ed78b2e8623e35e88e1ab6f285b38976f7e9625: parent #1120 moved to 82723adb2d1fded5f3ba741ac957d3dd5bdd8891 for a valid JWKS-selector test-contract repair. This child adopted that exact parent through an ordinary two-parent non-force merge. Fresh compare has merge-base 82723adb..., behind_by=0, and the child-only diff is unchanged at the same 13 ADR/Storybook/frontend paths. Exact-head Tests 35665039225 is Draft-policy skipped; all earlier browser/frontend/Storybook receipts remain predecessor-only.