fix(auth): harden the OIDC token path and state-cookie lifecycle

Token verification: jwtVerify now carries a closed asymmetric algorithm
allowlist (RS256/PS256/ES256/EdDSA — an HMAC alg would let anyone with
the client secret forge identities) and a 60 s clock tolerance for the
usual self-host drift between IdP and app box. The JWKS fetch — jose's
own, outside safeFetch by design — is disciplined instead by pinning
the jwks_uri host to the configured issuer's host byte-exact and
passing the same 15 s timeout safeFetch defaults to. requirePublicHost
stays deliberately unused on this surface: LAN/Tailscale-only IdPs are
a legitimate topology and the issuer is operator config, not user
input.

Discovery: the issuer comparison now tolerates exactly one trailing
slash on either side (a recurring IdP/operator mismatch); token
verification still matches the provider-sent issuer byte-exact.

State cookie: the cookie is set under Path=/api/auth/oidc but was
deleted without a path — RFC 6265 keys cookies by (name, domain,
path), so the deletion never matched and the single-use blob survived
its whole TTL. Every delete now repeats the exact path, via a shared
constant.

Also: the IdP error code recorded in the wide event is whitelisted
against the closed OAuth error-code set (unknown values collapse to
"other"); the status route drops an unreachable try/catch and a
duplicated annotate; the passkey conditional-UI autofill stays unarmed
under OIDC_ONLY (passkey login is a server-enforced dead end there);
env-manifest gains the three optional OIDC_* tuning vars; the env
example notes why the issuer should be HTTPS.
This commit is contained in:
Marc-André Bombeck committed 2026-07-14 11:53:53 +02:00
1 parent 9b8b31c864
commit 7592c73947
10 files changed
+252 -32

No files matched your search

+4
View File
@@ -119,6 +119,10 @@ API_TOKEN_HMAC_KEY="<openssl rand -hex 32>"
# flow for it yet -- so OIDC_ONLY="true" locks the iOS app out of sign-in
# entirely until a native SSO flow ships. Do not enable it on a deployment
# with active iOS users.
# Serve the issuer over HTTPS wherever possible: sign-in material (the
# authorization code, the token-endpoint response carrying the ID token)
# crosses this connection. Plain HTTP is defensible only on a fully trusted
# private network (IdP and app on the same box, a Tailscale mesh, a VPN).
# OIDC_ISSUER_URL=""
# OIDC_CLIENT_ID=""
# OIDC_CLIENT_SECRET=""
+19
View File
@@ -98,6 +98,25 @@
}
]
},
{
"name": "OIDC SSO options",
"description": "Optional tuning for an enabled OIDC provider. Each stands alone (deliberately NOT allOrNone — any subset is valid) and each has a safe default when unset.",
"required": false,
"variables": [
{
"name": "OIDC_SCOPES",
"purpose": "Space-separated scopes requested at authorization. Default: \"openid email profile\"."
},
{
"name": "OIDC_BUTTON_LABEL",
"purpose": "Label on the login page's SSO button. Default: \"Single Sign-On\"."
},
{
"name": "OIDC_ONLY",
"purpose": "\"true\" disables password/passkey login server-side (only effective when the OIDC provider group is fully configured). Default: additive login. Locks the native iOS app out of sign-in."
}
]
},
{
"name": "APNs (iOS push)",
"description": "All four core APNS_* vars together. Missing any disables APNs silently — the v1.4.40 AP-2 gap that motivated this manifest. allOrNone trips the synthetic <all-or-none> required row whenever 1-3 of the 4 are set.",
@@ -459,4 +459,25 @@ describe("GET /api/auth/oidc/callback", () => {
const res = await GET(validRequest());
expect(res.headers.get("location")).toContain("error=oidc_failed");
});
it("deletes the state cookie with the exact path it was set under", async () => {
// RFC 6265 keys cookies by (name, domain, path) — a deletion without
// `Path=/api/auth/oidc` would target a `/`-scoped cookie that does not
// exist and leave the real single-use blob alive.
const errorRes = await GET(makeRequest({ query: "?error=access_denied" }));
const errorCookie = errorRes.headers
.getSetCookie()
.find((c) => c.startsWith("oidc_auth_state="))!;
expect(errorCookie.toLowerCase()).toContain("path=/api/auth/oidc");
mockIdentity();
mockUserLookups({
byIdentity: userRow({ oidcIssuer: METADATA.issuer, oidcSub: "sub-1" }),
});
const successRes = await GET(validRequest());
const successCookie = successRes.headers
.getSetCookie()
.find((c) => c.startsWith("oidc_auth_state="))!;
expect(successCookie.toLowerCase()).toContain("path=/api/auth/oidc");
});
});
+47 -4
View File
@@ -22,6 +22,7 @@ import {
OIDC_MFA_COOKIE_PATH,
OIDC_MFA_TTL_MS,
OIDC_STATE_COOKIE,
OIDC_STATE_COOKIE_PATH,
} from "@/lib/auth/oidc-cookie";
import { createMfaChallenge } from "@/lib/auth/mfa/challenge";
import { syncMfaEnrollCookie } from "@/lib/auth/mfa-enrollment";
@@ -34,10 +35,47 @@ function errorRedirect(req: NextRequest, reason: string): NextResponse {
const response = NextResponse.redirect(
new URL(`${LOGIN_ERROR_URL}?error=${reason}`, req.url),
);
response.cookies.delete(OIDC_STATE_COOKIE);
deleteStateCookie(response);
return response;
}
/**
* The state cookie is set with `path: "/api/auth/oidc"`; the delete must
* repeat that path (RFC 6265 keys cookies by name+domain+path — a bare
* delete would target `/` and never match, leaving the single-use blob
* alive for the rest of its TTL).
*/
function deleteStateCookie(response: NextResponse): void {
response.cookies.delete({
name: OIDC_STATE_COOKIE,
path: OIDC_STATE_COOKIE_PATH,
});
}
/**
* Closed set of RFC 6749 §4.1.2.1 / OIDC Core §3.1.2.6 authorization error
* codes. The wide-event meta only ever carries a member of this set (or
* "other") — never the raw, attacker-influenceable query value.
*/
const KNOWN_IDP_ERROR_CODES = new Set([
"access_denied",
"invalid_request",
"unauthorized_client",
"unsupported_response_type",
"invalid_scope",
"server_error",
"temporarily_unavailable",
"interaction_required",
"login_required",
"account_selection_required",
"consent_required",
"invalid_request_uri",
"invalid_request_object",
"request_not_supported",
"request_uri_not_supported",
"registration_not_supported",
]);
interface StoredOidcState {
state: string;
nonce: string;
@@ -82,7 +120,12 @@ export const GET = apiHandler(async (req: NextRequest) => {
const { searchParams } = req.nextUrl;
const idpError = searchParams.get("error");
if (idpError) {
annotate({ meta: { reason: "idp_denied", idpError } });
annotate({
meta: {
reason: "idp_denied",
idpError: KNOWN_IDP_ERROR_CODES.has(idpError) ? idpError : "other",
},
});
return errorRedirect(req, "oidc_denied");
}
@@ -299,7 +342,7 @@ export const GET = apiHandler(async (req: NextRequest) => {
loginUrl.searchParams.set("next", stored.next);
}
const response = NextResponse.redirect(loginUrl);
response.cookies.delete(OIDC_STATE_COOKIE);
deleteStateCookie(response);
response.cookies.set(
OIDC_MFA_COOKIE,
JSON.stringify({ ticket: challenge.ticket, methods }),
@@ -341,7 +384,7 @@ export const GET = apiHandler(async (req: NextRequest) => {
await auditLog("auth.oidc.login", { userId: user.id, ipAddress: ip });
const response = NextResponse.redirect(new URL(stored.next, req.url));
response.cookies.delete(OIDC_STATE_COOKIE);
deleteStateCookie(response);
return response;
} catch (err) {
getEvent()?.setError(err);
+8 -2
View File
@@ -13,7 +13,11 @@ import {
getOidcRedirectUri,
sanitizeOidcNextPath,
} from "@/lib/auth/oidc";
import { OIDC_STATE_COOKIE, OIDC_STATE_TTL_MS } from "@/lib/auth/oidc-cookie";
import {
OIDC_STATE_COOKIE,
OIDC_STATE_COOKIE_PATH,
OIDC_STATE_TTL_MS,
} from "@/lib/auth/oidc-cookie";
/**
* Redirects the browser to the configured OIDC provider's authorization
@@ -84,7 +88,9 @@ export const GET = apiHandler(async (req: NextRequest) => {
// the session cookie and the Withings OAuth state cookie.
sameSite: "lax",
maxAge: Math.floor(OIDC_STATE_TTL_MS / 1000),
path: "/api/auth/oidc",
// Shared constant — the callback's delete must repeat this exact path
// (RFC 6265 keys cookies by name+domain+path).
path: OIDC_STATE_COOKIE_PATH,
});
return response;
+9 -14
View File
@@ -6,19 +6,14 @@ import { getOidcConfig, isOidcOnly } from "@/lib/auth/oidc";
export const dynamic = "force-dynamic";
/** Public — the login page reads this pre-session to decide whether to
* render the SSO button and whether to hide password/passkey login. */
* render the SSO button and whether to hide password/passkey login. Pure
* env reads; nothing here can throw. */
export const GET = apiHandler(async () => {
try {
const config = getOidcConfig();
annotate({ action: { name: "auth.oidc.status" } });
return apiSuccess({
enabled: config !== null,
buttonLabel: config?.buttonLabel ?? null,
only: isOidcOnly(),
});
} catch {
// Fail closed, matching registration-status.
annotate({ action: { name: "auth.oidc.status" } });
return apiSuccess({ enabled: false, buttonLabel: null, only: false });
}
annotate({ action: { name: "auth.oidc.status" } });
const config = getOidcConfig();
return apiSuccess({
enabled: config !== null,
buttonLabel: config?.buttonLabel ?? null,
only: isOidcOnly(),
});
});
+11 -1
View File
@@ -238,6 +238,11 @@ export default function LoginPage() {
methods?: MfaMethod[];
};
if (parsed.ticket) {
// A lazy useState initializer cannot replace this: `document` does
// not exist during the server render, and seeding client-only state
// there would desync hydration. Mount-time pickup of an external
// one-shot store is exactly the effect escape hatch.
// eslint-disable-next-line react-hooks/set-state-in-effect
setMfaChallenge({
ticket: parsed.ticket,
methods: parsed.methods ?? ["totp", "recovery"],
@@ -252,7 +257,12 @@ export default function LoginPage() {
// arm a discoverable-credential assertion bound to the username field so a
// returning user can pick their passkey straight from the autofill prompt.
// Best-effort: any abort (the user types a password instead) is swallowed.
// Waits for the OIDC status and stays off entirely under OIDC_ONLY —
// passkey login is a dead end there (server-enforced), so arming the
// browser's credential prompt would only dangle a door that is locked.
useEffect(() => {
if (oidcStatus === undefined) return;
if (oidcStatus.only === true) return;
if (autofillStarted.current) return;
autofillStarted.current = true;
let cancelled = false;
@@ -291,7 +301,7 @@ export default function LoginPage() {
cancelled = true;
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
}, [oidcStatus]);
return (
<div className="flex w-full max-w-sm flex-col gap-4">
+82 -1
View File
@@ -7,7 +7,10 @@ vi.mock("jose", () => ({
}));
import { safeFetch } from "@/lib/safe-fetch";
import { jwtVerify as mockJwtVerify } from "jose";
import {
createRemoteJWKSet as mockCreateRemoteJWKSet,
jwtVerify as mockJwtVerify,
} from "jose";
import {
_resetOidcCacheForTests,
buildAuthorizationUrl,
@@ -195,6 +198,28 @@ describe("discoverOidcMetadata", () => {
);
});
it("tolerates exactly one trailing slash between config and discovery issuer", async () => {
// Several IdPs mint their issuer with a canonical trailing slash while
// the operator configures without one. The provider-sent form is kept
// in the metadata — the ID token's `iss` must match IT byte-exact.
vi.mocked(safeFetch).mockResolvedValue(
jsonResponse({ ...METADATA_DOC, issuer: "https://idp.example.com/" }),
);
const config = getOidcConfig()!;
const metadata = await discoverOidcMetadata(config);
expect(metadata.issuer).toBe("https://idp.example.com/");
});
it("does not stretch the slash tolerance beyond a single trailing slash", async () => {
vi.mocked(safeFetch).mockResolvedValue(
jsonResponse({ ...METADATA_DOC, issuer: "https://idp.example.com//" }),
);
const config = getOidcConfig()!;
await expect(discoverOidcMetadata(config)).rejects.toThrow(
/issuer mismatch/,
);
});
it("rejects a discovery doc missing required fields", async () => {
vi.mocked(safeFetch).mockResolvedValue(
jsonResponse({ issuer: METADATA_DOC.issuer }),
@@ -344,6 +369,62 @@ describe("verifyIdToken", () => {
}),
).rejects.toThrow(/missing sub claim/);
});
it("verifies with a closed asymmetric algorithm allowlist and bounded clock tolerance", async () => {
vi.mocked(mockJwtVerify).mockResolvedValue({
payload: { sub: "user-sub-1", nonce: "nonce-1" },
} as never);
const config = getOidcConfig()!;
await verifyIdToken({
metadata: METADATA_DOC,
config,
idToken: "id-token",
nonce: "nonce-1",
});
expect(mockJwtVerify).toHaveBeenCalledWith(
"id-token",
expect.anything(),
expect.objectContaining({
issuer: METADATA_DOC.issuer,
audience: config.clientId,
algorithms: ["RS256", "PS256", "ES256", "EdDSA"],
clockTolerance: "60s",
}),
);
});
it("pins the JWKS fetch to the issuer's host with a bounded timeout", async () => {
vi.mocked(mockJwtVerify).mockResolvedValue({
payload: { sub: "user-sub-1", nonce: "nonce-1" },
} as never);
const config = getOidcConfig()!;
await verifyIdToken({
metadata: METADATA_DOC,
config,
idToken: "id-token",
nonce: "nonce-1",
});
expect(mockCreateRemoteJWKSet).toHaveBeenCalledWith(
new URL(METADATA_DOC.jwks_uri),
{ timeoutDuration: 15_000 },
);
});
it("rejects a discovery doc whose jwks_uri points at a foreign host", async () => {
const config = getOidcConfig()!;
await expect(
verifyIdToken({
metadata: {
...METADATA_DOC,
jwks_uri: "https://evil.example.com/jwks",
},
config,
idToken: "id-token",
nonce: "nonce-1",
}),
).rejects.toThrow(/jwks_uri host mismatch/);
expect(mockCreateRemoteJWKSet).not.toHaveBeenCalled();
});
});
describe("fetchUserinfoEmail", () => {
+7
View File
@@ -4,6 +4,13 @@
* codeVerifier, next}` across the browser round-trip to the IdP and back.
*/
export const OIDC_STATE_COOKIE = "oidc_auth_state";
/**
* RFC 6265 keys a cookie by (name, domain, path) — a delete must repeat the
* exact path the set used or it silently targets a different (non-existent)
* cookie and the stale state blob survives. Every set AND delete goes
* through this constant.
*/
export const OIDC_STATE_COOKIE_PATH = "/api/auth/oidc";
export const OIDC_STATE_TTL_MS = 10 * 60 * 1000;
/**
+44 -10
View File
@@ -7,6 +7,11 @@
* OAuth server) rather than pulling in a full OAuth/OIDC framework; `jose`
* is used only for JWKS fetch + ID-token signature/claims verification,
* which is not worth hand-rolling.
*
* None of the outbound calls opt into `requirePublicHost`: a LAN- or
* Tailscale-only IdP (Authentik/Keycloak/Authelia on the same box) is a
* legitimate, common self-host topology, and the issuer comes from operator
* env config — never from user input.
*/
import { createRemoteJWKSet, jwtVerify } from "jose";
import { safeFetch } from "@/lib/safe-fetch";
@@ -117,7 +122,13 @@ function isRecord(value: unknown): value is Record<string, unknown> {
* logins don't stampede the discovery endpoint. The doc's own `issuer`
* claim must match the configured issuer URL (RFC 8414 §3.3) — a
* mismatch means the provider is misconfigured or something is spoofing
* the discovery response.
* the discovery response. The comparison tolerates exactly ONE trailing
* slash on either side (several IdPs mint their issuer with a canonical
* trailing slash while operators configure without one, and vice versa);
* anything beyond that single-slash normalisation is still a hard reject.
* The provider-sent `issuer` is what gets cached and later matched against
* the ID token's `iss` claim byte-exact — the tolerance applies only to
* the operator-config comparison, never to token verification.
*/
export async function discoverOidcMetadata(
config: OidcConfig,
@@ -147,7 +158,12 @@ export async function discoverOidcMetadata(
) {
throw new Error("discovery document missing required fields");
}
if (json.issuer !== config.issuerUrl) {
const stripOneTrailingSlash = (value: string): string =>
value.endsWith("/") ? value.slice(0, -1) : value;
if (
stripOneTrailingSlash(json.issuer) !==
stripOneTrailingSlash(config.issuerUrl)
) {
throw new Error(
`discovery issuer mismatch: expected ${config.issuerUrl}, got ${json.issuer}`,
);
@@ -172,16 +188,25 @@ export async function discoverOidcMetadata(
return metadataInFlight;
}
function getJwks(metadata: OidcMetadata) {
function getJwks(metadata: OidcMetadata, config: OidcConfig) {
if (jwksCache && jwksCache.issuerUrl === metadata.issuer) {
return jwksCache.jwks;
}
// `jose`'s own fetch to the trusted, discovery-supplied `jwks_uri` is a
// deliberate, scoped exception to the safe-fetch-required convention —
// the target host comes from the operator-configured issuer, never from
// user input, and the ESLint rule only lints project source, not
// library internals.
const jwks = createRemoteJWKSet(new URL(metadata.jwks_uri));
// `jose`'s own fetch to the discovery-supplied `jwks_uri` is a
// deliberate, scoped exception to the safe-fetch-required convention
// (the ESLint rule only lints project source, not library internals).
// It is disciplined two ways instead: the `jwks_uri` host must exactly
// equal the operator-configured issuer's host — a spoofed or sloppy
// discovery doc cannot point key fetching at a third party — and the
// fetch carries the same 15 s timeout `safeFetch` defaults to.
const jwksUrl = new URL(metadata.jwks_uri);
const issuerHost = new URL(config.issuerUrl).host;
if (jwksUrl.host !== issuerHost) {
throw new Error(
`jwks_uri host mismatch: expected ${issuerHost}, got ${jwksUrl.host}`,
);
}
const jwks = createRemoteJWKSet(jwksUrl, { timeoutDuration: 15_000 });
jwksCache = { issuerUrl: metadata.issuer, jwks };
return jwks;
}
@@ -268,10 +293,19 @@ export async function verifyIdToken(params: {
idToken: string;
nonce: string;
}): Promise<OidcIdentity> {
const jwks = getJwks(params.metadata);
const jwks = getJwks(params.metadata, params.config);
const { payload } = await jwtVerify(params.idToken, jwks, {
issuer: params.metadata.issuer,
audience: params.config.clientId,
// Closed allowlist of asymmetric signature algorithms — every ID token
// must be provider-key signed. Never extend this with an HMAC (HS*)
// entry: a symmetric alg would let anyone who knows the client secret
// forge identities.
algorithms: ["RS256", "PS256", "ES256", "EdDSA"],
// Self-hosted IdP + app on different boxes routinely drift a few
// seconds apart; a minute of tolerance on exp/iat/nbf absorbs that
// without meaningfully extending a token's life.
clockTolerance: "60s",
});
if (typeof payload.sub !== "string" || !payload.sub) {