diff --git a/app/api/auth/sso/start/route.ts b/app/api/auth/sso/start/route.ts index 85fce655..8c3cdbde 100644 --- a/app/api/auth/sso/start/route.ts +++ b/app/api/auth/sso/start/route.ts @@ -5,6 +5,7 @@ import { encryptPayload } from '@/lib/auth/crypto'; import { generateCodeVerifierServer, generateCodeChallengeServer, generateStateServer } from '@/lib/oauth/pkce-server'; import { getRequiredConfig } from '@/lib/oauth/token-exchange'; import { discoverOAuth } from '@/lib/oauth/discovery'; +import { isPublicHttpUrl } from '@/lib/security/url-guard'; import { getOauthScopes } from '@/lib/oauth/tokens'; import { getCookieOptions } from '@/lib/oauth/cookie-config'; import { hasSessionSecret } from '@/lib/auth/session-secret'; @@ -39,7 +40,7 @@ export async function POST(request: NextRequest) { } const { clientId, discoveryUrl } = getRequiredConfig(serverId); - const metadata = await discoverOAuth(discoveryUrl); + const metadata = await discoverOAuth(discoveryUrl, { validateEndpoint: isPublicHttpUrl }); if (!metadata?.authorization_endpoint) { return NextResponse.json({ error: 'OAuth discovery failed' }, { status: 502 }); diff --git a/app/api/auth/totp-token-exchange/route.ts b/app/api/auth/totp-token-exchange/route.ts index 4c120c40..e19bc781 100644 --- a/app/api/auth/totp-token-exchange/route.ts +++ b/app/api/auth/totp-token-exchange/route.ts @@ -54,7 +54,7 @@ async function tryTokenRequest( async function findTokenEndpoint(serverUrl: string): Promise { // 1. Try OAuth discovery - const metadata = await discoverOAuth(serverUrl); + const metadata = await discoverOAuth(serverUrl, { validateEndpoint: isPublicHttpUrl }); if (metadata?.token_endpoint) return metadata.token_endpoint; // 2. Try common Stalwart token endpoint paths directly diff --git a/lib/__tests__/oauth-discovery.test.ts b/lib/__tests__/oauth-discovery.test.ts index 9e996f38..609a9294 100644 --- a/lib/__tests__/oauth-discovery.test.ts +++ b/lib/__tests__/oauth-discovery.test.ts @@ -1,22 +1,20 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { OAuthMetadata } from '../oauth/discovery'; -vi.mock('../security/url-guard', () => ({ - isPublicHttpUrl: vi.fn(async (urlString: string) => { - try { - const url = new URL(urlString); - if (url.protocol !== 'http:' && url.protocol !== 'https:') return false; - if (url.username || url.password) return false; - const host = url.hostname.toLowerCase(); - if (host === 'localhost' || host.endsWith('.local') || host.endsWith('.internal')) return false; - if (/^(127\.|169\.254\.|10\.|192\.168\.)/.test(host)) return false; - if (host === '::1' || host === '0.0.0.0') return false; - return true; - } catch { - return false; - } - }), -})); +const validateEndpoint = async (urlString: string) => { + try { + const url = new URL(urlString); + if (url.protocol !== 'http:' && url.protocol !== 'https:') return false; + if (url.username || url.password) return false; + const host = url.hostname.toLowerCase(); + if (host === 'localhost' || host.endsWith('.local') || host.endsWith('.internal')) return false; + if (/^(127\.|169\.254\.|10\.|192\.168\.)/.test(host)) return false; + if (host === '::1' || host === '0.0.0.0') return false; + return true; + } catch { + return false; + } +}; const VALID_METADATA: OAuthMetadata = { issuer: 'https://auth.example.com', @@ -43,7 +41,7 @@ describe('oauth/discovery', () => { json: () => Promise.resolve(VALID_METADATA), })); - const result = await discoverOAuth('https://mail.example.com'); + const result = await discoverOAuth('https://mail.example.com', { validateEndpoint }); expect(result).toEqual(VALID_METADATA); expect(fetch).toHaveBeenCalledTimes(1); @@ -60,7 +58,7 @@ describe('oauth/discovery', () => { json: () => Promise.resolve(VALID_METADATA), })); - const result = await discoverOAuth('https://fallback.example.com'); + const result = await discoverOAuth('https://fallback.example.com', { validateEndpoint }); expect(result).toEqual(VALID_METADATA); expect(fetch).toHaveBeenCalledTimes(2); @@ -76,7 +74,7 @@ describe('oauth/discovery', () => { .mockResolvedValueOnce({ ok: false, status: 404 }) .mockResolvedValueOnce({ ok: false, status: 404 })); - const result = await discoverOAuth('https://fail.example.com'); + const result = await discoverOAuth('https://fail.example.com', { validateEndpoint }); expect(result).toBeNull(); expect(consoleSpy).toHaveBeenCalled(); @@ -88,7 +86,7 @@ describe('oauth/discovery', () => { json: () => Promise.resolve(VALID_METADATA), })); - const result = await discoverOAuth('https://optional.example.com'); + const result = await discoverOAuth('https://optional.example.com', { validateEndpoint }); expect(result?.revocation_endpoint).toBe('https://auth.example.com/revoke'); expect(result?.end_session_endpoint).toBe('https://auth.example.com/logout'); @@ -103,7 +101,7 @@ describe('oauth/discovery', () => { }) .mockResolvedValueOnce({ ok: false, status: 404 })); - const result = await discoverOAuth('https://incomplete.example.com'); + const result = await discoverOAuth('https://incomplete.example.com', { validateEndpoint }); expect(result).toBeNull(); expect(consoleSpy).toHaveBeenCalled(); @@ -122,7 +120,7 @@ describe('oauth/discovery', () => { }) .mockResolvedValueOnce({ ok: false, status: 404 })); - const result = await discoverOAuth('https://evil.example.com'); + const result = await discoverOAuth('https://evil.example.com', { validateEndpoint }); expect(result).toBeNull(); expect(consoleSpy).toHaveBeenCalled(); @@ -142,7 +140,7 @@ describe('oauth/discovery', () => { }) .mockResolvedValueOnce({ ok: false, status: 404 })); - const result = await discoverOAuth('https://private-revoke.example.com'); + const result = await discoverOAuth('https://private-revoke.example.com', { validateEndpoint }); expect(result).toBeNull(); expect(consoleSpy).toHaveBeenCalled(); @@ -154,8 +152,8 @@ describe('oauth/discovery', () => { json: () => Promise.resolve(VALID_METADATA), })); - const first = await discoverOAuth('https://cached.example.com'); - const second = await discoverOAuth('https://cached.example.com'); + const first = await discoverOAuth('https://cached.example.com', { validateEndpoint }); + const second = await discoverOAuth('https://cached.example.com', { validateEndpoint }); expect(first).toEqual(VALID_METADATA); expect(second).toEqual(VALID_METADATA); diff --git a/lib/oauth/discovery.ts b/lib/oauth/discovery.ts index 84178d20..8a84ac68 100644 --- a/lib/oauth/discovery.ts +++ b/lib/oauth/discovery.ts @@ -1,5 +1,3 @@ -import { isPublicHttpUrl } from '../security/url-guard'; - export interface OAuthMetadata { issuer: string; authorization_endpoint: string; @@ -8,6 +6,17 @@ export interface OAuthMetadata { end_session_endpoint?: string; } +// Validates that a discovered endpoint URL is safe to follow. Server-side +// callers must pass this to gate against SSRF (typically isPublicHttpUrl from +// @/lib/security/url-guard, which uses node:dns and cannot be bundled for the +// browser). Client callers omit it: the browser handles outbound networking +// and an SSRF check isn't meaningful there. +export type EndpointValidator = (url: string) => Promise; + +export interface DiscoverOAuthOptions { + validateEndpoint?: EndpointValidator; +} + const CACHE_TTL_MS = 10 * 60 * 1000; const CACHE_MAX_ENTRIES = 64; const metadataCache = new Map(); @@ -26,19 +35,27 @@ function rememberMetadata(serverUrl: string, metadata: OAuthMetadata): void { // Endpoints come from an attacker-controllable JSON document when callers pass // a user-supplied serverUrl (e.g. /api/auth/totp-token-exchange under -// allowCustomJmapEndpoint). Without this gate, a malicious metadata document +// allowCustomJmapEndpoint). Without a validator, a malicious metadata document // could point token_endpoint at 169.254.169.254 or 127.0.0.1:* and turn the -// downstream fetch() into an SSRF with response-body reflection. -async function endpointsArePublic(endpoints: Array): Promise { +// downstream fetch() into an SSRF with response-body reflection. Server-side +// callers must pass `validateEndpoint`. +async function endpointsArePublic( + endpoints: Array, + validate: EndpointValidator | undefined, +): Promise { + if (!validate) return true; for (const endpoint of endpoints) { if (endpoint === undefined) continue; if (typeof endpoint !== 'string') return false; - if (!(await isPublicHttpUrl(endpoint))) return false; + if (!(await validate(endpoint))) return false; } return true; } -export async function discoverOAuth(serverUrl: string): Promise { +export async function discoverOAuth( + serverUrl: string, + options?: DiscoverOAuthOptions, +): Promise { const cached = metadataCache.get(serverUrl); if (cached && cached.expiresAt > Date.now()) return cached.metadata; if (cached) metadataCache.delete(serverUrl); @@ -65,7 +82,7 @@ export async function discoverOAuth(serverUrl: string): Promise { const { discoveryUrl } = getRequiredConfig(serverId); - const metadata = await discoverOAuth(discoveryUrl); + const metadata = await discoverOAuth(discoveryUrl, { validateEndpoint: isPublicHttpUrl }); if (!metadata?.token_endpoint) { throw new Error('OAuth token endpoint not found'); } @@ -55,7 +56,7 @@ export async function getTokenEndpoint(serverId?: string | null): Promise { const { discoveryUrl } = getRequiredConfig(serverId); - return discoverOAuth(discoveryUrl); + return discoverOAuth(discoveryUrl, { validateEndpoint: isPublicHttpUrl }); } export function buildOAuthParams(base: Record, serverId?: string | null): URLSearchParams {