diff --git a/packages/core/src/mcp/oauth-provider.test.ts b/packages/core/src/mcp/oauth-provider.test.ts index 6ff5ea0683..69e9e9374a 100644 --- a/packages/core/src/mcp/oauth-provider.test.ts +++ b/packages/core/src/mcp/oauth-provider.test.ts @@ -44,9 +44,10 @@ import type { import { MCPOAuthProvider } from './oauth-provider.js'; import type { OAuthToken } from './token-storage/types.js'; import { MCPOAuthTokenStorage } from './oauth-token-storage.js'; -import type { - OAuthAuthorizationServerMetadata, - OAuthProtectedResourceMetadata, +import { + OAuthUtils, + type OAuthAuthorizationServerMetadata, + type OAuthProtectedResourceMetadata, } from './oauth-utils.js'; import { coreEvents } from '../utils/events.js'; @@ -1369,4 +1370,98 @@ describe('MCPOAuthProvider', () => { ); }); }); + + describe('issuer discovery conformance', () => { + const registrationMetadata: OAuthAuthorizationServerMetadata = { + issuer: 'http://localhost:8888/realms/my-realm', + authorization_endpoint: + 'http://localhost:8888/realms/my-realm/protocol/openid-connect/auth', + token_endpoint: + 'http://localhost:8888/realms/my-realm/protocol/openid-connect/token', + registration_endpoint: + 'http://localhost:8888/realms/my-realm/clients-registrations/openid-connect', + }; + + it('falls back to path-based issuer when origin discovery fails', async () => { + const authProvider = new MCPOAuthProvider(); + const providerWithAccess = authProvider as unknown as { + discoverAuthServerMetadataForRegistration: ( + authorizationUrl: string, + ) => Promise<{ + issuerUrl: string; + metadata: OAuthAuthorizationServerMetadata; + }>; + }; + + vi.spyOn( + OAuthUtils, + 'discoverAuthorizationServerMetadata', + ).mockImplementation(async (issuer) => { + if (issuer === 'http://localhost:8888/realms/my-realm') { + return registrationMetadata; + } + return null; + }); + + const result = + await providerWithAccess.discoverAuthServerMetadataForRegistration( + 'http://localhost:8888/realms/my-realm/protocol/openid-connect/auth', + ); + + expect( + vi.mocked(OAuthUtils.discoverAuthorizationServerMetadata).mock.calls, + ).toEqual([ + ['http://localhost:8888'], + ['http://localhost:8888/realms/my-realm'], + ]); + expect(result.issuerUrl).toBe('http://localhost:8888/realms/my-realm'); + expect(result.metadata).toBe(registrationMetadata); + }); + + it('trims versioned segments from authorization endpoints', async () => { + const authProvider = new MCPOAuthProvider(); + const providerWithAccess = authProvider as unknown as { + discoverAuthServerMetadataForRegistration: ( + authorizationUrl: string, + ) => Promise<{ + issuerUrl: string; + metadata: OAuthAuthorizationServerMetadata; + }>; + }; + + const oktaMetadata: OAuthAuthorizationServerMetadata = { + issuer: 'https://auth.okta.local/oauth2/default', + authorization_endpoint: + 'https://auth.okta.local/oauth2/default/v1/authorize', + token_endpoint: 'https://auth.okta.local/oauth2/default/v1/token', + registration_endpoint: + 'https://auth.okta.local/oauth2/default/v1/register', + }; + + const attempts: string[] = []; + vi.spyOn( + OAuthUtils, + 'discoverAuthorizationServerMetadata', + ).mockImplementation(async (issuer) => { + attempts.push(issuer); + if (issuer === 'https://auth.okta.local/oauth2/default') { + return oktaMetadata; + } + return null; + }); + + const result = + await providerWithAccess.discoverAuthServerMetadataForRegistration( + 'https://auth.okta.local/oauth2/default/v1/authorize', + ); + + expect(attempts).toEqual([ + 'https://auth.okta.local', + 'https://auth.okta.local/oauth2/default/v1', + 'https://auth.okta.local/oauth2/default', + ]); + expect(result.issuerUrl).toBe('https://auth.okta.local/oauth2/default'); + expect(result.metadata).toBe(oktaMetadata); + }); + }); }); diff --git a/packages/core/src/mcp/oauth-provider.ts b/packages/core/src/mcp/oauth-provider.ts index c366ea9f52..0069a627fa 100644 --- a/packages/core/src/mcp/oauth-provider.ts +++ b/packages/core/src/mcp/oauth-provider.ts @@ -54,7 +54,7 @@ export interface OAuthTokenResponse { } /** - * Dynamic client registration request. + * Dynamic client registration request (RFC 7591). */ export interface OAuthClientRegistrationRequest { client_name: string; @@ -62,12 +62,11 @@ export interface OAuthClientRegistrationRequest { grant_types: string[]; response_types: string[]; token_endpoint_auth_method: string; - code_challenge_method?: string[]; scope?: string; } /** - * Dynamic client registration response. + * Dynamic client registration response (RFC 7591). */ export interface OAuthClientRegistrationResponse { client_id: string; @@ -78,7 +77,6 @@ export interface OAuthClientRegistrationResponse { grant_types: string[]; response_types: string[]; token_endpoint_auth_method: string; - code_challenge_method?: string[]; scope?: string; } @@ -125,7 +123,6 @@ export class MCPOAuthProvider { grant_types: ['authorization_code', 'refresh_token'], response_types: ['code'], token_endpoint_auth_method: 'none', // Public client - code_challenge_method: ['S256'], scope: config.scopes?.join(' ') || '', }; @@ -160,6 +157,85 @@ export class MCPOAuthProvider { return OAuthUtils.discoverOAuthConfig(mcpServerUrl); } + private async discoverAuthServerMetadataForRegistration( + authorizationUrl: string, + ): Promise<{ + issuerUrl: string; + metadata: NonNullable< + Awaited> + >; + }> { + const authUrl = new URL(authorizationUrl); + + // Preserve path components for issuers with path-based discovery (e.g., Keycloak) + // Extract issuer by removing the OIDC protocol-specific path suffix + // For example: http://localhost:8888/realms/my-realm/protocol/openid-connect/auth + // -> http://localhost:8888/realms/my-realm + const oidcPatterns = [ + '/protocol/openid-connect/auth', + '/protocol/openid-connect/authorize', + '/oauth2/authorize', + '/oauth/authorize', + '/authorize', + ]; + + let pathname = authUrl.pathname.replace(/\/$/, ''); // Trim trailing slash + for (const pattern of oidcPatterns) { + if (pathname.endsWith(pattern)) { + pathname = pathname.slice(0, -pattern.length); + break; + } + } + + const issuerCandidates = new Set(); + issuerCandidates.add(authUrl.origin); + + if (pathname) { + issuerCandidates.add(`${authUrl.origin}${pathname}`); + + const versionSegmentPattern = /^v\d+(\.\d+)?$/i; + const segments = pathname.split('/').filter(Boolean); + const lastSegment = segments.at(-1); + if (lastSegment && versionSegmentPattern.test(lastSegment)) { + const withoutVersionPath = segments.slice(0, -1); + if (withoutVersionPath.length) { + issuerCandidates.add( + `${authUrl.origin}/${withoutVersionPath.join('/')}`, + ); + } + } + } + + const attemptedIssuers = Array.from(issuerCandidates); + let selectedIssuer = attemptedIssuers[0]; + let discoveredMetadata: NonNullable< + Awaited> + > | null = null; + + for (const issuer of attemptedIssuers) { + debugLogger.debug(` Trying issuer URL: ${issuer}`); + const metadata = + await OAuthUtils.discoverAuthorizationServerMetadata(issuer); + if (metadata) { + selectedIssuer = issuer; + discoveredMetadata = metadata; + break; + } + } + + if (!discoveredMetadata) { + throw new Error( + `Failed to fetch authorization server metadata for client registration (attempted issuers: ${attemptedIssuers.join(', ')})`, + ); + } + + debugLogger.debug(` Selected issuer URL: ${selectedIssuer}`); + return { + issuerUrl: selectedIssuer, + metadata: discoveredMetadata, + }; + } + /** * Generate PKCE parameters for OAuth flow. * @@ -682,20 +758,11 @@ export class MCPOAuthProvider { ); } - const authUrl = new URL(config.authorizationUrl); - const serverUrl = `${authUrl.protocol}//${authUrl.host}`; - debugLogger.debug('→ Attempting dynamic client registration...'); - - // Get the authorization server metadata for registration - const authServerMetadata = - await OAuthUtils.discoverAuthorizationServerMetadata(serverUrl); - - if (!authServerMetadata) { - throw new Error( - 'Failed to fetch authorization server metadata for client registration', + const { metadata: authServerMetadata } = + await this.discoverAuthServerMetadataForRegistration( + config.authorizationUrl, ); - } registrationUrl = authServerMetadata.registration_endpoint; }