fix: keep the password sign-in from following a redirect to another server

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

@@ -1,9 +1,12 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import * as WebBrowser from 'expo-web-browser';
 
-vi.mock('../client-cert', () => ({
+// One fake server behind both: the client check and registration go out
-  secureFetch: vi.fn(),
+// with fetchWithoutForeignRedirect.
-}));
+vi.mock('../client-cert', () => {
+  const fetchFn = vi.fn();
+  return { secureFetch: fetchFn, fetchWithoutForeignRedirect: fetchFn };
+});
 
 import { secureFetch } from '../client-cert';
 import { loginWithPkce, pkceScopes, type OAuthMetadata } from '../oauth-native';

src/lib/__tests__/totp-login.test.ts

@@ -1,11 +1,6 @@
-import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 
-vi.mock('../client-cert', async (importOriginal) => {
+import { setManualFetchForTests } from '../client-cert';
-  const actual = await importOriginal<typeof import('../client-cert')>();
-  return { ...actual, secureFetch: vi.fn() };
-});
-
-import { secureFetch } from '../client-cert';
 import {
   exchangePasswordForTokens,
   resetLoginClients,
@@ -15,15 +10,21 @@ import {
 } from '../totp-login';
 import { describeLoginError } from '../login-errors';
 
-const fetchMock = secureFetch as unknown as ReturnType<typeof vi.fn>;
+// The fetch that honours `redirect: 'manual'` (Expo's on a device). Tests that
+// model React Native's own fetch take it away.
+const fetchMock = vi.fn();
 const BASE = 'https://mail.example.com';
 
-function json(body: unknown, status = 200, extra?: { url?: string; redirected?: boolean }): Response {
+beforeEach(() => {
+  setManualFetchForTests(fetchMock as unknown as Parameters<typeof setManualFetchForTests>[0]);
+});
+afterEach(() => {
+  setManualFetchForTests(undefined);
+  vi.unstubAllGlobals();
+});
+
+function json(body: unknown, status = 200): Response {
   const res = new Response(JSON.stringify(body), { status, headers: { 'Content-Type': 'application/json' } });
-  if (extra) {
-    Object.defineProperty(res, 'url', { value: extra.url });
-    Object.defineProperty(res, 'redirected', { value: extra.redirected });
-  }
   return res;
 }
 
@@ -147,16 +148,68 @@ describe('exchangePasswordForTokens', () => {
     expect((await exchangePasswordForTokens(BASE, 'u', 'pw', '1').catch((e) => e)).code).toBe('totp_required');
   });
 
-  it('does not post the password when the server redirects to another host', async () => {
+  // The probe can't stand in for this check: it is skipped with a remembered
-    const calls: string[] = [];
+  // client, and a redirect may apply to /api/auth alone.
-    fetchMock.mockImplementation(async (url: string) => {
+  function redirectingServer(target: string) {
-      calls.push(url);
+    const received: Call[] = [];
-      return json({}, 404, { url: 'https://evil.example/api/auth/client', redirected: true });
+    const server = async (url: string, init?: RequestInit): Promise<Response> => {
-    });
+      const body = typeof init?.body === 'string' ? init.body : undefined;
-    const err = await exchangePasswordForTokens(BASE, 'u', 'pw', '1').catch((e) => e);
+      received.push({ url, method: init?.method ?? 'GET', body });
+      if (url === `${BASE}/api/auth`) {
+        return new Response(null, { status: 307, headers: { Location: `${target}/api/auth` } });
+      }
+      if (url.startsWith(target)) return json({ type: 'authenticated', client_code: 'stolen' });
+      return json({}, 404);
+    };
+    return { received, server };
+  }
+
+  it('does not follow a redirect of the password post to another host', async () => {
+    const { received, server } = redirectingServer('https://evil.example');
+    fetchMock.mockImplementation(server);
+    const err = await exchangePasswordForTokens(BASE, 'u', 'pw', '1', { clientId: 'bulwark-webmail' }).catch((e) => e);
 
     expect(err).toBeInstanceOf(TotpLoginError);
-    expect(calls.some((u) => u.endsWith('/api/auth'))).toBe(false);
+    expect(err.code).toBe('login_failed');
+    expect(fetchMock.mock.calls.every(([, init]) => init?.redirect === 'manual')).toBe(true);
+    expect(received.some((c) => c.url.startsWith('https://evil.example'))).toBe(false);
+  });
+
+  it('follows a redirect within the server, keeping the post', async () => {
+    fetchMock.mockImplementation(async (url: string, init?: RequestInit) => {
+      if (url === 'http://mail.example.com/api/auth') {
+        return new Response(null, { status: 308, headers: { Location: `${BASE}/api/auth` } });
+      }
+      if (url === `${BASE}/api/auth` && init?.method === 'POST') return json({ type: 'authenticated', client_code: 'c' });
+      if (url === 'http://mail.example.com/auth/token') return json({ access_token: 'at', refresh_token: 'rt' });
+      return json({}, 404);
+    });
+    const tokens = await exchangePasswordForTokens('http://mail.example.com', 'u', 'pw', '1', { clientId: 'bulwark-webmail' });
+
+    expect(tokens.accessToken).toBe('at');
+  });
+
+  it('refuses the answer when the platform fetch followed a redirect to another host', async () => {
+    // React Native's fetch follows redirects itself and never sets
+    // `redirected`; only `url` tells where the answer came from.
+    setManualFetchForTests(null);
+    const { received, server } = redirectingServer('https://evil.example');
+    vi.stubGlobal('fetch', async (url: string, init?: RequestInit) => {
+      let res = await server(url, init);
+      let at = url;
+      const location = res.headers.get('location');
+      if (res.status === 307 && location) {
+        at = location;
+        res = await server(location, init);
+      }
+      Object.defineProperty(res, 'url', { value: at });
+      return res;
+    });
+    const err = await exchangePasswordForTokens(BASE, 'u', 'pw', '1', { clientId: 'bulwark-webmail' }).catch((e) => e);
+
+    expect(err).toBeInstanceOf(TotpLoginError);
+    expect(err.code).toBe('login_failed');
+    expect(received.some((c) => c.url.endsWith('/auth/token'))).toBe(false);
   });
 });
 

src/lib/client-cert.ts

@@ -239,3 +239,67 @@ export async function secureFetch(
     return buildResponse(raw);
   }
 }
+
+// ── fetchWithoutForeignRedirect ──────────────────────────────
+
+type ManualFetch = (url: string, init: RequestInit) => Promise<Response>;
+
+// undefined: not looked up yet; null: not available (unit tests in Node).
+let manualFetch: ManualFetch | null | undefined;
+
+// React Native's own fetch (whatwg-fetch over XMLHttpRequest) can't be told
+// not to follow redirects: OkHttp and NSURLSession follow them, a 307/308
+// with the original method and body, OkHttp even from https to http. Expo's
+// fetch takes `redirect: 'manual'` and hands the 3xx back.
+function getManualFetch(): ManualFetch | null {
+  if (manualFetch !== undefined) return manualFetch;
+  try {
+    // eslint-disable-next-line @typescript-eslint/no-require-imports
+    const mod = require('expo/fetch') as { fetch?: unknown };
+    manualFetch = typeof mod.fetch === 'function' ? (mod.fetch as ManualFetch) : null;
+  } catch {
+    manualFetch = null;
+  }
+  return manualFetch;
+}
+
+/** Test hook: the fetch that honours `redirect: 'manual'` (Expo's on a device). */
+export function setManualFetchForTests(impl: ManualFetch | null | undefined): void {
+  manualFetch = impl;
+}
+
+/**
+ * `secureFetch` for requests that carry a secret in the body (a password, a
+ * PKCE verifier, an authorization code): they follow a redirect only within
+ * the same host or an http→https upgrade of it, as `secureFetch`'s native
+ * path does; any other 3xx comes back to the caller unfollowed. Where no
+ * fetch can be told so (Node, or Expo's missing), it falls back to the
+ * platform fetch, and the caller still checks `response.url` afterwards.
+ */
+export async function fetchWithoutForeignRedirect(
+  url: string,
+  init?: SecureFetchInit,
+): Promise<Response> {
+  const native = getNative();
+  if (native && (await getClientCertAlias())) return secureFetch(url, init);
+  const doFetch: ManualFetch = getManualFetch() ?? ((u, i) => fetch(u, i));
+  const { timeoutMs: _timeoutMs, ...rest } = init ?? {};
+  let currentUrl = url;
+  for (let hop = 0; ; hop++) {
+    const response = await doFetch(currentUrl, { ...rest, redirect: 'manual' });
+    const status = response.status;
+    const location = response.headers?.get?.('location');
+    if (
+      (status === 301 || status === 302 || status === 303 || status === 307 || status === 308) &&
+      location &&
+      hop < MAX_REDIRECTS
+    ) {
+      const next = resolveRedirect(currentUrl, location);
+      if (mayFollowRedirect(currentUrl, next)) {
+        currentUrl = next;
+        continue;
+      }
+    }
+    return response;
+  }
+}

src/lib/totp-login.ts

@@ -24,7 +24,7 @@
 // Token auth also survives TOTP rotation, unlike Basic auth, which would have
 // to embed the ~30 s code in every request.
 
-import { mayFollowRedirect, secureFetch } from './client-cert';
+import { fetchWithoutForeignRedirect, mayFollowRedirect } from './client-cert';
 import { sha256Hex } from './sha256';
 import { base64UrlEncode, randomBytes, randomHex } from './random';
 import { HANDOFF_REDIRECT_URI, type OAuthTokens } from './oauth';
@@ -133,11 +133,17 @@ function withTimeout(ms: number): { signal?: AbortSignal; done: () => void } {
 }
 
 // The password and the PKCE verifier must not follow a redirect to another
-// host. The platform fetch follows redirects itself, so the check is on where
+// host. The requests go out with fetchWithoutForeignRedirect, which hands such
-// the answer came from; the client check below runs first and is a plain GET,
+// a redirect back unfollowed; where the platform followed it anyway, the
-// which keeps the password from being posted anywhere the server redirects.
+// answer's `url` says so. React Native's fetch never sets `redirected`, only
+// `url` (from the XHR's responseURL), so that is what counts.
 function assertSameServer(requested: string, response: Response): void {
-  if (response.redirected && response.url && !mayFollowRedirect(requested, response.url)) {
+  const status = response.status;
+  if (status >= 300 && status < 400) {
+    const location = response.headers?.get?.('location');
+    if (location) throw new TotpLoginError('login_failed', `${requested} redirects to another host`);
+  }
+  if (response.url && !mayFollowRedirect(requested, response.url)) {
     throw new TotpLoginError('login_failed', `${requested} redirects to another host`);
   }
 }
@@ -148,7 +154,7 @@ async function registerPublicClient(base: string): Promise<LoginClient> {
   const t = withTimeout(CLIENT_CHECK_TIMEOUT_MS);
   let response: Response;
   try {
-    response = await secureFetch(url, {
+    response = await fetchWithoutForeignRedirect(url, {
       method: 'POST',
       headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
       body: JSON.stringify({
@@ -203,7 +209,7 @@ export async function resolveLoginClient(serverUrl: string): Promise<LoginClient
   const t = withTimeout(CLIENT_CHECK_TIMEOUT_MS);
   let response: Response;
   try {
-    response = await secureFetch(url, {
+    response = await fetchWithoutForeignRedirect(url, {
       headers: { Accept: 'application/json' },
       signal: t.signal,
       timeoutMs: CLIENT_CHECK_TIMEOUT_MS,
@@ -344,7 +350,7 @@ async function requestLoginCode(
   const url = `${base}/api/auth`;
   let loginResponse: Response;
   try {
-    loginResponse = await secureFetch(url, {
+    loginResponse = await fetchWithoutForeignRedirect(url, {
       method: 'POST',
       headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
       body: JSON.stringify(stalwartLoginBody({
@@ -434,7 +440,7 @@ export async function exchangePasswordForTokens(
   const tokenUrl = `${base}/auth/token`;
   let tokenResponse: Response;
   try {
-    tokenResponse = await secureFetch(tokenUrl, {
+    tokenResponse = await fetchWithoutForeignRedirect(tokenUrl, {
       method: 'POST',
       headers: { 'Content-Type': 'application/x-www-form-urlencoded', Accept: 'application/json' },
       body: tokenParams.toString(),