use issuer instead of authorization_endpoint for oauth discovery (#17332)

Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
This commit is contained in:
garrettsparks
2026-02-18 14:38:04 -08:00
committed by GitHub
parent 221ea360b9
commit 037061e2e0
5 changed files with 49 additions and 10 deletions

View File

@@ -127,6 +127,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',
@@ -622,6 +623,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) => {

View File

@@ -27,6 +27,7 @@ export interface MCPOAuthConfig {
clientId?: string;
clientSecret?: string;
authorizationUrl?: string;
issuer?: string;
tokenUrl?: string;
scopes?: string[];
audiences?: string[];
@@ -161,14 +162,14 @@ export class MCPOAuthProvider {
}
private async discoverAuthServerMetadataForRegistration(
authorizationUrl: string,
issuer: string,
): Promise<{
issuerUrl: string;
metadata: NonNullable<
Awaited<ReturnType<typeof OAuthUtils.discoverAuthorizationServerMetadata>>
>;
}> {
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
@@ -784,6 +785,7 @@ export class MCPOAuthProvider {
config = {
...config,
authorizationUrl: discoveredConfig.authorizationUrl,
issuer: discoveredConfig.issuer,
tokenUrl: discoveredConfig.tokenUrl,
scopes: config.scopes || discoveredConfig.scopes || [],
// Preserve existing client credentials
@@ -814,6 +816,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
@@ -852,18 +855,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;
}

View File

@@ -252,6 +252,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'],
});
@@ -286,6 +287,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'],
});
@@ -302,6 +304,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', () => {

View File

@@ -146,6 +146,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,

View File

@@ -748,6 +748,7 @@ async function handleAutomaticOAuth(
const oauthAuthConfig = {
enabled: true,
authorizationUrl: oauthConfig.authorizationUrl,
issuer: oauthConfig.issuer,
tokenUrl: oauthConfig.tokenUrl,
scopes: oauthConfig.scopes || [],
};
@@ -1783,6 +1784,7 @@ export async function connectToMcpServer(
const oauthAuthConfig = {
enabled: true,
authorizationUrl: oauthConfig.authorizationUrl,
issuer: oauthConfig.issuer,
tokenUrl: oauthConfig.tokenUrl,
scopes: oauthConfig.scopes || [],
};