diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml new file mode 100644 index 00000000..8262218a --- /dev/null +++ b/.github/workflows/unit_tests.yml @@ -0,0 +1,24 @@ +name: unit tests + +on: + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: npm + + - name: Install dependencies + run: npm ci + + - name: Sync SvelteKit + run: npm run prepare --workspaces --if-present + + - name: Run unit tests + run: npm run test:unit --workspaces --if-present -- --run diff --git a/.gitignore b/.gitignore index 2fbe10d2..a75d4fb2 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,5 @@ build .claude security-scan.log security-fix.log +.idea +*.code-workspace \ No newline at end of file diff --git a/apps/api-manager/src/lib/components/Toast.svelte b/apps/api-manager/src/lib/components/Toast.svelte index b6888896..4025d52d 100644 --- a/apps/api-manager/src/lib/components/Toast.svelte +++ b/apps/api-manager/src/lib/components/Toast.svelte @@ -3,7 +3,7 @@ import { toaster } from '$lib/utils/toastService'; - + {#snippet children(toast)} diff --git a/apps/api-manager/src/lib/oauth/client.test.ts b/apps/api-manager/src/lib/oauth/client.test.ts new file mode 100644 index 00000000..d43d58e2 --- /dev/null +++ b/apps/api-manager/src/lib/oauth/client.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { OAuth2Client } from 'arctic'; +import { OAuth2ClientWithConfig } from './client'; + +// Build a JWT-shaped token (header.payload.signature) with a base64 payload. +function createMockJWT(payload: Record): string { + const header = btoa(JSON.stringify({ alg: 'RS256', typ: 'JWT' })); + const body = btoa(JSON.stringify(payload)); + return `${header}.${body}.mock-signature`; +} + +describe('OAuth2ClientWithConfig', () => { + let client: OAuth2ClientWithConfig; + + beforeEach(() => { + // arctic's OAuth2Client is globally mocked to return a plain object, which + // would hijack `this` in the subclass constructor and strip its prototype + // methods. Reset it to a no-op constructor so the real subclass instance is + // built. + vi.mocked(OAuth2Client).mockImplementation(function () {} as never); + client = new OAuth2ClientWithConfig('id', 'secret', 'http://localhost/callback'); + }); + + describe('constructor', () => { + it('creates an instance exposing the custom methods', () => { + expect(client.OIDCConfig).toBeUndefined(); + expect(typeof client.checkAccessTokenExpiration).toBe('function'); + }); + }); + + describe('checkAccessTokenExpiration (fail closed)', () => { + it('returns false for a token whose expiry is in the future', async () => { + const token = createMockJWT({ exp: Math.floor(Date.now() / 1000) + 3600 }); + await expect(client.checkAccessTokenExpiration(token)).resolves.toBe(false); + }); + + it('returns true for an expired token', async () => { + const token = createMockJWT({ exp: Math.floor(Date.now() / 1000) - 3600 }); + await expect(client.checkAccessTokenExpiration(token)).resolves.toBe(true); + }); + + it('treats a token without an exp claim as expired', async () => { + const token = createMockJWT({ sub: 'user-1' }); + await expect(client.checkAccessTokenExpiration(token)).resolves.toBe(true); + }); + + it('treats an undecodable token as expired instead of throwing', async () => { + await expect(client.checkAccessTokenExpiration('not.a.jwt')).resolves.toBe(true); + }); + + it('treats a malformed JWT payload as expired instead of throwing', async () => { + await expect( + client.checkAccessTokenExpiration('eyJhbGciOiJSUzI1NiJ9.invalid-payload.sig') + ).resolves.toBe(true); + }); + }); +}); diff --git a/apps/api-manager/src/lib/oauth/sessionHelper.ts b/apps/api-manager/src/lib/oauth/sessionHelper.ts index f3f57b79..63235d6b 100644 --- a/apps/api-manager/src/lib/oauth/sessionHelper.ts +++ b/apps/api-manager/src/lib/oauth/sessionHelper.ts @@ -26,13 +26,6 @@ export class SessionOAuthHelper { logger.debug(` Access token present: ${!!oauthData?.access_token}`); logger.debug(` Refresh token present: ${!!oauthData?.refresh_token}`); - if (oauthData?.access_token) { - logger.debug(` Access token length: ${oauthData.access_token.length}`); - logger.debug( - ` Access token preview: ${oauthData.access_token.substring(0, 30)}...`, - ); - } - if (!oauthData?.provider || !oauthData?.access_token) { logger.warn("🔐 SESSION OAUTH - Missing provider or access token"); return null; diff --git a/apps/api-manager/src/routes/(protected)/account-access/account-directory/+page.svelte b/apps/api-manager/src/routes/(protected)/account-access/account-directory/+page.svelte index b7713cbc..1238b00a 100644 --- a/apps/api-manager/src/routes/(protected)/account-access/account-directory/+page.svelte +++ b/apps/api-manager/src/routes/(protected)/account-access/account-directory/+page.svelte @@ -164,7 +164,7 @@ )} - diff --git a/apps/api-manager/src/routes/(protected)/account-access/accounts/+page.svelte b/apps/api-manager/src/routes/(protected)/account-access/accounts/+page.svelte index e8f06330..a0a4be6e 100644 --- a/apps/api-manager/src/routes/(protected)/account-access/accounts/+page.svelte +++ b/apps/api-manager/src/routes/(protected)/account-access/accounts/+page.svelte @@ -19,7 +19,7 @@ const res = await trackedFetch(`/proxy/obp/v6.0.0/banks/${encodeURIComponent(bankId)}/accounts`); if (!res.ok) { const data = await res.json().catch(() => ({})); - throw new Error(data.message ?? `HTTP ${res.status}`); + throw new Error(data.message || data.error || `HTTP ${res.status} Failed to fetch accounts`); } const data = await res.json(); accounts = data.accounts || []; diff --git a/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/+page.svelte b/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/+page.svelte index 22f179fc..6c61ec0c 100644 --- a/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/+page.svelte +++ b/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/+page.svelte @@ -146,7 +146,8 @@ ); if (!res.ok) { const data = await res.json().catch(() => ({})); - throw new Error(data.message ?? `HTTP ${res.status}`); + const msg = data.message || data.error || `HTTP ${res.status}`; + throw new Error(msg); } account = await res.json(); } catch (err) { @@ -171,8 +172,7 @@ ); if (!res.ok) { const data = await res.json().catch(() => ({})); - throw new Error(data.message ?? `HTTP ${res.status}`); - } + throw new Error(data.message || data.error || `Failed to fetch users with access for view ${vid}`); } const data = await res.json(); return { viewId: vid, users: data.users || [] }; }) @@ -391,7 +391,7 @@ ); if (!res.ok) { const data = await res.json().catch(() => ({})); - throw new Error(data.message ?? `HTTP ${res.status}`); + throw new Error(data.message || data.error || `HTTP ${res.status} Failed to fetch customer account links`); } const data = await res.json(); customerAccountLinks = data.links || []; @@ -434,7 +434,7 @@ if (!res.ok) { const data = await res.json().catch(() => ({})); - throw new Error(data.message ?? `HTTP ${res.status}`); + throw new Error(data.message || data.error || `HTTP ${res.status} Failed to create account attribute`); } const created = await res.json(); diff --git a/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/counterparties/+page.svelte b/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/counterparties/+page.svelte index b20bab72..1d6861fe 100644 --- a/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/counterparties/+page.svelte +++ b/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/counterparties/+page.svelte @@ -31,7 +31,7 @@ ); if (!res.ok) { const data = await res.json().catch(() => ({})); - throw new Error(data.message ?? `HTTP ${res.status}`); + throw new Error(data.message || data.error || `HTTP ${res.status} Failed to fetch counterparties`); } const data = await res.json(); counterparties = data.counterparties || []; diff --git a/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/counterparties/[counterparty_id]/+page.svelte b/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/counterparties/[counterparty_id]/+page.svelte index 4ee7e3e0..d40babba 100644 --- a/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/counterparties/[counterparty_id]/+page.svelte +++ b/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/counterparties/[counterparty_id]/+page.svelte @@ -79,7 +79,7 @@ ); if (!res.ok) { const data = await res.json().catch(() => ({})); - throw new Error(data.message ?? `HTTP ${res.status}`); + throw new Error(data.message || data.error || `HTTP ${res.status} Failed to fetch counterparty details`); } counterparty = await res.json(); } catch (err) { diff --git a/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/transactions/+page.svelte b/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/transactions/+page.svelte index dfde45b5..ea755c98 100644 --- a/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/transactions/+page.svelte +++ b/apps/api-manager/src/routes/(protected)/account-access/accounts/[bank_id]/[account_id]/[view_id]/transactions/+page.svelte @@ -32,7 +32,7 @@ ); if (!res.ok) { const data = await res.json().catch(() => ({})); - throw new Error(data.message ?? `HTTP ${res.status}`); + throw new Error(data.message || data.error || `HTTP ${res.status} Failed to fetch transactions`); } const data = await res.json(); transactions = data.transactions || []; diff --git a/apps/api-manager/src/routes/(protected)/banks/create/+page.svelte b/apps/api-manager/src/routes/(protected)/banks/create/+page.svelte index 69b277dd..290c3c7a 100644 --- a/apps/api-manager/src/routes/(protected)/banks/create/+page.svelte +++ b/apps/api-manager/src/routes/(protected)/banks/create/+page.svelte @@ -9,6 +9,7 @@ let logo = $state(""); let website = $state(""); let isSubmitting = $state(false); + let submitError = $state(null); // Bank routings - dynamic list let routings = $state>([]); @@ -35,6 +36,7 @@ } isSubmitting = true; + submitError = null; try { const requestBody: Record = { @@ -90,6 +92,7 @@ } catch (err) { const errorMessage = err instanceof Error ? err.message : "Failed to create bank"; + submitError = errorMessage; toast.error("Error", errorMessage); } finally { isSubmitting = false; @@ -290,6 +293,29 @@ + + {#if submitError} +
+ + + +
+

Error

+

{submitError}

+
+ +
+ {/if} +
{ // Redirect to the originally requested URL if available let redirectTo = event.cookies.get("obp_redirect_to") || "/"; event.cookies.delete("obp_redirect_to", { path: "/" }); - // Validate: must start with / to prevent open redirect - if (!redirectTo.startsWith("/")) { + // Validate: must be a same-origin relative path to prevent open redirect + if (!isSafeRelativeRedirect(redirectTo)) { redirectTo = "/"; } diff --git a/apps/api-manager/src/routes/login/obp/callback/+server.ts b/apps/api-manager/src/routes/login/obp/callback/+server.ts index d38e4732..7b8d0689 100644 --- a/apps/api-manager/src/routes/login/obp/callback/+server.ts +++ b/apps/api-manager/src/routes/login/obp/callback/+server.ts @@ -1,4 +1,4 @@ -import { createLogger } from "@obp/shared/utils"; +import { createLogger, isSafeRelativeRedirect } from "@obp/shared/utils"; const logger = createLogger("OBPLoginCallback"); import { oauth2ProviderFactory } from "$lib/oauth/providerFactory"; import type { OAuth2Tokens } from "arctic"; @@ -238,8 +238,8 @@ export async function GET(event: RequestEvent): Promise { // Redirect to the originally requested URL if available let redirectTo = event.cookies.get("obp_redirect_to") || "/"; event.cookies.delete("obp_redirect_to", { path: "/" }); - // Validate: must start with / to prevent open redirect - if (!redirectTo.startsWith("/")) { + // Validate: must be a same-origin relative path to prevent open redirect + if (!isSafeRelativeRedirect(redirectTo)) { redirectTo = "/"; } diff --git a/apps/api-manager/src/routes/proxy/[...path]/+server.ts b/apps/api-manager/src/routes/proxy/[...path]/+server.ts index 18994303..9da27658 100644 --- a/apps/api-manager/src/routes/proxy/[...path]/+server.ts +++ b/apps/api-manager/src/routes/proxy/[...path]/+server.ts @@ -5,6 +5,8 @@ import { createLogger } from '@obp/shared/utils'; const logger = createLogger("Proxy"); +const TIMEOUT_MS = 15_000; + /** * Generic catch-all proxy for OBP API. * @@ -34,7 +36,23 @@ export const fallback: RequestHandler = async ({ params, request, locals, url }) }); } - const obpPath = params.path; + const obpPath = params.path ?? ''; + + // Reject path-traversal and absolute-URL attempts before joining into the OBP URL. + if ( + obpPath === '' || + obpPath.includes('..') || + obpPath.startsWith('/') || + obpPath.includes('://') || + obpPath.includes('\0') + ) { + logger.warn(`Rejected proxy path: ${obpPath}`); + return new Response(JSON.stringify({ message: "Invalid path" }), { + status: 400, + headers: { "content-type": "application/json" }, + }); + } + const queryString = url.search; const obpUrl = `${env.PUBLIC_OBP_BASE_URL}/${obpPath}${queryString}`; @@ -54,13 +72,37 @@ export const fallback: RequestHandler = async ({ params, request, locals, url }) }; // Forward body for methods that have one + let requestBody: string | undefined; if (["POST", "PUT", "PATCH", "DELETE"].includes(request.method)) { - fetchOptions.body = await request.text(); + requestBody = await request.text(); + fetchOptions.body = requestBody; } const startTime = performance.now(); - const obpResponse = await fetch(obpUrl, fetchOptions); + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), TIMEOUT_MS); + fetchOptions.signal = controller.signal; + + let obpResponse: Response; + try { + obpResponse = await fetch(obpUrl, fetchOptions); + } catch (error: any) { + if (error.name === 'AbortError') { + logger.error(`Timeout: ${request.method} /${obpPath}`); + return new Response(JSON.stringify({ message: `Request to ${obpPath} timed out` }), { + status: 504, + headers: { "content-type": "application/json" }, + }); + } + logger.error(`Error: ${request.method} /${obpPath}:`, error); + return new Response(JSON.stringify({ message: error.message || 'Bad Gateway' }), { + status: 502, + headers: { "content-type": "application/json" }, + }); + } finally { + clearTimeout(timeout); + } const duration = performance.now() - startTime; const correlationId = obpResponse.headers.get("Correlation-Id") || ""; @@ -73,6 +115,36 @@ export const fallback: RequestHandler = async ({ params, request, locals, url }) logger.warn(`Slow request: ${request.method} /${obpPath} took ${duration.toFixed(0)}ms`); } + // Log full request/response details on non-2xx responses + if (!obpResponse.ok) { + const responseBodyText = await obpResponse.text(); + + const loggableHeaders: Record = { ...headers }; + if (loggableHeaders["Authorization"]) { + const token = loggableHeaders["Authorization"]; + loggableHeaders["Authorization"] = token.length > 30 + ? `${token.substring(0, 20)}...[truncated]` + : token; + } + + logger.warn(`Request failed — details: + URL: ${obpUrl} + Method: ${request.method} + Headers: ${JSON.stringify(loggableHeaders, null, 2)} + Body: ${requestBody ? requestBody : "(none)"} + ---- + Status: ${obpResponse.status} + Response: ${responseBodyText}`); + + return new Response(responseBodyText, { + status: obpResponse.status, + headers: { + "content-type": obpResponse.headers.get("content-type") || "application/json", + ...(correlationId ? { "Correlation-Id": correlationId } : {}), + }, + }); + } + // Stream the OBP response back unchanged return new Response(obpResponse.body, { status: obpResponse.status, diff --git a/apps/api-manager/test/setup-server.ts b/apps/api-manager/test/setup-server.ts new file mode 100644 index 00000000..c2b98862 --- /dev/null +++ b/apps/api-manager/test/setup-server.ts @@ -0,0 +1,50 @@ +import { vi, afterEach } from 'vitest'; + +// Mock the SvelteKit modules that are commonly used in tests +vi.mock('$app/environment', () => ({ + browser: false, + building: false, + dev: true, + version: 'test' +})); + +vi.mock('$app/navigation', () => ({ + goto: vi.fn(), + invalidate: vi.fn(), + invalidateAll: vi.fn(), + preloadData: vi.fn(), + preloadCode: vi.fn(), + beforeNavigate: vi.fn(), + afterNavigate: vi.fn(), + pushState: vi.fn(), + replaceState: vi.fn() +})); + +// Mock Arctic OAuth library. Its OAuth2Client is used as a base class; the mock +// returns a plain object, so tests that need the real subclass prototype reset +// this implementation in a beforeEach. +vi.mock('arctic', () => ({ + generateState: vi.fn(() => 'mock-state-123'), + OAuth2Client: vi.fn().mockImplementation(() => ({ + createAuthorizationURL: vi.fn(), + validateAuthorizationCode: vi.fn(), + refreshAccessToken: vi.fn() + })) +})); + +// Global fetch mock (individual tests override as needed) +const mockFetch = vi.fn(); +global.fetch = mockFetch; +mockFetch.mockImplementation(async (url: string | Request) => { + console.warn(`Unmocked fetch call to: ${typeof url === 'string' ? url : url.url}`); + return new Response(JSON.stringify({ error: 'Not found' }), { + status: 404, + headers: { 'content-type': 'application/json' } + }); +}); + +const originalConsole = { ...console }; +afterEach(() => { + vi.restoreAllMocks(); + Object.assign(console, originalConsole); +}); diff --git a/apps/api-manager/test/unit/routes/proxy.test.ts b/apps/api-manager/test/unit/routes/proxy.test.ts new file mode 100644 index 00000000..410a4a52 --- /dev/null +++ b/apps/api-manager/test/unit/routes/proxy.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('$env/dynamic/public', () => ({ env: { PUBLIC_OBP_BASE_URL: 'https://obp.example.com' } })); +vi.mock('$lib/oauth/sessionHelper', () => ({ + SessionOAuthHelper: { getSessionOAuth: vi.fn() } +})); + +import { fallback } from '$lib/../routes/proxy/[...path]/+server'; +import { SessionOAuthHelper } from '$lib/oauth/sessionHelper'; + +const getSessionOAuth = vi.mocked(SessionOAuthHelper.getSessionOAuth); + +function makeEvent({ + path, + method = 'GET', + search = '', + hasUser = true, + body +}: { + path: string; + method?: string; + search?: string; + hasUser?: boolean; + body?: string; +}) { + const target = `http://localhost/proxy/${path}${search}`; + return { + params: { path }, + request: new Request(target, { + method, + ...(body !== undefined ? { body } : {}) + }), + locals: { session: hasUser ? { data: { user: { user_id: 'u-1' } } } : { data: {} } }, + url: new URL(target) + } as never; +} + +describe('proxy fallback handler', () => { + beforeEach(() => { + getSessionOAuth.mockReset(); + // Default: an authenticated session with a usable access token. + getSessionOAuth.mockReturnValue({ accessToken: 'access-token-123' } as never); + }); + + it('returns 401 when there is no logged-in user', async () => { + const res = await fallback(makeEvent({ path: 'obp/v6.0.0/banks', hasUser: false })); + expect(res.status).toBe(401); + await expect(res.json()).resolves.toEqual({ message: 'Unauthorized' }); + }); + + it('returns 401 when the session has no access token', async () => { + getSessionOAuth.mockReturnValue(null as never); + const res = await fallback(makeEvent({ path: 'obp/v6.0.0/banks' })); + expect(res.status).toBe(401); + await expect(res.json()).resolves.toEqual({ message: 'No API access token available' }); + }); + + const badPaths: Array<[string, string]> = [ + ['empty path', ''], + ['parent traversal', '..'], + ['embedded traversal', 'obp/../../etc/passwd'], + ['leading slash / absolute path', '/obp/v6.0.0/banks'], + ['absolute URL', 'https://evil.example.com/steal'], + ['null byte', 'obp\0/banks'] + ]; + + for (const [label, path] of badPaths) { + it(`rejects ${label} with 400 and never calls fetch`, async () => { + const res = await fallback(makeEvent({ path })); + expect(res.status).toBe(400); + await expect(res.json()).resolves.toEqual({ message: 'Invalid path' }); + expect(global.fetch).not.toHaveBeenCalled(); + }); + } + + it('forwards a valid path to OBP with the bearer token and query string', async () => { + const upstream = vi + .fn() + .mockResolvedValue( + new Response('{"banks":[]}', { status: 200, headers: { 'content-type': 'application/json' } }) + ); + global.fetch = upstream as never; + + const res = await fallback(makeEvent({ path: 'obp/v6.0.0/banks', search: '?limit=5' })); + + expect(upstream).toHaveBeenCalledTimes(1); + const [calledUrl, options] = upstream.mock.calls[0]; + expect(calledUrl).toBe('https://obp.example.com/obp/v6.0.0/banks?limit=5'); + expect((options.headers as Record).Authorization).toBe('Bearer access-token-123'); + expect(res.status).toBe(200); + }); +}); diff --git a/apps/portal/package.json b/apps/portal/package.json index 34fdb618..c383e719 100644 --- a/apps/portal/package.json +++ b/apps/portal/package.json @@ -61,7 +61,7 @@ "@types/dompurify": "^3.0.5", "arctic": "^3.7.0", "bits-ui": "^2.8.10", - "dompurify": "^3.3.3", + "dompurify": "^3.4.1", "ioredis": "^5.6.1", "jwt-decode": "^4.0.0", "langchain": "^0.3.27", diff --git a/apps/portal/src/hooks.server.ts b/apps/portal/src/hooks.server.ts index be288915..6b848e5e 100644 --- a/apps/portal/src/hooks.server.ts +++ b/apps/portal/src/hooks.server.ts @@ -237,8 +237,17 @@ const checkAuthorization: Handle = async ({ event, resolve }) => { return response; }; +// Routes that carry a single-use secret token in the URL path — analytics must not +// see these paths (gtag reports page_location, which would exfiltrate the token). +const TOKEN_IN_URL_ROUTE_PREFIXES = ['/reset-password/', '/user_mgt/reset_password/']; + +function pathCarriesUrlToken(pathname: string): boolean { + return TOKEN_IN_URL_ROUTE_PREFIXES.some((prefix) => pathname.startsWith(prefix)); +} + const transformHTML: Handle = async ({ event, resolve }) => { - const analyticsScript = env.ENABLE_ANALYTICS === "true" && env.GTAG_ID + const skipAnalytics = pathCarriesUrlToken(event.url.pathname); + const analyticsScript = env.ENABLE_ANALYTICS === "true" && env.GTAG_ID && !skipAnalytics ? ` )'); + expect(html).toContain('href="#"'); + expect(html).not.toContain('data:text/html'); + }); + + it('rejects a protocol-relative link', () => { + const html = markdownToHtml('[x](//evil.com)'); + expect(html).toContain('href="#"'); + expect(html).not.toContain('href="//evil.com"'); + }); + + it('rejects a backslash-prefixed link', () => { + const html = markdownToHtml('[x](/\\evil.com)'); + expect(html).toContain('href="#"'); + }); + + it('keeps an absolute https link', () => { + const html = markdownToHtml('[ok](https://example.com/path)'); + expect(html).toContain('href="https://example.com/path"'); + }); + + it('keeps a site-relative link', () => { + const html = markdownToHtml('[rel](/docs)'); + expect(html).toContain('href="/docs"'); + }); + + it('keeps a fragment link', () => { + const html = markdownToHtml('[frag](#section)'); + expect(html).toContain('href="#section"'); + }); +}); + +describe('sanitizeLinkHref', () => { + it('escapes ampersands, quotes, and angle brackets in a safe href', () => { + const result = sanitizeLinkHref('/search?a=1&b=2">"]/); + expect(result).not.toContain('&b'); // bare ampersand is escaped + }); + + it('collapses unsafe schemes to a hash', () => { + expect(sanitizeLinkHref('vbscript:msgbox(1)')).toBe('#'); + expect(sanitizeLinkHref('//host')).toBe('#'); + expect(sanitizeLinkHref('/\\host')).toBe('#'); + }); + + it('trims surrounding whitespace before validating', () => { + expect(sanitizeLinkHref(' https://example.com ')).toBe('https://example.com'); + }); +}); + +describe('markdownToHtml text escaping', () => { + it('escapes raw HTML in the markdown body', () => { + const html = markdownToHtml('an tag'); + expect(html).toContain('<img'); + expect(html).not.toContain('/g, '>'); +} + +// Convert markdown to HTML (for rendering) +export function markdownToHtml(markdown: string): string { + return ( + markdown + // Escape HTML first + .replace(/&/g, '&') + .replace(//g, '>') + // Then apply markdown formatting + .replace(/\*\*([^*]+)\*\*/g, '$1') // Bold + .replace(/\*([^*]+)\*/g, '$1') // Italic + .replace(/`([^`]+)`/g, '$1') // Inline code + .replace( + /\[([^\]]+)\]\(([^)]+)\)/g, + (_match, text, href) => + `${text}` + ) // Links + .replace(/^#{1,6}\s+(.+)$/gm, '$1') // Headings to bold + .replace(/\n\n+/g, '

') // Paragraph breaks + .replace(/\n/g, ' ') // Single newlines to spaces + .replace(/^/, '

') // Wrap in paragraph + .replace(/$/, '

') + .replace(/

<\/p>/g, '') + ); // Remove empty paragraphs +} diff --git a/apps/portal/src/routes/(protected)/obp-consent-request/+page.server.ts b/apps/portal/src/routes/(protected)/obp-consent-request/+page.server.ts index 543d73f4..b6aedbb3 100644 --- a/apps/portal/src/routes/(protected)/obp-consent-request/+page.server.ts +++ b/apps/portal/src/routes/(protected)/obp-consent-request/+page.server.ts @@ -4,6 +4,7 @@ import type { RequestEvent, Actions } from '@sveltejs/kit'; import { redirect, isRedirect } from '@sveltejs/kit'; import { obp_requests } from '$lib/obp/requests'; import { OBPRequestError } from '@obp/shared/obp'; +import { oauth2ProviderFactory } from '$lib/oauth/providerFactory'; export async function load(event: RequestEvent) { const consentRequestId = event.url.searchParams.get('CONSENT_REQUEST_ID'); @@ -51,11 +52,17 @@ export async function load(event: RequestEvent) { logger.info(`Consent request response: ${JSON.stringify(consentRequest)}`); + // OBP-API returns `payload` as an already-parsed JSON object (JValue on the + // server side), not a JSON string — do not JSON.parse it again. let payload: any = {}; - try { - payload = JSON.parse(consentRequest.payload || '{}'); - } catch { - logger.warn('Could not parse consent request payload'); + if (consentRequest.payload && typeof consentRequest.payload === 'object') { + payload = consentRequest.payload; + } else if (typeof consentRequest.payload === 'string' && consentRequest.payload) { + try { + payload = JSON.parse(consentRequest.payload); + } catch { + logger.warn('Could not parse consent request payload'); + } } logger.info(`Parsed payload: ${JSON.stringify(payload)}`); @@ -73,8 +80,16 @@ export async function load(event: RequestEvent) { } } - // Store the OIDC return URL in session if present - const oidcReturnUrl = event.url.searchParams.get('oidc_return_url'); + // Store the OIDC return URL in session if present — must point back to a + // configured OIDC provider host, otherwise it's an open redirect / data leak. + const requestedOidcReturnUrl = event.url.searchParams.get('oidc_return_url'); + const oidcReturnUrl = + requestedOidcReturnUrl && oauth2ProviderFactory.isTrustedOidcReturnUrl(requestedOidcReturnUrl) + ? requestedOidcReturnUrl + : null; + if (requestedOidcReturnUrl && !oidcReturnUrl) { + logger.warn(`Rejected untrusted oidc_return_url: ${requestedOidcReturnUrl}`); + } if (oidcReturnUrl) { await event.locals.session.setData({ ...event.locals.session.data, diff --git a/apps/portal/src/routes/(protected)/obp-consent-request/+page.svelte b/apps/portal/src/routes/(protected)/obp-consent-request/+page.svelte index a1496d7f..4c71cda0 100644 --- a/apps/portal/src/routes/(protected)/obp-consent-request/+page.svelte +++ b/apps/portal/src/routes/(protected)/obp-consent-request/+page.svelte @@ -34,17 +34,12 @@ Consent Details -

- 
{JSON.stringify(data.payload, null, 2)}
-
-
{#if data.payload.everything !== undefined}
Access Level: - {data.payload.everything ? 'Full access' : 'Limited access'} + {data.payload.everything ? 'Full access to all your accounts' : 'Limited access'}
{/if} @@ -72,6 +67,46 @@
{/if}
+ + {#if !data.payload.everything && data.payload.account_access?.length} +
+

+ Accounts requested ({data.payload.account_access.length}) +

+
    + {#each data.payload.account_access as access} +
  • +
    + Account: + {access.account_routing?.address} + ({access.account_routing?.scheme}) +
    +
    + Permission: + {access.view_id} +
    +
    • + {/each} +
+
+ {/if} + + {#if data.payload.entitlements?.length} +
+

+ Roles requested ({data.payload.entitlements.length}) +

+
    + {#each data.payload.entitlements as entitlement} +
  • + {entitlement.role_name} ({entitlement.bank_id}) +
    • + {/each} +
+
+ {/if} diff --git a/apps/portal/src/routes/(protected)/uk-consent-request-sca/+page.server.ts b/apps/portal/src/routes/(protected)/uk-consent-request-sca/+page.server.ts new file mode 100644 index 00000000..f1b9b06d --- /dev/null +++ b/apps/portal/src/routes/(protected)/uk-consent-request-sca/+page.server.ts @@ -0,0 +1,102 @@ +import { createLogger } from '@obp/shared/utils'; +const logger = createLogger('UKConsentRequestSCA'); +import type { RequestEvent, Actions } from '@sveltejs/kit'; +import { redirect } from '@sveltejs/kit'; +import { obp_requests } from '$lib/obp/requests'; +import { OBPRequestError } from '@obp/shared/obp'; +import { oauth2ProviderFactory } from '$lib/oauth/providerFactory'; + +/** + * UK Open Banking consent SCA (OTP entry). + * + * Reached after /uk-consent-request started a challenge. The PSU enters the OTP; on a valid answer + * OBP-API authorises the consent (status -> AUTHORISED, bound to this user) and we hand control back + * to OBP-OIDC (with username + provider) so it mints a consent-bound auth code. + */ +export async function load(event: RequestEvent) { + const consentId = event.url.searchParams.get('CONSENT_ID'); + const bankId = event.url.searchParams.get('bank_id'); + const challengeId = event.url.searchParams.get('challenge_id'); + const requestedOidcReturnUrl = event.url.searchParams.get('oidc_return_url'); + + if (!consentId || !bankId || !challengeId) { + return { + loadError: 'Missing required parameter (CONSENT_ID, bank_id or challenge_id).', + consentId: consentId || '', + bankId: bankId || '', + challengeId: challengeId || '', + oidcReturnUrl: '' + }; + } + + const token = event.locals.session.data.oauth?.access_token; + if (!token) { + return { loadError: 'Unauthorized: No access token found in session.', consentId, bankId, challengeId, oidcReturnUrl: '' }; + } + + const oidcReturnUrl = + requestedOidcReturnUrl && oauth2ProviderFactory.isTrustedOidcReturnUrl(requestedOidcReturnUrl) + ? requestedOidcReturnUrl + : ''; + if (requestedOidcReturnUrl && !oidcReturnUrl) { + logger.warn(`Rejected untrusted oidc_return_url: ${requestedOidcReturnUrl}`); + } + + return { consentId, bankId, challengeId, oidcReturnUrl }; +} + +export const actions = { + default: async ({ request, locals }) => { + const formData = await request.formData(); + const otp = formData.get('otp') as string; + const consentId = formData.get('consentId') as string; + const bankId = formData.get('bankId') as string; + const challengeId = formData.get('challengeId') as string; + const oidcReturnUrlRaw = formData.get('oidcReturnUrl') as string; + + if (!otp) { + return { message: 'Please enter the OTP code.' }; + } + + const token = locals.session.data.oauth?.access_token; + if (!token) { + return { message: 'No access token found in session.' }; + } + + const oidcReturnUrl = + oidcReturnUrlRaw && oauth2ProviderFactory.isTrustedOidcReturnUrl(oidcReturnUrlRaw) + ? oidcReturnUrlRaw + : ''; + if (!oidcReturnUrl) { + return { message: 'No valid return URL was provided to complete the flow.' }; + } + + try { + // Verify the OTP and authorise the consent. OBP-API flips it to AUTHORISED and binds it to + // the PSU only if the challenge answer is correct; a wrong answer leaves it unauthorised. + await obp_requests.post( + `/obp/v5.1.0/banks/${bankId}/consents/${consentId}/authorise`, + { challenge_id: challengeId, answer: otp }, + token + ); + } catch (e) { + logger.error('Error verifying UK consent SCA:', e); + let errorMessage = 'Failed to verify OTP.'; + if (e instanceof OBPRequestError) { + errorMessage = e.message; + } + return { message: errorMessage }; + } + + // Hand control back to OBP-OIDC so it mints a consent-bound auth code. username + provider let + // OBP-OIDC's consent callback resolve the PSU (via GET /users/provider/{provider}/username/{username}). + const returnUrl = new URL(oidcReturnUrl); + returnUrl.searchParams.set('consent_id', consentId); + returnUrl.searchParams.set('consent_status', 'ACCEPTED'); + const username = locals.session.data.user?.username; + const provider = locals.session.data.user?.provider; + if (username) returnUrl.searchParams.set('username', username); + if (provider) returnUrl.searchParams.set('provider', provider); + redirect(303, returnUrl.toString()); + } +} satisfies Actions; diff --git a/apps/portal/src/routes/(protected)/uk-consent-request-sca/+page.svelte b/apps/portal/src/routes/(protected)/uk-consent-request-sca/+page.svelte new file mode 100644 index 00000000..9b7df79f --- /dev/null +++ b/apps/portal/src/routes/(protected)/uk-consent-request-sca/+page.svelte @@ -0,0 +1,49 @@ + + +
+

+ UK Open Banking — Verify +

+ + {#if data.loadError} +
+

{data.loadError}

+
+ {:else} +

+ A one-time passcode (OTP) has been sent to authorise this consent. Enter it below to complete + Strong Customer Authentication. +

+ + {#if form?.message} +
+

{form.message}

+
+ {/if} + +
+ + + + + + + + + +
+ {/if} +
diff --git a/apps/portal/src/routes/(protected)/uk-consent-request/+page.server.ts b/apps/portal/src/routes/(protected)/uk-consent-request/+page.server.ts new file mode 100644 index 00000000..4038acbe --- /dev/null +++ b/apps/portal/src/routes/(protected)/uk-consent-request/+page.server.ts @@ -0,0 +1,139 @@ +import { createLogger } from '@obp/shared/utils'; +const logger = createLogger('UKConsentRequest'); +import type { RequestEvent, Actions } from '@sveltejs/kit'; +import { redirect } from '@sveltejs/kit'; +import { obp_requests } from '$lib/obp/requests'; +import { OBPRequestError } from '@obp/shared/obp'; +import { oauth2ProviderFactory } from '$lib/oauth/providerFactory'; + +/** + * UK Open Banking consent approval. + * + * The TPP has already lodged an account-access consent (status AWAITINGAUTHORISATION) and routed + * the PSU here via OBP-OIDC. On approval we start Strong Customer Authentication (OBP-API issues a + * one-time challenge to the PSU) and route to /uk-consent-request-sca to collect the OTP; the + * consent is only authorised (status -> AUTHORISED, bound to this user) once the correct OTP is + * submitted, after which control returns to OBP-OIDC to mint a consent-bound auth code. + */ +// UK consent GET endpoints are version-specific (v3.1 vs v4.0.1 aisp), but return the same +// {Data: {Status, Permissions, ExpirationDateTime, ...}} shape — see OBP-API +// Http4sUKOBv310AccountAccess#getAccountAccessConsentsConsentId / Http4sUKOBv401AccountInfo#getAccountAccessConsentsConsentId. +function ukConsentGetPath(apiStandard: string, consentId: string): string { + return apiStandard === 'UKOpenBankingV401' + ? `/open-banking/v4.0.1/aisp/account-access-consents/${consentId}` + : `/open-banking/v3.1/account-access-consents/${consentId}`; +} + +export async function load(event: RequestEvent) { + const consentId = event.url.searchParams.get('CONSENT_ID'); + const bankId = event.url.searchParams.get('bank_id'); + const apiStandard = event.url.searchParams.get('api_standard') || 'UKOpenBanking'; + const requestedOidcReturnUrl = event.url.searchParams.get('oidc_return_url'); + + if (!consentId) { + return { loadError: 'Missing required parameter: CONSENT_ID.', consentId: '', bankId: bankId || '', apiStandard, oidcReturnUrl: '' }; + } + if (!bankId) { + return { loadError: 'Missing required parameter: bank_id.', consentId, bankId: '', apiStandard, oidcReturnUrl: '' }; + } + + const token = event.locals.session.data.oauth?.access_token; + if (!token) { + return { loadError: 'Unauthorized: No access token found in session.', consentId, bankId, apiStandard, oidcReturnUrl: '' }; + } + + // oidc_return_url must point back to a configured OIDC provider host — otherwise it becomes + // an open redirect that would also leak consent_id / user identity on completion. + const oidcReturnUrl = + requestedOidcReturnUrl && oauth2ProviderFactory.isTrustedOidcReturnUrl(requestedOidcReturnUrl) + ? requestedOidcReturnUrl + : ''; + if (requestedOidcReturnUrl && !oidcReturnUrl) { + logger.warn(`Rejected untrusted oidc_return_url: ${requestedOidcReturnUrl}`); + } + + // The UK consent is still awaiting authorisation and not yet bound to this user, so it can't + // be fetched via user/current/consents (which requires ownership). The UK GET-consent-by-id + // endpoint has no ownership check — any authenticated caller may inspect a consent by its ID, + // matching the UK Open Banking spec's PSU-facing consent-status check before authorisation. + let status = ''; + let permissions: string[] = []; + let expirationDateTime = ''; + try { + const consent = await obp_requests.get(ukConsentGetPath(apiStandard, consentId), token); + status = consent?.Data?.Status || ''; + permissions = consent?.Data?.Permissions || []; + expirationDateTime = consent?.Data?.ExpirationDateTime || ''; + } catch (e) { + // Non-fatal: fall back to the generic approval screen without permission detail. + logger.warn('Could not fetch UK consent details:', e); + } + + return { consentId, bankId, apiStandard, oidcReturnUrl, status, permissions, expirationDateTime }; +} + +export const actions = { + confirm: async ({ request, locals }) => { + const formData = await request.formData(); + const consentId = formData.get('consentId') as string; + const bankId = formData.get('bankId') as string; + const oidcReturnUrlRaw = formData.get('oidcReturnUrl') as string; + + const token = locals.session.data.oauth?.access_token; + if (!token) { + return { message: 'No access token found in session.' }; + } + + // Re-validate the return URL server-side — hidden form fields are attacker-controllable. + const oidcReturnUrl = + oidcReturnUrlRaw && oauth2ProviderFactory.isTrustedOidcReturnUrl(oidcReturnUrlRaw) + ? oidcReturnUrlRaw + : ''; + if (!oidcReturnUrl) { + logger.warn('No trusted OIDC return URL for UK consent flow'); + return { message: 'No valid return URL was provided to complete the flow.' }; + } + + let challengeId = ''; + try { + // Start SCA: OBP-API issues a one-time challenge (OTP) to the PSU. The consent is only + // authorised on the /uk-consent-request-sca page once the correct OTP is submitted. + const challengeResponse = await obp_requests.post( + `/obp/v5.1.0/banks/${bankId}/consents/${consentId}/authorise/challenge`, + {}, + token + ); + challengeId = challengeResponse.challenge_id; + } catch (e) { + logger.error('Error starting UK consent SCA challenge:', e); + let errorMessage = 'Failed to start Strong Customer Authentication.'; + if (e instanceof OBPRequestError) { + errorMessage = e.message; + } + return { message: errorMessage }; + } + + const params = new URLSearchParams({ + CONSENT_ID: consentId, + bank_id: bankId, + challenge_id: challengeId, + oidc_return_url: oidcReturnUrl + }); + redirect(303, `/uk-consent-request-sca?${params.toString()}`); + }, + + deny: async ({ request }) => { + const formData = await request.formData(); + const oidcReturnUrlRaw = formData.get('oidcReturnUrl') as string; + const oidcReturnUrl = + oidcReturnUrlRaw && oauth2ProviderFactory.isTrustedOidcReturnUrl(oidcReturnUrlRaw) + ? oidcReturnUrlRaw + : ''; + if (oidcReturnUrl) { + const returnUrl = new URL(oidcReturnUrl); + returnUrl.searchParams.set('consent_status', 'REJECTED'); + redirect(303, returnUrl.toString()); + } + redirect(303, '/'); + } +} satisfies Actions; diff --git a/apps/portal/src/routes/(protected)/uk-consent-request/+page.svelte b/apps/portal/src/routes/(protected)/uk-consent-request/+page.svelte new file mode 100644 index 00000000..2f83c74b --- /dev/null +++ b/apps/portal/src/routes/(protected)/uk-consent-request/+page.svelte @@ -0,0 +1,95 @@ + + +
+

+ UK Open Banking Consent +

+ + {#if data.loadError} +
+

{data.loadError}

+
+ {:else} +

+ An application is requesting access to your account information via UK Open Banking. + Please review the details below and confirm if you agree. +

+ + {#if form?.message} +
+

{form.message}

+
+ {/if} + + +
+

+ Consent Details +

+ +
+
+ Standard: + + {data.apiStandard === 'UKOpenBankingV401' ? 'UK Open Banking v4.0.1' : 'UK Open Banking'} + +
+ {#if data.bankId} +
+ Bank: + {data.bankId} +
+ {/if} +
+ Consent ID: + {data.consentId} +
+ {#if data.status} +
+ Status: + {data.status} +
+ {/if} + {#if data.expirationDateTime} +
+ Expires: + {data.expirationDateTime} +
+ {/if} +
+ + {#if data.permissions?.length} +
+

+ Permissions requested ({data.permissions.length}) +

+
    + {#each data.permissions as permission} +
  • {permission}
    • + {/each} +
+
+ {/if} +
+ + +
+
+ + + + +
+
+ + +
+
+ {/if} +
diff --git a/apps/portal/src/routes/featured/+page.svelte b/apps/portal/src/routes/featured/+page.svelte index 5595378f..9cd88c2a 100644 --- a/apps/portal/src/routes/featured/+page.svelte +++ b/apps/portal/src/routes/featured/+page.svelte @@ -1,5 +1,6 @@
Email Validation
- {#if data.success} + + + {#if !data.token} +
+
+ +
+

Validation Failed

+

No validation token provided. Please check your email for the validation link.

+
+ {:else if !form} +
+ +

Validating your email...

+
+ {:else if form.success}

Success!

-

{data.message}

+

{form.message}

Your email has been successfully validated. You can now log in to your account.

@@ -38,10 +75,10 @@

Validation Failed

-

{data.message}

- {#if data.errorCode} +

{form.message}

+ {#if form.errorCode}

- Error Code: {data.errorCode} + Error Code: {form.errorCode}

{/if}
@@ -71,14 +108,14 @@
{/if} - {#if data.data} + {#if form?.data}
View Details
    - {#each Object.entries(data.data) as [key, value]} + {#each Object.entries(form.data) as [key, value]}
  • {key}: {#if typeof value === 'object' && value !== null} @@ -93,4 +130,4 @@
{/if}
-
\ No newline at end of file + diff --git a/apps/portal/test/integration/oauth-flow.test.ts b/apps/portal/test/integration/oauth-flow.test.ts index f2f271cb..ef705303 100644 --- a/apps/portal/test/integration/oauth-flow.test.ts +++ b/apps/portal/test/integration/oauth-flow.test.ts @@ -1,549 +1,255 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { OAuth2ClientWithConfig } from '$lib/oauth/client'; -import { refreshAccessTokenInSession } from '$lib/oauth/session'; -import { generateState } from 'arctic'; -import { - mockOIDCConfiguration, - mockUser, - mockAccessToken, - mockRefreshToken, - mockOAuth2Tokens, - mockEnvironment -} from '../fixtures/oauth'; -import { createMockFetch, createMockSession, mockEnvVars, restoreAllMocks } from '../helpers'; - -// Mock Arctic -vi.mock('arctic', () => ({ - generateState: vi.fn(() => 'mock-state-123'), - OAuth2Client: vi.fn() -})); - -// Mock environment variables -vi.mock('$env/dynamic/private', () => ({ - env: mockEnvironment -})); - -vi.mock('$env/dynamic/public', () => ({ - env: mockEnvironment -})); - -// Mock the OAuth client module to avoid initialization issues -vi.mock('$lib/oauth/client', () => ({ - OAuth2ClientWithConfig: vi.fn().mockImplementation(() => ({ - OIDCConfig: undefined, - initOIDCConfig: vi.fn(), - checkAccessTokenExpiration: vi.fn(), - createAuthorizationURL: vi.fn(), - validateAuthorizationCode: vi.fn(), - refreshAccessToken: vi.fn() - })) -})); - -describe('OAuth Flow Integration Tests', () => { - let client: OAuth2ClientWithConfig; - let originalFetch: typeof global.fetch; - +import { mockEnvironment } from '../fixtures/oauth'; + +// providerFactory.ts reads $env/dynamic/private at import time; the server test +// setup does not mock $env, so provide it here. +vi.mock('$env/dynamic/private', () => ({ env: mockEnvironment })); + +import { OAuth2ProviderFactory, oauth2ProviderFactory } from '$lib/oauth/providerFactory'; +import { SessionOAuthHelper } from '$lib/oauth/sessionHelper'; +import type { OAuth2ClientWithConfig } from '$lib/oauth/client'; +import { createMockSession, restoreAllMocks } from '../helpers'; + +const WELL_KNOWN = 'https://idp.example.com/.well-known/openid-configuration'; +const AUTH_ENDPOINT = 'https://idp.example.com/protocol/openid-connect/auth'; +const TOKEN_ENDPOINT = 'https://idp.example.com/protocol/openid-connect/token'; + +// A hand-built client that exposes exactly the two fields the production code +// reads: OIDCConfig (for the token endpoint / trusted origins) and a +// refreshAccessToken function we control. Building it by hand sidesteps the +// globally-mocked arctic OAuth2Client and performs no network I/O. +function makeFakeClient(refreshAccessToken: ReturnType = vi.fn()) { + return { + OIDCConfig: { + authorization_endpoint: AUTH_ENDPOINT, + token_endpoint: TOKEN_ENDPOINT + }, + refreshAccessToken + } as unknown as OAuth2ClientWithConfig; +} + +// A strategy the built-in keycloak/obp-oidc/google strategies never match, whose +// initialize() returns a pre-baked client and performs no fetch. +function makeFakeStrategy(providerName: string, initialize: () => Promise) { + return { + providerName, + supports: (provider: string) => provider === providerName, + getProviderName: () => providerName, + initialize + }; +} + +// SessionOAuthHelper resolves its client from the shared singleton factory, so the +// refresh path is integration-tested against that singleton. 'fake-idp' is matched +// by no built-in strategy, so registering + initializing it exercises the real +// registerStrategy -> initializeProvider -> getClient path. A closure-held client +// lets each test supply a fresh, controllable client through that same real path. +const fakeIdpHolder: { client?: OAuth2ClientWithConfig } = {}; + +async function primeFakeIdp(client: OAuth2ClientWithConfig): Promise { + fakeIdpHolder.client = client; + if (!oauth2ProviderFactory.getStrategy('fake-idp')) { + oauth2ProviderFactory.registerStrategy( + makeFakeStrategy('fake-idp', async () => fakeIdpHolder.client!) as never + ); + } + await oauth2ProviderFactory.initializeProvider({ provider: 'fake-idp', url: WELL_KNOWN }); +} + +function primeGoogle(client: OAuth2ClientWithConfig): void { + // The built-in GoogleStrategy already claims 'google' and would try to construct + // a real client from unset GOOGLE_* env, so seed the initialized-client map + // directly to keep control while still driving the real getClient() lookup. + ( + oauth2ProviderFactory as unknown as { + initializedClients: Map; + } + ).initializedClients.set('google', client); +} + +function clearSingletonClients(): void { + ( + oauth2ProviderFactory as unknown as { + initializedClients: Map; + } + ).initializedClients.clear(); +} + +describe('OAuth Flow Integration', () => { beforeEach(() => { - originalFetch = global.fetch; - mockEnvVars(mockEnvironment); vi.clearAllMocks(); - - client = new OAuth2ClientWithConfig( - mockEnvironment.OBP_OAUTH_CLIENT_ID, - mockEnvironment.OBP_OAUTH_CLIENT_SECRET, - mockEnvironment.APP_CALLBACK_URL - ); - - // Mock inherited methods from Arctic OAuth2Client - Object.setPrototypeOf(client, { - ...Object.getPrototypeOf(client), - validateAuthorizationCode: vi.fn(), - refreshAccessToken: vi.fn(), - createAuthorizationURL: vi - .fn() - .mockImplementation((authEndpoint: string, state: string, scopes: string[]) => { - const url = new URL(authEndpoint); - url.searchParams.set('response_type', 'code'); - url.searchParams.set('client_id', mockEnvironment.OBP_OAUTH_CLIENT_ID); - url.searchParams.set('redirect_uri', mockEnvironment.APP_CALLBACK_URL); - url.searchParams.set('state', state); - url.searchParams.set('scope', scopes.join(' ')); - return url; - }) - }); + // The singleton retains state between tests; reset it so each test is independent. + clearSingletonClients(); }); afterEach(() => { - global.fetch = originalFetch; restoreAllMocks(); }); - describe('Complete OAuth Login Flow', () => { - it('should complete full OAuth flow from initialization to user session', async () => { - // Step 1: Initialize OIDC configuration - const mockFetch = createMockFetch([ - { - url: '/.well-known/openid-configuration', - response: mockOIDCConfiguration - }, - { - url: '/obp/v5.1.0/users/current', - response: mockUser - } - ]); - global.fetch = mockFetch; - - await client.initOIDCConfig( - 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' - ); - - expect(client.OIDCConfig).toEqual(mockOIDCConfiguration); - - // Step 2: Generate authorization URL - const state = vi.mocked(generateState)(); - const scopes = ['openid']; - const authUrl = client.createAuthorizationURL( - client.OIDCConfig.authorization_endpoint, - state, - scopes - ); - - expect(authUrl.toString()).toContain(mockOIDCConfiguration.authorization_endpoint); - expect(authUrl.searchParams.get('state')).toBe(state); - expect(authUrl.searchParams.get('scope')).toBe('openid'); - expect(authUrl.searchParams.get('client_id')).toBe(mockEnvironment.OBP_OAUTH_CLIENT_ID); - expect(authUrl.searchParams.get('redirect_uri')).toBe(mockEnvironment.APP_CALLBACK_URL); - - // Step 3: Mock token exchange (would happen after user authorization) - const mockTokens = { - accessToken: () => mockAccessToken, - refreshToken: () => mockRefreshToken, - accessTokenExpiresAt: () => new Date(Date.now() + 3600000), - refreshTokenExpiresAt: () => new Date(Date.now() + 86400000), - scopes: () => ['openid'] - }; - - vi.spyOn(client, 'validateAuthorizationCode').mockResolvedValue(mockTokens); - - const tokens = await client.validateAuthorizationCode( - client.OIDCConfig.token_endpoint, - 'auth-code-123', - null - ); + describe('Provider factory registration', () => { + it('registers a fake provider and exposes its client and origins', async () => { + const factory = new OAuth2ProviderFactory(); + const client = makeFakeClient(); - expect(tokens.accessToken()).toBe(mockAccessToken); - expect(tokens.refreshToken()).toBe(mockRefreshToken); + factory.registerStrategy(makeFakeStrategy('fake-idp', async () => client) as never); + const initialized = await factory.initializeProvider({ provider: 'fake-idp', url: WELL_KNOWN }); - // Step 4: Fetch user information - const userRequest = new Request( - `${mockEnvironment.PUBLIC_OBP_BASE_URL}/obp/v5.1.0/users/current` - ); - userRequest.headers.set('Authorization', `Bearer ${tokens.accessToken()}`); + expect(initialized).toBe(client); + expect(factory.hasAnyClients()).toBe(true); + expect(factory.getClient('fake-idp')).toBe(client); + expect(factory.getFirstAvailableProvider()).toBe('fake-idp'); - const userResponse = await fetch(userRequest); - const userData = await userResponse.json(); + expect(factory.getTrustedOidcOrigins().has('https://idp.example.com')).toBe(true); + expect(factory.isTrustedOidcReturnUrl('https://idp.example.com/callback?state=abc')).toBe(true); + expect(factory.isTrustedOidcReturnUrl('https://evil.example.com/callback')).toBe(false); + }); - expect(userData).toEqual(mockUser); + it('returns null and stores no client for an unknown provider', async () => { + const factory = new OAuth2ProviderFactory(); - // Step 5: Create session with user data - const session = createMockSession(); - await session.setData({ - user: userData, - oauth: { - access_token: tokens.accessToken(), - refresh_token: tokens.refreshToken() - } + const initialized = await factory.initializeProvider({ + provider: 'not-registered', + url: WELL_KNOWN }); - await session.save(); - expect(session.setData).toHaveBeenCalledWith({ - user: mockUser, - oauth: { - access_token: mockAccessToken, - refresh_token: mockRefreshToken - } - }); - expect(session.save).toHaveBeenCalled(); + expect(initialized).toBeNull(); + expect(factory.hasAnyClients()).toBe(false); + expect(factory.getFirstAvailableProvider()).toBeNull(); }); + }); - it('should handle OAuth flow with token refresh', async () => { - // Setup: Initialize client and session with existing tokens - const mockFetch = createMockFetch([ - { - url: '/.well-known/openid-configuration', - response: mockOIDCConfiguration - } - ]); - global.fetch = mockFetch; - - await client.initOIDCConfig( - 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' - ); + describe('Session token refresh via the shared factory', () => { + it('resolves the client and tokens from the singleton factory', async () => { + const client = makeFakeClient(); + await primeFakeIdp(client); const session = createMockSession({ - user: mockUser, oauth: { - access_token: mockAccessToken, - refresh_token: mockRefreshToken + provider: 'fake-idp', + access_token: 'access-token-1', + refresh_token: 'refresh-token-1', + id_token: 'id-token-1' } }); - // Step 1: Check if access token is expired - const isExpired = await client.checkAccessTokenExpiration(mockAccessToken); - expect(isExpired).toBe(false); // Mock token should not be expired - - // Step 2: Simulate expired token scenario and refresh - const newAccessToken = 'new-access-token-456'; - const newRefreshToken = 'new-refresh-token-456'; - - const mockRefreshedTokens = { - accessToken: () => newAccessToken, - refreshToken: () => newRefreshToken, - accessTokenExpiresAt: () => new Date(Date.now() + 3600000), - refreshTokenExpiresAt: () => new Date(Date.now() + 86400000), - scopes: () => ['openid'] - }; - - vi.spyOn(client, 'refreshAccessToken').mockResolvedValue(mockRefreshedTokens); - - await refreshAccessTokenInSession(session, client); + const sessionOAuth = SessionOAuthHelper.getSessionOAuth(session as never); - expect(client.refreshAccessToken).toHaveBeenCalledWith( - mockOIDCConfiguration.token_endpoint, - mockRefreshToken, - ['openid'] - ); - - expect(session.data.oauth).toEqual({ - access_token: newAccessToken, - refresh_token: newRefreshToken - }); + expect(sessionOAuth).not.toBeNull(); + expect(sessionOAuth?.client).toBe(client); + expect(sessionOAuth?.provider).toBe('fake-idp'); + expect(sessionOAuth?.accessToken).toBe('access-token-1'); + expect(sessionOAuth?.refreshToken).toBe('refresh-token-1'); + expect(sessionOAuth?.idToken).toBe('id-token-1'); }); - it('should handle OAuth errors gracefully throughout the flow', async () => { - // Test OIDC config failure - const failedFetch = createMockFetch([ - { - url: '/.well-known/openid-configuration', - response: { error: 'Not found' }, - status: 404 - } - ]); - global.fetch = failedFetch; - - await expect( - client.initOIDCConfig('https://invalid-oauth2.example.com/.well-known/openid-configuration') - ).rejects.toThrow('Failed to fetch OIDC config'); - - // Test token validation failure - const mockFetch = createMockFetch([ - { - url: '/.well-known/openid-configuration', - response: mockOIDCConfiguration - } - ]); - global.fetch = mockFetch; - - await client.initOIDCConfig( - 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' - ); - - vi.spyOn(client, 'validateAuthorizationCode').mockRejectedValue( - new Error('Invalid authorization code') - ); - - await expect( - client.validateAuthorizationCode(client.OIDCConfig!.token_endpoint, 'invalid-code', null) - ).rejects.toThrow('Invalid authorization code'); + it('refreshes and saves the updated tokens for a standard provider', async () => { + const refresh = vi.fn().mockResolvedValue({ + accessToken: () => 'new-access-token', + refreshToken: () => 'new-refresh-token', + idToken: () => 'new-id-token' + }); + const client = makeFakeClient(refresh); + await primeFakeIdp(client); - // Test token refresh failure const session = createMockSession({ - user: mockUser, oauth: { - access_token: mockAccessToken, - refresh_token: mockRefreshToken + provider: 'fake-idp', + access_token: 'old-access-token', + refresh_token: 'old-refresh-token', + id_token: 'old-id-token' } }); - vi.spyOn(client, 'refreshAccessToken').mockRejectedValue(new Error('Token refresh failed')); + await SessionOAuthHelper.refreshAccessToken(session as never); - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( - 'Failed to refresh access token. Please log in again.' - ); - }); - }); - - describe('OAuth Configuration Validation', () => { - it('should validate required OIDC endpoints', async () => { - const incompleteConfig = { - ...mockOIDCConfiguration - // Missing token_endpoint - }; - delete incompleteConfig.token_endpoint; - - const mockFetch = createMockFetch([ - { - url: '/.well-known/openid-configuration', - response: incompleteConfig - } - ]); - global.fetch = mockFetch; - - await expect( - client.initOIDCConfig( - 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' - ) - ).rejects.toThrow('Invalid OIDC config: Missing required endpoints.'); - }); - - it('should handle malformed OIDC responses', async () => { - const malformedConfig = { - issuer: 'https://test.example.com' - // Missing required fields - }; - - const mockFetch = createMockFetch([ - { - url: '/.well-known/openid-configuration', - response: malformedConfig - } - ]); - global.fetch = mockFetch; - - await expect( - client.initOIDCConfig( - 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' - ) - ).rejects.toThrow('Invalid OIDC config: Missing required endpoints.'); + expect(refresh).toHaveBeenCalledWith(TOKEN_ENDPOINT, 'old-refresh-token', ['openid']); + expect(session.data.oauth.access_token).toBe('new-access-token'); + expect(session.data.oauth.refresh_token).toBe('new-refresh-token'); + expect(session.data.oauth.id_token).toBe('new-id-token'); + expect(session.data.oauth.provider).toBe('fake-idp'); + expect(session.save).toHaveBeenCalled(); }); - }); - describe('Session Management Integration', () => { - it('should maintain session state throughout OAuth flow', async () => { - const session = createMockSession(); - - // Initial empty session - expect(session.data).toEqual({}); - - // After successful OAuth login - await session.setData({ - user: mockUser, - oauth: { - access_token: mockAccessToken, - refresh_token: mockRefreshToken - } + it('stores the id token as the access token for the google provider', async () => { + const refresh = vi.fn().mockResolvedValue({ + // Google access tokens are opaque; OBP verifies the id_token JWT instead. + accessToken: () => 'opaque-google-access-token', + refreshToken: () => 'new-refresh-token', + idToken: () => 'google-id-jwt' }); + const client = makeFakeClient(refresh); + primeGoogle(client); - expect(session.data.user).toEqual(mockUser); - expect(session.data.oauth.access_token).toBe(mockAccessToken); - expect(session.data.oauth.refresh_token).toBe(mockRefreshToken); - - // After token refresh - const newAccessToken = 'refreshed-token'; - session.data.oauth.access_token = newAccessToken; - - expect(session.data.oauth.access_token).toBe(newAccessToken); - expect(session.data.oauth.refresh_token).toBe(mockRefreshToken); // Should remain same - }); - - it('should handle session persistence across requests', async () => { const session = createMockSession({ - user: mockUser, oauth: { - access_token: mockAccessToken, - refresh_token: mockRefreshToken + provider: 'google', + access_token: 'old-access-token', + refresh_token: 'old-refresh-token' } }); - // Simulate session save and retrieve - await session.save(); - expect(session.save).toHaveBeenCalled(); + await SessionOAuthHelper.refreshAccessToken(session as never); - // Session data should persist - expect(session.data.user).toEqual(mockUser); - expect(session.data.oauth).toBeDefined(); + expect(refresh).toHaveBeenCalledWith(TOKEN_ENDPOINT, 'old-refresh-token', ['openid']); + expect(session.data.oauth.access_token).toBe('google-id-jwt'); + expect(session.data.oauth.id_token).toBe('google-id-jwt'); + expect(session.data.oauth.refresh_token).toBe('new-refresh-token'); + expect(session.save).toHaveBeenCalled(); }); - }); - describe('Token Lifecycle Management', () => { - beforeEach(async () => { - const mockFetch = createMockFetch([ - { - url: '/.well-known/openid-configuration', - response: mockOIDCConfiguration - } - ]); - global.fetch = mockFetch; - - await client.initOIDCConfig( - 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' - ); - }); + it('keeps the existing tokens when the refresh response omits them', async () => { + const refresh = vi.fn().mockResolvedValue({ + accessToken: () => 'rotated-access-token', + refreshToken: () => undefined, + idToken: () => undefined + }); + const client = makeFakeClient(refresh); + await primeFakeIdp(client); - it('should handle token expiration and renewal cycle', async () => { - // Create token with short expiration - const shortLivedToken = { - sub: 'test-user', - iss: 'https://test.example.com', - aud: ['test-client'], - exp: Math.floor(Date.now() / 1000) + 60, // Expires in 1 minute - iat: Math.floor(Date.now() / 1000) - }; - - const tokenString = - btoa(JSON.stringify({ alg: 'RS256' })) + - '.' + - btoa(JSON.stringify(shortLivedToken)) + - '.' + - 'signature'; - - // Check token is not yet expired - const isExpired = await client.checkAccessTokenExpiration(tokenString); - expect(isExpired).toBe(false); - - // Simulate time passing (token expires) - const expiredToken = { - ...shortLivedToken, - exp: Math.floor(Date.now() / 1000) - 60 // Expired 1 minute ago - }; - - const expiredTokenString = - btoa(JSON.stringify({ alg: 'RS256' })) + - '.' + - btoa(JSON.stringify(expiredToken)) + - '.' + - 'signature'; - - const isNowExpired = await client.checkAccessTokenExpiration(expiredTokenString); - expect(isNowExpired).toBe(true); - - // Should trigger token refresh const session = createMockSession({ - user: mockUser, oauth: { - access_token: expiredTokenString, - refresh_token: mockRefreshToken + provider: 'fake-idp', + access_token: 'old-access-token', + refresh_token: 'kept-refresh-token', + id_token: 'kept-id-token' } }); - const mockRefreshedTokens = { - accessToken: () => 'new-fresh-token', - refreshToken: () => 'new-refresh-token', - accessTokenExpiresAt: () => new Date(Date.now() + 3600000), - refreshTokenExpiresAt: () => new Date(Date.now() + 86400000), - scopes: () => ['openid'] - }; + await SessionOAuthHelper.refreshAccessToken(session as never); - vi.spyOn(client, 'refreshAccessToken').mockResolvedValue(mockRefreshedTokens); - - await refreshAccessTokenInSession(session, client); - - expect(session.data.oauth.access_token).toBe('new-fresh-token'); + expect(session.data.oauth.access_token).toBe('rotated-access-token'); + expect(session.data.oauth.refresh_token).toBe('kept-refresh-token'); + expect(session.data.oauth.id_token).toBe('kept-id-token'); + expect(session.save).toHaveBeenCalled(); }); - it('should handle concurrent token refresh requests', async () => { + it('throws and does not save when the client refresh fails', async () => { + const refresh = vi.fn().mockRejectedValue(new Error('token endpoint returned 400')); + const client = makeFakeClient(refresh); + await primeFakeIdp(client); + const session = createMockSession({ - user: mockUser, oauth: { - access_token: mockAccessToken, - refresh_token: mockRefreshToken + provider: 'fake-idp', + access_token: 'old-access-token', + refresh_token: 'old-refresh-token' } }); - const mockRefreshedTokens = { - accessToken: () => 'concurrently-refreshed-token', - refreshToken: () => 'new-refresh-token', - accessTokenExpiresAt: () => new Date(Date.now() + 3600000), - refreshTokenExpiresAt: () => new Date(Date.now() + 86400000), - scopes: () => ['openid'] - }; - - vi.spyOn(client, 'refreshAccessToken').mockResolvedValue(mockRefreshedTokens); - - // Simulate concurrent refresh requests - const refreshPromise1 = refreshAccessTokenInSession(session, client); - const refreshPromise2 = refreshAccessTokenInSession(session, client); - - await Promise.all([refreshPromise1, refreshPromise2]); - - // Both should complete successfully - expect(session.data.oauth.access_token).toBe('concurrently-refreshed-token'); - expect(client.refreshAccessToken).toHaveBeenCalledTimes(2); - }); - }); - - describe('Error Recovery Scenarios', () => { - it('should recover from network interruptions during OAuth flow', async () => { - // First attempt fails - const failingFetch = vi - .fn() - .mockRejectedValueOnce(new Error('Network error')) - .mockResolvedValueOnce({ - ok: true, - json: () => Promise.resolve(mockOIDCConfiguration) - }); - - global.fetch = failingFetch; - - // First attempt should fail - await expect( - client.initOIDCConfig( - 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' - ) - ).rejects.toThrow('Network error'); - - // Second attempt should succeed - await client.initOIDCConfig( - 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' + await expect(SessionOAuthHelper.refreshAccessToken(session as never)).rejects.toThrow( + 'Failed to refresh access token. Please log in again.' ); - expect(client.OIDCConfig).toEqual(mockOIDCConfiguration); + expect(session.data.oauth.access_token).toBe('old-access-token'); + expect(session.save).not.toHaveBeenCalled(); }); - it('should handle partial OAuth flow completion', async () => { - // Initialize client successfully - const mockFetch = createMockFetch([ - { - url: '/.well-known/openid-configuration', - response: mockOIDCConfiguration - } - ]); - global.fetch = mockFetch; - - await client.initOIDCConfig( - 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' - ); + it('throws when the session has no OAuth data', async () => { + const session = createMockSession({}); - // Token exchange fails - vi.spyOn(client, 'validateAuthorizationCode').mockRejectedValue( - new Error('Authorization server error') + await expect(SessionOAuthHelper.refreshAccessToken(session as never)).rejects.toThrow( + 'No valid OAuth data found in session. Please log in again.' ); - - await expect( - client.validateAuthorizationCode(client.OIDCConfig!.token_endpoint, 'code-123', null) - ).rejects.toThrow('Authorization server error'); - - // Client should still be usable for retry - expect(client.OIDCConfig).toEqual(mockOIDCConfiguration); - - // Retry with successful token exchange - const mockTokens = { - accessToken: () => mockAccessToken, - refreshToken: () => mockRefreshToken, - accessTokenExpiresAt: () => new Date(Date.now() + 3600000), - refreshTokenExpiresAt: () => new Date(Date.now() + 86400000), - scopes: () => ['openid'] - }; - - vi.spyOn(client, 'validateAuthorizationCode').mockResolvedValue(mockTokens); - - const tokens = await client.validateAuthorizationCode( - client.OIDCConfig!.token_endpoint, - 'code-456', - null - ); - - expect(tokens.accessToken()).toBe(mockAccessToken); }); }); }); diff --git a/apps/portal/test/setup.ts b/apps/portal/test/setup.ts index 63ca5e15..19cd2305 100644 --- a/apps/portal/test/setup.ts +++ b/apps/portal/test/setup.ts @@ -1,4 +1,4 @@ -import { vi } from 'vitest'; +import { afterEach, vi } from 'vitest'; import '@testing-library/jest-dom/vitest'; // Mock the SvelteKit modules that are commonly used in tests diff --git a/apps/portal/test/unit/oauth/client.test.ts b/apps/portal/test/unit/oauth/client.test.ts index 2af419be..28d45a51 100644 --- a/apps/portal/test/unit/oauth/client.test.ts +++ b/apps/portal/test/unit/oauth/client.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { OAuth2Client } from 'arctic'; import { OAuth2ClientWithConfig } from '$lib/oauth/client'; import { mockOIDCConfiguration, @@ -16,6 +17,12 @@ describe('OAuth2ClientWithConfig', () => { originalFetch = global.fetch; mockEnvVars(mockEnvironment); + // arctic's OAuth2Client is globally mocked to return a plain object, which + // would hijack `this` in the subclass constructor and strip the subclass's + // prototype methods. Reset it to a no-op constructor so `super()` leaves the + // real OAuth2ClientWithConfig instance intact. + vi.mocked(OAuth2Client).mockImplementation(function () {} as never); + client = new OAuth2ClientWithConfig( 'test-client-id', 'test-client-secret', @@ -58,7 +65,7 @@ describe('OAuth2ClientWithConfig', () => { ); }); - it('should throw error when fetch fails', async () => { + it('should not set OIDC config when fetch returns a non-ok response', async () => { const mockFetch = createMockFetch([ { url: '/.well-known/openid-configuration', @@ -68,12 +75,12 @@ describe('OAuth2ClientWithConfig', () => { ]); global.fetch = mockFetch; - await expect( - client.initOIDCConfig('https://invalid-url/.well-known/openid-configuration') - ).rejects.toThrow('Failed to fetch OIDC config: Error'); + await client.initOIDCConfig('https://invalid-url/.well-known/openid-configuration'); + + expect(client.OIDCConfig).toBeUndefined(); }); - it('should throw error when config is invalid (missing authorization_endpoint)', async () => { + it('should not set OIDC config when authorization_endpoint is missing', async () => { const invalidConfig = { ...mockOIDCConfiguration }; delete invalidConfig.authorization_endpoint; @@ -85,14 +92,14 @@ describe('OAuth2ClientWithConfig', () => { ]); global.fetch = mockFetch; - await expect( - client.initOIDCConfig( - 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' - ) - ).rejects.toThrow('Invalid OIDC config: Missing required endpoints.'); + await client.initOIDCConfig( + 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' + ); + + expect(client.OIDCConfig).toBeUndefined(); }); - it('should throw error when config is invalid (missing token_endpoint)', async () => { + it('should not set OIDC config when token_endpoint is missing', async () => { const invalidConfig = { ...mockOIDCConfiguration }; delete invalidConfig.token_endpoint; @@ -104,11 +111,11 @@ describe('OAuth2ClientWithConfig', () => { ]); global.fetch = mockFetch; - await expect( - client.initOIDCConfig( - 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' - ) - ).rejects.toThrow('Invalid OIDC config: Missing required endpoints.'); + await client.initOIDCConfig( + 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' + ); + + expect(client.OIDCConfig).toBeUndefined(); }); it('should handle network errors gracefully', async () => { @@ -138,10 +145,14 @@ describe('OAuth2ClientWithConfig', () => { ); expect(consoleSpy).toHaveBeenCalledWith( - 'OAuth2Client: Initializing OIDC configuration from OIDC Config URL:', + expect.stringContaining('[OAuth2Client]'), + 'Initializing OIDC configuration from OIDC Config URL:', 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' ); - expect(consoleSpy).toHaveBeenCalledWith('OAuth2Client: OIDC config initialization success.'); + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('[OAuth2Client]'), + 'OIDC config initialization success.' + ); consoleSpy.mockRestore(); }); @@ -164,7 +175,7 @@ describe('OAuth2ClientWithConfig', () => { expect(isExpired).toBe(true); }); - it('should return false for token without expiration', async () => { + it('should treat a token without expiration as expired (fail closed)', async () => { const tokenWithoutExp = createMockJWT({ ...mockAccessTokenPayload, exp: undefined @@ -172,20 +183,20 @@ describe('OAuth2ClientWithConfig', () => { const isExpired = await client.checkAccessTokenExpiration(tokenWithoutExp); - expect(isExpired).toBe(false); + expect(isExpired).toBe(true); }); - it('should throw error for invalid token format', async () => { + it('should treat an undecodable token as expired instead of throwing', async () => { const invalidToken = 'invalid.token.format'; - await expect(client.checkAccessTokenExpiration(invalidToken)).rejects.toThrow(); + await expect(client.checkAccessTokenExpiration(invalidToken)).resolves.toBe(true); }); - it('should handle malformed JWT payload', async () => { - // Create a JWT with invalid base64 payload + it('should treat a malformed JWT payload as expired instead of throwing', async () => { + // A JWT with an invalid base64 payload cannot be decoded. const invalidJWT = 'eyJhbGciOiJSUzI1NiJ9.invalid-payload.signature'; - await expect(client.checkAccessTokenExpiration(invalidJWT)).rejects.toThrow(); + await expect(client.checkAccessTokenExpiration(invalidJWT)).resolves.toBe(true); }); it('should log debug information', async () => { @@ -196,14 +207,18 @@ describe('OAuth2ClientWithConfig', () => { await client.checkAccessTokenExpiration(validToken); expect(consoleDebugSpy).toHaveBeenCalledWith( - 'OAuth2Client: Checking access token expiration...' + expect.stringContaining('[OAuth2Client]'), + 'Checking access token expiration...' + ); + expect(consoleDebugSpy).toHaveBeenCalledWith( + expect.stringContaining('[OAuth2Client]'), + 'Access token is valid.' ); - expect(consoleDebugSpy).toHaveBeenCalledWith('OAuth2Client: Access token is valid.'); consoleDebugSpy.mockRestore(); }); - it('should warn about invalid payload', async () => { + it('should warn about and fail closed on an invalid payload', async () => { const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); const tokenWithoutPayload = createMockJWT({}); @@ -211,9 +226,10 @@ describe('OAuth2ClientWithConfig', () => { const isExpired = await client.checkAccessTokenExpiration(tokenWithoutPayload); expect(consoleWarnSpy).toHaveBeenCalledWith( - 'OAuth2Client: Access token payload is invalid or missing expiration.' + expect.stringContaining('[OAuth2Client]'), + 'Access token payload is invalid or missing expiration.' ); - expect(isExpired).toBe(false); + expect(isExpired).toBe(true); consoleWarnSpy.mockRestore(); }); @@ -336,9 +352,7 @@ describe('OAuth2ClientWithConfig', () => { }); describe('error handling', () => { - it('should handle console errors gracefully', async () => { - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - + it('should wrap a fetch failure in a descriptive error', async () => { const mockFetch = vi.fn().mockRejectedValue(new Error('Network failure')); global.fetch = mockFetch; @@ -346,14 +360,7 @@ describe('OAuth2ClientWithConfig', () => { client.initOIDCConfig( 'https://test-oauth2.openbankproject.com/realms/obp-test/.well-known/openid-configuration' ) - ).rejects.toThrow('Network failure'); - - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'OAuth2Client: Error fetching OIDC config:', - expect.any(Error) - ); - - consoleErrorSpy.mockRestore(); + ).rejects.toThrow('Error fetching OIDC config'); }); it('should handle JSON parsing errors', async () => { diff --git a/apps/portal/test/unit/oauth/providerFactory.test.ts b/apps/portal/test/unit/oauth/providerFactory.test.ts new file mode 100644 index 00000000..e13228ee --- /dev/null +++ b/apps/portal/test/unit/oauth/providerFactory.test.ts @@ -0,0 +1,100 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { mockEnvironment } from '../../fixtures/oauth'; + +// providerFactory.ts reads $env/dynamic/private at import time; the server test +// setup does not mock $env, so provide it here. +vi.mock('$env/dynamic/private', () => ({ env: mockEnvironment })); + +import { OAuth2ProviderFactory } from '$lib/oauth/providerFactory'; +import type { OAuth2ClientWithConfig } from '$lib/oauth/client'; + +// A strategy the built-in keycloak/obp-oidc/google strategies never match, whose +// initialize() returns a client with a pre-baked OIDCConfig and performs no fetch. +function makeFakeStrategy(providerName: string, authEndpoint: string | undefined) { + return { + providerName, + supports: (provider: string) => provider === providerName, + getProviderName: () => providerName, + async initialize(): Promise { + return { + OIDCConfig: authEndpoint === undefined ? undefined : { authorization_endpoint: authEndpoint } + } as unknown as OAuth2ClientWithConfig; + } + }; +} + +async function factoryWith(...strategies: Array<{ provider: string; authEndpoint: string | undefined }>) { + const factory = new OAuth2ProviderFactory(); + for (const { provider, authEndpoint } of strategies) { + factory.registerStrategy(makeFakeStrategy(provider, authEndpoint) as never); + await factory.initializeProvider({ provider, url: 'https://well-known.example/config' }); + } + return factory; +} + +describe('OAuth2ProviderFactory trusted OIDC origins', () => { + let factory: OAuth2ProviderFactory; + + beforeEach(() => { + factory = new OAuth2ProviderFactory(); + }); + + it('has no trusted origins and trusts no return url when empty', () => { + expect(factory.getTrustedOidcOrigins().size).toBe(0); + expect(factory.isTrustedOidcReturnUrl('https://idp.example.com/auth')).toBe(false); + }); + + it('trusts a candidate that shares the configured provider origin', async () => { + const f = await factoryWith({ provider: 'fake-idp', authEndpoint: 'https://idp.example.com/auth' }); + expect(f.getTrustedOidcOrigins().has('https://idp.example.com')).toBe(true); + expect(f.isTrustedOidcReturnUrl('https://idp.example.com/auth')).toBe(true); + }); + + it('trusts the same origin with a different path and query', async () => { + const f = await factoryWith({ provider: 'fake-idp', authEndpoint: 'https://idp.example.com/auth' }); + expect(f.isTrustedOidcReturnUrl('https://idp.example.com/callback?state=abc')).toBe(true); + }); + + it('rejects a different port, scheme, or host', async () => { + const f = await factoryWith({ provider: 'fake-idp', authEndpoint: 'https://idp.example.com/auth' }); + expect(f.isTrustedOidcReturnUrl('https://idp.example.com:8443/auth')).toBe(false); + expect(f.isTrustedOidcReturnUrl('http://idp.example.com/auth')).toBe(false); + expect(f.isTrustedOidcReturnUrl('https://evil.com/auth')).toBe(false); + }); + + it('collects origins from every configured provider', async () => { + const f = await factoryWith( + { provider: 'fake-a', authEndpoint: 'https://a.example.com/auth' }, + { provider: 'fake-b', authEndpoint: 'https://b.example.com/auth' } + ); + const origins = f.getTrustedOidcOrigins(); + expect(origins.has('https://a.example.com')).toBe(true); + expect(origins.has('https://b.example.com')).toBe(true); + expect(origins.size).toBe(2); + }); + + it('deduplicates providers that share an origin', async () => { + const f = await factoryWith( + { provider: 'fake-a', authEndpoint: 'https://shared.example.com/auth' }, + { provider: 'fake-b', authEndpoint: 'https://shared.example.com/login' } + ); + expect(f.getTrustedOidcOrigins().size).toBe(1); + }); + + it('skips a client whose OIDCConfig is undefined without throwing', async () => { + const f = await factoryWith({ provider: 'fake-idp', authEndpoint: undefined }); + expect(f.getTrustedOidcOrigins().size).toBe(0); + expect(f.isTrustedOidcReturnUrl('https://idp.example.com/auth')).toBe(false); + }); + + it('skips a malformed authorization_endpoint without throwing', async () => { + const f = await factoryWith({ provider: 'fake-idp', authEndpoint: 'not a url' }); + expect(f.getTrustedOidcOrigins().size).toBe(0); + }); + + it('returns false for garbage candidate urls', async () => { + const f = await factoryWith({ provider: 'fake-idp', authEndpoint: 'https://idp.example.com/auth' }); + expect(f.isTrustedOidcReturnUrl('::::')).toBe(false); + expect(f.isTrustedOidcReturnUrl('')).toBe(false); + }); +}); diff --git a/apps/portal/test/unit/oauth/session.test.ts b/apps/portal/test/unit/oauth/session.test.ts index 6cc1d79e..0b7935b3 100644 --- a/apps/portal/test/unit/oauth/session.test.ts +++ b/apps/portal/test/unit/oauth/session.test.ts @@ -1,346 +1,328 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { refreshAccessTokenInSession } from '$lib/oauth/session'; -import { OAuth2ClientWithConfig } from '$lib/oauth/client'; -import { - mockOIDCConfiguration, - mockOAuth2Tokens, - mockAccessToken, - mockRefreshToken, - mockEnvironment -} from '../../fixtures/oauth'; -import { createMockSession, createMockFetch, mockEnvVars, restoreAllMocks } from '../../helpers'; - -describe('OAuth Session Utilities', () => { - let client: OAuth2ClientWithConfig; - let session: any; - let originalFetch: typeof global.fetch; - - beforeEach(() => { - originalFetch = global.fetch; - mockEnvVars(mockEnvironment); - - client = new OAuth2ClientWithConfig( - 'test-client-id', - 'test-client-secret', - 'http://localhost:3000/callback' - ); - - client.OIDCConfig = mockOIDCConfiguration; - - session = createMockSession({ - user: { - user_id: 'test-user-123', - email: 'test@example.com' - }, - oauth: { - access_token: mockAccessToken, - refresh_token: mockRefreshToken - } - }); - }); - +import { mockEnvironment, mockAccessToken, mockRefreshToken } from '../../fixtures/oauth'; +import { createMockSession, restoreAllMocks } from '../../helpers'; + +// sessionHelper -> providerFactory reads $env/dynamic/private at import time; the +// server test setup does not mock $env, so provide it here before importing units. +vi.mock('$env/dynamic/private', () => ({ env: mockEnvironment })); + +import { SessionOAuthHelper } from '$lib/oauth/sessionHelper'; +import { oauth2ProviderFactory } from '$lib/oauth/providerFactory'; +import type { OAuth2ClientWithConfig } from '$lib/oauth/client'; + +// The refresh path resolves the client from oauth2ProviderFactory.getClient(provider). +// Register a hand-built client directly into the singleton factory so we fully control +// its OIDCConfig and refreshAccessToken behaviour, with no network calls. +const TOKEN_ENDPOINT = + 'https://test-oauth2.openbankproject.com/realms/obp-test/protocol/openid-connect/token'; + +interface FakeTokens { + accessToken: () => string; + refreshToken: () => string | null; + idToken: () => string | undefined; +} + +function registerFakeClient( + provider: string, + options: { + tokenEndpoint?: string | undefined; + omitOIDCConfig?: boolean; + refresh?: () => Promise; + } = {} +): { client: OAuth2ClientWithConfig; refreshAccessToken: ReturnType } { + const refreshAccessToken = vi.fn( + options.refresh ?? (async () => makeTokens('unused', null, undefined)) + ); + + const client = { + OIDCConfig: options.omitOIDCConfig + ? undefined + : { token_endpoint: options.tokenEndpoint }, + refreshAccessToken + } as unknown as OAuth2ClientWithConfig; + + // getClient(provider) simply reads the private initializedClients map by key. + (oauth2ProviderFactory as unknown as { initializedClients: Map }) + .initializedClients.set(provider, client); + + return { client, refreshAccessToken }; +} + +function makeTokens( + accessToken: string, + refreshToken: string | null, + idToken: string | undefined +): FakeTokens { + return { + accessToken: () => accessToken, + refreshToken: () => refreshToken, + idToken: () => idToken + }; +} + +function clearFactoryClients() { + (oauth2ProviderFactory as unknown as { initializedClients: Map }) + .initializedClients.clear(); +} + +describe('SessionOAuthHelper', () => { afterEach(() => { - global.fetch = originalFetch; + clearFactoryClients(); restoreAllMocks(); }); - describe('refreshAccessTokenInSession', () => { - beforeEach(() => { - // Mock the inherited refreshAccessToken method from Arctic OAuth2Client - Object.setPrototypeOf(client, { - ...Object.getPrototypeOf(client), - refreshAccessToken: vi.fn() + describe('getSessionOAuth', () => { + it('returns null when session oauth has no provider', () => { + const session = createMockSession({ + oauth: { access_token: mockAccessToken, refresh_token: mockRefreshToken } }); - }); - - it('should successfully refresh access token', async () => { - const newAccessToken = 'new-access-token-123'; - const newRefreshToken = 'new-refresh-token-123'; - - // Mock the refreshAccessToken method - const mockRefreshTokens = { - accessToken: () => newAccessToken, - refreshToken: () => newRefreshToken, - accessTokenExpiresAt: () => new Date(Date.now() + 3600000), - refreshTokenExpiresAt: () => new Date(Date.now() + 86400000), - scopes: () => ['openid'] - }; - - vi.spyOn(client, 'refreshAccessToken').mockResolvedValue(mockRefreshTokens); - await refreshAccessTokenInSession(session, client); - - expect(client.refreshAccessToken).toHaveBeenCalledWith( - mockOIDCConfiguration.token_endpoint, - mockRefreshToken, - ['openid'] - ); + expect(SessionOAuthHelper.getSessionOAuth(session as never)).toBeNull(); + }); - expect(session.data.oauth).toEqual({ - access_token: newAccessToken, - refresh_token: newRefreshToken + it('returns null when session oauth has no access_token', () => { + const session = createMockSession({ + oauth: { provider: 'test-idp', refresh_token: mockRefreshToken } }); - expect(session.save).toHaveBeenCalled(); + expect(SessionOAuthHelper.getSessionOAuth(session as never)).toBeNull(); }); - it('should keep existing refresh token if new one is not provided', async () => { - const newAccessToken = 'new-access-token-123'; - - // Mock the refreshAccessToken method without refresh token - const mockRefreshTokens = { - accessToken: () => newAccessToken, - refreshToken: () => null, // No new refresh token - accessTokenExpiresAt: () => new Date(Date.now() + 3600000), - refreshTokenExpiresAt: () => new Date(Date.now() + 86400000), - scopes: () => ['openid'] - }; - - vi.spyOn(client, 'refreshAccessToken').mockResolvedValue(mockRefreshTokens); - - await refreshAccessTokenInSession(session, client); - - expect(session.data.oauth).toEqual({ - access_token: newAccessToken, - refresh_token: mockRefreshToken // Keep original refresh token + it('returns null when no client is registered for the provider', () => { + const session = createMockSession({ + oauth: { provider: 'unknown-idp', access_token: mockAccessToken } }); - expect(session.save).toHaveBeenCalled(); + expect(SessionOAuthHelper.getSessionOAuth(session as never)).toBeNull(); }); - it('should throw error when no refresh endpoint is available', async () => { - // Remove token endpoint from OIDC config - client.OIDCConfig = { ...mockOIDCConfiguration }; - delete client.OIDCConfig.token_endpoint; + it('returns client and tokens for a valid session', () => { + const { client } = registerFakeClient('test-idp', { tokenEndpoint: TOKEN_ENDPOINT }); + const session = createMockSession({ + oauth: { + provider: 'test-idp', + access_token: mockAccessToken, + refresh_token: mockRefreshToken, + id_token: 'id-token-123' + } + }); - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( - 'No refresh endpoint or refresh token found. Please log in again.' - ); + const result = SessionOAuthHelper.getSessionOAuth(session as never); - expect(session.save).not.toHaveBeenCalled(); + expect(result).toEqual({ + client, + provider: 'test-idp', + accessToken: mockAccessToken, + refreshToken: mockRefreshToken, + idToken: 'id-token-123' + }); }); + }); - it('should throw error when no refresh token is available', async () => { - // Remove refresh token from session - session.data.oauth = { - access_token: mockAccessToken - // No refresh_token - }; + describe('updateTokensInSession', () => { + it('throws when the session has no oauth data to update', async () => { + const session = createMockSession({ user: { user_id: 'u1' } }); - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( - 'No refresh endpoint or refresh token found. Please log in again.' - ); + await expect( + SessionOAuthHelper.updateTokensInSession(session as never, 'new-access') + ).rejects.toThrow('No OAuth data in session to update.'); expect(session.save).not.toHaveBeenCalled(); }); - it('should throw error when no oauth data exists in session', async () => { - // Remove oauth data from session - session.data = { - user: { - user_id: 'test-user-123', - email: 'test@example.com' + it('updates the access token and saves the session', async () => { + const session = createMockSession({ + oauth: { + provider: 'test-idp', + access_token: mockAccessToken, + refresh_token: mockRefreshToken, + id_token: 'old-id' } - // No oauth data - }; - - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( - 'No refresh endpoint or refresh token found. Please log in again.' - ); - - expect(session.save).not.toHaveBeenCalled(); - }); - - it('should throw error when refresh token request fails', async () => { - const refreshError = new Error('Token refresh failed'); - vi.spyOn(client, 'refreshAccessToken').mockRejectedValue(refreshError); + }); - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( - 'Failed to refresh access token. Please log in again.' + await SessionOAuthHelper.updateTokensInSession( + session as never, + 'new-access', + 'new-refresh', + 'new-id' ); - expect(session.save).not.toHaveBeenCalled(); + expect(session.data.oauth).toEqual({ + provider: 'test-idp', + access_token: 'new-access', + refresh_token: 'new-refresh', + id_token: 'new-id' + }); + expect(session.save).toHaveBeenCalled(); }); - it('should log debug information during refresh process', async () => { - const consoleDebugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {}); - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - - const newAccessToken = 'new-access-token-123'; - const mockRefreshTokens = { - accessToken: () => newAccessToken, - refreshToken: () => 'new-refresh-token', - accessTokenExpiresAt: () => new Date(Date.now() + 3600000), - refreshTokenExpiresAt: () => new Date(Date.now() + 86400000), - scopes: () => ['openid'] - }; - - vi.spyOn(client, 'refreshAccessToken').mockResolvedValue(mockRefreshTokens); - - await refreshAccessTokenInSession(session, client); + it('keeps the existing refresh and id tokens when new ones are not provided', async () => { + const session = createMockSession({ + oauth: { + provider: 'test-idp', + access_token: mockAccessToken, + refresh_token: mockRefreshToken, + id_token: 'old-id' + } + }); - expect(consoleDebugSpy).toHaveBeenCalledWith('Attempting to refresh access token...'); - expect(consoleDebugSpy).toHaveBeenCalledWith('Refreshing access token...'); - expect(consoleErrorSpy).not.toHaveBeenCalled(); + await SessionOAuthHelper.updateTokensInSession(session as never, 'new-access'); - consoleDebugSpy.mockRestore(); - consoleErrorSpy.mockRestore(); + expect(session.data.oauth).toEqual({ + provider: 'test-idp', + access_token: 'new-access', + refresh_token: mockRefreshToken, + id_token: 'old-id' + }); + expect(session.save).toHaveBeenCalled(); }); + }); - it('should log error when refresh fails', async () => { - const consoleDebugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {}); - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); - - // Remove refresh endpoint - client.OIDCConfig = { ...mockOIDCConfiguration }; - delete client.OIDCConfig.token_endpoint; - - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow(); + describe('refreshAccessToken', () => { + it('refreshes the access token, stores the new tokens and saves', async () => { + const { refreshAccessToken } = registerFakeClient('test-idp', { + tokenEndpoint: TOKEN_ENDPOINT, + refresh: async () => makeTokens('new-access-token', 'new-refresh-token', 'new-id-token') + }); + const session = createMockSession({ + oauth: { + provider: 'test-idp', + access_token: mockAccessToken, + refresh_token: mockRefreshToken + } + }); - expect(consoleDebugSpy).toHaveBeenCalledWith('Attempting to refresh access token...'); - expect(consoleWarnSpy).toHaveBeenCalledWith( - 'No refresh endpoint or refresh token found. Redirecting to login.' - ); + await SessionOAuthHelper.refreshAccessToken(session as never); - consoleDebugSpy.mockRestore(); - consoleErrorSpy.mockRestore(); - consoleWarnSpy.mockRestore(); + expect(refreshAccessToken).toHaveBeenCalledWith(TOKEN_ENDPOINT, mockRefreshToken, ['openid']); + expect(session.data.oauth).toEqual({ + provider: 'test-idp', + access_token: 'new-access-token', + refresh_token: 'new-refresh-token', + id_token: 'new-id-token' + }); + expect(session.save).toHaveBeenCalled(); }); - it('should log error when token refresh API call fails', async () => { - const consoleDebugSpy = vi.spyOn(console, 'debug').mockImplementation(() => {}); - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - - const refreshError = new Error('API call failed'); - vi.spyOn(client, 'refreshAccessToken').mockRejectedValue(refreshError); - - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( - 'Failed to refresh access token. Please log in again.' - ); + it('keeps the existing refresh token when the refresh response omits one', async () => { + registerFakeClient('test-idp', { + tokenEndpoint: TOKEN_ENDPOINT, + refresh: async () => makeTokens('new-access-token', null, 'new-id-token') + }); + const session = createMockSession({ + oauth: { + provider: 'test-idp', + access_token: mockAccessToken, + refresh_token: mockRefreshToken + } + }); - expect(consoleDebugSpy).toHaveBeenCalledWith('Attempting to refresh access token...'); - expect(consoleDebugSpy).toHaveBeenCalledWith('Refreshing access token...'); - expect(consoleErrorSpy).toHaveBeenCalledWith('Error refreshing access token:', refreshError); + await SessionOAuthHelper.refreshAccessToken(session as never); - consoleDebugSpy.mockRestore(); - consoleErrorSpy.mockRestore(); + expect(session.data.oauth.refresh_token).toBe(mockRefreshToken); + expect(session.data.oauth.access_token).toBe('new-access-token'); }); - it('should handle session save failure gracefully', async () => { - const saveError = new Error('Session save failed'); - session.save.mockRejectedValue(saveError); - - const newAccessToken = 'new-access-token-123'; - const mockRefreshTokens = { - accessToken: () => newAccessToken, - refreshToken: () => 'new-refresh-token', - accessTokenExpiresAt: () => new Date(Date.now() + 3600000), - refreshTokenExpiresAt: () => new Date(Date.now() + 86400000), - scopes: () => ['openid'] - }; - - vi.spyOn(client, 'refreshAccessToken').mockResolvedValue(mockRefreshTokens); - - // The function should still complete the refresh and only fail on save - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( - 'Failed to refresh access token. Please log in again.' - ); - - // Verify that the session data was updated even though save failed - expect(session.data.oauth.access_token).toBe(newAccessToken); - }); + it('stores the id token as the access token for the google provider', async () => { + registerFakeClient('google', { + tokenEndpoint: TOKEN_ENDPOINT, + refresh: async () => makeTokens('opaque-google-access', 'new-refresh-token', 'google-id-token') + }); + const session = createMockSession({ + oauth: { + provider: 'google', + access_token: mockAccessToken, + refresh_token: mockRefreshToken + } + }); - it('should work with minimal OIDC configuration', async () => { - // Use minimal OIDC config with just token endpoint - client.OIDCConfig = { - issuer: 'https://test.example.com', - authorization_endpoint: 'https://test.example.com/auth', - token_endpoint: 'https://test.example.com/token', - userinfo_endpoint: 'https://test.example.com/userinfo', - jwks_uri: 'https://test.example.com/certs', - response_types_supported: ['code'], - grant_types_supported: ['authorization_code'], - subject_types_supported: ['public'], - id_token_signing_alg_values_supported: ['RS256'] - }; - - const newAccessToken = 'new-access-token-123'; - const mockRefreshTokens = { - accessToken: () => newAccessToken, - refreshToken: () => 'new-refresh-token', - accessTokenExpiresAt: () => new Date(Date.now() + 3600000), - refreshTokenExpiresAt: () => new Date(Date.now() + 86400000), - scopes: () => ['openid'] - }; - - vi.spyOn(client, 'refreshAccessToken').mockResolvedValue(mockRefreshTokens); - - await refreshAccessTokenInSession(session, client); - - expect(client.refreshAccessToken).toHaveBeenCalledWith( - 'https://test.example.com/token', - mockRefreshToken, - ['openid'] - ); + await SessionOAuthHelper.refreshAccessToken(session as never); - expect(session.data.oauth.access_token).toBe(newAccessToken); + // Google access tokens are opaque; the id_token is what OBP verifies, so it is + // stored as the session access token. + expect(session.data.oauth.access_token).toBe('google-id-token'); + expect(session.data.oauth.id_token).toBe('google-id-token'); expect(session.save).toHaveBeenCalled(); }); - }); - describe('edge cases', () => { - it('should handle undefined session data', async () => { - session.data = undefined; + it('throws when there is no valid OAuth data in the session', async () => { + const session = createMockSession({ user: { user_id: 'u1' } }); - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( - 'Cannot read properties of undefined' - ); - }); - - it('should handle null session data', async () => { - session.data = null; - - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( - 'Cannot read properties of null' + await expect(SessionOAuthHelper.refreshAccessToken(session as never)).rejects.toThrow( + 'No valid OAuth data found in session. Please log in again.' ); + expect(session.save).not.toHaveBeenCalled(); }); - it('should handle empty session data', async () => { - session.data = {}; + it('throws when the client is registered but has no token endpoint', async () => { + const { refreshAccessToken } = registerFakeClient('test-idp', { + tokenEndpoint: undefined + }); + const session = createMockSession({ + oauth: { + provider: 'test-idp', + access_token: mockAccessToken, + refresh_token: mockRefreshToken + } + }); - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( + await expect(SessionOAuthHelper.refreshAccessToken(session as never)).rejects.toThrow( 'No refresh endpoint or refresh token found. Please log in again.' ); + expect(refreshAccessToken).not.toHaveBeenCalled(); + expect(session.save).not.toHaveBeenCalled(); }); - it('should handle session with oauth but no refresh token', async () => { - session.data = { + it('throws when the client has no OIDC config at all', async () => { + registerFakeClient('test-idp', { omitOIDCConfig: true }); + const session = createMockSession({ oauth: { - access_token: mockAccessToken - // Missing refresh_token + provider: 'test-idp', + access_token: mockAccessToken, + refresh_token: mockRefreshToken } - }; + }); - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( + await expect(SessionOAuthHelper.refreshAccessToken(session as never)).rejects.toThrow( 'No refresh endpoint or refresh token found. Please log in again.' ); + expect(session.save).not.toHaveBeenCalled(); }); - it('should handle empty refresh token', async () => { - session.data.oauth.refresh_token = ''; + it('throws when the session has no refresh token', async () => { + registerFakeClient('test-idp', { tokenEndpoint: TOKEN_ENDPOINT }); + const session = createMockSession({ + oauth: { + provider: 'test-idp', + access_token: mockAccessToken + } + }); - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( + await expect(SessionOAuthHelper.refreshAccessToken(session as never)).rejects.toThrow( 'No refresh endpoint or refresh token found. Please log in again.' ); + expect(session.save).not.toHaveBeenCalled(); }); - it('should handle client without OIDC config', async () => { - client.OIDCConfig = undefined; + it('throws a generic error when the token refresh request rejects', async () => { + registerFakeClient('test-idp', { + tokenEndpoint: TOKEN_ENDPOINT, + refresh: async () => { + throw new Error('Token refresh failed at the IdP'); + } + }); + const session = createMockSession({ + oauth: { + provider: 'test-idp', + access_token: mockAccessToken, + refresh_token: mockRefreshToken + } + }); - await expect(refreshAccessTokenInSession(session, client)).rejects.toThrow( - 'No refresh endpoint or refresh token found. Please log in again.' + await expect(SessionOAuthHelper.refreshAccessToken(session as never)).rejects.toThrow( + 'Failed to refresh access token. Please log in again.' ); + expect(session.save).not.toHaveBeenCalled(); }); }); }); diff --git a/apps/portal/test/unit/routes/login.server.test.ts b/apps/portal/test/unit/routes/login.server.test.ts index e0927627..52684f95 100644 --- a/apps/portal/test/unit/routes/login.server.test.ts +++ b/apps/portal/test/unit/routes/login.server.test.ts @@ -1,605 +1,101 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { GET } from '$lib/../routes/login/obp/+server'; -import { GET as CallbackGET } from '$lib/../routes/login/obp/callback/+server'; -import { generateState } from 'arctic'; -import { - mockOIDCConfiguration, - mockUser, - mockAccessToken, - mockRefreshToken, - mockEnvironment -} from '../../fixtures/oauth'; -import { createMockRequestEvent, createMockFetch, mockEnvVars, restoreAllMocks } from '../../helpers'; - -// Mock the oauth client module -vi.mock('$lib/oauth/client', () => ({ - obp_oauth: { - OIDCConfig: mockOIDCConfiguration, - createAuthorizationURL: vi.fn() +import { isRedirect, isHttpError } from '@sveltejs/kit'; +import { mockEnvironment } from '../../fixtures/oauth'; +import { createMockRequestEvent, mockEnvVars, restoreAllMocks } from '../../helpers'; + +// The route depends on the provider factory singleton. Mock the whole module so +// `getFirstAvailableProvider` is a vi.fn() we control; this also avoids pulling +// in the real factory's `$env/dynamic/private` import at module load time. +vi.mock('$lib/oauth/providerFactory', () => ({ + oauth2ProviderFactory: { + getFirstAvailableProvider: vi.fn() } })); -// Mock the generateState function -vi.mock('arctic', () => ({ - generateState: vi.fn() -})); - -// Mock environment variables -vi.mock('$env/dynamic/public', () => ({ - env: mockEnvironment -})); - -import { obp_oauth } from '$lib/oauth/client'; +import { oauth2ProviderFactory } from '$lib/oauth/providerFactory'; +import { GET } from '$lib/../routes/login/obp/+server'; describe('Login Server Routes', () => { - let originalFetch: typeof global.fetch; - beforeEach(() => { - originalFetch = global.fetch; mockEnvVars(mockEnvironment); vi.clearAllMocks(); }); afterEach(() => { - global.fetch = originalFetch; restoreAllMocks(); }); describe('/login/obp/+server.ts - GET', () => { - it('should redirect to OAuth authorization URL', () => { - const mockState = 'test-state-123'; - const mockAuthURL = new URL('https://auth.example.com/authorize?state=test-state-123&client_id=test-client'); - - vi.mocked(generateState).mockReturnValue(mockState); - vi.mocked(obp_oauth.createAuthorizationURL).mockReturnValue(mockAuthURL); + it('redirects to the generic provider route when a provider is available', () => { + vi.mocked(oauth2ProviderFactory.getFirstAvailableProvider).mockReturnValue('obp-oidc'); const event = createMockRequestEvent(); - const response = GET(event); - - expect(generateState).toHaveBeenCalled(); - expect(obp_oauth.createAuthorizationURL).toHaveBeenCalledWith( - mockOIDCConfiguration.authorization_endpoint, - mockState, - ['openid'] - ); + let thrown: unknown; + try { + GET(event); + expect.fail('GET should have thrown a redirect'); + } catch (e) { + thrown = e; + } - expect(response.status).toBe(302); - expect(response.headers.get('Location')).toBe(mockAuthURL.toString()); - - expect(event.cookies.set).toHaveBeenCalledWith('obp_oauth_state', mockState, { - httpOnly: true, - maxAge: 600, // 10 minutes - secure: false, // import.meta.env.PROD would be false in tests - path: '/', - sameSite: 'lax' - }); + expect(isRedirect(thrown)).toBe(true); + const redirect = thrown as { status: number; location: string }; + expect(redirect.status).toBe(302); + expect(redirect.location).toBe('/login/obp-oidc'); }); - it('should handle missing authorization endpoint', () => { - const mockState = 'test-state-123'; - vi.mocked(generateState).mockReturnValue(mockState); - - // Mock oauth client with missing authorization endpoint - vi.mocked(obp_oauth).OIDCConfig = { - ...mockOIDCConfiguration, - authorization_endpoint: undefined as any - }; + it('redirects using whichever provider key the factory returns', () => { + vi.mocked(oauth2ProviderFactory.getFirstAvailableProvider).mockReturnValue('keycloak'); const event = createMockRequestEvent(); - const response = GET(event); + let thrown: unknown; + try { + GET(event); + expect.fail('GET should have thrown a redirect'); + } catch (e) { + thrown = e; + } - expect(response.status).toBe(500); - expect(response.statusText).toBe('Internal Server Error'); + expect(isRedirect(thrown)).toBe(true); + const redirect = thrown as { status: number; location: string }; + expect(redirect.status).toBe(302); + expect(redirect.location).toBe('/login/keycloak'); }); - it('should handle oauth client errors', () => { - const mockState = 'test-state-123'; - vi.mocked(generateState).mockReturnValue(mockState); - vi.mocked(obp_oauth.createAuthorizationURL).mockImplementation(() => { - throw new Error('OAuth configuration error'); - }); - - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + it('throws a 500 error when no provider is configured', () => { + vi.mocked(oauth2ProviderFactory.getFirstAvailableProvider).mockReturnValue(null); const event = createMockRequestEvent(); - const response = GET(event); - - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'Error during OBP OAuth login:', - expect.any(Error) - ); - expect(response.status).toBe(500); - - consoleErrorSpy.mockRestore(); - }); - - it('should set secure cookie in production', () => { - const mockState = 'test-state-123'; - const mockAuthURL = new URL('https://auth.example.com/authorize'); - - vi.mocked(generateState).mockReturnValue(mockState); - vi.mocked(obp_oauth.createAuthorizationURL).mockReturnValue(mockAuthURL); - - // Mock production environment - const originalEnv = import.meta.env; - Object.defineProperty(import.meta, 'env', { - value: { ...originalEnv, PROD: true }, - writable: true - }); - - const event = createMockRequestEvent(); - - const response = GET(event); - - expect(event.cookies.set).toHaveBeenCalledWith('obp_oauth_state', mockState, { - httpOnly: true, - maxAge: 600, - secure: true, // Should be true in production - path: '/', - sameSite: 'lax' - }); - - // Restore original env - Object.defineProperty(import.meta, 'env', { - value: originalEnv, - writable: true - }); - }); - }); - - describe('/login/obp/callback/+server.ts - GET', () => { - beforeEach(() => { - // Reset oauth client mock - vi.mocked(obp_oauth).OIDCConfig = mockOIDCConfiguration; - vi.mocked(obp_oauth).validateAuthorizationCode = vi.fn(); - }); - - it('should successfully handle OAuth callback and redirect to home', async () => { - const mockCode = 'auth-code-123'; - const mockState = 'state-123'; - const mockTokens = { - accessToken: () => mockAccessToken, - refreshToken: () => mockRefreshToken - }; - - // Mock successful token exchange - vi.mocked(obp_oauth.validateAuthorizationCode).mockResolvedValue(mockTokens); - - // Mock successful user fetch - const mockFetch = createMockFetch([ - { - url: '/obp/v5.1.0/users/current', - response: mockUser - } - ]); - global.fetch = mockFetch; - - const event = createMockRequestEvent({ - url: new URL(`http://localhost:3000/login/obp/callback?code=${mockCode}&state=${mockState}`), - cookies: { - get: vi.fn().mockReturnValue(mockState), - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - expect(obp_oauth.validateAuthorizationCode).toHaveBeenCalledWith( - mockOIDCConfiguration.token_endpoint, - mockCode, - null - ); - - expect(mockFetch).toHaveBeenCalledWith( - expect.objectContaining({ - url: `${mockEnvironment.PUBLIC_OBP_BASE_URL}/obp/v5.1.0/users/current` - }) - ); - - expect(event.locals.session.setData).toHaveBeenCalledWith({ - user: mockUser, - oauth: { - access_token: mockAccessToken, - refresh_token: mockRefreshToken - } - }); - - expect(event.locals.session.save).toHaveBeenCalled(); - expect(response.status).toBe(302); - expect(response.headers.get('Location')).toBe('/'); - }); - - it('should handle OAuth error response with invalid_grant', async () => { - const mockState = 'state-123:obp-oidc'; - const event = createMockRequestEvent({ - url: new URL('http://localhost:3000/login/obp/callback?error=invalid_grant&error_description=Invalid+username+or+password&state=' + mockState), - cookies: { - get: vi.fn().mockReturnValue(mockState), - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - expect(event.cookies.delete).toHaveBeenCalledWith('obp_oauth_state', { path: '/' }); - expect(response.status).toBe(302); - expect(response.headers.get('Location')).toContain('/login?error='); - expect(decodeURIComponent(response.headers.get('Location') || '')).toContain('Invalid username or password'); - }); - - it('should handle OAuth error response with access_denied', async () => { - const mockState = 'state-123:obp-oidc'; - const event = createMockRequestEvent({ - url: new URL('http://localhost:3000/login/obp/callback?error=access_denied&error_description=User+denied+access&state=' + mockState), - cookies: { - get: vi.fn().mockReturnValue(mockState), - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - expect(event.cookies.delete).toHaveBeenCalledWith('obp_oauth_state', { path: '/' }); - expect(response.status).toBe(302); - expect(response.headers.get('Location')).toContain('/login?error='); - expect(decodeURIComponent(response.headers.get('Location') || '')).toContain('Access was denied'); - }); - - it('should handle OAuth error response with custom error_description', async () => { - const mockState = 'state-123:obp-oidc'; - const event = createMockRequestEvent({ - url: new URL('http://localhost:3000/login/obp/callback?error=server_error&error_description=Something+went+wrong&state=' + mockState), - cookies: { - get: vi.fn().mockReturnValue(mockState), - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - expect(event.cookies.delete).toHaveBeenCalledWith('obp_oauth_state', { path: '/' }); - expect(response.status).toBe(302); - expect(response.headers.get('Location')).toContain('/login?error='); - expect(decodeURIComponent(response.headers.get('Location') || '')).toContain('Something went wrong'); - }); - - it('should return 400 when required parameters are missing', async () => { - const event = createMockRequestEvent({ - url: new URL('http://localhost:3000/login/obp/callback'), // No code or state - cookies: { - get: vi.fn().mockReturnValue('stored-state'), - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - expect(response.status).toBe(400); - expect(await response.text()).toBe('Please restart the process.'); - }); - - it('should return 400 when stored state is missing', async () => { - const event = createMockRequestEvent({ - url: new URL('http://localhost:3000/login/obp/callback?code=test-code&state=test-state'), - cookies: { - get: vi.fn().mockReturnValue(null), // No stored state - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - expect(response.status).toBe(400); - expect(await response.text()).toBe('Please restart the process.'); - }); - - it('should return 400 when state does not match', async () => { - const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); - - const event = createMockRequestEvent({ - url: new URL('http://localhost:3000/login/obp/callback?code=test-code&state=different-state'), - cookies: { - get: vi.fn().mockReturnValue('stored-state'), // Different state - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - expect(consoleLogSpy).toHaveBeenCalledWith('State mismatch:', 'stored-state', 'different-state'); - expect(response.status).toBe(400); - expect(await response.text()).toBe('Please restart the process.'); + let thrown: unknown; + try { + GET(event); + expect.fail('GET should have thrown an error'); + } catch (e) { + thrown = e; + } - consoleLogSpy.mockRestore(); + expect(isRedirect(thrown)).toBe(false); + expect(isHttpError(thrown)).toBe(true); + const httpError = thrown as { status: number; body: { message: string } }; + expect(httpError.status).toBe(500); + expect(httpError.body.message).toBe('OAuth provider not configured'); }); - it('should handle token validation failure', async () => { - const mockCode = 'auth-code-123'; - const mockState = 'state-123'; - - vi.mocked(obp_oauth.validateAuthorizationCode).mockRejectedValue( - new Error('Invalid authorization code') - ); - - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - - const event = createMockRequestEvent({ - url: new URL(`http://localhost:3000/login/obp/callback?code=${mockCode}&state=${mockState}`), - cookies: { - get: vi.fn().mockReturnValue(mockState), - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'Error validating authorization code:', - expect.any(Error) - ); - expect(response.status).toBe(400); - expect(await response.text()).toBe('Log in failed, please restart the process.'); - - consoleErrorSpy.mockRestore(); - }); - - it('should handle missing token endpoint', async () => { - const mockCode = 'auth-code-123'; - const mockState = 'state-123'; - - // Mock oauth client with missing token endpoint - vi.mocked(obp_oauth).OIDCConfig = { - ...mockOIDCConfiguration, - token_endpoint: undefined as any - }; - - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - - const event = createMockRequestEvent({ - url: new URL(`http://localhost:3000/login/obp/callback?code=${mockCode}&state=${mockState}`), - cookies: { - get: vi.fn().mockReturnValue(mockState), - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'Error validating authorization code:', - expect.any(Error) - ); - expect(response.status).toBe(400); - - consoleErrorSpy.mockRestore(); - }); - - it('should handle user fetch failure', async () => { - const mockCode = 'auth-code-123'; - const mockState = 'state-123'; - const mockTokens = { - accessToken: () => mockAccessToken, - refreshToken: () => mockRefreshToken - }; - - vi.mocked(obp_oauth.validateAuthorizationCode).mockResolvedValue(mockTokens); - - // Mock failed user fetch - const mockFetch = createMockFetch([ - { - url: '/obp/v5.1.0/users/current', - response: { error: 'Unauthorized' }, - status: 401 - } - ]); - global.fetch = mockFetch; - - const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - - const event = createMockRequestEvent({ - url: new URL(`http://localhost:3000/login/obp/callback?code=${mockCode}&state=${mockState}`), - cookies: { - get: vi.fn().mockReturnValue(mockState), - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - expect(consoleErrorSpy).toHaveBeenCalledWith( - 'Failed to fetch current user:', - expect.any(String) - ); - expect(response.status).toBe(500); - expect(await response.text()).toBe('Failed to fetch current user'); - - consoleErrorSpy.mockRestore(); - }); - - it('should handle user without required fields', async () => { - const mockCode = 'auth-code-123'; - const mockState = 'state-123'; - const mockTokens = { - accessToken: () => mockAccessToken, - refreshToken: () => mockRefreshToken - }; - - vi.mocked(obp_oauth.validateAuthorizationCode).mockResolvedValue(mockTokens); - - // Mock user response without required fields - const incompleteUser = { - user_id: 'test-user-123' - // Missing email - }; - - const mockFetch = createMockFetch([ - { - url: '/obp/v5.1.0/users/current', - response: incompleteUser - } - ]); - global.fetch = mockFetch; - - const event = createMockRequestEvent({ - url: new URL(`http://localhost:3000/login/obp/callback?code=${mockCode}&state=${mockState}`), - cookies: { - get: vi.fn().mockReturnValue(mockState), - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - // Should not redirect since user doesn't have required fields - expect(response).toBeUndefined(); - expect(event.locals.session.setData).not.toHaveBeenCalled(); - }); - - it('should log user data and session information', async () => { - const mockCode = 'auth-code-123'; - const mockState = 'state-123'; - const mockTokens = { - accessToken: () => mockAccessToken, - refreshToken: () => mockRefreshToken - }; - - vi.mocked(obp_oauth.validateAuthorizationCode).mockResolvedValue(mockTokens); - - const mockFetch = createMockFetch([ - { - url: '/obp/v5.1.0/users/current', - response: mockUser - } - ]); - global.fetch = mockFetch; - - const consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); - - const event = createMockRequestEvent({ - url: new URL(`http://localhost:3000/login/obp/callback?code=${mockCode}&state=${mockState}`), - cookies: { - get: vi.fn().mockReturnValue(mockState), - set: vi.fn(), - delete: vi.fn() - } - }); - - await CallbackGET(event); - - expect(consoleLogSpy).toHaveBeenCalledWith('Current user data:', mockUser); - expect(consoleLogSpy).toHaveBeenCalledWith('Session data set:', expect.any(Object)); - - consoleLogSpy.mockRestore(); - }); - - it('should handle session save failure gracefully', async () => { - const mockCode = 'auth-code-123'; - const mockState = 'state-123'; - const mockTokens = { - accessToken: () => mockAccessToken, - refreshToken: () => mockRefreshToken - }; - - vi.mocked(obp_oauth.validateAuthorizationCode).mockResolvedValue(mockTokens); - - const mockFetch = createMockFetch([ - { - url: '/obp/v5.1.0/users/current', - response: mockUser - } - ]); - global.fetch = mockFetch; - - // Mock session save failure - const event = createMockRequestEvent({ - url: new URL(`http://localhost:3000/login/obp/callback?code=${mockCode}&state=${mockState}`), - cookies: { - get: vi.fn().mockReturnValue(mockState), - set: vi.fn(), - delete: vi.fn() - } - }); - - event.locals.session.save.mockRejectedValue(new Error('Session save failed')); - - // Should throw error when session save fails - await expect(CallbackGET(event)).rejects.toThrow('Session save failed'); - - expect(event.locals.session.setData).toHaveBeenCalled(); - }); - }); - - describe('edge cases', () => { - it('should handle malformed URLs in callback', async () => { - const event = createMockRequestEvent({ - url: new URL('http://localhost:3000/login/obp/callback?code=test%20code&state=test%20state'), - cookies: { - get: vi.fn().mockReturnValue('test state'), - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); - - expect(response.status).toBe(302); - expect(response.headers.get('Location')).toBe('/'); - }); - - it('should handle missing OIDC configuration', () => { - vi.mocked(obp_oauth).OIDCConfig = undefined; + it('consults the provider factory to decide where to redirect', () => { + vi.mocked(oauth2ProviderFactory.getFirstAvailableProvider).mockReturnValue('google'); const event = createMockRequestEvent(); - const response = GET(event); - - expect(response.status).toBe(500); - }); - - it('should handle empty user response', async () => { - const mockCode = 'auth-code-123'; - const mockState = 'state-123'; - const mockTokens = { - accessToken: () => mockAccessToken, - refreshToken: () => mockRefreshToken - }; - - vi.mocked(obp_oauth.validateAuthorizationCode).mockResolvedValue(mockTokens); - - const mockFetch = createMockFetch([ - { - url: '/obp/v5.1.0/users/current', - response: {} - } - ]); - global.fetch = mockFetch; - - const event = createMockRequestEvent({ - url: new URL(`http://localhost:3000/login/obp/callback?code=${mockCode}&state=${mockState}`), - cookies: { - get: vi.fn().mockReturnValue(mockState), - set: vi.fn(), - delete: vi.fn() - } - }); - - const response = await CallbackGET(event); + try { + GET(event); + } catch { + // expected: GET always throws (redirect or error) + } - expect(response).toBeUndefined(); + expect(oauth2ProviderFactory.getFirstAvailableProvider).toHaveBeenCalledTimes(1); }); }); }); diff --git a/apps/portal/test/unit/routes/login.svelte.test.ts b/apps/portal/test/unit/routes/login.svelte.test.ts index 52939948..8fb512c1 100644 --- a/apps/portal/test/unit/routes/login.svelte.test.ts +++ b/apps/portal/test/unit/routes/login.svelte.test.ts @@ -1,257 +1,127 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { render, screen } from '@testing-library/svelte'; -import userEvent from '@testing-library/user-event'; import LoginPage from '$lib/../routes/login/+page.svelte'; +// Note: `$app/navigation` (goto, invalidateAll) is globally mocked in test/setup.ts, +// so onMount's setInterval(invalidateAll) and goto usage do not blow up here. + +// Helper to build the PageData prop for the login page. Only the provider/error +// fields the component reads are supplied; cast to satisfy the fuller PageData type. +function makeData(overrides = {}) { + return { + availableProviders: [], + unavailableProviders: [], + ...overrides + } as never; +} + describe('Login Page Component', () => { beforeEach(() => { - // Clear any existing DOM content document.body.innerHTML = ''; }); afterEach(() => { - // Clean up after each test document.body.innerHTML = ''; }); - it('should render login page with title', () => { - render(LoginPage); + it('should render the Login title heading', () => { + render(LoginPage, { props: { data: makeData() } }); - const title = screen.getByRole('heading', { name: /login/i }); + const title = screen.getByRole('heading', { level: 1, name: /login/i }); expect(title).toBeInTheDocument(); expect(title).toHaveClass('h2'); }); - it('should render login button with correct link', () => { - render(LoginPage); - - const loginButton = screen.getByRole('button'); - expect(loginButton).toBeInTheDocument(); - expect(loginButton).toHaveClass('btn', 'preset-filled-primary-500'); - - const loginLink = screen.getByRole('link', { name: /login with open bank project/i }); - expect(loginLink).toBeInTheDocument(); - expect(loginLink).toHaveAttribute('href', '/login/obp'); - }); - - it('should have proper styling and layout', () => { - render(LoginPage); - - const container = document.querySelector('.flex.h-full.w-full.items-center.justify-center'); - expect(container).toBeInTheDocument(); - - const card = document.querySelector('.rounded-xl.mx-auto'); - expect(card).toBeInTheDocument(); - expect(card).toHaveClass( - 'rounded-xl', - 'mx-auto', - 'w-auto', - 'sm:w-sm', - 'md:w-lg', - 'h-xl', - 'bg-white/10', - 'p-4', - 'max-w-xl', - 'backdrop-blur-xs', - 'align-middle', - 'divide-primary-50-950', - 'divide-y' - ); - }); - - it('should center the login button', () => { - render(LoginPage); - - const loginButton = screen.getByRole('button'); - expect(loginButton).toHaveClass('mx-auto', 'block'); - }); - - it('should have responsive button width', () => { - render(LoginPage); - - const loginButton = screen.getByRole('button'); - expect(loginButton).toHaveClass('w-full', 'sm:w-1/2'); - }); - - it('should have proper button spacing', () => { - render(LoginPage); - - const loginButton = screen.getByRole('button'); - expect(loginButton).toHaveClass('my-4'); - }); - - it('should render accessible elements', () => { - render(LoginPage); - - // Check that the title is properly structured as a heading - const title = screen.getByRole('heading', { level: 1 }); - expect(title).toBeInTheDocument(); - - // Check that the button is properly accessible - const button = screen.getByRole('button'); - expect(button).toBeInTheDocument(); - - // Check that the link is properly accessible - const link = screen.getByRole('link'); - expect(link).toBeInTheDocument(); - }); - - it('should have correct text content', () => { - render(LoginPage); - - expect(screen.getByText('Login')).toBeInTheDocument(); - expect(screen.getByText('Login with Open Bank Project')).toBeInTheDocument(); - }); - - it('should render button as a clickable element', async () => { - const user = userEvent.setup(); - render(LoginPage); - - const button = screen.getByRole('button'); - - // The button should be interactive - expect(button).not.toBeDisabled(); - - // We can't test the actual navigation since it's handled by SvelteKit, - // but we can test that the button is clickable - await user.click(button); - - // The button should still exist after clicking (no error thrown) - expect(button).toBeInTheDocument(); - }); - - it('should have proper semantic structure', () => { - render(LoginPage); - - // Should have a main container - const container = document.querySelector('.flex.h-full.w-full'); - expect(container).toBeInTheDocument(); - - // Should have a card-like structure - const card = container?.querySelector('.rounded-xl.mx-auto'); - expect(card).toBeInTheDocument(); - - // Should have a heading inside the card - const heading = card?.querySelector('h1'); - expect(heading).toBeInTheDocument(); - - // Should have a button inside the card - const button = card?.querySelector('button'); - expect(button).toBeInTheDocument(); - }); + it('should render a provider button/link for each available provider', () => { + render(LoginPage, { + props: { + data: makeData({ + availableProviders: [{ provider: 'obp-oidc' }, { provider: 'keycloak' }] + }) + } + }); - it('should render with backdrop blur effect', () => { - render(LoginPage); + // obp-oidc provider + const oidcButton = screen.getByTestId('provider-obp-oidc'); + expect(oidcButton).toBeInTheDocument(); + const oidcLink = oidcButton.querySelector('a'); + expect(oidcLink).toHaveAttribute('href', '/login/obp-oidc'); + expect(oidcLink).toHaveTextContent('OBP OpenID Connect'); - const card = document.querySelector('.backdrop-blur-xs'); - expect(card).toBeInTheDocument(); + // keycloak provider + const keycloakButton = screen.getByTestId('provider-keycloak'); + expect(keycloakButton).toBeInTheDocument(); + const keycloakLink = keycloakButton.querySelector('a'); + expect(keycloakLink).toHaveAttribute('href', '/login/keycloak'); + expect(keycloakLink).toHaveTextContent('Keycloak'); }); - it('should have semi-transparent background', () => { - render(LoginPage); + it('should capitalize unknown provider names', () => { + render(LoginPage, { + props: { data: makeData({ availableProviders: [{ provider: 'github' }] }) } + }); - const card = document.querySelector('.bg-white\\/10'); - expect(card).toBeInTheDocument(); + const button = screen.getByTestId('provider-github'); + const link = button.querySelector('a'); + expect(link).toHaveAttribute('href', '/login/github'); + expect(link).toHaveTextContent('Github'); }); - it('should have divider styling', () => { - render(LoginPage); + it('should show a message when there are no available providers', () => { + render(LoginPage, { props: { data: makeData({ availableProviders: [] }) } }); - const card = document.querySelector('.divide-primary-50-950.divide-y'); - expect(card).toBeInTheDocument(); + expect( + screen.getByText(/no authentication providers available/i) + ).toBeInTheDocument(); + expect(screen.queryByTestId('provider-obp-oidc')).not.toBeInTheDocument(); }); - it('should be responsive across different screen sizes', () => { - render(LoginPage); - - // Check responsive width classes - const card = document.querySelector('.w-auto.sm\\:w-sm.md\\:w-lg'); - expect(card).toBeInTheDocument(); + it('should render the error block when an errorMessage is provided', () => { + render(LoginPage, { + props: { data: makeData({ errorMessage: 'Something went wrong' }) } + }); - // Check responsive button width classes - const button = screen.getByRole('button'); - expect(button).toHaveClass('w-full', 'sm:w-1/2'); + const errorBlock = screen.getByTestId('login-error'); + expect(errorBlock).toBeInTheDocument(); + expect(errorBlock).toHaveTextContent('Something went wrong'); }); - it('should maintain proper contrast and accessibility', () => { - render(LoginPage); - - // The text should be visible (not transparent or hidden) - const title = screen.getByRole('heading'); - const computedStyle = window.getComputedStyle(title); - expect(computedStyle.display).not.toBe('none'); - expect(computedStyle.visibility).not.toBe('hidden'); + it('should not render the error block when there is no errorMessage', () => { + render(LoginPage, { props: { data: makeData() } }); - // The button should be visible and interactive - const button = screen.getByRole('button'); - const buttonStyle = window.getComputedStyle(button); - expect(buttonStyle.display).not.toBe('none'); - expect(buttonStyle.visibility).not.toBe('hidden'); + expect(screen.queryByTestId('login-error')).not.toBeInTheDocument(); }); - it('should have consistent spacing and layout', () => { - render(LoginPage); - - // Check container spacing - const container = document.querySelector('.flex.h-full.w-full.items-center.justify-center'); - expect(container).toBeInTheDocument(); - - // Check card padding - const card = document.querySelector('.p-4'); - expect(card).toBeInTheDocument(); + it('should render unavailable providers with their error in the unavailable section', () => { + render(LoginPage, { + props: { + data: makeData({ + availableProviders: [{ provider: 'obp-oidc' }], + unavailableProviders: [{ provider: 'keycloak', error: 'Connection refused' }] + }) + } + }); - // Check button margin - const button = screen.getByRole('button'); - expect(button).toHaveClass('my-4'); + expect(screen.getByText(/currently unavailable/i)).toBeInTheDocument(); + expect(screen.getByText('Keycloak')).toBeInTheDocument(); + expect(screen.getByText('Connection refused')).toBeInTheDocument(); }); - it('should render all required CSS classes', () => { - render(LoginPage); - - // Test main container classes - const mainContainer = document.querySelector('.flex.h-full.w-full.items-center.justify-center'); - expect(mainContainer).toBeInTheDocument(); - - // Test card classes - const cardClasses = [ - 'rounded-xl', - 'mx-auto', - 'w-auto', - 'sm:w-sm', - 'md:w-lg', - 'h-xl', - 'bg-white/10', - 'p-4', - 'max-w-xl', - 'backdrop-blur-xs', - 'align-middle', - 'divide-primary-50-950', - 'divide-y' - ]; - - const card = document.querySelector('.rounded-xl'); - cardClasses.forEach(className => { - expect(card).toHaveClass(className); + it('should render unavailable providers even when there are no available providers', () => { + render(LoginPage, { + props: { + data: makeData({ + availableProviders: [], + unavailableProviders: [{ provider: 'obp-oidc', error: 'Timed out' }] + }) + } }); - // Test title classes - const title = screen.getByRole('heading'); - expect(title).toHaveClass('h2', 'text-center'); - - // Test button classes - const button = screen.getByRole('button'); - const buttonClasses = [ - 'btn', - 'preset-filled-primary-500', - 'mx-auto', - 'my-4', - 'block', - 'w-full', - 'sm:w-1/2' - ]; - - buttonClasses.forEach(className => { - expect(button).toHaveClass(className); - }); + expect( + screen.getByText(/no authentication providers available/i) + ).toBeInTheDocument(); + expect(screen.getByText(/currently unavailable/i)).toBeInTheDocument(); + expect(screen.getByText('OBP OpenID Connect')).toBeInTheDocument(); + expect(screen.getByText('Timed out')).toBeInTheDocument(); }); }); diff --git a/apps/portal/vite.config.ts b/apps/portal/vite.config.ts index d284df83..b2257ae9 100644 --- a/apps/portal/vite.config.ts +++ b/apps/portal/vite.config.ts @@ -2,15 +2,31 @@ import { svelteTesting } from '@testing-library/svelte/vite'; import tailwindcss from '@tailwindcss/vite'; import { sveltekit } from '@sveltejs/kit/vite'; import { defineConfig } from 'vite'; -import { readFileSync } from 'fs'; +import { existsSync, readFileSync } from 'fs'; +import { dirname, join } from 'path'; +import { fileURLToPath } from 'url'; import { buildInfoDefine } from '../../packages/shared/build-info.js'; // Get version from package.json const packageJson = JSON.parse(readFileSync('./package.json', 'utf-8')); const version = packageJson.version; +// Walk up to the directory that actually holds the hoisted node_modules. In a +// normal checkout this is the repo root; in a git worktree the dependencies live +// above the worktree boundary, so Vite's default fs.allow can't serve them +// (e.g. @testing-library/svelte's injected cleanup module) — allow that root. +function findDepsRoot(start: string): string { + let dir = start; + while (dir !== dirname(dir)) { + if (existsSync(join(dir, 'node_modules', '@testing-library'))) return dir; + dir = dirname(dir); + } + return start; +} +const depsRoot = findDepsRoot(dirname(fileURLToPath(import.meta.url))); + export default defineConfig({ - server: { port: 5174 }, + server: { port: 5174, fs: { allow: [depsRoot] } }, define: buildInfoDefine(version), plugins: [tailwindcss(), sveltekit()], test: { diff --git a/packages/shared/src/lib/avatar/generate.spec.ts b/packages/shared/src/lib/avatar/generate.spec.ts new file mode 100644 index 00000000..afb92170 --- /dev/null +++ b/packages/shared/src/lib/avatar/generate.spec.ts @@ -0,0 +1,56 @@ +import { describe, it, expect, vi } from 'vitest'; + +// generate.ts reads $env/dynamic/public at module load; provide it. +vi.mock('$env/dynamic/public', () => ({ env: { PUBLIC_OBP_BASE_URL: 'https://obp.example.com' } })); + +import { generateIdenticon, userAvatarSeed, roomAvatarSeed } from './generate'; + +describe('generateIdenticon', () => { + it('is deterministic for a given seed', () => { + expect(generateIdenticon('alice')).toEqual(generateIdenticon('alice')); + }); + + it('produces different output for different seeds', () => { + expect(generateIdenticon('alice')).not.toEqual(generateIdenticon('bob')); + }); + + it('returns a square grid of the requested size', () => { + const { grid } = generateIdenticon('alice', 5); + expect(grid).toHaveLength(5); + for (const row of grid) { + expect(row).toHaveLength(5); + } + }); + + it('honors a custom grid size', () => { + const { grid } = generateIdenticon('alice', 7); + expect(grid).toHaveLength(7); + expect(grid[0]).toHaveLength(7); + }); + + it('is horizontally mirror-symmetric', () => { + const { grid } = generateIdenticon('some-seed', 5); + for (const row of grid) { + for (let col = 0; col < row.length; col++) { + expect(row[col]).toBe(row[row.length - 1 - col]); + } + } + }); + + it('emits hsl color and background strings', () => { + const { color, background } = generateIdenticon('alice'); + expect(color).toMatch(/^hsl\(\d+, 65%, 50%\)$/); + expect(background).toMatch(/^hsl\(\d+, 30%, 92%\)$/); + }); +}); + +describe('avatar seeds', () => { + it('namespaces a user seed with the OBP host', () => { + expect(userAvatarSeed('bob')).toBe('https://obp.example.com|bob'); + }); + + it('namespaces a room seed distinctly from a user seed', () => { + expect(roomAvatarSeed('room-1')).toBe('https://obp.example.com|room|room-1'); + expect(roomAvatarSeed('bob')).not.toBe(userAvatarSeed('bob')); + }); +}); diff --git a/packages/shared/src/lib/server/oauth/client.ts b/packages/shared/src/lib/server/oauth/client.ts index 1380189f..73e15f36 100644 --- a/packages/shared/src/lib/server/oauth/client.ts +++ b/packages/shared/src/lib/server/oauth/client.ts @@ -1,4 +1,5 @@ import { createLogger } from '$shared/utils/logger'; +import { redactUrlEncodedBody } from '$shared/utils/redact'; const logger = createLogger('OAuth2Client'); import { OAuth2Client } from 'arctic'; import type { OpenIdConnectConfiguration, OAuth2AccessTokenPayload } from './types'; @@ -58,20 +59,23 @@ export class OAuth2ClientWithConfig extends OAuth2Client { } async checkAccessTokenExpiration(accessToken: string): Promise { - // Returns true if the access token is expired, false if it is valid + // Returns true if the access token is expired (or its validity can't be + // determined — treated as expired so the caller attempts a refresh rather + // than either 500ing or treating an unverifiable token as eternally valid), + // false if it is confirmed valid. logger.debug('Checking access token expiration...'); try { const payload = jwtDecode(accessToken) as OAuth2AccessTokenPayload; if (!payload || !payload.exp) { logger.warn('Access token payload is invalid or missing expiration.'); - return false; + return true; } const isExpired = Date.now() >= payload.exp * 1000; logger.debug(`Access token is ${isExpired ? 'expired' : 'valid'}.`); return isExpired; } catch (error) { - logger.error('Error decoding access token:', error); - throw error; + logger.warn('Could not decode access token; treating as expired:', error); + return true; } } @@ -110,7 +114,7 @@ export class OAuth2ClientWithConfig extends OAuth2Client { body.set('code_verifier', codeVerifier); } - logger.debug(`Token request body: ${body.toString()}`); + logger.debug(`Token request body: ${redactUrlEncodedBody(body)}`); const response = await fetch(tokenEndpoint, { method: 'POST', @@ -179,7 +183,7 @@ export class OAuth2ClientWithConfig extends OAuth2Client { body.set('code_verifier', codeVerifier); } - logger.debug(`Token request body: ${body.toString()}`); + logger.debug(`Token request body: ${redactUrlEncodedBody(body)}`); const response = await fetch(tokenEndpoint, { method: 'POST', diff --git a/packages/shared/src/lib/server/oauth/providerFactory.ts b/packages/shared/src/lib/server/oauth/providerFactory.ts index e0752b1d..a318f96c 100644 --- a/packages/shared/src/lib/server/oauth/providerFactory.ts +++ b/packages/shared/src/lib/server/oauth/providerFactory.ts @@ -209,4 +209,29 @@ export class OAuth2ProviderFactory { const providers = Array.from(this.initializedClients.keys()); return providers.length > 0 ? providers[0] : null; } + + // Origins of all configured OIDC providers' authorization endpoints — the only + // hosts a post-login/consent redirect (oidc_return_url) is allowed to target. + getTrustedOidcOrigins(): Set { + const origins = new Set(); + for (const client of this.initializedClients.values()) { + const authEndpoint = client.OIDCConfig?.authorization_endpoint; + if (authEndpoint) { + try { + origins.add(new URL(authEndpoint).origin); + } catch { + // ignore malformed config + } + } + } + return origins; + } + + isTrustedOidcReturnUrl(candidate: string): boolean { + try { + return this.getTrustedOidcOrigins().has(new URL(candidate).origin); + } catch { + return false; + } + } } diff --git a/packages/shared/src/lib/utils/index.ts b/packages/shared/src/lib/utils/index.ts index 6f765d86..9341dd39 100644 --- a/packages/shared/src/lib/utils/index.ts +++ b/packages/shared/src/lib/utils/index.ts @@ -4,3 +4,5 @@ export { extractUsernameFromJWT, isJWTExpired, getJWTPayload } from './jwt.js'; export type { JWTPayload } from './jwt.js'; export { toaster, toast } from './toastService.js'; export { getLegalMarkdownFromWebUIProps } from './loadLegalDocumentFromApi.js'; +export { isSafeRelativeRedirect } from './redirect.js'; +export { redactUrlEncodedBody } from './redact.js'; diff --git a/packages/shared/src/lib/utils/redact.test.ts b/packages/shared/src/lib/utils/redact.test.ts new file mode 100644 index 00000000..4676e7d2 --- /dev/null +++ b/packages/shared/src/lib/utils/redact.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect } from 'vitest'; +import { redactUrlEncodedBody } from './redact'; + +function parse(query: string): Record { + return Object.fromEntries(new URLSearchParams(query)); +} + +describe('redactUrlEncodedBody', () => { + const secretFields = ['refresh_token', 'code', 'client_secret', 'password', 'access_token', 'id_token']; + + for (const field of secretFields) { + it(`redacts the ${field} field`, () => { + const body = new URLSearchParams({ [field]: 'super-secret-value' }); + expect(parse(redactUrlEncodedBody(body))[field]).toBe('[redacted]'); + }); + } + + it('passes non-secret fields through verbatim', () => { + const body = new URLSearchParams({ grant_type: 'authorization_code', scope: 'openid profile' }); + const result = parse(redactUrlEncodedBody(body)); + expect(result.grant_type).toBe('authorization_code'); + expect(result.scope).toBe('openid profile'); + }); + + it('leaves a body with no secret fields unchanged', () => { + const body = new URLSearchParams({ grant_type: 'refresh', scope: 'openid' }); + expect(redactUrlEncodedBody(body)).toBe('grant_type=refresh&scope=openid'); + }); + + it('redacts secrets while preserving non-secret fields alongside them', () => { + const body = new URLSearchParams({ + grant_type: 'authorization_code', + code: 'auth-code-123', + client_secret: 'shhh', + scope: 'openid' + }); + const result = parse(redactUrlEncodedBody(body)); + expect(result).toEqual({ + grant_type: 'authorization_code', + code: '[redacted]', + client_secret: '[redacted]', + scope: 'openid' + }); + }); + + it('does not mutate the input URLSearchParams', () => { + const body = new URLSearchParams({ refresh_token: 'keep-me', grant_type: 'refresh_token' }); + redactUrlEncodedBody(body); + expect(body.get('refresh_token')).toBe('keep-me'); + }); + + it('does not add absent secret fields to the output', () => { + const body = new URLSearchParams({ grant_type: 'authorization_code' }); + const result = parse(redactUrlEncodedBody(body)); + expect(Object.keys(result)).toEqual(['grant_type']); + }); +}); diff --git a/packages/shared/src/lib/utils/redact.ts b/packages/shared/src/lib/utils/redact.ts new file mode 100644 index 00000000..d33bfd20 --- /dev/null +++ b/packages/shared/src/lib/utils/redact.ts @@ -0,0 +1,13 @@ +const SECRET_BODY_FIELDS = ['refresh_token', 'code', 'client_secret', 'password', 'access_token', 'id_token']; + +// Renders a URL-encoded OAuth request body for logging with secret fields redacted, +// since these bodies carry refresh tokens / auth codes / client secrets verbatim. +export function redactUrlEncodedBody(body: URLSearchParams): string { + const redacted = new URLSearchParams(body); + for (const field of SECRET_BODY_FIELDS) { + if (redacted.has(field)) { + redacted.set(field, '[redacted]'); + } + } + return redacted.toString(); +} diff --git a/packages/shared/src/lib/utils/redirect.test.ts b/packages/shared/src/lib/utils/redirect.test.ts new file mode 100644 index 00000000..45c677f8 --- /dev/null +++ b/packages/shared/src/lib/utils/redirect.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect } from 'vitest'; +import { isSafeRelativeRedirect } from './redirect'; + +describe('isSafeRelativeRedirect', () => { + it('rejects null', () => { + expect(isSafeRelativeRedirect(null)).toBe(false); + }); + + it('rejects undefined', () => { + expect(isSafeRelativeRedirect(undefined)).toBe(false); + }); + + it('rejects an empty string', () => { + expect(isSafeRelativeRedirect('')).toBe(false); + }); + + it('accepts a simple site-relative path', () => { + expect(isSafeRelativeRedirect('/dashboard')).toBe(true); + }); + + it('accepts the root path', () => { + expect(isSafeRelativeRedirect('/')).toBe(true); + }); + + it('accepts a relative path with query string', () => { + expect(isSafeRelativeRedirect('/a/b?c=d')).toBe(true); + }); + + it('rejects a protocol-relative URL', () => { + expect(isSafeRelativeRedirect('//evil.com')).toBe(false); + }); + + it('rejects a backslash-prefixed path that browsers resolve to another host', () => { + expect(isSafeRelativeRedirect('/\\evil.com')).toBe(false); + }); + + it('rejects an absolute http(s) URL', () => { + expect(isSafeRelativeRedirect('https://evil.com')).toBe(false); + }); + + it('rejects a path without a leading slash', () => { + expect(isSafeRelativeRedirect('dashboard')).toBe(false); + }); +}); diff --git a/packages/shared/src/lib/utils/redirect.ts b/packages/shared/src/lib/utils/redirect.ts new file mode 100644 index 00000000..7a4af4fb --- /dev/null +++ b/packages/shared/src/lib/utils/redirect.ts @@ -0,0 +1,7 @@ +// A same-page-origin relative path is safe to redirect to; protocol-relative +// (`//host`) and backslash (`/\host`) values are browser-resolved as absolute +// URLs to a different host and must be rejected. +export function isSafeRelativeRedirect(path: string | null | undefined): path is string { + if (!path) return false; + return path.startsWith('/') && !path.startsWith('//') && !path.startsWith('/\\'); +} diff --git a/packages/shared/src/routes/page.svelte.spec.ts b/packages/shared/src/routes/page.svelte.spec.ts deleted file mode 100644 index 9f8a7e6f..00000000 --- a/packages/shared/src/routes/page.svelte.spec.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { render, screen } from '@testing-library/svelte'; -import { describe, expect, it } from 'vitest'; -import Page from './+page.svelte'; - -describe('/+page.svelte', () => { - it('should render h1', () => { - render(Page); - - const heading = screen.getByRole('heading', { level: 1 }); - expect(heading).toBeInTheDocument(); - }); -});