diff --git a/packages/api/src/controllers/token.test.ts b/packages/api/src/controllers/token.test.ts index caa6926..d12850b 100644 --- a/packages/api/src/controllers/token.test.ts +++ b/packages/api/src/controllers/token.test.ts @@ -1,10 +1,12 @@ import assert from "node:assert/strict"; import { test } from "node:test"; -import { InvalidRequestError, UnauthorizedClientError } from "../errors.js"; +import { InvalidGrantError, InvalidRequestError, UnauthorizedClientError } from "../errors.js"; import { assertClientSecretMatches, + assertRefreshTokenClientBinding, buildUserIdTokenClaims, resolveGrantedScopes, + resolveSessionClientId, TokenRequestSchema, } from "./user/token.js"; @@ -93,3 +95,38 @@ test("buildUserIdTokenClaims omits nonce when not provided", () => { assert.equal(claims.nonce, undefined); }); + +test("resolveSessionClientId returns client id for valid session data", () => { + assert.equal(resolveSessionClientId({ clientId: "demo-client" }), "demo-client"); +}); + +test("resolveSessionClientId returns null when client id is missing", () => { + assert.equal(resolveSessionClientId({}), null); +}); + +test("resolveSessionClientId returns null for invalid shapes", () => { + assert.equal(resolveSessionClientId(null), null); + assert.equal(resolveSessionClientId("bad"), null); + assert.equal(resolveSessionClientId({ clientId: 42 }), null); +}); + +test("assertRefreshTokenClientBinding accepts matching client ids", () => { + assert.doesNotThrow(() => assertRefreshTokenClientBinding("demo-client", "demo-client")); +}); + +test("assertRefreshTokenClientBinding rejects mismatched client ids", () => { + assert.throws( + () => assertRefreshTokenClientBinding("demo-client", "other-client"), + (error: unknown) => + error instanceof InvalidGrantError && + error.message === "Refresh token was not issued to this client" + ); +}); + +test("assertRefreshTokenClientBinding rejects missing binding", () => { + assert.throws( + () => assertRefreshTokenClientBinding(null, "demo-client"), + (error: unknown) => + error instanceof InvalidGrantError && error.message === "Invalid or expired refresh token" + ); +}); diff --git a/packages/api/src/controllers/user/opaqueLoginFinish.ts b/packages/api/src/controllers/user/opaqueLoginFinish.ts index 5f82f73..39d98d7 100644 --- a/packages/api/src/controllers/user/opaqueLoginFinish.ts +++ b/packages/api/src/controllers/user/opaqueLoginFinish.ts @@ -144,6 +144,7 @@ export const postOpaqueLoginFinish = withRateLimit("opaque", (body) => { sub: user.sub, email: user.email || undefined, name: user.name || undefined, + clientId: "demo-public-client", otpRequired: otpRequired, otpVerified: false, }); diff --git a/packages/api/src/controllers/user/token.ts b/packages/api/src/controllers/user/token.ts index 5e64a92..a00edc9 100644 --- a/packages/api/src/controllers/user/token.ts +++ b/packages/api/src/controllers/user/token.ts @@ -4,16 +4,13 @@ import { InvalidGrantError, InvalidRequestError, UnauthorizedClientError } from import { genericErrors } from "../../http/openapi-helpers.js"; import { getCachedBody, withRateLimit } from "../../middleware/rateLimit.js"; import { signJWT } from "../../services/jwks.js"; -import { - createSession, - getActorFromRefreshToken, - refreshSessionWithToken, -} from "../../services/sessions.js"; +import { createSession, refreshSessionWithToken } from "../../services/sessions.js"; import { getSetting } from "../../services/settings.js"; import type { Context, ControllerSchema, IdTokenClaims, + SessionData, TokenRequest, TokenResponse, } from "../../types.js"; @@ -96,6 +93,23 @@ export function buildUserIdTokenClaims(data: { }; } +export function resolveSessionClientId(sessionData: unknown): string | null { + if (!sessionData || typeof sessionData !== "object") return null; + const maybeClientId = (sessionData as { clientId?: unknown }).clientId; + if (typeof maybeClientId !== "string" || maybeClientId.length === 0) return null; + return maybeClientId; +} + +export function assertRefreshTokenClientBinding( + issuedClientId: string | null, + authenticatedClientId: string | undefined +): void { + if (!issuedClientId) throw new InvalidGrantError("Invalid or expired refresh token"); + if (issuedClientId !== authenticatedClientId) { + throw new InvalidGrantError("Refresh token was not issued to this client"); + } +} + export const TokenRequestSchema = z.union([ z.object({ grant_type: z.literal("authorization_code"), @@ -187,11 +201,17 @@ export const postToken = withRateLimit("token")( clientAuthOk = true; } if (!clientAuthOk) throw new UnauthorizedClientError("Client authentication failed"); - const actor = await getActorFromRefreshToken(context, tokenRequest.refresh_token); - if (!actor || !actor.userSub) + const { sessions } = await import("../../db/schema.js"); + const { eq } = await import("drizzle-orm"); + const sessionRow = await context.db.query.sessions.findFirst({ + where: eq(sessions.refreshToken, tokenRequest.refresh_token), + }); + if (!sessionRow || !sessionRow.userSub) throw new InvalidGrantError("Invalid or expired refresh token"); + const issuedClientId = resolveSessionClientId(sessionRow.data); + assertRefreshTokenClientBinding(issuedClientId, providedClientId); const { getUserBySub } = await import("../../models/users.js"); - const user = await getUserBySub(context, actor.userSub); + const user = await getUserBySub(context, sessionRow.userSub); if (!user) throw new InvalidGrantError("User not found"); if (!providedClientId) throw new UnauthorizedClientError("Client authentication failed"); const client = await (await import("../../models/clients.js")).getClient( @@ -225,12 +245,7 @@ export const postToken = withRateLimit("token")( ? client.idTokenLifetimeSeconds : defaultIdTtl; let amr: string[] | undefined = ["pwd"]; - const { sessions } = await import("../../db/schema.js"); - const { eq } = await import("drizzle-orm"); - const sess = await context.db.query.sessions.findFirst({ - where: eq(sessions.refreshToken, tokenRequest.refresh_token), - }); - const data = (sess?.data || {}) as Record; + const data = (sessionRow.data || {}) as Record; if (data && data.otpVerified === true) amr = ["pwd", "otp"]; const issuer = await resolveIssuer(context); const idTokenClaims = buildUserIdTokenClaims({ @@ -527,7 +542,8 @@ export const postToken = withRateLimit("token")( sub: user.sub, email: user.email || undefined, name: user.name || undefined, - }; + clientId: authenticatedClientId, + } satisfies SessionData; const s = await createSession(context, "user", sessionData); tokenResponse.refresh_token = s.refreshToken; diff --git a/packages/api/src/models/registration.ts b/packages/api/src/models/registration.ts index f96d3f7..b41c147 100644 --- a/packages/api/src/models/registration.ts +++ b/packages/api/src/models/registration.ts @@ -46,6 +46,7 @@ export async function userOpaqueRegisterFinish( sub, email: data.email, name: data.name, + clientId: "demo-public-client", }); return { sub, accessToken: sessionInfo.sessionId, refreshToken: sessionInfo.refreshToken }; } diff --git a/packages/api/src/types.ts b/packages/api/src/types.ts index fe32075..ae9f0fa 100644 --- a/packages/api/src/types.ts +++ b/packages/api/src/types.ts @@ -318,6 +318,7 @@ export interface SessionData { sub?: string; email?: string; name?: string; + clientId?: string; adminId?: string; adminRole?: "read" | "write"; pendingAuthId?: string; diff --git a/packages/brochureware/public/whitepaper.generated.md b/packages/brochureware/public/whitepaper.generated.md index 356041a..cd01528 100644 --- a/packages/brochureware/public/whitepaper.generated.md +++ b/packages/brochureware/public/whitepaper.generated.md @@ -149,7 +149,7 @@ Server stores - Pending authorization requests; authorization codes with `has_zk`, `zk_pub_kid`, and `drk_hash`. - JWKS (public JWKs; private JWKs encrypted at rest when KEK is available). - Clients, settings (including runtime flags for OIDC/PKCE, id token TTLs, etc.). -- Sessions and refresh tokens. +- Sessions and refresh tokens, including issuing `client_id` binding for refresh token ownership. - Audit logs (admin actions). Server never stores @@ -159,6 +159,7 @@ Server never stores Sessions and claims - SPA model: `/session` for minimal info; `/refresh-token` rotates session tokens. +- OIDC refresh grant enforces client binding: cross-client refresh token reuse is rejected. - ID tokens may include `permissions` and `groups` claims when configured. ## 6) Endpoints (as implemented) diff --git a/specs/0_SECURITY_WHITEPAPER.md b/specs/0_SECURITY_WHITEPAPER.md index 31fd5c9..70b978f 100644 --- a/specs/0_SECURITY_WHITEPAPER.md +++ b/specs/0_SECURITY_WHITEPAPER.md @@ -163,7 +163,7 @@ Server stores - Pending authorization requests; authorization codes with `has_zk`, `zk_pub_kid`, and `drk_hash`. - JWKS (public JWKs; private JWKs encrypted at rest when KEK is available). - Clients, settings (including runtime flags for OIDC/PKCE, id token TTLs, etc.). -- Sessions and refresh tokens. +- Sessions and refresh tokens, including issuing `client_id` binding for refresh token ownership. - Audit logs (admin actions). Server never stores @@ -173,6 +173,7 @@ Server never stores Sessions and claims - SPA model: `/session` for minimal info; `/refresh-token` rotates session tokens. +- OIDC refresh grant enforces client binding: cross-client refresh token reuse is rejected. - ID tokens may include `permissions` and `groups` claims when configured. ## 6) Endpoints (as implemented) diff --git a/specs/2_CORE.md b/specs/2_CORE.md index 4a230da..8f07c5e 100644 --- a/specs/2_CORE.md +++ b/specs/2_CORE.md @@ -224,7 +224,8 @@ Same as above **+** `&zk_pub=` (ephemeral ECDH public key). - Validates code (unexpired, not consumed), PKCE (S256), client, `redirect_uri`. - Mints `id_token` (and `access_token` if enabled by settings). -- Returns a `refresh_token` bound to the session. +- Returns a `refresh_token` bound to the session and issuing `client_id`. +- Refresh grant enforces `client_id` binding: only the original client can rotate that refresh token. - Optionally includes user authorization data as custom claims when configured: `permissions` (array of strings) and `groups` (array of strings). These reflect the union of direct user permissions and permissions derived from groups. - **SECURITY**: If the code had `has_zk=true`, include ONLY `zk_drk_hash` for verification. Server NEVER returns `zk_drk_jwe` as it doesn't store it. - Consume the code.