docs(auth): record fase-8 logout + migration 014 hardening
- CLAUDE.md status line + logout paragraph + migration 013 gotcha updated to cover the INSERT+UPDATE two-step GoTrue behaviour and the RP-initiated logout flow; new SvelteKit gotcha block spelling out the signOut→$effect race that produced "PKCE code verifier not found in storage" and how the isLoggingOut flag + /logged-out public route close it. - docs/history/fase-8-auth-logout.md captures both prod bugs with symptom, root cause, fix, the Keycloak id_token_hint tradeoff (why the one-click confirmation page is accepted UX), and the test deltas. - docs/development.md auth.ts one-liner updated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -4,7 +4,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
|
||||
|
||||
## Project Status
|
||||
|
||||
**Fase 0–5 complete. Fase 6 complete (deploy readiness: PWA injectManifest SW + manifest + iOS meta tags + placeholder icons + Kong rate-limit on invitations + dev JWT rotation + prod env template + rls-audit.sh + Justfile lighthouse/rls-audit/test-rate-limit). Production stack deployed to ambrosio (OVH VPS) at https://erosi.oier.ovh (app + Supabase) + https://auth.oier.ovh (Keycloak); Let's Encrypt certs via host Caddy; PWA strategy switched to generateSW to unblock the prod build (see the "injectManifest hardcoded filename" gotcha below). Fase 7 complete: 12 new Playwright tests fill the collective-flow UI gap (O-01..O-03 onboarding, I-01..I-05 invitation acceptance, MC-02..MC-05 admin-manage) — writing them surfaced + fixed three latent bugs in the product code (atomic `create_collective()` RPC, pendingInvitationToken restore after login, manage page not reloading on late $currentCollective hydration). Plus migration 013 (auth.users role/aud default trigger) — found after the first real self-registration on prod via Keycloak→GoTrue left the user with empty `role`/`aud` columns, breaking every REST call with `role "" does not exist`. 255 tests green: 41 pgTAP + 140 Vitest integration + 15 Vitest unit + 58 Playwright + 1 Vitest rate-limit (gated, run via `just test-rate-limit`). 3 skipped in `just test-all` (2 Realtime presence — upstream `handle_out/3` bug; 1 rate-limit — gated). Push notifications + CI Docker + final production icons deliberately out of scope. ✅**
|
||||
**Fase 0–5 complete. Fase 6 complete (deploy readiness: PWA injectManifest SW + manifest + iOS meta tags + placeholder icons + Kong rate-limit on invitations + dev JWT rotation + prod env template + rls-audit.sh + Justfile lighthouse/rls-audit/test-rate-limit). Production stack deployed to ambrosio (OVH VPS) at https://erosi.oier.ovh (app + Supabase) + https://auth.oier.ovh (Keycloak); Let's Encrypt certs via host Caddy; PWA strategy switched to generateSW to unblock the prod build (see the "injectManifest hardcoded filename" gotcha below). Fase 7 complete: 12 new Playwright tests fill the collective-flow UI gap (O-01..O-03 onboarding, I-01..I-05 invitation acceptance, MC-02..MC-05 admin-manage) — writing them surfaced + fixed three latent bugs in the product code (atomic `create_collective()` RPC, pendingInvitationToken restore after login, manage page not reloading on late $currentCollective hydration). Plus migration 013 (auth.users role/aud default trigger) — found after the first real self-registration on prod via Keycloak→GoTrue left the user with empty `role`/`aud` columns, breaking every REST call with `role "" does not exist`. Post-fase-7 prod auth hardening (2026-04-20..21, see `docs/history/fase-8-auth-logout.md`): migration 014 extends the role/aud guard to UPDATE (GoTrue's pop-ORM resends role='' right after the INSERT and clobbered the INSERT trigger's backfill); `logout()` now does RP-initiated Keycloak end-session + lands on a new public `/logged-out` route, which both actually ends the Keycloak SSO cookie (so the next login re-prompts) and closes a PKCE-verifier race that produced "PKCE code verifier not found in storage" on prod. 259 tests green: 45 pgTAP + 140 Vitest integration + 15 Vitest unit + 59 Playwright + 1 Vitest rate-limit (gated, run via `just test-rate-limit`). 3 skipped in `just test-all` (2 Realtime presence — upstream `handle_out/3` bug; 1 rate-limit — gated). Push notifications + CI Docker + final production icons deliberately out of scope. ✅**
|
||||
|
||||
## Documentation Map
|
||||
|
||||
@@ -12,6 +12,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
|
||||
- `docs/development.md` — local dev stack, repo layout, Justfile + test commands, endpoints, credentials, secrets
|
||||
- `docs/deployment.md` — production deploy to ambrosio (erosi.oier.ovh) + prod-specific fixes
|
||||
- `docs/history/fase-{1,2a,2b,3,4,5,6,7}.md` — per-phase record of what was built
|
||||
- `docs/history/fase-8-auth-logout.md` — post-fase-7 prod auth hardening (migration 014 + RP-initiated logout)
|
||||
- `analysis/analisis-funcional.md` — complete functional specification (domain model, use cases, data models, business rules)
|
||||
- `plan/fase-*.md` — phase-by-phase implementation plans (Fase 0 through Fase 7)
|
||||
- `design/slate_collective/DESIGN.md` + `design/*/screen.png` — "Monolith Editorial" direction + per-screen Stitch mockups. Consult before changing UI.
|
||||
@@ -40,7 +41,7 @@ Key decisions:
|
||||
- **Invitations are link-only** — no email sent. Admin generates a link and shares it manually.
|
||||
- **Multiple collectives per user** — a user can belong to several collectives and switch between them in the sidebar.
|
||||
|
||||
Logout is a single call — `supabase.auth.signOut()` — which clears the GoTrue session. Keycloak's own SSO session persists (user won't be prompted for credentials on next login until the Keycloak session expires), which is acceptable for this app.
|
||||
Logout is RP-initiated: `logout()` in `$lib/auth` calls `supabase.auth.signOut()` to clear the GoTrue session, then redirects to Keycloak's `/protocol/openid-connect/logout` endpoint with `client_id` + `post_logout_redirect_uri=/logged-out`. Keycloak clears its SSO cookie and bounces the browser back to the public `/logged-out` page (outside the `(app)` group so the auth-gate `$effect` never runs). Keycloak shows a one-click "Do you want to log out?" confirmation because GoTrue's proxy flow does not surface the Keycloak `id_token` to the client — without `id_token_hint`, Keycloak always asks; this is the accepted UX. An `isLoggingOut` module flag suppresses the `(app)` layout `$effect` between `signOut()` and the end-session redirect, which otherwise races the navigation and can produce "PKCE code verifier not found in storage" errors.
|
||||
|
||||
## Sync Strategy
|
||||
|
||||
@@ -109,7 +110,7 @@ Defined in `keycloak/realm-export.json` and `supabase/seed.sql`. All use passwor
|
||||
|
||||
- **Kong env var substitution via `kong-start.sh`** — Kong 2.8 does not interpolate `${VAR}` in declarative config files. `infra/kong-start.sh` runs `perl -pe 's/\$\{(\w+)\}/$ENV{$1}/ge'` on `kong.yml` before starting Kong. Without this, Kong loads the literal string `${SUPABASE_ANON_KEY}` as the API key, rejecting all requests.
|
||||
|
||||
- **GoTrue v2.158.1 inserts OIDC-provisioned users with empty-string `role` and `aud`.** For users created via the external provider path (Keycloak → GoTrue), `auth.users.role` and `auth.users.aud` are left as `''` instead of the expected `'authenticated'`. The JWT minted from that row carries the empty role, and PostgREST's per-request `SET LOCAL role = $role_claim` then fails with `role "" does not exist` — every REST call errors out before policies or RPCs even run. Dev never surfaced this because `seed.sql` hardcodes both columns. Migration `013_auth_users_role_default.sql` backfills existing rows and installs a BEFORE INSERT trigger on `auth.users` that fills empty/NULL role + aud with `'authenticated'`. Users in existing browser sessions must log out + back in once for a fresh JWT after the migration is applied.
|
||||
- **GoTrue v2.158.1 inserts OIDC-provisioned users with empty-string `role` and `aud`, AND re-clears role via a follow-up UPDATE.** For users created via the external provider path (Keycloak → GoTrue), `auth.users.role` and `auth.users.aud` are left as `''` instead of the expected `'authenticated'`. The JWT minted from that row carries the empty role, and PostgREST's per-request `SET LOCAL role = $role_claim` then fails with `role "" does not exist` — every REST call errors out before policies or RPCs even run. Dev never surfaced this because `seed.sql` hardcodes both columns. Two migrations are required, not one: **013** backfills existing rows and installs a BEFORE INSERT trigger on `auth.users`; **014** installs a matching BEFORE UPDATE trigger because GoTrue immediately UPDATEs the same row (pop ORM resends the whole struct including the zero-value `role=''`), clobbering the insert-time backfill. Both triggers call the same `auth.ensure_user_role_default()` function. Symptom on prod (ambrosio) was two rows with `created_at`/`updated_at` ~150–250ms apart, empty `role`, `aud='authenticated'` (the UPDATE kept aud because GoTrue sends `'authenticated'` for it). Users in existing browser sessions must log out + back in once for a fresh JWT after the migrations are applied. Covered by `supabase/tests/008_auth_user_role_default.sql` (11 pgTAP assertions including UPDATE-clobber behaviour).
|
||||
|
||||
### SvelteKit / frontend
|
||||
|
||||
@@ -121,6 +122,8 @@ Defined in `keycloak/realm-export.json` and `supabase/seed.sql`. All use passwor
|
||||
|
||||
- **Supabase JS v2 + SvelteKit: `loadUserCollectives` (or any PostgREST query) inside `onAuthStateChange` must be deferred with `setTimeout(..., 0)`.** GoTrue's default `navigator.locks`-based auth lock is held while the `onAuthStateChange` callback runs. Any `supabase.from(...).select(...)` called inside that callback calls `getAccessToken() → getSession() → initializePromise → _acquireLock`, which is the same lock that's already held by the emit logic — the promise never resolves, the query hangs forever. Fix: schedule the query off the callback's microtask chain with `setTimeout(async () => { await loadUserCollectives(session.user.id); … }, 0)`. As a defensive extra, `$lib/supabase` passes a pass-through `auth.lock` option so the whole lock path is a no-op. Both are needed: without the defer, some browsers still deadlock; without the pass-through, others do. Removing either one breaks the Playwright E2E suite.
|
||||
|
||||
- **Logout must be RP-initiated AND guarded against the auth-gate `$effect`.** A naïve logout (`supabase.auth.signOut()` only) has two problems at once: (a) Keycloak's SSO cookie is untouched, so the `(app)` layout's `$effect` immediately re-fires `login()` and Keycloak silently re-authenticates from the SSO cookie — logout looks broken; (b) the `signOut` → `onAuthStateChange` → `$effect` → `signInWithOAuth` chain races `signOut`'s async storage clear, and can clobber the PKCE verifier just written by `signInWithOAuth`, producing "PKCE code verifier not found in storage" on prod (dev doesn't hit it because the loopback round-trip is sub-ms). Fix, in `$lib/auth`: set a module-level `isLoggingOut = true`, call `signOut()`, then `window.location.assign(keycloakEndSessionUrl)` with `client_id` + `post_logout_redirect_uri=${origin}/logged-out`. The `(app)` layout `$effect` reads `isLoggingOut` and skips its auto-login branch. `/logged-out` is a public route outside the `(app)` group so the effect never runs there at all. Keycloak shows a "Do you want to log out?" confirmation because we cannot pass `id_token_hint` (GoTrue's proxy flow doesn't expose the Keycloak id_token in the Supabase session — only `provider_token` and `provider_refresh_token`); clicking once is accepted UX. Covered by A-05 + A-07 in `apps/web/tests/e2e/auth.test.ts`.
|
||||
|
||||
- **Never call `crypto.randomUUID()` directly — use `generateId()` from `$lib/utils/id`.** `crypto.randomUUID` is gated behind secure contexts (HTTPS / localhost / 127.0.0.1 / file://). When the app is served over a bare LAN IP for on-device phone previews (`http://192.168.1.167:5173`), `window.crypto.randomUUID` is `undefined` and every optimistic-id site crashes `handleAdd` with `TypeError: crypto.randomUUID is not a function`. `generateId()` uses `crypto.randomUUID()` when available and falls back to an RFC 4122 v4 assembly via `crypto.getRandomValues()` (which IS exposed on insecure contexts). Covered by `src/lib/utils/id.test.ts` (U-01..U-04) + `tests/e2e/insecure-context.test.ts` (INS-01, stubs `randomUUID = undefined` via `addInitScript` and drives the add-item flow).
|
||||
|
||||
### PWA / build
|
||||
|
||||
@@ -18,7 +18,7 @@
|
||||
```
|
||||
apps/web/src/lib/
|
||||
supabase.ts # Supabase singleton client (PKCE, GoTrue auth)
|
||||
auth.ts # login() / logout() via supabase.auth.signInWithOAuth (Keycloak provider)
|
||||
auth.ts # login() via signInWithOAuth; logout() = signOut + RP-initiated Keycloak end-session → /logged-out
|
||||
stores/ # Svelte global stores
|
||||
sync/ # offline queue + conflict resolution
|
||||
components/ # reusable UI components
|
||||
|
||||
82
docs/history/fase-8-auth-logout.md
Normal file
82
docs/history/fase-8-auth-logout.md
Normal file
@@ -0,0 +1,82 @@
|
||||
# Fase 8 — prod auth hardening (2026-04-20..21)
|
||||
|
||||
Two prod-only bugs observed on ambrosio after fase-7 shipped, both in the Keycloak → GoTrue → Supabase auth integration. Neither is reachable from dev because dev uses pre-seeded `auth.users` rows and localhost round-trips.
|
||||
|
||||
## Bug 1 — empty `auth.users.role` on new self-registrations
|
||||
|
||||
### Symptom
|
||||
After the first real self-registrations via Keycloak, every REST call from those users errored. PostgREST logged `role "" does not exist`. The users existed in `auth.users` but with `role=''` and `aud='authenticated'`.
|
||||
|
||||
### Root cause
|
||||
GoTrue v2.158.1's OIDC-provisioning path first INSERTs the user with empty `role` and `aud`, then immediately UPDATEs the same row (the pop ORM resends the full struct — `last_sign_in_at`, etc. — with the zero-value `role=''` still in the field). The INSERT trigger from migration 013 filled both columns correctly, but the follow-up UPDATE clobbered `role` back to empty. The footprint in `auth.users` is `created_at` / `updated_at` ~150–250ms apart with `role=''` and `aud='authenticated'` (aud stays filled because GoTrue's UPDATE sends `'authenticated'`).
|
||||
|
||||
### Fix — migration 014
|
||||
`supabase/migrations/014_auth_users_role_update_trigger.sql`:
|
||||
|
||||
- Re-backfills any rows already clobbered by the UPDATE (idempotent).
|
||||
- Installs `ensure_role_default_update` — a BEFORE UPDATE trigger on `auth.users` reusing the existing `auth.ensure_user_role_default()` function from migration 013.
|
||||
|
||||
Applied directly to prod DB 2026-04-20 (rsync + `psql < migration.sql` + manual insert into `_applied_migrations`). Two users who had been broken were backfilled in the same statement. pgTAP `008_auth_user_role_default.sql` was extended from 7 to 11 assertions to cover the UPDATE path + unrelated-UPDATE no-op behaviour.
|
||||
|
||||
## Bug 2 — logout doesn't log out + "PKCE code verifier not found in storage"
|
||||
|
||||
### Symptom
|
||||
User reported two things, connected:
|
||||
1. Clicking Logout returned them to the app still logged in (Keycloak re-authenticated silently).
|
||||
2. Occasionally the browser console showed `PKCE code verifier not found in storage. This can happen if the auth flow was initiated in a different browser or device, or if the storage was cleared.`
|
||||
|
||||
### Root cause
|
||||
Old `logout()` was `supabase.auth.signOut()` only, which clears the Supabase/GoTrue session but never touches Keycloak's SSO cookie. `(app)/+layout.svelte`'s `$effect` watches `$isAuthenticated` and fires `login()` the moment auth becomes false. So clicking logout produced this chain:
|
||||
|
||||
1. `signOut()` clears Supabase session.
|
||||
2. `onAuthStateChange` fires `SIGNED_OUT`.
|
||||
3. `$effect` sees `!$isAuthenticated && !$authLoading` → calls `login()`.
|
||||
4. `signInWithOAuth` writes a fresh PKCE verifier to localStorage, redirects to Keycloak.
|
||||
5. Keycloak SSO cookie still valid → immediate redirect back to `/auth/callback?code=…`.
|
||||
6. Callback runs `exchangeCodeForSession(code)`.
|
||||
|
||||
In principle step 6 should find the verifier from step 4, but the chain is race-adjacent and breaks intermittently on prod:
|
||||
|
||||
- supabase-js v2.46's `signOut()` clears storage through an async adapter; if that clear lands after the verifier write, it wipes it.
|
||||
- The `$effect` can fire twice in quick succession (Svelte 5 reactive re-emits) — two concurrent `signInWithOAuth` calls overwrite each other's verifiers while only one `window.location.assign` wins.
|
||||
- iOS Safari occasionally drops localStorage writes issued immediately before navigation.
|
||||
|
||||
Dev never reproduced it because the loopback round-trip is sub-ms.
|
||||
|
||||
### Fix — RP-initiated OIDC logout
|
||||
- `apps/web/src/lib/auth.ts` — `logout()` now sets a module-level `isLoggingOut = true`, calls `signOut()`, then `window.location.assign()` to Keycloak's `/protocol/openid-connect/logout?client_id=colectivo-web&post_logout_redirect_uri=${origin}/logged-out`. Keycloak clears the SSO cookie and redirects back.
|
||||
- `apps/web/src/routes/(app)/+layout.svelte` — `$effect` reads `isLoggingOut` and skips `login()` during the logout navigation, closing the PKCE race.
|
||||
- `apps/web/src/routes/logged-out/+page.svelte` — new public route outside the `(app)` group, so the auth-gate effect never runs there. Shows a "Signed out" message + "Sign in" button that triggers a normal `login()`.
|
||||
- `apps/web/messages/{en,es}.json` — added `logged_out_title`, `logged_out_subtitle`, `logged_out_sign_in`.
|
||||
|
||||
### Accepted UX caveat — Keycloak confirmation page
|
||||
Keycloak only skips the "Do you want to log out?" confirmation page when `id_token_hint` is supplied. GoTrue's session object exposes `provider_token` and `provider_refresh_token` but NOT the Keycloak `id_token` (confirmed in `@supabase/auth-js` types — no `provider_id_token` field). We can't derive the id_token client-side because colectivo-web is a confidential client and the token-endpoint refresh flow requires `client_secret`. Options to eliminate the click were considered and rejected as overkill:
|
||||
|
||||
- **Server-side id_token fetch** — a SvelteKit `+server.ts` endpoint that uses service_role to SELECT `auth.identities.identity_data.id_token` for the current user, then returns the end-session URL with `id_token_hint`. Feasible (adapter-node is in use) but the one-click confirmation is standard OIDC and clear UX; the complexity wasn't justified.
|
||||
|
||||
The confirmation is documented in the A-05 / A-07 E2E tests — a helper `confirmKeycloakLogout(page)` clicks through it.
|
||||
|
||||
## Tests added or changed
|
||||
|
||||
- `supabase/tests/008_auth_user_role_default.sql` — plan bumped 7 → 11. New assertions: `ensure_role_default_update` trigger exists; UPDATE to `role=''`/`aud=''` gets reverted; unrelated UPDATE leaves explicit `service_role` untouched.
|
||||
- `apps/web/tests/e2e/auth.test.ts` — A-05 rewritten to assert landing on `/logged-out` + no PKCE error in console + Supabase session cleared from localStorage. New A-07 asserts that the next login after logout prompts for Keycloak credentials (i.e. SSO cookie was actually cleared).
|
||||
|
||||
Totals after the fix: 259 tests green (45 pgTAP + 140 Vitest integration + 15 Vitest unit + 59 Playwright), same 3 skips as before.
|
||||
|
||||
## Deploy
|
||||
|
||||
- Migration 014 applied directly on prod 2026-04-20 (SQL-only change, no need for full rebuild).
|
||||
- Web code (RP-initiated logout + `/logged-out`) deployed via `infra/scripts/deploy-erosi.sh` on 2026-04-21. Migrations were skipped (already applied); app image rebuilt + rolling restart.
|
||||
|
||||
## Files touched
|
||||
|
||||
- `supabase/migrations/014_auth_users_role_update_trigger.sql` (new)
|
||||
- `supabase/tests/008_auth_user_role_default.sql` (+4 assertions)
|
||||
- `apps/web/src/lib/auth.ts`
|
||||
- `apps/web/src/routes/(app)/+layout.svelte`
|
||||
- `apps/web/src/routes/logged-out/+page.svelte` (new)
|
||||
- `apps/web/messages/en.json`, `apps/web/messages/es.json`
|
||||
- `apps/web/tests/e2e/auth.test.ts`
|
||||
- `CLAUDE.md`, `docs/development.md`, `docs/history/fase-8-auth-logout.md` (this file)
|
||||
|
||||
Commits: `9067ab8` (migration 014), `ca12516` (logout fix).
|
||||
Reference in New Issue
Block a user