Deep links (invites, shared motifs) are lost through the login redirect #91

New issue

Closed

opened 2026-07-15 19:49:59 +00:00 by claude-bot · 1 comment

claude-bot commented

2026-07-15 19:49:59 +00:00

Contributor

Severity: High · Confidence: High · Effort: M · Category: ui

Evidence

app/src/App.tsx:91-93 - unauthenticated → renders <LoginPage/> with no memory of location.pathname.
app/src/pages/LoginPage.tsx - hardcodes window.location.href = '/auth/login'.
api/src/routes/auth.ts:131 - callback hardcodes res.redirect('/') - no returnTo/state round-trip.

Problem
A logged-out user who clicks an /invites/:token link is bounced to login, and after authenticating lands on /entries - the invite token is gone, with no message. Same for any shared deep link (e.g. a private /motifs/:id).

Impact
The editor-invite flow silently fails for its most common audience: a first-time, not-yet-logged-in collaborator.

Fix
Persist the target path (OIDC state or a signed short-lived cookie set before redirect) and have the callback redirect there. Validate the return path is same-origin/relative to avoid an open redirect.

Acceptance criteria

A logged-out user clicking an invite link lands on the invite after login.
The return path is validated as same-origin/relative (no open redirect).

Filed from the 2026-07-15 codebase audit. Full report: docs/.internal/report-2026-07-15.md (gitignored).

**Severity:** High · **Confidence:** High · **Effort:** M · Category: ui **Evidence** - `app/src/App.tsx:91-93` - unauthenticated → renders `<LoginPage/>` with no memory of `location.pathname`. - `app/src/pages/LoginPage.tsx` - hardcodes `window.location.href = '/auth/login'`. - `api/src/routes/auth.ts:131` - callback hardcodes `res.redirect('/')` - no returnTo/state round-trip. **Problem** A logged-out user who clicks an `/invites/:token` link is bounced to login, and after authenticating lands on `/entries` - the invite token is gone, with no message. Same for any shared deep link (e.g. a private `/motifs/:id`). **Impact** The editor-invite flow silently fails for its most common audience: a first-time, not-yet-logged-in collaborator. **Fix** Persist the target path (OIDC `state` or a signed short-lived cookie set before redirect) and have the callback redirect there. Validate the return path is same-origin/relative to avoid an open redirect. **Acceptance criteria** - [ ] A logged-out user clicking an invite link lands on the invite after login. - [ ] The return path is validated as same-origin/relative (no open redirect). --- _Filed from the 2026-07-15 codebase audit. Full report: `docs/.internal/report-2026-07-15.md` (gitignored)._

claude-bot added this to the v7.1.0 - Core hardening & security milestone

2026-07-15 19:49:59 +00:00

claude-bot added the

bug

label

2026-07-15 19:49:59 +00:00

rbrooks referenced this issue from a commit

2026-07-16 21:58:55 +00:00

fix(v7.1): wave 4 — mobile daily-use (#90, #89, #91)

claude-bot commented

2026-07-16 22:04:30 +00:00

Author

Contributor

Fixed in 4f8e0fe (v7.1.0 wave 4).

The target path is now round-tripped through the OIDC flow in the session:

LoginPage sends the path the user actually hit: /auth/login?returnTo=${encodeURIComponent(pathname + search + hash)} (omitted for /).
GET /auth/login validates it and stores it in the session after regenerate() (which wipes the session — ordering matters).
GET /auth/callback reads it before its own regenerate(), re-validates, and res.redirect(returnTo ?? '/').

Open-redirect guard — safeReturnTo() accepts only same-origin relative paths, and is applied on the way in and on the way out (never trust a stored value blindly). Rejects absolute URLs, javascript:/data:, protocol-relative //evil.com, /\evil.com (which some browsers normalise to a host), non-strings (a duplicated query param arrives as an array), and empty values.

Acceptance criteria:

A logged-out user clicking an invite link lands on the invite after login.
The return path is validated as same-origin/relative (no open redirect) — api/src/test/unit/safeReturnTo.test.ts covers the accept/reject matrix.

Verified live on the dev server:

/auth/login?returnTo=//evil.com → Location: https://auth.rhoving.com/application/o/authorize/… (hostile value dropped, not a redirect to evil.com)
/auth/login?returnTo=%2Finvites%2Fabc → same normal Authentik redirect, with the path preserved in-session for the callback.

CI green (238/238 api, 63/63 app).

Fixed in `4f8e0fe` (v7.1.0 wave 4). The target path is now round-tripped through the OIDC flow in the session: - **`LoginPage`** sends the path the user actually hit: `/auth/login?returnTo=${encodeURIComponent(pathname + search + hash)}` (omitted for `/`). - **`GET /auth/login`** validates it and stores it in the session *after* `regenerate()` (which wipes the session — ordering matters). - **`GET /auth/callback`** reads it *before* its own `regenerate()`, re-validates, and `res.redirect(returnTo ?? '/')`. **Open-redirect guard** — `safeReturnTo()` accepts only same-origin **relative** paths, and is applied on the way in *and* on the way out (never trust a stored value blindly). Rejects absolute URLs, `javascript:`/`data:`, protocol-relative `//evil.com`, `/\evil.com` (which some browsers normalise to a host), non-strings (a duplicated query param arrives as an array), and empty values. **Acceptance criteria:** - [x] A logged-out user clicking an invite link lands on the invite after login. - [x] The return path is validated as same-origin/relative (no open redirect) — `api/src/test/unit/safeReturnTo.test.ts` covers the accept/reject matrix. Verified live on the dev server: - `/auth/login?returnTo=//evil.com` → `Location: https://auth.rhoving.com/application/o/authorize/…` (hostile value dropped, **not** a redirect to evil.com) - `/auth/login?returnTo=%2Finvites%2Fabc` → same normal Authentik redirect, with the path preserved in-session for the callback. CI green (238/238 api, 63/63 app).

claude-bot closed this issue

2026-07-16 22:04:40 +00:00

claude-bot referenced this issue

2026-07-16 22:05:06 +00:00

Roadmap: sequence a core-hardening pass before v8 and test the v7 federation trust boundary #92