From e35cd82f5f4dab0c8edde626512aecb83825f175 Mon Sep 17 00:00:00 2001 From: Michael Brusegard <56915010+michaelbrusegard@users.noreply.github.com> Date: Fri, 18 Sep 2026 04:01:58 -0700 Subject: [PATCH] fix(auth): verify external identity providers during handoff Allow webmail-configured OAuth providers on separate hosts without trusting callback-supplied endpoints or broadly allowing sibling domains. Bind the client, JMAP server, issuer and token endpoint to HTTPS metadata from the selected webmail for both handoff and pairing. --- src/lib/__tests__/oauth-handoff.test.ts | 164 ++++++++++++++++++++++++ src/lib/oauth.ts | 62 ++++++++- 2 files changed, 222 insertions(+), 4 deletions(-) create mode 100644 src/lib/__tests__/oauth-handoff.test.ts diff --git a/src/lib/__tests__/oauth-handoff.test.ts b/src/lib/__tests__/oauth-handoff.test.ts new file mode 100644 index 00000000..328cc9b9 --- /dev/null +++ b/src/lib/__tests__/oauth-handoff.test.ts @@ -0,0 +1,164 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import * as WebBrowser from 'expo-web-browser'; + +const { mockSecureFetch } = vi.hoisted(() => ({ mockSecureFetch: vi.fn() })); +vi.mock('../client-cert', () => ({ secureFetch: mockSecureFetch })); + +import { redeemPairingCode, runWebmailHandoff } from '../oauth'; + +const WEBMAIL = 'https://webmail.example.com'; +const SERVER = 'https://mail.example.com'; +const ISSUER = 'https://id.example.net'; +const TOKEN_ENDPOINT = `${ISSUER}/token`; + +let config: Record; +let metadata: Record; +let bundle: Record; +let stateMismatch: boolean; + +function response(url: string, data: unknown): Response { + return { ok: true, status: 200, url, redirected: false, json: async () => data } as Response; +} + +beforeEach(() => { + vi.resetAllMocks(); + config = { + jmapServerUrl: SERVER, + oauthEnabled: true, + oauthClientId: 'stalwart', + oauthIssuerUrl: ISSUER, + }; + metadata = { issuer: ISSUER, token_endpoint: TOKEN_ENDPOINT }; + bundle = { + flow: 'oauth', server_url: SERVER, access_token: 'test-access', + refresh_token: 'test-refresh', client_id: 'stalwart', token_endpoint: TOKEN_ENDPOINT, + }; + stateMismatch = false; + vi.mocked(WebBrowser.openAuthSessionAsync).mockImplementation(async (url) => ({ + type: 'success', + url: `bulwarkmobile://auth/callback#${new URLSearchParams({ + ...bundle, + state: stateMismatch ? 'wrong-state' : new URL(url).searchParams.get('mobile_state')!, + })}`, + })); + mockSecureFetch.mockImplementation(async (url: string) => { + if (url === `${WEBMAIL}/api/auth/pair/redeem`) return response(url, bundle); + if (url === `${WEBMAIL}/api/config`) return response(url, config); + if (url === `${WEBMAIL}/api/auth/oauth/metadata`) return response(url, metadata); + throw new Error(`Unexpected fetch: ${url}`); + }); +}); + +afterEach(() => vi.useRealTimers()); + +for (const flow of ['handoff', 'pairing'] as const) { + const login = () => flow === 'handoff' + ? runWebmailHandoff(WEBMAIL) + : redeemPairingCode(WEBMAIL, 'test-pair-code'); + + describe(`${flow} external identity provider`, () => { + it('accepts an external token endpoint verified through the selected webmail', async () => { + await expect(login()).resolves.toMatchObject({ + flow: 'oauth', serverUrl: SERVER, + tokens: { tokenEndpoint: TOKEN_ENDPOINT, clientId: 'stalwart', source: flow }, + }); + const discoveryCalls = mockSecureFetch.mock.calls.filter(([url]) => !url.endsWith('/pair/redeem')); + expect(discoveryCalls.map(([url]) => url)).toEqual([ + `${WEBMAIL}/api/config`, `${WEBMAIL}/api/auth/oauth/metadata`, + ]); + for (const [, options] of discoveryCalls) { + expect(options).toMatchObject({ + headers: { Accept: 'application/json' }, redirect: 'error', timeoutMs: 8000, + }); + expect(options.signal).toBeInstanceOf(AbortSignal); + expect(options.body).toBeUndefined(); + } + }); + + it('also accepts a configured sibling-host provider', async () => { + config.oauthIssuerUrl = metadata.issuer = 'https://id.example.com'; + metadata.token_endpoint = bundle.token_endpoint = 'https://id.example.com/token'; + await expect(login()).resolves.toMatchObject({ flow: 'oauth' }); + }); + + it('keeps same-host login independent of webmail metadata', async () => { + bundle.token_endpoint = `${SERVER}/auth/token`; + await expect(login()).resolves.toMatchObject({ flow: 'oauth' }); + expect(mockSecureFetch.mock.calls.filter(([url]) => !url.endsWith('/pair/redeem'))).toHaveLength(0); + }); + + const mismatches: [string, () => void][] = [ + ['substituted token endpoint', () => { bundle.token_endpoint = 'https://attacker.test/token'; }], + ['lookalike token hostname', () => { bundle.token_endpoint = 'https://id.example.net.attacker.test/token'; }], + ['different token path', () => { bundle.token_endpoint = `${ISSUER}/other`; }], + ['different token query', () => { bundle.token_endpoint = `${TOKEN_ENDPOINT}?other=1`; }], + ['token URL with credentials', () => { bundle.token_endpoint = metadata.token_endpoint = 'https://user@id.example.net/token'; }], + ['token URL with a fragment', () => { bundle.token_endpoint = metadata.token_endpoint = `${TOKEN_ENDPOINT}#other`; }], + ['cleartext token endpoint', () => { bundle.token_endpoint = metadata.token_endpoint = 'http://id.example.net/token'; }], + ['different client', () => { bundle.client_id = 'different-client'; }], + ['different JMAP server', () => { bundle.server_url = 'https://other.example.com'; }], + ['disabled OAuth', () => { config.oauthEnabled = false; }], + ['non-boolean OAuth flag', () => { config.oauthEnabled = 'true'; }], + ['missing issuer', () => { delete config.oauthIssuerUrl; }], + ['cleartext issuer', () => { config.oauthIssuerUrl = metadata.issuer = 'http://id.example.net'; }], + ['issuer mismatch', () => { metadata.issuer = 'https://other-id.example.net'; }], + ['missing token endpoint', () => { delete metadata.token_endpoint; }], + ]; + it.each(mismatches)('rejects %s', async (_name, mutate) => { + mutate(); + await expect(login()).rejects.toThrow('token endpoint is not trusted'); + }); + + it.each(['/api/config', '/api/auth/oauth/metadata'])('fails closed when %s cannot be fetched', async (path) => { + const normal = mockSecureFetch.getMockImplementation()!; + mockSecureFetch.mockImplementation((url, options) => { + if (url.endsWith(path)) throw new Error('Network unavailable'); + return normal(url, options); + }); + await expect(login()).rejects.toThrow('token endpoint is not trusted'); + }); + + const badResponses: [string, Partial][] = [ + ['HTTP error', { ok: false, status: 503 }], + ['cross-origin redirect', { url: 'https://attacker.test/api/config', redirected: true }], + ['same-origin redirect', { url: `${WEBMAIL}/different`, redirected: true }], + ['redirect back to the requested URL', { url: `${WEBMAIL}/api/config`, redirected: true }], + ['missing final URL (native client-certificate bridge)', { url: '' }], + ['invalid JSON', { json: async () => { throw new SyntaxError('Invalid JSON'); } }], + ]; + it.each(badResponses)('fails closed for metadata with %s', async (_name, overrides) => { + const normal = mockSecureFetch.getMockImplementation()!; + mockSecureFetch.mockImplementation(async (url, options) => { + const result = await normal(url, options); + return url.endsWith('/pair/redeem') ? result : { ...result, ...overrides }; + }); + await expect(login()).rejects.toThrow('token endpoint is not trusted'); + }); + + it('aborts metadata discovery after its deadline', async () => { + vi.useFakeTimers(); + const normal = mockSecureFetch.getMockImplementation()!; + mockSecureFetch.mockImplementation((url, options) => { + if (!url.endsWith('/api/config')) return normal(url, options); + return new Promise((_resolve, reject) => options.signal.addEventListener('abort', () => reject(new Error('Aborted')))); + }); + const result = expect(login()).rejects.toThrow('token endpoint is not trusted'); + await vi.advanceTimersByTimeAsync(8000); + await result; + }); + }); +} + +it('rejects a mismatched callback state before fetching metadata', async () => { + stateMismatch = true; + await expect(runWebmailHandoff(WEBMAIL)).rejects.toThrow('State mismatch'); + expect(mockSecureFetch).not.toHaveBeenCalled(); +}); + +it('leaves password handoff unchanged', async () => { + bundle = { flow: 'password', server_url: SERVER, username: 'user@example.com', password: 'test-password' }; + await expect(runWebmailHandoff(WEBMAIL)).resolves.toEqual({ + flow: 'password', serverUrl: SERVER, username: 'user@example.com', password: 'test-password', + }); + expect(mockSecureFetch).not.toHaveBeenCalled(); +}); diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts index 31ba28d2..01759fc4 100644 --- a/src/lib/oauth.ts +++ b/src/lib/oauth.ts @@ -63,8 +63,8 @@ function randomState(): string { } // Anything from the redirect fragment is attacker-influenced: another app can -// register the same custom scheme. Only accept https endpoints, and only a -// token endpoint that lives on the mail server's own host (or the webmail's). +// register the same custom scheme. External token endpoints need verification +// through the selected webmail, rather than trusting the redirect alone. function isHttpsUrl(value: string | null): value is string { return !!value && /^https:\/\/[^/?#\s]+/i.test(value); } @@ -92,6 +92,60 @@ export function isAcceptableTokenEndpoint( return allowed.some((h) => host === h || host.endsWith(`.${h}`) || h.endsWith(`.${host}`)); } +// External IdPs must be authorized by the selected webmail over TLS, never +// by callback fields or a shared DNS suffix. Bind all returned identifiers. +async function isTrustedHandoffEndpoint( + tokenEndpoint: string, + serverUrl: string, + webmailUrl: string, + clientId: string, +): Promise { + if (isAcceptableTokenEndpoint(tokenEndpoint, serverUrl, webmailUrl)) return true; + function httpsUrl(value: unknown): value is string { + if (typeof value !== 'string') return false; + try { + const url = new URL(value); + return url.protocol === 'https:' && !url.username && !url.password && !url.hash; + } catch { + return false; + } + } + if (!httpsUrl(webmailUrl) || !httpsUrl(tokenEndpoint) || !httpsUrl(serverUrl)) return false; + const base = webmailUrl.replace(/\/+$/, ''); + try { + const load = async (path: string) => { + const url = `${base}${path}`; + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 8000); + try { + const response = await secureFetch(url, { + headers: { Accept: 'application/json' }, + redirect: 'error', + signal: controller.signal, + timeoutMs: 8000, + }); + // The native client-certificate bridge omits the final URL. Without + // transport evidence of its destination, external-IdP trust must fail. + if (!response.ok || response.url !== url || response.redirected) { + throw new Error('Untrusted metadata response'); + } + return await response.json(); + } finally { + clearTimeout(timeout); + } + }; + const config = await load('/api/config'); + if ( + config.oauthEnabled !== true || config.oauthClientId !== clientId || + config.jmapServerUrl !== serverUrl || !httpsUrl(config.oauthIssuerUrl) + ) return false; + const metadata = await load('/api/auth/oauth/metadata'); + return metadata.issuer === config.oauthIssuerUrl && metadata.token_endpoint === tokenEndpoint; + } catch { + return false; + } +} + function buildHandoffUrl(webmailUrl: string, state: string): string { const base = webmailUrl.replace(/\/+$/, ''); const params = new URLSearchParams({ @@ -154,7 +208,7 @@ export async function runWebmailHandoff(webmailUrl: string): Promise