diff --git a/src/lib/__tests__/oauth-pairing.test.ts b/src/lib/__tests__/oauth-pairing.test.ts index 978ce3b0..23e5c9a4 100644 --- a/src/lib/__tests__/oauth-pairing.test.ts +++ b/src/lib/__tests__/oauth-pairing.test.ts @@ -6,6 +6,7 @@ const { mockSecureFetch } = vi.hoisted(() => ({ vi.mock('../client-cert', () => ({ secureFetch: mockSecureFetch, + fetchWithoutForeignRedirect: mockSecureFetch, })); import { diff --git a/src/lib/__tests__/oauth-redirects.test.ts b/src/lib/__tests__/oauth-redirects.test.ts new file mode 100644 index 00000000..b1c4c1ac --- /dev/null +++ b/src/lib/__tests__/oauth-redirects.test.ts @@ -0,0 +1,151 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import * as WebBrowser from 'expo-web-browser'; + +import { setManualFetchForTests } from '../client-cert'; +import { HandoffError, TransientRefreshError, refreshOAuthAccessToken, resetRefreshRotations } from '../oauth'; +import { discoverOAuthMetadata, loginWithPkce, revokeRefreshToken, type OAuthMetadata } from '../oauth-native'; + +// The refresh token, the code + verifier and the metadata that decides where +// they go must not follow a redirect to another host. One fake network sits +// behind both fetches: the one told `redirect: 'manual'` (Expo's on a device) +// gets the 3xx back; React Native's own follows a 307/308 anywhere with the +// method and body, as OkHttp does. + +const SERVER = 'https://mail.example.com'; +// An external identity provider: its token endpoint is on another host than +// the mail server, and that is fine. +const IDP = 'https://id.example.net'; +const EVIL = 'https://evil.example'; + +interface Hit { url: string; body?: string } +let hits: Hit[]; +let routes: Record Response>; + +function json(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } }); +} + +function redirect(location: string, status = 307): Response { + return new Response(null, { status, headers: { Location: location } }); +} + +async function network(url: string, init?: RequestInit): Promise { + hits.push({ url, body: typeof init?.body === 'string' ? init.body : undefined }); + const route = routes[url.split('?')[0]]; + const response = route ? route() : json({ error: 'not found' }, 404); + const location = response.headers.get('location'); + if (location && init?.redirect !== 'manual') return network(location, init); + return response; +} + +const evilHits = () => hits.filter((h) => h.url.startsWith(EVIL)); + +beforeEach(() => { + hits = []; + routes = {}; + resetRefreshRotations(); + setManualFetchForTests(network); + vi.stubGlobal('fetch', vi.fn(network)); +}); +afterEach(() => { + setManualFetchForTests(undefined); + vi.unstubAllGlobals(); +}); + +const tokens = { + accessToken: 'a1', + refreshToken: 'r1', + expiresAt: Date.now() + 5_000, + tokenEndpoint: `${IDP}/token`, + clientId: 'bulwark-webmail', +}; + +describe('token refresh', () => { + it('does not take the refresh token along a redirect to another host', async () => { + routes[`${IDP}/token`] = () => redirect(`${EVIL}/token`); + + await expect(refreshOAuthAccessToken(tokens)).rejects.toBeInstanceOf(TransientRefreshError); + + expect(evilHits()).toEqual([]); + }); + + it("follows a redirect within the token endpoint's own host, not the server's", async () => { + routes[`${IDP}/token`] = () => redirect(`${IDP}/oauth/token`, 308); + routes[`${IDP}/oauth/token`] = () => json({ access_token: 'a2', refresh_token: 'r2', expires_in: 3600 }); + + const next = await refreshOAuthAccessToken(tokens); + + expect(next.accessToken).toBe('a2'); + expect(hits.map((h) => h.url)).toEqual([`${IDP}/token`, `${IDP}/oauth/token`]); + }); +}); + +describe('PKCE code exchange', () => { + it('does not take the code and verifier along a redirect to another host', async () => { + vi.mocked(WebBrowser.openAuthSessionAsync).mockImplementationOnce(async (authUrl: string) => ({ + type: 'success', + url: `bulwarkmobile://auth/callback?code=c1&state=${new URL(authUrl).searchParams.get('state')}`, + })); + routes[`${IDP}/token`] = () => redirect(`${EVIL}/token`); + const metadata: OAuthMetadata = { authorization_endpoint: `${IDP}/authorize`, token_endpoint: `${IDP}/token` }; + + await expect(loginWithPkce(SERVER, metadata, { clientId: 'bulwark-webmail' })).rejects.toBeInstanceOf(HandoffError); + + expect(hits.map((h) => h.url)).toEqual([`${IDP}/token`]); + expect(evilHits()).toEqual([]); + }); +}); + +describe('revocation', () => { + it('reaches an identity provider on another host but not a redirect away from it', async () => { + routes[`${SERVER}/.well-known/oauth-authorization-server`] = () => json({ + authorization_endpoint: `${IDP}/authorize`, + token_endpoint: `${IDP}/token`, + revocation_endpoint: `${IDP}/revoke`, + }); + routes[`${IDP}/revoke`] = () => redirect(`${EVIL}/revoke`); + + await revokeRefreshToken(SERVER, { ...tokens, source: 'native' }); + + expect(hits.map((h) => h.url)).toContain(`${IDP}/revoke`); + expect(evilHits()).toEqual([]); + }); +}); + +describe('metadata discovery', () => { + it('ignores metadata reached through a redirect to another host', async () => { + routes[`${SERVER}/.well-known/oauth-authorization-server`] = () => + redirect(`${EVIL}/.well-known/oauth-authorization-server`, 302); + routes[`${EVIL}/.well-known/oauth-authorization-server`] = () => json({ + authorization_endpoint: `${EVIL}/authorize`, + token_endpoint: `${EVIL}/token`, + registration_endpoint: `${EVIL}/register`, + }); + + expect(await discoverOAuthMetadata(SERVER)).toBeNull(); + expect(evilHits()).toEqual([]); + }); + + it('ignores it where the platform followed the redirect anyway', async () => { + // No fetch that takes `redirect: 'manual'`: the answer's url gives the + // redirect away. + setManualFetchForTests(null); + vi.stubGlobal('fetch', vi.fn(async (url: string) => { + hits.push({ url }); + const response = json({ authorization_endpoint: `${EVIL}/authorize`, token_endpoint: `${EVIL}/token` }); + Object.defineProperty(response, 'url', { value: `${EVIL}/.well-known/oauth-authorization-server` }); + return response; + })); + + expect(await discoverOAuthMetadata(SERVER)).toBeNull(); + }); + + it('takes metadata from the server itself', async () => { + routes[`${SERVER}/.well-known/oauth-authorization-server`] = () => json({ + authorization_endpoint: `${IDP}/authorize`, + token_endpoint: `${IDP}/token`, + }); + + expect((await discoverOAuthMetadata(SERVER))?.token_endpoint).toBe(`${IDP}/token`); + }); +}); diff --git a/src/lib/__tests__/oauth.test.ts b/src/lib/__tests__/oauth.test.ts index c5fd4f2e..deed5e1e 100644 --- a/src/lib/__tests__/oauth.test.ts +++ b/src/lib/__tests__/oauth.test.ts @@ -6,6 +6,7 @@ const { mockSecureFetch } = vi.hoisted(() => ({ vi.mock('../client-cert', () => ({ secureFetch: mockSecureFetch, + fetchWithoutForeignRedirect: mockSecureFetch, })); import { forgetRefreshRotations, refreshOAuthAccessToken, resetRefreshRotations, type OAuthTokens } from '../oauth'; diff --git a/src/lib/__tests__/stored-token-refresh.test.ts b/src/lib/__tests__/stored-token-refresh.test.ts index 82cb6b2c..1682c6c7 100644 --- a/src/lib/__tests__/stored-token-refresh.test.ts +++ b/src/lib/__tests__/stored-token-refresh.test.ts @@ -5,7 +5,7 @@ const { mockSecureFetch, store } = vi.hoisted(() => ({ store: new Map(), })); -vi.mock('../client-cert', () => ({ secureFetch: mockSecureFetch })); +vi.mock('../client-cert', () => ({ secureFetch: mockSecureFetch, fetchWithoutForeignRedirect: mockSecureFetch })); vi.mock('../../api/jmap-client', () => ({ jmapClient: { getStoredCredentials: vi.fn(async (id: string) => (store.has(id) ? structuredClone(store.get(id)) : null)), diff --git a/src/lib/oauth-native.ts b/src/lib/oauth-native.ts index accf6858..9b8ed8dd 100644 --- a/src/lib/oauth-native.ts +++ b/src/lib/oauth-native.ts @@ -11,7 +11,7 @@ // the hand-off produces, so `connectWithOAuth` and the refresh path apply. import * as WebBrowser from 'expo-web-browser'; -import { secureFetch } from './client-cert'; +import { fetchWithoutForeignRedirect, followedOnlyAllowedRedirects, mayFollowRedirect, secureFetch } from './client-cert'; import { HANDOFF_REDIRECT_URI, HandoffError, HandoffCancelledError, type OAuthTokens } from './oauth'; import { generateCodeChallenge, @@ -51,7 +51,9 @@ function sameHost(a: string, b: string): boolean { /** * RFC 8414 / OIDC discovery on the mail server origin. Returns null when the * server advertises no authorization server (plain Stalwart without OAuth, - * or a non-Stalwart JMAP server). + * or a non-Stalwart JMAP server). The document decides where registration, + * the sign-in page and the code exchange go, so one reached through a + * redirect to another host doesn't count. */ export async function discoverOAuthMetadata(serverUrl: string): Promise { const base = serverUrl.replace(/\/+$/, ''); @@ -62,8 +64,11 @@ export async function discoverOAuthMetadata(serverUrl: string): Promise; if ( typeof data.authorization_endpoint === 'string' && @@ -239,9 +244,11 @@ export async function loginWithPkce( redirect_uri: redirectUri, code_verifier: verifier, }); + // The code and verifier follow a redirect only within the token endpoint's + // own host, which may be an identity provider's rather than the server's. let response: Response; try { - response = await secureFetch(metadata.token_endpoint, { + response = await fetchWithoutForeignRedirect(metadata.token_endpoint, { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json' }, body: body.toString(), @@ -284,7 +291,7 @@ export async function revokeRefreshToken(serverUrl: string, tokens: OAuthTokens) token_type_hint: 'refresh_token', client_id: tokens.clientId, }); - await secureFetch(endpoint, { + await fetchWithoutForeignRedirect(endpoint, { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, body: body.toString(), diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts index 3c0e2ef4..48a25031 100644 --- a/src/lib/oauth.ts +++ b/src/lib/oauth.ts @@ -1,5 +1,5 @@ import * as WebBrowser from 'expo-web-browser'; -import { secureFetch } from './client-cert'; +import { fetchWithoutForeignRedirect, secureFetch } from './client-cert'; import { randomHex } from './random'; // Webmail-mediated login. The app opens the webmail's normal login page with @@ -623,9 +623,12 @@ export async function refreshOAuthAccessToken(tokens: OAuthTokens): Promise