docs(260606-tv8): pre-dispatch plan for sign-in redirect fix
This commit is contained in:
Lucas Berger
+204
@@ -0,0 +1,204 @@
|
|||||||
|
---
|
||||||
|
phase: quick-260606-tv8
|
||||||
|
plan: 01
|
||||||
|
type: execute
|
||||||
|
wave: 1
|
||||||
|
depends_on: []
|
||||||
|
files_modified:
|
||||||
|
- apps/api/src/index.ts
|
||||||
|
- apps/api/tests/routes/login.test.ts
|
||||||
|
- apps/pwa/src/api/client.ts
|
||||||
|
- apps/pwa/src/lib/loginRedirect.ts
|
||||||
|
- apps/pwa/src/lib/loginRedirect.test.ts
|
||||||
|
- apps/pwa/src/components/CalendarShell.tsx
|
||||||
|
autonomous: true
|
||||||
|
requirements: [AUTH-ENTRY-01]
|
||||||
|
must_haves:
|
||||||
|
truths:
|
||||||
|
- "An unauthenticated top-level browser nav to /api/login completes the OIDC login and lands back on the app, logged in"
|
||||||
|
- "An unauthenticated PWA load redirects the browser to /api/login instead of showing a dead-end 'Sign-in required'"
|
||||||
|
- "A genuine /api/me failure does NOT cause an infinite login redirect loop (one-shot guard)"
|
||||||
|
artifacts:
|
||||||
|
- path: apps/api/src/index.ts
|
||||||
|
provides: "Guarded GET /api/login route that redirects to /"
|
||||||
|
contains: "/api/login"
|
||||||
|
- path: apps/pwa/src/lib/loginRedirect.ts
|
||||||
|
provides: "One-shot sessionStorage-guarded login-redirect helper"
|
||||||
|
- path: apps/api/tests/routes/login.test.ts
|
||||||
|
provides: "Backend test: /api/login redirects to /"
|
||||||
|
- path: apps/pwa/src/lib/loginRedirect.test.ts
|
||||||
|
provides: "Frontend test: unauth redirect + one-shot loop guard"
|
||||||
|
key_links:
|
||||||
|
- from: apps/pwa/src/components/CalendarShell.tsx
|
||||||
|
to: apps/pwa/src/lib/loginRedirect.ts
|
||||||
|
via: "useEffect on meQuery.isError calls maybeRedirectToLogin()"
|
||||||
|
pattern: "maybeRedirectToLogin"
|
||||||
|
- from: apps/pwa/src/components/CalendarShell.tsx
|
||||||
|
to: apps/pwa/src/lib/loginRedirect.ts
|
||||||
|
via: "meQuery.isSuccess clears the one-shot flag"
|
||||||
|
pattern: "clearLoginRedirect"
|
||||||
|
---
|
||||||
|
|
||||||
|
<objective>
|
||||||
|
Fix the missing sign-in redirect: unauthenticated visitors to the PWA currently hit a
|
||||||
|
dead-end "Sign-in required" message with no way to log in, because the SPA reaches the API
|
||||||
|
only via `fetch()`, and the OIDC guard's 302 to Authelia is CORS-blocked for XHR.
|
||||||
|
|
||||||
|
Two coordinated changes:
|
||||||
|
1. Backend: add a guarded `GET /api/login` route that redirects to `/`. A TOP-LEVEL browser
|
||||||
|
navigation (not fetch) to this guarded route triggers the full OIDC login flow and returns
|
||||||
|
to the app cleanly — no CORS problem (Authelia 302 is followed at the document level).
|
||||||
|
2. Frontend: when `/api/me` fails because the user is unauthenticated, perform a full-page
|
||||||
|
navigation to `/api/login` (one-shot, loop-guarded via sessionStorage) instead of the
|
||||||
|
dead-end message.
|
||||||
|
|
||||||
|
Purpose: Restore the auth entry path for the Phase 03 PWA (Gate 2 live-verification gap).
|
||||||
|
Output: Backend login route + tests; frontend redirect helper + wiring + tests.
|
||||||
|
</objective>
|
||||||
|
|
||||||
|
<execution_context>
|
||||||
|
@$HOME/.claude/get-shit-done/workflows/execute-plan.md
|
||||||
|
@$HOME/.claude/get-shit-done/templates/summary.md
|
||||||
|
</execution_context>
|
||||||
|
|
||||||
|
<context>
|
||||||
|
@.planning/STATE.md
|
||||||
|
@./CLAUDE.md
|
||||||
|
@apps/api/src/index.ts
|
||||||
|
@apps/api/src/auth/middleware.ts
|
||||||
|
@apps/pwa/src/api/client.ts
|
||||||
|
@apps/pwa/src/components/CalendarShell.tsx
|
||||||
|
@apps/api/tests/routes/me.test.ts
|
||||||
|
@apps/pwa/src/components/CalendarShell.test.tsx
|
||||||
|
</context>
|
||||||
|
|
||||||
|
<tasks>
|
||||||
|
|
||||||
|
<task type="auto" tdd="true">
|
||||||
|
<name>Task 1: Add guarded GET /api/login backend route + test</name>
|
||||||
|
<files>apps/api/src/index.ts, apps/api/tests/routes/login.test.ts</files>
|
||||||
|
<behavior>
|
||||||
|
- GET /api/login returns a redirect (302) to '/' when the request reaches the handler
|
||||||
|
(i.e. the OIDC guard has already let it through, or dev-bypass is active).
|
||||||
|
- The route is mounted AFTER the `app.use('/api/*', oidcAuthMiddleware())` guard so that an
|
||||||
|
unauthenticated top-level nav is intercepted by the guard first (guard 302 → Authelia →
|
||||||
|
/callback → middleware sets `continue` back to /api/login → handler 302 → /).
|
||||||
|
- Under dev-bypass (DEV_AUTH_BYPASS=true), the guard is not mounted, so /api/login still
|
||||||
|
reaches the handler and redirects to '/'.
|
||||||
|
</behavior>
|
||||||
|
<action>
|
||||||
|
In apps/api/src/index.ts, register `app.get('/api/login', (c) => c.redirect('/'))` in the
|
||||||
|
protected-routes block AFTER the `if (!devBypassActive) { app.use('/api/*', oidcAuthMiddleware()) }`
|
||||||
|
guard and alongside the existing `app.route('/api/me', ...)` etc. mounts (currently ~lines
|
||||||
|
48-51). Add a short comment explaining the flow: top-level nav → guard 302 → Authelia → /callback
|
||||||
|
→ `continue` cookie returns to /api/login (now authenticated) → redirect to /. Do NOT place it
|
||||||
|
before the guard, and do NOT use `app.route` (it is a single bare GET, not a router).
|
||||||
|
|
||||||
|
Create apps/api/tests/routes/login.test.ts mirroring apps/api/tests/routes/me.test.ts:
|
||||||
|
- Reuse the same hoisted DB mock and the oidcAuthMiddleware passthrough mock pattern from me.test.ts.
|
||||||
|
- Test under DEV_AUTH_BYPASS='true' (set process.env BEFORE importing app; vi.resetModules per test):
|
||||||
|
`await app.request('/api/login')` returns status 302 and the `location` header equals '/'.
|
||||||
|
- Test under no DEV_AUTH_BYPASS with the oidcAuthMiddleware passthrough mock: `/api/login`
|
||||||
|
reaches the handler and returns 302 → '/' (passthrough lets it through, mirroring me.test's
|
||||||
|
OIDC-path block).
|
||||||
|
Use `res.headers.get('location')` to assert the redirect target.
|
||||||
|
</action>
|
||||||
|
<verify>
|
||||||
|
<automated>cd apps/api && pnpm exec vitest run tests/routes/login.test.ts && pnpm exec tsc --noEmit</automated>
|
||||||
|
</verify>
|
||||||
|
<done>GET /api/login redirects to '/' under both bypass and OIDC-passthrough paths; login.test.ts passes; apps/api tsc clean.</done>
|
||||||
|
</task>
|
||||||
|
|
||||||
|
<task type="auto" tdd="true">
|
||||||
|
<name>Task 2: Add one-shot login-redirect helper + tests</name>
|
||||||
|
<files>apps/pwa/src/lib/loginRedirect.ts, apps/pwa/src/lib/loginRedirect.test.ts, apps/pwa/src/api/client.ts</files>
|
||||||
|
<behavior>
|
||||||
|
- maybeRedirectToLogin(): if the sessionStorage flag 'familysync.loginRedirectAttempted' is
|
||||||
|
NOT set, set it and assign window.location.href = '/api/login' (returns true = redirected).
|
||||||
|
If the flag IS already set, do nothing and return false (caller falls through to the
|
||||||
|
"Sign-in required" message — we already bounced through login and still can't authenticate).
|
||||||
|
- clearLoginRedirect(): removes the sessionStorage flag (called on a successful /api/me load).
|
||||||
|
- Helper must guard against `typeof window === 'undefined'` / missing sessionStorage so it is
|
||||||
|
safe under SSR/test environments.
|
||||||
|
</behavior>
|
||||||
|
<action>
|
||||||
|
Create apps/pwa/src/lib/loginRedirect.ts exporting:
|
||||||
|
- const LOGIN_REDIRECT_KEY = 'familysync.loginRedirectAttempted'
|
||||||
|
- maybeRedirectToLogin(): boolean — implements the one-shot guard above using
|
||||||
|
sessionStorage.getItem/setItem and `window.location.href = '/api/login'`. Guard with a
|
||||||
|
window/sessionStorage availability check; return false if unavailable.
|
||||||
|
- clearLoginRedirect(): void — sessionStorage.removeItem(LOGIN_REDIRECT_KEY) (guarded).
|
||||||
|
|
||||||
|
Update the stale/misleading comment block at apps/pwa/src/api/client.ts top (lines 1-10) and
|
||||||
|
the inline comment at the fetchMe failure path (~line 30): replace the false claim that "the
|
||||||
|
browser will follow the 302 redirect to Authelia automatically" — for XHR/fetch the cross-origin
|
||||||
|
302 to Authelia is CORS-blocked, so re-auth requires a TOP-LEVEL navigation to /api/login (see
|
||||||
|
loginRedirect.ts). Keep fetchMe itself a pure data fetch (still throws on non-ok); do NOT put the
|
||||||
|
redirect inside fetchMe.
|
||||||
|
|
||||||
|
Create apps/pwa/src/lib/loginRedirect.test.ts (vitest + jsdom):
|
||||||
|
- beforeEach: clear sessionStorage; stub window.location with a writable href (e.g.
|
||||||
|
Object.defineProperty(window, 'location', { value: { href: '' }, writable: true }) or
|
||||||
|
vi.stubGlobal as the existing tests do).
|
||||||
|
- maybeRedirectToLogin() sets href to '/api/login', sets the flag, returns true on first call.
|
||||||
|
- A second maybeRedirectToLogin() call does NOT change href again and returns false (one-shot).
|
||||||
|
- clearLoginRedirect() removes the flag; a subsequent maybeRedirectToLogin() redirects again.
|
||||||
|
</action>
|
||||||
|
<verify>
|
||||||
|
<automated>cd apps/pwa && pnpm exec vitest run src/lib/loginRedirect.test.ts && pnpm exec tsc --noEmit</automated>
|
||||||
|
</verify>
|
||||||
|
<done>loginRedirect helper enforces one-shot redirect; tests pass; client.ts comment corrected; apps/pwa tsc clean.</done>
|
||||||
|
</task>
|
||||||
|
|
||||||
|
<task type="auto">
|
||||||
|
<name>Task 3: Wire redirect into CalendarShell meQuery handling</name>
|
||||||
|
<files>apps/pwa/src/components/CalendarShell.tsx</files>
|
||||||
|
<action>
|
||||||
|
In apps/pwa/src/components/CalendarShell.tsx, import maybeRedirectToLogin and clearLoginRedirect
|
||||||
|
from '../lib/loginRedirect.js'.
|
||||||
|
|
||||||
|
Add two useEffects (place near the existing eventsService sync effect, ~line 171):
|
||||||
|
- On meQuery.isError: call maybeRedirectToLogin(). If it returns true (a redirect was triggered),
|
||||||
|
the page is navigating away — the "Sign-in required" branch will unmount; if it returns false
|
||||||
|
(already attempted), let the existing dead-end branch render. Depend on [meQuery.isError].
|
||||||
|
- On meQuery.isSuccess: call clearLoginRedirect() so a later session expiry can redirect again.
|
||||||
|
Depend on [meQuery.isSuccess].
|
||||||
|
|
||||||
|
Leave the existing `if (meQuery.isError) { return <Sign-in required /> }` block as the fall-through
|
||||||
|
for the already-attempted case (do not delete it). Do NOT call window.location directly in the
|
||||||
|
component — go through the helper so the one-shot guard is centralized and tested.
|
||||||
|
|
||||||
|
Verify the CalendarShell.test.tsx smoke test still passes (it mocks fetchMe success, so neither
|
||||||
|
redirect path fires); if the test environment lacks window.location/sessionStorage stubs and the
|
||||||
|
success path now calls clearLoginRedirect, ensure the helper's guards keep it a no-op (handled in
|
||||||
|
Task 2). If the existing test mocks meQuery.isError anywhere, confirm it still renders without an
|
||||||
|
unhandled navigation (the helper redirect is guarded and href assignment is inert under jsdom).
|
||||||
|
</action>
|
||||||
|
<verify>
|
||||||
|
<automated>cd apps/pwa && pnpm exec vitest run src/components/CalendarShell.test.tsx && pnpm exec tsc --noEmit</automated>
|
||||||
|
</verify>
|
||||||
|
<done>CalendarShell redirects to /api/login on unauthenticated meQuery error (one-shot) and clears the flag on success; CalendarShell smoke test + apps/pwa tsc clean.</done>
|
||||||
|
</task>
|
||||||
|
|
||||||
|
</tasks>
|
||||||
|
|
||||||
|
<verification>
|
||||||
|
- Backend: `cd apps/api && pnpm exec vitest run && pnpm exec tsc --noEmit` — login.test.ts green, no type errors.
|
||||||
|
- Frontend: `cd apps/pwa && pnpm exec vitest run && pnpm exec tsc --noEmit` — loginRedirect + CalendarShell tests green, no type errors.
|
||||||
|
- Grep confirms the previously-missing redirect now exists: `grep -rn "api/login" apps/pwa/src` returns the helper, and `grep -n "/api/login" apps/api/src/index.ts` returns the route.
|
||||||
|
</verification>
|
||||||
|
|
||||||
|
<success_criteria>
|
||||||
|
- `GET /api/login` is mounted behind the OIDC guard and redirects authenticated requests to '/'.
|
||||||
|
- Unauthenticated PWA load triggers a single full-page navigation to '/api/login' (no CORS-blocked XHR, no infinite loop).
|
||||||
|
- A genuine backend error after one redirect falls through to "Sign-in required" instead of looping.
|
||||||
|
- The stale comment in client.ts no longer claims fetch follows the Authelia 302 automatically.
|
||||||
|
- All unit tests pass; both apps type-check clean.
|
||||||
|
</success_criteria>
|
||||||
|
|
||||||
|
<output>
|
||||||
|
Create `.planning/quick/260606-tv8-fix-missing-sign-in-redirect-in-the-pwa-/260606-tv8-SUMMARY.md` when done.
|
||||||
|
|
||||||
|
OUT OF SCOPE (orchestrator handles after merge, in main tree): docker compose rebuild,
|
||||||
|
browser/playwright re-test, tunnel deploy. Do NOT run docker or playwright in any task.
|
||||||
|
</output>
|
||||||
Reference in New Issue
Block a user