From 069689a855c8679e142510cec7d61d94ccc62977 Mon Sep 17 00:00:00 2001 From: garrettsparks Date: Thu, 22 Jan 2026 11:53:39 -0800 Subject: [PATCH] use issuer instead of authorization_endpoint for dynamic client registration flow --- packages/core/src/mcp/oauth-provider.test.ts | 22 ++++++++++++++++++++ packages/core/src/mcp/oauth-provider.ts | 19 ++++++++--------- packages/core/src/mcp/oauth-utils.test.ts | 15 +++++++++++++ packages/core/src/mcp/oauth-utils.ts | 1 + packages/core/src/tools/mcp-client.ts | 2 ++ 5 files changed, 49 insertions(+), 10 deletions(-) diff --git a/packages/core/src/mcp/oauth-provider.test.ts b/packages/core/src/mcp/oauth-provider.test.ts index a06d2134629..9ab2109861f 100644 --- a/packages/core/src/mcp/oauth-provider.test.ts +++ b/packages/core/src/mcp/oauth-provider.test.ts @@ -122,6 +122,7 @@ describe('MCPOAuthProvider', () => { clientId: 'test-client-id', clientSecret: 'test-client-secret', authorizationUrl: 'https://auth.example.com/authorize', + issuer: 'https://auth.example.com', tokenUrl: 'https://auth.example.com/token', scopes: ['read', 'write'], redirectUri: 'http://localhost:7777/oauth/callback', @@ -617,6 +618,27 @@ describe('MCPOAuthProvider', () => { ); }); + it('should throw error when issuer is missing and dynamic registration is needed', async () => { + const configWithoutIssuer: MCPOAuthConfig = { + enabled: mockConfig.enabled, + authorizationUrl: mockConfig.authorizationUrl, + tokenUrl: mockConfig.tokenUrl, + scopes: mockConfig.scopes, + redirectUri: mockConfig.redirectUri, + audiences: mockConfig.audiences, + }; + + mockHttpServer.listen.mockImplementation((port, callback) => { + callback?.(); + }); + + const authProvider = new MCPOAuthProvider(); + + await expect( + authProvider.authenticate('test-server', configWithoutIssuer), + ).rejects.toThrow('Cannot perform dynamic registration without issuer'); + }); + it('should handle OAuth callback errors', async () => { let callbackHandler: unknown; vi.mocked(http.createServer).mockImplementation((handler) => { diff --git a/packages/core/src/mcp/oauth-provider.ts b/packages/core/src/mcp/oauth-provider.ts index b79ec693a30..05c28208173 100644 --- a/packages/core/src/mcp/oauth-provider.ts +++ b/packages/core/src/mcp/oauth-provider.ts @@ -27,6 +27,7 @@ export interface MCPOAuthConfig { clientId?: string; clientSecret?: string; authorizationUrl?: string; + issuer?: string; tokenUrl?: string; scopes?: string[]; audiences?: string[]; @@ -160,14 +161,14 @@ export class MCPOAuthProvider { } private async discoverAuthServerMetadataForRegistration( - authorizationUrl: string, + issuer: string, ): Promise<{ issuerUrl: string; metadata: NonNullable< Awaited> >; }> { - const authUrl = new URL(authorizationUrl); + const authUrl = new URL(issuer); // Preserve path components for issuers with path-based discovery (e.g., Keycloak) // Extract issuer by removing the OIDC protocol-specific path suffix @@ -785,6 +786,7 @@ export class MCPOAuthProvider { config = { ...config, authorizationUrl: discoveredConfig.authorizationUrl, + issuer: discoveredConfig.issuer, tokenUrl: discoveredConfig.tokenUrl, scopes: config.scopes || discoveredConfig.scopes || [], // Preserve existing client credentials @@ -815,6 +817,7 @@ export class MCPOAuthProvider { ...config, authorizationUrl: discoveredConfig.authorizationUrl, tokenUrl: discoveredConfig.tokenUrl, + issuer: discoveredConfig.issuer, scopes: config.scopes || discoveredConfig.scopes || [], registrationUrl: discoveredConfig.registrationUrl, // Preserve existing client credentials @@ -853,18 +856,14 @@ export class MCPOAuthProvider { // If no registration URL was previously discovered, try to discover it if (!registrationUrl) { - // Extract server URL from authorization URL - if (!config.authorizationUrl) { - throw new Error( - 'Cannot perform dynamic registration without authorization URL', - ); + // Use the issuer to discover registration endpoint + if (!config.issuer) { + throw new Error('Cannot perform dynamic registration without issuer'); } debugLogger.debug('→ Attempting dynamic client registration...'); const { metadata: authServerMetadata } = - await this.discoverAuthServerMetadataForRegistration( - config.authorizationUrl, - ); + await this.discoverAuthServerMetadataForRegistration(config.issuer); registrationUrl = authServerMetadata.registration_endpoint; } diff --git a/packages/core/src/mcp/oauth-utils.test.ts b/packages/core/src/mcp/oauth-utils.test.ts index 0e17f3c32e0..2baf8683d22 100644 --- a/packages/core/src/mcp/oauth-utils.test.ts +++ b/packages/core/src/mcp/oauth-utils.test.ts @@ -243,6 +243,7 @@ describe('OAuthUtils', () => { expect(config).toEqual({ authorizationUrl: 'https://auth.example.com/authorize', + issuer: 'https://auth.example.com', tokenUrl: 'https://auth.example.com/token', scopes: ['read', 'write'], }); @@ -277,6 +278,7 @@ describe('OAuthUtils', () => { expect(config).toEqual({ authorizationUrl: 'https://auth.example.com/authorize', + issuer: 'https://auth.example.com', tokenUrl: 'https://auth.example.com/token', scopes: ['read', 'write'], }); @@ -293,6 +295,19 @@ describe('OAuthUtils', () => { expect(config.scopes).toEqual([]); }); + + it('should use issuer from metadata', () => { + const metadata: OAuthAuthorizationServerMetadata = { + issuer: 'https://auth.example.com', + authorization_endpoint: 'https://auth.example.com/oauth/authorize', + token_endpoint: 'https://auth.example.com/token', + scopes_supported: ['read', 'write'], + }; + + const config = OAuthUtils.metadataToOAuthConfig(metadata); + + expect(config.issuer).toBe('https://auth.example.com'); + }); }); describe('parseWWWAuthenticateHeader', () => { diff --git a/packages/core/src/mcp/oauth-utils.ts b/packages/core/src/mcp/oauth-utils.ts index de87838a2a9..da39e6e7c33 100644 --- a/packages/core/src/mcp/oauth-utils.ts +++ b/packages/core/src/mcp/oauth-utils.ts @@ -148,6 +148,7 @@ export class OAuthUtils { ): MCPOAuthConfig { return { authorizationUrl: metadata.authorization_endpoint, + issuer: metadata.issuer, tokenUrl: metadata.token_endpoint, scopes: metadata.scopes_supported || [], registrationUrl: metadata.registration_endpoint, diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index 872a5019d4a..ed3e4503c64 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -604,6 +604,7 @@ async function handleAutomaticOAuth( const oauthAuthConfig = { enabled: true, authorizationUrl: oauthConfig.authorizationUrl, + issuer: oauthConfig.issuer, tokenUrl: oauthConfig.tokenUrl, scopes: oauthConfig.scopes || [], }; @@ -1606,6 +1607,7 @@ export async function connectToMcpServer( const oauthAuthConfig = { enabled: true, authorizationUrl: oauthConfig.authorizationUrl, + issuer: oauthConfig.issuer, tokenUrl: oauthConfig.tokenUrl, scopes: oauthConfig.scopes || [], };