diff --git a/src/app.ts b/src/app.ts index cd6eb45..7acf7cc 100644 --- a/src/app.ts +++ b/src/app.ts @@ -222,8 +222,7 @@ export function createApp(options: AppOptions = {}): Server { // OAuth2 login challenge (§6): Hydra hands the browser here when another app logs in // *through* us. Resolve it via the Kratos session and accept; an unauthenticated user - // bounces to our themed login and returns here once signed in. Challenge looked up over - // Hydra's admin API. Nothing first-party needs this — it's the OAuth2-provider role only. + // bounces to our themed login and returns here once signed in. Provider-only. if (hydra && kratos && pathname === "/oauth2/login" && (method === "GET" || method === "HEAD")) { const challenge = ctx.url.searchParams.get("login_challenge"); if (!challenge) { @@ -288,8 +287,7 @@ export function createApp(options: AppOptions = {}): Server { return; } } catch (err) { - // Stale/invalid/consumed challenge (Hydra 4xx — back button, slow login, re-used URL): - // recoverable 400, not a 500. A genuine Hydra outage (5xx) rethrows → 500. + // Stale/consumed challenge (Hydra 4xx) → recoverable 400; a genuine outage (5xx) → 500 (as /oauth2/login). if (err instanceof HydraError && err.status < 500) { res.writeHead(400, { "content-type": "text/plain; charset=utf-8" }).end("This authorization request has expired. Please start again from the application you were signing in to."); return; diff --git a/todo.md b/todo.md index cf141e1..eab8425 100644 --- a/todo.md +++ b/todo.md @@ -106,7 +106,7 @@ everything via Docker. - [x] Consent-challenge handler: show / auto-accept first-party, grant scopes, accept/reject. → `src/hydra-admin.ts` gains the consent half of the handshake (`getConsentRequest`/`acceptConsentRequest`/`rejectConsentRequest` + `ConsentRequest`/`AcceptConsent`/`ConsentSession` types; the login/consent URL builder folded into one `reqUrl(kind,…)` + a shared `put()`). `src/oauth-consent.ts` (pure, sibling of `oauth-login.ts`): `resolveConsentChallenge` → **skip** (Hydra already consented / a skip-consent client) or **first-party** (the client's Hydra `metadata.first_party === true`) ⇒ auto-accept, else return a `view` to show the themed consent screen; `acceptConsent` (re-reads the challenge so scopes/audience are **never** client-supplied) + `rejectConsent` (access_denied). The grant carries an OIDC `session.id_token` with `email`/`name` projected from the Kratos identity (`whoami` traits; absent ⇒ omitted). Wired in `app.ts` at `GET|POST /oauth2/consent` (gated on `hydra`+`kratos`): GET shows/auto-accepts (sets the page CSRF cookie when fresh), POST is **CSRF-guarded** (same signed double-submit as `/logout`) and dispatches `decision=allow`→accept / else→reject → 303 to Hydra; a stale/consumed challenge (Hydra 4xx) degrades to a recoverable 400, a genuine outage (5xx) → 500 (mirrors `/oauth2/login`). `views/oauth-consent.ejs` + `partials/consent-body.ejs` reuse the auth-card: the consent screen lists the requested scopes (friendly labels for the standard OIDC ones) with Allow/Deny submit buttons. Tests-first: `hydra-admin.test.ts` (consent request contracts), `oauth-consent.test.ts` (skip/first-party/third-party/audience/id_token/accept-refetch/reject matrix), `app.test.ts` HTTP integration (auto-accept / screen render+CSRF cookie / allow+deny / forged-CSRF→403 / missing→400 / stale→400 / outage→500). Stability-reviewer run as a local PR: APPROVE, no Critical/High. Extended the full-stack E2E `e2e/oauth-login.spec.ts` to drive the **whole** authorization-code flow against real Hydra — login accept → follow the login_verifier through Hydra → web's consent screen (third-party client `e2e-login`, scopes listed) → Allow → consent_verifier → the client callback with a real `code` (per-host cookie jars; Hydra resume URLs rebased onto the compose host). typecheck + 262 units + 8 visual + OAuth login+consent E2E green; stack torn down. OAuth2 client registration is the next §6 item. - [x] OAuth2 client registration (admin UI or CLI). → Built-in **OAuth2 clients** admin screen (`src/admin-clients.ts`, `/admin/clients`) — the §6 client side of Hydra (apps that log in *through* us). `src/hydra-admin.ts` gains the client half of the admin API: `createClient`/`listClients`/`getClient`/`deleteClient` over `/admin/clients` (+ a `nextPageToken` Link parser, mirrors kratos-admin) and the registration fields on `OAuth2Client`. The screen mirrors the §5 Users/Roles pattern — pure builders (`toClientView`, `clientPayload`, `validateClientInput`, `parseRedirectUris`, `buildClients{List,Form,Detail}Model`) + `handleAdminClients` (the imperative shell app.ts dispatches `/admin/clients*` to). Routes: `GET /admin/clients` (list — search/paginate over one Hydra page), `GET|POST /admin/clients/new`+`/` (register), `GET /admin/clients/:id` (read-only detail), `GET|POST …/:id/delete` (confirm + delete). Register builds a standard authorization-code client (+ refresh_token), confidential (`client_secret_basic`) or public (PKCE, `none`), with an optional first-party (auto-consent) flag; **Hydra returns the `client_secret` once**, so the register POST renders the new client's detail page with the one-time secret directly (no PRG) — never re-shown (`getClient` carries no secret; detail asserts it). Writes go **only to Hydra**; gated admin-only (anon→/login, non-admin→403) + every mutation CSRF-guarded, like §5; a Hydra 4xx (bad redirect/scope) re-renders the form (400), a 5xx → 500 (mirrors `oauth-login.ts`); `:id` via `safeDecode` (malformed→404). Wired into the shared `adminSection` (Users·Groups·Roles·**OAuth2 clients**, `i-globe`) so it shows for admins, invisible otherwise. New views (`admin/clients`,`client-form`,`client-detail` + `partials/client-{form,detail}-body`) reuse the shell/filter-bar/data-table/field blocks; one `.detail-list` CSS rule. Tests-first: `hydra-admin.test.ts` (client CRUD contracts incl. Link pagination/404→null/204), `admin-clients.test.ts` (builder/validation/payload matrix), `app.test.ts` HTTP integration (gate/list/register-shows-secret-once/invalid+CSRF-reject/detail-hides-secret/delete + malformed-`%`→404). Stability-reviewer run as a local PR: APPROVE, no Critical/High; addressed its one nit (dropped a dead `URL.protocol` check in `validateClientInput`). Boot-verified the client CRUD live against real Hydra v26.2.0 (create→201 w/ one-time secret → list finds it → get → delete → get null); torn down. typecheck + 274 units green. Review/comment/test-cleanup are the next §6 items. - [x] Run the architecture and the product reviewer agents on the _whole_ project, not just the latest changes, and address their issues. → Ran both on the whole project (weighted to the §6 Hydra OAuth2 surfaces). Architecture: **no Critical**; Product: **no Critical** this checkpoint. **Fixed now (tests-first):** (1) HIGH (arch) — `/oauth2/logout` was published to Hydra (`hydra.yml` `urls.logout`) and asserted in `hydra.test.ts`, but had **no handler** (a dead/published contract — Hydra's RP-initiated logout would 404). Added `hydra-admin.acceptLogoutRequest` (PUT logout/accept, folded into the shared `reqUrl(kind…)`) + a `GET /oauth2/logout` branch: accept the `logout_challenge` → 303 to Hydra's post-logout redirect; missing challenge → 400; a stale/consumed 4xx → recoverable 400, a 5xx outage → 500 (byte-identical degrade to the login/consent siblings). GET-accept is safe — the challenge is Hydra-minted + single-use; the first-party `POST /logout` still owns ending the Kratos session + our JWT cookie. (2) HIGH (arch) — added `oauth2` to `RESERVED_PLUGIN_IDS` (a `plugins/oauth2/` folder would silently shadow the provider routes — the one route surface the §4 reserved-id fix didn't cover; discovery now refuses it loud). (3) Product **Blocker** — the consent screen never told the user *whose* account they were authorizing (informed-consent gap on shared devices). It now renders "Signed in as ``" (`ConsentView.account` from `whoami`) + a "Not you? Sign out" form (CSRF-guarded by the same signed double-submit). (4) MEDIUM (arch) — consent `accept()` now projects id_token claims **only when** the live Kratos session subject `===` the challenge subject Hydra bound at login (never leak a mismatched session's email/name into the issued token; guards the auto-accept path too). (5) Product nits — register-form confidential-vs-public guidance ("Browser/mobile apps can't keep a secret — choose Public…") and a client-detail "to change a client, delete and re-register — the secret is shown only once" note (covers the no-edit friction + lost-secret-on-reload). Stability-reviewer run as a local PR: **APPROVE, no Critical/High**; addressed its actionable follow-ups (README §6 now documents the logout handler + the consent identity line; a comment notes the GET-accept is Hydra-validated). Extended `e2e/oauth-login.spec.ts` to assert the consent screen names the signed-in account; **boot-verified the full OAuth2 login+consent flow live against real Hydra v26.2.0** (E2E green) then torn down. typecheck + 279 units green. **Deferred (reviewer-scoped, not the §6 checkpoint):** the host **internal route-table** (fold the admin/oauth if-ladder into one `{method,prefix,handler}` table, derive `RESERVED_PLUGIN_IDS`/`allowedMethods` from it — arch M1, the long-deferred §2/§5 item) → **§9**: H1/H2 are now point-fixed, so M1 is reduced to a pure dedup/structural improvement (Medium) best done as a focused standalone change to the central dispatcher, not bundled into a review-fix; the **RP-initiated-logout browser/live E2E** (needs token-exchange + `id_token_hint` + `post_logout_redirect_uris`) → **§8** (owns full E2E — the handler is unit+HTTP covered and reuses the `complete()` path already live-verified by the login/consent accepts); the redirect-URI **scheme allowlist** + the `safeUrl()` href helper (arch L1) → **§7** (first untrusted-URL flow); full **client edit** (registration-only by design — the detail page now says delete+re-register), the blank **empty-list** state (known §5 deferral), and success-flash after writes → §8/polish; unconditional `refresh_token` + raw custom-scope labels (product 🟢) → future. -- [ ] Go over all comments in the code and the README and try to make it shorter and more information dense. Remove not strictly needed stuff. +- [x] Go over all comments in the code and the README and try to make it shorter and more information dense. Remove not strictly needed stuff. → Pass over the §6 Hydra/OAuth2 accretion. The §6 modules (`hydra-admin`/`oauth-login`/`oauth-consent`/`admin-clients`) were authored dense, so the wins are in `app.ts`, where the three sibling OAuth2 handlers had drifted toward repeating the same prose: trimmed the `/oauth2/login` block header (dropped "Challenge looked up over Hydra's admin API" — derivable from `resolveLoginChallenge` — and condensed the provider-role sentence to "Provider-only.", matching the consent/logout blocks); collapsed the `/oauth2/consent` degrade comment to the same one-liner the `/oauth2/logout` sibling already uses, so the canonical 4xx→400 / 5xx→500 explanation lives once (the `/oauth2/login` block). Left intact: the logout block's GET-accept safety rationale (reviewer-requested, todo line 108), the EJS view config-doc headers (the only schema for untyped locals), and the §6 README **OAuth2 provider (Hydra)** + admin-clients sections (authored concise in §6, todo line 108). The README **Status** note / `_(planned)_` markers / Layout refresh stays §9's (line 133). typecheck + 279 units green. - [ ] Go over all tests and combine/unify ones that cover the same stuff or are very related and could be combined in a good way. Remove tests that aren't helping, we only want tests that are actually helpful to us. ## 7. Example plugin (reference)