From a2caa4cca50dbb87c22ca6a00aae6edd694decd5 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 29 Oct 2026 02:44:00 +0100 Subject: [PATCH] fix: register the app's own client on Stalwart 1.0 behind another public address and say when the server can't be reached --- src/lib/__tests__/oauth-native.test.ts | 31 ++++++++++++++++++++ src/lib/__tests__/totp-login.test.ts | 35 +++++++++++++++++++++- src/lib/oauth-native.ts | 28 ++++++++++++++---- src/lib/totp-login.ts | 40 +++++++++++++++++--------- 4 files changed, 115 insertions(+), 19 deletions(-) diff --git a/src/lib/__tests__/oauth-native.test.ts b/src/lib/__tests__/oauth-native.test.ts index 2cc3c9dc..e28e50e7 100644 --- a/src/lib/__tests__/oauth-native.test.ts +++ b/src/lib/__tests__/oauth-native.test.ts @@ -102,6 +102,37 @@ describe('loginWithPkce on Stalwart', () => { expect(redirectUri).toBe(REGISTERED_REDIRECT_URI); }); + it('registers its client on 1.0 when the metadata names another public address', async () => { + const registered: string[] = []; + fetchMock.mockImplementation(async (url: string) => { + if (url.startsWith('https://mail.example.com/api/auth/client')) return answer({ error: 'invalid_client' }); + if (url.endsWith('/auth/register')) { + registered.push(url); + return answer({ client_id: 'swc1.pub' }, 201); + } + return answer({}, 404); + }); + const advertised: OAuthMetadata = { + ...stalwart, + issuer: 'https://stalwart.internal.example.net', + authorization_endpoint: 'https://stalwart.internal.example.net/login', + token_endpoint: 'https://stalwart.internal.example.net/auth/token', + registration_endpoint: 'https://stalwart.internal.example.net/auth/register', + }; + const { params } = await openedSession(advertised); + + expect(params.get('client_id')).toBe('swc1.pub'); + expect(registered).toEqual(['https://stalwart.internal.example.net/auth/register']); + }); + + it('says the server is unreachable instead of opening the browser with a client it may not know', async () => { + fetchMock.mockRejectedValue(new TypeError('Network request failed')); + const err = await loginWithPkce('https://mail.example.com', stalwart).catch((e) => e); + + expect(err.name).toBe('NetworkError'); + expect(mockOpenAuthSession).not.toHaveBeenCalled(); + }); + it('keeps the webmail client and the app scheme on 0.16', async () => { fetchMock.mockImplementation(async () => answer({ status: 404 }, 404)); const { params, redirectUri } = await openedSession(stalwart); diff --git a/src/lib/__tests__/totp-login.test.ts b/src/lib/__tests__/totp-login.test.ts index d661f0e3..62a80b0c 100644 --- a/src/lib/__tests__/totp-login.test.ts +++ b/src/lib/__tests__/totp-login.test.ts @@ -219,10 +219,43 @@ describe('resolveLoginClient', () => { resetLoginClients(); }); - it('keeps the webmail client when the server is unreachable, without remembering it', async () => { + it('says the server is unreachable, not that it refused the app, and remembers nothing', async () => { fetchMock.mockRejectedValueOnce(new TypeError('Network request failed')); + const err = await exchangePasswordForTokens(BASE, 'u', 'pw', '1').catch((e) => e); + + expect(err.name).toBe('NetworkError'); + expect(describeLoginError(err, { serverUrl: BASE }).title).toBe("Can't reach mail.example.com"); + expect(fetchMock.mock.calls.some(([url]) => url === `${BASE}/api/auth`)).toBe(false); + fakeServer('1.0'); + expect((await resolveLoginClient(BASE)).clientId).toBe('swc1.1'); + }); + + it.each([503, 429])('does not remember the webmail client after a %i from the client check', async (status) => { + fetchMock.mockResolvedValueOnce(json({ error: 'busy' }, status)); expect((await resolveLoginClient(BASE)).clientId).toBe('bulwark-webmail'); fakeServer('1.0'); expect((await resolveLoginClient(BASE)).clientId).toBe('swc1.1'); }); + + it('remembers the webmail client on a server without the client check', async () => { + fakeServer('0.16'); + await resolveLoginClient(BASE); + const calls = fakeServer('0.16'); + expect((await resolveLoginClient(BASE)).clientId).toBe('bulwark-webmail'); + expect(calls).toHaveLength(0); + }); + + it('registers at the endpoint the OAuth metadata names', async () => { + const urls: string[] = []; + fetchMock.mockImplementation(async (url: string) => { + urls.push(url.split('?')[0]); + if (url.startsWith(`${BASE}/api/auth/client`)) return json({ error: 'invalid_client' }); + if (url === 'https://public.example.com/auth/register') return json({ client_id: 'swc1.pub' }, 201); + return json({}, 404); + }); + const client = await resolveLoginClient(BASE, { registrationEndpoint: 'https://public.example.com/auth/register' }); + + expect(client.clientId).toBe('swc1.pub'); + expect(urls).toEqual([`${BASE}/api/auth/client`, 'https://public.example.com/auth/register']); + }); }); diff --git a/src/lib/oauth-native.ts b/src/lib/oauth-native.ts index f5df3754..accf6858 100644 --- a/src/lib/oauth-native.ts +++ b/src/lib/oauth-native.ts @@ -20,12 +20,14 @@ import { DEFAULT_CLIENT_ID, } from './totp-login'; import { randomHex } from './random'; +import { NetworkError } from '../api/jmap-client'; export interface OAuthMetadata { issuer?: string; authorization_endpoint: string; token_endpoint: string; revocation_endpoint?: string; + registration_endpoint?: string; scopes_supported?: string[]; prompt_values_supported?: string[]; } @@ -148,14 +150,22 @@ export function pkceScopes(metadata: OAuthMetadata): string { * The client the browser sign-in uses. When the server is its own * authorization server (Stalwart), the same check as the password sign-in * decides: 0.16 takes `bulwark-webmail` on `bulwarkmobile://`, 1.0 knows no - * such client and gets one the app registers. An external identity provider - * keeps the webmail's client id. + * such client and gets one the app registers. Stalwart may name its + * endpoints on the public address it is configured with rather than the one + * the user typed; it still lists its data scopes, and the client is + * registered where its metadata says. An external identity provider keeps + * the webmail's client id. */ async function pkceClient(serverUrl: string, metadata: OAuthMetadata): Promise<{ clientId: string; redirectUri: string }> { - if (sameHost(metadata.authorization_endpoint, serverUrl) && sameHost(metadata.token_endpoint, serverUrl)) { + const onServer = sameHost(metadata.authorization_endpoint, serverUrl) && sameHost(metadata.token_endpoint, serverUrl); + const stalwart = !!metadata.scopes_supported?.some((s) => typeof s === 'string' && s.startsWith(DATA_SCOPE_PREFIX)); + if (onServer || stalwart) { try { - return await resolveLoginClient(serverUrl); - } catch { + return await resolveLoginClient(serverUrl, { registrationEndpoint: registrationEndpoint(metadata) }); + } catch (err) { + // Unreachable: say so, rather than let the browser show the server + // refusing a client it doesn't know. + if (err instanceof NetworkError) throw err; // A refused registration or a redirect away: the browser shows what // the server says about the default client. } @@ -163,6 +173,14 @@ async function pkceClient(serverUrl: string, metadata: OAuthMetadata): Promise<{ return { clientId: DEFAULT_CLIENT_ID, redirectUri: HANDOFF_REDIRECT_URI }; } +function registrationEndpoint(metadata: OAuthMetadata): string | undefined { + if (metadata.registration_endpoint && /^https?:\/\//i.test(metadata.registration_endpoint)) { + return metadata.registration_endpoint; + } + const issuer = metadata.issuer ? /^(https?:\/\/[^/?#]+)/i.exec(metadata.issuer)?.[1] : undefined; + return issuer ? `${issuer}/auth/register` : undefined; +} + /** * Run the authorization-code + PKCE flow in the system browser. Throws * `HandoffCancelledError` when the user closes the browser. With `addAccount` diff --git a/src/lib/totp-login.ts b/src/lib/totp-login.ts index b14223a5..3c26caf7 100644 --- a/src/lib/totp-login.ts +++ b/src/lib/totp-login.ts @@ -148,9 +148,13 @@ function assertSameServer(requested: string, response: Response): void { } } -/** RFC 7591 registration as a public client, which 1.0 allows without credentials. */ -async function registerPublicClient(base: string): Promise { - const url = `${base}/auth/register`; +/** + * RFC 7591 registration as a public client, which 1.0 allows without + * credentials. `url` is the registration endpoint the server's OAuth metadata + * names, which may be on the public address it advertises rather than the one + * the user typed; the id is sealed with the server's key, so it serves both. + */ +async function registerPublicClient(base: string, url = `${base}/auth/register`): Promise { const t = withTimeout(CLIENT_CHECK_TIMEOUT_MS); let response: Response; try { @@ -188,11 +192,16 @@ async function registerPublicClient(base: string): Promise { * The OAuth client to sign in to `serverUrl` as. Stalwart 1.0's client check * (GET /api/auth/client, which 0.16 doesn't have) says whether it knows the * webmail's client id with the app's redirect URI; when it doesn't, the app - * registers its own public client. Any other server, and 0.16, gets - * `bulwark-webmail` as before. Unreachable: the default, uncached, and the - * request that needs it fails on its own. + * registers its own public client, at `registrationEndpoint` when the OAuth + * metadata names one. Any other server, and 0.16, gets `bulwark-webmail` as + * before. A server having a bad moment (5xx, 429) gets the default without + * it being remembered; an unreachable one throws `NetworkError`, as the + * default would only end in the server refusing a client it doesn't know. */ -export async function resolveLoginClient(serverUrl: string): Promise { +export async function resolveLoginClient( + serverUrl: string, + opts?: { registrationEndpoint?: string }, +): Promise { const base = trimBase(serverUrl); const cached = loginClients.get(base); if (cached) return cached; @@ -214,13 +223,14 @@ export async function resolveLoginClient(serverUrl: string): Promise= 500 || response.status === 429 || response.status === 408) return fallback; // 0.16 and servers that aren't Stalwart: no such endpoint. if (!response.ok) { rememberClient(base, fallback); @@ -233,7 +243,10 @@ export async function resolveLoginClient(serverUrl: string): Promise