# Deep-Link Redirects Survive Login — Mechanism, Consumers, Open Gaps (MC #106384) # Deep-Link Redirects Survive Login — Mechanism, Consumers, and Open Gaps **MC:** #106384 | **Status:** LIVE on azdo/main (merge commit `480d95ca`, PR 272) | **Verified against:** `azdo/main` @ `50e4223a` (2026-07-28) | **Independent peer-verify verdict:** PARTIAL (angie-106384-peer) | **Last updated:** 2026-07-28 **One-line summary:** a user hitting a private URL while logged out is now returned to that exact URL after signing in — except on one landing (an already-open bug, #106423) — where before this fix the `redirect` parameter was written by middleware and read by nothing, so every login silently discarded it. --- ## 1. The problem this replaced Before this fix there were two independent defects, both required to explain the reported symptom (CEO clicks an emailed link like `/expenses/new?type=purchase`, has to log in, lands on `/dashboard` instead): 1. **Half 1** — `middleware.ts` built the login-redirect URL from the request **pathname only**. The query string was silently dropped before the user ever saw the login screen. 2. **Half 2** — even where a `redirect` parameter did survive, **nothing in the app read it**. The login success handler hardcoded `router.replace('/dashboard')`. A parameter that is written and never read is a control that renders and does nothing — the same class of defect as the dead filters/dead locale switcher this audit program was raised to find. Fixing only those two would still not have worked: a real Entra sign-in does not return to the URL the user was on at all (see §2). The shipped fix (PR 272, two witness rounds + one independent post-merge peer-verify) addresses five loss points end to end. This page documents the mechanism as it exists in the code today, not as originally scoped. --- ## 2. End-to-end mechanism ``` GET /expenses/new?type=purchase (unauthenticated) → middleware.ts:93 builds /login?redirect=%2Fexpenses%2Fnew%3Ftype%3Dpurchase (carries PATHNAME + SEARCH — this is the Half-1 fix) → user clicks "Sign in with Microsoft" → use-entra-auth.ts:214 signInWithMicrosoft() reads the CURRENT ?redirect= param and stashes it in sessionStorage — the LAST moment it still exists → instance.loginRedirect() hands control to Entra → https:///authorize?...&redirect_uri=https://app.bilko.cloud&response_mode=fragment (bare origin, no path, no query — see §2.1) → back to https://app.bilko.cloud/#code=... window.location.search is EMPTY here — the query param is gone for good → use-entra-auth.ts:170 handleEntraLogin() calls resolvePostLoginPath(...), which falls through the (now-empty) query param to the STASH → router.replace('/expenses/new?type=purchase') ← the deep link, recovered ``` ### 2.1 Why the query param cannot survive the Entra round trip MSAL is configured (`msal-config.ts`) with `redirectUri` defaulting to the bare page origin, `navigateToLoginRequestUrl: false`, and CIAM returns the auth code in the URL **fragment** (`response_mode=fragment`), not the query. This was measured live, credential-free, against production: the authorize request the app actually sends to Entra carries no path and no query, and the browser lands back on `https://app.bilko.cloud/#code=...` with `window.location.search === ''`. Any code that reads only `window.location.search` on the way back — which is exactly what the first version of this fix did — will always fall through to the dashboard fallback, regardless of how carefully the query param was carried on the way *in*. This was the fifth loss point found in witness round 1 and is why the stash (§3) exists at all. --- ## 3. The stash — surviving the round trip `apps/web/lib/safe-redirect.ts` defines the mechanism:
PropertyValue
Storage key`bilko_post_login_redirect` (`STASH_KEY`, safe-redirect.ts:92)
Storage mechanism`sessionStorage`, not `localStorage` — survives a full-page redirect in the same tab, dies with the tab, matches MSAL's own `cacheLocation` (also sessionStorage, chosen for XSS reasons)
Written`stashRedirectTarget()` — one call site, `use-entra-auth.ts:214`, inside `signInWithMicrosoft()`, immediately before `loginRedirect()`
TTL10 minutes (`STASH_TTL_MS`, safe-redirect.ts:96) — long enough for a sign-in, short enough that a stale value cannot hijack an unrelated later login in the same tab
Sanitizedon write (invalid target is never stored) AND on read (a stored value is not trusted more than a URL param just because the app put it there)
### 3.1 Why the read is deliberately non-consuming `readStashedRedirectTarget()` (safe-redirect.ts:129) does **not** delete the key when it reads it. This is deliberate and load-bearing, not an oversight: there are two consumers that both run on the way back from Entra (`handleEntraLogin` and the `auth-provider.tsx` safety net, §4), and **their execution order is not guaranteed**. If the first reader to run cleared the stash, the second would find nothing, fall through to `/dashboard`, and could overwrite the correct navigation the first reader just made. Both readers see the same answer instead. The value is retired by three other means: the next sign-in click (which unconditionally overwrites or clears it before any navigation happens), logout (§5), and the 10-minute TTL. --- ## 4. Two consumers of `resolvePostLoginPath` — one fixed, one still open `resolvePostLoginPath(params)` = `readRedirectTarget(params) ?? readStashedRedirectTarget() ?? '/dashboard'` (param-first, see §5). There are exactly two call sites in the app that ever consulted the stash-fallback version. A third call site (`/demo`) was later re-scoped to **stop** using the fallback — see §4.1.
ConsumerFile:lineScoped to genuine post-Entra return?
`handleEntraLogin``lib/msal/use-entra-auth.ts:170`YES — this function only runs after MSAL fires `LOGIN_SUCCESS`
Authenticated-landing safety net`lib/auth-provider.tsx:109`**NO — see §4.2, this is the open defect**
### 4.1 `/demo` — re-scoped, does NOT use the stash **Correction to the originating tickets:** the instant-demo page lives at `apps/web/app/(auth)/demo/page.tsx`, not under a `(dashboard)` route group as earlier ticket text said. The code is the source of truth here. `demo/page.tsx` was the **sixth loss point**: it hardcoded `router.replace('/dashboard')`, so the journey *deep link → `/login?redirect=...` → instant sign-in → `/demo?...&redirect=...`* still landed on `/dashboard` even once the Entra path was fixed, because the cross-domain hop to `/demo` forwards the whole query string and the page just wasn't reading it. The fix (demo/page.tsx:190) is **not** `resolvePostLoginPath(searchParams)` — it is `readRedirectTarget(searchParams) ?? DEFAULT_POST_LOGIN_PATH`, i.e. it reads **only its own query params, never the stash**. This distinction was found the hard way: a first attempt used the stash-fallback version and a witness proved live that an abandoned sign-in leaves a target in the stash, and a later `/demo?country=HR` visit in the **same tab**, inside the 10-minute TTL, silently followed the old target instead of going to the demo the user just asked for — a journey where that version of the branch was worse than main. `/demo` has no Entra redirect of its own to survive, so it never needs the stash; it already carries the param whenever one exists. ### 4.2 OPEN DEFECT — the safety net was never re-scoped (MC #106423) The independent post-merge peer-verify (angie-106384-peer, verdict PARTIAL) found that `/demo` was fixed on the surface where the stash-leak bug was *found*, but `auth-provider.tsx:90-114` — the "authenticated user landing on `/` or `/login`" safety net — is the **other** caller of `resolvePostLoginPath` and was **not** given the same treatment. It still falls back to the stash for **any** authenticated landing on `/` or `/login`, not only a genuine post-Entra return. Proven by rendering the real `AuthProvider` component directly (not by reading the code): with an already-authenticated client state, a plain navigation to `/` with no query param, and a stash pre-seeded from an earlier sign-in, `router.replace` fired with the **stale stashed target** instead of leaving the user on `/`. **Reachability, honestly scoped narrower than the original N1 finding:** because the stash is non-consuming by design (§3.1), even a fully **successful** sign-in leaves the target sitting in `sessionStorage` for up to 10 more minutes. There is no in-app link to bare `/` from inside the authenticated dashboard shell, so this requires the user to navigate to the site root directly (URL bar, bookmark, external link) within that window while a stash from their own earlier sign-in — completed or abandoned — is still live. Real and user-visible, but a specific navigation habit rather than the default customer-facing path `/demo` is. **Why no test caught this:** no test in the entire suite renders `AuthProvider` or drives `handleEntraLogin` — flagged as a residual (R6) in the very first witness round and never closed. The fix that scoped `/demo` correctly (§4.1) left this sibling caller untouched because nothing exercises it. **Status: OPEN, tracked as MC #106423.** The prescribed fix is the same one-line treatment `/demo` got: this effect should read only whether it is landing here as a genuine post-Entra return (empty search, code in the fragment) versus any other authenticated landing on `/` or `/login`, and only consult the stash for the former — plus a test that actually renders `AuthProvider`, so this class of gap stops being invisible to the suite. --- ## 5. Why the cleanup lives in `authStore.logout()`, not `signOutFromEntra` **This is the single most useful fact on this page for whoever touches this code next.** `use-entra-auth.ts` defines and exports a function called `signOutFromEntra` (line 250) which does the "correct" full teardown: it calls the real logout, clears the marker cookie, clears the locale choice, calls `clearStashedRedirectTarget()` (line 271), and finally calls MSAL's `logoutRedirect` to end the Entra session itself. Reading that function in isolation, it looks like the obvious place the stash gets cleaned up. **It is not called anywhere.** Grepping `lib`, `app`, `components` for `signOutFromEntra` at `azdo/main` today returns exactly: the interface member declaration, the function definition, and the object literal that exports it from the hook. Zero call sites. The two logout paths a user can actually reach in this app are: - `components/top-bar.tsx:203` — `await useAuthStore.getState().logout()`, the user-menu "Log out" button - `components/DemoBannerWrapper.tsx` — "exit demo" also routes through the real `logout()` Neither of those goes anywhere near `signOutFromEntra`. So while this fix was being built, the first version of the cleanup was written correctly and placed in `signOutFromEntra` — and reproduced, inside its own fix, the **exact defect class the fix exists to kill**: a control that reads correctly and has no consumer. The original bug was a `redirect` query param nobody read; the accidental recreation was a cleanup function nobody called. **The actual fix:** `clearStashedRedirectTarget()` was moved into `lib/stores/auth-store.ts:233`, inside `logout()` itself — the one function both real logout paths in the app call through. That is confirmed reachable from both buttons. The lesson generalises past this one bug: **for any fix of this shape, enumerate every caller of the mechanism, including teardown** — not just the one the bug report happened to describe. **What this means for `signOutFromEntra` today:** it is dead code on a cleanup path, tracked separately as **MC #106421**, still open. That ticket deliberately does *not* conclude "delete it" — it asks whether Bilko's real logout is *supposed* to also end the user's Microsoft/Entra session (the way `signOutFromEntra`'s `logoutRedirect` call would do) and currently does not, because nothing calls the function that would do it. If that's the case, the actual bug is that `signOutFromEntra` is **not wired up** — a possible security-relevant gap (Entra SSO session surviving a Bilko logout on a shared machine), not simply dead code to delete. That question is still open. --- ## 6. Param-first on the return leg — the decision, and its residual `resolvePostLoginPath` checks the URL query param **before** the stash. This was deliberately re-examined (not just carried over) after a reviewer asked whether a fresh param should ever lose to an old stash, and the answer is: it depends which leg you're on. - **The true post-Entra return** (bare origin, code in the fragment) has **no query at all** — `window.location.search` is empty (§2.1). The stash wins here by default; param-vs-stash does not even arise. - **The contested case** is `/login?redirect=X` reached with a live `bilko_auth` cookie but no in-tab MSAL session — e.g. a link opened in a new tab, or a repeat visit later the same day. Here `X` was written seconds ago, by `middleware.ts:93` or `auth-provider.tsx:126`, from the path the user **just requested**. The stash, by construction, is older — written up to 10 minutes earlier, at the start of a *previous* sign-in. **Param-first was kept** on the reasoning that preferring a 10-minute-old stash over a just-expressed navigation would generalise the exact bug found in the `/demo` case (§4.1) rather than fix anything. **Residual, stated plainly:** this reasoning holds only while *every* writer of the `redirect` query param is writing genuine, current user intent. Today there are exactly two writers — `middleware.ts:93` and `auth-provider.tsx:126` — and both were re-checked and confirmed to derive the value from the path the user was actually just bounced from. If a future change adds a third writer of that param for some other purpose, or repurposes one of the two existing writers, param-first silently becomes the wrong rule on this leg. Re-verify this list before trusting the rule again. --- ## 7. What is NOT verified live **The real browser round-trip through Entra — redirect\_uri, the fragment-mode code return, and sessionStorage surviving the cross-origin navigation to the CIAM host and back — has never been observed by us with a real, credentialed sign-in.** There are no Entra credentials in the agent environment this work was built and reviewed in. What *was* observed live, credential-free, and should not be conflated with the above: - The authorize request the app sends to Entra (redirect\_uri = bare origin, response\_mode = fragment) — captured from a real headless click on the "Sign in with Microsoft" button, no password entered. - The mechanism the whole fix rests on — a value written to `sessionStorage` on `app.bilko.cloud`, invisible while the browser is on the CIAM host (that *is* the trap this fix exists to survive), then intact again back on `app.bilko.cloud` with an empty `window.location.search` — reproduced against production with a real navigation to the CIAM host and back. - The `/demo` journey end to end, including the open-redirect guard surviving a real navigation decision, on a production build. What remains genuinely unobserved: a **successfully authenticated** user's landing page after a real password/consent flow. That half of the original defect is verified in code and by component-level tests, not by watching it happen. Do not present this mechanism as fully live-verified — it is proven live up to the boundary Entra credentials impose, and proven in code beyond that boundary. --- ## 8. Related open tickets --- ## 9. Source Verified against `azdo/main` @ `50e4223a` (2026-07-28), which includes the merge commit `480d95ca` (PR 272, MC #106384). - `apps/web/middleware.ts` - `apps/web/lib/safe-redirect.ts` - `apps/web/lib/msal/use-entra-auth.ts` - `apps/web/lib/msal/msal-config.ts` - `apps/web/lib/auth-provider.tsx` - `apps/web/lib/stores/auth-store.ts` - `apps/web/app/(auth)/demo/page.tsx` - `apps/web/components/top-bar.tsx` Evidence: `~/system/evidence/106384/fix-verdict.md`, `witness-verdict.md` (two rounds), `peer-verify-verdict.md` (independent, post-merge, verdict PARTIAL).