mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-10 22:21:22 -07:00
Validate OAuth resource parameter matches MCP server URL (#15289)
This commit is contained in:
@@ -255,6 +255,7 @@ describe('MCPOAuthProvider', () => {
|
||||
delete configWithoutAuth.tokenUrl;
|
||||
|
||||
const mockResourceMetadata = {
|
||||
resource: 'https://api.example.com/',
|
||||
authorization_servers: ['https://discovered.auth.com'],
|
||||
};
|
||||
|
||||
@@ -513,7 +514,7 @@ describe('MCPOAuthProvider', () => {
|
||||
delete configWithoutClientAndAuthorizationUrl.authorizationUrl;
|
||||
|
||||
const mockResourceMetadata: OAuthProtectedResourceMetadata = {
|
||||
resource: 'https://api.example.com',
|
||||
resource: 'https://api.example.com/',
|
||||
authorization_servers: ['https://auth.example.com'],
|
||||
};
|
||||
|
||||
@@ -1474,6 +1475,7 @@ describe('MCPOAuthProvider', () => {
|
||||
delete configWithUserScopes.tokenUrl;
|
||||
|
||||
const mockResourceMetadata = {
|
||||
resource: 'https://api.example.com/',
|
||||
authorization_servers: ['https://discovered.auth.com'],
|
||||
};
|
||||
|
||||
@@ -1562,6 +1564,7 @@ describe('MCPOAuthProvider', () => {
|
||||
delete configWithoutScopes.tokenUrl;
|
||||
|
||||
const mockResourceMetadata = {
|
||||
resource: 'https://api.example.com/',
|
||||
authorization_servers: ['https://discovered.auth.com'],
|
||||
};
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ import { openBrowserSecurely } from '../utils/secure-browser-launcher.js';
|
||||
import type { OAuthToken } from './token-storage/types.js';
|
||||
import { MCPOAuthTokenStorage } from './oauth-token-storage.js';
|
||||
import { getErrorMessage } from '../utils/errors.js';
|
||||
import { OAuthUtils } from './oauth-utils.js';
|
||||
import { OAuthUtils, ResourceMismatchError } from './oauth-utils.js';
|
||||
import { coreEvents } from '../utils/events.js';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
|
||||
@@ -744,6 +744,7 @@ export class MCPOAuthProvider {
|
||||
const discoveredConfig =
|
||||
await OAuthUtils.discoverOAuthFromWWWAuthenticate(
|
||||
wwwAuthenticate,
|
||||
mcpServerUrl,
|
||||
);
|
||||
if (discoveredConfig) {
|
||||
// Merge discovered config with existing config, preserving clientId and clientSecret
|
||||
@@ -760,6 +761,11 @@ export class MCPOAuthProvider {
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
// Re-throw security validation errors
|
||||
if (error instanceof ResourceMismatchError) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
debugLogger.debug(
|
||||
`Failed to check endpoint for authentication requirements: ${getErrorMessage(error)}`,
|
||||
);
|
||||
|
||||
@@ -210,6 +210,60 @@ describe('OAuthUtils', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('discoverOAuthConfig', () => {
|
||||
const mockResourceMetadata: OAuthProtectedResourceMetadata = {
|
||||
resource: 'https://example.com/mcp',
|
||||
authorization_servers: ['https://auth.example.com'],
|
||||
bearer_methods_supported: ['header'],
|
||||
};
|
||||
|
||||
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 succeed when resource metadata matches server URL', async () => {
|
||||
mockFetch
|
||||
// fetchProtectedResourceMetadata
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve(mockResourceMetadata),
|
||||
})
|
||||
// discoverAuthorizationServerMetadata
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve(mockAuthServerMetadata),
|
||||
});
|
||||
|
||||
const config = await OAuthUtils.discoverOAuthConfig(
|
||||
'https://example.com/mcp',
|
||||
);
|
||||
|
||||
expect(config).toEqual({
|
||||
authorizationUrl: 'https://auth.example.com/authorize',
|
||||
tokenUrl: 'https://auth.example.com/token',
|
||||
scopes: ['read', 'write'],
|
||||
});
|
||||
});
|
||||
|
||||
it('should throw error when resource metadata does not match server URL', async () => {
|
||||
mockFetch.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () =>
|
||||
Promise.resolve({
|
||||
...mockResourceMetadata,
|
||||
resource: 'https://malicious.com/mcp',
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(
|
||||
OAuthUtils.discoverOAuthConfig('https://example.com/mcp'),
|
||||
).rejects.toThrow(/does not match expected/);
|
||||
});
|
||||
});
|
||||
|
||||
describe('metadataToOAuthConfig', () => {
|
||||
it('should convert metadata to OAuth config', () => {
|
||||
const metadata: OAuthAuthorizationServerMetadata = {
|
||||
|
||||
@@ -8,6 +8,16 @@ import type { MCPOAuthConfig } from './oauth-provider.js';
|
||||
import { getErrorMessage } from '../utils/errors.js';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
|
||||
/**
|
||||
* Error thrown when the discovered resource metadata does not match the expected resource.
|
||||
*/
|
||||
export class ResourceMismatchError extends Error {
|
||||
constructor(message: string) {
|
||||
super(message);
|
||||
this.name = 'ResourceMismatchError';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* OAuth authorization server metadata as per RFC 8414.
|
||||
*/
|
||||
@@ -243,6 +253,18 @@ export class OAuthUtils {
|
||||
}
|
||||
}
|
||||
|
||||
if (resourceMetadata) {
|
||||
// RFC 9728 Section 7.3: The client MUST ensure that the resource identifier URL
|
||||
// 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.
|
||||
const expectedResource = this.buildResourceParameter(serverUrl);
|
||||
if (resourceMetadata.resource !== expectedResource) {
|
||||
throw new ResourceMismatchError(
|
||||
`Protected resource ${resourceMetadata.resource} does not match expected ${expectedResource}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (resourceMetadata?.authorization_servers?.length) {
|
||||
// Use the first authorization server
|
||||
const authServerUrl = resourceMetadata.authorization_servers[0];
|
||||
@@ -279,6 +301,9 @@ export class OAuthUtils {
|
||||
|
||||
return null;
|
||||
} catch (error) {
|
||||
if (error instanceof ResourceMismatchError) {
|
||||
throw error;
|
||||
}
|
||||
debugLogger.debug(
|
||||
`Failed to discover OAuth configuration: ${getErrorMessage(error)}`,
|
||||
);
|
||||
@@ -305,10 +330,12 @@ export class OAuthUtils {
|
||||
* Discover OAuth configuration from WWW-Authenticate header.
|
||||
*
|
||||
* @param wwwAuthenticate The WWW-Authenticate header value
|
||||
* @param mcpServerUrl Optional MCP server URL to validate against the resource metadata
|
||||
* @returns The discovered OAuth configuration or null if not available
|
||||
*/
|
||||
static async discoverOAuthFromWWWAuthenticate(
|
||||
wwwAuthenticate: string,
|
||||
mcpServerUrl?: string,
|
||||
): Promise<MCPOAuthConfig | null> {
|
||||
const resourceMetadataUri =
|
||||
this.parseWWWAuthenticateHeader(wwwAuthenticate);
|
||||
@@ -318,6 +345,17 @@ export class OAuthUtils {
|
||||
|
||||
const resourceMetadata =
|
||||
await this.fetchProtectedResourceMetadata(resourceMetadataUri);
|
||||
|
||||
if (resourceMetadata && mcpServerUrl) {
|
||||
// Validate resource parameter per RFC 9728 Section 7.3
|
||||
const expectedResource = this.buildResourceParameter(mcpServerUrl);
|
||||
if (resourceMetadata.resource !== expectedResource) {
|
||||
throw new ResourceMismatchError(
|
||||
`Protected resource ${resourceMetadata.resource} does not match expected ${expectedResource}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (!resourceMetadata?.authorization_servers?.length) {
|
||||
return null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user