feat(auth): (unauthorized) 403 page (closes #17) #23

Merged

forgejo_admin merged 1 commit from 17-unauthorized-page into main 2026-05-03 20:28:55 +00:00

Conversation 1 Commits 1 Files changed 5 +196

forgejo_admin commented 2026-05-03 19:37:03 +00:00

Owner

Copy link

Summary

Implements the (unauthorized) route group that renders the 403 page when the auth hook (#15, merged via PR #20) rewrites authenticated-but-not-admin requests to /__unauthorized. Final sub-task of #2 (Keycloak cookie SSR auth + admin role gate) — the parent's checklist is now complete.

The page preserves the user-visible URL (the rewrite is internal), drops the root admin chrome via the @-sigil layout reset, displays the user's name with a clear message about the missing admin realm role, and offers a same-origin <form>-POST sign-out button that hits #16's CSRF-protected /auth/logout (no JS required).

Closes #17

Changes

Five new files, no modifications to existing files:

• src/routes/(unauthorized)/+layout@.svelte — layout reset (note the @ sigil). A bare +layout.svelte would still be wrapped by the root admin chrome; the @ makes this a top-level layout component that does not inherit from src/routes/+layout.svelte. Imports app.css so the --color-* and --space-* tokens still resolve.
• src/routes/(unauthorized)/+layout.server.ts — load({ locals }) => ({ user: locals.user }). Required because the page-level +page.server.ts load throws error(403, …), and SvelteKit's contract is that an error() thrown from a page load discards the page load's return value before rendering +error.svelte. Layout loads survive the throw, so this file is what populates $page.data.user in the error boundary. The @ sigil resets layout component inheritance only — server loads run through the full chain regardless.
• src/routes/(unauthorized)/__unauthorized/+page.server.ts — throws error(403, 'Admin role required') from @sveltejs/kit. The throw both sets HTTP status 403 (setHeaders cannot do this — it's a header API, not a status API) and routes rendering to the co-located +error.svelte.
• src/routes/(unauthorized)/__unauthorized/+page.svelte — placeholder. SvelteKit needs a +page.svelte for the route to resolve, but it never renders because the load throws first. One-line HTML comment.
• src/routes/(unauthorized)/__unauthorized/+error.svelte — the actual 403 UI. Heading "Admin access required", user name from $page.data.user.name (with email and a generic fallback), the error message from $page.error.message, and the sign-out form. Pure CSS in a <style> block using the existing --color-* / --space-* custom-property tokens — no framework imports, no new deps, no images.

Funnel-Auth Review

Per feedback_funnel_requires_auth, every funnel-exposed surface needs documented auth.

This PR is the consumer of the hook's role-gate decision; it doesn't introduce a new ingress and doesn't add any path that bypasses authentication. The funnel-auth posture is unchanged from PR #20:

• The hook (frozen, src/hooks.server.ts) 302s anonymous requests to /auth/login before any route handler, including this one, runs. An anonymous user can never observe /__unauthorized — they will be sent to login first regardless of which path they hit.
• The only way into the (unauthorized) group is the hook's internal rewrite (event.url.pathname = '/__unauthorized') when an authenticated user lacks the admin role. The browser-visible URL stays at the original path (e.g. /players); the rewrite is server-internal.
• This page reads locals.user, which is populated by the hook from a verified Keycloak JWT — same auth source as the rest of the app. No new lookup, no new identity surface.
• The "Sign out" button posts cross-<form> to /auth/logout (#16), which already enforces an Origin-header CSRF check against the canonical app URL.
• Surface area closed: "what does a non-admin authenticated user see?" The pre-#17 answer was "the SvelteKit fallback error page with no sign-out path"; the post-#17 answer is "a deliberate 403 page with their name and a one-click sign-out". This narrows the social-engineering surface around stale or wrong-account sessions and gives the user a clean exit instead of a dead-end error screen.

No new headers, no new endpoints, no new auth flows.

Test Plan

Automated (run in this PR's worktree, /tmp/westside-admin-17):

• npm run check exits 0 — 0 ERRORS 0 WARNINGS 0 FILES_WITH_PROBLEMS. App.Locals.user typing reaches the loads as expected.
• npm test exits 0 — 25/25 lib tests still pass (no regressions; this PR adds no new tests because the route layer has no unit-testable surface — see "Discovered scope" below).
• npm run build succeeds for adapter-node. Build output includes entries/pages/(unauthorized)/__unauthorized/_error.svelte.js and _page.svelte.js, confirming the route group compiled.
• npx eslint src/routes/(unauthorized)/ clean.
• npx prettier --check src/routes/(unauthorized)/ clean (the pre-existing .woodpecker.yaml prettier warning is on main and unrelated to this PR).

Manual (per AC, post-merge in the deployed env):

• Log in as a non-admin Keycloak user (or strip the admin realm role from a test user); hit /players → expect URL stays /players, page body is the 403 UI, network tab shows 403 Forbidden. Verifiable via curl -i -b cookie.txt https://westside-admin.tail5b443a.ts.net/players — first response line HTTP/1.1 403 Forbidden.
• Inspect the rendered HTML — confirm the root layout's <header class="site-header"> markup is NOT present (the @ sigil is doing its job).
• Click "Sign out" → cookie cleared, redirected through Keycloak SLO, lands on /.
• Disable JS in the browser → the sign-out button still submits the form (<form action="/auth/logout" method="POST"> works without JS).

Review Checklist

• No CSS frameworks (no Tailwind per feedback_no_tailwind). Pure CSS in <style> blocks using existing app.css tokens.
• No new dependencies (package.json unchanged).
• No new images / icons in static/.
• TypeScript strict — no any casts. Loads use the generated $types (LayoutServerLoad, PageServerLoad).
• Out-of-scope files NOT modified: src/lib/server/keycloak.ts, src/hooks.server.ts, src/routes/auth/*, src/routes/+layout.svelte, src/app.d.ts.
• Branch name follows convention: 17-unauthorized-page.
• Layout reset uses the @ sigil (+layout@.svelte), not a bare +layout.svelte.
• Page-level load throws error(403, …) from @sveltejs/kit (not setHeaders).
• User name reaches the error boundary via +layout.server.ts (not +page.server.ts, whose return value the throw discards).
• Sign-out button is a real <form action="/auth/logout" method="POST"> (no JS-only fetch).
• Funnel-auth review included in PR body.

Related Notes

• Closes forgejo_admin/westside-admin#17
• Parent: forgejo_admin/westside-admin#2 (Keycloak cookie SSR auth + admin role gate) — this is the LAST of 4 sub-tasks; parent's checklist is now complete.
• Sibling sub-tasks (all merged):
  • #14 — keycloak.ts SSR auth lib + vitest setup (PR #18)
  • #15 — hooks.server.ts admin role gate (PR #20)
  • #16 — /auth/login + /callback + /logout endpoints (PR #21)
• Conventions / memory:
  • feedback_no_tailwind — pure CSS only.
  • feedback_funnel_requires_auth — funnel-auth posture documented above.
• Architecture: arch-dataflow-westside-admin (the funnel-auth → cookie-auth → role-gate → (unauthorized) 403 path is the closing edge of the auth subgraph for this app).

## Summary Implements the `(unauthorized)` route group that renders the 403 page when the auth hook (`#15`, merged via PR #20) rewrites authenticated-but-not-admin requests to `/__unauthorized`. Final sub-task of `#2` (Keycloak cookie SSR auth + admin role gate) — the parent's checklist is now complete. The page preserves the user-visible URL (the rewrite is internal), drops the root admin chrome via the `@`-sigil layout reset, displays the user's name with a clear message about the missing `admin` realm role, and offers a same-origin `<form>`-POST sign-out button that hits `#16`'s CSRF-protected `/auth/logout` (no JS required). Closes #17 ## Changes Five new files, no modifications to existing files: - `src/routes/(unauthorized)/+layout@.svelte` — layout reset (note the `@` sigil). A bare `+layout.svelte` would still be wrapped by the root admin chrome; the `@` makes this a top-level layout component that does not inherit from `src/routes/+layout.svelte`. Imports `app.css` so the `--color-*` and `--space-*` tokens still resolve. - `src/routes/(unauthorized)/+layout.server.ts` — `load({ locals }) => ({ user: locals.user })`. Required because the page-level `+page.server.ts` `load` throws `error(403, …)`, and SvelteKit's contract is that an `error()` thrown from a *page* load discards the page load's return value before rendering `+error.svelte`. Layout loads survive the throw, so this file is what populates `$page.data.user` in the error boundary. The `@` sigil resets layout *component* inheritance only — server loads run through the full chain regardless. - `src/routes/(unauthorized)/__unauthorized/+page.server.ts` — throws `error(403, 'Admin role required')` from `@sveltejs/kit`. The throw both sets HTTP status 403 (`setHeaders` cannot do this — it's a header API, not a status API) and routes rendering to the co-located `+error.svelte`. - `src/routes/(unauthorized)/__unauthorized/+page.svelte` — placeholder. SvelteKit needs a `+page.svelte` for the route to resolve, but it never renders because the load throws first. One-line HTML comment. - `src/routes/(unauthorized)/__unauthorized/+error.svelte` — the actual 403 UI. Heading "Admin access required", user name from `$page.data.user.name` (with `email` and a generic fallback), the error message from `$page.error.message`, and the sign-out form. Pure CSS in a `<style>` block using the existing `--color-*` / `--space-*` custom-property tokens — no framework imports, no new deps, no images. ## Funnel-Auth Review Per `feedback_funnel_requires_auth`, every funnel-exposed surface needs documented auth. This PR is the *consumer* of the hook's role-gate decision; it doesn't introduce a new ingress and doesn't add any path that bypasses authentication. The funnel-auth posture is unchanged from PR #20: - The hook (frozen, `src/hooks.server.ts`) 302s anonymous requests to `/auth/login` *before* any route handler, including this one, runs. An anonymous user can never observe `/__unauthorized` — they will be sent to login first regardless of which path they hit. - The only way into the `(unauthorized)` group is the hook's internal rewrite (`event.url.pathname = '/__unauthorized'`) when an *authenticated* user lacks the `admin` role. The browser-visible URL stays at the original path (e.g. `/players`); the rewrite is server-internal. - This page reads `locals.user`, which is populated by the hook from a verified Keycloak JWT — same auth source as the rest of the app. No new lookup, no new identity surface. - The "Sign out" button posts cross-`<form>` to `/auth/logout` (`#16`), which already enforces an `Origin`-header CSRF check against the canonical app URL. - Surface area closed: "what does a non-admin authenticated user see?" The pre-`#17` answer was "the SvelteKit fallback error page with no sign-out path"; the post-`#17` answer is "a deliberate 403 page with their name and a one-click sign-out". This narrows the social-engineering surface around stale or wrong-account sessions and gives the user a clean exit instead of a dead-end error screen. No new headers, no new endpoints, no new auth flows. ## Test Plan Automated (run in this PR's worktree, `/tmp/westside-admin-17`): - [x] `npm run check` exits 0 — `0 ERRORS 0 WARNINGS 0 FILES_WITH_PROBLEMS`. `App.Locals.user` typing reaches the loads as expected. - [x] `npm test` exits 0 — 25/25 lib tests still pass (no regressions; this PR adds no new tests because the route layer has no unit-testable surface — see "Discovered scope" below). - [x] `npm run build` succeeds for `adapter-node`. Build output includes `entries/pages/(unauthorized)/__unauthorized/_error.svelte.js` and `_page.svelte.js`, confirming the route group compiled. - [x] `npx eslint src/routes/(unauthorized)/` clean. - [x] `npx prettier --check src/routes/(unauthorized)/` clean (the pre-existing `.woodpecker.yaml` prettier warning is on `main` and unrelated to this PR). Manual (per AC, post-merge in the deployed env): - [ ] Log in as a non-admin Keycloak user (or strip the `admin` realm role from a test user); hit `/players` → expect URL stays `/players`, page body is the 403 UI, network tab shows `403 Forbidden`. Verifiable via `curl -i -b cookie.txt https://westside-admin.tail5b443a.ts.net/players` — first response line `HTTP/1.1 403 Forbidden`. - [ ] Inspect the rendered HTML — confirm the root layout's `<header class="site-header">` markup is NOT present (the `@` sigil is doing its job). - [ ] Click "Sign out" → cookie cleared, redirected through Keycloak SLO, lands on `/`. - [ ] Disable JS in the browser → the sign-out button still submits the form (`<form action="/auth/logout" method="POST">` works without JS). ## Review Checklist - [x] No CSS frameworks (no Tailwind per `feedback_no_tailwind`). Pure CSS in `<style>` blocks using existing `app.css` tokens. - [x] No new dependencies (`package.json` unchanged). - [x] No new images / icons in `static/`. - [x] TypeScript strict — no `any` casts. Loads use the generated `$types` (`LayoutServerLoad`, `PageServerLoad`). - [x] Out-of-scope files NOT modified: `src/lib/server/keycloak.ts`, `src/hooks.server.ts`, `src/routes/auth/*`, `src/routes/+layout.svelte`, `src/app.d.ts`. - [x] Branch name follows convention: `17-unauthorized-page`. - [x] Layout reset uses the `@` sigil (`+layout@.svelte`), not a bare `+layout.svelte`. - [x] Page-level load throws `error(403, …)` from `@sveltejs/kit` (not `setHeaders`). - [x] User name reaches the error boundary via `+layout.server.ts` (not `+page.server.ts`, whose return value the throw discards). - [x] Sign-out button is a real `<form action="/auth/logout" method="POST">` (no JS-only fetch). - [x] Funnel-auth review included in PR body. ## Related Notes - Closes `forgejo_admin/westside-admin#17` - Parent: `forgejo_admin/westside-admin#2` (Keycloak cookie SSR auth + admin role gate) — this is the LAST of 4 sub-tasks; parent's checklist is now complete. - Sibling sub-tasks (all merged): - `#14` — keycloak.ts SSR auth lib + vitest setup (PR #18) - `#15` — hooks.server.ts admin role gate (PR #20) - `#16` — /auth/login + /callback + /logout endpoints (PR #21) - Conventions / memory: - `feedback_no_tailwind` — pure CSS only. - `feedback_funnel_requires_auth` — funnel-auth posture documented above. - Architecture: `arch-dataflow-westside-admin` (the funnel-auth → cookie-auth → role-gate → `(unauthorized)` 403 path is the closing edge of the auth subgraph for this app).

forgejo_admin added 1 commit 2026-05-03 19:37:03 +00:00

feat(auth): (unauthorized) 403 page (closes #17) ... 9afbc8e367

Final sub-task of #2 (Keycloak cookie SSR auth + admin role gate).

The auth hook (#15, merged via PR #20) rewrites authenticated-but-not-admin
requests to the internal path `/__unauthorized` before resolve. This commit
adds the route group that renders the 403 UI at that path while preserving
the user-visible URL the user originally hit.

Files:
- src/routes/(unauthorized)/+layout@.svelte
    Layout reset (note the @ sigil). Drops the root admin chrome so the
    403 page does not advertise admin nav to users without the admin role.
- src/routes/(unauthorized)/+layout.server.ts
    Layout-level load that reads locals.user. Required because the page
    load throws error(403, ...), which discards the page load's return
    value before rendering +error.svelte. Layout loads survive the throw,
    so $page.data.user is populated in the error boundary.
- src/routes/(unauthorized)/__unauthorized/+page.server.ts
    Throws error(403, 'Admin role required') from @sveltejs/kit. The throw
    sets HTTP status 403 (setHeaders cannot do this) and routes rendering
    to +error.svelte.
- src/routes/(unauthorized)/__unauthorized/+page.svelte
    Placeholder. Never renders because the load throws first; required by
    SvelteKit so the route resolves.
- src/routes/(unauthorized)/__unauthorized/+error.svelte
    The actual 403 UI. Heading, user name from $page.data.user, message
    from $page.error.message, and a same-origin <form action="/auth/logout"
    method="POST"> sign-out button (no JS, matches #16's CSRF contract).

Constraints honored: no Tailwind, no new deps, pure CSS in <style> blocks
using the existing app.css custom-property tokens.

Verification:
- npm run check exits 0 (0 errors, 0 warnings)
- npm test exits 0 (25/25 lib tests still pass)
- npm run build succeeds (adapter-node)
- npx eslint clean on the new files

forgejo_admin commented 2026-05-03 19:39:26 +00:00

Author

Owner

Copy link

PR #23 Review

DOMAIN REVIEW

Tech stack: SvelteKit (Svelte 5 runes) + TypeScript + adapter-node + pure CSS. Auth surface only — no DB, no FastAPI, no IaC.

1. Layout reset via @ sigil — VERIFIED

src/routes/(unauthorized)/+layout@.svelte is named with the @ sigil before the extension (not a bare +layout.svelte). This is the documented SvelteKit mechanism for opting out of root-layout component inheritance. The build artifact path mentioned in the PR body (entries/pages/(unauthorized)/__unauthorized/_error.svelte.js) is consistent with the route-group resolving correctly. The file imports ../../app.css so design tokens still resolve, and uses {@render children()} (Svelte 5 snippet API). Wrapped in a <main> with sane defaults (max-width, centered, token-based padding). Correct.

2. +layout.server.ts data persistence — VERIFIED

load({ locals }) => ({ user: locals.user }) exactly as required. The doc comment correctly states the SvelteKit contract: an error() thrown from a page load discards that page-load's return value, but layout loads survive and persist into the error boundary. This is the only mechanism that makes $page.data.user available in +error.svelte. Typed via the generated LayoutServerLoad. Correct.

3. HTTP 403 status — VERIFIED

+page.server.ts imports error from @sveltejs/kit and throw error(403, 'Admin role required'). SvelteKit guarantees the throw sets the response status to 403 and routes to the nearest +error.svelte. The PR body correctly notes that setHeaders cannot set status (header API, not status API). Typed via PageServerLoad. Correct.

4. Sign-out form — VERIFIED

<form action="/auth/logout" method="POST"> with a <button type="submit">Sign out</button>. No on:submit, no fetch, no JS handler. Same-origin POST satisfies the Origin-header CSRF check installed by PR #21. Works without JS. Correct.

5. No Tailwind, no new dependencies — VERIFIED

No framework imports anywhere. Pure CSS in <style> blocks using existing --color-* and --space-* tokens from app.css. package.json is not in the diff (additions=196 deletions=0 across 5 new files only).

6. +page.svelte placeholder is intentionally inert — VERIFIED

Single HTML comment line. Never renders because the load throws first. SvelteKit requires a +page.svelte for the route to resolve. Correct.

Out-of-bounds files — VERIFIED

The diff is 5 new files only, all under src/routes/(unauthorized)/. Untouched: src/lib/server/keycloak.ts, src/hooks.server.ts, src/routes/auth/*, src/routes/+layout.svelte, src/app.d.ts. Strict scope discipline.

TypeScript / leakage checks — VERIFIED

• No any casts. Both loads use generated $types.
• No console.log anywhere.
• No tokens, cookies, or secret material rendered. The error UI displays only name, email, and the static error message.
• Display fallback chain name || email || 'an authenticated user' is graceful and avoids undefined rendering.
• $page.error?.message ?? 'Admin role required' defends against missing error message.

Funnel-auth posture — VERIFIED

Per feedback_funnel_requires_auth, the PR body contains a dedicated Funnel-Auth Review section. The audit holds: this PR is a consumer of the existing hook's role-gate decision, not a new ingress. Anonymous users cannot reach /__unauthorized — the hook 302s them to /auth/login before any route handler runs. The only way in is the hook's internal rewrite for authenticated-but-not-admin requests. No new headers, no new endpoints, no new auth flows.

BLOCKERS

None.

NITS

1. Hardcoded color hex in +error.svelte — border: 1px solid #e5e7eb and .body code { background: #f3f4f6 } are raw hex while the rest of the file uses --color-* tokens. If --color-border and --color-code-bg (or similar) don't exist in app.css yet, this is a token-system gap rather than a dev-side miss. Worth filing a follow-up to extend the token palette so future components don't repeat the inline hex. Non-blocking.

2. Magic numbers for type-size and border-radius — 0.75rem, 0.875rem, 1.75rem, 0.5rem, 0.375rem, 0.25rem, 0.125rem appear inline. Same root cause: app.css apparently has --space-* but not --font-size-* or --radius-* tokens. Not this PR's job to invent the token system. Non-blocking.

3. No automated test on the route-group surface — The PR body explicitly acknowledges this ("the route layer has no unit-testable surface") and points to the existing 25/25 lib tests. Reasonable: throwing-error route loads are awkward to unit-test; the meaningful coverage is the post-merge manual AC walk-through (curl the path, inspect rendered HTML, click sign out, disable JS retest). Not a blocker per the QA agent's feedback_qa_ci_blockers rule because this isn't a "test that won't run in CI" — it's a "no-test-warranted by design" call, and the lib-layer tests covering keycloak.ts (PR #18) already exercise the auth primitives.

SOP COMPLIANCE

• Branch named 17-unauthorized-page — matches {issue-number}-{kebab-case-purpose}.
• PR body has ## Summary, ## Changes, ## Test Plan, ## Related Notes. Adds ## Funnel-Auth Review and ## Review Checklist (above-spec).
• No plan slug applicable (this is straight kanban work consuming sub-task #17 of parent #2).
• Closes #17 referenced in body.
• Test Plan documents npm run check (0/0), npm test (25/25), npm run build success, eslint and prettier clean. Pre-existing unrelated .woodpecker.yaml prettier warning noted as on main, not this PR's responsibility.
• No secrets, .env files, or credentials committed.
• No scope creep — 5 new files, all within (unauthorized)/. Out-of-bounds files explicitly listed and untouched.
• Funnel-auth review present per feedback_funnel_requires_auth.
• No Tailwind, no new dependencies per feedback_no_tailwind.

PROCESS OBSERVATIONS

• This is the closing edge of issue #2's auth subgraph. After merge, parent's checklist is complete (sub-tasks #14 → #15 → #16 → #17 all green). Clean kanban progression.
• Documentation discipline is exemplary: every non-obvious SvelteKit gotcha (the @ sigil semantics, the page-load-throw discards return value contract, why setHeaders can't set status) is captured in inline file-header comments. Future readers won't have to re-derive these from SvelteKit docs.
• The test gap on the route-group surface is honest and surfaces a generalizable platform observation: SvelteKit error-boundary routes have low unit-testable surface and benefit from a Playwright integration suite. If westside-admin accumulates more (unauthorized)-style groups, a follow-up issue to add Playwright coverage on the auth-state matrix (anon → 302, auth+role → 200, auth-no-role → 403) would be appropriate. Not for this PR.
• DORA: small, surgical, additive-only PR (196 lines, 0 deletions, 5 files, all in one route group). Low change-failure-risk. Deployment frequency: positive — this unblocks #2 closeout.

VERDICT: APPROVED

## PR #23 Review ### DOMAIN REVIEW **Tech stack:** SvelteKit (Svelte 5 runes) + TypeScript + adapter-node + pure CSS. Auth surface only — no DB, no FastAPI, no IaC. **1. Layout reset via `@` sigil — VERIFIED** `src/routes/(unauthorized)/+layout@.svelte` is named with the `@` sigil before the extension (not a bare `+layout.svelte`). This is the documented SvelteKit mechanism for opting out of root-layout component inheritance. The build artifact path mentioned in the PR body (`entries/pages/(unauthorized)/__unauthorized/_error.svelte.js`) is consistent with the route-group resolving correctly. The file imports `../../app.css` so design tokens still resolve, and uses `{@render children()}` (Svelte 5 snippet API). Wrapped in a `<main>` with sane defaults (max-width, centered, token-based padding). Correct. **2. `+layout.server.ts` data persistence — VERIFIED** `load({ locals }) => ({ user: locals.user })` exactly as required. The doc comment correctly states the SvelteKit contract: an `error()` thrown from a *page* load discards that page-load's return value, but *layout* loads survive and persist into the error boundary. This is the only mechanism that makes `$page.data.user` available in `+error.svelte`. Typed via the generated `LayoutServerLoad`. Correct. **3. HTTP 403 status — VERIFIED** `+page.server.ts` imports `error` from `@sveltejs/kit` and `throw error(403, 'Admin role required')`. SvelteKit guarantees the throw sets the response status to 403 *and* routes to the nearest `+error.svelte`. The PR body correctly notes that `setHeaders` cannot set status (header API, not status API). Typed via `PageServerLoad`. Correct. **4. Sign-out form — VERIFIED** `<form action="/auth/logout" method="POST">` with a `<button type="submit">Sign out</button>`. No `on:submit`, no `fetch`, no JS handler. Same-origin POST satisfies the Origin-header CSRF check installed by PR #21. Works without JS. Correct. **5. No Tailwind, no new dependencies — VERIFIED** No framework imports anywhere. Pure CSS in `<style>` blocks using existing `--color-*` and `--space-*` tokens from `app.css`. `package.json` is not in the diff (additions=196 deletions=0 across 5 *new* files only). **6. `+page.svelte` placeholder is intentionally inert — VERIFIED** Single HTML comment line. Never renders because the load throws first. SvelteKit requires a `+page.svelte` for the route to resolve. Correct. **Out-of-bounds files — VERIFIED** The diff is 5 new files only, all under `src/routes/(unauthorized)/`. Untouched: `src/lib/server/keycloak.ts`, `src/hooks.server.ts`, `src/routes/auth/*`, `src/routes/+layout.svelte`, `src/app.d.ts`. Strict scope discipline. **TypeScript / leakage checks — VERIFIED** - No `any` casts. Both loads use generated `$types`. - No `console.log` anywhere. - No tokens, cookies, or secret material rendered. The error UI displays only `name`, `email`, and the static error message. - Display fallback chain `name || email || 'an authenticated user'` is graceful and avoids `undefined` rendering. - `$page.error?.message ?? 'Admin role required'` defends against missing error message. **Funnel-auth posture — VERIFIED** Per `feedback_funnel_requires_auth`, the PR body contains a dedicated Funnel-Auth Review section. The audit holds: this PR is a consumer of the existing hook's role-gate decision, not a new ingress. Anonymous users cannot reach `/__unauthorized` — the hook 302s them to `/auth/login` before any route handler runs. The only way in is the hook's internal rewrite for authenticated-but-not-admin requests. No new headers, no new endpoints, no new auth flows. ### BLOCKERS None. ### NITS 1. **Hardcoded color hex in `+error.svelte`** — `border: 1px solid #e5e7eb` and `.body code { background: #f3f4f6 }` are raw hex while the rest of the file uses `--color-*` tokens. If `--color-border` and `--color-code-bg` (or similar) don't exist in `app.css` yet, this is a token-system gap rather than a dev-side miss. Worth filing a follow-up to extend the token palette so future components don't repeat the inline hex. Non-blocking. 2. **Magic numbers for type-size and border-radius** — `0.75rem`, `0.875rem`, `1.75rem`, `0.5rem`, `0.375rem`, `0.25rem`, `0.125rem` appear inline. Same root cause: `app.css` apparently has `--space-*` but not `--font-size-*` or `--radius-*` tokens. Not this PR's job to invent the token system. Non-blocking. 3. **No automated test on the route-group surface** — The PR body explicitly acknowledges this ("the route layer has no unit-testable surface") and points to the existing 25/25 lib tests. Reasonable: throwing-error route loads are awkward to unit-test; the meaningful coverage is the post-merge manual AC walk-through (curl the path, inspect rendered HTML, click sign out, disable JS retest). Not a blocker per the QA agent's `feedback_qa_ci_blockers` rule because this isn't a "test that won't run in CI" — it's a "no-test-warranted by design" call, and the lib-layer tests covering keycloak.ts (PR #18) already exercise the auth primitives. ### SOP COMPLIANCE - [x] Branch named `17-unauthorized-page` — matches `{issue-number}-{kebab-case-purpose}`. - [x] PR body has `## Summary`, `## Changes`, `## Test Plan`, `## Related Notes`. Adds `## Funnel-Auth Review` and `## Review Checklist` (above-spec). - [x] No plan slug applicable (this is straight kanban work consuming sub-task #17 of parent #2). - [x] Closes #17 referenced in body. - [x] Test Plan documents `npm run check` (0/0), `npm test` (25/25), `npm run build` success, eslint and prettier clean. Pre-existing unrelated `.woodpecker.yaml` prettier warning noted as on `main`, not this PR's responsibility. - [x] No secrets, `.env` files, or credentials committed. - [x] No scope creep — 5 new files, all within `(unauthorized)/`. Out-of-bounds files explicitly listed and untouched. - [x] Funnel-auth review present per `feedback_funnel_requires_auth`. - [x] No Tailwind, no new dependencies per `feedback_no_tailwind`. ### PROCESS OBSERVATIONS - This is the closing edge of issue #2's auth subgraph. After merge, parent's checklist is complete (sub-tasks #14 → #15 → #16 → #17 all green). Clean kanban progression. - Documentation discipline is exemplary: every non-obvious SvelteKit gotcha (the `@` sigil semantics, the page-load-throw discards return value contract, why `setHeaders` can't set status) is captured in inline file-header comments. Future readers won't have to re-derive these from SvelteKit docs. - The test gap on the route-group surface is honest and surfaces a generalizable platform observation: SvelteKit error-boundary routes have low unit-testable surface and benefit from a Playwright integration suite. If `westside-admin` accumulates more `(unauthorized)`-style groups, a follow-up issue to add Playwright coverage on the auth-state matrix (anon → 302, auth+role → 200, auth-no-role → 403) would be appropriate. Not for this PR. - DORA: small, surgical, additive-only PR (196 lines, 0 deletions, 5 files, all in one route group). Low change-failure-risk. Deployment frequency: positive — this unblocks #2 closeout. ### VERDICT: APPROVED

forgejo_admin merged commit 5073187074 into main 2026-05-03 20:28:55 +00:00

forgejo_admin deleted branch 17-unauthorized-page 2026-05-03 20:28:55 +00:00

forgejo_admin referenced this pull request from a commit 2026-05-03 20:28:56 +00:00

feat(auth): (unauthorized) 403 page (closes #17) (#23)

forgejo_admin referenced this pull request 2026-05-03 20:29:32 +00:00

Keycloak cookie SSR auth + admin role gate (hooks.server.ts) #2

forgejo_admin referenced this pull request 2026-05-03 20:52:56 +00:00

[BUG] Cookie SSR auth loops infinitely — session cookie value (4719B) exceeds browser per-cookie limit #24

forgejo_admin referenced this pull request 2026-05-03 21:37:41 +00:00

fix(auth): accept azp claim for client identity (closes #26) #27

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

forgejo_admin/westside-admin!23

No description provided.