fix: keep refresh tokens, sign-in codes and OAuth metadata from following a redirect to another host

src/lib/__tests__/oauth-pairing.test.ts

@@ -6,6 +6,7 @@ const { mockSecureFetch } = vi.hoisted(() => ({
 
 vi.mock('../client-cert', () => ({
   secureFetch: mockSecureFetch,
+  fetchWithoutForeignRedirect: mockSecureFetch,
 }));
 
 import {

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<string, () => 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<Response> {
+  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`);
+  });
+});

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';

src/lib/__tests__/stored-token-refresh.test.ts

@@ -5,7 +5,7 @@ const { mockSecureFetch, store } = vi.hoisted(() => ({
   store: new Map<string, unknown>(),
 }));
 
-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)),

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<OAuthMetadata | null> {
   const base = serverUrl.replace(/\/+$/, '');
@@ -62,8 +64,11 @@ export async function discoverOAuthMetadata(serverUrl: string): Promise<OAuthMet
   for (const url of urls) {
     const t = withTimeout(DISCOVERY_TIMEOUT_MS);
     try {
-      const res = await secureFetch(url, { headers: { Accept: 'application/json' }, signal: t.signal });
+      const res = await fetchWithoutForeignRedirect(url, { headers: { Accept: 'application/json' }, signal: t.signal });
       if (!res.ok) continue;
+      // Where no fetch could be told not to follow (see client-cert), the
+      // answer's url says whether it came from elsewhere.
+      if (res.url && !followedOnlyAllowedRedirects(res) && !mayFollowRedirect(url, res.url)) continue;
       const data = (await res.json()) as Partial<OAuthMetadata>;
       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(),

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<OAut
           refresh_token: tokens.refreshToken!,
           client_id: tokens.clientId,
         });
+        // The refresh token goes in the body: a redirect to another host than
+        // the token endpoint's own comes back unfollowed (a transient failure
+        // below) instead of taking the token along.
         let response: Response;
         try {
-          response = await secureFetch(tokens.tokenEndpoint, {
+          response = await fetchWithoutForeignRedirect(tokens.tokenEndpoint, {
             method: 'POST',
             headers: {
               'Content-Type': 'application/x-www-form-urlencoded',