diff --git a/src/content/docs.md b/src/content/docs.md index 3be7ade..2bc9313 100644 --- a/src/content/docs.md +++ b/src/content/docs.md @@ -427,6 +427,7 @@ Success response (same as authorization_code grant): > - Codes expire after 10 minutes > - The device code is single-use — deleted after successful token exchange > - No PKCE required (the device code itself is high-entropy proof) +> - Confidential clients (anything with a `client_secret`) must send it on **both** `/auth/device` and the token poll > - User codes use unambiguous characters (no 0/O, 1/l/I confusion) > - The `verification_uri_complete` can be shown as a QR code @@ -576,6 +577,17 @@ Content-Type: application/json } ``` +`grant_types` is optional and defaults to `["authorization_code", "refresh_token"]`. Supported values are `authorization_code`, `refresh_token`, and `urn:ietf:params:oauth:grant-type:device_code`. Indiko stores what you register and refuses any other grant at the token endpoint, so ask for what you'll actually use. + +`redirect_uris` is required only when you register `authorization_code`. A device-only client has nowhere to redirect, so it can leave the field out entirely rather than inventing a placeholder: + +```json +{ + "client_name": "My CLI", + "grant_types": ["urn:ietf:params:oauth:grant-type:device_code", "refresh_token"] +} +``` + Response `201 Created`: ```json diff --git a/src/lib/oauth/client-auth.ts b/src/lib/oauth/client-auth.ts new file mode 100644 index 0000000..1413171 --- /dev/null +++ b/src/lib/oauth/client-auth.ts @@ -0,0 +1,93 @@ +import { db } from "../../db"; +import { verifySecret } from "../secrets"; +import { oauthError, unauthorizedResponse } from "./errors"; + +// Grants indiko can issue tokens for. A DCR client registers a subset. +export const SUPPORTED_GRANT_TYPES = [ + "authorization_code", + "refresh_token", + "urn:ietf:params:oauth:grant-type:device_code", +] as const; + +export const DEVICE_GRANT_TYPE = "urn:ietf:params:oauth:grant-type:device_code"; + +/** + * Verify pre-registered client credentials; returns a Response on failure. + * + * RFC 6749 §3.2.1: a client that was issued credentials MUST authenticate at + * the token endpoint. RFC 8628 §3.1 and §3.4 carry that requirement to the + * device authorization request and to device_code polling, so this applies to + * every grant, not just authorization_code. Auto-registered (URL) clients are + * public and have nothing to prove. + */ +export function verifyClientCredentials( + client_id: string | undefined, + client_secret: string | undefined, +): Response | null { + // No client_id means we can't look up the app; treat as public/unknown. + if (!client_id) { + return null; + } + + const app = db + .query( + "SELECT is_preregistered, client_secret_hash FROM apps WHERE client_id = ?", + ) + .get(client_id) as + | { is_preregistered: number; client_secret_hash: string | null } + | undefined; + + // client_secret sent for an unknown client + if (client_secret && !app) { + return oauthError(400, "invalid_client", "Unknown client"); + } + + if (app?.is_preregistered !== 1) { + return null; // public client, nothing to check + } + + if (!client_secret) { + return unauthorizedResponse( + "invalid_client", + "client_secret is required for pre-registered clients", + ); + } + + if (!app.client_secret_hash) { + return oauthError(500, "server_error", "Client secret not configured"); + } + + if (!verifySecret(client_secret, app.client_secret_hash)) { + return unauthorizedResponse("invalid_client", "Invalid client_secret"); + } + + return null; +} + +/** + * Enforce the grant types a client registered for (RFC 7591 §2). + * + * Only DCR clients carry a registered list; NULL means unrestricted, which is + * every auto-registered URL client and every app that predates the column. + */ +export function verifyGrantAllowed( + client_id: string | undefined, + grant: string, +): Response | null { + if (!client_id) return null; + + const app = db + .query("SELECT grant_types FROM apps WHERE client_id = ?") + .get(client_id) as { grant_types: string | null } | undefined; + + if (!app?.grant_types) return null; + + const allowed = JSON.parse(app.grant_types) as string[]; + if (allowed.includes(grant)) return null; + + return oauthError( + 400, + "unauthorized_client", + `Client is not registered for grant type ${grant}`, + ); +} diff --git a/src/migrations/014_add_app_grant_types.sql b/src/migrations/014_add_app_grant_types.sql new file mode 100644 index 0000000..4b7a31a --- /dev/null +++ b/src/migrations/014_add_app_grant_types.sql @@ -0,0 +1,8 @@ +-- Grant types a client registered for (RFC 7591 §2), as a JSON array. +-- Only clients registered through /oauth/register declare one. NULL means +-- unrestricted: every auto-registered URL client, and every app that predates +-- this column, keeps working exactly as before. +-- Storing it is what lets registration require redirect_uris only for clients +-- that actually redirect, and lets the token endpoint refuse a grant the client +-- never registered for. +ALTER TABLE apps ADD COLUMN grant_types TEXT DEFAULT NULL; diff --git a/src/routes/oauth/device.ts b/src/routes/oauth/device.ts index 118597e..d185836 100644 --- a/src/routes/oauth/device.ts +++ b/src/routes/oauth/device.ts @@ -1,6 +1,11 @@ import crypto from "node:crypto"; import { db } from "../../db"; import { getClientIp } from "../../lib/client-ip"; +import { + DEVICE_GRANT_TYPE, + verifyClientCredentials, + verifyGrantAllowed, +} from "../../lib/oauth/client-auth"; import { ensureApp } from "../../lib/oauth/client-metadata"; import { NO_STORE_HEADERS, @@ -98,6 +103,17 @@ export async function deviceAuthorization(req: Request): Promise { return oauthError(400, "invalid_request", "Invalid client_id URL format"); } + // RFC 8628 §3.1: the device authorization request carries the same client + // authentication requirements as the token endpoint. + const credentialError = verifyClientCredentials( + clientId, + body.client_secret, + ); + if (credentialError) return credentialError; + + const grantError = verifyGrantAllowed(clientId, DEVICE_GRANT_TYPE); + if (grantError) return grantError; + // Auto-register the client if not already known (same as authorization flow). // Device flow has no redirect_uri, so pass a same-origin placeholder. const appResult = await ensureApp(clientId, `${clientId}callback`); diff --git a/src/routes/oauth/register.ts b/src/routes/oauth/register.ts index 6697b82..f5cdd56 100644 --- a/src/routes/oauth/register.ts +++ b/src/routes/oauth/register.ts @@ -1,6 +1,7 @@ import { nanoid } from "nanoid"; import { db } from "../../db"; import { getClientIp } from "../../lib/client-ip"; +import { SUPPORTED_GRANT_TYPES } from "../../lib/oauth/client-auth"; import { NO_STORE_HEADERS, oauthError } from "../../lib/oauth/errors"; import { hashSecret } from "../../lib/secrets"; @@ -38,8 +39,14 @@ interface RegisterBody { logo_uri?: unknown; client_uri?: unknown; token_endpoint_auth_method?: unknown; + grant_types?: unknown; } +// RFC 7591 §2 defaults grant_types to ["authorization_code"]. We add +// refresh_token so a client that omits the field keeps the behaviour it had +// before the field was read at all. +const DEFAULT_GRANT_TYPES = ["authorization_code", "refresh_token"]; + function asStringArray(value: unknown): string[] | null { if (!Array.isArray(value)) return null; const out: string[] = []; @@ -88,12 +95,52 @@ export async function registerClient(req: Request): Promise { return oauthError(400, "invalid_client_metadata", "Body must be JSON"); } - const redirectUris = asStringArray(body.redirect_uris); - if (!redirectUris || redirectUris.length === 0) { + let grantTypes = DEFAULT_GRANT_TYPES; + if (body.grant_types !== undefined) { + const requested = asStringArray(body.grant_types); + if (!requested || requested.length === 0) { + return oauthError( + 400, + "invalid_client_metadata", + "grant_types must be a non-empty array of strings", + ); + } + const unsupported = requested.filter( + (g) => !SUPPORTED_GRANT_TYPES.includes(g as never), + ); + if (unsupported.length > 0) { + return oauthError( + 400, + "invalid_client_metadata", + `Unsupported grant_types: ${unsupported.join(", ")}`, + ); + } + grantTypes = requested; + } + + // RFC 7591 §2: redirect_uris is required for clients using redirect-based + // flows. A device-only client has nothing honest to put there, so don't make + // it invent a placeholder. + const redirectsRequired = grantTypes.includes("authorization_code"); + + // A present-but-malformed redirect_uris is still an error, even when the + // grant types make it optional. Absent is the only way to skip it. + const parsedRedirects = + body.redirect_uris === undefined ? [] : asStringArray(body.redirect_uris); + if (!parsedRedirects) { + return oauthError( + 400, + "invalid_redirect_uri", + "redirect_uris must be an array of URI strings", + ); + } + const redirectUris = parsedRedirects; + + if (redirectsRequired && redirectUris.length === 0) { return oauthError( 400, "invalid_redirect_uri", - "redirect_uris must be a non-empty array of URIs", + "redirect_uris must be a non-empty array of URIs for the authorization_code grant", ); } @@ -125,13 +172,14 @@ export async function registerClient(req: Request): Promise { const now = Math.floor(Date.now() / 1000); db.query( - "INSERT INTO apps (client_id, redirect_uris, name, logo_url, is_preregistered, client_secret_hash, first_seen, last_used) VALUES (?, ?, ?, ?, 1, ?, ?, ?)", + "INSERT INTO apps (client_id, redirect_uris, name, logo_url, is_preregistered, client_secret_hash, grant_types, first_seen, last_used) VALUES (?, ?, ?, ?, 1, ?, ?, ?, ?)", ).run( clientId, JSON.stringify(redirectUris), clientName, logoUri, clientSecretHash, + JSON.stringify(grantTypes), now, now, ); @@ -150,8 +198,10 @@ export async function registerClient(req: Request): Promise { client_name: clientName ?? undefined, logo_uri: logoUri ?? undefined, token_endpoint_auth_method: "client_secret_post", - grant_types: ["authorization_code", "refresh_token"], - response_types: ["code"], + grant_types: grantTypes, + // RFC 7591 §2: response_types pairs with the authorization_code grant. + // A device-only client never gets an authorization response. + response_types: redirectsRequired ? ["code"] : [], issuer: origin, }, { status: 201, headers: NO_STORE_HEADERS }, diff --git a/src/routes/oauth/token.ts b/src/routes/oauth/token.ts index 5c36368..1aa65ca 100644 --- a/src/routes/oauth/token.ts +++ b/src/routes/oauth/token.ts @@ -1,14 +1,16 @@ import crypto from "node:crypto"; import { db } from "../../db"; import { getClientIp } from "../../lib/client-ip"; +import { + verifyClientCredentials, + verifyGrantAllowed, +} from "../../lib/oauth/client-auth"; import { NO_STORE_HEADERS, oauthError, parseBody, - unauthorizedResponse, } from "../../lib/oauth/errors"; import { canonicalizeURL, verifyPKCE } from "../../lib/oauth/urls"; -import { verifySecret } from "../../lib/secrets"; import { signIDToken } from "../../oidc"; const ACCESS_TOKEN_TTL = 3600; // 1 hour @@ -73,6 +75,15 @@ export async function token(req: Request): Promise { ); } + // RFC 7591 §2: a client may only use the grants it registered for. + if (body.client_id) { + const grantError = verifyGrantAllowed( + canonicalizeURL(body.client_id), + grant_type, + ); + if (grantError) return grantError; + } + if (grant_type === "refresh_token") { return handleRefreshTokenGrant(body); } @@ -242,7 +253,7 @@ async function handleRefreshTokenGrant( async function handleDeviceCodeGrant( body: Record, ): Promise { - const { device_code, client_id: rawClientId } = body; + const { device_code, client_id: rawClientId, client_secret } = body; if (!device_code) { return oauthError( @@ -301,9 +312,13 @@ async function handleDeviceCodeGrant( return oauthError(400, "invalid_grant", "client_id mismatch"); } - // RFC 8628 is a public-client flow: the device code itself is high-entropy - // proof of possession, and the client_id binding above is the security - // boundary. No client_secret required, even for pre-registered clients. + // For a public client the device_code is itself high-entropy proof of + // possession and the client_id binding above is the security boundary. A + // client issued a secret is confidential on every grant, though: RFC 8628 + // §3.4 carries RFC 6749 §3.2.1's authentication requirement to device_code + // polling, so knowing the client_id must not be enough to collect its token. + const credentialError = verifyClientCredentials(clientId, client_secret); + if (credentialError) return credentialError; // Rate limiting: enforce minimum poll interval (RFC 8628 §3.5) if (deviceCode.last_polled_at) { @@ -428,51 +443,6 @@ async function handleDeviceCodeGrant( return Response.json(deviceResponse, { headers: NO_STORE_HEADERS }); } -// Verify pre-registered client credentials; returns a Response on failure -function verifyClientCredentials( - client_id: string | undefined, - client_secret: string | undefined, -): Response | null { - // No client_id means we can't look up the app; treat as public/unknown. - if (!client_id) { - return null; - } - - const app = db - .query( - "SELECT is_preregistered, client_secret_hash FROM apps WHERE client_id = ?", - ) - .get(client_id) as - | { is_preregistered: number; client_secret_hash: string | null } - | undefined; - - // client_secret sent for an unknown client - if (client_secret && !app) { - return oauthError(400, "invalid_client", "Unknown client"); - } - - if (app?.is_preregistered !== 1) { - return null; // public client, nothing to check - } - - if (!client_secret) { - return unauthorizedResponse( - "invalid_client", - "client_secret is required for pre-registered clients", - ); - } - - if (!app.client_secret_hash) { - return oauthError(500, "server_error", "Client secret not configured"); - } - - if (!verifySecret(client_secret, app.client_secret_hash)) { - return unauthorizedResponse("invalid_client", "Invalid client_secret"); - } - - return null; -} - async function handleAuthorizationCodeGrant( body: Record, ): Promise { diff --git a/test/device.test.ts b/test/device.test.ts index d22af00..ace8680 100644 --- a/test/device.test.ts +++ b/test/device.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, test } from "bun:test"; +import { hashSecret } from "../src/lib/secrets"; import { deviceAuthorization } from "../src/routes/oauth/device"; import { token } from "../src/routes/oauth/token"; import { createUser, db } from "./helpers/db"; @@ -272,3 +273,83 @@ describe("token endpoint: device_code grant", () => { expect(row.interval).toBe(10); // 5 + 5 penalty }); }); + +describe("device flow: confidential client authentication", () => { + const CONF_ID = "ikc_confidential"; + const SECRET = "iks_topsecret"; + + function seedConfidentialApp( + grantTypes: string[] = [ + "urn:ietf:params:oauth:grant-type:device_code", + "refresh_token", + ], + ) { + db.query( + "INSERT INTO apps (client_id, redirect_uris, name, is_preregistered, client_secret_hash, grant_types) VALUES (?, ?, ?, 1, ?, ?)", + ).run( + CONF_ID, + JSON.stringify([]), + "Confidential Device Client", + hashSecret(SECRET), + JSON.stringify(grantTypes), + ); + } + + test("device authorization request without the secret is rejected", async () => { + seedConfidentialApp(); + const res = await deviceAuthorization(deviceReq({ client_id: CONF_ID })); + expect(res.status).toBe(401); + expect((await res.json()).error).toBe("invalid_client"); + }); + + test("device authorization request with the secret succeeds", async () => { + seedConfidentialApp(); + const res = await deviceAuthorization( + deviceReq({ client_id: CONF_ID, client_secret: SECRET }), + ); + expect(res.status).toBe(200); + }); + + test("knowing the client_id is not enough to poll out a token", async () => { + seedConfidentialApp(); + const userId = createUser({ username: "kieran" }); + const dc = (await ( + await deviceAuthorization( + deviceReq({ client_id: CONF_ID, client_secret: SECRET }), + ) + ).json()) as { device_code: string; user_code: string }; + approveDeviceCode(dc.user_code, userId); + + // The attacker has the device_code and client_id but no secret + const res = await token( + tokenReq({ + grant_type: "urn:ietf:params:oauth:grant-type:device_code", + device_code: dc.device_code, + client_id: CONF_ID, + }), + ); + expect(res.status).toBe(401); + expect((await res.json()).error).toBe("invalid_client"); + }); + + test("a device-only client cannot use the authorization_code grant", async () => { + seedConfidentialApp(); + const res = await token( + tokenReq({ + grant_type: "authorization_code", + code: "whatever", + client_id: CONF_ID, + client_secret: SECRET, + code_verifier: "a".repeat(43), + }), + ); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe("unauthorized_client"); + }); + + test("public clients still need no secret", async () => { + seedApp(); + const res = await deviceAuthorization(deviceReq({ client_id: CLIENT_ID })); + expect(res.status).toBe(200); + }); +}); diff --git a/test/register.test.ts b/test/register.test.ts index fea8a77..2bcc8a9 100644 --- a/test/register.test.ts +++ b/test/register.test.ts @@ -75,6 +75,67 @@ describe("POST /oauth/register (RFC 7591)", () => { expect(res.status).toBe(400); }); + test("a device-only client registers with no redirect_uris at all", async () => { + const res = await registerClient( + registerReq({ + client_name: "lard", + grant_types: [ + "urn:ietf:params:oauth:grant-type:device_code", + "refresh_token", + ], + }), + ); + expect(res.status).toBe(201); + + const json = (await res.json()) as { + client_id: string; + grant_types: string[]; + response_types: string[]; + redirect_uris: string[]; + }; + // The response echoes what was registered, not a hardcoded guess + expect(json.grant_types).toEqual([ + "urn:ietf:params:oauth:grant-type:device_code", + "refresh_token", + ]); + expect(json.response_types).toEqual([]); + expect(json.redirect_uris).toEqual([]); + + const app = db + .query("SELECT grant_types FROM apps WHERE client_id = ?") + .get(json.client_id) as { grant_types: string }; + expect(JSON.parse(app.grant_types)).toContain( + "urn:ietf:params:oauth:grant-type:device_code", + ); + }); + + test("still requires redirect_uris when authorization_code is registered", async () => { + const res = await registerClient( + registerReq({ grant_types: ["authorization_code"] }), + ); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe("invalid_redirect_uri"); + }); + + test("rejects an unsupported grant type", async () => { + const res = await registerClient( + registerReq({ + redirect_uris: ["https://x.com/cb"], + grant_types: ["password"], + }), + ); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe("invalid_client_metadata"); + }); + + test("omitting grant_types keeps the previous default", async () => { + const res = await registerClient( + registerReq({ redirect_uris: ["https://x.com/cb"] }), + ); + const json = (await res.json()) as { grant_types: string[] }; + expect(json.grant_types).toEqual(["authorization_code", "refresh_token"]); + }); + test("no-store cache header", async () => { const res = await registerClient( registerReq({ redirect_uris: ["https://x.com/cb"] }),