mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-30 15:04:16 -07:00
fix(mcp): handle equivalent root resource URLs in OAuth validation (#20231)
This commit is contained in:
@@ -272,6 +272,34 @@ describe('OAuthUtils', () => {
|
|||||||
OAuthUtils.discoverOAuthConfig('https://example.com/mcp'),
|
OAuthUtils.discoverOAuthConfig('https://example.com/mcp'),
|
||||||
).rejects.toThrow(/does not match expected/);
|
).rejects.toThrow(/does not match expected/);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should accept equivalent root resources with and without trailing slash', async () => {
|
||||||
|
mockFetch
|
||||||
|
// fetchProtectedResourceMetadata
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
ok: true,
|
||||||
|
json: () =>
|
||||||
|
Promise.resolve({
|
||||||
|
resource: 'https://example.com',
|
||||||
|
authorization_servers: ['https://auth.example.com'],
|
||||||
|
bearer_methods_supported: ['header'],
|
||||||
|
}),
|
||||||
|
})
|
||||||
|
// discoverAuthorizationServerMetadata
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
ok: true,
|
||||||
|
json: () => Promise.resolve(mockAuthServerMetadata),
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
OAuthUtils.discoverOAuthConfig('https://example.com'),
|
||||||
|
).resolves.toEqual({
|
||||||
|
authorizationUrl: 'https://auth.example.com/authorize',
|
||||||
|
issuer: 'https://auth.example.com',
|
||||||
|
tokenUrl: 'https://auth.example.com/token',
|
||||||
|
scopes: ['read', 'write'],
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('metadataToOAuthConfig', () => {
|
describe('metadataToOAuthConfig', () => {
|
||||||
@@ -336,6 +364,45 @@ describe('OAuthUtils', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('discoverOAuthFromWWWAuthenticate', () => {
|
||||||
|
const mockAuthServerMetadata: OAuthAuthorizationServerMetadata = {
|
||||||
|
issuer: 'https://auth.example.com',
|
||||||
|
authorization_endpoint: 'https://auth.example.com/authorize',
|
||||||
|
token_endpoint: 'https://auth.example.com/token',
|
||||||
|
scopes_supported: ['read', 'write'],
|
||||||
|
};
|
||||||
|
|
||||||
|
it('should accept equivalent root resources with and without trailing slash', async () => {
|
||||||
|
mockFetch
|
||||||
|
// fetchProtectedResourceMetadata(resource_metadata URL)
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
ok: true,
|
||||||
|
json: () =>
|
||||||
|
Promise.resolve({
|
||||||
|
resource: 'https://example.com',
|
||||||
|
authorization_servers: ['https://auth.example.com'],
|
||||||
|
}),
|
||||||
|
})
|
||||||
|
// discoverAuthorizationServerMetadata(auth server well-known URL)
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
ok: true,
|
||||||
|
json: () => Promise.resolve(mockAuthServerMetadata),
|
||||||
|
});
|
||||||
|
|
||||||
|
const result = await OAuthUtils.discoverOAuthFromWWWAuthenticate(
|
||||||
|
'Bearer realm="example", resource_metadata="https://example.com/.well-known/oauth-protected-resource"',
|
||||||
|
'https://example.com/',
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result).toEqual({
|
||||||
|
authorizationUrl: 'https://auth.example.com/authorize',
|
||||||
|
issuer: 'https://auth.example.com',
|
||||||
|
tokenUrl: 'https://auth.example.com/token',
|
||||||
|
scopes: ['read', 'write'],
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe('extractBaseUrl', () => {
|
describe('extractBaseUrl', () => {
|
||||||
it('should extract base URL from MCP server URL', () => {
|
it('should extract base URL from MCP server URL', () => {
|
||||||
const result = OAuthUtils.extractBaseUrl('https://example.com/mcp/v1');
|
const result = OAuthUtils.extractBaseUrl('https://example.com/mcp/v1');
|
||||||
|
|||||||
@@ -257,7 +257,12 @@ export class OAuthUtils {
|
|||||||
// it is using as the prefix for the metadata request exactly matches the value
|
// it is using as the prefix for the metadata request exactly matches the value
|
||||||
// of the resource metadata parameter in the protected resource metadata document.
|
// of the resource metadata parameter in the protected resource metadata document.
|
||||||
const expectedResource = this.buildResourceParameter(serverUrl);
|
const expectedResource = this.buildResourceParameter(serverUrl);
|
||||||
if (resourceMetadata.resource !== expectedResource) {
|
if (
|
||||||
|
!this.isEquivalentResourceIdentifier(
|
||||||
|
resourceMetadata.resource,
|
||||||
|
expectedResource,
|
||||||
|
)
|
||||||
|
) {
|
||||||
throw new ResourceMismatchError(
|
throw new ResourceMismatchError(
|
||||||
`Protected resource ${resourceMetadata.resource} does not match expected ${expectedResource}`,
|
`Protected resource ${resourceMetadata.resource} does not match expected ${expectedResource}`,
|
||||||
);
|
);
|
||||||
@@ -348,7 +353,12 @@ export class OAuthUtils {
|
|||||||
if (resourceMetadata && mcpServerUrl) {
|
if (resourceMetadata && mcpServerUrl) {
|
||||||
// Validate resource parameter per RFC 9728 Section 7.3
|
// Validate resource parameter per RFC 9728 Section 7.3
|
||||||
const expectedResource = this.buildResourceParameter(mcpServerUrl);
|
const expectedResource = this.buildResourceParameter(mcpServerUrl);
|
||||||
if (resourceMetadata.resource !== expectedResource) {
|
if (
|
||||||
|
!this.isEquivalentResourceIdentifier(
|
||||||
|
resourceMetadata.resource,
|
||||||
|
expectedResource,
|
||||||
|
)
|
||||||
|
) {
|
||||||
throw new ResourceMismatchError(
|
throw new ResourceMismatchError(
|
||||||
`Protected resource ${resourceMetadata.resource} does not match expected ${expectedResource}`,
|
`Protected resource ${resourceMetadata.resource} does not match expected ${expectedResource}`,
|
||||||
);
|
);
|
||||||
@@ -402,6 +412,21 @@ export class OAuthUtils {
|
|||||||
return `${url.protocol}//${url.host}${url.pathname}`;
|
return `${url.protocol}//${url.host}${url.pathname}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static isEquivalentResourceIdentifier(
|
||||||
|
discoveredResource: string,
|
||||||
|
expectedResource: string,
|
||||||
|
): boolean {
|
||||||
|
const normalize = (resource: string): string => {
|
||||||
|
try {
|
||||||
|
return this.buildResourceParameter(resource);
|
||||||
|
} catch {
|
||||||
|
return resource;
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
return normalize(discoveredResource) === normalize(expectedResource);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Parses a JWT string to extract its expiry time.
|
* Parses a JWT string to extract its expiry time.
|
||||||
* @param idToken The JWT ID token.
|
* @param idToken The JWT ID token.
|
||||||
|
|||||||
Reference in New Issue
Block a user