fix: add validation for OAuth redirect URLs
- Create shared validateRedirectUrl utility to centralize URL validation - Only allow HTTP and HTTPS protocols in OAuth authorization URLs - Fix three vulnerable redirect paths that bypassed validation: - AuthDebugger.tsx: Direct window.location.href redirect - OAuthFlowProgress.tsx: window.open() without validation - OAuthFlowProgress.tsx: Direct anchor href attribute - Add comprehensive test coverage for URL validation - Update existing auth.ts to use shared validation utility This prevents malicious MCP servers from injecting javascript: or data: protocol URLs that would execute XSS attacks when users are redirected to the authorization endpoint. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
7fc3de7648
commit
650f3090d2
5 changed files with 197 additions and 12 deletions
|
|
@ -6,6 +6,7 @@ import { AuthDebuggerState, EMPTY_DEBUGGER_STATE } from "../lib/auth-types";
|
|||
import { OAuthFlowProgress } from "./OAuthFlowProgress";
|
||||
import { OAuthStateMachine } from "../lib/oauth-state-machine";
|
||||
import { SESSION_KEYS } from "../lib/constants";
|
||||
import { validateRedirectUrl } from "@/utils/urlValidation";
|
||||
|
||||
export interface AuthDebuggerProps {
|
||||
serverUrl: string;
|
||||
|
|
@ -163,6 +164,22 @@ const AuthDebugger = ({
|
|||
currentState.oauthStep === "authorization_code" &&
|
||||
currentState.authorizationUrl
|
||||
) {
|
||||
// Validate the URL before redirecting
|
||||
try {
|
||||
validateRedirectUrl(currentState.authorizationUrl);
|
||||
} catch (error) {
|
||||
updateAuthState({
|
||||
...currentState,
|
||||
isInitiatingAuth: false,
|
||||
latestError: error instanceof Error ? error : new Error(String(error)),
|
||||
statusMessage: {
|
||||
type: "error",
|
||||
message: `Invalid authorization URL: ${error instanceof Error ? error.message : String(error)}`,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
// Store the current auth state before redirecting
|
||||
sessionStorage.setItem(
|
||||
SESSION_KEYS.AUTH_DEBUGGER_STATE,
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ import { Button } from "./ui/button";
|
|||
import { DebugInspectorOAuthClientProvider } from "@/lib/auth";
|
||||
import { useEffect, useMemo, useState } from "react";
|
||||
import { OAuthClientInformation } from "@modelcontextprotocol/sdk/shared/auth.js";
|
||||
import { validateRedirectUrl } from "@/utils/urlValidation";
|
||||
import { useToast } from "@/lib/hooks/useToast";
|
||||
|
||||
interface OAuthStepProps {
|
||||
label: string;
|
||||
|
|
@ -71,6 +73,7 @@ export const OAuthFlowProgress = ({
|
|||
updateAuthState,
|
||||
proceedToNextStep,
|
||||
}: OAuthFlowProgressProps) => {
|
||||
const { toast } = useToast();
|
||||
const provider = useMemo(
|
||||
() => new DebugInspectorOAuthClientProvider(serverUrl),
|
||||
[serverUrl],
|
||||
|
|
@ -239,16 +242,25 @@ export const OAuthFlowProgress = ({
|
|||
<p className="text-xs break-all">
|
||||
{authState.authorizationUrl}
|
||||
</p>
|
||||
<a
|
||||
href={authState.authorizationUrl}
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
<button
|
||||
onClick={() => {
|
||||
try {
|
||||
validateRedirectUrl(authState.authorizationUrl!);
|
||||
window.open(authState.authorizationUrl!, "_blank");
|
||||
} catch (error) {
|
||||
toast({
|
||||
title: "Invalid URL",
|
||||
description: error instanceof Error ? error.message : "The authorization URL is not valid",
|
||||
variant: "destructive",
|
||||
});
|
||||
}
|
||||
}}
|
||||
className="flex items-center text-blue-500 hover:text-blue-700"
|
||||
aria-label="Open authorization URL in new tab"
|
||||
title="Open authorization URL"
|
||||
>
|
||||
<ExternalLink className="h-4 w-4" />
|
||||
</a>
|
||||
</button>
|
||||
</div>
|
||||
<p className="text-xs text-muted-foreground mt-2">
|
||||
Click the link to authorize in your browser. After
|
||||
|
|
@ -354,7 +366,18 @@ export const OAuthFlowProgress = ({
|
|||
authState.authorizationUrl && (
|
||||
<Button
|
||||
variant="outline"
|
||||
onClick={() => window.open(authState.authorizationUrl!, "_blank")}
|
||||
onClick={() => {
|
||||
try {
|
||||
validateRedirectUrl(authState.authorizationUrl!);
|
||||
window.open(authState.authorizationUrl!, "_blank");
|
||||
} catch (error) {
|
||||
toast({
|
||||
title: "Invalid URL",
|
||||
description: error instanceof Error ? error.message : "The authorization URL is not valid",
|
||||
variant: "destructive",
|
||||
});
|
||||
}
|
||||
}}
|
||||
>
|
||||
Open in New Tab
|
||||
</Button>
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import {
|
|||
import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/sdk/client/auth.js";
|
||||
import { SESSION_KEYS, getServerSpecificKey } from "./constants";
|
||||
import { generateOAuthState } from "@/utils/oauthUtils";
|
||||
import { validateRedirectUrl } from "@/utils/urlValidation";
|
||||
|
||||
/**
|
||||
* Discovers OAuth scopes from server metadata, with preference for resource metadata scopes
|
||||
|
|
@ -182,12 +183,8 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider {
|
|||
}
|
||||
|
||||
redirectToAuthorization(authorizationUrl: URL) {
|
||||
if (
|
||||
authorizationUrl.protocol !== "http:" &&
|
||||
authorizationUrl.protocol !== "https:"
|
||||
) {
|
||||
throw new Error("Authorization URL must be HTTP or HTTPS");
|
||||
}
|
||||
// Validate the URL using the shared utility
|
||||
validateRedirectUrl(authorizationUrl.href);
|
||||
window.location.href = authorizationUrl.href;
|
||||
}
|
||||
|
||||
|
|
|
|||
127
client/src/utils/__tests__/urlValidation.test.ts
Normal file
127
client/src/utils/__tests__/urlValidation.test.ts
Normal file
|
|
@ -0,0 +1,127 @@
|
|||
import { validateRedirectUrl } from "../urlValidation";
|
||||
|
||||
describe("validateRedirectUrl", () => {
|
||||
describe("valid URLs", () => {
|
||||
it("should allow HTTP URLs", () => {
|
||||
expect(() => validateRedirectUrl("http://example.com")).not.toThrow();
|
||||
});
|
||||
|
||||
it("should allow HTTPS URLs", () => {
|
||||
expect(() => validateRedirectUrl("https://example.com")).not.toThrow();
|
||||
});
|
||||
|
||||
it("should allow URLs with ports", () => {
|
||||
expect(() => validateRedirectUrl("https://example.com:8080")).not.toThrow();
|
||||
});
|
||||
|
||||
it("should allow URLs with paths", () => {
|
||||
expect(() => validateRedirectUrl("https://example.com/path/to/auth")).not.toThrow();
|
||||
});
|
||||
|
||||
it("should allow URLs with query parameters", () => {
|
||||
expect(() => validateRedirectUrl("https://example.com?param=value")).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("invalid URLs - XSS vectors", () => {
|
||||
it("should block javascript: protocol", () => {
|
||||
expect(() => validateRedirectUrl("javascript:alert('XSS')")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS"
|
||||
);
|
||||
});
|
||||
|
||||
it("should block javascript: with encoded characters", () => {
|
||||
expect(() => validateRedirectUrl("javascript:alert%28%27XSS%27%29")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS"
|
||||
);
|
||||
});
|
||||
|
||||
it("should block data: protocol", () => {
|
||||
expect(() => validateRedirectUrl("data:text/html,<script>alert('XSS')</script>")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS"
|
||||
);
|
||||
});
|
||||
|
||||
it("should block vbscript: protocol", () => {
|
||||
expect(() => validateRedirectUrl("vbscript:msgbox")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS"
|
||||
);
|
||||
});
|
||||
|
||||
it("should block file: protocol", () => {
|
||||
expect(() => validateRedirectUrl("file:///etc/passwd")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS"
|
||||
);
|
||||
});
|
||||
|
||||
it("should block about: protocol", () => {
|
||||
expect(() => validateRedirectUrl("about:blank")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS"
|
||||
);
|
||||
});
|
||||
|
||||
it("should block custom protocols", () => {
|
||||
expect(() => validateRedirectUrl("custom://example")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS"
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("edge cases", () => {
|
||||
it("should handle malformed URLs", () => {
|
||||
expect(() => validateRedirectUrl("not a url")).toThrow(
|
||||
"Invalid URL: not a url"
|
||||
);
|
||||
});
|
||||
|
||||
it("should handle empty string", () => {
|
||||
expect(() => validateRedirectUrl("")).toThrow(
|
||||
"Invalid URL: "
|
||||
);
|
||||
});
|
||||
|
||||
it("should handle URLs with unicode characters", () => {
|
||||
expect(() => validateRedirectUrl("https://例え.jp")).not.toThrow();
|
||||
});
|
||||
|
||||
it("should handle URLs with case variations", () => {
|
||||
expect(() => validateRedirectUrl("HTTPS://EXAMPLE.COM")).not.toThrow();
|
||||
expect(() => validateRedirectUrl("HtTpS://example.com")).not.toThrow();
|
||||
});
|
||||
|
||||
it("should handle protocol-relative URLs as invalid", () => {
|
||||
expect(() => validateRedirectUrl("//example.com")).toThrow(
|
||||
"Invalid URL: //example.com"
|
||||
);
|
||||
});
|
||||
|
||||
it("should handle URLs with authentication", () => {
|
||||
expect(() => validateRedirectUrl("https://user:pass@example.com")).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("security considerations", () => {
|
||||
it("should not be fooled by whitespace", () => {
|
||||
expect(() => validateRedirectUrl(" javascript:alert('XSS')")).toThrow();
|
||||
expect(() => validateRedirectUrl("javascript:alert('XSS') ")).toThrow();
|
||||
});
|
||||
|
||||
it("should handle null bytes", () => {
|
||||
expect(() => validateRedirectUrl("java\x00script:alert('XSS')")).toThrow();
|
||||
});
|
||||
|
||||
it("should handle tab characters", () => {
|
||||
expect(() => validateRedirectUrl("java\tscript:alert('XSS')")).toThrow();
|
||||
});
|
||||
|
||||
it("should handle newlines", () => {
|
||||
expect(() => validateRedirectUrl("java\nscript:alert('XSS')")).toThrow();
|
||||
});
|
||||
|
||||
it("should handle mixed case protocols", () => {
|
||||
expect(() => validateRedirectUrl("JaVaScRiPt:alert('XSS')")).toThrow(
|
||||
"Authorization URL must be HTTP or HTTPS"
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
21
client/src/utils/urlValidation.ts
Normal file
21
client/src/utils/urlValidation.ts
Normal file
|
|
@ -0,0 +1,21 @@
|
|||
/**
|
||||
* Validates that a URL is safe for redirection.
|
||||
* Only allows HTTP and HTTPS protocols to prevent XSS attacks.
|
||||
*
|
||||
* @param url - The URL string to validate
|
||||
* @throws Error if the URL has an unsafe protocol
|
||||
*/
|
||||
export function validateRedirectUrl(url: string): void {
|
||||
try {
|
||||
const parsedUrl = new URL(url);
|
||||
if (parsedUrl.protocol !== "http:" && parsedUrl.protocol !== "https:") {
|
||||
throw new Error("Authorization URL must be HTTP or HTTPS");
|
||||
}
|
||||
} catch (error) {
|
||||
if (error instanceof Error && error.message === "Authorization URL must be HTTP or HTTPS") {
|
||||
throw error;
|
||||
}
|
||||
// If URL parsing fails, it's also invalid
|
||||
throw new Error(`Invalid URL: ${url}`);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue