Merge pull request #55 from michaelbrusegard/fix/external-idp-handoff

fix(auth): verify external identity providers during handoff
2026-11-02 16:07:12 +00:00 · 2026-10-06 23:29:29 +02:00
parent 76180b332d e35cd82f5f
commit aa612e6f0e
2 changed files with 225 additions and 6 deletions
@@ -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<string, unknown>;
+let metadata: Record<string, unknown>;
+let bundle: Record<string, string>;
+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<Response>][] = [
+      ['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();
+});
@@ -104,8 +104,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);
 }
@@ -133,6 +133,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<boolean> {
+  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({
@@ -206,7 +260,7 @@ export async function runWebmailHandoff(
    if (!accessToken || !tokenEndpoint || !clientId) {
      throw new HandoffError('Sign-in response missing OAuth tokens');
    }
-    if (!isAcceptableTokenEndpoint(tokenEndpoint, serverUrl, webmailUrl)) {
+    if (!(await isTrustedHandoffEndpoint(tokenEndpoint, serverUrl, webmailUrl, clientId))) {
      throw new HandoffError('Sign-in response token endpoint is not trusted');
    }
    const refreshToken = params.get('refresh_token') ?? undefined;
@@ -455,9 +509,10 @@ export async function redeemPairingCode(webmailUrl: string, code: string): Promi
    if (!accessToken || !tokenEndpoint || !clientId) {
      throw new PairingError('bad_response', 'Pairing response missing token material', { host });
    }
-    // The mail server's own host, or the webmail's token proxy
-    // (`<webmail>/api/auth/pair/token`); nothing else gets the refresh token.
-    if (!isAcceptableTokenEndpoint(tokenEndpoint, serverUrl, base)) {
+    // The mail server's own host, the webmail's token proxy
+    // (`<webmail>/api/auth/pair/token`), or an external identity provider the
+    // webmail itself names; nothing else gets the refresh token.
+    if (!(await isTrustedHandoffEndpoint(tokenEndpoint, serverUrl, base, clientId))) {
      throw new PairingError('untrusted', 'Pairing response token endpoint is not trusted', { host });
    }
    const expiresIn = typeof data.expires_in === 'number' && Number.isFinite(data.expires_in) ? data.expires_in : undefined;