diff --git a/src/content/docs.md b/src/content/docs.md index 2bc9313..3afd308 100644 --- a/src/content/docs.md +++ b/src/content/docs.md @@ -584,10 +584,30 @@ Content-Type: application/json ```json { "client_name": "My CLI", + "token_endpoint_auth_method": "none", "grant_types": ["urn:ietf:params:oauth:grant-type:device_code", "refresh_token"] } ``` +### confidential or public? + +`token_endpoint_auth_method` decides whether you get a `client_secret` at all. It defaults to `client_secret_post`; pass `"none"` to register a public client and no secret is issued. + +Pick by asking where the secret would live: + +| Situation | Register as | +| --- | --- | +| Server-side app, secret in its own environment | `client_secret_post` | +| CLI or desktop app that registers **once per install** and stores its own credentials | `client_secret_post` | +| CLI shipping one shared secret compiled into the binary | `none` | +| Anything a user can extract with `strings` | `none` | + +A secret distributed to every user isn't a secret, and registering as confidential with one is worse than registering public: it tells Indiko to treat requests as authenticated when they aren't. Per-install dynamic registration is the way to have real credentials in a CLI, since each copy gets its own. + +Public clients aren't unprotected. The authorization code flow still requires PKCE from everyone, and the device flow's `device_code` is high-entropy proof of possession on its own. + +> **Confidential clients and the device flow:** if you did register with a secret, send it on **both** `POST /auth/device` and every token poll. RFC 8628 applies the same client authentication rules as the token endpoint, so a secret that only shows up at one of the two will be rejected at the other. + Response `201 Created`: ```json diff --git a/src/lib/oauth/client-auth.ts b/src/lib/oauth/client-auth.ts index 1413171..26b7fe8 100644 --- a/src/lib/oauth/client-auth.ts +++ b/src/lib/oauth/client-auth.ts @@ -43,20 +43,24 @@ export function verifyClientCredentials( } if (app?.is_preregistered !== 1) { - return null; // public client, nothing to check + return null; // auto-registered URL client, nothing to check + } + + // Holding a secret hash is what makes a client confidential. A client that + // registered with token_endpoint_auth_method "none" — a CLI that cannot keep + // a secret, say — has none to verify and authenticates by other means: PKCE + // on the auth-code grant, the device_code itself on the device grant. + if (!app.client_secret_hash) { + return null; } if (!client_secret) { return unauthorizedResponse( "invalid_client", - "client_secret is required for pre-registered clients", + "client_secret is required for this client", ); } - 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"); } diff --git a/src/routes/oauth/register.ts b/src/routes/oauth/register.ts index f5cdd56..7ffe9a8 100644 --- a/src/routes/oauth/register.ts +++ b/src/routes/oauth/register.ts @@ -166,9 +166,26 @@ export async function registerClient(req: Request): Promise { typeof body.client_name === "string" ? body.client_name : null; const logoUri = typeof body.logo_uri === "string" ? body.logo_uri : null; + // RFC 7591 §2 / RFC 6749 §2.1: a client that cannot keep a secret should say + // so and register as public. A CLI shipping one secret in its binary is + // public no matter what it claims, and a decorative password is worse than + // none — it invites everyone to treat the client as authenticated. + const authMethod = + body.token_endpoint_auth_method === undefined + ? "client_secret_post" + : body.token_endpoint_auth_method; + if (authMethod !== "client_secret_post" && authMethod !== "none") { + return oauthError( + 400, + "invalid_client_metadata", + "token_endpoint_auth_method must be client_secret_post or none", + ); + } + const isConfidential = authMethod === "client_secret_post"; + const clientId = generateClientId(); - const clientSecret = generateClientSecret(); - const clientSecretHash = hashSecret(clientSecret); + const clientSecret = isConfidential ? generateClientSecret() : null; + const clientSecretHash = clientSecret ? hashSecret(clientSecret) : null; const now = Math.floor(Date.now() / 1000); db.query( @@ -191,13 +208,15 @@ export async function registerClient(req: Request): Promise { return Response.json( { client_id: clientId, - client_secret: clientSecret, + client_secret: clientSecret ?? undefined, client_id_issued_at: now, - client_secret_expires_at: 0, // never expires + // RFC 7591 §3.2.1: client_secret_expires_at only applies when a secret + // was issued. + client_secret_expires_at: clientSecret ? 0 : undefined, // 0 = never redirect_uris: redirectUris, client_name: clientName ?? undefined, logo_uri: logoUri ?? undefined, - token_endpoint_auth_method: "client_secret_post", + token_endpoint_auth_method: authMethod, grant_types: grantTypes, // RFC 7591 §2: response_types pairs with the authorization_code grant. // A device-only client never gets an authorization response. diff --git a/test/device.test.ts b/test/device.test.ts index ace8680..927bac7 100644 --- a/test/device.test.ts +++ b/test/device.test.ts @@ -353,3 +353,37 @@ describe("device flow: confidential client authentication", () => { expect(res.status).toBe(200); }); }); + +describe("device flow: public DCR client (token_endpoint_auth_method none)", () => { + const PUB_ID = "ikc_publiccli"; + + function seedPublicApp() { + db.query( + "INSERT INTO apps (client_id, redirect_uris, name, is_preregistered, client_secret_hash, grant_types) VALUES (?, ?, ?, 1, NULL, ?)", + ).run( + PUB_ID, + JSON.stringify([]), + "My CLI", + JSON.stringify(["urn:ietf:params:oauth:grant-type:device_code"]), + ); + } + + test("a secretless client is not asked for a secret", async () => { + seedPublicApp(); + const res = await deviceAuthorization(deviceReq({ client_id: PUB_ID })); + expect(res.status).toBe(200); + }); + + test("full device flow works end to end with no credentials", async () => { + seedPublicApp(); + const userId = createUser({ username: "kieran" }); + const dc = (await ( + await deviceAuthorization(deviceReq({ client_id: PUB_ID })) + ).json()) as { device_code: string; user_code: string }; + approveDeviceCode(dc.user_code, userId); + + const res = await pollToken(dc.device_code, PUB_ID); + expect(res.status).toBe(200); + expect((await res.json()).access_token).toBeTruthy(); + }); +}); diff --git a/test/register.test.ts b/test/register.test.ts index 2bcc8a9..254da8f 100644 --- a/test/register.test.ts +++ b/test/register.test.ts @@ -136,6 +136,43 @@ describe("POST /oauth/register (RFC 7591)", () => { expect(json.grant_types).toEqual(["authorization_code", "refresh_token"]); }); + test("a public client registers with no secret at all", async () => { + const res = await registerClient( + registerReq({ + client_name: "my-cli", + token_endpoint_auth_method: "none", + grant_types: ["urn:ietf:params:oauth:grant-type:device_code"], + }), + ); + expect(res.status).toBe(201); + + const json = (await res.json()) as { + client_id: string; + client_secret?: string; + client_secret_expires_at?: number; + token_endpoint_auth_method: string; + }; + expect(json.client_secret).toBeUndefined(); + expect(json.client_secret_expires_at).toBeUndefined(); + expect(json.token_endpoint_auth_method).toBe("none"); + + const app = db + .query("SELECT client_secret_hash FROM apps WHERE client_id = ?") + .get(json.client_id) as { client_secret_hash: string | null }; + expect(app.client_secret_hash).toBeNull(); + }); + + test("rejects an unsupported token_endpoint_auth_method", async () => { + const res = await registerClient( + registerReq({ + redirect_uris: ["https://x.com/cb"], + token_endpoint_auth_method: "client_secret_jwt", + }), + ); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe("invalid_client_metadata"); + }); + test("no-store cache header", async () => { const res = await registerClient( registerReq({ redirect_uris: ["https://x.com/cb"] }),