Redirect an anonymous hit on /settings to sign-in, not a 500 #191

Merged
lilleman merged 7 commits from item-20 into main 2026-10-31 09:18:48 +01:00
3 changed files with 9 additions and 13 deletions
Showing only changes of commit 943520566e - Show all commits
+1 -1
View File
@@ -1506,7 +1506,7 @@ no Node/browsers on the host. Five suites, each on its own stack:
| `visual.spec.ts` + `language.spec.ts` | `e2e-tests/compose.visual.yml` | Ory-free: the rendered design system (app shell, theme switch, mobile off-canvas, icon sprite, CSRF-guarded sign-out, landing, 404), plugin permission-gating, and [language switching](#languages-i18n) | | `visual.spec.ts` + `language.spec.ts` | `e2e-tests/compose.visual.yml` | Ory-free: the rendered design system (app shell, theme switch, mobile off-canvas, icon sprite, CSRF-guarded sign-out, landing, 404), plugin permission-gating, and [language switching](#languages-i18n) |
| `auth-refresh.spec.ts` | `e2e-tests/compose.auth.yml` | The real Ory stack with the session→JWT TTL cut to 8s: a lapsed JWT is silently re-minted from the live Kratos session, and once revoked the stale cookie is cleared | | `auth-refresh.spec.ts` | `e2e-tests/compose.auth.yml` | The real Ory stack with the session→JWT TTL cut to 8s: a lapsed JWT is silently re-minted from the live Kratos session, and once revoked the stale cookie is cleared |
| `oauth-login.spec.ts` | `e2e-tests/compose.oauth.yml` | Another app logging in through us — register a client, start an authorization flow, `/oauth2/login` accept, `/oauth2/consent` Allow → authorization code | | `oauth-login.spec.ts` | `e2e-tests/compose.oauth.yml` | Another app logging in through us — register a client, start an authorization flow, `/oauth2/login` accept, `/oauth2/consent` Allow → authorization code |
| `full-flow.spec.ts` | `e2e-tests/compose.full.yml` | The browser UI: password + mocked-SSO login, menu filtering by permission, admin users/groups/permissions CRUD, OAuth2-clients register → one-time secret → delete, a gated plugin page, an anonymous `/settings` visit signing in first, logout | | `full-flow.spec.ts` | `e2e-tests/compose.full.yml` | The browser UI: password + mocked-SSO login, menu filtering by permission, admin users/groups/permissions CRUD, OAuth2-clients register → one-time secret → delete, a gated plugin page, logout |
| `devstack-login.spec.ts` | `compose.override.yml` + `e2e-tests/compose.devstack.yml` | The plain `docker compose up` topology on the **host network**: sign-in works both from the banner's `localhost:3000` and from `127.0.0.1:3000` via the [canonical-host redirect](#canonical-host-one-public-url) | | `devstack-login.spec.ts` | `compose.override.yml` + `e2e-tests/compose.devstack.yml` | The plain `docker compose up` topology on the **host network**: sign-in works both from the banner's `localhost:3000` and from `127.0.0.1:3000` via the [canonical-host redirect](#canonical-host-one-public-url) |
```bash ```bash
-2
View File
@@ -242,8 +242,6 @@ test("return_to: a deep link while logged out returns to that page after login",
test("settings: an anonymous visit signs in first, then lands on the settings page", async ({ page }) => { test("settings: an anonymous visit signs in first, then lands on the settings page", async ({ page }) => {
test.setTimeout(90_000); test.setTimeout(90_000);
// /settings is public (recovery lands there with a Kratos session but no app JWT), so Kratos' 401
// for an anonymous settings flow must send the visitor to sign in, not to the 500 page.
await page.goto("/settings"); await page.goto("/settings");
await expect(page).toHaveURL(/\/login(\?|$)/); await expect(page).toHaveURL(/\/login(\?|$)/);
await page.fill('input[name="identifier"]', ADMIN_EMAIL); await page.fill('input[name="identifier"]', ADMIN_EMAIL);
+8 -10
View File
@@ -38,13 +38,10 @@ const FORBIDDEN: RouteResult = { status: 403, view: "403" };
const selfOrigin = (ctx: RequestContext, secure: boolean): string => const selfOrigin = (ctx: RequestContext, secure: boolean): string =>
`${secure ? "https" : "http"}://${ctx.req.headers.host ?? "127.0.0.1:3000"}`; `${secure ? "https" : "http"}://${ctx.req.headers.host ?? "127.0.0.1:3000"}`;
// Where Kratos lands the browser after a freshly initialised flow, or undefined for its default // Where Kratos lands the browser after sign-in, or undefined for its default. A host-relative
// completion route. A first-party deep link (host-relative, from the gate's return_to) is wrapped // return_to (the gate's deep link) or a chosen language goes through /auth/complete, which mints the
// through /auth/complete so the session JWT is minted before the user reaches the page; an absolute // session JWT first and carries the language across Kratos; any other target (the OAuth2 login
// target (the OAuth2 login challenge) is passed as-is — Kratos allow-lists it. localPath rejects an // challenge) is passed as-is, since Kratos allow-lists it.
// off-origin "//evil.com". The target is the host's, not Kratos', so the language the visitor picked
// on the sign-in page survives the round-trip and lands on the page after it; without this the
// most-travelled path in the product (pick Swedish → sign in) drops back to Accept-Language.
function flowReturnTo(ctx: RequestContext, secureCookies: boolean): string | undefined { function flowReturnTo(ctx: RequestContext, secureCookies: boolean): string | undefined {
const raw = ctx.url.searchParams.get("return_to"); const raw = ctx.url.searchParams.get("return_to");
const local = localPath(raw); const local = localPath(raw);
@@ -56,13 +53,14 @@ function flowReturnTo(ctx: RequestContext, secureCookies: boolean): string | und
return complete.toString(); return complete.toString();
} }
// What a failed Kratos call renders instead of the flow; rethrows what is genuinely unexpected. // What a failed Kratos call returns instead of the flow; rethrows what is genuinely unexpected.
function flowFailure(err: unknown, ctx: RequestContext, flowType: FlowType): RouteResult { function flowFailure(err: unknown, ctx: RequestContext, flowType: FlowType): RouteResult {
const pathname = ctx.url.pathname; const pathname = ctx.url.pathname;
// Expired/unknown flow → restart by re-initialising (drop the stale ?flow=). // Expired/unknown flow → restart by re-initialising (drop the stale ?flow=).
if (err instanceof KratosError && [403, 404, 410].includes(err.status)) return { redirect: pathname }; if (err instanceof KratosError && [403, 404, 410].includes(err.status)) return { redirect: pathname };
// No Kratos session on a page that needs one (/settings is public, so the gate never sent them // No Kratos session on a page that needs one. /settings is public, since recovery lands there with
// to sign in): sign in, then come back. Not for /login itself, which would redirect to itself. // a Kratos session but no app JWT, so the gate never sends an anonymous visitor to sign in.
// Not for /login itself, which would redirect to itself.
if (err instanceof KratosError && err.status === 401 && flowType !== "login") { if (err instanceof KratosError && err.status === 401 && flowType !== "login") {
return { redirect: `/login?return_to=${encodeURIComponent(pathname)}` }; return { redirect: `/login?return_to=${encodeURIComponent(pathname)}` };
} }