diff --git a/src/components/oauth/ConsentPage.tsx b/src/components/oauth/ConsentPage.tsx index 817be11..f41a7e8 100644 --- a/src/components/oauth/ConsentPage.tsx +++ b/src/components/oauth/ConsentPage.tsx @@ -11,6 +11,7 @@ import { authClient } from '@/lib/auth-client'; import { apiRequest, showErrorToast } from '@/lib/utils'; import { toast, Toaster } from 'sonner'; import { Shield, User, Mail, ChevronRight, AlertTriangle, Loader2 } from 'lucide-react'; +import { isValidRedirectUri, parseRedirectUris } from '@/lib/utils/oauth-validation'; interface OAuthApplication { id: string; @@ -44,6 +45,7 @@ export default function ConsentPage() { const params = new URLSearchParams(window.location.search); const clientId = params.get('client_id'); const scope = params.get('scope'); + const redirectUri = params.get('redirect_uri'); if (!clientId) { setError('Invalid authorization request: missing client ID'); @@ -59,6 +61,16 @@ export default function ConsentPage() { return; } + // Validate redirect URI if provided + if (redirectUri) { + const authorizedUris = parseRedirectUris(app.redirectURLs); + + if (!isValidRedirectUri(redirectUri, authorizedUris)) { + setError('Invalid authorization request: unauthorized redirect URI'); + return; + } + } + setApplication(app); // Parse requested scopes @@ -91,8 +103,27 @@ export default function ConsentPage() { // If denied, redirect back to the application with error const params = new URLSearchParams(window.location.search); const redirectUri = params.get('redirect_uri'); - if (redirectUri) { - window.location.href = `${redirectUri}?error=access_denied`; + + if (redirectUri && application) { + // Validate redirect URI against authorized URIs + const authorizedUris = parseRedirectUris(application.redirectURLs); + + if (isValidRedirectUri(redirectUri, authorizedUris)) { + try { + // Parse and reconstruct the URL to ensure it's safe + const url = new URL(redirectUri); + url.searchParams.set('error', 'access_denied'); + + // Safe to redirect - URI has been validated and sanitized + window.location.href = url.toString(); + } catch (e) { + console.error('Failed to parse redirect URI:', e); + setError('Invalid redirect URI'); + } + } else { + console.error('Unauthorized redirect URI:', redirectUri); + setError('Invalid redirect URI'); + } } } } catch (error) { diff --git a/src/lib/utils/oauth-validation.test.ts b/src/lib/utils/oauth-validation.test.ts new file mode 100644 index 0000000..1580ef7 --- /dev/null +++ b/src/lib/utils/oauth-validation.test.ts @@ -0,0 +1,85 @@ +import { describe, test, expect } from "bun:test"; +import { isValidRedirectUri, parseRedirectUris } from "./oauth-validation"; + +describe("OAuth Validation", () => { + describe("parseRedirectUris", () => { + test("parses comma-separated URIs", () => { + const result = parseRedirectUris("https://app1.com,https://app2.com, https://app3.com "); + expect(result).toEqual([ + "https://app1.com", + "https://app2.com", + "https://app3.com" + ]); + }); + + test("handles empty string", () => { + expect(parseRedirectUris("")).toEqual([]); + }); + + test("filters out empty values", () => { + const result = parseRedirectUris("https://app1.com,,https://app2.com,"); + expect(result).toEqual(["https://app1.com", "https://app2.com"]); + }); + }); + + describe("isValidRedirectUri", () => { + test("validates exact match", () => { + const authorizedUris = ["https://app.example.com/callback"]; + + expect(isValidRedirectUri("https://app.example.com/callback", authorizedUris)).toBe(true); + expect(isValidRedirectUri("https://app.example.com/other", authorizedUris)).toBe(false); + }); + + test("validates wildcard paths", () => { + const authorizedUris = ["https://app.example.com/*"]; + + expect(isValidRedirectUri("https://app.example.com/", authorizedUris)).toBe(true); + expect(isValidRedirectUri("https://app.example.com/callback", authorizedUris)).toBe(true); + expect(isValidRedirectUri("https://app.example.com/deep/path", authorizedUris)).toBe(true); + + // Different domain should fail + expect(isValidRedirectUri("https://evil.com/callback", authorizedUris)).toBe(false); + }); + + test("validates protocol", () => { + const authorizedUris = ["https://app.example.com/callback"]; + + // HTTP instead of HTTPS should fail + expect(isValidRedirectUri("http://app.example.com/callback", authorizedUris)).toBe(false); + }); + + test("validates host and port", () => { + const authorizedUris = ["https://app.example.com:3000/callback"]; + + // Different port should fail + expect(isValidRedirectUri("https://app.example.com/callback", authorizedUris)).toBe(false); + expect(isValidRedirectUri("https://app.example.com:3000/callback", authorizedUris)).toBe(true); + expect(isValidRedirectUri("https://app.example.com:4000/callback", authorizedUris)).toBe(false); + }); + + test("handles invalid URIs", () => { + const authorizedUris = ["not-a-valid-uri", "https://valid.com"]; + + // Invalid redirect URI + expect(isValidRedirectUri("not-a-valid-uri", authorizedUris)).toBe(false); + + // Valid redirect URI with invalid authorized URI should still work if it matches valid one + expect(isValidRedirectUri("https://valid.com", authorizedUris)).toBe(true); + }); + + test("handles empty inputs", () => { + expect(isValidRedirectUri("", ["https://app.com"])).toBe(false); + expect(isValidRedirectUri("https://app.com", [])).toBe(false); + }); + + test("prevents open redirect attacks", () => { + const authorizedUris = ["https://app.example.com/callback"]; + + // Various attack vectors + expect(isValidRedirectUri("https://app.example.com.evil.com/callback", authorizedUris)).toBe(false); + expect(isValidRedirectUri("https://app.example.com@evil.com/callback", authorizedUris)).toBe(false); + expect(isValidRedirectUri("//evil.com/callback", authorizedUris)).toBe(false); + expect(isValidRedirectUri("https:evil.com/callback", authorizedUris)).toBe(false); + }); + }); +}); \ No newline at end of file diff --git a/src/lib/utils/oauth-validation.ts b/src/lib/utils/oauth-validation.ts new file mode 100644 index 0000000..3a64b55 --- /dev/null +++ b/src/lib/utils/oauth-validation.ts @@ -0,0 +1,59 @@ +/** + * Validates a redirect URI against a list of authorized URIs + * @param redirectUri The redirect URI to validate + * @param authorizedUris List of authorized redirect URIs + * @returns true if the redirect URI is authorized, false otherwise + */ +export function isValidRedirectUri(redirectUri: string, authorizedUris: string[]): boolean { + if (!redirectUri || authorizedUris.length === 0) { + return false; + } + + try { + // Parse the redirect URI to ensure it's valid + const redirectUrl = new URL(redirectUri); + + return authorizedUris.some(authorizedUri => { + try { + // Handle wildcard paths (e.g., https://example.com/*) + if (authorizedUri.endsWith('/*')) { + const baseUri = authorizedUri.slice(0, -2); + const baseUrl = new URL(baseUri); + + // Check protocol, host, and port match + return redirectUrl.protocol === baseUrl.protocol && + redirectUrl.host === baseUrl.host && + redirectUrl.pathname.startsWith(baseUrl.pathname); + } + + // Handle exact match + const authorizedUrl = new URL(authorizedUri); + + // For exact match, everything must match including path and query params + return redirectUrl.href === authorizedUrl.href; + } catch { + // If authorized URI is not a valid URL, treat as invalid + return false; + } + }); + } catch { + // If redirect URI is not a valid URL, it's invalid + return false; + } +} + +/** + * Parses a comma-separated list of redirect URIs and trims whitespace + * @param redirectUrls Comma-separated list of redirect URIs + * @returns Array of trimmed redirect URIs + */ +export function parseRedirectUris(redirectUrls: string): string[] { + if (!redirectUrls) { + return []; + } + + return redirectUrls + .split(',') + .map(uri => uri.trim()) + .filter(uri => uri.length > 0); +} \ No newline at end of file diff --git a/src/pages/api/auth/debug.ts b/src/pages/api/auth/debug.ts index 5dfdbc6..3267bf2 100644 --- a/src/pages/api/auth/debug.ts +++ b/src/pages/api/auth/debug.ts @@ -27,9 +27,13 @@ export const GET: APIRoute = async ({ request }) => { headers: { "Content-Type": "application/json" }, }); } catch (error) { + // Log full error details server-side for debugging + console.error("Debug endpoint error:", error); + + // Only return safe error information to the client return new Response(JSON.stringify({ success: false, - error: error instanceof Error ? error.message : "Unknown error", + error: error instanceof Error ? error.message : "An unexpected error occurred" }), { status: 500, headers: { "Content-Type": "application/json" }, @@ -60,10 +64,13 @@ export const POST: APIRoute = async ({ request }) => { headers: { "Content-Type": "application/json" }, }); } catch (error) { + // Log full error details server-side for debugging + console.error("Debug endpoint error:", error); + + // Only return safe error information to the client return new Response(JSON.stringify({ success: false, - error: error instanceof Error ? error.message : "Unknown error", - details: error + error: error instanceof Error ? error.message : "An unexpected error occurred" }), { status: 500, headers: { "Content-Type": "application/json" },