fix: preserve path components in OAuth issuer URLs (#12448)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Gregory Shikhman <cornmander@cornmander.com>
This commit is contained in:
Chris Coutinho
2025-11-02 22:19:46 +01:00
committed by GitHub
parent c0495ce2f9
commit 9187f6f6d1
2 changed files with 182 additions and 20 deletions

View File

@@ -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);
});
});
});

View File

@@ -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<ReturnType<typeof OAuthUtils.discoverAuthorizationServerMetadata>>
>;
}> {
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<string>();
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<ReturnType<typeof OAuthUtils.discoverAuthorizationServerMetadata>>
> | 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;
}