From 97162c9d4cf52d6228037b04198eabfc59a749bc Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Sat, 31 Oct 2026 14:52:20 -0400 Subject: [PATCH 1/2] feat(oidc): let a confidential application opt out of PKCE --- .changeset/oidc-client-pkce-opt-out.md | 5 ++ docs/admin-operations.md | 3 +- docs/adr/0001-openid-provider.md | 4 +- openapi.json | 27 +++++- src/controllers/oidcApplications.ts | 16 ++++ src/generated/api.ts | 20 ++++- ...31120000-add-oidc-client-pkce-required.cjs | 28 ++++++ src/models/oidcClients.ts | 4 + src/schemas/authEvent.types.ts | 1 + src/services/oidcApplicationService.ts | 4 + src/services/oidcClientRegistry.ts | 3 + .../integration/oidc/oidcApplications.spec.ts | 85 +++++++++++++++++++ .../unit/services/oidcClientRegistry.spec.ts | 23 +++++ 13 files changed, 218 insertions(+), 5 deletions(-) create mode 100644 .changeset/oidc-client-pkce-opt-out.md create mode 100644 src/migrations/20261031120000-add-oidc-client-pkce-required.cjs diff --git a/.changeset/oidc-client-pkce-opt-out.md b/.changeset/oidc-client-pkce-opt-out.md new file mode 100644 index 00000000..7c311ad2 --- /dev/null +++ b/.changeset/oidc-client-pkce-opt-out.md @@ -0,0 +1,5 @@ +--- +'seamless-auth-api': minor +--- + +OpenID Provider applications gain `pkceRequired`, `true` by default and for every existing application. An admin can set it to `false` on a confidential client only (RFC 9700 section 2.1.1), through `POST` or `PATCH /admin/applications`. A public client is refused. The provider service then receives `urn:seamless:pkce_required: false` for that client and requires `nonce` in place of PKCE whenever it asks for an ID token. Each change is recorded as `admin_oidc_application_pkce_updated`. ADR 0001 D1 is amended to match. diff --git a/docs/admin-operations.md b/docs/admin-operations.md index bc9c1a58..29769d35 100644 --- a/docs/admin-operations.md +++ b/docs/admin-operations.md @@ -357,6 +357,7 @@ The API refuses these on create and on the merged result of every update (#410): | -------------------------------------------------------------------------------------------- | ------------------------ | | Only the `authorization_code`, `refresh_token`, `client_credentials` and device grants exist | Schema | | Public clients authenticate with PKCE alone: no secret, no keys, no `client_credentials` | Schema | +| Only a confidential client may set `pkceRequired: false` | Schema, provider service | | `client_secret_jwt` cannot be chosen | Schema | | Redirects are exact, except the RFC 8252 loopback port (`127.0.0.1`, `[::1]`) | Schema, provider service | | `localhost`, fragments and embedded credentials are refused as redirects | Schema | @@ -366,7 +367,7 @@ The API refuses these on create and on the merged result of every update (#410): | Shared secrets need `allow_client_secrets` | API, provider service | | Third-party clients need `third_party_clients_enabled` | API, provider service | -PKCE with `S256` for every client, `code` as the only response type, and refresh token rotation are fixed in the provider service's configuration and cannot be changed per client. +PKCE with `S256` is required for every client unless a confidential client sets `pkceRequired: false` (RFC 9700 section 2.1.1). An opted-out client must send `nonce` whenever it asks for an ID token, and each change is recorded as `admin_oidc_application_pkce_updated`. `code` as the only response type and refresh token rotation are fixed in the provider service's configuration and cannot be changed per client. ## Audit Events diff --git a/docs/adr/0001-openid-provider.md b/docs/adr/0001-openid-provider.md index 2829512f..cd7441a8 100644 --- a/docs/adr/0001-openid-provider.md +++ b/docs/adr/0001-openid-provider.md @@ -85,7 +85,7 @@ Writing our own provider was rejected. Each RFC above, and keeping a certified r **Locked configuration (#410).** - Only the `code` response type: no implicit or hybrid. -- PKCE required for every client. +- PKCE required by default for every client, with a per-client opt-out for confidential clients only (amended 2026-10-31, below). - Exact redirect URI matching, except the RFC 8252 loopback port rule. - Refresh tokens always rotated. - `devInteractions` and dynamic registration off. @@ -93,6 +93,8 @@ Writing our own provider was rejected. Each RFC above, and keeping a certified r DPoP and PAR are available. The API's admin routes refuse client settings that would loosen this list (D12). +**Amendment, 2026-10-31: per-client PKCE opt-out.** PKCE stays required by default. An admin can set `pkceRequired: false` on a confidential client (`private_key_jwt` or a shared secret), as RFC 9700 section 2.1.1 allows when the client authenticates and binds the code to the transaction another way. The API and the edge both refuse the opt-out on a public client. An opted-out client must send `nonce` on every authorization request that asks for an ID token, which binds the code to the RP's transaction in place of PKCE. The reason is OpenID certification: the conformance suite's Basic OP and Config OP test plans send code requests without PKCE. The change is recorded as `admin_oidc_application_pkce_updated`. + **Issuer.** The edge's public URL, for example `https://login.example.com`, is stored in `system_config` as `oidc_issuer`. Discovery is at `/.well-known/openid-configuration` on the edge, and the provider JWKS is at `/jwks`. The API refuses an `oidc_issuer` equal to `ISSUER`. ## D2. The API stays private diff --git a/openapi.json b/openapi.json index 3026af3c..dbbad57f 100644 --- a/openapi.json +++ b/openapi.json @@ -5281,6 +5281,7 @@ "admin_oidc_application_created", "admin_oidc_application_deleted", "admin_oidc_application_keys_updated", + "admin_oidc_application_pkce_updated", "admin_oidc_application_resources_updated", "admin_oidc_application_secret_rotated", "admin_oidc_application_updated", @@ -5402,6 +5403,7 @@ "admin_oidc_application_created", "admin_oidc_application_deleted", "admin_oidc_application_keys_updated", + "admin_oidc_application_pkce_updated", "admin_oidc_application_resources_updated", "admin_oidc_application_secret_rotated", "admin_oidc_application_updated", @@ -12666,7 +12668,8 @@ "urn:seamless:first_party": true, "urn:seamless:access_token_ttl": 0, "urn:seamless:id_token_ttl": 0, - "urn:seamless:refresh_token_ttl": 0 + "urn:seamless:refresh_token_ttl": 0, + "urn:seamless:pkce_required": true }, "schema": { "type": "object", @@ -12746,7 +12749,8 @@ "type": "integer", "minimum": 0, "exclusiveMinimum": true - } + }, + "urn:seamless:pkce_required": { "type": "boolean" } }, "required": [ "client_id", @@ -13901,6 +13905,7 @@ }, "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, + "pkceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -13946,6 +13951,7 @@ "grantTypes", "deviceFlowEnabled", "noticesEnabled", + "pkceRequired", "jwks", "jwksUri", "hasSecret", @@ -14034,6 +14040,7 @@ }, "deviceFlowEnabled": { "type": "boolean", "default": false }, "noticesEnabled": { "type": "boolean", "default": false }, + "pkceRequired": { "type": "boolean", "default": true }, "backchannelLogoutUri": { "type": "string", "nullable": true, @@ -14106,6 +14113,7 @@ "grantTypes": [null], "deviceFlowEnabled": true, "noticesEnabled": true, + "pkceRequired": true, "jwks": { "keys": [null] }, "jwksUri": "string", "hasSecret": true, @@ -14153,6 +14161,7 @@ }, "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, + "pkceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -14198,6 +14207,7 @@ "grantTypes", "deviceFlowEnabled", "noticesEnabled", + "pkceRequired", "jwks", "jwksUri", "hasSecret", @@ -14351,6 +14361,7 @@ "grantTypes": [null], "deviceFlowEnabled": true, "noticesEnabled": true, + "pkceRequired": true, "jwks": { "keys": [null] }, "jwksUri": "string", "hasSecret": true, @@ -14397,6 +14408,7 @@ }, "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, + "pkceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -14442,6 +14454,7 @@ "grantTypes", "deviceFlowEnabled", "noticesEnabled", + "pkceRequired", "jwks", "jwksUri", "hasSecret", @@ -14587,6 +14600,7 @@ }, "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, + "pkceRequired": { "type": "boolean" }, "backchannelLogoutUri": { "type": "string", "nullable": true, @@ -14636,6 +14650,7 @@ "grantTypes": [null], "deviceFlowEnabled": true, "noticesEnabled": true, + "pkceRequired": true, "jwks": { "keys": [null] }, "jwksUri": "string", "hasSecret": true, @@ -14682,6 +14697,7 @@ }, "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, + "pkceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -14727,6 +14743,7 @@ "grantTypes", "deviceFlowEnabled", "noticesEnabled", + "pkceRequired", "jwks", "jwksUri", "hasSecret", @@ -15138,6 +15155,7 @@ "grantTypes": [null], "deviceFlowEnabled": true, "noticesEnabled": true, + "pkceRequired": true, "jwks": { "keys": [null] }, "jwksUri": "string", "hasSecret": true, @@ -15185,6 +15203,7 @@ }, "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, + "pkceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -15230,6 +15249,7 @@ "grantTypes", "deviceFlowEnabled", "noticesEnabled", + "pkceRequired", "jwks", "jwksUri", "hasSecret", @@ -15444,6 +15464,7 @@ "grantTypes": [null], "deviceFlowEnabled": true, "noticesEnabled": true, + "pkceRequired": true, "jwks": { "keys": [null] }, "jwksUri": "string", "hasSecret": true, @@ -15490,6 +15511,7 @@ }, "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, + "pkceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -15535,6 +15557,7 @@ "grantTypes", "deviceFlowEnabled", "noticesEnabled", + "pkceRequired", "jwks", "jwksUri", "hasSecret", diff --git a/src/controllers/oidcApplications.ts b/src/controllers/oidcApplications.ts index 36e926dc..fc3e3abc 100644 --- a/src/controllers/oidcApplications.ts +++ b/src/controllers/oidcApplications.ts @@ -76,10 +76,18 @@ export async function createApplication(req: Request, res: Response) { tokenEndpointAuthMethod: application.tokenEndpointAuthMethod, grantTypes: application.grantTypes, deviceFlowEnabled: application.deviceFlowEnabled, + pkceRequired: application.pkceRequired, redirectUris: application.redirectUris, }, }); + if (!application.pkceRequired) { + await audit(req, { + type: 'admin_oidc_application_pkce_updated', + metadata: { clientId: client.id, pkceRequired: false }, + }); + } + return res.status(201).json({ application, ...(clientSecret ? { clientSecret } : {}) }); } catch (error) { return sendError(res, error); @@ -88,8 +96,16 @@ export async function createApplication(req: Request, res: Response) { export async function updateApplication(req: Request, res: Response) { try { + const pkceBefore = (await findOidcApplication(req.params.id as string)).pkceRequired; const client = await updateOidcApplication(req.params.id as string, req.body); + if (client.pkceRequired !== pkceBefore) { + await audit(req, { + type: 'admin_oidc_application_pkce_updated', + metadata: { clientId: client.id, pkceRequired: client.pkceRequired }, + }); + } + await audit(req, { type: 'admin_oidc_application_updated', metadata: { diff --git a/src/generated/api.ts b/src/generated/api.ts index 87256c4d..8a50a0d4 100644 --- a/src/generated/api.ts +++ b/src/generated/api.ts @@ -4970,6 +4970,7 @@ export interface paths { | 'admin_oidc_application_created' | 'admin_oidc_application_deleted' | 'admin_oidc_application_keys_updated' + | 'admin_oidc_application_pkce_updated' | 'admin_oidc_application_resources_updated' | 'admin_oidc_application_secret_rotated' | 'admin_oidc_application_updated' @@ -5085,6 +5086,7 @@ export interface paths { | 'admin_oidc_application_created' | 'admin_oidc_application_deleted' | 'admin_oidc_application_keys_updated' + | 'admin_oidc_application_pkce_updated' | 'admin_oidc_application_resources_updated' | 'admin_oidc_application_secret_rotated' | 'admin_oidc_application_updated' @@ -12596,7 +12598,8 @@ export interface paths { * "urn:seamless:first_party": true, * "urn:seamless:access_token_ttl": 0, * "urn:seamless:id_token_ttl": 0, - * "urn:seamless:refresh_token_ttl": 0 + * "urn:seamless:refresh_token_ttl": 0, + * "urn:seamless:pkce_required": true * } */ 'application/json': { @@ -12636,6 +12639,7 @@ export interface paths { 'urn:seamless:access_token_ttl'?: number; 'urn:seamless:id_token_ttl'?: number; 'urn:seamless:refresh_token_ttl'?: number; + 'urn:seamless:pkce_required'?: boolean; }; }; }; @@ -13834,6 +13838,7 @@ export interface paths { grantTypes: ('authorization_code' | 'refresh_token' | 'client_credentials')[]; deviceFlowEnabled: boolean; noticesEnabled: boolean; + pkceRequired: boolean; jwks: { keys: ({ kty: string; @@ -13934,6 +13939,8 @@ export interface paths { deviceFlowEnabled?: boolean; /** @default false */ noticesEnabled?: boolean; + /** @default true */ + pkceRequired?: boolean; /** Format: uri */ backchannelLogoutUri?: string | null; accessTokenTtlSeconds?: number | null; @@ -13982,6 +13989,7 @@ export interface paths { * ], * "deviceFlowEnabled": true, * "noticesEnabled": true, + * "pkceRequired": true, * "jwks": { * "keys": [ * null @@ -14017,6 +14025,7 @@ export interface paths { grantTypes: ('authorization_code' | 'refresh_token' | 'client_credentials')[]; deviceFlowEnabled: boolean; noticesEnabled: boolean; + pkceRequired: boolean; jwks: { keys: ({ kty: string; @@ -14190,6 +14199,7 @@ export interface paths { * ], * "deviceFlowEnabled": true, * "noticesEnabled": true, + * "pkceRequired": true, * "jwks": { * "keys": [ * null @@ -14224,6 +14234,7 @@ export interface paths { grantTypes: ('authorization_code' | 'refresh_token' | 'client_credentials')[]; deviceFlowEnabled: boolean; noticesEnabled: boolean; + pkceRequired: boolean; jwks: { keys: ({ kty: string; @@ -14580,6 +14591,7 @@ export interface paths { grantTypes?: ('authorization_code' | 'refresh_token' | 'client_credentials')[]; deviceFlowEnabled?: boolean; noticesEnabled?: boolean; + pkceRequired?: boolean; /** Format: uri */ backchannelLogoutUri?: string | null; accessTokenTtlSeconds?: number | null; @@ -14616,6 +14628,7 @@ export interface paths { * ], * "deviceFlowEnabled": true, * "noticesEnabled": true, + * "pkceRequired": true, * "jwks": { * "keys": [ * null @@ -14650,6 +14663,7 @@ export interface paths { grantTypes: ('authorization_code' | 'refresh_token' | 'client_credentials')[]; deviceFlowEnabled: boolean; noticesEnabled: boolean; + pkceRequired: boolean; jwks: { keys: ({ kty: string; @@ -14841,6 +14855,7 @@ export interface paths { * ], * "deviceFlowEnabled": true, * "noticesEnabled": true, + * "pkceRequired": true, * "jwks": { * "keys": [ * null @@ -14876,6 +14891,7 @@ export interface paths { grantTypes: ('authorization_code' | 'refresh_token' | 'client_credentials')[]; deviceFlowEnabled: boolean; noticesEnabled: boolean; + pkceRequired: boolean; jwks: { keys: ({ kty: string; @@ -15092,6 +15108,7 @@ export interface paths { * ], * "deviceFlowEnabled": true, * "noticesEnabled": true, + * "pkceRequired": true, * "jwks": { * "keys": [ * null @@ -15126,6 +15143,7 @@ export interface paths { grantTypes: ('authorization_code' | 'refresh_token' | 'client_credentials')[]; deviceFlowEnabled: boolean; noticesEnabled: boolean; + pkceRequired: boolean; jwks: { keys: ({ kty: string; diff --git a/src/migrations/20261031120000-add-oidc-client-pkce-required.cjs b/src/migrations/20261031120000-add-oidc-client-pkce-required.cjs new file mode 100644 index 00000000..38f1504b --- /dev/null +++ b/src/migrations/20261031120000-add-oidc-client-pkce-required.cjs @@ -0,0 +1,28 @@ +/* + * Copyright © 2026 Fells Code, LLC + * Licensed under the Apache License, Version 2.0 + * See LICENSE file in the project root for full license information + */ + +'use strict'; + +/** + * PKCE stays required for every existing client. Only a confidential client may opt out + * (ADR 0001, D1). + * + * @type {import('sequelize-cli').Migration} + */ +module.exports = { + async up(queryInterface) { + await queryInterface.sequelize.query(` + ALTER TABLE public.oidc_clients + ADD COLUMN IF NOT EXISTS "pkceRequired" boolean DEFAULT true NOT NULL; + `); + }, + + async down(queryInterface) { + await queryInterface.sequelize.query(` + ALTER TABLE public.oidc_clients DROP COLUMN IF EXISTS "pkceRequired"; + `); + }, +}; diff --git a/src/models/oidcClients.ts b/src/models/oidcClients.ts index 89316dca..83ab3801 100644 --- a/src/models/oidcClients.ts +++ b/src/models/oidcClients.ts @@ -30,6 +30,7 @@ export interface OidcClientAttributes { refreshTokenTtlSeconds?: number | null; deviceFlowEnabled: boolean; noticesEnabled: boolean; + pkceRequired: boolean; disabledAt?: Date | null; createdAt?: Date; updatedAt?: Date; @@ -50,6 +51,7 @@ type OidcClientCreationAttributes = Optional< | 'refreshTokenTtlSeconds' | 'deviceFlowEnabled' | 'noticesEnabled' + | 'pkceRequired' | 'disabledAt' >; @@ -74,6 +76,7 @@ export class OidcClient declare refreshTokenTtlSeconds: number | null; declare deviceFlowEnabled: boolean; declare noticesEnabled: boolean; + declare pkceRequired: boolean; declare disabledAt: Date | null; declare readonly createdAt: Date; declare readonly updatedAt: Date; @@ -103,6 +106,7 @@ const initializeOidcClientModel = (sequelize: Sequelize) => { refreshTokenTtlSeconds: { type: DataTypes.INTEGER, allowNull: true }, deviceFlowEnabled: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: false }, noticesEnabled: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: false }, + pkceRequired: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: true }, disabledAt: { type: DataTypes.DATE, allowNull: true }, }, { diff --git a/src/schemas/authEvent.types.ts b/src/schemas/authEvent.types.ts index c26e14d0..ea0ce630 100644 --- a/src/schemas/authEvent.types.ts +++ b/src/schemas/authEvent.types.ts @@ -22,6 +22,7 @@ export const AUTH_EVENT_TYPES = [ 'admin_oidc_application_created', 'admin_oidc_application_deleted', 'admin_oidc_application_keys_updated', + 'admin_oidc_application_pkce_updated', 'admin_oidc_application_resources_updated', 'admin_oidc_application_secret_rotated', 'admin_oidc_application_updated', diff --git a/src/services/oidcApplicationService.ts b/src/services/oidcApplicationService.ts index e6831353..de58e8d9 100644 --- a/src/services/oidcApplicationService.ts +++ b/src/services/oidcApplicationService.ts @@ -61,6 +61,7 @@ export function serializeOidcApplication(client: OidcClient): OidcApplication { ), deviceFlowEnabled: client.deviceFlowEnabled, noticesEnabled: client.noticesEnabled, + pkceRequired: client.pkceRequired, jwks: (client.jwks as OidcApplication['jwks']) ?? null, jwksUri: client.jwksUri ?? null, hasSecret: Boolean(client.secretLookup), @@ -151,6 +152,7 @@ export async function createOidcApplication(input: OidcApplicationCreateRequest) refreshTokenTtlSeconds: input.refreshTokenTtlSeconds ?? null, deviceFlowEnabled: input.deviceFlowEnabled, noticesEnabled: input.noticesEnabled, + pkceRequired: input.pkceRequired, }); return { client, clientSecret: issued?.secret }; @@ -181,6 +183,7 @@ export async function updateOidcApplication(id: string, patch: OidcApplicationUp grantTypes: current.grantTypes, deviceFlowEnabled: current.deviceFlowEnabled, noticesEnabled: current.noticesEnabled, + pkceRequired: current.pkceRequired, backchannelLogoutUri: current.backchannelLogoutUri, accessTokenTtlSeconds: current.accessTokenTtlSeconds, idTokenTtlSeconds: current.idTokenTtlSeconds, @@ -204,6 +207,7 @@ export async function updateOidcApplication(id: string, patch: OidcApplicationUp grantTypes: storedGrantTypes(next.grantTypes, next.deviceFlowEnabled), deviceFlowEnabled: next.deviceFlowEnabled, noticesEnabled: next.noticesEnabled, + pkceRequired: next.pkceRequired, backchannelLogoutUri: next.backchannelLogoutUri ?? null, accessTokenTtlSeconds: next.accessTokenTtlSeconds ?? null, idTokenTtlSeconds: next.idTokenTtlSeconds ?? null, diff --git a/src/services/oidcClientRegistry.ts b/src/services/oidcClientRegistry.ts index 2817bb99..cce3c359 100644 --- a/src/services/oidcClientRegistry.ts +++ b/src/services/oidcClientRegistry.ts @@ -84,6 +84,9 @@ export async function getOidcClientMetadata(id: string): Promise = {}): Row { refreshTokenTtlSeconds: null, deviceFlowEnabled: false, noticesEnabled: false, + pkceRequired: true, disabledAt: null, createdAt: new Date('2026-10-30T12:00:00.000Z'), updatedAt: new Date('2026-10-30T12:00:00.000Z'), @@ -185,6 +186,49 @@ describe('POST /admin/applications', () => { expect(res.body.application.jwks).toEqual({ keys: [jwk] }); }); + it('lets a confidential client opt out of PKCE, and audits the opt-out', async () => { + const res = await request(app) + .post('/admin/applications') + .send({ + name: 'Grafana', + clientType: 'confidential', + firstParty: true, + redirectUris: ['https://grafana.example.com/cb'], + jwks: { keys: [jwk] }, + pkceRequired: false, + }); + + expect(res.status).toBe(201); + expect(res.body.application.pkceRequired).toBe(false); + expect(OidcClient.create).toHaveBeenCalledWith( + expect.objectContaining({ pkceRequired: false }), + ); + expect(loggedTypes()).toEqual([ + 'admin_oidc_application_created', + 'admin_oidc_application_pkce_updated', + ]); + expect(vi.mocked(AuthEventService.log).mock.calls[1][0].metadata).toEqual({ + clientId: vi.mocked(OidcClient.create).mock.calls[0][0].id, + pkceRequired: false, + }); + }); + + it('refuses a PKCE opt-out on a public client', async () => { + const res = await request(app) + .post('/admin/applications') + .send({ + name: 'Desktop', + clientType: 'public', + firstParty: true, + redirectUris: ['http://127.0.0.1/callback'], + pkceRequired: false, + }); + + expect(res.status).toBe(400); + expect(res.body.details.issues[0].path).toEqual(['pkceRequired']); + expect(OidcClient.create).not.toHaveBeenCalled(); + }); + it('stores the device grant for a first-party device client', async () => { const res = await request(app) .post('/admin/applications') @@ -314,6 +358,47 @@ describe('PATCH /admin/applications/:id', () => { }); }); + it('audits a PKCE change on its own, and not a patch that leaves it alone', async () => { + const existing = row(); + vi.mocked(OidcClient.findByPk).mockResolvedValue(existing as never); + + const off = await request(app).patch('/admin/applications/app-1').send({ pkceRequired: false }); + expect(off.status).toBe(200); + expect(off.body.application.pkceRequired).toBe(false); + expect(existing.update).toHaveBeenCalledWith(expect.objectContaining({ pkceRequired: false })); + expect(loggedTypes()).toEqual([ + 'admin_oidc_application_pkce_updated', + 'admin_oidc_application_updated', + ]); + + vi.mocked(AuthEventService.log).mockClear(); + await request(app).patch('/admin/applications/app-1').send({ pkceRequired: false }); + expect(loggedTypes()).toEqual(['admin_oidc_application_updated']); + + vi.mocked(AuthEventService.log).mockClear(); + await request(app).patch('/admin/applications/app-1').send({ pkceRequired: true }); + expect(vi.mocked(AuthEventService.log).mock.calls[0][0]).toMatchObject({ + type: 'admin_oidc_application_pkce_updated', + metadata: { clientId: 'app-1', pkceRequired: true }, + }); + }); + + it('refuses a PKCE opt-out on a public client', async () => { + vi.mocked(OidcClient.findByPk).mockResolvedValue( + row({ + clientType: 'public', + tokenEndpointAuthMethod: 'none', + jwksUri: null, + redirectUris: ['http://127.0.0.1/callback'], + }) as never, + ); + + const res = await request(app).patch('/admin/applications/app-1').send({ pkceRequired: false }); + + expect(res.status).toBe(400); + expect(res.body.details.issues[0].path).toEqual(['pkceRequired']); + }); + it('re-checks the merged result', async () => { vi.mocked(OidcClient.findByPk).mockResolvedValue(row() as never); diff --git a/tests/unit/services/oidcClientRegistry.spec.ts b/tests/unit/services/oidcClientRegistry.spec.ts index 8f6a8d9c..43759c83 100644 --- a/tests/unit/services/oidcClientRegistry.spec.ts +++ b/tests/unit/services/oidcClientRegistry.spec.ts @@ -91,6 +91,29 @@ describe('getOidcClientMetadata', () => { }); }); + it('sends a PKCE opt-out only for a confidential client', async () => { + vi.mocked(OidcClient.findByPk).mockResolvedValue( + client({ clientType: 'confidential', pkceRequired: false }) as never, + ); + expect(await getOidcClientMetadata('app')).toMatchObject({ + 'urn:seamless:pkce_required': false, + }); + + vi.mocked(OidcClient.findByPk).mockResolvedValue( + client({ + clientType: 'public', + tokenEndpointAuthMethod: 'none', + pkceRequired: false, + }) as never, + ); + expect(await getOidcClientMetadata('app')).not.toHaveProperty('urn:seamless:pkce_required'); + + vi.mocked(OidcClient.findByPk).mockResolvedValue( + client({ clientType: 'confidential', pkceRequired: true }) as never, + ); + expect(await getOidcClientMetadata('app')).not.toHaveProperty('urn:seamless:pkce_required'); + }); + it('lists the device grant only for a first-party client with the flag on', async () => { const device = 'urn:ietf:params:oauth:grant-type:device_code'; const grantTypes = [device, 'refresh_token']; From a0cec1c389d658de66b1a7c30fdeb71b8aeb7706 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Sat, 31 Oct 2026 18:08:57 -0400 Subject: [PATCH 2/2] feat(oidc): per-application nonce requirement for clients without PKCE --- .changeset/oidc-client-pkce-opt-out.md | 2 +- docs/admin-operations.md | 3 +- docs/adr/0001-openid-provider.md | 4 +- openapi.json | 27 +++++++- src/controllers/oidcApplications.ts | 17 ++++- src/generated/api.ts | 20 +++++- ...31120000-add-oidc-client-pkce-required.cjs | 10 +-- src/models/oidcClients.ts | 4 ++ src/schemas/authEvent.types.ts | 1 + src/services/oidcApplicationService.ts | 4 ++ src/services/oidcClientRegistry.ts | 5 +- .../integration/oidc/oidcApplications.spec.ts | 64 +++++++++++++++++++ .../unit/services/oidcClientRegistry.spec.ts | 15 +++++ 13 files changed, 163 insertions(+), 13 deletions(-) diff --git a/.changeset/oidc-client-pkce-opt-out.md b/.changeset/oidc-client-pkce-opt-out.md index 7c311ad2..a5c17fca 100644 --- a/.changeset/oidc-client-pkce-opt-out.md +++ b/.changeset/oidc-client-pkce-opt-out.md @@ -2,4 +2,4 @@ 'seamless-auth-api': minor --- -OpenID Provider applications gain `pkceRequired`, `true` by default and for every existing application. An admin can set it to `false` on a confidential client only (RFC 9700 section 2.1.1), through `POST` or `PATCH /admin/applications`. A public client is refused. The provider service then receives `urn:seamless:pkce_required: false` for that client and requires `nonce` in place of PKCE whenever it asks for an ID token. Each change is recorded as `admin_oidc_application_pkce_updated`. ADR 0001 D1 is amended to match. +OpenID Provider applications gain `pkceRequired`, `true` by default and for every existing application. An admin can set it to `false` on a confidential client only (RFC 9700 section 2.1.1), through `POST` or `PATCH /admin/applications`. A public client is refused. The provider service then receives `urn:seamless:pkce_required: false` for that client and requires `nonce` in place of PKCE whenever it asks for an ID token. Applications also gain `nonceRequired`, `true` by default, which can be turned off only once `pkceRequired` is `false`; the provider service then receives `urn:seamless:nonce_required: false` and accepts code requests with neither. Each change is recorded as `admin_oidc_application_pkce_updated` or `admin_oidc_application_nonce_updated`. ADR 0001 D1 is amended to match. diff --git a/docs/admin-operations.md b/docs/admin-operations.md index 29769d35..37287855 100644 --- a/docs/admin-operations.md +++ b/docs/admin-operations.md @@ -358,6 +358,7 @@ The API refuses these on create and on the merged result of every update (#410): | Only the `authorization_code`, `refresh_token`, `client_credentials` and device grants exist | Schema | | Public clients authenticate with PKCE alone: no secret, no keys, no `client_credentials` | Schema | | Only a confidential client may set `pkceRequired: false` | Schema, provider service | +| `nonceRequired: false` only once `pkceRequired` is `false` | Schema, provider service | | `client_secret_jwt` cannot be chosen | Schema | | Redirects are exact, except the RFC 8252 loopback port (`127.0.0.1`, `[::1]`) | Schema, provider service | | `localhost`, fragments and embedded credentials are refused as redirects | Schema | @@ -367,7 +368,7 @@ The API refuses these on create and on the merged result of every update (#410): | Shared secrets need `allow_client_secrets` | API, provider service | | Third-party clients need `third_party_clients_enabled` | API, provider service | -PKCE with `S256` is required for every client unless a confidential client sets `pkceRequired: false` (RFC 9700 section 2.1.1). An opted-out client must send `nonce` whenever it asks for an ID token, and each change is recorded as `admin_oidc_application_pkce_updated`. `code` as the only response type and refresh token rotation are fixed in the provider service's configuration and cannot be changed per client. +PKCE with `S256` is required for every client unless a confidential client sets `pkceRequired: false` (RFC 9700 section 2.1.1). An opted-out client must send `nonce` whenever it asks for an ID token unless `nonceRequired` is also `false`. Each change is recorded as `admin_oidc_application_pkce_updated` or `admin_oidc_application_nonce_updated`. Both fields are in every application response, so a client with both off can be flagged: it relies on `state` and client authentication alone against code injection. Like every application write, either change needs step-up. `code` as the only response type and refresh token rotation are fixed in the provider service's configuration and cannot be changed per client. ## Audit Events diff --git a/docs/adr/0001-openid-provider.md b/docs/adr/0001-openid-provider.md index cd7441a8..6cd49664 100644 --- a/docs/adr/0001-openid-provider.md +++ b/docs/adr/0001-openid-provider.md @@ -85,7 +85,7 @@ Writing our own provider was rejected. Each RFC above, and keeping a certified r **Locked configuration (#410).** - Only the `code` response type: no implicit or hybrid. -- PKCE required by default for every client, with a per-client opt-out for confidential clients only (amended 2026-10-31, below). +- PKCE required by default for every client, with a per-client opt-out for confidential clients only that keeps the nonce on unless separately disabled (amended 2026-10-31, below). - Exact redirect URI matching, except the RFC 8252 loopback port rule. - Refresh tokens always rotated. - `devInteractions` and dynamic registration off. @@ -93,7 +93,7 @@ Writing our own provider was rejected. Each RFC above, and keeping a certified r DPoP and PAR are available. The API's admin routes refuse client settings that would loosen this list (D12). -**Amendment, 2026-10-31: per-client PKCE opt-out.** PKCE stays required by default. An admin can set `pkceRequired: false` on a confidential client (`private_key_jwt` or a shared secret), as RFC 9700 section 2.1.1 allows when the client authenticates and binds the code to the transaction another way. The API and the edge both refuse the opt-out on a public client. An opted-out client must send `nonce` on every authorization request that asks for an ID token, which binds the code to the RP's transaction in place of PKCE. The reason is OpenID certification: the conformance suite's Basic OP and Config OP test plans send code requests without PKCE. The change is recorded as `admin_oidc_application_pkce_updated`. +**Amendment, 2026-10-31: per-client PKCE opt-out.** PKCE stays required by default. An admin can set `pkceRequired: false` on a confidential client (`private_key_jwt` or a shared secret), as RFC 9700 section 2.1.1 allows when the client authenticates and binds the code to the transaction another way. The API and the edge both refuse the opt-out on a public client. An opted-out client must send `nonce` on every authorization request that asks for an ID token, which binds the code to the RP's transaction in place of PKCE. A second flag, `nonceRequired`, is on by default and can be turned off only on a client that has already opted out of PKCE. A client with both off relies on `state` and client authentication alone against code injection, so each flag is a separate admin write with step-up, recorded as `admin_oidc_application_pkce_updated` and `admin_oidc_application_nonce_updated`, and both are in every application response so the dashboard and CLI can warn about it. The reason is OpenID certification: the conformance suite's Basic OP, Config OP and Form Post OP plans send code requests without PKCE, and one of their modules sends one without a nonce and expects it to succeed. **Issuer.** The edge's public URL, for example `https://login.example.com`, is stored in `system_config` as `oidc_issuer`. Discovery is at `/.well-known/openid-configuration` on the edge, and the provider JWKS is at `/jwks`. The API refuses an `oidc_issuer` equal to `ISSUER`. diff --git a/openapi.json b/openapi.json index dbbad57f..c087da37 100644 --- a/openapi.json +++ b/openapi.json @@ -5281,6 +5281,7 @@ "admin_oidc_application_created", "admin_oidc_application_deleted", "admin_oidc_application_keys_updated", + "admin_oidc_application_nonce_updated", "admin_oidc_application_pkce_updated", "admin_oidc_application_resources_updated", "admin_oidc_application_secret_rotated", @@ -5403,6 +5404,7 @@ "admin_oidc_application_created", "admin_oidc_application_deleted", "admin_oidc_application_keys_updated", + "admin_oidc_application_nonce_updated", "admin_oidc_application_pkce_updated", "admin_oidc_application_resources_updated", "admin_oidc_application_secret_rotated", @@ -12669,7 +12671,8 @@ "urn:seamless:access_token_ttl": 0, "urn:seamless:id_token_ttl": 0, "urn:seamless:refresh_token_ttl": 0, - "urn:seamless:pkce_required": true + "urn:seamless:pkce_required": true, + "urn:seamless:nonce_required": true }, "schema": { "type": "object", @@ -12750,7 +12753,8 @@ "minimum": 0, "exclusiveMinimum": true }, - "urn:seamless:pkce_required": { "type": "boolean" } + "urn:seamless:pkce_required": { "type": "boolean" }, + "urn:seamless:nonce_required": { "type": "boolean" } }, "required": [ "client_id", @@ -13906,6 +13910,7 @@ "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, "pkceRequired": { "type": "boolean" }, + "nonceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -13952,6 +13957,7 @@ "deviceFlowEnabled", "noticesEnabled", "pkceRequired", + "nonceRequired", "jwks", "jwksUri", "hasSecret", @@ -14041,6 +14047,7 @@ "deviceFlowEnabled": { "type": "boolean", "default": false }, "noticesEnabled": { "type": "boolean", "default": false }, "pkceRequired": { "type": "boolean", "default": true }, + "nonceRequired": { "type": "boolean", "default": true }, "backchannelLogoutUri": { "type": "string", "nullable": true, @@ -14114,6 +14121,7 @@ "deviceFlowEnabled": true, "noticesEnabled": true, "pkceRequired": true, + "nonceRequired": true, "jwks": { "keys": [null] }, "jwksUri": "string", "hasSecret": true, @@ -14162,6 +14170,7 @@ "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, "pkceRequired": { "type": "boolean" }, + "nonceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -14208,6 +14217,7 @@ "deviceFlowEnabled", "noticesEnabled", "pkceRequired", + "nonceRequired", "jwks", "jwksUri", "hasSecret", @@ -14362,6 +14372,7 @@ "deviceFlowEnabled": true, "noticesEnabled": true, "pkceRequired": true, + "nonceRequired": true, "jwks": { "keys": [null] }, "jwksUri": "string", "hasSecret": true, @@ -14409,6 +14420,7 @@ "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, "pkceRequired": { "type": "boolean" }, + "nonceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -14455,6 +14467,7 @@ "deviceFlowEnabled", "noticesEnabled", "pkceRequired", + "nonceRequired", "jwks", "jwksUri", "hasSecret", @@ -14601,6 +14614,7 @@ "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, "pkceRequired": { "type": "boolean" }, + "nonceRequired": { "type": "boolean" }, "backchannelLogoutUri": { "type": "string", "nullable": true, @@ -14651,6 +14665,7 @@ "deviceFlowEnabled": true, "noticesEnabled": true, "pkceRequired": true, + "nonceRequired": true, "jwks": { "keys": [null] }, "jwksUri": "string", "hasSecret": true, @@ -14698,6 +14713,7 @@ "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, "pkceRequired": { "type": "boolean" }, + "nonceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -14744,6 +14760,7 @@ "deviceFlowEnabled", "noticesEnabled", "pkceRequired", + "nonceRequired", "jwks", "jwksUri", "hasSecret", @@ -15156,6 +15173,7 @@ "deviceFlowEnabled": true, "noticesEnabled": true, "pkceRequired": true, + "nonceRequired": true, "jwks": { "keys": [null] }, "jwksUri": "string", "hasSecret": true, @@ -15204,6 +15222,7 @@ "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, "pkceRequired": { "type": "boolean" }, + "nonceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -15250,6 +15269,7 @@ "deviceFlowEnabled", "noticesEnabled", "pkceRequired", + "nonceRequired", "jwks", "jwksUri", "hasSecret", @@ -15465,6 +15485,7 @@ "deviceFlowEnabled": true, "noticesEnabled": true, "pkceRequired": true, + "nonceRequired": true, "jwks": { "keys": [null] }, "jwksUri": "string", "hasSecret": true, @@ -15512,6 +15533,7 @@ "deviceFlowEnabled": { "type": "boolean" }, "noticesEnabled": { "type": "boolean" }, "pkceRequired": { "type": "boolean" }, + "nonceRequired": { "type": "boolean" }, "jwks": { "type": "object", "nullable": true, @@ -15558,6 +15580,7 @@ "deviceFlowEnabled", "noticesEnabled", "pkceRequired", + "nonceRequired", "jwks", "jwksUri", "hasSecret", diff --git a/src/controllers/oidcApplications.ts b/src/controllers/oidcApplications.ts index fc3e3abc..b82a05a2 100644 --- a/src/controllers/oidcApplications.ts +++ b/src/controllers/oidcApplications.ts @@ -77,6 +77,7 @@ export async function createApplication(req: Request, res: Response) { grantTypes: application.grantTypes, deviceFlowEnabled: application.deviceFlowEnabled, pkceRequired: application.pkceRequired, + nonceRequired: application.nonceRequired, redirectUris: application.redirectUris, }, }); @@ -87,6 +88,12 @@ export async function createApplication(req: Request, res: Response) { metadata: { clientId: client.id, pkceRequired: false }, }); } + if (!application.nonceRequired) { + await audit(req, { + type: 'admin_oidc_application_nonce_updated', + metadata: { clientId: client.id, nonceRequired: false }, + }); + } return res.status(201).json({ application, ...(clientSecret ? { clientSecret } : {}) }); } catch (error) { @@ -96,7 +103,9 @@ export async function createApplication(req: Request, res: Response) { export async function updateApplication(req: Request, res: Response) { try { - const pkceBefore = (await findOidcApplication(req.params.id as string)).pkceRequired; + const { pkceRequired: pkceBefore, nonceRequired: nonceBefore } = await findOidcApplication( + req.params.id as string, + ); const client = await updateOidcApplication(req.params.id as string, req.body); if (client.pkceRequired !== pkceBefore) { @@ -105,6 +114,12 @@ export async function updateApplication(req: Request, res: Response) { metadata: { clientId: client.id, pkceRequired: client.pkceRequired }, }); } + if (client.nonceRequired !== nonceBefore) { + await audit(req, { + type: 'admin_oidc_application_nonce_updated', + metadata: { clientId: client.id, nonceRequired: client.nonceRequired }, + }); + } await audit(req, { type: 'admin_oidc_application_updated', diff --git a/src/generated/api.ts b/src/generated/api.ts index 8a50a0d4..af40ebac 100644 --- a/src/generated/api.ts +++ b/src/generated/api.ts @@ -4970,6 +4970,7 @@ export interface paths { | 'admin_oidc_application_created' | 'admin_oidc_application_deleted' | 'admin_oidc_application_keys_updated' + | 'admin_oidc_application_nonce_updated' | 'admin_oidc_application_pkce_updated' | 'admin_oidc_application_resources_updated' | 'admin_oidc_application_secret_rotated' @@ -5086,6 +5087,7 @@ export interface paths { | 'admin_oidc_application_created' | 'admin_oidc_application_deleted' | 'admin_oidc_application_keys_updated' + | 'admin_oidc_application_nonce_updated' | 'admin_oidc_application_pkce_updated' | 'admin_oidc_application_resources_updated' | 'admin_oidc_application_secret_rotated' @@ -12599,7 +12601,8 @@ export interface paths { * "urn:seamless:access_token_ttl": 0, * "urn:seamless:id_token_ttl": 0, * "urn:seamless:refresh_token_ttl": 0, - * "urn:seamless:pkce_required": true + * "urn:seamless:pkce_required": true, + * "urn:seamless:nonce_required": true * } */ 'application/json': { @@ -12640,6 +12643,7 @@ export interface paths { 'urn:seamless:id_token_ttl'?: number; 'urn:seamless:refresh_token_ttl'?: number; 'urn:seamless:pkce_required'?: boolean; + 'urn:seamless:nonce_required'?: boolean; }; }; }; @@ -13839,6 +13843,7 @@ export interface paths { deviceFlowEnabled: boolean; noticesEnabled: boolean; pkceRequired: boolean; + nonceRequired: boolean; jwks: { keys: ({ kty: string; @@ -13941,6 +13946,8 @@ export interface paths { noticesEnabled?: boolean; /** @default true */ pkceRequired?: boolean; + /** @default true */ + nonceRequired?: boolean; /** Format: uri */ backchannelLogoutUri?: string | null; accessTokenTtlSeconds?: number | null; @@ -13990,6 +13997,7 @@ export interface paths { * "deviceFlowEnabled": true, * "noticesEnabled": true, * "pkceRequired": true, + * "nonceRequired": true, * "jwks": { * "keys": [ * null @@ -14026,6 +14034,7 @@ export interface paths { deviceFlowEnabled: boolean; noticesEnabled: boolean; pkceRequired: boolean; + nonceRequired: boolean; jwks: { keys: ({ kty: string; @@ -14200,6 +14209,7 @@ export interface paths { * "deviceFlowEnabled": true, * "noticesEnabled": true, * "pkceRequired": true, + * "nonceRequired": true, * "jwks": { * "keys": [ * null @@ -14235,6 +14245,7 @@ export interface paths { deviceFlowEnabled: boolean; noticesEnabled: boolean; pkceRequired: boolean; + nonceRequired: boolean; jwks: { keys: ({ kty: string; @@ -14592,6 +14603,7 @@ export interface paths { deviceFlowEnabled?: boolean; noticesEnabled?: boolean; pkceRequired?: boolean; + nonceRequired?: boolean; /** Format: uri */ backchannelLogoutUri?: string | null; accessTokenTtlSeconds?: number | null; @@ -14629,6 +14641,7 @@ export interface paths { * "deviceFlowEnabled": true, * "noticesEnabled": true, * "pkceRequired": true, + * "nonceRequired": true, * "jwks": { * "keys": [ * null @@ -14664,6 +14677,7 @@ export interface paths { deviceFlowEnabled: boolean; noticesEnabled: boolean; pkceRequired: boolean; + nonceRequired: boolean; jwks: { keys: ({ kty: string; @@ -14856,6 +14870,7 @@ export interface paths { * "deviceFlowEnabled": true, * "noticesEnabled": true, * "pkceRequired": true, + * "nonceRequired": true, * "jwks": { * "keys": [ * null @@ -14892,6 +14907,7 @@ export interface paths { deviceFlowEnabled: boolean; noticesEnabled: boolean; pkceRequired: boolean; + nonceRequired: boolean; jwks: { keys: ({ kty: string; @@ -15109,6 +15125,7 @@ export interface paths { * "deviceFlowEnabled": true, * "noticesEnabled": true, * "pkceRequired": true, + * "nonceRequired": true, * "jwks": { * "keys": [ * null @@ -15144,6 +15161,7 @@ export interface paths { deviceFlowEnabled: boolean; noticesEnabled: boolean; pkceRequired: boolean; + nonceRequired: boolean; jwks: { keys: ({ kty: string; diff --git a/src/migrations/20261031120000-add-oidc-client-pkce-required.cjs b/src/migrations/20261031120000-add-oidc-client-pkce-required.cjs index 38f1504b..61704475 100644 --- a/src/migrations/20261031120000-add-oidc-client-pkce-required.cjs +++ b/src/migrations/20261031120000-add-oidc-client-pkce-required.cjs @@ -7,8 +7,8 @@ 'use strict'; /** - * PKCE stays required for every existing client. Only a confidential client may opt out - * (ADR 0001, D1). + * PKCE and the nonce stay required for every existing client. Only a confidential client + * may opt out of PKCE, and only then of the nonce (ADR 0001, D1). * * @type {import('sequelize-cli').Migration} */ @@ -16,13 +16,15 @@ module.exports = { async up(queryInterface) { await queryInterface.sequelize.query(` ALTER TABLE public.oidc_clients - ADD COLUMN IF NOT EXISTS "pkceRequired" boolean DEFAULT true NOT NULL; + ADD COLUMN IF NOT EXISTS "pkceRequired" boolean DEFAULT true NOT NULL, + ADD COLUMN IF NOT EXISTS "nonceRequired" boolean DEFAULT true NOT NULL; `); }, async down(queryInterface) { await queryInterface.sequelize.query(` - ALTER TABLE public.oidc_clients DROP COLUMN IF EXISTS "pkceRequired"; + ALTER TABLE public.oidc_clients DROP COLUMN IF EXISTS "pkceRequired", + DROP COLUMN IF EXISTS "nonceRequired"; `); }, }; diff --git a/src/models/oidcClients.ts b/src/models/oidcClients.ts index 83ab3801..bbfbd8eb 100644 --- a/src/models/oidcClients.ts +++ b/src/models/oidcClients.ts @@ -31,6 +31,7 @@ export interface OidcClientAttributes { deviceFlowEnabled: boolean; noticesEnabled: boolean; pkceRequired: boolean; + nonceRequired: boolean; disabledAt?: Date | null; createdAt?: Date; updatedAt?: Date; @@ -52,6 +53,7 @@ type OidcClientCreationAttributes = Optional< | 'deviceFlowEnabled' | 'noticesEnabled' | 'pkceRequired' + | 'nonceRequired' | 'disabledAt' >; @@ -77,6 +79,7 @@ export class OidcClient declare deviceFlowEnabled: boolean; declare noticesEnabled: boolean; declare pkceRequired: boolean; + declare nonceRequired: boolean; declare disabledAt: Date | null; declare readonly createdAt: Date; declare readonly updatedAt: Date; @@ -107,6 +110,7 @@ const initializeOidcClientModel = (sequelize: Sequelize) => { deviceFlowEnabled: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: false }, noticesEnabled: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: false }, pkceRequired: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: true }, + nonceRequired: { type: DataTypes.BOOLEAN, allowNull: false, defaultValue: true }, disabledAt: { type: DataTypes.DATE, allowNull: true }, }, { diff --git a/src/schemas/authEvent.types.ts b/src/schemas/authEvent.types.ts index ea0ce630..a0382807 100644 --- a/src/schemas/authEvent.types.ts +++ b/src/schemas/authEvent.types.ts @@ -22,6 +22,7 @@ export const AUTH_EVENT_TYPES = [ 'admin_oidc_application_created', 'admin_oidc_application_deleted', 'admin_oidc_application_keys_updated', + 'admin_oidc_application_nonce_updated', 'admin_oidc_application_pkce_updated', 'admin_oidc_application_resources_updated', 'admin_oidc_application_secret_rotated', diff --git a/src/services/oidcApplicationService.ts b/src/services/oidcApplicationService.ts index de58e8d9..1bd37a5c 100644 --- a/src/services/oidcApplicationService.ts +++ b/src/services/oidcApplicationService.ts @@ -62,6 +62,7 @@ export function serializeOidcApplication(client: OidcClient): OidcApplication { deviceFlowEnabled: client.deviceFlowEnabled, noticesEnabled: client.noticesEnabled, pkceRequired: client.pkceRequired, + nonceRequired: client.nonceRequired, jwks: (client.jwks as OidcApplication['jwks']) ?? null, jwksUri: client.jwksUri ?? null, hasSecret: Boolean(client.secretLookup), @@ -153,6 +154,7 @@ export async function createOidcApplication(input: OidcApplicationCreateRequest) deviceFlowEnabled: input.deviceFlowEnabled, noticesEnabled: input.noticesEnabled, pkceRequired: input.pkceRequired, + nonceRequired: input.nonceRequired, }); return { client, clientSecret: issued?.secret }; @@ -184,6 +186,7 @@ export async function updateOidcApplication(id: string, patch: OidcApplicationUp deviceFlowEnabled: current.deviceFlowEnabled, noticesEnabled: current.noticesEnabled, pkceRequired: current.pkceRequired, + nonceRequired: current.nonceRequired, backchannelLogoutUri: current.backchannelLogoutUri, accessTokenTtlSeconds: current.accessTokenTtlSeconds, idTokenTtlSeconds: current.idTokenTtlSeconds, @@ -208,6 +211,7 @@ export async function updateOidcApplication(id: string, patch: OidcApplicationUp deviceFlowEnabled: next.deviceFlowEnabled, noticesEnabled: next.noticesEnabled, pkceRequired: next.pkceRequired, + nonceRequired: next.nonceRequired, backchannelLogoutUri: next.backchannelLogoutUri ?? null, accessTokenTtlSeconds: next.accessTokenTtlSeconds ?? null, idTokenTtlSeconds: next.idTokenTtlSeconds ?? null, diff --git a/src/services/oidcClientRegistry.ts b/src/services/oidcClientRegistry.ts index cce3c359..a4c4f13b 100644 --- a/src/services/oidcClientRegistry.ts +++ b/src/services/oidcClientRegistry.ts @@ -85,7 +85,10 @@ export async function getOidcClientMetadata(id: string): Promise = {}): Row { deviceFlowEnabled: false, noticesEnabled: false, pkceRequired: true, + nonceRequired: true, disabledAt: null, createdAt: new Date('2026-10-30T12:00:00.000Z'), updatedAt: new Date('2026-10-30T12:00:00.000Z'), @@ -213,6 +214,48 @@ describe('POST /admin/applications', () => { }); }); + it('lets a client without PKCE turn the nonce requirement off, and audits it', async () => { + const res = await request(app) + .post('/admin/applications') + .send({ + name: 'Conformance', + clientType: 'confidential', + firstParty: true, + redirectUris: ['https://conformance.example.com/cb'], + jwks: { keys: [jwk] }, + pkceRequired: false, + nonceRequired: false, + }); + + expect(res.status).toBe(201); + expect(res.body.application).toMatchObject({ pkceRequired: false, nonceRequired: false }); + expect(loggedTypes()).toEqual([ + 'admin_oidc_application_created', + 'admin_oidc_application_pkce_updated', + 'admin_oidc_application_nonce_updated', + ]); + expect(vi.mocked(AuthEventService.log).mock.calls[2][0].metadata).toEqual({ + clientId: vi.mocked(OidcClient.create).mock.calls[0][0].id, + nonceRequired: false, + }); + }); + + it('refuses to drop the nonce requirement while PKCE is required', async () => { + const res = await request(app) + .post('/admin/applications') + .send({ + name: 'Grafana', + clientType: 'confidential', + firstParty: true, + redirectUris: ['https://grafana.example.com/cb'], + jwks: { keys: [jwk] }, + nonceRequired: false, + }); + + expect(res.status).toBe(400); + expect(res.body.details.issues[0].path).toEqual(['nonceRequired']); + }); + it('refuses a PKCE opt-out on a public client', async () => { const res = await request(app) .post('/admin/applications') @@ -383,6 +426,27 @@ describe('PATCH /admin/applications/:id', () => { }); }); + it('audits a nonce requirement change, and re-checks it against PKCE', async () => { + const existing = row({ pkceRequired: false }); + vi.mocked(OidcClient.findByPk).mockResolvedValue(existing as never); + + const off = await request(app) + .patch('/admin/applications/app-1') + .send({ nonceRequired: false }); + expect(off.status).toBe(200); + expect(off.body.application.nonceRequired).toBe(false); + expect(loggedTypes()).toEqual([ + 'admin_oidc_application_nonce_updated', + 'admin_oidc_application_updated', + ]); + + const pkceBack = await request(app) + .patch('/admin/applications/app-1') + .send({ pkceRequired: true }); + expect(pkceBack.status).toBe(400); + expect(pkceBack.body.details.issues[0].path).toEqual(['nonceRequired']); + }); + it('refuses a PKCE opt-out on a public client', async () => { vi.mocked(OidcClient.findByPk).mockResolvedValue( row({ diff --git a/tests/unit/services/oidcClientRegistry.spec.ts b/tests/unit/services/oidcClientRegistry.spec.ts index 43759c83..0d6fef08 100644 --- a/tests/unit/services/oidcClientRegistry.spec.ts +++ b/tests/unit/services/oidcClientRegistry.spec.ts @@ -114,6 +114,21 @@ describe('getOidcClientMetadata', () => { expect(await getOidcClientMetadata('app')).not.toHaveProperty('urn:seamless:pkce_required'); }); + it('sends a nonce opt-out only alongside a PKCE opt-out', async () => { + vi.mocked(OidcClient.findByPk).mockResolvedValue( + client({ clientType: 'confidential', pkceRequired: false, nonceRequired: false }) as never, + ); + expect(await getOidcClientMetadata('app')).toMatchObject({ + 'urn:seamless:pkce_required': false, + 'urn:seamless:nonce_required': false, + }); + + vi.mocked(OidcClient.findByPk).mockResolvedValue( + client({ clientType: 'confidential', pkceRequired: true, nonceRequired: false }) as never, + ); + expect(await getOidcClientMetadata('app')).not.toHaveProperty('urn:seamless:nonce_required'); + }); + it('lists the device grant only for a first-party client with the flag on', async () => { const device = 'urn:ietf:params:oauth:grant-type:device_code'; const grantTypes = [device, 'refresh_token'];