From 6c4904102aeb853283cb2de829e84114f4a620af Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Wed, 2 Jul 2025 13:27:00 +0200 Subject: [PATCH 01/28] chore: add migrations file Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- ...20250701120000_oidc_client_registration.js | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js diff --git a/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js b/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js new file mode 100644 index 0000000000..9f40831ae4 --- /dev/null +++ b/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js @@ -0,0 +1,174 @@ +/* + * Copyright 2024 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// @ts-check + +/** + * @param {import('knex').Knex} knex + */ +exports.up = async function up(knex) { + await knex.schema.createTable('oidc_clients', table => { + table.comment( + 'OIDC clients that are registered via dynamic client registration', + ); + + table + .string('client_id') + .primary() + .notNullable() + .comment('The unique client ID of the client'); + + table + .string('client_secret') + .notNullable() + .comment('The client secret of the client'); + + table + .string('client_name') + .notNullable() + .comment('The name of the client, should be human readable'); + + table + .timestamp('created_at', { useTz: false, precision: 0 }) + .notNullable() + .defaultTo(knex.fn.now()) + .comment('Client registration timestamp'); + + table + .timestamp('expires_at', { useTz: false, precision: 0 }) + .nullable() + .comment('Client registration expiration timestamp'); + + table + .text('response_types', 'longtext') + .notNullable() + .comment('JSON array of supported response types'); + + table + .text('grant_types', 'longtext') + .notNullable() + .comment('JSON array of supported grant types'); + + table + .text('scope') + .nullable() + .comment('Space-separated list of allowed scopes'); + + table + .text('metadata', 'longtext') + .nullable() + .comment('Additional client metadata as JSON'); + }); + + await knex.schema.createTable('oidc_authorization_codes', table => { + table.comment('Authorization codes for OIDC authorization code flow'); + + table.string('code').primary().notNullable().comment('Authorization code'); + + table + .string('client_id') + .notNullable() + .comment('Client ID that requested the code'); + + table + .string('user_entity_ref') + .notNullable() + .comment('User entity reference who authorized'); + + table + .text('redirect_uri') + .notNullable() + .comment('Redirect URI used in authorization request'); + + table.text('scope').nullable().comment('Requested scopes'); + + table.string('code_challenge').nullable().comment('PKCE code challenge'); + + table + .string('code_challenge_method') + .nullable() + .comment('PKCE code challenge method'); + + table.string('nonce').nullable().comment('Nonce value for ID token'); + + table + .timestamp('created_at', { useTz: false, precision: 0 }) + .notNullable() + .defaultTo(knex.fn.now()) + .comment('Code creation timestamp'); + + table + .timestamp('expires_at', { useTz: false, precision: 0 }) + .notNullable() + .comment('Code expiration timestamp'); + + table + .boolean('used') + .defaultTo(false) + .comment('Whether the code has been used'); + + table.foreign('client_id').references('client_id').inTable('oidc_clients'); + }); + + await knex.schema.createTable('oidc_access_tokens', table => { + table.comment('Access tokens issued by OIDC server'); + + table + .string('token_id') + .primary() + .notNullable() + .comment('Unique token identifier'); + + table + .string('client_id') + .notNullable() + .comment('Client ID that owns the token'); + + table + .string('user_entity_ref') + .notNullable() + .comment('User entity reference'); + + table.text('scope').nullable().comment('Token scopes'); + + table + .timestamp('created_at', { useTz: false, precision: 0 }) + .notNullable() + .defaultTo(knex.fn.now()) + .comment('Token creation timestamp'); + + table + .timestamp('expires_at', { useTz: false, precision: 0 }) + .notNullable() + .comment('Token expiration timestamp'); + + table + .boolean('revoked') + .defaultTo(false) + .comment('Whether the token has been revoked'); + + table.foreign('client_id').references('client_id').inTable('oidc_clients'); + }); +}; + +/** + * @param {import('knex').Knex} knex + */ +exports.down = async function down(knex) { + await knex.schema.dropTable('oidc_access_tokens'); + await knex.schema.dropTable('oidc_authorization_codes'); + await knex.schema.dropTable('oidc_clients'); +}; From 64dc5463ba2671faaae38d8f495bf772981b0cb2 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Wed, 2 Jul 2025 17:45:45 +0200 Subject: [PATCH 02/28] feat: started to add some tests for the oidc database Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- ...20250701120000_oidc_client_registration.js | 5 + .../src/database/OidcDatabase.test.ts | 199 +++++++++++++++ .../auth-backend/src/database/OidcDatabase.ts | 229 ++++++++++++++++++ 3 files changed, 433 insertions(+) create mode 100644 plugins/auth-backend/src/database/OidcDatabase.test.ts create mode 100644 plugins/auth-backend/src/database/OidcDatabase.ts diff --git a/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js b/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js index 9f40831ae4..d4863965de 100644 --- a/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js +++ b/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js @@ -41,6 +41,11 @@ exports.up = async function up(knex) { .notNullable() .comment('The name of the client, should be human readable'); + table + .text('redirect_uris', 'longtext') + .notNullable() + .comment('JSON array of valid redirect URIs'); + table .timestamp('created_at', { useTz: false, precision: 0 }) .notNullable() diff --git a/plugins/auth-backend/src/database/OidcDatabase.test.ts b/plugins/auth-backend/src/database/OidcDatabase.test.ts new file mode 100644 index 0000000000..bf00159b49 --- /dev/null +++ b/plugins/auth-backend/src/database/OidcDatabase.test.ts @@ -0,0 +1,199 @@ +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; +import { AuthDatabase } from './AuthDatabase'; +import { OidcDatabase } from './OidcDatabase'; +import { resolvePackagePath } from '@backstage/backend-plugin-api'; + +describe('Oidc Database', () => { + const databases = TestDatabases.create(); + + async function createOidcDatabase(databaseId: TestDatabaseId) { + const knex = await databases.init(databaseId); + + await knex.migrate.latest({ + directory: resolvePackagePath( + '@backstage/plugin-auth-backend', + 'migrations', + ), + }); + + return { + oidc: await OidcDatabase.create({ + database: AuthDatabase.create({ + getClient: async () => knex, + }), + }), + }; + } + + describe.each(databases.eachSupportedId())('%p', databaseId => { + describe('Client', () => { + it('should create and return a client', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + await expect( + oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }), + ).resolves.toEqual({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: undefined, + expiresAt: undefined, + metadata: undefined, + createdAt: expect.any(String), + }); + }); + + it('should return the client thats created in a list', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const client = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + await expect( + oidc.getClient({ clientId: 'test-client' }), + ).resolves.toEqual(client); + }); + + it('should return null if the client does not exist', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + await expect( + oidc.getClient({ clientId: 'test-client' }), + ).resolves.toBeNull(); + }); + }); + + describe('Authorization Code', () => { + it('should create and return an authorization code', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const mockClient = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + const authorizationCode = await oidc.createAuthorizationCode({ + code: 'test-code', + clientId: mockClient.clientId, + userEntityRef: 'user:default/blam', + redirectUri: 'https://example.com/callback', + scope: undefined, + codeChallenge: 'test-challenge', + codeChallengeMethod: 'S256', + nonce: 'test-nonce', + expiresAt: '2025-01-01', + }); + + await expect( + oidc.getAuthorizationCode({ code: 'test-code' }), + ).resolves.toEqual(authorizationCode); + }); + + it('should return null if the authorization code does not exist', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + await expect( + oidc.getAuthorizationCode({ code: 'test-code' }), + ).resolves.toBeNull(); + }); + + it('should return the authorization code when created', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const mockClient = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + const authorizationCode = await oidc.createAuthorizationCode({ + code: 'test-code', + clientId: mockClient.clientId, + userEntityRef: 'user:default/blam', + redirectUri: 'https://example.com/callback', + scope: undefined, + codeChallenge: 'test-challenge', + codeChallengeMethod: 'S256', + nonce: 'test-nonce', + expiresAt: '2025-01-01', + }); + + await expect( + oidc.getAuthorizationCode({ code: 'test-code' }), + ).resolves.toEqual(authorizationCode); + }); + + it('should allow updating the authorization code', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const mockClient = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + const authorizationCode = await oidc.createAuthorizationCode({ + code: 'test-code', + clientId: mockClient.clientId, + userEntityRef: 'user:default/blam', + redirectUri: 'https://example.com/callback', + codeChallenge: 'test-challenge', + codeChallengeMethod: 'S256', + nonce: 'test-nonce', + expiresAt: '2025-01-01', + }); + + await expect( + oidc.updateAuthorizationCode({ + code: 'test-code', + used: true, + }), + ).resolves.toEqual({ + ...authorizationCode, + used: true, + }); + }); + }); + }); +}); diff --git a/plugins/auth-backend/src/database/OidcDatabase.ts b/plugins/auth-backend/src/database/OidcDatabase.ts new file mode 100644 index 0000000000..ae3cfc42a6 --- /dev/null +++ b/plugins/auth-backend/src/database/OidcDatabase.ts @@ -0,0 +1,229 @@ +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { Knex } from 'knex'; +import { AuthDatabase } from './AuthDatabase'; + +import { DateTime } from 'luxon'; + +type OidcClientRow = { + client_id: string; + client_secret: string; + client_name: string; + created_at: string; + expires_at: string | null; + response_types: string; + grant_types: string; + redirect_uris: string; + scope: string | null; + metadata: string | null; +}; + +type OidcAuthorizationCodeRow = { + code: string; + client_id: string; + user_entity_ref: string; + redirect_uri: string; + scope: string | null; + code_challenge: string | null; + code_challenge_method: string | null; + nonce: string | null; + created_at: string; + expires_at: string; + used?: boolean; +}; + +type Client = { + clientId: string; + clientName: string; + clientSecret: string; + redirectUris: string[]; + responseTypes: string[]; + grantTypes: string[]; + scope?: string; + expiresAt?: string; + metadata?: Record; + createdAt: string; +}; + +type AuthorizationCode = { + code: string; + clientId: string; + userEntityRef: string; + redirectUri: string; + scope?: string; + codeChallenge?: string; + codeChallengeMethod?: string; + nonce?: string; + createdAt: string; + expiresAt: string; + used: boolean; +}; + +export class OidcDatabase { + private constructor(private readonly db: Knex) {} + + static async create(options: { database: AuthDatabase }) { + const client = await options.database.get(); + return new OidcDatabase(client); + } + + async createClient(client: Omit) { + const now = DateTime.now().toString(); + + await this.db('oidc_clients').insert({ + client_id: client.clientId, + client_secret: client.clientSecret, + client_name: client.clientName, + created_at: now, + expires_at: client.expiresAt, + response_types: JSON.stringify(client.responseTypes), + grant_types: JSON.stringify(client.grantTypes), + redirect_uris: JSON.stringify(client.redirectUris), + scope: client.scope, + metadata: JSON.stringify(client.metadata), + }); + + return { + ...client, + createdAt: now, + }; + } + + async getClient({ clientId }: { clientId: string }) { + const client = await this.db('oidc_clients') + .where('client_id', clientId) + .first(); + + if (!client) { + return null; + } + + return this.rowToClient(client) as Client; + } + + async createAuthorizationCode( + authorizationCode: Omit, + ) { + const now = DateTime.now().toString(); + + await this.db('oidc_authorization_codes').insert({ + code: authorizationCode.code, + client_id: authorizationCode.clientId, + user_entity_ref: authorizationCode.userEntityRef, + redirect_uri: authorizationCode.redirectUri, + scope: authorizationCode.scope, + code_challenge: authorizationCode.codeChallenge, + code_challenge_method: authorizationCode.codeChallengeMethod, + nonce: authorizationCode.nonce, + expires_at: authorizationCode.expiresAt, + created_at: now, + used: false, + }); + + return { + ...authorizationCode, + createdAt: now, + used: false, + }; + } + + async getAuthorizationCode({ code }: { code: string }) { + const authorizationCode = await this.db( + 'oidc_authorization_codes', + ) + .where('code', code) + .first(); + + if (!authorizationCode) { + return null; + } + + return this.rowToAuthorizationCode(authorizationCode) as AuthorizationCode; + } + + async updateAuthorizationCode( + authorizationCode: Partial & { code: string }, + ) { + const row = this.authorizationCodeToRow(authorizationCode); + const updatedFields = Object.fromEntries( + Object.entries(row).filter(([_, value]) => value !== undefined), + ); + console.log(updatedFields); + const updated = await this.db( + 'oidc_authorization_codes', + ) + .where('code', authorizationCode.code) + .update(updatedFields) + .returning('*'); + + return this.rowToAuthorizationCode(updated[0]) as AuthorizationCode; + } + + private rowToClient(row: Partial): Partial { + return { + clientId: row.client_id, + clientName: row.client_name, + clientSecret: row.client_secret, + redirectUris: row.redirect_uris + ? JSON.parse(row.redirect_uris) + : undefined, + responseTypes: row.response_types + ? JSON.parse(row.response_types) + : undefined, + grantTypes: row.grant_types ? JSON.parse(row.grant_types) : undefined, + scope: row.scope ?? undefined, + expiresAt: row.expires_at ?? undefined, + metadata: row.metadata ? JSON.parse(row.metadata) : undefined, + createdAt: row.created_at, + }; + } + + private authorizationCodeToRow( + authorizationCode: Partial, + ): Partial { + return { + code: authorizationCode.code, + client_id: authorizationCode.clientId, + user_entity_ref: authorizationCode.userEntityRef, + redirect_uri: authorizationCode.redirectUri, + scope: authorizationCode.scope, + code_challenge: authorizationCode.codeChallenge, + code_challenge_method: authorizationCode.codeChallengeMethod, + nonce: authorizationCode.nonce, + created_at: authorizationCode.createdAt, + expires_at: authorizationCode.expiresAt, + used: authorizationCode.used, + }; + } + + private rowToAuthorizationCode( + row: Partial, + ): Partial { + return { + code: row.code, + clientId: row.client_id, + userEntityRef: row.user_entity_ref, + redirectUri: row.redirect_uri, + scope: row.scope ?? undefined, + codeChallenge: row.code_challenge ?? undefined, + codeChallengeMethod: row.code_challenge_method ?? undefined, + nonce: row.nonce ?? undefined, + createdAt: row.created_at, + expiresAt: row.expires_at, + used: Boolean(row.used), + }; + } +} From ac54ac21d315864d5ed2872a21d45aa97455d36e Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Wed, 2 Jul 2025 19:30:25 +0200 Subject: [PATCH 03/28] chore: implementing access token management Signed-off-by: benjdlambert --- .../src/database/OidcDatabase.test.ts | 95 +++++++++++++++- .../auth-backend/src/database/OidcDatabase.ts | 107 +++++++++++++++++- 2 files changed, 197 insertions(+), 5 deletions(-) diff --git a/plugins/auth-backend/src/database/OidcDatabase.test.ts b/plugins/auth-backend/src/database/OidcDatabase.test.ts index bf00159b49..65f9e9824c 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.test.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.test.ts @@ -41,7 +41,7 @@ describe('Oidc Database', () => { } describe.each(databases.eachSupportedId())('%p', databaseId => { - describe('Client', () => { + describe('Clients', () => { it('should create and return a client', async () => { const { oidc } = await createOidcDatabase(databaseId); @@ -94,7 +94,7 @@ describe('Oidc Database', () => { }); }); - describe('Authorization Code', () => { + describe('Authorization Codes', () => { it('should create and return an authorization code', async () => { const { oidc } = await createOidcDatabase(databaseId); @@ -195,5 +195,96 @@ describe('Oidc Database', () => { }); }); }); + + describe('Access Tokens', () => { + it('should create and return an access token', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const mockClient = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + const accessToken = await oidc.createAccessToken({ + tokenId: 'test-token', + clientId: mockClient.clientId, + userEntityRef: 'user:default/blam', + expiresAt: '2025-01-01', + revoked: false, + }); + + await expect( + oidc.getAccessToken({ tokenId: 'test-token' }), + ).resolves.toEqual(accessToken); + }); + + it('should return null if the access token does not exist', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + await expect( + oidc.getAccessToken({ tokenId: 'test-token' }), + ).resolves.toBeNull(); + }); + + it('should return the access token when created', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const mockClient = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + const accessToken = await oidc.createAccessToken({ + tokenId: 'test-token', + clientId: mockClient.clientId, + userEntityRef: 'user:default/blam', + expiresAt: '2025-01-01', + revoked: false, + }); + + await expect( + oidc.getAccessToken({ tokenId: 'test-token' }), + ).resolves.toEqual(accessToken); + }); + + it('should allow updating the access token', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const mockClient = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + const accessToken = await oidc.createAccessToken({ + tokenId: 'test-token', + clientId: mockClient.clientId, + userEntityRef: 'user:default/blam', + expiresAt: '2025-01-01', + revoked: false, + }); + + await expect( + oidc.updateAccessToken({ + tokenId: 'test-token', + revoked: true, + }), + ).resolves.toEqual({ + ...accessToken, + revoked: true, + }); + }); + }); }); }); diff --git a/plugins/auth-backend/src/database/OidcDatabase.ts b/plugins/auth-backend/src/database/OidcDatabase.ts index ae3cfc42a6..cbaf101ed9 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.ts @@ -45,6 +45,16 @@ type OidcAuthorizationCodeRow = { used?: boolean; }; +type OidcAccessTokenRow = { + token_id: string; + client_id: string; + user_entity_ref: string; + scope: string | null; + created_at: string; + expires_at: string; + revoked?: boolean; +}; + type Client = { clientId: string; clientName: string; @@ -72,6 +82,23 @@ type AuthorizationCode = { used: boolean; }; +type AccessToken = { + tokenId: string; + clientId: string; + userEntityRef: string; + scope?: string; + createdAt: string; + expiresAt: string; + revoked?: boolean; +}; + +/** + * This class is an implementation for the Database operations that power the OIDC sign-in flow. + * + * This class provides database operations for OpenID Connect (OIDC) authentication flows. + * It manages OIDC clients, authorization codes, and access tokens in the database, as well as the consent requests + * for the frontend plugin to accept. + */ export class OidcDatabase { private constructor(private readonly db: Knex) {} @@ -161,15 +188,61 @@ export class OidcDatabase { const updatedFields = Object.fromEntries( Object.entries(row).filter(([_, value]) => value !== undefined), ); - console.log(updatedFields); - const updated = await this.db( + + const [updated] = await this.db( 'oidc_authorization_codes', ) .where('code', authorizationCode.code) .update(updatedFields) .returning('*'); - return this.rowToAuthorizationCode(updated[0]) as AuthorizationCode; + return this.rowToAuthorizationCode(updated) as AuthorizationCode; + } + + async createAccessToken(accessToken: Omit) { + const now = DateTime.now().toString(); + + await this.db('oidc_access_tokens').insert({ + token_id: accessToken.tokenId, + client_id: accessToken.clientId, + user_entity_ref: accessToken.userEntityRef, + scope: accessToken.scope, + created_at: now, + expires_at: accessToken.expiresAt, + revoked: accessToken.revoked ?? false, + }); + + return { + ...accessToken, + createdAt: now, + }; + } + + async getAccessToken({ tokenId }: { tokenId: string }) { + const accessToken = await this.db('oidc_access_tokens') + .where('token_id', tokenId) + .first(); + + if (!accessToken) { + return null; + } + + return this.rowToAccessToken(accessToken) as AccessToken; + } + + async updateAccessToken( + accessToken: Partial & { tokenId: string }, + ) { + const row = this.accessTokenToRow(accessToken); + const updatedFields = Object.fromEntries( + Object.entries(row).filter(([_, value]) => value !== undefined), + ); + const [updated] = await this.db('oidc_access_tokens') + .where('token_id', accessToken.tokenId) + .update(updatedFields) + .returning('*'); + + return this.rowToAccessToken(updated) as AccessToken; } private rowToClient(row: Partial): Partial { @@ -226,4 +299,32 @@ export class OidcDatabase { used: Boolean(row.used), }; } + + private accessTokenToRow( + accessToken: Partial, + ): Partial { + return { + token_id: accessToken.tokenId, + client_id: accessToken.clientId, + user_entity_ref: accessToken.userEntityRef, + scope: accessToken.scope, + created_at: accessToken.createdAt, + expires_at: accessToken.expiresAt, + revoked: accessToken.revoked, + }; + } + + private rowToAccessToken( + row: Partial, + ): Partial { + return { + tokenId: row.token_id, + clientId: row.client_id, + userEntityRef: row.user_entity_ref, + scope: row.scope ?? undefined, + createdAt: row.created_at, + expiresAt: row.expires_at, + revoked: Boolean(row.revoked), + }; + } } From bbda7485f6439938d5f5933f5a07fd09a922be18 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Thu, 3 Jul 2025 10:24:53 +0200 Subject: [PATCH 04/28] feat: adding client register Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- .../src/service/OidcRouter.test.ts | 2 ++ .../auth-backend/src/service/OidcRouter.ts | 28 ++++++++++++++++++- .../auth-backend/src/service/OidcService.ts | 28 +++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index dbc6c88f93..8e53abbcaf 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -37,6 +37,8 @@ describe('OidcRouter', () => { }), } as unknown as UserInfoDatabase; + const mockOidc = {}; + const { server } = await startTestBackend({ features: [ createBackendPlugin({ diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index 6ada071c44..219e026a53 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -15,10 +15,12 @@ */ import Router from 'express-promise-router'; import { OidcService } from './OidcService'; -import { AuthenticationError } from '@backstage/errors'; +import { AuthenticationError, isError } from '@backstage/errors'; import { AuthService } from '@backstage/backend-plugin-api'; import { TokenIssuer } from '../identity/types'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; +import { rest } from 'lodash'; +import { OidcDatabase } from '../database/OidcDatabase'; export class OidcRouter { private constructor(private readonly oidc: OidcService) {} @@ -28,6 +30,7 @@ export class OidcRouter { tokenIssuer: TokenIssuer; baseUrl: string; userInfo: UserInfoDatabase; + oidc: OidcDatabase; }) { return new OidcRouter(OidcService.create(options)); } @@ -68,6 +71,29 @@ export class OidcRouter { res.json(userInfo); }); + router.get('/v1/register', async (req, res) => { + // todo(blam): maybe add zod types for validating input + const registrationRequest = req.body; + if (!registrationRequest.redirect_uris?.length) { + res.status(400).json({ + error: 'invalid_request', + error_description: 'redirect_uris is required', + }); + return; + } + + try { + res.json(await this.oidc.registerClient(registrationRequest)); + } catch (e) { + res.status(500).json({ + error: 'server_error', + error_description: `Failed to register client: ${ + isError(e) ? e.message : 'Unknown error' + }`, + }); + } + }); + return router; } } diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index 5024b2cc8c..919c0dac64 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -18,6 +18,8 @@ import { TokenIssuer } from '../identity/types'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; import { InputError } from '@backstage/errors'; import { decodeJwt } from 'jose'; +import crypto from 'crypto'; +import { OidcDatabase } from '../database/OidcDatabase'; export class OidcService { private constructor( @@ -25,6 +27,7 @@ export class OidcService { private readonly tokenIssuer: TokenIssuer, private readonly baseUrl: string, private readonly userInfo: UserInfoDatabase, + private readonly oidc: OidcDatabase, ) {} static create(options: { @@ -32,12 +35,14 @@ export class OidcService { tokenIssuer: TokenIssuer; baseUrl: string; userInfo: UserInfoDatabase; + oidc: OidcDatabase; }) { return new OidcService( options.auth, options.tokenIssuer, options.baseUrl, options.userInfo, + options.oidc, ); } @@ -65,6 +70,8 @@ export class OidcService { token_endpoint_auth_methods_supported: [], claims_supported: ['sub', 'ent'], grant_types_supported: [], + authorization_endpoint: `${this.baseUrl}/v1/authorize`, + registration_endpoint: `${this.baseUrl}/v1/register`, }; } @@ -89,4 +96,25 @@ export class OidcService { } return await this.userInfo.getUserInfo(userEntityRef); } + + public async registerClient(opts: { + responseTypes?: string[]; + grantTypes?: string[]; + clientName: string; + redirectUris?: string[]; + scope?: string; + }) { + const generatedClientId = crypto.randomUUID(); + const generatedClientSecret = crypto.randomUUID(); + + return await this.oidc.createClient({ + clientId: generatedClientId, + clientName: opts.clientName, + clientSecret: generatedClientSecret, + redirectUris: opts.redirectUris ?? [], + responseTypes: opts.responseTypes ?? ['code'], + grantTypes: opts.grantTypes ?? ['authorization_code'], + scope: opts.scope, + }); + } } From 628322d19bbd1284195ed6a7cac6743c0192e6cc Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Thu, 3 Jul 2025 10:46:06 +0200 Subject: [PATCH 05/28] chore: issue a token for guest entity ref Signed-off-by: benjdlambert --- .../src/service/OidcRouter.test.ts | 17 +- .../auth-backend/src/service/OidcRouter.ts | 116 +++++++++- .../auth-backend/src/service/OidcService.ts | 200 +++++++++++++++++- plugins/auth-backend/src/service/router.ts | 4 + 4 files changed, 327 insertions(+), 10 deletions(-) diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index 8e53abbcaf..ccc2426035 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -23,6 +23,7 @@ import Router from 'express-promise-router'; import request from 'supertest'; import { OidcRouter } from './OidcRouter'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; +import { OidcDatabase } from '../database/OidcDatabase'; describe('OidcRouter', () => { describe('/v1/userinfo', () => { @@ -37,7 +38,12 @@ describe('OidcRouter', () => { }), } as unknown as UserInfoDatabase; - const mockOidc = {}; + const mockOidc = { + createClient: jest.fn().mockResolvedValue({ + clientId: 'test', + clientSecret: 'test', + }), + } as unknown as OidcDatabase; const { server } = await startTestBackend({ features: [ @@ -55,6 +61,7 @@ describe('OidcRouter', () => { tokenIssuer: {} as any, baseUrl: 'http://localhost:7000', userInfo: mockUserInfo, + oidc: mockOidc, }).getRouter(), ); httpRouter.use(router); @@ -101,6 +108,13 @@ describe('OidcRouter', () => { }), } as unknown as UserInfoDatabase; + const mockOidc = { + createClient: jest.fn().mockResolvedValue({ + clientId: 'test', + clientSecret: 'test', + }), + } as unknown as OidcDatabase; + const { server } = await startTestBackend({ features: [ createBackendPlugin({ @@ -117,6 +131,7 @@ describe('OidcRouter', () => { tokenIssuer: {} as any, baseUrl: 'http://localhost:7000', userInfo: mockUserInfo, + oidc: mockOidc, }).getRouter(), ); httpRouter.use(router); diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index 219e026a53..721dedf8c3 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -19,7 +19,6 @@ import { AuthenticationError, isError } from '@backstage/errors'; import { AuthService } from '@backstage/backend-plugin-api'; import { TokenIssuer } from '../identity/types'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; -import { rest } from 'lodash'; import { OidcDatabase } from '../database/OidcDatabase'; export class OidcRouter { @@ -47,11 +46,118 @@ export class OidcRouter { res.json({ keys }); }); - router.get('/v1/token', (_req, res) => { - res.status(501).send('Not Implemented'); + router.get('/v1/authorize', async (req, res) => { + // todo(blam): maybe add zod types for validating input + const { + client_id: clientId, + redirect_uri: redirectUri, + response_type: responseType, + scope, + state, + nonce, + code_challenge: codeChallenge, + code_challenge_method: codeChallengeMethod, + } = req.query; + + if (!clientId || !redirectUri || !responseType) { + return res.status(400).json({ + error: 'invalid_request', + error_description: + 'Missing required parameters: client_id, redirect_uri, response_type', + }); + } + + try { + // For simplicity, we'll use a default user entity ref for now + // In a real implementation, this should be obtained from the authenticated user + const userEntityRef = 'user:default/guest'; + + const { redirectUrl } = await this.oidc.authorize({ + clientId: clientId as string, + redirectUri: redirectUri as string, + responseType: responseType as string, + scope: scope as string, + state: state as string, + nonce: nonce as string, + codeChallenge: codeChallenge as string, + codeChallengeMethod: codeChallengeMethod as string, + userEntityRef, + }); + + return res.redirect(redirectUrl); + } catch (error) { + const errorParams = new URLSearchParams(); + errorParams.append( + 'error', + isError(error) ? error.name : 'server_error', + ); + errorParams.append( + 'error_description', + isError(error) ? error.message : 'Unknown error', + ); + if (state) { + errorParams.append('state', state as string); + } + + const redirectUrl = new URL(redirectUri as string); + redirectUrl.search = errorParams.toString(); + return res.redirect(redirectUrl.toString()); + } }); - // This endpoint doesn't use the regular HttpAuthoidc, since the contract + router.post('/v1/token', async (req, res) => { + // todo(blam): maybe add zod types for validating input + const { + grant_type: grantType, + code, + client_id: clientId, + client_secret: clientSecret, + redirect_uri: redirectUri, + code_verifier: codeVerifier, + } = req.body; + + if (!grantType || !code || !clientId || !clientSecret || !redirectUri) { + return res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing required parameters', + }); + } + + try { + const result = await this.oidc.exchangeCodeForToken({ + code, + clientId, + clientSecret, + redirectUri, + codeVerifier, + grantType, + }); + + return res.json(result); + } catch (error) { + if (isError(error)) { + if (error.name === 'AuthenticationError') { + return res.status(401).json({ + error: 'invalid_client', + error_description: error.message, + }); + } + if (error.name === 'InputError') { + return res.status(400).json({ + error: 'invalid_request', + error_description: error.message, + }); + } + } + + return res.status(500).json({ + error: 'server_error', + error_description: isError(error) ? error.message : 'Unknown error', + }); + } + }); + + // This endpoint doesn't use the regular HttpAuth, since the contract // is specifically for the header to be communicated in the Authorization // header, regardless of token type router.get('/v1/userinfo', async (req, res) => { @@ -71,7 +177,7 @@ export class OidcRouter { res.json(userInfo); }); - router.get('/v1/register', async (req, res) => { + router.post('/v1/register', async (req, res) => { // todo(blam): maybe add zod types for validating input const registrationRequest = req.body; if (!registrationRequest.redirect_uris?.length) { diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index 919c0dac64..104c2290ae 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -16,10 +16,11 @@ import { AuthService } from '@backstage/backend-plugin-api'; import { TokenIssuer } from '../identity/types'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; -import { InputError } from '@backstage/errors'; +import { InputError, AuthenticationError } from '@backstage/errors'; import { decodeJwt } from 'jose'; import crypto from 'crypto'; import { OidcDatabase } from '../database/OidcDatabase'; +import { DateTime } from 'luxon'; export class OidcService { private constructor( @@ -52,7 +53,7 @@ export class OidcService { token_endpoint: `${this.baseUrl}/v1/token`, userinfo_endpoint: `${this.baseUrl}/v1/userinfo`, jwks_uri: `${this.baseUrl}/.well-known/jwks.json`, - response_types_supported: ['id_token'], + response_types_supported: ['code', 'id_token'], subject_types_supported: ['public'], id_token_signing_alg_values_supported: [ 'RS256', @@ -67,11 +68,15 @@ export class OidcService { 'EdDSA', ], scopes_supported: ['openid'], - token_endpoint_auth_methods_supported: [], + token_endpoint_auth_methods_supported: [ + 'client_secret_basic', + 'client_secret_post', + ], claims_supported: ['sub', 'ent'], - grant_types_supported: [], + grant_types_supported: ['authorization_code'], authorization_endpoint: `${this.baseUrl}/v1/authorize`, registration_endpoint: `${this.baseUrl}/v1/register`, + code_challenge_methods_supported: ['S256', 'plain'], }; } @@ -117,4 +122,191 @@ export class OidcService { scope: opts.scope, }); } + + public async authorize(opts: { + clientId: string; + redirectUri: string; + responseType: string; + scope?: string; + state?: string; + nonce?: string; + codeChallenge?: string; + codeChallengeMethod?: string; + userEntityRef: string; + }) { + const { + clientId, + redirectUri, + responseType, + scope, + state, + nonce, + codeChallenge, + codeChallengeMethod, + userEntityRef, + } = opts; + + if (responseType !== 'code') { + throw new InputError('Only authorization code flow is supported'); + } + + const client = await this.oidc.getClient({ clientId }); + if (!client) { + throw new InputError('Invalid client_id'); + } + + if (!client.redirectUris.includes(redirectUri)) { + throw new InputError('Invalid redirect_uri'); + } + + if (codeChallenge) { + if ( + !codeChallengeMethod || + !['S256', 'plain'].includes(codeChallengeMethod) + ) { + throw new InputError('Invalid code_challenge_method'); + } + } + + const authorizationCode = crypto.randomBytes(32).toString('base64url'); + const expiresAt = DateTime.now().plus({ minutes: 10 }).toISO(); + + await this.oidc.createAuthorizationCode({ + code: authorizationCode, + clientId, + userEntityRef, + redirectUri, + scope, + codeChallenge, + codeChallengeMethod, + nonce, + expiresAt, + }); + + const redirectUrl = new URL(redirectUri); + redirectUrl.searchParams.append('code', authorizationCode); + if (state) { + redirectUrl.searchParams.append('state', state); + } + + return { + redirectUrl: redirectUrl.toString(), + }; + } + + public async exchangeCodeForToken(params: { + code: string; + clientId: string; + clientSecret: string; + redirectUri: string; + codeVerifier?: string; + grantType: string; + }) { + const { + code, + clientId, + clientSecret, + redirectUri, + codeVerifier, + grantType, + } = params; + + if (grantType !== 'authorization_code') { + throw new InputError('Unsupported grant type'); + } + + const client = await this.oidc.getClient({ clientId }); + if (!client) { + throw new AuthenticationError('Invalid client'); + } + + if (client.clientSecret !== clientSecret) { + throw new AuthenticationError('Invalid client credentials'); + } + + const authCode = await this.oidc.getAuthorizationCode({ code }); + if (!authCode) { + throw new AuthenticationError('Invalid authorization code'); + } + + if (DateTime.fromISO(authCode.expiresAt) < DateTime.now()) { + throw new AuthenticationError('Authorization code expired'); + } + + if (authCode.used) { + throw new AuthenticationError('Authorization code already used'); + } + + if (authCode.clientId !== clientId) { + throw new AuthenticationError('Client ID mismatch'); + } + + if (authCode.redirectUri !== redirectUri) { + throw new AuthenticationError('Redirect URI mismatch'); + } + + if (authCode.codeChallenge) { + if (!codeVerifier) { + throw new AuthenticationError('Code verifier required for PKCE'); + } + + if ( + !this.verifyPkce( + authCode.codeChallenge, + codeVerifier, + authCode.codeChallengeMethod, + ) + ) { + throw new AuthenticationError('Invalid code verifier'); + } + } + + await this.oidc.updateAuthorizationCode({ + code, + used: true, + }); + + const accessTokenId = crypto.randomUUID(); + const expiresAt = DateTime.now().plus({ hours: 1 }).toISO(); + + await this.oidc.createAccessToken({ + tokenId: accessTokenId, + clientId, + userEntityRef: authCode.userEntityRef, + scope: authCode.scope, + expiresAt, + }); + + const { token } = await this.tokenIssuer.issueToken({ + claims: { + sub: authCode.userEntityRef, + }, + }); + + return { + access_token: token, + token_type: 'Bearer', + expires_in: 3600, + id_token: token, + scope: authCode.scope || 'openid', + }; + } + + private verifyPkce( + codeChallenge: string, + codeVerifier: string, + method?: string, + ): boolean { + if (!method || method === 'plain') { + return codeChallenge === codeVerifier; + } + + if (method === 'S256') { + const hash = crypto.createHash('sha256').update(codeVerifier).digest(); + const base64urlHash = hash.toString('base64url'); + return codeChallenge === base64urlHash; + } + + return false; + } } diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 5012c90106..f81390daf8 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -40,6 +40,7 @@ import { StaticTokenIssuer } from '../identity/StaticTokenIssuer'; import { StaticKeyStore } from '../identity/StaticKeyStore'; import { bindProviderRouters, ProviderFactories } from '../providers/router'; import { OidcRouter } from './OidcRouter'; +import { OidcDatabase } from '../database/OidcDatabase'; interface RouterOptions { logger: LoggerService; @@ -147,11 +148,14 @@ export async function createRouter( userInfo, }); + const oidc = await OidcDatabase.create({ database }); + const oidcRouter = OidcRouter.create({ auth: options.auth, tokenIssuer, baseUrl: authUrl, userInfo, + oidc, }); router.use(oidcRouter.getRouter()); From 0d142d95ec0e770ca1d255d14ccfe7cfb8770e75 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Thu, 3 Jul 2025 11:16:51 +0200 Subject: [PATCH 06/28] chore: implementing the register and code exchange Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- .../auth-backend/src/database/OidcDatabase.ts | 18 +++++-- .../auth-backend/src/service/OidcRouter.ts | 49 +++++++++++++++---- plugins/auth-backend/src/service/router.ts | 1 + plugins/mcp-actions-backend/src/plugin.ts | 27 +++++++++- 4 files changed, 80 insertions(+), 15 deletions(-) diff --git a/plugins/auth-backend/src/database/OidcDatabase.ts b/plugins/auth-backend/src/database/OidcDatabase.ts index cbaf101ed9..2eeacb7ffe 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.ts @@ -93,11 +93,8 @@ type AccessToken = { }; /** - * This class is an implementation for the Database operations that power the OIDC sign-in flow. - * * This class provides database operations for OpenID Connect (OIDC) authentication flows. - * It manages OIDC clients, authorization codes, and access tokens in the database, as well as the consent requests - * for the frontend plugin to accept. + * It manages OIDC clients, authorization codes, and access tokens in the database. */ export class OidcDatabase { private constructor(private readonly db: Knex) {} @@ -109,7 +106,18 @@ export class OidcDatabase { async createClient(client: Omit) { const now = DateTime.now().toString(); - + console.log({ + client_id: client.clientId, + client_secret: client.clientSecret, + client_name: client.clientName, + created_at: now, + expires_at: client.expiresAt, + response_types: JSON.stringify(client.responseTypes), + grant_types: JSON.stringify(client.grantTypes), + redirect_uris: JSON.stringify(client.redirectUris), + scope: client.scope, + metadata: JSON.stringify(client.metadata), + }); await this.db('oidc_clients').insert({ client_id: client.clientId, client_secret: client.clientSecret, diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index 721dedf8c3..9706554ba5 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -16,27 +16,34 @@ import Router from 'express-promise-router'; import { OidcService } from './OidcService'; import { AuthenticationError, isError } from '@backstage/errors'; -import { AuthService } from '@backstage/backend-plugin-api'; +import { AuthService, LoggerService } from '@backstage/backend-plugin-api'; import { TokenIssuer } from '../identity/types'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; import { OidcDatabase } from '../database/OidcDatabase'; +import { json } from 'express'; export class OidcRouter { - private constructor(private readonly oidc: OidcService) {} + private constructor( + private readonly oidc: OidcService, + private readonly logger: LoggerService, + ) {} static create(options: { auth: AuthService; tokenIssuer: TokenIssuer; baseUrl: string; + logger: LoggerService; userInfo: UserInfoDatabase; oidc: OidcDatabase; }) { - return new OidcRouter(OidcService.create(options)); + return new OidcRouter(OidcService.create(options), options.logger); } public getRouter() { const router = Router(); + router.use(json()); + router.get('/.well-known/openid-configuration', (_req, res) => { res.json(this.oidc.getConfiguration()); }); @@ -60,6 +67,7 @@ export class OidcRouter { } = req.query; if (!clientId || !redirectUri || !responseType) { + this.logger.error(`Failed to authorize: Missing required parameters`); return res.status(400).json({ error: 'invalid_request', error_description: @@ -68,8 +76,8 @@ export class OidcRouter { } try { - // For simplicity, we'll use a default user entity ref for now - // In a real implementation, this should be obtained from the authenticated user + // use default user entity ref for now, as we need a redirect to the frontend plugin + // for the consent flow in order to issue the right token for the right user. const userEntityRef = 'user:default/guest'; const { redirectUrl } = await this.oidc.authorize({ @@ -117,6 +125,9 @@ export class OidcRouter { } = req.body; if (!grantType || !code || !clientId || !clientSecret || !redirectUri) { + this.logger.error( + `Failed to exchange code for token: Missing required parameters`, + ); return res.status(400).json({ error: 'invalid_request', error_description: 'Missing required parameters', @@ -135,6 +146,12 @@ export class OidcRouter { return res.json(result); } catch (error) { + const description = isError(error) ? error.message : 'Unknown error'; + this.logger.error( + `Failed to exchange code for token: ${description}`, + error, + ); + if (isError(error)) { if (error.name === 'AuthenticationError') { return res.status(401).json({ @@ -180,6 +197,7 @@ export class OidcRouter { router.post('/v1/register', async (req, res) => { // todo(blam): maybe add zod types for validating input const registrationRequest = req.body; + if (!registrationRequest.redirect_uris?.length) { res.status(400).json({ error: 'invalid_request', @@ -189,13 +207,26 @@ export class OidcRouter { } try { - res.json(await this.oidc.registerClient(registrationRequest)); + const client = await this.oidc.registerClient({ + clientName: registrationRequest.client_name, + redirectUris: registrationRequest.redirect_uris, + responseTypes: registrationRequest.response_types, + grantTypes: registrationRequest.grant_types, + scope: registrationRequest.scope, + }); + + res.status(201).json({ + client_id: client.clientId, + redirect_uris: client.redirectUris, + client_secret: client.clientSecret, + }); } catch (e) { + const description = isError(e) ? e.message : 'Unknown error'; + this.logger.error(`Failed to register client: ${description}`, e); + res.status(500).json({ error: 'server_error', - error_description: `Failed to register client: ${ - isError(e) ? e.message : 'Unknown error' - }`, + error_description: `Failed to register client: ${description}`, }); } }); diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index f81390daf8..5eb3450451 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -156,6 +156,7 @@ export async function createRouter( baseUrl: authUrl, userInfo, oidc, + logger, }); router.use(oidcRouter.getRouter()); diff --git a/plugins/mcp-actions-backend/src/plugin.ts b/plugins/mcp-actions-backend/src/plugin.ts index f1e89417e9..2e29847964 100644 --- a/plugins/mcp-actions-backend/src/plugin.ts +++ b/plugins/mcp-actions-backend/src/plugin.ts @@ -42,8 +42,17 @@ export const mcpPlugin = createBackendPlugin({ httpRouter: coreServices.httpRouter, actions: actionsServiceRef, registry: actionsRegistryServiceRef, + rootRouter: coreServices.rootHttpRouter, + discovery: coreServices.discovery, }, - async init({ actions, logger, httpRouter, httpAuth }) { + async init({ + actions, + logger, + httpRouter, + httpAuth, + rootRouter, + discovery, + }) { const mcpService = await McpService.create({ actions, }); @@ -66,6 +75,22 @@ export const mcpPlugin = createBackendPlugin({ router.use('/v1', streamableRouter); httpRouter.use(router); + + // todo(blam): there's probably a better way to proxy this, but it's required + // for mcp auth spec that it lives on the root of the mcp entrypoint server. + const authRouter = Router(); + authRouter.use('/', async (_, res) => { + const authBaseUrl = await discovery.getBaseUrl('auth'); + + const oidcResponse = await fetch( + `${authBaseUrl}/.well-known/openid-configuration`, + ); + + const oidcResponseJson = await oidcResponse.json(); + + res.json(oidcResponseJson); + }); + rootRouter.use('/.well-known/oauth-authorization-server', authRouter); }, }); }, From e0473b52e9bcbffec466c3fa8bf5be4d36be5d14 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Thu, 3 Jul 2025 11:28:11 +0200 Subject: [PATCH 07/28] chore: little cleanup Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- plugins/auth-backend/src/database/OidcDatabase.ts | 13 +------------ plugins/auth-backend/src/service/OidcRouter.ts | 8 +++++++- plugins/auth-backend/src/service/OidcService.ts | 8 ++++---- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/plugins/auth-backend/src/database/OidcDatabase.ts b/plugins/auth-backend/src/database/OidcDatabase.ts index 2eeacb7ffe..12554fba57 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.ts @@ -106,18 +106,7 @@ export class OidcDatabase { async createClient(client: Omit) { const now = DateTime.now().toString(); - console.log({ - client_id: client.clientId, - client_secret: client.clientSecret, - client_name: client.clientName, - created_at: now, - expires_at: client.expiresAt, - response_types: JSON.stringify(client.responseTypes), - grant_types: JSON.stringify(client.grantTypes), - redirect_uris: JSON.stringify(client.redirectUris), - scope: client.scope, - metadata: JSON.stringify(client.metadata), - }); + await this.db('oidc_clients').insert({ client_id: client.clientId, client_secret: client.clientSecret, diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index 9706554ba5..0e14b91d79 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -144,7 +144,13 @@ export class OidcRouter { grantType, }); - return res.json(result); + return res.json({ + access_token: result.accessToken, + token_type: result.tokenType, + expires_in: result.expiresIn, + id_token: result.idToken, + scope: result.scope, + }); } catch (error) { const description = isError(error) ? error.message : 'Unknown error'; this.logger.error( diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index 104c2290ae..cc60b2b432 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -284,10 +284,10 @@ export class OidcService { }); return { - access_token: token, - token_type: 'Bearer', - expires_in: 3600, - id_token: token, + accessToken: token, + tokenType: 'Bearer', + expiresIn: 3600, + idToken: token, scope: authCode.scope || 'openid', }; } From b50e18b25fb4b264ac66945ea60f760eb3963fef Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Thu, 3 Jul 2025 19:32:37 +0200 Subject: [PATCH 08/28] chore: reworking the migrations to simplify the tables structure Signed-off-by: benjdlambert --- ...20250701120000_oidc_client_registration.js | 158 +++++++++++------- 1 file changed, 97 insertions(+), 61 deletions(-) diff --git a/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js b/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js index d4863965de..bf8ee521f0 100644 --- a/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js +++ b/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js @@ -1,5 +1,5 @@ /* - * Copyright 2024 The Backstage Authors + * Copyright 2025 The Backstage Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,8 @@ * @param {import('knex').Knex} knex */ exports.up = async function up(knex) { + // These tables make up the OIDC client registration flow. + // Clients are the top of the tree, that are created by the client registration flow. await knex.schema.createTable('oidc_clients', table => { table.comment( 'OIDC clients that are registered via dynamic client registration', @@ -41,36 +43,27 @@ exports.up = async function up(knex) { .notNullable() .comment('The name of the client, should be human readable'); - table - .text('redirect_uris', 'longtext') - .notNullable() - .comment('JSON array of valid redirect URIs'); - - table - .timestamp('created_at', { useTz: false, precision: 0 }) - .notNullable() - .defaultTo(knex.fn.now()) - .comment('Client registration timestamp'); - table .timestamp('expires_at', { useTz: false, precision: 0 }) .nullable() .comment('Client registration expiration timestamp'); table - .text('response_types', 'longtext') + .text('response_types') .notNullable() .comment('JSON array of supported response types'); table - .text('grant_types', 'longtext') + .text('grant_types') .notNullable() .comment('JSON array of supported grant types'); table - .text('scope') - .nullable() - .comment('Space-separated list of allowed scopes'); + .text('redirect_uris', 'longtext') + .notNullable() + .comment('Allowed redirect URIs as JSON array'); + + table.text('scope').nullable().comment('Default scopes for the client'); table .text('metadata', 'longtext') @@ -78,27 +71,29 @@ exports.up = async function up(knex) { .comment('Additional client metadata as JSON'); }); - await knex.schema.createTable('oidc_authorization_codes', table => { - table.comment('Authorization codes for OIDC authorization code flow'); - - table.string('code').primary().notNullable().comment('Authorization code'); + await knex.schema.createTable('oauth_authorization_sessions', table => { + table.comment('Core OAuth authorization sessions with shared context'); table - .string('client_id') + .string('id') + .primary() .notNullable() - .comment('Client ID that requested the code'); + .comment('Unique session identifier'); + + table.string('client_id').notNullable().comment('OIDC client identifier'); table .string('user_entity_ref') - .notNullable() - .comment('User entity reference who authorized'); + .nullable() + .comment('Backstage user entity reference'); - table - .text('redirect_uri') - .notNullable() - .comment('Redirect URI used in authorization request'); + table.text('redirect_uri').notNullable().comment('Client redirect URI'); - table.text('scope').nullable().comment('Requested scopes'); + table.text('scope').nullable().comment('Requested scopes space-separated'); + + table.string('state').nullable().comment('Client state parameter'); + + table.string('response_type').notNullable().comment('OAuth2 response type'); table.string('code_challenge').nullable().comment('PKCE code challenge'); @@ -107,65 +102,104 @@ exports.up = async function up(knex) { .nullable() .comment('PKCE code challenge method'); - table.string('nonce').nullable().comment('Nonce value for ID token'); + table.string('nonce').nullable().comment('OIDC nonce parameter'); table - .timestamp('created_at', { useTz: false, precision: 0 }) - .notNullable() - .defaultTo(knex.fn.now()) - .comment('Code creation timestamp'); + .enum('status', ['pending', 'approved', 'rejected', 'expired']) + .defaultTo('pending') + .comment('Authorization session status'); table .timestamp('expires_at', { useTz: false, precision: 0 }) .notNullable() - .comment('Code expiration timestamp'); + .comment('Session expiration timestamp'); + + table.foreign('client_id').references('client_id').inTable('oidc_clients'); + table.index(['client_id', 'user_entity_ref']); + table.index(['status', 'expires_at']); + }); + + await knex.schema.createTable('oidc_consent_requests', table => { + table.comment('User consent requests for OAuth authorization'); + + table + .string('id') + .primary() + .notNullable() + .comment('Unique consent request identifier'); + + table + .string('session_id') + .notNullable() + .comment('Authorization session identifier'); + + table + .timestamp('expires_at', { useTz: false, precision: 0 }) + .notNullable() + .comment('Consent request expiration timestamp'); + + table + .foreign('session_id') + .references('id') + .inTable('oauth_authorization_sessions') + .onDelete('CASCADE'); + }); + + await knex.schema.createTable('oidc_authorization_codes', table => { + table.comment('OAuth authorization codes for code exchange flow'); + + table + .string('code') + .primary() + .notNullable() + .comment('Unique authorization code'); + + table + .string('session_id') + .notNullable() + .comment('Authorization session identifier'); + + table + .timestamp('expires_at', { useTz: false, precision: 0 }) + .notNullable() + .comment('Authorization code expiration timestamp'); table .boolean('used') .defaultTo(false) - .comment('Whether the code has been used'); + .comment('Whether the authorization code has been used'); - table.foreign('client_id').references('client_id').inTable('oidc_clients'); + table + .foreign('session_id') + .references('id') + .inTable('oauth_authorization_sessions') + .onDelete('CASCADE'); }); await knex.schema.createTable('oidc_access_tokens', table => { - table.comment('Access tokens issued by OIDC server'); + table.comment('OAuth access tokens for API access'); table .string('token_id') .primary() .notNullable() - .comment('Unique token identifier'); + .comment('Unique access token identifier'); table - .string('client_id') + .string('session_id') .notNullable() - .comment('Client ID that owns the token'); - - table - .string('user_entity_ref') - .notNullable() - .comment('User entity reference'); - - table.text('scope').nullable().comment('Token scopes'); - - table - .timestamp('created_at', { useTz: false, precision: 0 }) - .notNullable() - .defaultTo(knex.fn.now()) - .comment('Token creation timestamp'); + .comment('Authorization session identifier'); table .timestamp('expires_at', { useTz: false, precision: 0 }) .notNullable() - .comment('Token expiration timestamp'); + .comment('Access token expiration timestamp'); table - .boolean('revoked') - .defaultTo(false) - .comment('Whether the token has been revoked'); - - table.foreign('client_id').references('client_id').inTable('oidc_clients'); + .foreign('session_id') + .references('id') + .inTable('oauth_authorization_sessions') + .onDelete('CASCADE'); }); }; @@ -175,5 +209,7 @@ exports.up = async function up(knex) { exports.down = async function down(knex) { await knex.schema.dropTable('oidc_access_tokens'); await knex.schema.dropTable('oidc_authorization_codes'); + await knex.schema.dropTable('oidc_consent_requests'); + await knex.schema.dropTable('oauth_authorization_sessions'); await knex.schema.dropTable('oidc_clients'); }; From 5b084e7223fc46df914bec322731ac5b88a45f4d Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Thu, 3 Jul 2025 19:33:15 +0200 Subject: [PATCH 09/28] chore: reworking the API for oidc-database Signed-off-by: benjdlambert --- .../src/database/OidcDatabase.test.ts | 375 ++++++++++++------ .../auth-backend/src/database/OidcDatabase.ts | 321 +++++++++------ 2 files changed, 448 insertions(+), 248 deletions(-) diff --git a/plugins/auth-backend/src/database/OidcDatabase.test.ts b/plugins/auth-backend/src/database/OidcDatabase.test.ts index 65f9e9824c..f9c2b83511 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.test.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.test.ts @@ -64,7 +64,6 @@ describe('Oidc Database', () => { scope: undefined, expiresAt: undefined, metadata: undefined, - createdAt: expect.any(String), }); }); @@ -94,11 +93,195 @@ describe('Oidc Database', () => { }); }); + describe('Authorization Sessions', () => { + it('should create and return an authorization session', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const client = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + const session = await oidc.createAuthorizationSession({ + id: 'test-session', + clientId: client.clientId, + userEntityRef: 'user:default/blam', + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + state: 'test-state', + codeChallenge: 'test-challenge', + codeChallengeMethod: 'S256', + nonce: 'test-nonce', + expiresAt: '2025-01-01T00:00:00Z', + }); + + expect(session).toEqual( + expect.objectContaining({ + id: 'test-session', + clientId: client.clientId, + userEntityRef: 'user:default/blam', + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + state: 'test-state', + codeChallenge: 'test-challenge', + codeChallengeMethod: 'S256', + nonce: 'test-nonce', + expiresAt: '2025-01-01T00:00:00Z', + status: 'pending', + }), + ); + }); + + it('should allow updating the authorization session', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const client = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + const session = await oidc.createAuthorizationSession({ + id: 'test-session', + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + expiresAt: '2025-01-01T00:00:00Z', + }); + + await expect( + oidc.updateAuthorizationSession({ + id: 'test-session', + userEntityRef: 'user:default/blam', + status: 'approved', + }), + ).resolves.toEqual({ + ...session, + userEntityRef: 'user:default/blam', + status: 'approved', + }); + }); + }); + + describe('Consent Requests', () => { + it('should create and return a consent request', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const client = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + const session = await oidc.createAuthorizationSession({ + id: 'test-session', + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + expiresAt: '2025-01-01T00:00:00Z', + }); + + const consentRequest = await oidc.createConsentRequest({ + id: 'test-consent', + sessionId: session.id, + expiresAt: '2025-01-01T00:00:00Z', + }); + + await expect( + oidc.getConsentRequest({ id: 'test-consent' }), + ).resolves.toEqual(consentRequest); + }); + + it('should return consent request with session data', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const client = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + const session = await oidc.createAuthorizationSession({ + id: 'test-session', + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + state: 'test-state', + expiresAt: '2025-01-01T00:00:00Z', + }); + + const consentRequest = await oidc.createConsentRequest({ + id: 'test-consent', + sessionId: session.id, + expiresAt: '2025-01-01T00:00:00Z', + }); + + const consentFromDb = await oidc.getConsentRequest({ + id: 'test-consent', + }); + const sessionFromDb = await oidc.getAuthorizationSession({ + id: consentFromDb!.sessionId, + }); + + expect(consentFromDb).toEqual(consentRequest); + expect(sessionFromDb).toEqual(session); + }); + + it('should delete consent request', async () => { + const { oidc } = await createOidcDatabase(databaseId); + + const client = await oidc.createClient({ + clientId: 'test-client', + clientName: 'Test Client', + clientSecret: 'test-secret', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }); + + const session = await oidc.createAuthorizationSession({ + id: 'test-session', + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + expiresAt: '2025-01-01T00:00:00Z', + }); + + await oidc.createConsentRequest({ + id: 'test-consent', + sessionId: session.id, + expiresAt: '2025-01-01T00:00:00Z', + }); + + await oidc.deleteConsentRequest({ id: 'test-consent' }); + + await expect( + oidc.getConsentRequest({ id: 'test-consent' }), + ).resolves.toBeNull(); + }); + }); + describe('Authorization Codes', () => { it('should create and return an authorization code', async () => { const { oidc } = await createOidcDatabase(databaseId); - const mockClient = await oidc.createClient({ + const client = await oidc.createClient({ clientId: 'test-client', clientName: 'Test Client', clientSecret: 'test-secret', @@ -107,35 +290,33 @@ describe('Oidc Database', () => { grantTypes: ['authorization_code'], }); - const authorizationCode = await oidc.createAuthorizationCode({ - code: 'test-code', - clientId: mockClient.clientId, - userEntityRef: 'user:default/blam', + const session = await oidc.createAuthorizationSession({ + id: 'test-session', + clientId: client.clientId, redirectUri: 'https://example.com/callback', - scope: undefined, - codeChallenge: 'test-challenge', - codeChallengeMethod: 'S256', - nonce: 'test-nonce', - expiresAt: '2025-01-01', + responseType: 'code', + expiresAt: '2025-01-01T00:00:00Z', }); - await expect( - oidc.getAuthorizationCode({ code: 'test-code' }), - ).resolves.toEqual(authorizationCode); + const authCode = await oidc.createAuthorizationCode({ + code: 'test-code', + sessionId: session.id, + expiresAt: '2025-01-01T00:00:00Z', + }); + + expect(authCode).toEqual( + expect.objectContaining({ + code: 'test-code', + sessionId: session.id, + expiresAt: '2025-01-01T00:00:00Z', + }), + ); }); - it('should return null if the authorization code does not exist', async () => { + it('should return authorization code with session data', async () => { const { oidc } = await createOidcDatabase(databaseId); - await expect( - oidc.getAuthorizationCode({ code: 'test-code' }), - ).resolves.toBeNull(); - }); - - it('should return the authorization code when created', async () => { - const { oidc } = await createOidcDatabase(databaseId); - - const mockClient = await oidc.createClient({ + const client = await oidc.createClient({ clientId: 'test-client', clientName: 'Test Client', clientSecret: 'test-secret', @@ -144,27 +325,40 @@ describe('Oidc Database', () => { grantTypes: ['authorization_code'], }); - const authorizationCode = await oidc.createAuthorizationCode({ - code: 'test-code', - clientId: mockClient.clientId, + const session = await oidc.createAuthorizationSession({ + id: 'test-session', + clientId: client.clientId, userEntityRef: 'user:default/blam', redirectUri: 'https://example.com/callback', - scope: undefined, + responseType: 'code', + scope: 'openid', codeChallenge: 'test-challenge', codeChallengeMethod: 'S256', nonce: 'test-nonce', - expiresAt: '2025-01-01', + expiresAt: '2025-01-01T00:00:00Z', }); - await expect( - oidc.getAuthorizationCode({ code: 'test-code' }), - ).resolves.toEqual(authorizationCode); + const authCode = await oidc.createAuthorizationCode({ + code: 'test-code', + sessionId: session.id, + expiresAt: '2025-01-01T00:00:00Z', + }); + + const authCodeFromDb = await oidc.getAuthorizationCode({ + code: 'test-code', + }); + const sessionFromDb = await oidc.getAuthorizationSession({ + id: authCodeFromDb!.sessionId, + }); + + expect(authCodeFromDb).toEqual(authCode); + expect(sessionFromDb).toEqual(session); }); it('should allow updating the authorization code', async () => { const { oidc } = await createOidcDatabase(databaseId); - const mockClient = await oidc.createClient({ + const client = await oidc.createClient({ clientId: 'test-client', clientName: 'Test Client', clientSecret: 'test-secret', @@ -173,24 +367,27 @@ describe('Oidc Database', () => { grantTypes: ['authorization_code'], }); - const authorizationCode = await oidc.createAuthorizationCode({ - code: 'test-code', - clientId: mockClient.clientId, - userEntityRef: 'user:default/blam', + const session = await oidc.createAuthorizationSession({ + id: 'test-session', + clientId: client.clientId, redirectUri: 'https://example.com/callback', - codeChallenge: 'test-challenge', - codeChallengeMethod: 'S256', - nonce: 'test-nonce', - expiresAt: '2025-01-01', + responseType: 'code', + expiresAt: '2025-01-01T00:00:00Z', }); - await expect( - oidc.updateAuthorizationCode({ - code: 'test-code', - used: true, - }), - ).resolves.toEqual({ - ...authorizationCode, + const authCode = await oidc.createAuthorizationCode({ + code: 'test-code', + sessionId: session.id, + expiresAt: '2025-01-01T00:00:00Z', + }); + + const updatedAuthCode = await oidc.updateAuthorizationCode({ + code: 'test-code', + used: true, + }); + + expect(updatedAuthCode).toEqual({ + ...authCode, used: true, }); }); @@ -200,7 +397,7 @@ describe('Oidc Database', () => { it('should create and return an access token', async () => { const { oidc } = await createOidcDatabase(databaseId); - const mockClient = await oidc.createClient({ + const client = await oidc.createClient({ clientId: 'test-client', clientName: 'Test Client', clientSecret: 'test-secret', @@ -209,81 +406,27 @@ describe('Oidc Database', () => { grantTypes: ['authorization_code'], }); - const accessToken = await oidc.createAccessToken({ - tokenId: 'test-token', - clientId: mockClient.clientId, - userEntityRef: 'user:default/blam', - expiresAt: '2025-01-01', - revoked: false, - }); - - await expect( - oidc.getAccessToken({ tokenId: 'test-token' }), - ).resolves.toEqual(accessToken); - }); - - it('should return null if the access token does not exist', async () => { - const { oidc } = await createOidcDatabase(databaseId); - - await expect( - oidc.getAccessToken({ tokenId: 'test-token' }), - ).resolves.toBeNull(); - }); - - it('should return the access token when created', async () => { - const { oidc } = await createOidcDatabase(databaseId); - - const mockClient = await oidc.createClient({ - clientId: 'test-client', - clientName: 'Test Client', - clientSecret: 'test-secret', - redirectUris: ['https://example.com/callback'], - responseTypes: ['code'], - grantTypes: ['authorization_code'], + const session = await oidc.createAuthorizationSession({ + id: 'test-session', + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + expiresAt: '2025-01-01T00:00:00Z', }); const accessToken = await oidc.createAccessToken({ tokenId: 'test-token', - clientId: mockClient.clientId, - userEntityRef: 'user:default/blam', - expiresAt: '2025-01-01', - revoked: false, + sessionId: session.id, + expiresAt: '2025-01-01T00:00:00Z', }); - await expect( - oidc.getAccessToken({ tokenId: 'test-token' }), - ).resolves.toEqual(accessToken); - }); - - it('should allow updating the access token', async () => { - const { oidc } = await createOidcDatabase(databaseId); - - const mockClient = await oidc.createClient({ - clientId: 'test-client', - clientName: 'Test Client', - clientSecret: 'test-secret', - redirectUris: ['https://example.com/callback'], - responseTypes: ['code'], - grantTypes: ['authorization_code'], - }); - - const accessToken = await oidc.createAccessToken({ - tokenId: 'test-token', - clientId: mockClient.clientId, - userEntityRef: 'user:default/blam', - expiresAt: '2025-01-01', - revoked: false, - }); - - await expect( - oidc.updateAccessToken({ + expect(accessToken).toEqual( + expect.objectContaining({ tokenId: 'test-token', - revoked: true, + sessionId: session.id, + expiresAt: '2025-01-01T00:00:00Z', }), - ).resolves.toEqual({ - ...accessToken, - revoked: true, - }); + ); }); }); }); diff --git a/plugins/auth-backend/src/database/OidcDatabase.ts b/plugins/auth-backend/src/database/OidcDatabase.ts index 12554fba57..0c75290f12 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.ts @@ -16,13 +16,10 @@ import { Knex } from 'knex'; import { AuthDatabase } from './AuthDatabase'; -import { DateTime } from 'luxon'; - type OidcClientRow = { client_id: string; client_secret: string; client_name: string; - created_at: string; expires_at: string | null; response_types: string; grant_types: string; @@ -31,31 +28,41 @@ type OidcClientRow = { metadata: string | null; }; -type OidcAuthorizationCodeRow = { - code: string; +type OAuthAuthorizationSessionRow = { + id: string; client_id: string; - user_entity_ref: string; + user_entity_ref: string | null; redirect_uri: string; scope: string | null; + state: string | null; + response_type: string; code_challenge: string | null; code_challenge_method: string | null; nonce: string | null; - created_at: string; + status: 'pending' | 'approved' | 'rejected' | 'expired'; expires_at: string; - used?: boolean; +}; + +type OidcConsentRequestRow = { + id: string; + session_id: string; + expires_at: string; +}; + +type OidcAuthorizationCodeRow = { + code: string; + session_id: string; + expires_at: string; + used: boolean; }; type OidcAccessTokenRow = { token_id: string; - client_id: string; - user_entity_ref: string; - scope: string | null; - created_at: string; + session_id: string; expires_at: string; - revoked?: boolean; }; -type Client = { +export type Client = { clientId: string; clientName: string; clientSecret: string; @@ -65,31 +72,40 @@ type Client = { scope?: string; expiresAt?: string; metadata?: Record; - createdAt: string; }; -type AuthorizationCode = { - code: string; +export type AuthorizationSession = { + id: string; clientId: string; - userEntityRef: string; + userEntityRef?: string; redirectUri: string; scope?: string; + state?: string; + responseType: string; codeChallenge?: string; codeChallengeMethod?: string; nonce?: string; - createdAt: string; + status: 'pending' | 'approved' | 'rejected' | 'expired'; + expiresAt: string; +}; + +export type ConsentRequest = { + id: string; + sessionId: string; + expiresAt: string; +}; + +export type AuthorizationCode = { + code: string; + sessionId: string; expiresAt: string; used: boolean; }; -type AccessToken = { +export type AccessToken = { tokenId: string; - clientId: string; - userEntityRef: string; - scope?: string; - createdAt: string; + sessionId: string; expiresAt: string; - revoked?: boolean; }; /** @@ -104,14 +120,11 @@ export class OidcDatabase { return new OidcDatabase(client); } - async createClient(client: Omit) { - const now = DateTime.now().toString(); - + async createClient(client: Client) { await this.db('oidc_clients').insert({ client_id: client.clientId, client_secret: client.clientSecret, client_name: client.clientName, - created_at: now, expires_at: client.expiresAt, response_types: JSON.stringify(client.responseTypes), grant_types: JSON.stringify(client.grantTypes), @@ -120,10 +133,7 @@ export class OidcDatabase { metadata: JSON.stringify(client.metadata), }); - return { - ...client, - createdAt: now, - }; + return client; } async getClient({ clientId }: { clientId: string }) { @@ -138,44 +148,122 @@ export class OidcDatabase { return this.rowToClient(client) as Client; } - async createAuthorizationCode( - authorizationCode: Omit, + async createAuthorizationSession( + session: Omit, ) { - const now = DateTime.now().toString(); + await this.db( + 'oauth_authorization_sessions', + ).insert({ + id: session.id, + client_id: session.clientId, + user_entity_ref: session.userEntityRef, + redirect_uri: session.redirectUri, + scope: session.scope, + state: session.state, + response_type: session.responseType, + code_challenge: session.codeChallenge, + code_challenge_method: session.codeChallengeMethod, + nonce: session.nonce, + status: 'pending', + expires_at: session.expiresAt, + }); + return { + ...session, + status: 'pending', + }; + } + + async updateAuthorizationSession( + session: Partial & { id: string }, + ) { + const row = this.authorizationSessionToRow(session); + const updatedFields = Object.fromEntries( + Object.entries(row).filter(([_, value]) => value !== undefined), + ); + + const [updated] = await this.db( + 'oauth_authorization_sessions', + ) + .where('id', session.id) + .update(updatedFields) + .returning('*'); + + return this.rowToAuthorizationSession(updated) as AuthorizationSession; + } + + async createConsentRequest(consentRequest: ConsentRequest) { + await this.db('oidc_consent_requests').insert({ + id: consentRequest.id, + session_id: consentRequest.sessionId, + expires_at: consentRequest.expiresAt, + }); + + return consentRequest; + } + + async getConsentRequest({ id }: { id: string }) { + const consentRequest = await this.db( + 'oidc_consent_requests', + ) + .where('id', id) + .first(); + + if (!consentRequest) { + return null; + } + + return this.rowToConsentRequest(consentRequest) as ConsentRequest; + } + + async getAuthorizationSession({ id }: { id: string }) { + const session = await this.db( + 'oauth_authorization_sessions', + ) + .where('id', id) + .first(); + + if (!session) { + return null; + } + + return this.rowToAuthorizationSession(session) as AuthorizationSession; + } + + async deleteConsentRequest({ id }: { id: string }) { + await this.db('oidc_consent_requests') + .where('id', id) + .delete(); + } + + async createAuthorizationCode( + authorizationCode: Omit, + ) { await this.db('oidc_authorization_codes').insert({ code: authorizationCode.code, - client_id: authorizationCode.clientId, - user_entity_ref: authorizationCode.userEntityRef, - redirect_uri: authorizationCode.redirectUri, - scope: authorizationCode.scope, - code_challenge: authorizationCode.codeChallenge, - code_challenge_method: authorizationCode.codeChallengeMethod, - nonce: authorizationCode.nonce, + session_id: authorizationCode.sessionId, expires_at: authorizationCode.expiresAt, - created_at: now, used: false, }); return { ...authorizationCode, - createdAt: now, used: false, }; } async getAuthorizationCode({ code }: { code: string }) { - const authorizationCode = await this.db( + const authCode = await this.db( 'oidc_authorization_codes', ) .where('code', code) .first(); - if (!authorizationCode) { + if (!authCode) { return null; } - return this.rowToAuthorizationCode(authorizationCode) as AuthorizationCode; + return this.rowToAuthorizationCode(authCode) as AuthorizationCode; } async updateAuthorizationCode( @@ -196,50 +284,14 @@ export class OidcDatabase { return this.rowToAuthorizationCode(updated) as AuthorizationCode; } - async createAccessToken(accessToken: Omit) { - const now = DateTime.now().toString(); - + async createAccessToken(accessToken: AccessToken) { await this.db('oidc_access_tokens').insert({ token_id: accessToken.tokenId, - client_id: accessToken.clientId, - user_entity_ref: accessToken.userEntityRef, - scope: accessToken.scope, - created_at: now, + session_id: accessToken.sessionId, expires_at: accessToken.expiresAt, - revoked: accessToken.revoked ?? false, }); - return { - ...accessToken, - createdAt: now, - }; - } - - async getAccessToken({ tokenId }: { tokenId: string }) { - const accessToken = await this.db('oidc_access_tokens') - .where('token_id', tokenId) - .first(); - - if (!accessToken) { - return null; - } - - return this.rowToAccessToken(accessToken) as AccessToken; - } - - async updateAccessToken( - accessToken: Partial & { tokenId: string }, - ) { - const row = this.accessTokenToRow(accessToken); - const updatedFields = Object.fromEntries( - Object.entries(row).filter(([_, value]) => value !== undefined), - ); - const [updated] = await this.db('oidc_access_tokens') - .where('token_id', accessToken.tokenId) - .update(updatedFields) - .returning('*'); - - return this.rowToAccessToken(updated) as AccessToken; + return accessToken; } private rowToClient(row: Partial): Partial { @@ -257,7 +309,54 @@ export class OidcDatabase { scope: row.scope ?? undefined, expiresAt: row.expires_at ?? undefined, metadata: row.metadata ? JSON.parse(row.metadata) : undefined, - createdAt: row.created_at, + }; + } + + private authorizationSessionToRow( + session: Partial, + ): Partial { + return { + id: session.id, + client_id: session.clientId, + user_entity_ref: session.userEntityRef, + redirect_uri: session.redirectUri, + scope: session.scope, + state: session.state, + response_type: session.responseType, + code_challenge: session.codeChallenge, + code_challenge_method: session.codeChallengeMethod, + nonce: session.nonce, + status: session.status, + expires_at: session.expiresAt, + }; + } + + private rowToAuthorizationSession( + row: Partial, + ): Partial { + return { + id: row.id, + clientId: row.client_id, + userEntityRef: row.user_entity_ref ?? undefined, + redirectUri: row.redirect_uri, + scope: row.scope ?? undefined, + state: row.state ?? undefined, + responseType: row.response_type, + codeChallenge: row.code_challenge ?? undefined, + codeChallengeMethod: row.code_challenge_method ?? undefined, + nonce: row.nonce ?? undefined, + status: row.status, + expiresAt: row.expires_at, + }; + } + + private rowToConsentRequest( + row: Partial, + ): Partial { + return { + id: row.id, + sessionId: row.session_id, + expiresAt: row.expires_at, }; } @@ -266,14 +365,7 @@ export class OidcDatabase { ): Partial { return { code: authorizationCode.code, - client_id: authorizationCode.clientId, - user_entity_ref: authorizationCode.userEntityRef, - redirect_uri: authorizationCode.redirectUri, - scope: authorizationCode.scope, - code_challenge: authorizationCode.codeChallenge, - code_challenge_method: authorizationCode.codeChallengeMethod, - nonce: authorizationCode.nonce, - created_at: authorizationCode.createdAt, + session_id: authorizationCode.sessionId, expires_at: authorizationCode.expiresAt, used: authorizationCode.used, }; @@ -284,44 +376,9 @@ export class OidcDatabase { ): Partial { return { code: row.code, - clientId: row.client_id, - userEntityRef: row.user_entity_ref, - redirectUri: row.redirect_uri, - scope: row.scope ?? undefined, - codeChallenge: row.code_challenge ?? undefined, - codeChallengeMethod: row.code_challenge_method ?? undefined, - nonce: row.nonce ?? undefined, - createdAt: row.created_at, + sessionId: row.session_id, expiresAt: row.expires_at, used: Boolean(row.used), }; } - - private accessTokenToRow( - accessToken: Partial, - ): Partial { - return { - token_id: accessToken.tokenId, - client_id: accessToken.clientId, - user_entity_ref: accessToken.userEntityRef, - scope: accessToken.scope, - created_at: accessToken.createdAt, - expires_at: accessToken.expiresAt, - revoked: accessToken.revoked, - }; - } - - private rowToAccessToken( - row: Partial, - ): Partial { - return { - tokenId: row.token_id, - clientId: row.client_id, - userEntityRef: row.user_entity_ref, - scope: row.scope ?? undefined, - createdAt: row.created_at, - expiresAt: row.expires_at, - revoked: Boolean(row.revoked), - }; - } } From eb2297fe6d792dd03aa766b97802adee7d1f18b7 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Thu, 3 Jul 2025 19:36:00 +0200 Subject: [PATCH 10/28] chore: updating the oidc service to handle consent Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- .../auth-backend/src/service/OidcService.ts | 249 ++++++++++++++++-- 1 file changed, 234 insertions(+), 15 deletions(-) diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index cc60b2b432..4214f53a8c 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -112,6 +112,9 @@ export class OidcService { const generatedClientId = crypto.randomUUID(); const generatedClientSecret = crypto.randomUUID(); + // todo(blam): add validation for redirectUris here. + // should be a list of urls and / or allowed schemes or something. + return await this.oidc.createClient({ clientId: generatedClientId, clientName: opts.clientName, @@ -123,6 +126,194 @@ export class OidcService { }); } + public async createConsentRequest(opts: { + clientId: string; + redirectUri: string; + responseType: string; + scope?: string; + state?: string; + nonce?: string; + codeChallenge?: string; + codeChallengeMethod?: string; + }) { + const { + clientId, + redirectUri, + responseType, + scope, + state, + nonce, + codeChallenge, + codeChallengeMethod, + } = opts; + + if (responseType !== 'code') { + throw new InputError('Only authorization code flow is supported'); + } + + const client = await this.oidc.getClient({ clientId }); + if (!client) { + throw new InputError('Invalid client_id'); + } + + if (!client.redirectUris.includes(redirectUri)) { + throw new InputError('Invalid redirect_uri'); + } + + if (codeChallenge) { + if ( + !codeChallengeMethod || + !['S256', 'plain'].includes(codeChallengeMethod) + ) { + throw new InputError('Invalid code_challenge_method'); + } + } + + const sessionId = crypto.randomUUID(); + const sessionExpiresAt = DateTime.now().plus({ hours: 1 }).toISO(); + + await this.oidc.createAuthorizationSession({ + id: sessionId, + clientId, + redirectUri, + responseType, + scope, + state, + codeChallenge, + codeChallengeMethod, + nonce, + expiresAt: sessionExpiresAt, + }); + + const consentRequestId = crypto.randomUUID(); + const consentExpiresAt = DateTime.now().plus({ minutes: 30 }).toISO(); + + await this.oidc.createConsentRequest({ + id: consentRequestId, + sessionId, + expiresAt: consentExpiresAt, + }); + + return { + consentRequestId, + clientName: client.clientName, + scope, + redirectUri, + }; + } + + public async approveConsentRequest(opts: { + consentRequestId: string; + userEntityRef: string; + }) { + const { consentRequestId, userEntityRef } = opts; + + const consentRequest = await this.oidc.getConsentRequest({ + id: consentRequestId, + }); + if (!consentRequest) { + throw new InputError('Invalid consent request'); + } + + if (DateTime.fromISO(consentRequest.expiresAt) < DateTime.now()) { + throw new InputError('Consent request expired'); + } + + const session = await this.oidc.getAuthorizationSession({ + id: consentRequest.sessionId, + }); + if (!session) { + throw new InputError('Invalid authorization session'); + } + + if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { + throw new InputError('Authorization session expired'); + } + + await this.oidc.updateAuthorizationSession({ + id: session.id, + userEntityRef, + status: 'approved', + }); + + const authorizationCode = crypto.randomBytes(32).toString('base64url'); + const codeExpiresAt = DateTime.now().plus({ minutes: 10 }).toISO(); + + await this.oidc.createAuthorizationCode({ + code: authorizationCode, + sessionId: session.id, + expiresAt: codeExpiresAt, + }); + + await this.oidc.deleteConsentRequest({ id: consentRequestId }); + + const redirectUrl = new URL(session.redirectUri); + redirectUrl.searchParams.append('code', authorizationCode); + if (session.state) { + redirectUrl.searchParams.append('state', session.state); + } + + return { + redirectUrl: redirectUrl.toString(), + }; + } + + public async getConsentRequest(opts: { consentRequestId: string }) { + const consentRequest = await this.oidc.getConsentRequest({ + id: opts.consentRequestId, + }); + if (!consentRequest) { + throw new InputError('Invalid consent request'); + } + + if (DateTime.fromISO(consentRequest.expiresAt) < DateTime.now()) { + throw new InputError('Consent request expired'); + } + + const session = await this.oidc.getAuthorizationSession({ + id: consentRequest.sessionId, + }); + + if (!session) { + throw new InputError('Invalid authorization session'); + } + + const client = await this.oidc.getClient({ clientId: session.clientId }); + if (!client) { + throw new InputError('Invalid client_id'); + } + + return { + id: consentRequest.id, + clientId: session.clientId, + clientName: client.clientName, + redirectUri: session.redirectUri, + scope: session.scope, + state: session.state, + responseType: session.responseType, + codeChallenge: session.codeChallenge, + codeChallengeMethod: session.codeChallengeMethod, + nonce: session.nonce, + expiresAt: consentRequest.expiresAt, + }; + } + + public async deleteConsentRequest(opts: { consentRequestId: string }) { + const consentRequest = await this.oidc.getConsentRequest({ + id: opts.consentRequestId, + }); + if (!consentRequest) { + return; + } + + await this.oidc.updateAuthorizationSession({ + id: consentRequest.sessionId, + status: 'rejected', + }); + + await this.oidc.deleteConsentRequest({ id: opts.consentRequestId }); + } + public async authorize(opts: { clientId: string; redirectUri: string; @@ -168,19 +359,35 @@ export class OidcService { } } - const authorizationCode = crypto.randomBytes(32).toString('base64url'); - const expiresAt = DateTime.now().plus({ minutes: 10 }).toISO(); + const sessionId = crypto.randomUUID(); + const sessionExpiresAt = DateTime.now().plus({ hours: 1 }).toISO(); - await this.oidc.createAuthorizationCode({ - code: authorizationCode, + await this.oidc.createAuthorizationSession({ + id: sessionId, clientId, userEntityRef, redirectUri, + responseType, scope, + state, codeChallenge, codeChallengeMethod, nonce, - expiresAt, + expiresAt: sessionExpiresAt, + }); + + await this.oidc.updateAuthorizationSession({ + id: sessionId, + status: 'approved', + }); + + const authorizationCode = crypto.randomBytes(32).toString('base64url'); + const codeExpiresAt = DateTime.now().plus({ minutes: 10 }).toISO(); + + await this.oidc.createAuthorizationCode({ + code: authorizationCode, + sessionId, + expiresAt: codeExpiresAt, }); const redirectUrl = new URL(redirectUri); @@ -237,24 +444,38 @@ export class OidcService { throw new AuthenticationError('Authorization code already used'); } - if (authCode.clientId !== clientId) { + const session = await this.oidc.getAuthorizationSession({ + id: authCode.sessionId, + }); + if (!session) { + throw new AuthenticationError('Invalid authorization session'); + } + if (session.clientId !== clientId) { throw new AuthenticationError('Client ID mismatch'); } - if (authCode.redirectUri !== redirectUri) { + if (session.redirectUri !== redirectUri) { throw new AuthenticationError('Redirect URI mismatch'); } - if (authCode.codeChallenge) { + if (session.status !== 'approved') { + throw new AuthenticationError('Authorization not approved'); + } + + if (!session.userEntityRef) { + throw new AuthenticationError('No user associated with authorization'); + } + + if (session.codeChallenge) { if (!codeVerifier) { throw new AuthenticationError('Code verifier required for PKCE'); } if ( !this.verifyPkce( - authCode.codeChallenge, + session.codeChallenge, codeVerifier, - authCode.codeChallengeMethod, + session.codeChallengeMethod, ) ) { throw new AuthenticationError('Invalid code verifier'); @@ -271,15 +492,13 @@ export class OidcService { await this.oidc.createAccessToken({ tokenId: accessTokenId, - clientId, - userEntityRef: authCode.userEntityRef, - scope: authCode.scope, + sessionId: session.id, expiresAt, }); const { token } = await this.tokenIssuer.issueToken({ claims: { - sub: authCode.userEntityRef, + sub: session.userEntityRef, }, }); @@ -288,7 +507,7 @@ export class OidcService { tokenType: 'Bearer', expiresIn: 3600, idToken: token, - scope: authCode.scope || 'openid', + scope: session.scope || 'openid', }; } From ff251064ae57fce729d3cf7a0164523e28e89ffb Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Thu, 3 Jul 2025 19:36:24 +0200 Subject: [PATCH 11/28] chore: implementing the routers Signed-off-by: benjdlambert --- .../src/service/OidcRouter.test.ts | 4 + .../auth-backend/src/service/OidcRouter.ts | 183 ++++++++++++++++-- plugins/auth-backend/src/service/router.ts | 1 + 3 files changed, 177 insertions(+), 11 deletions(-) diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index ccc2426035..c0f1543487 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -60,6 +60,8 @@ describe('OidcRouter', () => { auth, tokenIssuer: {} as any, baseUrl: 'http://localhost:7000', + appUrl: 'http://localhost:3000', + logger: mockServices.logger.mock(), userInfo: mockUserInfo, oidc: mockOidc, }).getRouter(), @@ -130,6 +132,8 @@ describe('OidcRouter', () => { auth, tokenIssuer: {} as any, baseUrl: 'http://localhost:7000', + appUrl: 'http://localhost:3000', + logger: mockServices.logger.mock(), userInfo: mockUserInfo, oidc: mockOidc, }).getRouter(), diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index 0e14b91d79..ba2f2ccef9 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -26,17 +26,25 @@ export class OidcRouter { private constructor( private readonly oidc: OidcService, private readonly logger: LoggerService, + private readonly auth: AuthService, + private readonly appUrl: string, ) {} static create(options: { auth: AuthService; tokenIssuer: TokenIssuer; baseUrl: string; + appUrl: string; logger: LoggerService; userInfo: UserInfoDatabase; oidc: OidcDatabase; }) { - return new OidcRouter(OidcService.create(options), options.logger); + return new OidcRouter( + OidcService.create(options), + options.logger, + options.auth, + options.appUrl, + ); } public getRouter() { @@ -44,15 +52,25 @@ export class OidcRouter { router.use(json()); + // OpenID Provider Configuration endpoint + // https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfig + // Returns the OpenID Provider Configuration document containing metadata about the provider router.get('/.well-known/openid-configuration', (_req, res) => { res.json(this.oidc.getConfiguration()); }); + // JSON Web Key Set endpoint + // https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.10.1.1 + // Returns the public keys used to verify JWTs issued by this provider router.get('/.well-known/jwks.json', async (_req, res) => { const { keys } = await this.oidc.listPublicKeys(); res.json({ keys }); }); + // Authorization endpoint + // https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest + // Handles the initial authorization request from the client, validates parameters, + // and redirects to the consent page for user approval router.get('/v1/authorize', async (req, res) => { // todo(blam): maybe add zod types for validating input const { @@ -76,11 +94,7 @@ export class OidcRouter { } try { - // use default user entity ref for now, as we need a redirect to the frontend plugin - // for the consent flow in order to issue the right token for the right user. - const userEntityRef = 'user:default/guest'; - - const { redirectUrl } = await this.oidc.authorize({ + const result = await this.oidc.createConsentRequest({ clientId: clientId as string, redirectUri: redirectUri as string, responseType: responseType as string, @@ -89,10 +103,14 @@ export class OidcRouter { nonce: nonce as string, codeChallenge: codeChallenge as string, codeChallengeMethod: codeChallengeMethod as string, - userEntityRef, }); - return res.redirect(redirectUrl); + // todo(blam): maybe this URL could be overridable by config if + // the plugin is mounted somewhere else? + const consentUrl = new URL('/oidc/consent', this.appUrl); + consentUrl.searchParams.append('consent_id', result.consentRequestId); + + return res.redirect(consentUrl.toString()); } catch (error) { const errorParams = new URLSearchParams(); errorParams.append( @@ -113,6 +131,146 @@ export class OidcRouter { } }); + // Consent request details endpoint + // Returns consent request details for the frontend consent page + router.get('/v1/consent/:consentId', async (req, res) => { + const { consentId } = req.params; + + if (!consentId) { + return res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing consent ID', + }); + } + + try { + const consentRequest = await this.oidc.getConsentRequest({ + consentRequestId: consentId, + }); + + return res.json({ + id: consentRequest.id, + clientName: consentRequest.clientName, + scope: consentRequest.scope, + redirectUri: consentRequest.redirectUri, + }); + } catch (error) { + this.logger.error( + `Failed to get consent request: ${ + isError(error) ? error.message : 'Unknown error' + }`, + error, + ); + return res.status(404).json({ + error: 'not_found', + error_description: 'Consent request not found or expired', + }); + } + }); + + // Consent approval endpoint + // Handles user approval of consent requests and generates authorization codes + router.post('/v1/consent/:consentId/approve', async (req, res) => { + const { consentId } = req.params; + + if (!consentId) { + return res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing consent ID', + }); + } + + try { + const authHeader = req.headers.authorization; + if (!authHeader?.startsWith('Bearer ')) { + return res.status(401).json({ + error: 'unauthorized', + error_description: 'Bearer token required', + }); + } + + const token = authHeader.substring(7); + const credentials = await this.auth.authenticate(token); + if (!this.auth.isPrincipal(credentials, 'user')) { + return res.status(401).json({ + error: 'unauthorized', + error_description: 'Authentication required', + }); + } + + const userEntityRef = credentials.principal.userEntityRef; + + const result = await this.oidc.approveConsentRequest({ + consentRequestId: consentId, + userEntityRef, + }); + + return res.json({ + redirectUrl: result.redirectUrl, + }); + } catch (error) { + this.logger.error( + `Failed to approve consent: ${ + isError(error) ? error.message : 'Unknown error' + }`, + error, + ); + return res.status(400).json({ + error: 'invalid_request', + error_description: isError(error) ? error.message : 'Unknown error', + }); + } + }); + + // Consent rejection endpoint + // Handles user rejection of consent requests and redirects with error + router.post('/v1/consent/:consentId/reject', async (req, res) => { + const { consentId } = req.params; + + if (!consentId) { + return res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing consent ID', + }); + } + + try { + const consentRequest = await this.oidc.getConsentRequest({ + consentRequestId: consentId, + }); + + await this.oidc.deleteConsentRequest({ consentRequestId: consentId }); + + const errorParams = new URLSearchParams(); + errorParams.append('error', 'access_denied'); + errorParams.append('error_description', 'User denied the request'); + if (consentRequest.state) { + errorParams.append('state', consentRequest.state); + } + + const redirectUrl = new URL(consentRequest.redirectUri); + redirectUrl.search = errorParams.toString(); + + return res.json({ + redirectUrl: redirectUrl.toString(), + }); + } catch (error) { + this.logger.error( + `Failed to reject consent: ${ + isError(error) ? error.message : 'Unknown error' + }`, + error, + ); + return res.status(400).json({ + error: 'invalid_request', + error_description: isError(error) ? error.message : 'Unknown error', + }); + } + }); + + // Token endpoint + // https://openid.net/specs/openid-connect-core-1_0.html#TokenRequest + // Exchanges authorization codes for access tokens and ID tokens router.post('/v1/token', async (req, res) => { // todo(blam): maybe add zod types for validating input const { @@ -180,9 +338,9 @@ export class OidcRouter { } }); - // This endpoint doesn't use the regular HttpAuth, since the contract - // is specifically for the header to be communicated in the Authorization - // header, regardless of token type + // UserInfo endpoint + // https://openid.net/specs/openid-connect-core-1_0.html#UserInfo + // Returns claims about the authenticated user using an access token router.get('/v1/userinfo', async (req, res) => { const matches = req.headers.authorization?.match(/^Bearer[ ]+(\S+)$/i); const token = matches?.[1]; @@ -200,6 +358,9 @@ export class OidcRouter { res.json(userInfo); }); + // Dynamic Client Registration endpoint + // https://openid.net/specs/openid-connect-registration-1_0.html#ClientRegistration + // Allows clients to register themselves dynamically with the provider router.post('/v1/register', async (req, res) => { // todo(blam): maybe add zod types for validating input const registrationRequest = req.body; diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 5eb3450451..28f218c749 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -154,6 +154,7 @@ export async function createRouter( auth: options.auth, tokenIssuer, baseUrl: authUrl, + appUrl, userInfo, oidc, logger, From e31a1e2c0c71dbe5998bff45bd84d30267581094 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Thu, 3 Jul 2025 19:47:41 +0200 Subject: [PATCH 12/28] chore: fixing redirect path Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- plugins/auth-backend/src/service/OidcRouter.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index ba2f2ccef9..1db444db59 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -107,8 +107,10 @@ export class OidcRouter { // todo(blam): maybe this URL could be overridable by config if // the plugin is mounted somewhere else? - const consentUrl = new URL('/oidc/consent', this.appUrl); - consentUrl.searchParams.append('consent_id', result.consentRequestId); + const consentUrl = new URL( + `/auth/consent/${result.consentRequestId}`, + this.appUrl, + ); return res.redirect(consentUrl.toString()); } catch (error) { From ebe65724e4f7c404c781320170d18d6cf7f2bfed Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Fri, 4 Jul 2025 08:39:16 +0200 Subject: [PATCH 13/28] chore: added some tests for oidcservice Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- .../auth-backend/src/service/OidcRouter.ts | 13 +- .../src/service/OidcService.test.ts | 649 ++++++++++++++++++ 2 files changed, 654 insertions(+), 8 deletions(-) create mode 100644 plugins/auth-backend/src/service/OidcService.test.ts diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index 1db444db59..c7bb42e1f5 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -257,15 +257,12 @@ export class OidcRouter { redirectUrl: redirectUrl.toString(), }); } catch (error) { - this.logger.error( - `Failed to reject consent: ${ - isError(error) ? error.message : 'Unknown error' - }`, - error, - ); + const description = isError(error) ? error.message : 'Unknown error'; + this.logger.error(`Failed to reject consent: ${description}`, error); + return res.status(400).json({ error: 'invalid_request', - error_description: isError(error) ? error.message : 'Unknown error', + error_description: description, }); } }); @@ -335,7 +332,7 @@ export class OidcRouter { return res.status(500).json({ error: 'server_error', - error_description: isError(error) ? error.message : 'Unknown error', + error_description: description, }); } }); diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts new file mode 100644 index 0000000000..fa4c0a12ac --- /dev/null +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -0,0 +1,649 @@ +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { + mockServices, + TestDatabaseId, + TestDatabases, +} from '@backstage/backend-test-utils'; +import { OidcService } from './OidcService'; +import { + BackstageCredentials, + BackstageServicePrincipal, + BackstageUserPrincipal, + resolvePackagePath, +} from '@backstage/backend-plugin-api'; +import { AuthDatabase } from '../database/AuthDatabase'; +import { OidcDatabase } from '../database/OidcDatabase'; +import { UserInfoDatabase } from '../database/UserInfoDatabase'; +import { InputError, AuthenticationError } from '@backstage/errors'; +import crypto from 'crypto'; +import { AnyJWK, TokenIssuer } from '../identity/types'; + +describe('OidcService', () => { + const databases = TestDatabases.create(); + + async function createOidcService(databaseId: TestDatabaseId) { + const knex = await databases.init(databaseId); + + await knex.migrate.latest({ + directory: resolvePackagePath( + '@backstage/plugin-auth-backend', + 'migrations', + ), + }); + + const oidcDatabase = await OidcDatabase.create({ + database: AuthDatabase.create({ + getClient: async () => knex, + }), + }); + + const mockAuth = mockServices.auth.mock(); + const mockTokenIssuer = { + issueToken: jest.fn(), + listPublicKeys: jest.fn(), + } as jest.Mocked; + + const mockUserInfo = { + addUserInfo: jest.fn(), + getUserInfo: jest.fn(), + } as unknown as jest.Mocked; + + return { + service: OidcService.create({ + auth: mockAuth, + tokenIssuer: mockTokenIssuer, + baseUrl: 'http://mock-base-url', + userInfo: mockUserInfo, + oidc: oidcDatabase, + }), + mocks: { + auth: mockAuth, + tokenIssuer: mockTokenIssuer, + userInfo: mockUserInfo, + }, + }; + } + + describe.each(databases.eachSupportedId())('%p', databaseId => { + describe('getConfiguration', () => { + it('should return OIDC configuration', async () => { + const { service } = await createOidcService(databaseId); + + const config = service.getConfiguration(); + + expect(config).toEqual({ + issuer: 'http://mock-base-url', + token_endpoint: 'http://mock-base-url/v1/token', + userinfo_endpoint: 'http://mock-base-url/v1/userinfo', + jwks_uri: 'http://mock-base-url/.well-known/jwks.json', + response_types_supported: ['code', 'id_token'], + subject_types_supported: ['public'], + id_token_signing_alg_values_supported: [ + 'RS256', + 'RS384', + 'RS512', + 'ES256', + 'ES384', + 'ES512', + 'PS256', + 'PS384', + 'PS512', + 'EdDSA', + ], + scopes_supported: ['openid'], + token_endpoint_auth_methods_supported: [ + 'client_secret_basic', + 'client_secret_post', + ], + claims_supported: ['sub', 'ent'], + grant_types_supported: ['authorization_code'], + authorization_endpoint: 'http://mock-base-url/v1/authorize', + registration_endpoint: 'http://mock-base-url/v1/register', + code_challenge_methods_supported: ['S256', 'plain'], + }); + }); + }); + + describe('listPublicKeys', () => { + it('should return public keys from token issuer', async () => { + const { service, mocks } = await createOidcService(databaseId); + const mockKeys = [{ kid: 'key-1', use: 'sig' }] as AnyJWK[]; + mocks.tokenIssuer.listPublicKeys.mockResolvedValue({ keys: mockKeys }); + + const { keys } = await service.listPublicKeys(); + + expect(keys).toEqual(mockKeys); + expect(mocks.tokenIssuer.listPublicKeys).toHaveBeenCalledTimes(1); + }); + }); + + describe('getUserInfo', () => { + it('should return user info for valid token', async () => { + const { service, mocks } = await createOidcService(databaseId); + const mockCredentials: BackstageCredentials = { + principal: { + type: 'user', + userEntityRef: 'user:default/test', + }, + $$type: '@backstage/BackstageCredentials', + }; + const mockUserInfo = { sub: 'user:default/test', name: 'Test User' }; + + mocks.auth.authenticate.mockResolvedValue(mockCredentials); + mocks.auth.isPrincipal.mockReturnValue(true); + mocks.userInfo.getUserInfo.mockResolvedValue({ + claims: mockUserInfo, + }); + + const mockToken = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvdGVzdCJ9.signature'; + + const userInfo = await service.getUserInfo({ token: mockToken }); + + expect(userInfo).toEqual({ + claims: mockUserInfo, + }); + + expect(mocks.auth.authenticate).toHaveBeenCalledWith(mockToken, { + allowLimitedAccess: true, + }); + + expect(mocks.userInfo.getUserInfo).toHaveBeenCalledWith( + 'user:default/test', + ); + }); + + it('should throw error for non-user principal', async () => { + const { service, mocks } = await createOidcService(databaseId); + const mockCredentials: BackstageCredentials = + { + principal: { + type: 'service', + subject: 'test-service', + }, + $$type: '@backstage/BackstageCredentials', + }; + + mocks.auth.authenticate.mockResolvedValue(mockCredentials); + mocks.auth.isPrincipal.mockReturnValue(false); + + const mockToken = + 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJ1c2VyOmRlZmF1bHQvdGVzdCJ9.signature'; + + await expect(service.getUserInfo({ token: mockToken })).rejects.toThrow( + 'Userinfo endpoint must be called with a token that represents a user principal', + ); + }); + }); + + describe('registerClient', () => { + it('should create a new client with generated credentials', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: 'openid', + }); + + expect(client).toEqual( + expect.objectContaining({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: 'openid', + }), + ); + expect(client.clientId).toBeDefined(); + expect(client.clientSecret).toBeDefined(); + }); + + it('should create a client with default values', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + }); + + expect(client).toEqual( + expect.objectContaining({ + clientName: 'Test Client', + redirectUris: [], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + }), + ); + }); + }); + + describe('createConsentRequest', () => { + it('should create a consent request for valid client', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const consent = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + state: 'test-state', + }); + + expect(consent).toEqual({ + consentRequestId: expect.any(String), + clientName: 'Test Client', + scope: 'openid', + redirectUri: 'https://example.com/callback', + }); + }); + + it('should throw error for invalid client', async () => { + const { service } = await createOidcService(databaseId); + + await expect( + service.createConsentRequest({ + clientId: 'invalid-client', + redirectUri: 'https://example.com/callback', + responseType: 'code', + }), + ).rejects.toThrow('Invalid client_id'); + }); + + it('should throw error for invalid redirect URI', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + await expect( + service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://invalid.com/callback', + responseType: 'code', + }), + ).rejects.toThrow('Invalid redirect_uri'); + }); + + it('should throw error for unsupported response type', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + await expect( + service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'token', + }), + ).rejects.toThrow('Only authorization code flow is supported'); + }); + + it('should handle PKCE parameters', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const consent = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + codeChallenge: 'test-challenge', + codeChallengeMethod: 'S256', + }); + + expect(consent.consentRequestId).toBeDefined(); + }); + + it('should throw error for invalid PKCE method', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + await expect( + service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + codeChallenge: 'test-challenge', + codeChallengeMethod: 'invalid', + }), + ).rejects.toThrow('Invalid code_challenge_method'); + }); + }); + + describe('approveConsentRequest', () => { + it('should approve a valid consent request', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const consent = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + state: 'test-state', + }); + + const result = await service.approveConsentRequest({ + consentRequestId: consent.consentRequestId, + userEntityRef: 'user:default/test', + }); + + expect(result.redirectUrl).toMatch( + /^https:\/\/example\.com\/callback\?code=.+&state=test-state$/, + ); + }); + + it('should throw error for invalid consent request', async () => { + const { service } = await createOidcService(databaseId); + + await expect( + service.approveConsentRequest({ + consentRequestId: 'invalid-consent', + userEntityRef: 'user:default/test', + }), + ).rejects.toThrow('Invalid consent request'); + }); + }); + + describe('getConsentRequest', () => { + it('should return consent request details', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const consent = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + state: 'test-state', + }); + + const details = await service.getConsentRequest({ + consentRequestId: consent.consentRequestId, + }); + + expect(details).toEqual( + expect.objectContaining({ + id: consent.consentRequestId, + clientId: client.clientId, + clientName: 'Test Client', + redirectUri: 'https://example.com/callback', + scope: 'openid', + state: 'test-state', + responseType: 'code', + }), + ); + }); + }); + + describe('deleteConsentRequest', () => { + it('should delete a consent request', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const consent = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.deleteConsentRequest({ + consentRequestId: consent.consentRequestId, + }); + + await expect( + service.getConsentRequest({ + consentRequestId: consent.consentRequestId, + }), + ).rejects.toThrow('Invalid consent request'); + }); + + it('should handle deleting non-existent consent request', async () => { + const { service } = await createOidcService(databaseId); + + await expect( + service.deleteConsentRequest({ + consentRequestId: 'non-existent', + }), + ).resolves.not.toThrow(); + }); + }); + + describe('authorize', () => { + it('should create direct authorization', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const result = await service.authorize({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + userEntityRef: 'user:default/test', + state: 'test-state', + }); + + expect(result.redirectUrl).toMatch( + /^https:\/\/example\.com\/callback\?code=.+&state=test-state$/, + ); + }); + + it('should throw error for invalid client', async () => { + const { service } = await createOidcService(databaseId); + + await expect( + service.authorize({ + clientId: 'invalid-client', + redirectUri: 'https://example.com/callback', + responseType: 'code', + userEntityRef: 'user:default/test', + }), + ).rejects.toThrow('Invalid client_id'); + }); + }); + + describe('exchangeCodeForToken', () => { + it('should exchange valid code for tokens', async () => { + const { service, mocks } = await createOidcService(databaseId); + const mockToken = 'mock-jwt-token'; + mocks.tokenIssuer.issueToken.mockResolvedValue({ token: mockToken }); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authResult = await service.authorize({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + userEntityRef: 'user:default/test', + scope: 'openid', + }); + + const code = new URL(authResult.redirectUrl).searchParams.get('code')!; + + const tokenResult = await service.exchangeCodeForToken({ + code, + clientId: client.clientId, + clientSecret: client.clientSecret, + redirectUri: 'https://example.com/callback', + grantType: 'authorization_code', + }); + + expect(tokenResult).toEqual({ + accessToken: mockToken, + tokenType: 'Bearer', + expiresIn: 3600, + idToken: mockToken, + scope: 'openid', + }); + }); + + it('should throw error for invalid grant type', async () => { + const { service } = await createOidcService(databaseId); + + await expect( + service.exchangeCodeForToken({ + code: 'test-code', + clientId: 'test-client', + clientSecret: 'test-secret', + redirectUri: 'https://example.com/callback', + grantType: 'client_credentials', + }), + ).rejects.toThrow('Unsupported grant type'); + }); + + it('should throw error for invalid client', async () => { + const { service } = await createOidcService(databaseId); + + await expect( + service.exchangeCodeForToken({ + code: 'test-code', + clientId: 'invalid-client', + clientSecret: 'test-secret', + redirectUri: 'https://example.com/callback', + grantType: 'authorization_code', + }), + ).rejects.toThrow('Invalid client'); + }); + + it('should throw error for invalid client secret', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + await expect( + service.exchangeCodeForToken({ + code: 'test-code', + clientId: client.clientId, + clientSecret: 'invalid-secret', + redirectUri: 'https://example.com/callback', + grantType: 'authorization_code', + }), + ).rejects.toThrow('Invalid client credentials'); + }); + + it('should handle PKCE verification', async () => { + const { service, mocks } = await createOidcService(databaseId); + const mockToken = 'mock-jwt-token'; + mocks.tokenIssuer.issueToken.mockResolvedValue({ token: mockToken }); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const codeVerifier = 'test-code-verifier'; + const codeChallenge = crypto + .createHash('sha256') + .update(codeVerifier) + .digest('base64url'); + + const authResult = await service.authorize({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + userEntityRef: 'user:default/test', + codeChallenge, + codeChallengeMethod: 'S256', + }); + + const code = new URL(authResult.redirectUrl).searchParams.get('code')!; + + const tokenResult = await service.exchangeCodeForToken({ + code, + clientId: client.clientId, + clientSecret: client.clientSecret, + redirectUri: 'https://example.com/callback', + grantType: 'authorization_code', + codeVerifier, + }); + + expect(tokenResult.accessToken).toBe(mockToken); + }); + + it('should throw error for invalid PKCE verifier', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const codeChallenge = 'test-challenge'; + const authResult = await service.authorize({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + userEntityRef: 'user:default/test', + codeChallenge, + codeChallengeMethod: 'S256', + }); + + const code = new URL(authResult.redirectUrl).searchParams.get('code')!; + + await expect( + service.exchangeCodeForToken({ + code, + clientId: client.clientId, + clientSecret: client.clientSecret, + redirectUri: 'https://example.com/callback', + grantType: 'authorization_code', + codeVerifier: 'invalid-verifier', + }), + ).rejects.toThrow('Invalid code verifier'); + }); + }); + }); +}); From 0d320ca888ffcfa20e6ab50273b130d7d199cf89 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Fri, 4 Jul 2025 09:21:22 +0200 Subject: [PATCH 14/28] chore: added some tests for oidcrouter and refactor Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- .../src/service/OidcRouter.test.ts | 1000 ++++++++++++++--- 1 file changed, 867 insertions(+), 133 deletions(-) diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index c0f1543487..150065ede5 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -17,156 +17,890 @@ import { coreServices, createBackendPlugin, + resolvePackagePath, } from '@backstage/backend-plugin-api'; -import { mockServices, startTestBackend } from '@backstage/backend-test-utils'; -import Router from 'express-promise-router'; +import { + mockServices, + startTestBackend, + TestDatabases, + TestDatabaseId, +} from '@backstage/backend-test-utils'; import request from 'supertest'; +import crypto from 'crypto'; import { OidcRouter } from './OidcRouter'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; import { OidcDatabase } from '../database/OidcDatabase'; +import { AuthDatabase } from '../database/AuthDatabase'; +import { OidcService } from '../service/OidcService'; +import { TokenIssuer } from '../identity/types'; describe('OidcRouter', () => { - describe('/v1/userinfo', () => { - it('should return user info for full tokens', async () => { - const auth = mockServices.auth.mock(); - const mockUserInfo = { - getUserInfo: jest.fn().mockResolvedValue({ - claims: { - sub: 'k/ns:n', - ent: ['k/ns:a', 'k/ns:b'], - }, - }), - } as unknown as UserInfoDatabase; + const databases = TestDatabases.create(); - const mockOidc = { - createClient: jest.fn().mockResolvedValue({ - clientId: 'test', - clientSecret: 'test', - }), - } as unknown as OidcDatabase; + async function createRouter(databaseId: TestDatabaseId) { + const knex = await databases.init(databaseId); - const { server } = await startTestBackend({ - features: [ - createBackendPlugin({ - pluginId: 'auth', - register(reg) { - reg.registerInit({ - deps: { httpRouter: coreServices.httpRouter }, - async init({ httpRouter }) { - const router = Router(); - - router.use( - OidcRouter.create({ - auth, - tokenIssuer: {} as any, - baseUrl: 'http://localhost:7000', - appUrl: 'http://localhost:3000', - logger: mockServices.logger.mock(), - userInfo: mockUserInfo, - oidc: mockOidc, - }).getRouter(), - ); - httpRouter.use(router); - httpRouter.addAuthPolicy({ - path: '/', - allow: 'unauthenticated', - }); - }, - }); - }, - }), - ], - }); - - auth.authenticate.mockResolvedValueOnce({} as any); - auth.isPrincipal.mockReturnValueOnce(true); - - await request(server) - .get('/api/auth/v1/userinfo') - .set( - 'Authorization', - `Bearer h.${btoa( - JSON.stringify({ sub: 'k/ns:n', ent: ['k/ns:a', 'k/ns:b'] }), - )}.s`, - ) - .expect(200, { - claims: { - sub: 'k/ns:n', - ent: ['k/ns:a', 'k/ns:b'], - }, - }); - - expect(mockUserInfo.getUserInfo).toHaveBeenCalledWith('k/ns:n'); + await knex.migrate.latest({ + directory: resolvePackagePath( + '@backstage/plugin-auth-backend', + 'migrations', + ), }); - it('should return user info for limited tokens', async () => { - const auth = mockServices.auth.mock(); - const mockUserInfo = { - getUserInfo: jest.fn().mockResolvedValue({ - claims: { - sub: 'k/ns:n', - ent: ['k/ns:a', 'k/ns:b'], - }, - }), - } as unknown as UserInfoDatabase; - - const mockOidc = { - createClient: jest.fn().mockResolvedValue({ - clientId: 'test', - clientSecret: 'test', - }), - } as unknown as OidcDatabase; - - const { server } = await startTestBackend({ - features: [ - createBackendPlugin({ - pluginId: 'auth', - register(reg) { - reg.registerInit({ - deps: { httpRouter: coreServices.httpRouter }, - async init({ httpRouter }) { - const router = Router(); - - router.use( - OidcRouter.create({ - auth, - tokenIssuer: {} as any, - baseUrl: 'http://localhost:7000', - appUrl: 'http://localhost:3000', - logger: mockServices.logger.mock(), - userInfo: mockUserInfo, - oidc: mockOidc, - }).getRouter(), - ); - httpRouter.use(router); - httpRouter.addAuthPolicy({ - path: '/', - allow: 'unauthenticated', - }); - }, - }); - }, - }), - ], - }); - - auth.authenticate.mockResolvedValueOnce({} as any); - auth.isPrincipal.mockReturnValueOnce(true); - - await request(server) - .get('/api/auth/v1/userinfo') - .set( - 'Authorization', - `Bearer h.${btoa(JSON.stringify({ sub: 'k/ns:n' }))}.s`, - ) - .expect(200, { + const authDatabase = AuthDatabase.create({ + getClient: async () => knex, + }); + + const oidcDatabase = await OidcDatabase.create({ + database: authDatabase, + }); + + const userInfoDatabase = await UserInfoDatabase.create({ + database: authDatabase, + }); + + const mockTokenIssuer = { + issueToken: jest.fn(), + listPublicKeys: jest.fn(), + } as unknown as jest.Mocked; + + const mockAuth = mockServices.auth.mock(); + + const oidcService = OidcService.create({ + auth: mockAuth, + tokenIssuer: mockTokenIssuer, + baseUrl: 'http://localhost:7000', + userInfo: userInfoDatabase, + oidc: oidcDatabase, + }); + + const oidcRouter = OidcRouter.create({ + auth: mockAuth, + tokenIssuer: mockTokenIssuer, + baseUrl: 'http://localhost:7000', + appUrl: 'http://localhost:3000', + logger: mockServices.logger.mock(), + userInfo: userInfoDatabase, + oidc: oidcDatabase, + }); + + return { + router: oidcRouter, + mocks: { + auth: mockAuth, + oidc: oidcDatabase, + userInfo: userInfoDatabase, + service: oidcService, + tokenIssuer: mockTokenIssuer, + }, + }; + } + + describe.each(databases.eachSupportedId())('%p', databaseId => { + describe('/v1/userinfo', () => { + it('should return user info for full tokens', async () => { + const { + mocks: { auth, userInfo }, + router, + } = await createRouter(databaseId); + + await userInfo.addUserInfo({ claims: { sub: 'k/ns:n', ent: ['k/ns:a', 'k/ns:b'], + exp: Math.floor(Date.now() / 1000) + 3600, }, }); - expect(mockUserInfo.getUserInfo).toHaveBeenCalledWith('k/ns:n'); + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + auth.authenticate.mockResolvedValueOnce({} as any); + auth.isPrincipal.mockReturnValueOnce(true); + + const response = await request(server) + .get('/api/auth/v1/userinfo') + .set( + 'Authorization', + `Bearer h.${btoa( + JSON.stringify({ sub: 'k/ns:n', ent: ['k/ns:a', 'k/ns:b'] }), + )}.s`, + ) + .expect(200); + + expect(response.body).toEqual({ + claims: { + sub: 'k/ns:n', + ent: ['k/ns:a', 'k/ns:b'], + exp: expect.any(Number), + }, + }); + }); + + it('should return user info for limited tokens', async () => { + const { + mocks: { auth, userInfo }, + router, + } = await createRouter(databaseId); + + await userInfo.addUserInfo({ + claims: { + sub: 'k/ns:n', + ent: ['k/ns:a', 'k/ns:b'], + exp: Math.floor(Date.now() / 1000) + 3600, + }, + }); + + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + auth.authenticate.mockResolvedValueOnce({} as any); + auth.isPrincipal.mockReturnValueOnce(true); + + const response = await request(server) + .get('/api/auth/v1/userinfo') + .set( + 'Authorization', + `Bearer h.${btoa(JSON.stringify({ sub: 'k/ns:n' }))}.s`, + ) + .expect(200); + + expect(response.body).toEqual({ + claims: { + sub: 'k/ns:n', + ent: ['k/ns:a', 'k/ns:b'], + exp: expect.any(Number), + }, + }); + }); + }); + + describe('consent flow', () => { + it('should register a client', async () => { + const { router } = await createRouter(databaseId); + + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + const response = await request(server) + .post('/api/auth/v1/register') + .send({ + client_name: 'Test Client', + redirect_uris: ['https://example.com/callback'], + response_types: ['code'], + grant_types: ['authorization_code'], + scope: 'openid', + }) + .expect(201); + + expect(response.body).toEqual({ + client_id: expect.any(String), + client_secret: expect.any(String), + redirect_uris: ['https://example.com/callback'], + }); + }); + + it('should create a consent request via authorization endpoint', async () => { + const { + mocks: { service }, + router, + } = await createRouter(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: 'openid', + }); + + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + const response = await request(server) + .get('/api/auth/v1/authorize') + .query({ + client_id: client.clientId, + redirect_uri: 'https://example.com/callback', + response_type: 'code', + scope: 'openid', + state: 'test-state', + }) + .expect(302); + + expect(response.header.location).toMatch( + /^http:\/\/localhost:3000\/auth\/consent\/[a-f0-9-]+$/, + ); + }); + + it('should get consent request details', async () => { + const { + mocks: { service }, + router, + } = await createRouter(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: 'openid', + }); + + const consentRequest = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + state: 'test-state', + }); + + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + const response = await request(server) + .get(`/api/auth/v1/consent/${consentRequest.consentRequestId}`) + .expect(200); + + expect(response.body).toEqual({ + id: consentRequest.consentRequestId, + clientName: 'Test Client', + scope: 'openid', + redirectUri: 'https://example.com/callback', + }); + }); + + it('should approve consent request', async () => { + const { + mocks: { auth, service }, + router, + } = await createRouter(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: 'openid', + }); + + const consentRequest = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + state: 'test-state', + }); + + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + auth.authenticate.mockResolvedValueOnce({ + principal: { + type: 'user', + userEntityRef: 'user:default/test-user', + }, + $$type: '@backstage/BackstageCredentials', + }); + + auth.isPrincipal.mockReturnValueOnce(true); + + const response = await request(server) + .post( + `/api/auth/v1/consent/${consentRequest.consentRequestId}/approve`, + ) + .set('Authorization', 'Bearer test-token') + .expect(200); + + expect(response.body).toEqual({ + redirectUrl: expect.stringMatching( + /^https:\/\/example\.com\/callback\?code=[\w-]+&state=test-state$/, + ), + }); + }); + + it('should reject consent request', async () => { + const { + mocks: { service }, + router, + } = await createRouter(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: 'openid', + }); + + const consentRequest = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + state: 'test-state', + }); + + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + const response = await request(server) + .post( + `/api/auth/v1/consent/${consentRequest.consentRequestId}/reject`, + ) + .expect(200); + + expect(response.body).toEqual({ + redirectUrl: expect.stringMatching( + /^https:\/\/example\.com\/callback\?error=access_denied&error_description=User\+denied\+the\+request&state=test-state$/, + ), + }); + }); + }); + + describe('token exchange', () => { + it('should exchange authorization code for tokens', async () => { + const { + mocks: { auth, service, tokenIssuer }, + router, + } = await createRouter(databaseId); + + auth.authenticate.mockResolvedValueOnce({ + principal: { + type: 'user', + userEntityRef: 'user:default/test-user', + }, + $$type: '@backstage/BackstageCredentials', + }); + auth.isPrincipal.mockReturnValueOnce(true); + + tokenIssuer.issueToken.mockResolvedValue({ + token: 'mock-access-token', + }); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: 'openid', + }); + + const consentRequest = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + state: 'test-state', + }); + + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + const approvalResponse = await request(server) + .post( + `/api/auth/v1/consent/${consentRequest.consentRequestId}/approve`, + ) + .set('Authorization', 'Bearer test-token') + .expect(200); + + const redirectUrl = new URL(approvalResponse.body.redirectUrl); + const authorizationCode = redirectUrl.searchParams.get('code'); + + expect(authorizationCode).toBeDefined(); + + const tokenResponse = await request(server) + .post('/api/auth/v1/token') + .send({ + grant_type: 'authorization_code', + code: authorizationCode, + client_id: client.clientId, + client_secret: client.clientSecret, + redirect_uri: 'https://example.com/callback', + }) + .expect(200); + + expect(tokenResponse.body).toEqual({ + access_token: 'mock-access-token', + token_type: 'Bearer', + expires_in: 3600, + id_token: 'mock-access-token', + scope: 'openid', + }); + + expect(tokenIssuer.issueToken).toHaveBeenCalledWith({ + claims: { + sub: 'user:default/test-user', + }, + }); + }); + + it('should exchange authorization code for tokens with PKCE', async () => { + const { + mocks: { auth, service, tokenIssuer }, + router, + } = await createRouter(databaseId); + + tokenIssuer.issueToken.mockResolvedValue({ + token: 'mock-access-token-pkce', + }); + + auth.authenticate.mockResolvedValueOnce({ + principal: { + type: 'user', + userEntityRef: 'user:default/test-user-pkce', + }, + $$type: '@backstage/BackstageCredentials', + }); + auth.isPrincipal.mockReturnValueOnce(true); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: 'openid', + }); + + const codeVerifier = + 'test-code-verifier-123456789012345678901234567890123456789012345'; + const codeChallenge = codeVerifier; + + const consentRequest = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + state: 'test-state', + codeChallenge, + codeChallengeMethod: 'plain', + }); + + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + const approvalResponse = await request(server) + .post( + `/api/auth/v1/consent/${consentRequest.consentRequestId}/approve`, + ) + .set('Authorization', 'Bearer test-token') + .expect(200); + + const redirectUrl = new URL(approvalResponse.body.redirectUrl); + const authorizationCode = redirectUrl.searchParams.get('code'); + + expect(authorizationCode).toBeDefined(); + + const tokenResponse = await request(server) + .post('/api/auth/v1/token') + .send({ + grant_type: 'authorization_code', + code: authorizationCode, + client_id: client.clientId, + client_secret: client.clientSecret, + redirect_uri: 'https://example.com/callback', + code_verifier: codeVerifier, + }) + .expect(200); + + expect(tokenResponse.body).toEqual({ + access_token: 'mock-access-token-pkce', + token_type: 'Bearer', + expires_in: 3600, + id_token: 'mock-access-token-pkce', + scope: 'openid', + }); + + expect(tokenIssuer.issueToken).toHaveBeenCalledWith({ + claims: { + sub: 'user:default/test-user-pkce', + }, + }); + }); + + it('should reject token exchange with invalid client credentials', async () => { + const { + mocks: { auth, service }, + router, + } = await createRouter(databaseId); + + auth.authenticate.mockResolvedValueOnce({ + principal: { + type: 'user', + userEntityRef: 'user:default/test-user', + }, + $$type: '@backstage/BackstageCredentials', + }); + auth.isPrincipal.mockReturnValueOnce(true); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: 'openid', + }); + + const consentRequest = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + }); + + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + const approvalResponse = await request(server) + .post( + `/api/auth/v1/consent/${consentRequest.consentRequestId}/approve`, + ) + .set('Authorization', 'Bearer test-token') + .expect(200); + + const redirectUrl = new URL(approvalResponse.body.redirectUrl); + const authorizationCode = redirectUrl.searchParams.get('code'); + + const tokenResponse = await request(server) + .post('/api/auth/v1/token') + .send({ + grant_type: 'authorization_code', + code: authorizationCode, + client_id: client.clientId, + client_secret: 'invalid-secret', + redirect_uri: 'https://example.com/callback', + }) + .expect(401); + + expect(tokenResponse.body).toEqual({ + error: 'invalid_client', + error_description: 'Invalid client credentials', + }); + }); + + it('should reject token exchange with invalid authorization code', async () => { + const { + mocks: { service }, + router, + } = await createRouter(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: 'openid', + }); + + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + const tokenResponse = await request(server) + .post('/api/auth/v1/token') + .send({ + grant_type: 'authorization_code', + code: 'invalid-code', + client_id: client.clientId, + client_secret: client.clientSecret, + redirect_uri: 'https://example.com/callback', + }) + .expect(401); + + expect(tokenResponse.body).toEqual({ + error: 'invalid_client', + error_description: 'Invalid authorization code', + }); + }); + + it('should exchange authorization code for tokens with PKCE S256', async () => { + const { + mocks: { auth, service, tokenIssuer }, + router, + } = await createRouter(databaseId); + + tokenIssuer.issueToken.mockResolvedValue({ + token: 'mock-access-token-s256', + }); + + auth.authenticate.mockResolvedValueOnce({ + principal: { + type: 'user', + userEntityRef: 'user:default/test-user-s256', + }, + $$type: '@backstage/BackstageCredentials', + }); + auth.isPrincipal.mockReturnValueOnce(true); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + responseTypes: ['code'], + grantTypes: ['authorization_code'], + scope: 'openid', + }); + + const codeVerifier = + 'test-code-verifier-s256-123456789012345678901234567890123456789'; + const codeChallenge = crypto + .createHash('sha256') + .update(codeVerifier) + .digest('base64url'); + + const consentRequest = await service.createConsentRequest({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + scope: 'openid', + state: 'test-state', + codeChallenge, + codeChallengeMethod: 'S256', + }); + + const { server } = await startTestBackend({ + features: [ + createBackendPlugin({ + pluginId: 'auth', + register(reg) { + reg.registerInit({ + deps: { httpRouter: coreServices.httpRouter }, + async init({ httpRouter }) { + httpRouter.use(router.getRouter()); + httpRouter.addAuthPolicy({ + path: '/', + allow: 'unauthenticated', + }); + }, + }); + }, + }), + ], + }); + + const approvalResponse = await request(server) + .post( + `/api/auth/v1/consent/${consentRequest.consentRequestId}/approve`, + ) + .set('Authorization', 'Bearer test-token') + .expect(200); + + const redirectUrl = new URL(approvalResponse.body.redirectUrl); + const authorizationCode = redirectUrl.searchParams.get('code'); + + expect(authorizationCode).toBeDefined(); + + const tokenResponse = await request(server) + .post('/api/auth/v1/token') + .send({ + grant_type: 'authorization_code', + code: authorizationCode, + client_id: client.clientId, + client_secret: client.clientSecret, + redirect_uri: 'https://example.com/callback', + code_verifier: codeVerifier, + }) + .expect(200); + + expect(tokenResponse.body).toEqual({ + access_token: 'mock-access-token-s256', + token_type: 'Bearer', + expires_in: 3600, + id_token: 'mock-access-token-s256', + scope: 'openid', + }); + + expect(tokenIssuer.issueToken).toHaveBeenCalledWith({ + claims: { + sub: 'user:default/test-user-s256', + }, + }); + }); }); }); }); From bf372ab53f70b156a9e5cc7c2d4c30e68df53648 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Mon, 7 Jul 2025 12:21:57 +0200 Subject: [PATCH 15/28] chore: cleanup and simplify Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- ...20250701120000_oidc_client_registration.js | 63 +------- .../src/database/OidcDatabase.test.ts | 142 ------------------ .../auth-backend/src/database/OidcDatabase.ts | 66 -------- .../src/service/OidcRouter.test.ts | 80 ++++------ .../auth-backend/src/service/OidcRouter.ts | 110 +++++++------- .../src/service/OidcService.test.ts | 80 +++++----- .../auth-backend/src/service/OidcService.ts | 88 ++++------- plugins/auth-backend/src/service/router.ts | 4 + 8 files changed, 166 insertions(+), 467 deletions(-) diff --git a/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js b/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js index bf8ee521f0..e175c922c1 100644 --- a/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js +++ b/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js @@ -43,11 +43,6 @@ exports.up = async function up(knex) { .notNullable() .comment('The name of the client, should be human readable'); - table - .timestamp('expires_at', { useTz: false, precision: 0 }) - .nullable() - .comment('Client registration expiration timestamp'); - table .text('response_types') .notNullable() @@ -110,7 +105,7 @@ exports.up = async function up(knex) { .comment('Authorization session status'); table - .timestamp('expires_at', { useTz: false, precision: 0 }) + .timestamp('expires_at', { useTz: true, precision: 0 }) .notNullable() .comment('Session expiration timestamp'); @@ -119,32 +114,6 @@ exports.up = async function up(knex) { table.index(['status', 'expires_at']); }); - await knex.schema.createTable('oidc_consent_requests', table => { - table.comment('User consent requests for OAuth authorization'); - - table - .string('id') - .primary() - .notNullable() - .comment('Unique consent request identifier'); - - table - .string('session_id') - .notNullable() - .comment('Authorization session identifier'); - - table - .timestamp('expires_at', { useTz: false, precision: 0 }) - .notNullable() - .comment('Consent request expiration timestamp'); - - table - .foreign('session_id') - .references('id') - .inTable('oauth_authorization_sessions') - .onDelete('CASCADE'); - }); - await knex.schema.createTable('oidc_authorization_codes', table => { table.comment('OAuth authorization codes for code exchange flow'); @@ -160,7 +129,7 @@ exports.up = async function up(knex) { .comment('Authorization session identifier'); table - .timestamp('expires_at', { useTz: false, precision: 0 }) + .timestamp('expires_at', { useTz: true, precision: 0 }) .notNullable() .comment('Authorization code expiration timestamp'); @@ -175,41 +144,13 @@ exports.up = async function up(knex) { .inTable('oauth_authorization_sessions') .onDelete('CASCADE'); }); - - await knex.schema.createTable('oidc_access_tokens', table => { - table.comment('OAuth access tokens for API access'); - - table - .string('token_id') - .primary() - .notNullable() - .comment('Unique access token identifier'); - - table - .string('session_id') - .notNullable() - .comment('Authorization session identifier'); - - table - .timestamp('expires_at', { useTz: false, precision: 0 }) - .notNullable() - .comment('Access token expiration timestamp'); - - table - .foreign('session_id') - .references('id') - .inTable('oauth_authorization_sessions') - .onDelete('CASCADE'); - }); }; /** * @param {import('knex').Knex} knex */ exports.down = async function down(knex) { - await knex.schema.dropTable('oidc_access_tokens'); await knex.schema.dropTable('oidc_authorization_codes'); - await knex.schema.dropTable('oidc_consent_requests'); await knex.schema.dropTable('oauth_authorization_sessions'); await knex.schema.dropTable('oidc_clients'); }; diff --git a/plugins/auth-backend/src/database/OidcDatabase.test.ts b/plugins/auth-backend/src/database/OidcDatabase.test.ts index f9c2b83511..82064361a5 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.test.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.test.ts @@ -172,111 +172,6 @@ describe('Oidc Database', () => { }); }); - describe('Consent Requests', () => { - it('should create and return a consent request', async () => { - const { oidc } = await createOidcDatabase(databaseId); - - const client = await oidc.createClient({ - clientId: 'test-client', - clientName: 'Test Client', - clientSecret: 'test-secret', - redirectUris: ['https://example.com/callback'], - responseTypes: ['code'], - grantTypes: ['authorization_code'], - }); - - const session = await oidc.createAuthorizationSession({ - id: 'test-session', - clientId: client.clientId, - redirectUri: 'https://example.com/callback', - responseType: 'code', - expiresAt: '2025-01-01T00:00:00Z', - }); - - const consentRequest = await oidc.createConsentRequest({ - id: 'test-consent', - sessionId: session.id, - expiresAt: '2025-01-01T00:00:00Z', - }); - - await expect( - oidc.getConsentRequest({ id: 'test-consent' }), - ).resolves.toEqual(consentRequest); - }); - - it('should return consent request with session data', async () => { - const { oidc } = await createOidcDatabase(databaseId); - - const client = await oidc.createClient({ - clientId: 'test-client', - clientName: 'Test Client', - clientSecret: 'test-secret', - redirectUris: ['https://example.com/callback'], - responseTypes: ['code'], - grantTypes: ['authorization_code'], - }); - - const session = await oidc.createAuthorizationSession({ - id: 'test-session', - clientId: client.clientId, - redirectUri: 'https://example.com/callback', - responseType: 'code', - scope: 'openid', - state: 'test-state', - expiresAt: '2025-01-01T00:00:00Z', - }); - - const consentRequest = await oidc.createConsentRequest({ - id: 'test-consent', - sessionId: session.id, - expiresAt: '2025-01-01T00:00:00Z', - }); - - const consentFromDb = await oidc.getConsentRequest({ - id: 'test-consent', - }); - const sessionFromDb = await oidc.getAuthorizationSession({ - id: consentFromDb!.sessionId, - }); - - expect(consentFromDb).toEqual(consentRequest); - expect(sessionFromDb).toEqual(session); - }); - - it('should delete consent request', async () => { - const { oidc } = await createOidcDatabase(databaseId); - - const client = await oidc.createClient({ - clientId: 'test-client', - clientName: 'Test Client', - clientSecret: 'test-secret', - redirectUris: ['https://example.com/callback'], - responseTypes: ['code'], - grantTypes: ['authorization_code'], - }); - - const session = await oidc.createAuthorizationSession({ - id: 'test-session', - clientId: client.clientId, - redirectUri: 'https://example.com/callback', - responseType: 'code', - expiresAt: '2025-01-01T00:00:00Z', - }); - - await oidc.createConsentRequest({ - id: 'test-consent', - sessionId: session.id, - expiresAt: '2025-01-01T00:00:00Z', - }); - - await oidc.deleteConsentRequest({ id: 'test-consent' }); - - await expect( - oidc.getConsentRequest({ id: 'test-consent' }), - ).resolves.toBeNull(); - }); - }); - describe('Authorization Codes', () => { it('should create and return an authorization code', async () => { const { oidc } = await createOidcDatabase(databaseId); @@ -392,42 +287,5 @@ describe('Oidc Database', () => { }); }); }); - - describe('Access Tokens', () => { - it('should create and return an access token', async () => { - const { oidc } = await createOidcDatabase(databaseId); - - const client = await oidc.createClient({ - clientId: 'test-client', - clientName: 'Test Client', - clientSecret: 'test-secret', - redirectUris: ['https://example.com/callback'], - responseTypes: ['code'], - grantTypes: ['authorization_code'], - }); - - const session = await oidc.createAuthorizationSession({ - id: 'test-session', - clientId: client.clientId, - redirectUri: 'https://example.com/callback', - responseType: 'code', - expiresAt: '2025-01-01T00:00:00Z', - }); - - const accessToken = await oidc.createAccessToken({ - tokenId: 'test-token', - sessionId: session.id, - expiresAt: '2025-01-01T00:00:00Z', - }); - - expect(accessToken).toEqual( - expect.objectContaining({ - tokenId: 'test-token', - sessionId: session.id, - expiresAt: '2025-01-01T00:00:00Z', - }), - ); - }); - }); }); }); diff --git a/plugins/auth-backend/src/database/OidcDatabase.ts b/plugins/auth-backend/src/database/OidcDatabase.ts index 0c75290f12..17dd4f2c06 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.ts @@ -20,7 +20,6 @@ type OidcClientRow = { client_id: string; client_secret: string; client_name: string; - expires_at: string | null; response_types: string; grant_types: string; redirect_uris: string; @@ -43,12 +42,6 @@ type OAuthAuthorizationSessionRow = { expires_at: string; }; -type OidcConsentRequestRow = { - id: string; - session_id: string; - expires_at: string; -}; - type OidcAuthorizationCodeRow = { code: string; session_id: string; @@ -56,12 +49,6 @@ type OidcAuthorizationCodeRow = { used: boolean; }; -type OidcAccessTokenRow = { - token_id: string; - session_id: string; - expires_at: string; -}; - export type Client = { clientId: string; clientName: string; @@ -70,7 +57,6 @@ export type Client = { responseTypes: string[]; grantTypes: string[]; scope?: string; - expiresAt?: string; metadata?: Record; }; @@ -125,7 +111,6 @@ export class OidcDatabase { client_id: client.clientId, client_secret: client.clientSecret, client_name: client.clientName, - expires_at: client.expiresAt, response_types: JSON.stringify(client.responseTypes), grant_types: JSON.stringify(client.grantTypes), redirect_uris: JSON.stringify(client.redirectUris), @@ -192,30 +177,6 @@ export class OidcDatabase { return this.rowToAuthorizationSession(updated) as AuthorizationSession; } - async createConsentRequest(consentRequest: ConsentRequest) { - await this.db('oidc_consent_requests').insert({ - id: consentRequest.id, - session_id: consentRequest.sessionId, - expires_at: consentRequest.expiresAt, - }); - - return consentRequest; - } - - async getConsentRequest({ id }: { id: string }) { - const consentRequest = await this.db( - 'oidc_consent_requests', - ) - .where('id', id) - .first(); - - if (!consentRequest) { - return null; - } - - return this.rowToConsentRequest(consentRequest) as ConsentRequest; - } - async getAuthorizationSession({ id }: { id: string }) { const session = await this.db( 'oauth_authorization_sessions', @@ -230,12 +191,6 @@ export class OidcDatabase { return this.rowToAuthorizationSession(session) as AuthorizationSession; } - async deleteConsentRequest({ id }: { id: string }) { - await this.db('oidc_consent_requests') - .where('id', id) - .delete(); - } - async createAuthorizationCode( authorizationCode: Omit, ) { @@ -284,16 +239,6 @@ export class OidcDatabase { return this.rowToAuthorizationCode(updated) as AuthorizationCode; } - async createAccessToken(accessToken: AccessToken) { - await this.db('oidc_access_tokens').insert({ - token_id: accessToken.tokenId, - session_id: accessToken.sessionId, - expires_at: accessToken.expiresAt, - }); - - return accessToken; - } - private rowToClient(row: Partial): Partial { return { clientId: row.client_id, @@ -307,7 +252,6 @@ export class OidcDatabase { : undefined, grantTypes: row.grant_types ? JSON.parse(row.grant_types) : undefined, scope: row.scope ?? undefined, - expiresAt: row.expires_at ?? undefined, metadata: row.metadata ? JSON.parse(row.metadata) : undefined, }; } @@ -350,16 +294,6 @@ export class OidcDatabase { }; } - private rowToConsentRequest( - row: Partial, - ): Partial { - return { - id: row.id, - sessionId: row.session_id, - expiresAt: row.expires_at, - }; - } - private authorizationCodeToRow( authorizationCode: Partial, ): Partial { diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index 150065ede5..579c96470f 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -24,6 +24,7 @@ import { startTestBackend, TestDatabases, TestDatabaseId, + mockCredentials, } from '@backstage/backend-test-utils'; import request from 'supertest'; import crypto from 'crypto'; @@ -35,6 +36,8 @@ import { OidcService } from '../service/OidcService'; import { TokenIssuer } from '../identity/types'; describe('OidcRouter', () => { + const MOCK_USER_TOKEN = 'mock-user-token'; + const MOCK_USER_ENTITY_REF = 'user:default/test-user'; const databases = TestDatabases.create(); async function createRouter(databaseId: TestDatabaseId) { @@ -82,6 +85,9 @@ describe('OidcRouter', () => { logger: mockServices.logger.mock(), userInfo: userInfoDatabase, oidc: oidcDatabase, + httpAuth: mockServices.httpAuth({ + defaultCredentials: mockCredentials.user(), + }), }); return { @@ -209,7 +215,7 @@ describe('OidcRouter', () => { }); }); - describe('consent flow', () => { + describe('auth flow', () => { it('should register a client', async () => { const { router } = await createRouter(databaseId); @@ -251,7 +257,7 @@ describe('OidcRouter', () => { }); }); - it('should create a consent request via authorization endpoint', async () => { + it('should create an authorization session via authorization endpoint', async () => { const { mocks: { service }, router, @@ -297,11 +303,11 @@ describe('OidcRouter', () => { .expect(302); expect(response.header.location).toMatch( - /^http:\/\/localhost:3000\/auth\/consent\/[a-f0-9-]+$/, + /^http:\/\/localhost:3000\/auth\/sessions\/[a-f0-9-]+$/, ); }); - it('should get consent request details', async () => { + it('should get auth session details', async () => { const { mocks: { service }, router, @@ -315,7 +321,7 @@ describe('OidcRouter', () => { scope: 'openid', }); - const consentRequest = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', @@ -344,11 +350,11 @@ describe('OidcRouter', () => { }); const response = await request(server) - .get(`/api/auth/v1/consent/${consentRequest.consentRequestId}`) + .get(`/api/auth/v1/sessions/${authSession.id}`) .expect(200); expect(response.body).toEqual({ - id: consentRequest.consentRequestId, + id: authSession.id, clientName: 'Test Client', scope: 'openid', redirectUri: 'https://example.com/callback', @@ -369,7 +375,7 @@ describe('OidcRouter', () => { scope: 'openid', }); - const consentRequest = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', @@ -397,21 +403,11 @@ describe('OidcRouter', () => { ], }); - auth.authenticate.mockResolvedValueOnce({ - principal: { - type: 'user', - userEntityRef: 'user:default/test-user', - }, - $$type: '@backstage/BackstageCredentials', - }); - auth.isPrincipal.mockReturnValueOnce(true); const response = await request(server) - .post( - `/api/auth/v1/consent/${consentRequest.consentRequestId}/approve`, - ) - .set('Authorization', 'Bearer test-token') + .post(`/api/auth/v1/sessions/${authSession.id}/approve`) + .set('Authorization', `Bearer ${MOCK_USER_TOKEN}`) .expect(200); expect(response.body).toEqual({ @@ -421,7 +417,7 @@ describe('OidcRouter', () => { }); }); - it('should reject consent request', async () => { + it('should reject auth session', async () => { const { mocks: { service }, router, @@ -435,7 +431,7 @@ describe('OidcRouter', () => { scope: 'openid', }); - const consentRequest = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', @@ -464,9 +460,7 @@ describe('OidcRouter', () => { }); const response = await request(server) - .post( - `/api/auth/v1/consent/${consentRequest.consentRequestId}/reject`, - ) + .post(`/api/auth/v1/sessions/${authSession.id}/reject`) .expect(200); expect(response.body).toEqual({ @@ -505,7 +499,7 @@ describe('OidcRouter', () => { scope: 'openid', }); - const consentRequest = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', @@ -534,10 +528,8 @@ describe('OidcRouter', () => { }); const approvalResponse = await request(server) - .post( - `/api/auth/v1/consent/${consentRequest.consentRequestId}/approve`, - ) - .set('Authorization', 'Bearer test-token') + .post(`/api/auth/v1/sessions/${authSession.id}/approve`) + .set('Authorization', `Bearer ${MOCK_USER_TOKEN}`) .expect(200); const redirectUrl = new URL(approvalResponse.body.redirectUrl); @@ -566,7 +558,7 @@ describe('OidcRouter', () => { expect(tokenIssuer.issueToken).toHaveBeenCalledWith({ claims: { - sub: 'user:default/test-user', + sub: MOCK_USER_ENTITY_REF, }, }); }); @@ -602,7 +594,7 @@ describe('OidcRouter', () => { 'test-code-verifier-123456789012345678901234567890123456789012345'; const codeChallenge = codeVerifier; - const consentRequest = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', @@ -633,10 +625,8 @@ describe('OidcRouter', () => { }); const approvalResponse = await request(server) - .post( - `/api/auth/v1/consent/${consentRequest.consentRequestId}/approve`, - ) - .set('Authorization', 'Bearer test-token') + .post(`/api/auth/v1/sessions/${authSession.id}/approve`) + .set('Authorization', `Bearer ${MOCK_USER_TOKEN}`) .expect(200); const redirectUrl = new URL(approvalResponse.body.redirectUrl); @@ -666,7 +656,7 @@ describe('OidcRouter', () => { expect(tokenIssuer.issueToken).toHaveBeenCalledWith({ claims: { - sub: 'user:default/test-user-pkce', + sub: MOCK_USER_ENTITY_REF, }, }); }); @@ -694,7 +684,7 @@ describe('OidcRouter', () => { scope: 'openid', }); - const consentRequest = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', @@ -722,10 +712,8 @@ describe('OidcRouter', () => { }); const approvalResponse = await request(server) - .post( - `/api/auth/v1/consent/${consentRequest.consentRequestId}/approve`, - ) - .set('Authorization', 'Bearer test-token') + .post(`/api/auth/v1/sessions/${authSession.id}/approve`) + .set('Authorization', `Bearer ${MOCK_USER_TOKEN}`) .expect(200); const redirectUrl = new URL(approvalResponse.body.redirectUrl); @@ -833,7 +821,7 @@ describe('OidcRouter', () => { .update(codeVerifier) .digest('base64url'); - const consentRequest = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', @@ -864,10 +852,8 @@ describe('OidcRouter', () => { }); const approvalResponse = await request(server) - .post( - `/api/auth/v1/consent/${consentRequest.consentRequestId}/approve`, - ) - .set('Authorization', 'Bearer test-token') + .post(`/api/auth/v1/sessions/${authSession.id}/approve`) + .set('Authorization', `Bearer ${MOCK_USER_TOKEN}`) .expect(200); const redirectUrl = new URL(approvalResponse.body.redirectUrl); diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index c7bb42e1f5..c07ffd5adb 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -16,7 +16,11 @@ import Router from 'express-promise-router'; import { OidcService } from './OidcService'; import { AuthenticationError, isError } from '@backstage/errors'; -import { AuthService, LoggerService } from '@backstage/backend-plugin-api'; +import { + AuthService, + HttpAuthService, + LoggerService, +} from '@backstage/backend-plugin-api'; import { TokenIssuer } from '../identity/types'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; import { OidcDatabase } from '../database/OidcDatabase'; @@ -28,6 +32,7 @@ export class OidcRouter { private readonly logger: LoggerService, private readonly auth: AuthService, private readonly appUrl: string, + private readonly httpAuth: HttpAuthService, ) {} static create(options: { @@ -38,12 +43,14 @@ export class OidcRouter { logger: LoggerService; userInfo: UserInfoDatabase; oidc: OidcDatabase; + httpAuth: HttpAuthService; }) { return new OidcRouter( OidcService.create(options), options.logger, options.auth, options.appUrl, + options.httpAuth, ); } @@ -70,7 +77,7 @@ export class OidcRouter { // Authorization endpoint // https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest // Handles the initial authorization request from the client, validates parameters, - // and redirects to the consent page for user approval + // and redirects to the Authorization Session page for user approval router.get('/v1/authorize', async (req, res) => { // todo(blam): maybe add zod types for validating input const { @@ -94,7 +101,7 @@ export class OidcRouter { } try { - const result = await this.oidc.createConsentRequest({ + const result = await this.oidc.createAuthorizationSession({ clientId: clientId as string, redirectUri: redirectUri as string, responseType: responseType as string, @@ -107,12 +114,13 @@ export class OidcRouter { // todo(blam): maybe this URL could be overridable by config if // the plugin is mounted somewhere else? - const consentUrl = new URL( - `/auth/consent/${result.consentRequestId}`, + // support slashes in baseUrl? + const authSessionRedirectUrl = new URL( + `/auth/sessions/${result.id}`, this.appUrl, ); - return res.redirect(consentUrl.toString()); + return res.redirect(authSessionRedirectUrl.toString()); } catch (error) { const errorParams = new URLSearchParams(); errorParams.append( @@ -133,77 +141,69 @@ export class OidcRouter { } }); - // Consent request details endpoint - // Returns consent request details for the frontend consent page - router.get('/v1/consent/:consentId', async (req, res) => { - const { consentId } = req.params; + // Authorization Session request details endpoint + // Returns Authorization Session request details for the frontned + router.get('/v1/sessions/:sessionId', async (req, res) => { + const { sessionId } = req.params; - if (!consentId) { + if (!sessionId) { return res.status(400).json({ error: 'invalid_request', - error_description: 'Missing consent ID', + error_description: 'Missing Authorization Session ID', }); } try { - const consentRequest = await this.oidc.getConsentRequest({ - consentRequestId: consentId, + const session = await this.oidc.getAuthorizationSession({ + sessionId, }); return res.json({ - id: consentRequest.id, - clientName: consentRequest.clientName, - scope: consentRequest.scope, - redirectUri: consentRequest.redirectUri, + id: session.id, + clientName: session.clientName, + scope: session.scope, + redirectUri: session.redirectUri, }); } catch (error) { this.logger.error( - `Failed to get consent request: ${ + `Failed to get authorization session: ${ isError(error) ? error.message : 'Unknown error' }`, error, ); return res.status(404).json({ error: 'not_found', - error_description: 'Consent request not found or expired', + error_description: 'Authorization session not found or expired', }); } }); - // Consent approval endpoint - // Handles user approval of consent requests and generates authorization codes - router.post('/v1/consent/:consentId/approve', async (req, res) => { - const { consentId } = req.params; + // Authorization Session approval endpoint + // Handles user approval of Authorization Session requests and generates authorization codes + router.post('/v1/sessions/:sessionId/approve', async (req, res) => { + const { sessionId } = req.params; - if (!consentId) { + if (!sessionId) { return res.status(400).json({ error: 'invalid_request', - error_description: 'Missing consent ID', + error_description: 'Missing authorization session ID', }); } try { - const authHeader = req.headers.authorization; - if (!authHeader?.startsWith('Bearer ')) { - return res.status(401).json({ - error: 'unauthorized', - error_description: 'Bearer token required', - }); - } + const httpCredentials = await this.httpAuth.credentials(req); - const token = authHeader.substring(7); - const credentials = await this.auth.authenticate(token); - if (!this.auth.isPrincipal(credentials, 'user')) { + if (!this.auth.isPrincipal(httpCredentials, 'user')) { return res.status(401).json({ error: 'unauthorized', error_description: 'Authentication required', }); } - const userEntityRef = credentials.principal.userEntityRef; + const userEntityRef = httpCredentials.principal.userEntityRef; - const result = await this.oidc.approveConsentRequest({ - consentRequestId: consentId, + const result = await this.oidc.approveAuthorizationSession({ + sessionId, userEntityRef, }); @@ -211,8 +211,9 @@ export class OidcRouter { redirectUrl: result.redirectUrl, }); } catch (error) { + console.log(error); this.logger.error( - `Failed to approve consent: ${ + `Failed to approve authorization session: ${ isError(error) ? error.message : 'Unknown error' }`, error, @@ -224,33 +225,33 @@ export class OidcRouter { } }); - // Consent rejection endpoint - // Handles user rejection of consent requests and redirects with error - router.post('/v1/consent/:consentId/reject', async (req, res) => { - const { consentId } = req.params; + // Authorization Session rejection endpoint + // Handles user rejection of Authorization Session requests and redirects with error + router.post('/v1/sessions/:sessionId/reject', async (req, res) => { + const { sessionId } = req.params; - if (!consentId) { + if (!sessionId) { return res.status(400).json({ error: 'invalid_request', - error_description: 'Missing consent ID', + error_description: 'Missing authorization session ID', }); } try { - const consentRequest = await this.oidc.getConsentRequest({ - consentRequestId: consentId, + const session = await this.oidc.getAuthorizationSession({ + sessionId, }); - await this.oidc.deleteConsentRequest({ consentRequestId: consentId }); + await this.oidc.rejectAuthorizationSession({ sessionId }); const errorParams = new URLSearchParams(); errorParams.append('error', 'access_denied'); errorParams.append('error_description', 'User denied the request'); - if (consentRequest.state) { - errorParams.append('state', consentRequest.state); + if (session.state) { + errorParams.append('state', session.state); } - const redirectUrl = new URL(consentRequest.redirectUri); + const redirectUrl = new URL(session.redirectUri); redirectUrl.search = errorParams.toString(); return res.json({ @@ -258,7 +259,10 @@ export class OidcRouter { }); } catch (error) { const description = isError(error) ? error.message : 'Unknown error'; - this.logger.error(`Failed to reject consent: ${description}`, error); + this.logger.error( + `Failed to reject authorization session: ${description}`, + error, + ); return res.status(400).json({ error: 'invalid_request', diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index fa4c0a12ac..5067cde458 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -233,8 +233,8 @@ describe('OidcService', () => { }); }); - describe('createConsentRequest', () => { - it('should create a consent request for valid client', async () => { + describe('createAuthorizationSession', () => { + it('should create a authorization session for valid client', async () => { const { service } = await createOidcService(databaseId); const client = await service.registerClient({ @@ -242,7 +242,7 @@ describe('OidcService', () => { redirectUris: ['https://example.com/callback'], }); - const consent = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', @@ -250,8 +250,8 @@ describe('OidcService', () => { state: 'test-state', }); - expect(consent).toEqual({ - consentRequestId: expect.any(String), + expect(authSession).toEqual({ + id: expect.any(String), clientName: 'Test Client', scope: 'openid', redirectUri: 'https://example.com/callback', @@ -262,7 +262,7 @@ describe('OidcService', () => { const { service } = await createOidcService(databaseId); await expect( - service.createConsentRequest({ + service.createAuthorizationSession({ clientId: 'invalid-client', redirectUri: 'https://example.com/callback', responseType: 'code', @@ -279,7 +279,7 @@ describe('OidcService', () => { }); await expect( - service.createConsentRequest({ + service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://invalid.com/callback', responseType: 'code', @@ -296,7 +296,7 @@ describe('OidcService', () => { }); await expect( - service.createConsentRequest({ + service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'token', @@ -312,7 +312,7 @@ describe('OidcService', () => { redirectUris: ['https://example.com/callback'], }); - const consent = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', @@ -320,7 +320,7 @@ describe('OidcService', () => { codeChallengeMethod: 'S256', }); - expect(consent.consentRequestId).toBeDefined(); + expect(authSession.id).toBeDefined(); }); it('should throw error for invalid PKCE method', async () => { @@ -332,7 +332,7 @@ describe('OidcService', () => { }); await expect( - service.createConsentRequest({ + service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', @@ -343,8 +343,8 @@ describe('OidcService', () => { }); }); - describe('approveConsentRequest', () => { - it('should approve a valid consent request', async () => { + describe('approveAuthorizationSession', () => { + it('should approve a valid authorization session', async () => { const { service } = await createOidcService(databaseId); const client = await service.registerClient({ @@ -352,15 +352,15 @@ describe('OidcService', () => { redirectUris: ['https://example.com/callback'], }); - const consent = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', state: 'test-state', }); - const result = await service.approveConsentRequest({ - consentRequestId: consent.consentRequestId, + const result = await service.approveAuthorizationSession({ + sessionId: authSession.id, userEntityRef: 'user:default/test', }); @@ -369,20 +369,20 @@ describe('OidcService', () => { ); }); - it('should throw error for invalid consent request', async () => { + it('should throw error for invalid authorization session', async () => { const { service } = await createOidcService(databaseId); await expect( - service.approveConsentRequest({ - consentRequestId: 'invalid-consent', + service.approveAuthorizationSession({ + sessionId: 'invalid-session', userEntityRef: 'user:default/test', }), - ).rejects.toThrow('Invalid consent request'); + ).rejects.toThrow('Invalid authorization session'); }); }); - describe('getConsentRequest', () => { - it('should return consent request details', async () => { + describe('getAuthorizationSession', () => { + it('should return authorization session details', async () => { const { service } = await createOidcService(databaseId); const client = await service.registerClient({ @@ -390,7 +390,7 @@ describe('OidcService', () => { redirectUris: ['https://example.com/callback'], }); - const consent = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', @@ -398,13 +398,13 @@ describe('OidcService', () => { state: 'test-state', }); - const details = await service.getConsentRequest({ - consentRequestId: consent.consentRequestId, + const details = await service.getAuthorizationSession({ + sessionId: authSession.id, }); expect(details).toEqual( expect.objectContaining({ - id: consent.consentRequestId, + id: authSession.id, clientId: client.clientId, clientName: 'Test Client', redirectUri: 'https://example.com/callback', @@ -416,8 +416,8 @@ describe('OidcService', () => { }); }); - describe('deleteConsentRequest', () => { - it('should delete a consent request', async () => { + describe('rejectAuthorizationSession', () => { + it('should delete a authorization session', async () => { const { service } = await createOidcService(databaseId); const client = await service.registerClient({ @@ -425,31 +425,35 @@ describe('OidcService', () => { redirectUris: ['https://example.com/callback'], }); - const consent = await service.createConsentRequest({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', }); - await service.deleteConsentRequest({ - consentRequestId: consent.consentRequestId, + await service.rejectAuthorizationSession({ + sessionId: authSession.id, }); await expect( - service.getConsentRequest({ - consentRequestId: consent.consentRequestId, + service.getAuthorizationSession({ + sessionId: authSession.id, }), - ).rejects.toThrow('Invalid consent request'); + ).resolves.toEqual( + expect.objectContaining({ + status: 'rejected', + }), + ); }); - it('should handle deleting non-existent consent request', async () => { + it('should throw error for invalid authorization session', async () => { const { service } = await createOidcService(databaseId); await expect( - service.deleteConsentRequest({ - consentRequestId: 'non-existent', + service.rejectAuthorizationSession({ + sessionId: 'invalid-session', }), - ).resolves.not.toThrow(); + ).rejects.toThrow('Invalid authorization session'); }); }); diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index 4214f53a8c..e89dfeac6f 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -126,7 +126,7 @@ export class OidcService { }); } - public async createConsentRequest(opts: { + public async createAuthorizationSession(opts: { clientId: string; redirectUri: string; responseType: string; @@ -185,43 +185,24 @@ export class OidcService { expiresAt: sessionExpiresAt, }); - const consentRequestId = crypto.randomUUID(); - const consentExpiresAt = DateTime.now().plus({ minutes: 30 }).toISO(); - - await this.oidc.createConsentRequest({ - id: consentRequestId, - sessionId, - expiresAt: consentExpiresAt, - }); - return { - consentRequestId, + id: sessionId, clientName: client.clientName, scope, redirectUri, }; } - public async approveConsentRequest(opts: { - consentRequestId: string; + public async approveAuthorizationSession(opts: { + sessionId: string; userEntityRef: string; }) { - const { consentRequestId, userEntityRef } = opts; - - const consentRequest = await this.oidc.getConsentRequest({ - id: consentRequestId, - }); - if (!consentRequest) { - throw new InputError('Invalid consent request'); - } - - if (DateTime.fromISO(consentRequest.expiresAt) < DateTime.now()) { - throw new InputError('Consent request expired'); - } + const { sessionId, userEntityRef } = opts; const session = await this.oidc.getAuthorizationSession({ - id: consentRequest.sessionId, + id: sessionId, }); + if (!session) { throw new InputError('Invalid authorization session'); } @@ -245,9 +226,8 @@ export class OidcService { expiresAt: codeExpiresAt, }); - await this.oidc.deleteConsentRequest({ id: consentRequestId }); - const redirectUrl = new URL(session.redirectUri); + redirectUrl.searchParams.append('code', authorizationCode); if (session.state) { redirectUrl.searchParams.append('state', session.state); @@ -258,33 +238,26 @@ export class OidcService { }; } - public async getConsentRequest(opts: { consentRequestId: string }) { - const consentRequest = await this.oidc.getConsentRequest({ - id: opts.consentRequestId, - }); - if (!consentRequest) { - throw new InputError('Invalid consent request'); - } - - if (DateTime.fromISO(consentRequest.expiresAt) < DateTime.now()) { - throw new InputError('Consent request expired'); - } - + public async getAuthorizationSession(opts: { sessionId: string }) { const session = await this.oidc.getAuthorizationSession({ - id: consentRequest.sessionId, + id: opts.sessionId, }); if (!session) { throw new InputError('Invalid authorization session'); } + if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { + throw new InputError('Authorization session expired'); + } + const client = await this.oidc.getClient({ clientId: session.clientId }); if (!client) { throw new InputError('Invalid client_id'); } return { - id: consentRequest.id, + id: session.id, clientId: session.clientId, clientName: client.clientName, redirectUri: session.redirectUri, @@ -294,24 +267,28 @@ export class OidcService { codeChallenge: session.codeChallenge, codeChallengeMethod: session.codeChallengeMethod, nonce: session.nonce, - expiresAt: consentRequest.expiresAt, + expiresAt: session.expiresAt, + status: session.status, }; } - public async deleteConsentRequest(opts: { consentRequestId: string }) { - const consentRequest = await this.oidc.getConsentRequest({ - id: opts.consentRequestId, + public async rejectAuthorizationSession(opts: { sessionId: string }) { + const session = await this.oidc.getAuthorizationSession({ + id: opts.sessionId, }); - if (!consentRequest) { - return; + + if (!session) { + throw new InputError('Invalid authorization session'); + } + + if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { + throw new InputError('Authorization session expired'); } await this.oidc.updateAuthorizationSession({ - id: consentRequest.sessionId, + id: session.id, status: 'rejected', }); - - await this.oidc.deleteConsentRequest({ id: opts.consentRequestId }); } public async authorize(opts: { @@ -487,15 +464,6 @@ export class OidcService { used: true, }); - const accessTokenId = crypto.randomUUID(); - const expiresAt = DateTime.now().plus({ hours: 1 }).toISO(); - - await this.oidc.createAccessToken({ - tokenId: accessTokenId, - sessionId: session.id, - expiresAt, - }); - const { token } = await this.tokenIssuer.issueToken({ claims: { sub: session.userEntityRef, diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 28f218c749..1f5fea7cb5 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -21,6 +21,7 @@ import { AuthService, DatabaseService, DiscoveryService, + HttpAuthService, LoggerService, RootConfigService, } from '@backstage/backend-plugin-api'; @@ -52,6 +53,7 @@ interface RouterOptions { providerFactories?: ProviderFactories; catalog: CatalogService; ownershipResolver?: AuthOwnershipResolver; + httpAuth: HttpAuthService; } export async function createRouter( @@ -64,6 +66,7 @@ export async function createRouter( database: db, tokenFactoryAlgorithm, providerFactories = {}, + httpAuth, } = options; const router = Router(); @@ -158,6 +161,7 @@ export async function createRouter( userInfo, oidc, logger, + httpAuth, }); router.use(oidcRouter.getRouter()); From 1122bb29acd56322a5c7bd51c2175185a3e8e628 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Mon, 7 Jul 2025 14:02:02 +0200 Subject: [PATCH 16/28] feat: add sqlreports and fixing up Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- plugins/auth-backend/report.sql.md | 53 +++++++++++++++++++ plugins/auth-backend/src/authPlugin.ts | 3 ++ .../src/service/OidcRouter.test.ts | 43 +++++++++------ .../auth-backend/src/service/OidcRouter.ts | 1 - .../src/service/OidcService.test.ts | 1 - 5 files changed, 82 insertions(+), 19 deletions(-) diff --git a/plugins/auth-backend/report.sql.md b/plugins/auth-backend/report.sql.md index b135414af6..7622a5750e 100644 --- a/plugins/auth-backend/report.sql.md +++ b/plugins/auth-backend/report.sql.md @@ -5,6 +5,59 @@ > [!WARNING] > Failed to migrate down from '20220321100910_timestamptz_again.js' +## Table `oauth_authorization_sessions` + +| Column | Type | Nullable | Max Length | Default | +| ----------------------- | -------------------------- | -------- | ---------- | ----------------- | +| `client_id` | `character varying` | false | 255 | - | +| `code_challenge` | `character varying` | true | 255 | - | +| `code_challenge_method` | `character varying` | true | 255 | - | +| `expires_at` | `timestamp with time zone` | false | - | - | +| `id` | `character varying` | false | 255 | - | +| `nonce` | `character varying` | true | 255 | - | +| `redirect_uri` | `text` | false | - | - | +| `response_type` | `character varying` | false | 255 | - | +| `scope` | `text` | true | - | - | +| `state` | `character varying` | true | 255 | - | +| `status` | `text` | true | - | `'pending'::text` | +| `user_entity_ref` | `character varying` | true | 255 | - | + +### Indices + +- `oauth_authorization_sessions_client_id_user_entity_ref_index` (`client_id`, `user_entity_ref`) +- `oauth_authorization_sessions_pkey` (`id`) unique primary +- `oauth_authorization_sessions_status_expires_at_index` (`status`, `expires_at`) + +## Table `oidc_authorization_codes` + +| Column | Type | Nullable | Max Length | Default | +| ------------ | -------------------------- | -------- | ---------- | ------- | +| `code` | `character varying` | false | 255 | - | +| `expires_at` | `timestamp with time zone` | false | - | - | +| `session_id` | `character varying` | false | 255 | - | +| `used` | `boolean` | true | - | `false` | + +### Indices + +- `oidc_authorization_codes_pkey` (`code`) unique primary + +## Table `oidc_clients` + +| Column | Type | Nullable | Max Length | Default | +| ---------------- | ------------------- | -------- | ---------- | ------- | +| `client_id` | `character varying` | false | 255 | - | +| `client_name` | `character varying` | false | 255 | - | +| `client_secret` | `character varying` | false | 255 | - | +| `grant_types` | `text` | false | - | - | +| `metadata` | `text` | true | - | - | +| `redirect_uris` | `text` | false | - | - | +| `response_types` | `text` | false | - | - | +| `scope` | `text` | true | - | - | + +### Indices + +- `oidc_clients_pkey` (`client_id`) unique primary + ## Table `sessions` | Column | Type | Nullable | Max Length | Default | diff --git a/plugins/auth-backend/src/authPlugin.ts b/plugins/auth-backend/src/authPlugin.ts index f3877d48cd..025d72fcb7 100644 --- a/plugins/auth-backend/src/authPlugin.ts +++ b/plugins/auth-backend/src/authPlugin.ts @@ -66,6 +66,7 @@ export const authPlugin = createBackendPlugin({ database: coreServices.database, discovery: coreServices.discovery, auth: coreServices.auth, + httpAuth: coreServices.httpAuth, catalog: catalogServiceRef, }, async init({ @@ -75,6 +76,7 @@ export const authPlugin = createBackendPlugin({ database, discovery, auth, + httpAuth, catalog, }) { const router = await createRouter({ @@ -86,6 +88,7 @@ export const authPlugin = createBackendPlugin({ catalog, providerFactories: Object.fromEntries(providers), ownershipResolver, + httpAuth, }); httpRouter.addAuthPolicy({ path: '/', diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index 579c96470f..fe5182251f 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -24,7 +24,6 @@ import { startTestBackend, TestDatabases, TestDatabaseId, - mockCredentials, } from '@backstage/backend-test-utils'; import request from 'supertest'; import crypto from 'crypto'; @@ -68,6 +67,7 @@ describe('OidcRouter', () => { } as unknown as jest.Mocked; const mockAuth = mockServices.auth.mock(); + const mockHttpAuth = mockServices.httpAuth.mock(); const oidcService = OidcService.create({ auth: mockAuth, @@ -85,14 +85,13 @@ describe('OidcRouter', () => { logger: mockServices.logger.mock(), userInfo: userInfoDatabase, oidc: oidcDatabase, - httpAuth: mockServices.httpAuth({ - defaultCredentials: mockCredentials.user(), - }), + httpAuth: mockHttpAuth, }); return { router: oidcRouter, mocks: { + httpAuth: mockHttpAuth, auth: mockAuth, oidc: oidcDatabase, userInfo: userInfoDatabase, @@ -138,7 +137,6 @@ describe('OidcRouter', () => { ], }); - auth.authenticate.mockResolvedValueOnce({} as any); auth.isPrincipal.mockReturnValueOnce(true); const response = await request(server) @@ -194,7 +192,6 @@ describe('OidcRouter', () => { ], }); - auth.authenticate.mockResolvedValueOnce({} as any); auth.isPrincipal.mockReturnValueOnce(true); const response = await request(server) @@ -361,9 +358,9 @@ describe('OidcRouter', () => { }); }); - it('should approve consent request', async () => { + it('should approve authorization session', async () => { const { - mocks: { auth, service }, + mocks: { auth, service, httpAuth }, router, } = await createRouter(databaseId); @@ -403,6 +400,14 @@ describe('OidcRouter', () => { ], }); + httpAuth.credentials.mockResolvedValueOnce({ + principal: { + type: 'user', + userEntityRef: 'user:default/test-user', + }, + $$type: '@backstage/BackstageCredentials', + }); + auth.isPrincipal.mockReturnValueOnce(true); const response = await request(server) @@ -474,17 +479,18 @@ describe('OidcRouter', () => { describe('token exchange', () => { it('should exchange authorization code for tokens', async () => { const { - mocks: { auth, service, tokenIssuer }, + mocks: { auth, service, tokenIssuer, httpAuth }, router, } = await createRouter(databaseId); - auth.authenticate.mockResolvedValueOnce({ + httpAuth.credentials.mockResolvedValueOnce({ principal: { type: 'user', userEntityRef: 'user:default/test-user', }, $$type: '@backstage/BackstageCredentials', }); + auth.isPrincipal.mockReturnValueOnce(true); tokenIssuer.issueToken.mockResolvedValue({ @@ -565,7 +571,7 @@ describe('OidcRouter', () => { it('should exchange authorization code for tokens with PKCE', async () => { const { - mocks: { auth, service, tokenIssuer }, + mocks: { auth, service, tokenIssuer, httpAuth }, router, } = await createRouter(databaseId); @@ -573,13 +579,14 @@ describe('OidcRouter', () => { token: 'mock-access-token-pkce', }); - auth.authenticate.mockResolvedValueOnce({ + httpAuth.credentials.mockResolvedValueOnce({ principal: { type: 'user', userEntityRef: 'user:default/test-user-pkce', }, $$type: '@backstage/BackstageCredentials', }); + auth.isPrincipal.mockReturnValueOnce(true); const client = await service.registerClient({ @@ -656,24 +663,25 @@ describe('OidcRouter', () => { expect(tokenIssuer.issueToken).toHaveBeenCalledWith({ claims: { - sub: MOCK_USER_ENTITY_REF, + sub: 'user:default/test-user-pkce', }, }); }); it('should reject token exchange with invalid client credentials', async () => { const { - mocks: { auth, service }, + mocks: { auth, service, httpAuth }, router, } = await createRouter(databaseId); - auth.authenticate.mockResolvedValueOnce({ + httpAuth.credentials.mockResolvedValueOnce({ principal: { type: 'user', userEntityRef: 'user:default/test-user', }, $$type: '@backstage/BackstageCredentials', }); + auth.isPrincipal.mockReturnValueOnce(true); const client = await service.registerClient({ @@ -789,7 +797,7 @@ describe('OidcRouter', () => { it('should exchange authorization code for tokens with PKCE S256', async () => { const { - mocks: { auth, service, tokenIssuer }, + mocks: { auth, service, tokenIssuer, httpAuth }, router, } = await createRouter(databaseId); @@ -797,13 +805,14 @@ describe('OidcRouter', () => { token: 'mock-access-token-s256', }); - auth.authenticate.mockResolvedValueOnce({ + httpAuth.credentials.mockResolvedValueOnce({ principal: { type: 'user', userEntityRef: 'user:default/test-user-s256', }, $$type: '@backstage/BackstageCredentials', }); + auth.isPrincipal.mockReturnValueOnce(true); const client = await service.registerClient({ diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index c07ffd5adb..a0acc57cec 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -211,7 +211,6 @@ export class OidcRouter { redirectUrl: result.redirectUrl, }); } catch (error) { - console.log(error); this.logger.error( `Failed to approve authorization session: ${ isError(error) ? error.message : 'Unknown error' diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index 5067cde458..4820559b01 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -28,7 +28,6 @@ import { import { AuthDatabase } from '../database/AuthDatabase'; import { OidcDatabase } from '../database/OidcDatabase'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; -import { InputError, AuthenticationError } from '@backstage/errors'; import crypto from 'crypto'; import { AnyJWK, TokenIssuer } from '../identity/types'; From 75e0cdbc0b9069d7fa7483312fe49338ec36f251 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Thu, 10 Jul 2025 07:52:54 +0200 Subject: [PATCH 17/28] chore: when session has been accepted or approved it should return not found from apio Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- .../auth-backend/src/service/OidcRouter.ts | 14 +- .../src/service/OidcService.test.ts | 165 +++++++++++++++++- .../auth-backend/src/service/OidcService.ts | 27 ++- 3 files changed, 186 insertions(+), 20 deletions(-) diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index a0acc57cec..60c0e42f92 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -165,15 +165,14 @@ export class OidcRouter { redirectUri: session.redirectUri, }); } catch (error) { + const description = isError(error) ? error.message : 'Unknown error'; this.logger.error( - `Failed to get authorization session: ${ - isError(error) ? error.message : 'Unknown error' - }`, + `Failed to get authorization session: ${description}`, error, ); return res.status(404).json({ error: 'not_found', - error_description: 'Authorization session not found or expired', + error_description: description, }); } }); @@ -211,15 +210,14 @@ export class OidcRouter { redirectUrl: result.redirectUrl, }); } catch (error) { + const description = isError(error) ? error.message : 'Unknown error'; this.logger.error( - `Failed to approve authorization session: ${ - isError(error) ? error.message : 'Unknown error' - }`, + `Failed to approve authorization session: ${description}`, error, ); return res.status(400).json({ error: 'invalid_request', - error_description: isError(error) ? error.message : 'Unknown error', + error_description: description, }); } }); diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index 4820559b01..017d3ab481 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -378,6 +378,59 @@ describe('OidcService', () => { }), ).rejects.toThrow('Invalid authorization session'); }); + + it('should throw error when trying to approve an already approved session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }); + + await expect( + service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }), + ).rejects.toThrow('Authorization session not found or expired'); + }); + + it('should throw error when trying to approve an already rejected session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.rejectAuthorizationSession({ + sessionId: authSession.id, + }); + + await expect( + service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }), + ).rejects.toThrow('Authorization session not found or expired'); + }); }); describe('getAuthorizationSession', () => { @@ -413,10 +466,34 @@ describe('OidcService', () => { }), ); }); - }); - describe('rejectAuthorizationSession', () => { - it('should delete a authorization session', async () => { + it('should throw error when trying to get an already approved session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }); + + await expect( + service.getAuthorizationSession({ + sessionId: authSession.id, + }), + ).rejects.toThrow('Authorization session not found or expired'); + }); + + it('should throw error when trying to get an already rejected session', async () => { const { service } = await createOidcService(databaseId); const client = await service.registerClient({ @@ -438,11 +515,34 @@ describe('OidcService', () => { service.getAuthorizationSession({ sessionId: authSession.id, }), - ).resolves.toEqual( - expect.objectContaining({ - status: 'rejected', + ).rejects.toThrow('Authorization session not found or expired'); + }); + }); + + describe('rejectAuthorizationSession', () => { + it('should reject a authorization session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.rejectAuthorizationSession({ + sessionId: authSession.id, + }); + + await expect( + service.getAuthorizationSession({ + sessionId: authSession.id, }), - ); + ).rejects.toThrow('Authorization session not found or expired'); }); it('should throw error for invalid authorization session', async () => { @@ -454,6 +554,57 @@ describe('OidcService', () => { }), ).rejects.toThrow('Invalid authorization session'); }); + + it('should throw error when trying to reject an already approved session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }); + + await expect( + service.rejectAuthorizationSession({ + sessionId: authSession.id, + }), + ).rejects.toThrow('Authorization session not found or expired'); + }); + + it('should throw error when trying to reject an already rejected session', async () => { + const { service } = await createOidcService(databaseId); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://example.com/callback'], + }); + + const authSession = await service.createAuthorizationSession({ + clientId: client.clientId, + redirectUri: 'https://example.com/callback', + responseType: 'code', + }); + + await service.rejectAuthorizationSession({ + sessionId: authSession.id, + }); + + await expect( + service.rejectAuthorizationSession({ + sessionId: authSession.id, + }), + ).rejects.toThrow('Authorization session not found or expired'); + }); }); describe('authorize', () => { diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index e89dfeac6f..7809d2fd00 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -16,7 +16,11 @@ import { AuthService } from '@backstage/backend-plugin-api'; import { TokenIssuer } from '../identity/types'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; -import { InputError, AuthenticationError } from '@backstage/errors'; +import { + InputError, + AuthenticationError, + NotFoundError, +} from '@backstage/errors'; import { decodeJwt } from 'jose'; import crypto from 'crypto'; import { OidcDatabase } from '../database/OidcDatabase'; @@ -204,13 +208,17 @@ export class OidcService { }); if (!session) { - throw new InputError('Invalid authorization session'); + throw new NotFoundError('Invalid authorization session'); } if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { throw new InputError('Authorization session expired'); } + if (session.status !== 'pending') { + throw new NotFoundError('Authorization session not found or expired'); + } + await this.oidc.updateAuthorizationSession({ id: session.id, userEntityRef, @@ -244,13 +252,17 @@ export class OidcService { }); if (!session) { - throw new InputError('Invalid authorization session'); + throw new NotFoundError('Invalid authorization session'); } if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { throw new InputError('Authorization session expired'); } + if (session.status !== 'pending') { + throw new NotFoundError('Authorization session not found or expired'); + } + const client = await this.oidc.getClient({ clientId: session.clientId }); if (!client) { throw new InputError('Invalid client_id'); @@ -278,13 +290,17 @@ export class OidcService { }); if (!session) { - throw new InputError('Invalid authorization session'); + throw new NotFoundError('Invalid authorization session'); } if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { throw new InputError('Authorization session expired'); } + if (session.status !== 'pending') { + throw new NotFoundError('Authorization session not found or expired'); + } + await this.oidc.updateAuthorizationSession({ id: session.id, status: 'rejected', @@ -424,8 +440,9 @@ export class OidcService { const session = await this.oidc.getAuthorizationSession({ id: authCode.sessionId, }); + if (!session) { - throw new AuthenticationError('Invalid authorization session'); + throw new NotFoundError('Invalid authorization session'); } if (session.clientId !== clientId) { throw new AuthenticationError('Client ID mismatch'); From 1d47bf37f59dc8927ccb3f103217e4d05f2ce035 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Mon, 21 Jul 2025 16:21:28 +0200 Subject: [PATCH 18/28] chore: add changesets Signed-off-by: benjdlambert --- .changeset/eleven-doors-down.md | 5 +++++ .changeset/eleven-doors-own.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/eleven-doors-down.md create mode 100644 .changeset/eleven-doors-own.md diff --git a/.changeset/eleven-doors-down.md b/.changeset/eleven-doors-down.md new file mode 100644 index 0000000000..47cb99dd03 --- /dev/null +++ b/.changeset/eleven-doors-down.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-mcp-actions-backend': patch +--- + +Proxy `/.well-known/oauth-authorization-server` to `/.well-known/openid-configuration` on `auth-backend` diff --git a/.changeset/eleven-doors-own.md b/.changeset/eleven-doors-own.md new file mode 100644 index 0000000000..1308aab3eb --- /dev/null +++ b/.changeset/eleven-doors-own.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Implementing Dynamic Client Registration with the OIDC server From e81f461ed88eac0c8328bb49977615b589cb8b4c Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Mon, 8 Sep 2025 10:35:41 +0200 Subject: [PATCH 19/28] chore: fix support for returning Signed-off-by: benjdlambert --- .../auth-backend/src/database/OidcDatabase.ts | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/plugins/auth-backend/src/database/OidcDatabase.ts b/plugins/auth-backend/src/database/OidcDatabase.ts index 17dd4f2c06..f4f99ef6a6 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.ts @@ -167,6 +167,32 @@ export class OidcDatabase { Object.entries(row).filter(([_, value]) => value !== undefined), ); + // MySQL and SQLite3 don't support RETURNING + if ( + this.db.client.config.client.includes('sqlite3') || + this.db.client.config.client.includes('mysql') + ) { + return await this.db.transaction(async trx => { + await trx('oauth_authorization_sessions') + .where('id', session.id) + .update(updatedFields); + + const updated = await trx( + 'oauth_authorization_sessions', + ) + .where('id', session.id) + .first(); + + if (!updated) { + throw new Error( + `Failed to retrieve updated authorization session with id ${session.id}`, + ); + } + + return this.rowToAuthorizationSession(updated) as AuthorizationSession; + }); + } + const [updated] = await this.db( 'oauth_authorization_sessions', ) @@ -229,6 +255,32 @@ export class OidcDatabase { Object.entries(row).filter(([_, value]) => value !== undefined), ); + // MySQL and SQLite3 don't support RETURNING + if ( + this.db.client.config.client.includes('sqlite3') || + this.db.client.config.client.includes('mysql') + ) { + return await this.db.transaction(async trx => { + await trx('oidc_authorization_codes') + .where('code', authorizationCode.code) + .update(updatedFields); + + const updated = await trx( + 'oidc_authorization_codes', + ) + .where('code', authorizationCode.code) + .first(); + + if (!updated) { + throw new Error( + `Failed to retrieve updated authorization code with code ${authorizationCode.code}`, + ); + } + + return this.rowToAuthorizationCode(updated) as AuthorizationCode; + }); + } + const [updated] = await this.db( 'oidc_authorization_codes', ) From 838429ac898c9bd67e28a873770b187e8be69ace Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Mon, 8 Sep 2025 10:52:05 +0200 Subject: [PATCH 20/28] chore: fix some more typescript errors Signed-off-by: benjdlambert --- .../src/database/TestDatabases.ts | 2 +- .../src/database/OidcDatabase.test.ts | 20 +++++++-------- .../auth-backend/src/database/OidcDatabase.ts | 25 +++++++++++++------ .../src/service/OidcService.test.ts | 2 ++ .../auth-backend/src/service/OidcService.ts | 16 ++++++------ 5 files changed, 38 insertions(+), 27 deletions(-) diff --git a/packages/backend-test-utils/src/database/TestDatabases.ts b/packages/backend-test-utils/src/database/TestDatabases.ts index cd20e2997b..00fae4130a 100644 --- a/packages/backend-test-utils/src/database/TestDatabases.ts +++ b/packages/backend-test-utils/src/database/TestDatabases.ts @@ -104,7 +104,7 @@ export class TestDatabases { if (supportedIds.length > 0) { afterAll(async () => { await databases.shutdown(); - }); + }, 30_000); } return databases; diff --git a/plugins/auth-backend/src/database/OidcDatabase.test.ts b/plugins/auth-backend/src/database/OidcDatabase.test.ts index 82064361a5..9965069acf 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.test.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.test.ts @@ -117,7 +117,7 @@ describe('Oidc Database', () => { codeChallenge: 'test-challenge', codeChallengeMethod: 'S256', nonce: 'test-nonce', - expiresAt: '2025-01-01T00:00:00Z', + expiresAt: new Date('2025-01-01T00:00:00Z'), }); expect(session).toEqual( @@ -132,7 +132,7 @@ describe('Oidc Database', () => { codeChallenge: 'test-challenge', codeChallengeMethod: 'S256', nonce: 'test-nonce', - expiresAt: '2025-01-01T00:00:00Z', + expiresAt: new Date('2025-01-01T00:00:00Z'), status: 'pending', }), ); @@ -155,7 +155,7 @@ describe('Oidc Database', () => { clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', - expiresAt: '2025-01-01T00:00:00Z', + expiresAt: new Date('2025-01-01T00:00:00Z'), }); await expect( @@ -190,20 +190,20 @@ describe('Oidc Database', () => { clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', - expiresAt: '2025-01-01T00:00:00Z', + expiresAt: new Date('2025-01-01T00:00:00Z'), }); const authCode = await oidc.createAuthorizationCode({ code: 'test-code', sessionId: session.id, - expiresAt: '2025-01-01T00:00:00Z', + expiresAt: new Date('2025-01-01T00:00:00Z'), }); expect(authCode).toEqual( expect.objectContaining({ code: 'test-code', sessionId: session.id, - expiresAt: '2025-01-01T00:00:00Z', + expiresAt: new Date('2025-01-01T00:00:00Z'), }), ); }); @@ -230,13 +230,13 @@ describe('Oidc Database', () => { codeChallenge: 'test-challenge', codeChallengeMethod: 'S256', nonce: 'test-nonce', - expiresAt: '2025-01-01T00:00:00Z', + expiresAt: new Date('2025-01-01T00:00:00Z'), }); const authCode = await oidc.createAuthorizationCode({ code: 'test-code', sessionId: session.id, - expiresAt: '2025-01-01T00:00:00Z', + expiresAt: new Date('2025-01-01T00:00:00Z'), }); const authCodeFromDb = await oidc.getAuthorizationCode({ @@ -267,13 +267,13 @@ describe('Oidc Database', () => { clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', - expiresAt: '2025-01-01T00:00:00Z', + expiresAt: new Date('2025-01-01T00:00:00Z'), }); const authCode = await oidc.createAuthorizationCode({ code: 'test-code', sessionId: session.id, - expiresAt: '2025-01-01T00:00:00Z', + expiresAt: new Date('2025-01-01T00:00:00Z'), }); const updatedAuthCode = await oidc.updateAuthorizationCode({ diff --git a/plugins/auth-backend/src/database/OidcDatabase.ts b/plugins/auth-backend/src/database/OidcDatabase.ts index f4f99ef6a6..ec9a879803 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.ts @@ -16,6 +16,15 @@ import { Knex } from 'knex'; import { AuthDatabase } from './AuthDatabase'; +function toDate(value?: Date | string | number): Date | undefined { + if (!value) { + return undefined; + } + + return typeof value === 'string' || typeof value === 'number' + ? new Date(value) + : value; +} type OidcClientRow = { client_id: string; client_secret: string; @@ -39,13 +48,13 @@ type OAuthAuthorizationSessionRow = { code_challenge_method: string | null; nonce: string | null; status: 'pending' | 'approved' | 'rejected' | 'expired'; - expires_at: string; + expires_at: Date | string; }; type OidcAuthorizationCodeRow = { code: string; session_id: string; - expires_at: string; + expires_at: Date | string; used: boolean; }; @@ -72,26 +81,26 @@ export type AuthorizationSession = { codeChallengeMethod?: string; nonce?: string; status: 'pending' | 'approved' | 'rejected' | 'expired'; - expiresAt: string; + expiresAt: Date; }; export type ConsentRequest = { id: string; sessionId: string; - expiresAt: string; + expiresAt: Date; }; export type AuthorizationCode = { code: string; sessionId: string; - expiresAt: string; + expiresAt: Date; used: boolean; }; export type AccessToken = { tokenId: string; sessionId: string; - expiresAt: string; + expiresAt: Date; }; /** @@ -342,7 +351,7 @@ export class OidcDatabase { codeChallengeMethod: row.code_challenge_method ?? undefined, nonce: row.nonce ?? undefined, status: row.status, - expiresAt: row.expires_at, + expiresAt: toDate(row.expires_at), }; } @@ -363,7 +372,7 @@ export class OidcDatabase { return { code: row.code, sessionId: row.session_id, - expiresAt: row.expires_at, + expiresAt: toDate(row.expires_at), used: Boolean(row.used), }; } diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index 017d3ab481..11a37e20bf 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -31,6 +31,8 @@ import { UserInfoDatabase } from '../database/UserInfoDatabase'; import crypto from 'crypto'; import { AnyJWK, TokenIssuer } from '../identity/types'; +jest.setTimeout(60_000); + describe('OidcService', () => { const databases = TestDatabases.create(); diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index 7809d2fd00..4b47d963c9 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -174,7 +174,7 @@ export class OidcService { } const sessionId = crypto.randomUUID(); - const sessionExpiresAt = DateTime.now().plus({ hours: 1 }).toISO(); + const sessionExpiresAt = DateTime.now().plus({ hours: 1 }).toJSDate(); await this.oidc.createAuthorizationSession({ id: sessionId, @@ -211,7 +211,7 @@ export class OidcService { throw new NotFoundError('Invalid authorization session'); } - if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { + if (DateTime.fromJSDate(session.expiresAt) < DateTime.now()) { throw new InputError('Authorization session expired'); } @@ -226,7 +226,7 @@ export class OidcService { }); const authorizationCode = crypto.randomBytes(32).toString('base64url'); - const codeExpiresAt = DateTime.now().plus({ minutes: 10 }).toISO(); + const codeExpiresAt = DateTime.now().plus({ minutes: 10 }).toJSDate(); await this.oidc.createAuthorizationCode({ code: authorizationCode, @@ -255,7 +255,7 @@ export class OidcService { throw new NotFoundError('Invalid authorization session'); } - if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { + if (DateTime.fromJSDate(session.expiresAt) < DateTime.now()) { throw new InputError('Authorization session expired'); } @@ -293,7 +293,7 @@ export class OidcService { throw new NotFoundError('Invalid authorization session'); } - if (DateTime.fromISO(session.expiresAt) < DateTime.now()) { + if (DateTime.fromJSDate(session.expiresAt) < DateTime.now()) { throw new InputError('Authorization session expired'); } @@ -353,7 +353,7 @@ export class OidcService { } const sessionId = crypto.randomUUID(); - const sessionExpiresAt = DateTime.now().plus({ hours: 1 }).toISO(); + const sessionExpiresAt = DateTime.now().plus({ hours: 1 }).toJSDate(); await this.oidc.createAuthorizationSession({ id: sessionId, @@ -375,7 +375,7 @@ export class OidcService { }); const authorizationCode = crypto.randomBytes(32).toString('base64url'); - const codeExpiresAt = DateTime.now().plus({ minutes: 10 }).toISO(); + const codeExpiresAt = DateTime.now().plus({ minutes: 10 }).toJSDate(); await this.oidc.createAuthorizationCode({ code: authorizationCode, @@ -429,7 +429,7 @@ export class OidcService { throw new AuthenticationError('Invalid authorization code'); } - if (DateTime.fromISO(authCode.expiresAt) < DateTime.now()) { + if (DateTime.fromJSDate(authCode.expiresAt) < DateTime.now()) { throw new AuthenticationError('Authorization code expired'); } From 025fdd20ea1de2fa14c28bf8ee6c8d921222a593 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Mon, 8 Sep 2025 13:12:36 +0200 Subject: [PATCH 21/28] chore: clientId and clientSecret are not to be passed to the token endpoint Signed-off-by: benjdlambert --- .../src/service/OidcRouter.test.ts | 97 +------------------ .../auth-backend/src/service/OidcRouter.ts | 6 +- .../src/service/OidcService.test.ts | 41 -------- .../auth-backend/src/service/OidcService.ts | 23 +---- 4 files changed, 3 insertions(+), 164 deletions(-) diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index fe5182251f..6362fd9a02 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -548,8 +548,6 @@ describe('OidcRouter', () => { .send({ grant_type: 'authorization_code', code: authorizationCode, - client_id: client.clientId, - client_secret: client.clientSecret, redirect_uri: 'https://example.com/callback', }) .expect(200); @@ -646,8 +644,6 @@ describe('OidcRouter', () => { .send({ grant_type: 'authorization_code', code: authorizationCode, - client_id: client.clientId, - client_secret: client.clientSecret, redirect_uri: 'https://example.com/callback', code_verifier: codeVerifier, }) @@ -668,95 +664,8 @@ describe('OidcRouter', () => { }); }); - it('should reject token exchange with invalid client credentials', async () => { - const { - mocks: { auth, service, httpAuth }, - router, - } = await createRouter(databaseId); - - httpAuth.credentials.mockResolvedValueOnce({ - principal: { - type: 'user', - userEntityRef: 'user:default/test-user', - }, - $$type: '@backstage/BackstageCredentials', - }); - - auth.isPrincipal.mockReturnValueOnce(true); - - const client = await service.registerClient({ - clientName: 'Test Client', - redirectUris: ['https://example.com/callback'], - responseTypes: ['code'], - grantTypes: ['authorization_code'], - scope: 'openid', - }); - - const authSession = await service.createAuthorizationSession({ - clientId: client.clientId, - redirectUri: 'https://example.com/callback', - responseType: 'code', - scope: 'openid', - }); - - const { server } = await startTestBackend({ - features: [ - createBackendPlugin({ - pluginId: 'auth', - register(reg) { - reg.registerInit({ - deps: { httpRouter: coreServices.httpRouter }, - async init({ httpRouter }) { - httpRouter.use(router.getRouter()); - httpRouter.addAuthPolicy({ - path: '/', - allow: 'unauthenticated', - }); - }, - }); - }, - }), - ], - }); - - const approvalResponse = await request(server) - .post(`/api/auth/v1/sessions/${authSession.id}/approve`) - .set('Authorization', `Bearer ${MOCK_USER_TOKEN}`) - .expect(200); - - const redirectUrl = new URL(approvalResponse.body.redirectUrl); - const authorizationCode = redirectUrl.searchParams.get('code'); - - const tokenResponse = await request(server) - .post('/api/auth/v1/token') - .send({ - grant_type: 'authorization_code', - code: authorizationCode, - client_id: client.clientId, - client_secret: 'invalid-secret', - redirect_uri: 'https://example.com/callback', - }) - .expect(401); - - expect(tokenResponse.body).toEqual({ - error: 'invalid_client', - error_description: 'Invalid client credentials', - }); - }); - it('should reject token exchange with invalid authorization code', async () => { - const { - mocks: { service }, - router, - } = await createRouter(databaseId); - - const client = await service.registerClient({ - clientName: 'Test Client', - redirectUris: ['https://example.com/callback'], - responseTypes: ['code'], - grantTypes: ['authorization_code'], - scope: 'openid', - }); + const { router } = await createRouter(databaseId); const { server } = await startTestBackend({ features: [ @@ -783,8 +692,6 @@ describe('OidcRouter', () => { .send({ grant_type: 'authorization_code', code: 'invalid-code', - client_id: client.clientId, - client_secret: client.clientSecret, redirect_uri: 'https://example.com/callback', }) .expect(401); @@ -875,8 +782,6 @@ describe('OidcRouter', () => { .send({ grant_type: 'authorization_code', code: authorizationCode, - client_id: client.clientId, - client_secret: client.clientSecret, redirect_uri: 'https://example.com/callback', code_verifier: codeVerifier, }) diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index 60c0e42f92..c64d41a73e 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -276,13 +276,11 @@ export class OidcRouter { const { grant_type: grantType, code, - client_id: clientId, - client_secret: clientSecret, redirect_uri: redirectUri, code_verifier: codeVerifier, } = req.body; - if (!grantType || !code || !clientId || !clientSecret || !redirectUri) { + if (!grantType || !code || !redirectUri) { this.logger.error( `Failed to exchange code for token: Missing required parameters`, ); @@ -295,8 +293,6 @@ export class OidcRouter { try { const result = await this.oidc.exchangeCodeForToken({ code, - clientId, - clientSecret, redirectUri, codeVerifier, grantType, diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index 11a37e20bf..b1f03c68ed 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -668,8 +668,6 @@ describe('OidcService', () => { const tokenResult = await service.exchangeCodeForToken({ code, - clientId: client.clientId, - clientSecret: client.clientSecret, redirectUri: 'https://example.com/callback', grantType: 'authorization_code', }); @@ -689,47 +687,12 @@ describe('OidcService', () => { await expect( service.exchangeCodeForToken({ code: 'test-code', - clientId: 'test-client', - clientSecret: 'test-secret', redirectUri: 'https://example.com/callback', grantType: 'client_credentials', }), ).rejects.toThrow('Unsupported grant type'); }); - it('should throw error for invalid client', async () => { - const { service } = await createOidcService(databaseId); - - await expect( - service.exchangeCodeForToken({ - code: 'test-code', - clientId: 'invalid-client', - clientSecret: 'test-secret', - redirectUri: 'https://example.com/callback', - grantType: 'authorization_code', - }), - ).rejects.toThrow('Invalid client'); - }); - - it('should throw error for invalid client secret', async () => { - const { service } = await createOidcService(databaseId); - - const client = await service.registerClient({ - clientName: 'Test Client', - redirectUris: ['https://example.com/callback'], - }); - - await expect( - service.exchangeCodeForToken({ - code: 'test-code', - clientId: client.clientId, - clientSecret: 'invalid-secret', - redirectUri: 'https://example.com/callback', - grantType: 'authorization_code', - }), - ).rejects.toThrow('Invalid client credentials'); - }); - it('should handle PKCE verification', async () => { const { service, mocks } = await createOidcService(databaseId); const mockToken = 'mock-jwt-token'; @@ -759,8 +722,6 @@ describe('OidcService', () => { const tokenResult = await service.exchangeCodeForToken({ code, - clientId: client.clientId, - clientSecret: client.clientSecret, redirectUri: 'https://example.com/callback', grantType: 'authorization_code', codeVerifier, @@ -792,8 +753,6 @@ describe('OidcService', () => { await expect( service.exchangeCodeForToken({ code, - clientId: client.clientId, - clientSecret: client.clientSecret, redirectUri: 'https://example.com/callback', grantType: 'authorization_code', codeVerifier: 'invalid-verifier', diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index 4b47d963c9..2b7eb40bc9 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -396,34 +396,16 @@ export class OidcService { public async exchangeCodeForToken(params: { code: string; - clientId: string; - clientSecret: string; redirectUri: string; codeVerifier?: string; grantType: string; }) { - const { - code, - clientId, - clientSecret, - redirectUri, - codeVerifier, - grantType, - } = params; + const { code, redirectUri, codeVerifier, grantType } = params; if (grantType !== 'authorization_code') { throw new InputError('Unsupported grant type'); } - const client = await this.oidc.getClient({ clientId }); - if (!client) { - throw new AuthenticationError('Invalid client'); - } - - if (client.clientSecret !== clientSecret) { - throw new AuthenticationError('Invalid client credentials'); - } - const authCode = await this.oidc.getAuthorizationCode({ code }); if (!authCode) { throw new AuthenticationError('Invalid authorization code'); @@ -444,9 +426,6 @@ export class OidcService { if (!session) { throw new NotFoundError('Invalid authorization session'); } - if (session.clientId !== clientId) { - throw new AuthenticationError('Client ID mismatch'); - } if (session.redirectUri !== redirectUri) { throw new AuthenticationError('Redirect URI mismatch'); From 225cdf5bdf05b767947fb59f1468ebdcdb68c0e9 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Mon, 8 Sep 2025 14:27:05 +0200 Subject: [PATCH 22/28] chore: wrap up things in a feature flag Signed-off-by: benjdlambert --- app-config.yaml | 2 + .../src/service/OidcRouter.test.ts | 1 + .../auth-backend/src/service/OidcRouter.ts | 595 +++++++++--------- plugins/auth-backend/src/service/router.ts | 4 + plugins/mcp-actions-backend/src/plugin.ts | 35 +- 5 files changed, 328 insertions(+), 309 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index 60842ee61a..e32609c1a1 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -209,6 +209,8 @@ scaffolder: defaultCommitMessage: 'Initial commit' auth: + experimental: + enableDynamicClientRegistration: true ### Add auth.keyStore.provider to more granularly control how to store JWK data when running # the auth-backend. # diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index 6362fd9a02..8714eb7f62 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -86,6 +86,7 @@ describe('OidcRouter', () => { userInfo: userInfoDatabase, oidc: oidcDatabase, httpAuth: mockHttpAuth, + enableDynamicClientRegistration: true, }); return { diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index c64d41a73e..6c39115528 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -33,6 +33,7 @@ export class OidcRouter { private readonly auth: AuthService, private readonly appUrl: string, private readonly httpAuth: HttpAuthService, + private readonly enableDynamicClientRegistration: boolean, ) {} static create(options: { @@ -44,6 +45,7 @@ export class OidcRouter { userInfo: UserInfoDatabase; oidc: OidcDatabase; httpAuth: HttpAuthService; + enableDynamicClientRegistration: boolean; }) { return new OidcRouter( OidcService.create(options), @@ -51,6 +53,7 @@ export class OidcRouter { options.auth, options.appUrl, options.httpAuth, + options.enableDynamicClientRegistration, ); } @@ -74,266 +77,6 @@ export class OidcRouter { res.json({ keys }); }); - // Authorization endpoint - // https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest - // Handles the initial authorization request from the client, validates parameters, - // and redirects to the Authorization Session page for user approval - router.get('/v1/authorize', async (req, res) => { - // todo(blam): maybe add zod types for validating input - const { - client_id: clientId, - redirect_uri: redirectUri, - response_type: responseType, - scope, - state, - nonce, - code_challenge: codeChallenge, - code_challenge_method: codeChallengeMethod, - } = req.query; - - if (!clientId || !redirectUri || !responseType) { - this.logger.error(`Failed to authorize: Missing required parameters`); - return res.status(400).json({ - error: 'invalid_request', - error_description: - 'Missing required parameters: client_id, redirect_uri, response_type', - }); - } - - try { - const result = await this.oidc.createAuthorizationSession({ - clientId: clientId as string, - redirectUri: redirectUri as string, - responseType: responseType as string, - scope: scope as string, - state: state as string, - nonce: nonce as string, - codeChallenge: codeChallenge as string, - codeChallengeMethod: codeChallengeMethod as string, - }); - - // todo(blam): maybe this URL could be overridable by config if - // the plugin is mounted somewhere else? - // support slashes in baseUrl? - const authSessionRedirectUrl = new URL( - `/auth/sessions/${result.id}`, - this.appUrl, - ); - - return res.redirect(authSessionRedirectUrl.toString()); - } catch (error) { - const errorParams = new URLSearchParams(); - errorParams.append( - 'error', - isError(error) ? error.name : 'server_error', - ); - errorParams.append( - 'error_description', - isError(error) ? error.message : 'Unknown error', - ); - if (state) { - errorParams.append('state', state as string); - } - - const redirectUrl = new URL(redirectUri as string); - redirectUrl.search = errorParams.toString(); - return res.redirect(redirectUrl.toString()); - } - }); - - // Authorization Session request details endpoint - // Returns Authorization Session request details for the frontned - router.get('/v1/sessions/:sessionId', async (req, res) => { - const { sessionId } = req.params; - - if (!sessionId) { - return res.status(400).json({ - error: 'invalid_request', - error_description: 'Missing Authorization Session ID', - }); - } - - try { - const session = await this.oidc.getAuthorizationSession({ - sessionId, - }); - - return res.json({ - id: session.id, - clientName: session.clientName, - scope: session.scope, - redirectUri: session.redirectUri, - }); - } catch (error) { - const description = isError(error) ? error.message : 'Unknown error'; - this.logger.error( - `Failed to get authorization session: ${description}`, - error, - ); - return res.status(404).json({ - error: 'not_found', - error_description: description, - }); - } - }); - - // Authorization Session approval endpoint - // Handles user approval of Authorization Session requests and generates authorization codes - router.post('/v1/sessions/:sessionId/approve', async (req, res) => { - const { sessionId } = req.params; - - if (!sessionId) { - return res.status(400).json({ - error: 'invalid_request', - error_description: 'Missing authorization session ID', - }); - } - - try { - const httpCredentials = await this.httpAuth.credentials(req); - - if (!this.auth.isPrincipal(httpCredentials, 'user')) { - return res.status(401).json({ - error: 'unauthorized', - error_description: 'Authentication required', - }); - } - - const userEntityRef = httpCredentials.principal.userEntityRef; - - const result = await this.oidc.approveAuthorizationSession({ - sessionId, - userEntityRef, - }); - - return res.json({ - redirectUrl: result.redirectUrl, - }); - } catch (error) { - const description = isError(error) ? error.message : 'Unknown error'; - this.logger.error( - `Failed to approve authorization session: ${description}`, - error, - ); - return res.status(400).json({ - error: 'invalid_request', - error_description: description, - }); - } - }); - - // Authorization Session rejection endpoint - // Handles user rejection of Authorization Session requests and redirects with error - router.post('/v1/sessions/:sessionId/reject', async (req, res) => { - const { sessionId } = req.params; - - if (!sessionId) { - return res.status(400).json({ - error: 'invalid_request', - error_description: 'Missing authorization session ID', - }); - } - - try { - const session = await this.oidc.getAuthorizationSession({ - sessionId, - }); - - await this.oidc.rejectAuthorizationSession({ sessionId }); - - const errorParams = new URLSearchParams(); - errorParams.append('error', 'access_denied'); - errorParams.append('error_description', 'User denied the request'); - if (session.state) { - errorParams.append('state', session.state); - } - - const redirectUrl = new URL(session.redirectUri); - redirectUrl.search = errorParams.toString(); - - return res.json({ - redirectUrl: redirectUrl.toString(), - }); - } catch (error) { - const description = isError(error) ? error.message : 'Unknown error'; - this.logger.error( - `Failed to reject authorization session: ${description}`, - error, - ); - - return res.status(400).json({ - error: 'invalid_request', - error_description: description, - }); - } - }); - - // Token endpoint - // https://openid.net/specs/openid-connect-core-1_0.html#TokenRequest - // Exchanges authorization codes for access tokens and ID tokens - router.post('/v1/token', async (req, res) => { - // todo(blam): maybe add zod types for validating input - const { - grant_type: grantType, - code, - redirect_uri: redirectUri, - code_verifier: codeVerifier, - } = req.body; - - if (!grantType || !code || !redirectUri) { - this.logger.error( - `Failed to exchange code for token: Missing required parameters`, - ); - return res.status(400).json({ - error: 'invalid_request', - error_description: 'Missing required parameters', - }); - } - - try { - const result = await this.oidc.exchangeCodeForToken({ - code, - redirectUri, - codeVerifier, - grantType, - }); - - return res.json({ - access_token: result.accessToken, - token_type: result.tokenType, - expires_in: result.expiresIn, - id_token: result.idToken, - scope: result.scope, - }); - } catch (error) { - const description = isError(error) ? error.message : 'Unknown error'; - this.logger.error( - `Failed to exchange code for token: ${description}`, - error, - ); - - if (isError(error)) { - if (error.name === 'AuthenticationError') { - return res.status(401).json({ - error: 'invalid_client', - error_description: error.message, - }); - } - if (error.name === 'InputError') { - return res.status(400).json({ - error: 'invalid_request', - error_description: error.message, - }); - } - } - - return res.status(500).json({ - error: 'server_error', - error_description: description, - }); - } - }); - // UserInfo endpoint // https://openid.net/specs/openid-connect-core-1_0.html#UserInfo // Returns claims about the authenticated user using an access token @@ -354,45 +97,307 @@ export class OidcRouter { res.json(userInfo); }); - // Dynamic Client Registration endpoint - // https://openid.net/specs/openid-connect-registration-1_0.html#ClientRegistration - // Allows clients to register themselves dynamically with the provider - router.post('/v1/register', async (req, res) => { - // todo(blam): maybe add zod types for validating input - const registrationRequest = req.body; + if (this.enableDynamicClientRegistration) { + // Authorization endpoint + // https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest + // Handles the initial authorization request from the client, validates parameters, + // and redirects to the Authorization Session page for user approval + router.get('/v1/authorize', async (req, res) => { + // todo(blam): maybe add zod types for validating input + const { + client_id: clientId, + redirect_uri: redirectUri, + response_type: responseType, + scope, + state, + nonce, + code_challenge: codeChallenge, + code_challenge_method: codeChallengeMethod, + } = req.query; - if (!registrationRequest.redirect_uris?.length) { - res.status(400).json({ - error: 'invalid_request', - error_description: 'redirect_uris is required', - }); - return; - } + if (!clientId || !redirectUri || !responseType) { + this.logger.error(`Failed to authorize: Missing required parameters`); + return res.status(400).json({ + error: 'invalid_request', + error_description: + 'Missing required parameters: client_id, redirect_uri, response_type', + }); + } - try { - const client = await this.oidc.registerClient({ - clientName: registrationRequest.client_name, - redirectUris: registrationRequest.redirect_uris, - responseTypes: registrationRequest.response_types, - grantTypes: registrationRequest.grant_types, - scope: registrationRequest.scope, - }); + try { + const result = await this.oidc.createAuthorizationSession({ + clientId: clientId as string, + redirectUri: redirectUri as string, + responseType: responseType as string, + scope: scope as string, + state: state as string, + nonce: nonce as string, + codeChallenge: codeChallenge as string, + codeChallengeMethod: codeChallengeMethod as string, + }); - res.status(201).json({ - client_id: client.clientId, - redirect_uris: client.redirectUris, - client_secret: client.clientSecret, - }); - } catch (e) { - const description = isError(e) ? e.message : 'Unknown error'; - this.logger.error(`Failed to register client: ${description}`, e); + // todo(blam): maybe this URL could be overridable by config if + // the plugin is mounted somewhere else? + // support slashes in baseUrl? + const authSessionRedirectUrl = new URL( + `/auth/sessions/${result.id}`, + this.appUrl, + ); - res.status(500).json({ - error: 'server_error', - error_description: `Failed to register client: ${description}`, - }); - } - }); + return res.redirect(authSessionRedirectUrl.toString()); + } catch (error) { + const errorParams = new URLSearchParams(); + errorParams.append( + 'error', + isError(error) ? error.name : 'server_error', + ); + errorParams.append( + 'error_description', + isError(error) ? error.message : 'Unknown error', + ); + if (state) { + errorParams.append('state', state as string); + } + + const redirectUrl = new URL(redirectUri as string); + redirectUrl.search = errorParams.toString(); + return res.redirect(redirectUrl.toString()); + } + }); + + // Authorization Session request details endpoint + // Returns Authorization Session request details for the frontned + router.get('/v1/sessions/:sessionId', async (req, res) => { + const { sessionId } = req.params; + + if (!sessionId) { + return res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing Authorization Session ID', + }); + } + + try { + const session = await this.oidc.getAuthorizationSession({ + sessionId, + }); + + return res.json({ + id: session.id, + clientName: session.clientName, + scope: session.scope, + redirectUri: session.redirectUri, + }); + } catch (error) { + const description = isError(error) ? error.message : 'Unknown error'; + this.logger.error( + `Failed to get authorization session: ${description}`, + error, + ); + return res.status(404).json({ + error: 'not_found', + error_description: description, + }); + } + }); + + // Authorization Session approval endpoint + // Handles user approval of Authorization Session requests and generates authorization codes + router.post('/v1/sessions/:sessionId/approve', async (req, res) => { + const { sessionId } = req.params; + + if (!sessionId) { + return res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing authorization session ID', + }); + } + + try { + const httpCredentials = await this.httpAuth.credentials(req); + + if (!this.auth.isPrincipal(httpCredentials, 'user')) { + return res.status(401).json({ + error: 'unauthorized', + error_description: 'Authentication required', + }); + } + + const userEntityRef = httpCredentials.principal.userEntityRef; + + const result = await this.oidc.approveAuthorizationSession({ + sessionId, + userEntityRef, + }); + + return res.json({ + redirectUrl: result.redirectUrl, + }); + } catch (error) { + const description = isError(error) ? error.message : 'Unknown error'; + this.logger.error( + `Failed to approve authorization session: ${description}`, + error, + ); + return res.status(400).json({ + error: 'invalid_request', + error_description: description, + }); + } + }); + + // Authorization Session rejection endpoint + // Handles user rejection of Authorization Session requests and redirects with error + router.post('/v1/sessions/:sessionId/reject', async (req, res) => { + const { sessionId } = req.params; + + if (!sessionId) { + return res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing authorization session ID', + }); + } + + try { + const session = await this.oidc.getAuthorizationSession({ + sessionId, + }); + + await this.oidc.rejectAuthorizationSession({ sessionId }); + + const errorParams = new URLSearchParams(); + errorParams.append('error', 'access_denied'); + errorParams.append('error_description', 'User denied the request'); + if (session.state) { + errorParams.append('state', session.state); + } + + const redirectUrl = new URL(session.redirectUri); + redirectUrl.search = errorParams.toString(); + + return res.json({ + redirectUrl: redirectUrl.toString(), + }); + } catch (error) { + const description = isError(error) ? error.message : 'Unknown error'; + this.logger.error( + `Failed to reject authorization session: ${description}`, + error, + ); + + return res.status(400).json({ + error: 'invalid_request', + error_description: description, + }); + } + }); + + // Token endpoint + // https://openid.net/specs/openid-connect-core-1_0.html#TokenRequest + // Exchanges authorization codes for access tokens and ID tokens + router.post('/v1/token', async (req, res) => { + // todo(blam): maybe add zod types for validating input + const { + grant_type: grantType, + code, + redirect_uri: redirectUri, + code_verifier: codeVerifier, + } = req.body; + + if (!grantType || !code || !redirectUri) { + this.logger.error( + `Failed to exchange code for token: Missing required parameters`, + ); + return res.status(400).json({ + error: 'invalid_request', + error_description: 'Missing required parameters', + }); + } + + try { + const result = await this.oidc.exchangeCodeForToken({ + code, + redirectUri, + codeVerifier, + grantType, + }); + + return res.json({ + access_token: result.accessToken, + token_type: result.tokenType, + expires_in: result.expiresIn, + id_token: result.idToken, + scope: result.scope, + }); + } catch (error) { + const description = isError(error) ? error.message : 'Unknown error'; + this.logger.error( + `Failed to exchange code for token: ${description}`, + error, + ); + + if (isError(error)) { + if (error.name === 'AuthenticationError') { + return res.status(401).json({ + error: 'invalid_client', + error_description: error.message, + }); + } + if (error.name === 'InputError') { + return res.status(400).json({ + error: 'invalid_request', + error_description: error.message, + }); + } + } + + return res.status(500).json({ + error: 'server_error', + error_description: description, + }); + } + }); + + // Dynamic Client Registration endpoint + // https://openid.net/specs/openid-connect-registration-1_0.html#ClientRegistration + // Allows clients to register themselves dynamically with the provider + router.post('/v1/register', async (req, res) => { + // todo(blam): maybe add zod types for validating input + const registrationRequest = req.body; + + if (!registrationRequest.redirect_uris?.length) { + res.status(400).json({ + error: 'invalid_request', + error_description: 'redirect_uris is required', + }); + return; + } + + try { + const client = await this.oidc.registerClient({ + clientName: registrationRequest.client_name, + redirectUris: registrationRequest.redirect_uris, + responseTypes: registrationRequest.response_types, + grantTypes: registrationRequest.grant_types, + scope: registrationRequest.scope, + }); + + res.status(201).json({ + client_id: client.clientId, + redirect_uris: client.redirectUris, + client_secret: client.clientSecret, + }); + } catch (e) { + const description = isError(e) ? e.message : 'Unknown error'; + this.logger.error(`Failed to register client: ${description}`, e); + + res.status(500).json({ + error: 'server_error', + error_description: `Failed to register client: ${description}`, + }); + } + }); + } return router; } diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 1f5fea7cb5..d2790ff35c 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -162,6 +162,10 @@ export async function createRouter( oidc, logger, httpAuth, + enableDynamicClientRegistration: + config.getOptionalBoolean( + 'auth.experimental.enableDynamicClientRegistration', + ) ?? false, }); router.use(oidcRouter.getRouter()); diff --git a/plugins/mcp-actions-backend/src/plugin.ts b/plugins/mcp-actions-backend/src/plugin.ts index 2e29847964..bcac77921c 100644 --- a/plugins/mcp-actions-backend/src/plugin.ts +++ b/plugins/mcp-actions-backend/src/plugin.ts @@ -17,7 +17,8 @@ import { coreServices, createBackendPlugin, } from '@backstage/backend-plugin-api'; -import { json, Router } from 'express'; +import { json } from 'express'; +import Router from 'express-promise-router'; import { McpService } from './services/McpService'; import { createStreamableRouter } from './routers/createStreamableRouter'; import { createSseRouter } from './routers/createSseRouter'; @@ -44,6 +45,7 @@ export const mcpPlugin = createBackendPlugin({ registry: actionsRegistryServiceRef, rootRouter: coreServices.rootHttpRouter, discovery: coreServices.discovery, + config: coreServices.rootConfig, }, async init({ actions, @@ -52,6 +54,7 @@ export const mcpPlugin = createBackendPlugin({ httpAuth, rootRouter, discovery, + config, }) { const mcpService = await McpService.create({ actions, @@ -76,21 +79,25 @@ export const mcpPlugin = createBackendPlugin({ httpRouter.use(router); - // todo(blam): there's probably a better way to proxy this, but it's required - // for mcp auth spec that it lives on the root of the mcp entrypoint server. - const authRouter = Router(); - authRouter.use('/', async (_, res) => { - const authBaseUrl = await discovery.getBaseUrl('auth'); + if ( + config.getOptionalBoolean( + 'auth.experimental.enableDynamicClientRegistration', + ) + ) { + // This should be replaced with throwing a WWW-Authenticate header, but that doesn't seem to be supported by + // many of the MCP client as of yet. So this seems to be the oldest version of the spec thats implemented. + rootRouter.use( + '/.well-known/oauth-authorization-server', + async (_, res) => { + const authBaseUrl = await discovery.getBaseUrl('auth'); + const oidcResponse = await fetch( + `${authBaseUrl}/.well-known/openid-configuration`, + ); - const oidcResponse = await fetch( - `${authBaseUrl}/.well-known/openid-configuration`, + res.json(await oidcResponse.json()); + }, ); - - const oidcResponseJson = await oidcResponse.json(); - - res.json(oidcResponseJson); - }); - rootRouter.use('/.well-known/oauth-authorization-server', authRouter); + } }, }); }, From 75b5880cb790721b5d9af691ff53d0eb593b8f24 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Mon, 8 Sep 2025 15:09:30 +0200 Subject: [PATCH 23/28] chore: Fixing changesets ] Signed-off-by: benjdlambert --- .changeset/eleven-doors-down.md | 2 +- .changeset/eleven-doors-own.md | 2 +- packages/backend-test-utils/src/database/TestDatabases.ts | 2 +- plugins/auth-backend/src/database/OidcDatabase.test.ts | 2 ++ plugins/auth-backend/src/service/OidcRouter.test.ts | 2 ++ 5 files changed, 7 insertions(+), 3 deletions(-) diff --git a/.changeset/eleven-doors-down.md b/.changeset/eleven-doors-down.md index 47cb99dd03..a253828c54 100644 --- a/.changeset/eleven-doors-down.md +++ b/.changeset/eleven-doors-down.md @@ -2,4 +2,4 @@ '@backstage/plugin-mcp-actions-backend': patch --- -Proxy `/.well-known/oauth-authorization-server` to `/.well-known/openid-configuration` on `auth-backend` +Proxy `/.well-known/oauth-authorization-server` to `/.well-known/openid-configuration` on `auth-backend` when `auth.experimental.enableDynamicClientRegistration` is enabled. diff --git a/.changeset/eleven-doors-own.md b/.changeset/eleven-doors-own.md index 1308aab3eb..8c83082b22 100644 --- a/.changeset/eleven-doors-own.md +++ b/.changeset/eleven-doors-own.md @@ -2,4 +2,4 @@ '@backstage/plugin-auth-backend': patch --- -Implementing Dynamic Client Registration with the OIDC server +Implementing Dynamic Client Registration with the OIDC server. You can enable this by setting `auth.experimental.enableDynamicClientRegistration` in `app-config.yaml`. This is highly experimental, but feedback welcome. diff --git a/packages/backend-test-utils/src/database/TestDatabases.ts b/packages/backend-test-utils/src/database/TestDatabases.ts index 00fae4130a..cd20e2997b 100644 --- a/packages/backend-test-utils/src/database/TestDatabases.ts +++ b/packages/backend-test-utils/src/database/TestDatabases.ts @@ -104,7 +104,7 @@ export class TestDatabases { if (supportedIds.length > 0) { afterAll(async () => { await databases.shutdown(); - }, 30_000); + }); } return databases; diff --git a/plugins/auth-backend/src/database/OidcDatabase.test.ts b/plugins/auth-backend/src/database/OidcDatabase.test.ts index 9965069acf..19285aec0a 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.test.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.test.ts @@ -18,6 +18,8 @@ import { AuthDatabase } from './AuthDatabase'; import { OidcDatabase } from './OidcDatabase'; import { resolvePackagePath } from '@backstage/backend-plugin-api'; +jest.setTimeout(60_000); + describe('Oidc Database', () => { const databases = TestDatabases.create(); diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index 8714eb7f62..bffe9ee509 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -34,6 +34,8 @@ import { AuthDatabase } from '../database/AuthDatabase'; import { OidcService } from '../service/OidcService'; import { TokenIssuer } from '../identity/types'; +jest.setTimeout(60_000); + describe('OidcRouter', () => { const MOCK_USER_TOKEN = 'mock-user-token'; const MOCK_USER_ENTITY_REF = 'user:default/test-user'; From a4b9f94d4f4358084f13b47c7931eaec68d13274 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Mon, 8 Sep 2025 15:14:05 +0200 Subject: [PATCH 24/28] chore: fix experimental flag Signed-off-by: benjdlambert --- .changeset/eleven-doors-down.md | 2 +- .changeset/eleven-doors-own.md | 2 +- plugins/auth-backend/src/service/router.ts | 2 +- plugins/mcp-actions-backend/src/plugin.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.changeset/eleven-doors-down.md b/.changeset/eleven-doors-down.md index a253828c54..0d380c8ddf 100644 --- a/.changeset/eleven-doors-down.md +++ b/.changeset/eleven-doors-down.md @@ -2,4 +2,4 @@ '@backstage/plugin-mcp-actions-backend': patch --- -Proxy `/.well-known/oauth-authorization-server` to `/.well-known/openid-configuration` on `auth-backend` when `auth.experimental.enableDynamicClientRegistration` is enabled. +Proxy `/.well-known/oauth-authorization-server` to `/.well-known/openid-configuration` on `auth-backend` when `auth.experimentalDynamicClientRegistration.enabled` is enabled. diff --git a/.changeset/eleven-doors-own.md b/.changeset/eleven-doors-own.md index 8c83082b22..1da0297e5c 100644 --- a/.changeset/eleven-doors-own.md +++ b/.changeset/eleven-doors-own.md @@ -2,4 +2,4 @@ '@backstage/plugin-auth-backend': patch --- -Implementing Dynamic Client Registration with the OIDC server. You can enable this by setting `auth.experimental.enableDynamicClientRegistration` in `app-config.yaml`. This is highly experimental, but feedback welcome. +Implementing Dynamic Client Registration with the OIDC server. You can enable this by setting `auth.experimentalDynamicClientRegistration.enabled` in `app-config.yaml`. This is highly experimental, but feedback welcome. diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index d2790ff35c..0d5b26078d 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -164,7 +164,7 @@ export async function createRouter( httpAuth, enableDynamicClientRegistration: config.getOptionalBoolean( - 'auth.experimental.enableDynamicClientRegistration', + 'auth.experimentalDynamicClientRegistration.enabled', ) ?? false, }); diff --git a/plugins/mcp-actions-backend/src/plugin.ts b/plugins/mcp-actions-backend/src/plugin.ts index bcac77921c..d04df6cdf5 100644 --- a/plugins/mcp-actions-backend/src/plugin.ts +++ b/plugins/mcp-actions-backend/src/plugin.ts @@ -81,7 +81,7 @@ export const mcpPlugin = createBackendPlugin({ if ( config.getOptionalBoolean( - 'auth.experimental.enableDynamicClientRegistration', + 'auth.experimentalDynamicClientRegistration.enabled', ) ) { // This should be replaced with throwing a WWW-Authenticate header, but that doesn't seem to be supported by From ff15f3032970aa35015ce245f68ba00f03fc3283 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Mon, 8 Sep 2025 17:48:33 +0200 Subject: [PATCH 25/28] feat: implementing fixes for wildcard matching for callback URLs Signed-off-by: benjdlambert --- .../src/service/OidcRouter.test.ts | 14 ++++++- .../auth-backend/src/service/OidcRouter.ts | 15 ++++--- .../src/service/OidcService.test.ts | 42 +++++++++++++++++++ .../auth-backend/src/service/OidcService.ts | 22 ++++++++-- plugins/auth-backend/src/service/router.ts | 5 +-- 5 files changed, 84 insertions(+), 14 deletions(-) diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index bffe9ee509..81430ef05e 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -70,6 +70,15 @@ describe('OidcRouter', () => { const mockAuth = mockServices.auth.mock(); const mockHttpAuth = mockServices.httpAuth.mock(); + const mockConfig = mockServices.rootConfig({ + data: { + auth: { + experimentalDynamicClientRegistration: { + enabled: true, + }, + }, + }, + }); const oidcService = OidcService.create({ auth: mockAuth, @@ -77,6 +86,7 @@ describe('OidcRouter', () => { baseUrl: 'http://localhost:7000', userInfo: userInfoDatabase, oidc: oidcDatabase, + config: mockConfig, }); const oidcRouter = OidcRouter.create({ @@ -88,7 +98,7 @@ describe('OidcRouter', () => { userInfo: userInfoDatabase, oidc: oidcDatabase, httpAuth: mockHttpAuth, - enableDynamicClientRegistration: true, + config: mockConfig, }); return { @@ -303,7 +313,7 @@ describe('OidcRouter', () => { .expect(302); expect(response.header.location).toMatch( - /^http:\/\/localhost:3000\/auth\/sessions\/[a-f0-9-]+$/, + /^http:\/\/localhost:3000\/oauth2\/authorize\/[a-f0-9-]+$/, ); }); diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index 6c39115528..7090fadf0b 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -20,6 +20,7 @@ import { AuthService, HttpAuthService, LoggerService, + RootConfigService, } from '@backstage/backend-plugin-api'; import { TokenIssuer } from '../identity/types'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; @@ -33,7 +34,7 @@ export class OidcRouter { private readonly auth: AuthService, private readonly appUrl: string, private readonly httpAuth: HttpAuthService, - private readonly enableDynamicClientRegistration: boolean, + private readonly config: RootConfigService, ) {} static create(options: { @@ -45,7 +46,7 @@ export class OidcRouter { userInfo: UserInfoDatabase; oidc: OidcDatabase; httpAuth: HttpAuthService; - enableDynamicClientRegistration: boolean; + config: RootConfigService; }) { return new OidcRouter( OidcService.create(options), @@ -53,7 +54,7 @@ export class OidcRouter { options.auth, options.appUrl, options.httpAuth, - options.enableDynamicClientRegistration, + options.config, ); } @@ -97,7 +98,11 @@ export class OidcRouter { res.json(userInfo); }); - if (this.enableDynamicClientRegistration) { + if ( + this.config.getOptionalBoolean( + 'auth.experimentalDynamicClientRegistration.enabled', + ) + ) { // Authorization endpoint // https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest // Handles the initial authorization request from the client, validates parameters, @@ -140,7 +145,7 @@ export class OidcRouter { // the plugin is mounted somewhere else? // support slashes in baseUrl? const authSessionRedirectUrl = new URL( - `/auth/sessions/${result.id}`, + `/oauth2/authorize/${result.id}`, this.appUrl, ); diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index b1f03c68ed..7829be3841 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -63,6 +63,8 @@ describe('OidcService', () => { getUserInfo: jest.fn(), } as unknown as jest.Mocked; + const mockConfig = mockServices.rootConfig.mock(); + return { service: OidcService.create({ auth: mockAuth, @@ -70,11 +72,13 @@ describe('OidcService', () => { baseUrl: 'http://mock-base-url', userInfo: mockUserInfo, oidc: oidcDatabase, + config: mockConfig, }), mocks: { auth: mockAuth, tokenIssuer: mockTokenIssuer, userInfo: mockUserInfo, + config: mockConfig, }, }; } @@ -216,6 +220,44 @@ describe('OidcService', () => { expect(client.clientSecret).toBeDefined(); }); + it('should throw an error for invalid redirect URI', async () => { + const { + service, + mocks: { config }, + } = await createOidcService(databaseId); + + config.getOptionalStringArray.mockReturnValue([ + 'https://example.com/*', + ]); + + await expect( + service.registerClient({ + clientName: 'Test Client', + redirectUris: ['https://invalid.com/callback'], + }), + ).rejects.toThrow('Invalid redirect_uri'); + }); + + it('should create a new client with valid redirect URI', async () => { + const { + service, + mocks: { config }, + } = await createOidcService(databaseId); + + config.getOptionalStringArray.mockReturnValue(['cursor://*']); + + const client = await service.registerClient({ + clientName: 'Test Client', + redirectUris: ['cursor://callback'], + }); + + expect(client).toEqual( + expect.objectContaining({ + redirectUris: ['cursor://callback'], + }), + ); + }); + it('should create a client with default values', async () => { const { service } = await createOidcService(databaseId); diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index 2b7eb40bc9..3139ccc73d 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { AuthService } from '@backstage/backend-plugin-api'; +import { AuthService, RootConfigService } from '@backstage/backend-plugin-api'; import { TokenIssuer } from '../identity/types'; import { UserInfoDatabase } from '../database/UserInfoDatabase'; import { @@ -33,6 +33,7 @@ export class OidcService { private readonly baseUrl: string, private readonly userInfo: UserInfoDatabase, private readonly oidc: OidcDatabase, + private readonly config: RootConfigService, ) {} static create(options: { @@ -41,6 +42,7 @@ export class OidcService { baseUrl: string; userInfo: UserInfoDatabase; oidc: OidcDatabase; + config: RootConfigService; }) { return new OidcService( options.auth, @@ -48,6 +50,7 @@ export class OidcService { options.baseUrl, options.userInfo, options.oidc, + options.config, ); } @@ -116,8 +119,21 @@ export class OidcService { const generatedClientId = crypto.randomUUID(); const generatedClientSecret = crypto.randomUUID(); - // todo(blam): add validation for redirectUris here. - // should be a list of urls and / or allowed schemes or something. + const allowedRedirectUriPatterns = this.config.getOptionalStringArray( + 'auth.experimentalDynamicClientRegistration.allowedRedirectUriPatterns', + ); + + if (allowedRedirectUriPatterns) { + for (const redirectUri of opts.redirectUris ?? []) { + if ( + !allowedRedirectUriPatterns.some(pattern => + new RegExp(pattern).test(redirectUri), + ) + ) { + throw new InputError('Invalid redirect_uri'); + } + } + } return await this.oidc.createClient({ clientId: generatedClientId, diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 0d5b26078d..0f0d5b2830 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -162,10 +162,7 @@ export async function createRouter( oidc, logger, httpAuth, - enableDynamicClientRegistration: - config.getOptionalBoolean( - 'auth.experimentalDynamicClientRegistration.enabled', - ) ?? false, + config, }); router.use(oidcRouter.getRouter()); From c9f1fb203a0105b0747fd6d047c45aa59f607f88 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Tue, 9 Sep 2025 10:46:45 +0200 Subject: [PATCH 26/28] chore: cleanup Signed-off-by: benjdlambert --- app-config.yaml | 7 ++++-- .../auth-backend/src/service/OidcRouter.ts | 22 ++++++++++++------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index e32609c1a1..eacf9a96aa 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -209,8 +209,11 @@ scaffolder: defaultCommitMessage: 'Initial commit' auth: - experimental: - enableDynamicClientRegistration: true + experimentalDynamicClientRegistration: + enabled: true + allowedRedirectUriPatterns: + - cursor://* + ### Add auth.keyStore.provider to more granularly control how to store JWK data when running # the auth-backend. # diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index 7090fadf0b..f20b5f4ede 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -228,7 +228,7 @@ export class OidcRouter { }); } - const userEntityRef = httpCredentials.principal.userEntityRef; + const { userEntityRef } = httpCredentials.principal; const result = await this.oidc.approveAuthorizationSession({ sessionId, @@ -368,9 +368,15 @@ export class OidcRouter { // Allows clients to register themselves dynamically with the provider router.post('/v1/register', async (req, res) => { // todo(blam): maybe add zod types for validating input - const registrationRequest = req.body; + const { + client_name: clientName, + redirect_uris: redirectUris, + response_types: responseTypes, + grant_types: grantTypes, + scope, + } = req.body; - if (!registrationRequest.redirect_uris?.length) { + if (!redirectUris?.length) { res.status(400).json({ error: 'invalid_request', error_description: 'redirect_uris is required', @@ -380,11 +386,11 @@ export class OidcRouter { try { const client = await this.oidc.registerClient({ - clientName: registrationRequest.client_name, - redirectUris: registrationRequest.redirect_uris, - responseTypes: registrationRequest.response_types, - grantTypes: registrationRequest.grant_types, - scope: registrationRequest.scope, + clientName, + redirectUris, + responseTypes, + grantTypes, + scope, }); res.status(201).json({ From ec6cb6bce220e854674001e54aefecd96f7c0962 Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Tue, 9 Sep 2025 13:17:00 +0200 Subject: [PATCH 27/28] chore: code review comments Signed-off-by: benjdlambert --- ...0250909120000_oidc_client_registration.js} | 0 .../auth-backend/src/database/OidcDatabase.ts | 36 +++-- plugins/auth-backend/src/migrations.test.ts | 140 ++++++++++++++++++ .../src/service/OidcRouter.test.ts | 49 +++--- .../auth-backend/src/service/OidcRouter.ts | 37 +++-- .../src/service/OidcService.test.ts | 67 ++++----- .../auth-backend/src/service/OidcService.ts | 95 +----------- 7 files changed, 247 insertions(+), 177 deletions(-) rename plugins/auth-backend/migrations/{20250701120000_oidc_client_registration.js => 20250909120000_oidc_client_registration.js} (100%) diff --git a/plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js b/plugins/auth-backend/migrations/20250909120000_oidc_client_registration.js similarity index 100% rename from plugins/auth-backend/migrations/20250701120000_oidc_client_registration.js rename to plugins/auth-backend/migrations/20250909120000_oidc_client_registration.js diff --git a/plugins/auth-backend/src/database/OidcDatabase.ts b/plugins/auth-backend/src/database/OidcDatabase.ts index ec9a879803..ebb6619400 100644 --- a/plugins/auth-backend/src/database/OidcDatabase.ts +++ b/plugins/auth-backend/src/database/OidcDatabase.ts @@ -202,14 +202,24 @@ export class OidcDatabase { }); } - const [updated] = await this.db( + const returnedRows = await this.db( 'oauth_authorization_sessions', ) .where('id', session.id) .update(updatedFields) .returning('*'); - return this.rowToAuthorizationSession(updated) as AuthorizationSession; + if (returnedRows.length !== 1) { + throw new Error( + `Failed to retrieve updated authorization session with id ${session.id}`, + ); + } + + const [returnedSession] = returnedRows; + + return this.rowToAuthorizationSession( + returnedSession, + ) as AuthorizationSession; } async getAuthorizationSession({ id }: { id: string }) { @@ -290,17 +300,25 @@ export class OidcDatabase { }); } - const [updated] = await this.db( + const returnedRows = await this.db( 'oidc_authorization_codes', ) .where('code', authorizationCode.code) .update(updatedFields) .returning('*'); - return this.rowToAuthorizationCode(updated) as AuthorizationCode; + if (returnedRows.length !== 1) { + throw new Error( + `Failed to retrieve updated authorization code with code ${authorizationCode.code}`, + ); + } + + const [returnedCode] = returnedRows; + + return this.rowToAuthorizationCode(returnedCode) as AuthorizationCode; } - private rowToClient(row: Partial): Partial { + private rowToClient(row: OidcClientRow): Client { return { clientId: row.client_id, clientName: row.client_name, @@ -332,12 +350,12 @@ export class OidcDatabase { code_challenge_method: session.codeChallengeMethod, nonce: session.nonce, status: session.status, - expires_at: session.expiresAt, + expires_at: toDate(session.expiresAt), }; } private rowToAuthorizationSession( - row: Partial, + row: OAuthAuthorizationSessionRow, ): Partial { return { id: row.id, @@ -361,13 +379,13 @@ export class OidcDatabase { return { code: authorizationCode.code, session_id: authorizationCode.sessionId, - expires_at: authorizationCode.expiresAt, + expires_at: toDate(authorizationCode.expiresAt), used: authorizationCode.used, }; } private rowToAuthorizationCode( - row: Partial, + row: OidcAuthorizationCodeRow, ): Partial { return { code: row.code, diff --git a/plugins/auth-backend/src/migrations.test.ts b/plugins/auth-backend/src/migrations.test.ts index f9575c70fd..7cb6f982ea 100644 --- a/plugins/auth-backend/src/migrations.test.ts +++ b/plugins/auth-backend/src/migrations.test.ts @@ -186,4 +186,144 @@ describe('migrations', () => { await knex.destroy(); }, ); + + it.each(databases.eachSupportedId())( + '20250909120000_oidc_client_registration.js, %p', + async databaseId => { + const knex = await databases.init(databaseId); + + await migrateUntilBefore( + knex, + '20250909120000_oidc_client_registration.js', + ); + await migrateUpOnce(knex); + + await knex + .insert({ + client_id: 'test-client-id', + client_secret: 'test-client-secret', + client_name: 'Test Client', + response_types: JSON.stringify(['code']), + grant_types: JSON.stringify(['authorization_code']), + redirect_uris: JSON.stringify(['https://example.com/callback']), + scope: 'openid profile', + metadata: JSON.stringify({ description: 'Test client' }), + }) + .into('oidc_clients'); + + await expect( + knex('oidc_clients').where('client_id', 'test-client-id').first(), + ).resolves.toEqual({ + client_id: 'test-client-id', + client_secret: 'test-client-secret', + client_name: 'Test Client', + response_types: JSON.stringify(['code']), + grant_types: JSON.stringify(['authorization_code']), + redirect_uris: JSON.stringify(['https://example.com/callback']), + scope: 'openid profile', + metadata: JSON.stringify({ description: 'Test client' }), + }); + + await knex + .insert({ + id: 'test-session-id', + client_id: 'test-client-id', + user_entity_ref: 'user:default/test-user', + redirect_uri: 'https://example.com/callback', + scope: 'openid', + state: 'test-state', + response_type: 'code', + code_challenge: 'test-challenge', + code_challenge_method: 'S256', + nonce: 'test-nonce', + status: 'pending', + expires_at: new Date(Date.now() + 3600000), + }) + .into('oauth_authorization_sessions'); + + await expect( + knex('oauth_authorization_sessions') + .where('id', 'test-session-id') + .first(), + ).resolves.toEqual( + expect.objectContaining({ + id: 'test-session-id', + client_id: 'test-client-id', + user_entity_ref: 'user:default/test-user', + redirect_uri: 'https://example.com/callback', + scope: 'openid', + state: 'test-state', + response_type: 'code', + code_challenge: 'test-challenge', + code_challenge_method: 'S256', + nonce: 'test-nonce', + status: 'pending', + }), + ); + + await knex + .insert({ + code: 'test-auth-code', + session_id: 'test-session-id', + expires_at: new Date(Date.now() + 600000), + used: false, + }) + .into('oidc_authorization_codes'); + + await expect( + knex('oidc_authorization_codes') + .where('code', 'test-auth-code') + .first(), + ).resolves.toEqual( + expect.objectContaining({ + code: 'test-auth-code', + session_id: 'test-session-id', + }), + ); + + await expect( + knex + .insert({ + id: 'invalid-session', + client_id: 'non-existent-client', + redirect_uri: 'https://example.com/callback', + response_type: 'code', + expires_at: new Date(), + }) + .into('oauth_authorization_sessions'), + ).rejects.toThrow(); + + await expect( + knex + .insert({ + code: 'invalid-code', + session_id: 'non-existent-session', + expires_at: new Date(), + }) + .into('oidc_authorization_codes'), + ).rejects.toThrow(); + + await knex('oauth_authorization_sessions') + .where('id', 'test-session-id') + .del(); + + await expect( + knex('oidc_authorization_codes').where('session_id', 'test-session-id'), + ).resolves.toHaveLength(0); + + await migrateDownOnce(knex); + + const tables = [ + 'oidc_clients', + 'oauth_authorization_sessions', + 'oidc_authorization_codes', + ]; + + for (const table of tables) { + await expect(knex.schema.hasTable(table)).resolves.toBe(false); + } + + await knex.destroy(); + }, + ); }); diff --git a/plugins/auth-backend/src/service/OidcRouter.test.ts b/plugins/auth-backend/src/service/OidcRouter.test.ts index 81430ef05e..de865a9c43 100644 --- a/plugins/auth-backend/src/service/OidcRouter.test.ts +++ b/plugins/auth-backend/src/service/OidcRouter.test.ts @@ -24,6 +24,7 @@ import { startTestBackend, TestDatabases, TestDatabaseId, + mockCredentials, } from '@backstage/backend-test-utils'; import request from 'supertest'; import crypto from 'crypto'; @@ -413,13 +414,9 @@ describe('OidcRouter', () => { ], }); - httpAuth.credentials.mockResolvedValueOnce({ - principal: { - type: 'user', - userEntityRef: 'user:default/test-user', - }, - $$type: '@backstage/BackstageCredentials', - }); + httpAuth.credentials.mockResolvedValueOnce( + mockCredentials.user('user:default/test-user'), + ); auth.isPrincipal.mockReturnValueOnce(true); @@ -437,7 +434,7 @@ describe('OidcRouter', () => { it('should reject auth session', async () => { const { - mocks: { service }, + mocks: { service, httpAuth, auth }, router, } = await createRouter(databaseId); @@ -457,6 +454,12 @@ describe('OidcRouter', () => { state: 'test-state', }); + httpAuth.credentials.mockResolvedValueOnce( + mockCredentials.user('user:default/test-user'), + ); + + auth.isPrincipal.mockReturnValueOnce(true); + const { server } = await startTestBackend({ features: [ createBackendPlugin({ @@ -496,13 +499,9 @@ describe('OidcRouter', () => { router, } = await createRouter(databaseId); - httpAuth.credentials.mockResolvedValueOnce({ - principal: { - type: 'user', - userEntityRef: 'user:default/test-user', - }, - $$type: '@backstage/BackstageCredentials', - }); + httpAuth.credentials.mockResolvedValueOnce( + mockCredentials.user('user:default/test-user'), + ); auth.isPrincipal.mockReturnValueOnce(true); @@ -590,13 +589,9 @@ describe('OidcRouter', () => { token: 'mock-access-token-pkce', }); - httpAuth.credentials.mockResolvedValueOnce({ - principal: { - type: 'user', - userEntityRef: 'user:default/test-user-pkce', - }, - $$type: '@backstage/BackstageCredentials', - }); + httpAuth.credentials.mockResolvedValueOnce( + mockCredentials.user('user:default/test-user-pkce'), + ); auth.isPrincipal.mockReturnValueOnce(true); @@ -725,13 +720,9 @@ describe('OidcRouter', () => { token: 'mock-access-token-s256', }); - httpAuth.credentials.mockResolvedValueOnce({ - principal: { - type: 'user', - userEntityRef: 'user:default/test-user-s256', - }, - $$type: '@backstage/BackstageCredentials', - }); + httpAuth.credentials.mockResolvedValueOnce( + mockCredentials.user('user:default/test-user-s256'), + ); auth.isPrincipal.mockReturnValueOnce(true); diff --git a/plugins/auth-backend/src/service/OidcRouter.ts b/plugins/auth-backend/src/service/OidcRouter.ts index f20b5f4ede..9a3308b4eb 100644 --- a/plugins/auth-backend/src/service/OidcRouter.ts +++ b/plugins/auth-backend/src/service/OidcRouter.ts @@ -134,19 +134,19 @@ export class OidcRouter { clientId: clientId as string, redirectUri: redirectUri as string, responseType: responseType as string, - scope: scope as string, - state: state as string, - nonce: nonce as string, - codeChallenge: codeChallenge as string, - codeChallengeMethod: codeChallengeMethod as string, + scope: scope as string | undefined, + state: state as string | undefined, + nonce: nonce as string | undefined, + codeChallenge: codeChallenge as string | undefined, + codeChallengeMethod: codeChallengeMethod as string | undefined, }); // todo(blam): maybe this URL could be overridable by config if // the plugin is mounted somewhere else? // support slashes in baseUrl? const authSessionRedirectUrl = new URL( - `/oauth2/authorize/${result.id}`, - this.appUrl, + `./oauth2/authorize/${result.id}`, + ensureTrailingSlash(this.appUrl), ); return res.redirect(authSessionRedirectUrl.toString()); @@ -171,7 +171,7 @@ export class OidcRouter { }); // Authorization Session request details endpoint - // Returns Authorization Session request details for the frontned + // Returns Authorization Session request details for the frontend router.get('/v1/sessions/:sessionId', async (req, res) => { const { sessionId } = req.params; @@ -263,12 +263,25 @@ export class OidcRouter { }); } + const httpCredentials = await this.httpAuth.credentials(req); + + if (!this.auth.isPrincipal(httpCredentials, 'user')) { + return res.status(401).json({ + error: 'unauthorized', + error_description: 'Authentication required', + }); + } + + const { userEntityRef } = httpCredentials.principal; try { const session = await this.oidc.getAuthorizationSession({ sessionId, }); - await this.oidc.rejectAuthorizationSession({ sessionId }); + await this.oidc.rejectAuthorizationSession({ + sessionId, + userEntityRef, + }); const errorParams = new URLSearchParams(); errorParams.append('error', 'access_denied'); @@ -413,3 +426,9 @@ export class OidcRouter { return router; } } +function ensureTrailingSlash(appUrl: string): string | URL | undefined { + if (appUrl.endsWith('/')) { + return appUrl; + } + return `${appUrl}/`; +} diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index 7829be3841..328a753e4c 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -466,6 +466,7 @@ describe('OidcService', () => { await service.rejectAuthorizationSession({ sessionId: authSession.id, + userEntityRef: 'user:default/test', }); await expect( @@ -553,6 +554,7 @@ describe('OidcService', () => { await service.rejectAuthorizationSession({ sessionId: authSession.id, + userEntityRef: 'user:default/test', }); await expect( @@ -580,6 +582,7 @@ describe('OidcService', () => { await service.rejectAuthorizationSession({ sessionId: authSession.id, + userEntityRef: 'user:default/test', }); await expect( @@ -595,6 +598,7 @@ describe('OidcService', () => { await expect( service.rejectAuthorizationSession({ sessionId: 'invalid-session', + userEntityRef: 'user:default/test', }), ).rejects.toThrow('Invalid authorization session'); }); @@ -621,6 +625,7 @@ describe('OidcService', () => { await expect( service.rejectAuthorizationSession({ sessionId: authSession.id, + userEntityRef: 'user:default/test', }), ).rejects.toThrow('Authorization session not found or expired'); }); @@ -641,49 +646,15 @@ describe('OidcService', () => { await service.rejectAuthorizationSession({ sessionId: authSession.id, + userEntityRef: 'user:default/test', }); await expect( service.rejectAuthorizationSession({ sessionId: authSession.id, - }), - ).rejects.toThrow('Authorization session not found or expired'); - }); - }); - - describe('authorize', () => { - it('should create direct authorization', async () => { - const { service } = await createOidcService(databaseId); - - const client = await service.registerClient({ - clientName: 'Test Client', - redirectUris: ['https://example.com/callback'], - }); - - const result = await service.authorize({ - clientId: client.clientId, - redirectUri: 'https://example.com/callback', - responseType: 'code', - userEntityRef: 'user:default/test', - state: 'test-state', - }); - - expect(result.redirectUrl).toMatch( - /^https:\/\/example\.com\/callback\?code=.+&state=test-state$/, - ); - }); - - it('should throw error for invalid client', async () => { - const { service } = await createOidcService(databaseId); - - await expect( - service.authorize({ - clientId: 'invalid-client', - redirectUri: 'https://example.com/callback', - responseType: 'code', userEntityRef: 'user:default/test', }), - ).rejects.toThrow('Invalid client_id'); + ).rejects.toThrow('Authorization session not found or expired'); }); }); @@ -698,14 +669,18 @@ describe('OidcService', () => { redirectUris: ['https://example.com/callback'], }); - const authResult = await service.authorize({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', - userEntityRef: 'user:default/test', scope: 'openid', }); + const authResult = await service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }); + const code = new URL(authResult.redirectUrl).searchParams.get('code')!; const tokenResult = await service.exchangeCodeForToken({ @@ -751,15 +726,19 @@ describe('OidcService', () => { .update(codeVerifier) .digest('base64url'); - const authResult = await service.authorize({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', - userEntityRef: 'user:default/test', codeChallenge, codeChallengeMethod: 'S256', }); + const authResult = await service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }); + const code = new URL(authResult.redirectUrl).searchParams.get('code')!; const tokenResult = await service.exchangeCodeForToken({ @@ -781,15 +760,19 @@ describe('OidcService', () => { }); const codeChallenge = 'test-challenge'; - const authResult = await service.authorize({ + const authSession = await service.createAuthorizationSession({ clientId: client.clientId, redirectUri: 'https://example.com/callback', responseType: 'code', - userEntityRef: 'user:default/test', codeChallenge, codeChallengeMethod: 'S256', }); + const authResult = await service.approveAuthorizationSession({ + sessionId: authSession.id, + userEntityRef: 'user:default/test', + }); + const code = new URL(authResult.redirectUrl).searchParams.get('code')!; await expect( diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index 3139ccc73d..e8a308148a 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -300,9 +300,14 @@ export class OidcService { }; } - public async rejectAuthorizationSession(opts: { sessionId: string }) { + public async rejectAuthorizationSession(opts: { + sessionId: string; + userEntityRef: string; + }) { + const { sessionId, userEntityRef } = opts; + const session = await this.oidc.getAuthorizationSession({ - id: opts.sessionId, + id: sessionId, }); if (!session) { @@ -320,94 +325,8 @@ export class OidcService { await this.oidc.updateAuthorizationSession({ id: session.id, status: 'rejected', - }); - } - - public async authorize(opts: { - clientId: string; - redirectUri: string; - responseType: string; - scope?: string; - state?: string; - nonce?: string; - codeChallenge?: string; - codeChallengeMethod?: string; - userEntityRef: string; - }) { - const { - clientId, - redirectUri, - responseType, - scope, - state, - nonce, - codeChallenge, - codeChallengeMethod, userEntityRef, - } = opts; - - if (responseType !== 'code') { - throw new InputError('Only authorization code flow is supported'); - } - - const client = await this.oidc.getClient({ clientId }); - if (!client) { - throw new InputError('Invalid client_id'); - } - - if (!client.redirectUris.includes(redirectUri)) { - throw new InputError('Invalid redirect_uri'); - } - - if (codeChallenge) { - if ( - !codeChallengeMethod || - !['S256', 'plain'].includes(codeChallengeMethod) - ) { - throw new InputError('Invalid code_challenge_method'); - } - } - - const sessionId = crypto.randomUUID(); - const sessionExpiresAt = DateTime.now().plus({ hours: 1 }).toJSDate(); - - await this.oidc.createAuthorizationSession({ - id: sessionId, - clientId, - userEntityRef, - redirectUri, - responseType, - scope, - state, - codeChallenge, - codeChallengeMethod, - nonce, - expiresAt: sessionExpiresAt, }); - - await this.oidc.updateAuthorizationSession({ - id: sessionId, - status: 'approved', - }); - - const authorizationCode = crypto.randomBytes(32).toString('base64url'); - const codeExpiresAt = DateTime.now().plus({ minutes: 10 }).toJSDate(); - - await this.oidc.createAuthorizationCode({ - code: authorizationCode, - sessionId, - expiresAt: codeExpiresAt, - }); - - const redirectUrl = new URL(redirectUri); - redirectUrl.searchParams.append('code', authorizationCode); - if (state) { - redirectUrl.searchParams.append('state', state); - } - - return { - redirectUrl: redirectUrl.toString(), - }; } public async exchangeCodeForToken(params: { From c2afe12dfd479e95b6cb256c64befcee24a42d0f Mon Sep 17 00:00:00 2001 From: benjdlambert Date: Tue, 9 Sep 2025 13:50:38 +0200 Subject: [PATCH 28/28] chore: cleanup a little bit more :tada: Signed-off-by: benjdlambert Signed-off-by: benjdlambert --- ...20250909120000_oidc_client_registration.js | 9 +++++--- plugins/auth-backend/package.json | 1 + plugins/auth-backend/src/migrations.test.ts | 22 ------------------- .../src/service/OidcService.test.ts | 6 ++--- .../auth-backend/src/service/OidcService.ts | 19 ++++++++-------- yarn.lock | 10 +++++++++ 6 files changed, 29 insertions(+), 38 deletions(-) diff --git a/plugins/auth-backend/migrations/20250909120000_oidc_client_registration.js b/plugins/auth-backend/migrations/20250909120000_oidc_client_registration.js index e175c922c1..87391c3467 100644 --- a/plugins/auth-backend/migrations/20250909120000_oidc_client_registration.js +++ b/plugins/auth-backend/migrations/20250909120000_oidc_client_registration.js @@ -44,12 +44,12 @@ exports.up = async function up(knex) { .comment('The name of the client, should be human readable'); table - .text('response_types') + .text('response_types', 'longtext') .notNullable() .comment('JSON array of supported response types'); table - .text('grant_types') + .text('grant_types', 'longtext') .notNullable() .comment('JSON array of supported grant types'); @@ -82,7 +82,10 @@ exports.up = async function up(knex) { .nullable() .comment('Backstage user entity reference'); - table.text('redirect_uri').notNullable().comment('Client redirect URI'); + table + .text('redirect_uri', 'longtext') + .notNullable() + .comment('Client redirect URI'); table.text('scope').nullable().comment('Requested scopes space-separated'); diff --git a/plugins/auth-backend/package.json b/plugins/auth-backend/package.json index 0dddd935e6..9507df7fc3 100644 --- a/plugins/auth-backend/package.json +++ b/plugins/auth-backend/package.json @@ -60,6 +60,7 @@ "knex": "^3.0.0", "lodash": "^4.17.21", "luxon": "^3.0.0", + "matcher": "^4.0.0", "minimatch": "^9.0.0", "passport": "^0.7.0", "uuid": "^11.0.0" diff --git a/plugins/auth-backend/src/migrations.test.ts b/plugins/auth-backend/src/migrations.test.ts index 7cb6f982ea..df7063351a 100644 --- a/plugins/auth-backend/src/migrations.test.ts +++ b/plugins/auth-backend/src/migrations.test.ts @@ -281,28 +281,6 @@ describe('migrations', () => { }), ); - await expect( - knex - .insert({ - id: 'invalid-session', - client_id: 'non-existent-client', - redirect_uri: 'https://example.com/callback', - response_type: 'code', - expires_at: new Date(), - }) - .into('oauth_authorization_sessions'), - ).rejects.toThrow(); - - await expect( - knex - .insert({ - code: 'invalid-code', - session_id: 'non-existent-session', - expires_at: new Date(), - }) - .into('oidc_authorization_codes'), - ).rejects.toThrow(); - await knex('oauth_authorization_sessions') .where('id', 'test-session-id') .del(); diff --git a/plugins/auth-backend/src/service/OidcService.test.ts b/plugins/auth-backend/src/service/OidcService.test.ts index 328a753e4c..e4e1673397 100644 --- a/plugins/auth-backend/src/service/OidcService.test.ts +++ b/plugins/auth-backend/src/service/OidcService.test.ts @@ -244,16 +244,16 @@ describe('OidcService', () => { mocks: { config }, } = await createOidcService(databaseId); - config.getOptionalStringArray.mockReturnValue(['cursor://*']); + config.getOptionalStringArray.mockReturnValue(['cursor:*']); const client = await service.registerClient({ clientName: 'Test Client', - redirectUris: ['cursor://callback'], + redirectUris: ['cursor://callback/asd?asd=asd'], }); expect(client).toEqual( expect.objectContaining({ - redirectUris: ['cursor://callback'], + redirectUris: ['cursor://callback/asd?asd=asd'], }), ); }); diff --git a/plugins/auth-backend/src/service/OidcService.ts b/plugins/auth-backend/src/service/OidcService.ts index e8a308148a..b4c6bb122b 100644 --- a/plugins/auth-backend/src/service/OidcService.ts +++ b/plugins/auth-backend/src/service/OidcService.ts @@ -25,6 +25,7 @@ import { decodeJwt } from 'jose'; import crypto from 'crypto'; import { OidcDatabase } from '../database/OidcDatabase'; import { DateTime } from 'luxon'; +import matcher from 'matcher'; export class OidcService { private constructor( @@ -121,17 +122,15 @@ export class OidcService { const allowedRedirectUriPatterns = this.config.getOptionalStringArray( 'auth.experimentalDynamicClientRegistration.allowedRedirectUriPatterns', - ); + ) ?? ['*']; - if (allowedRedirectUriPatterns) { - for (const redirectUri of opts.redirectUris ?? []) { - if ( - !allowedRedirectUriPatterns.some(pattern => - new RegExp(pattern).test(redirectUri), - ) - ) { - throw new InputError('Invalid redirect_uri'); - } + for (const redirectUri of opts.redirectUris ?? []) { + if ( + !allowedRedirectUriPatterns.some(pattern => + matcher.isMatch(redirectUri, pattern), + ) + ) { + throw new InputError('Invalid redirect_uri'); } } diff --git a/yarn.lock b/yarn.lock index 853749b99e..5d78326d94 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4196,6 +4196,7 @@ __metadata: knex: "npm:^3.0.0" lodash: "npm:^4.17.21" luxon: "npm:^3.0.0" + matcher: "npm:^4.0.0" minimatch: "npm:^9.0.0" passport: "npm:^0.7.0" supertest: "npm:^7.0.0" @@ -37208,6 +37209,15 @@ __metadata: languageName: node linkType: hard +"matcher@npm:^4.0.0": + version: 4.0.0 + resolution: "matcher@npm:4.0.0" + dependencies: + escape-string-regexp: "npm:^4.0.0" + checksum: 10/d338aff31d8dfd3626873e43777f46b123579734d53bb8d18d64b08a822ba5e8d39f5fe2e23403258e6143aa0cbe20a15662720d825cd0d3af961d5a44230328 + languageName: node + linkType: hard + "material-ui-confirm@npm:^3.0.12": version: 3.0.18 resolution: "material-ui-confirm@npm:3.0.18"