From d345d15b0a844b7cbb54a0e85fcf12dfd5ac273a Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Thu, 5 Nov 2026 01:35:54 +0100 Subject: [PATCH] fix: sign in on stalwart 1.0.3, which refuses the bulwark-mobile client's scopes --- src/lib/__tests__/oauth-native.test.ts | 46 ++++++++++++++++++ src/lib/__tests__/totp-login.test.ts | 45 ++++++++++++++++-- src/lib/oauth-native.ts | 19 ++++++-- src/lib/totp-login.ts | 64 +++++++++++++++++++------- 4 files changed, 151 insertions(+), 23 deletions(-) diff --git a/src/lib/__tests__/oauth-native.test.ts b/src/lib/__tests__/oauth-native.test.ts index e28e50e7..4c020b35 100644 --- a/src/lib/__tests__/oauth-native.test.ts +++ b/src/lib/__tests__/oauth-native.test.ts @@ -151,6 +151,52 @@ describe('loginWithPkce on Stalwart', () => { expect(scopes).not.toContain('profile'); }); + // 1.0.3 dropped first-party clients: the bulwark-mobile client the webmail + // setup registered now allows only openid and offline_access, and refuses + // the whole request with invalid_scope. + const stalwart103: OAuthMetadata = { + ...stalwart, + scopes_supported: [ + ...stalwart.scopes_supported!, + 'urn:stalwart:oauth:scope:security', + 'urn:stalwart:oauth:scope:settings', + 'urn:stalwart:oauth:scope:full', + ], + }; + + it('registers its own client on 1.0.3 when bulwark-mobile does not allow the scopes it asks for', async () => { + const checked: URLSearchParams[] = []; + fetchMock.mockImplementation(async (url: string) => { + if (url.includes('/api/auth/client')) { + const q = new URL(url).searchParams; + checked.push(q); + if (q.get('client_id') !== 'bulwark-mobile') return answer({ error: 'invalid_client' }); + const extra = (q.get('scope') ?? '').split(' ').filter((s) => s && s !== 'openid' && s !== 'offline_access'); + return extra.length + ? answer({ error: 'invalid_request', redirect: true, location: `${REGISTERED_REDIRECT_URI}?error=invalid_scope` }) + : answer({ name: 'Bulwark Mobile (auto-configured)', scopes: ['Full access to your account'] }); + } + if (url.endsWith('/auth/register')) return answer({ client_id: 'swc1.own' }, 201); + return answer({}, 404); + }); + const { params } = await openedSession(stalwart103); + + expect(params.get('client_id')).toBe('swc1.own'); + // The check asked with the scopes the browser then asks for. + expect(checked[0].get('scope')).toBe(params.get('scope')); + }); + + it("asks 1.0.3 for the account's settings, but not for full access", async () => { + fetchMock.mockImplementation(async () => answer({ status: 404 }, 404)); + const { params } = await openedSession(stalwart103); + const scopes = params.get('scope')!.split(' '); + + expect(scopes).toEqual(expect.arrayContaining(['urn:stalwart:oauth:scope:security', 'urn:stalwart:oauth:scope:settings'])); + expect(scopes).not.toContain('urn:stalwart:oauth:scope:full'); + // A server that doesn't list them isn't asked for them. + expect(pkceScopes(stalwart)).not.toContain('urn:stalwart:'); + }); + it('asks another identity provider for the usual OpenID scopes', () => { expect(pkceScopes({ ...metadata, scopes_supported: ['openid', 'email', 'profile', 'offline_access', 'groups'] })) .toBe('openid email profile offline_access'); diff --git a/src/lib/__tests__/totp-login.test.ts b/src/lib/__tests__/totp-login.test.ts index f2f5c424..d1a3b8a8 100644 --- a/src/lib/__tests__/totp-login.test.ts +++ b/src/lib/__tests__/totp-login.test.ts @@ -34,16 +34,26 @@ interface Call { url: string; method: string; body?: string } // A fake Stalwart of either line. `v1` knows no client but the ones it // registered (ids `swc1.`) and those in `admin` (registered by an admin // for the given redirect URI), takes `{query, consent}` and answers with a -// location; 0.16 has no client check and answers `client_code`. +// location; 0.16 has no client check and answers `client_code`. An admin +// client listed in `adminScopes` allows only those scopes besides openid and +// offline_access (1.0.3, which has no first-party clients), and a request +// naming another is refused with invalid_scope; any other client allows all. function fakeServer( version: '0.16' | '1.0', answer?: (query: URLSearchParams) => unknown, admin: Record = {}, + adminScopes: Record = {}, ) { const calls: Call[] = []; let registered = 0; const knows = (clientId: string, redirectUri: string | null) => clientId.startsWith('swc1.') || (clientId in admin && admin[clientId] === redirectUri); + const allows = (clientId: string, scope: string | null) => { + const allowed = adminScopes[clientId]; + if (!allowed) return true; + return (scope ?? '').split(' ').every((s) => !s || s === 'openid' || s === 'offline_access' || allowed.includes(s)); + }; + const scopeRefused = (redirectUri: string | null) => `${redirectUri}?error=invalid_scope&error_description=The+requested+scope+is+not+allowed+for+this+client.`; fetchMock.mockImplementation(async (url: string, init?: RequestInit) => { const call = { url, method: init?.method ?? 'GET', body: typeof init?.body === 'string' ? init.body : undefined }; calls.push(call); @@ -51,9 +61,13 @@ function fakeServer( if (path === '/api/auth/client') { if (version === '0.16') return json({ type: 'about:blank', status: 404 }, 404); const params = new URL(url).searchParams; - return json(knows(params.get('client_id') ?? '', params.get('redirect_uri')) + const clientId = params.get('client_id') ?? ''; + if (!knows(clientId, params.get('redirect_uri'))) { + return json({ error: 'invalid_client', description: 'Unknown client id', redirect: false }); + } + return json(allows(clientId, params.get('scope')) ? { name: 'Bulwark Mobile' } - : { error: 'invalid_client', description: 'Unknown client id', redirect: false }); + : { error: 'invalid_request', redirect: true, location: scopeRefused(params.get('redirect_uri')) }); } if (path === '/auth/register') { if (version === '0.16') return json({ error: 'invalid_redirect_uri' }, 400); @@ -67,6 +81,7 @@ function fakeServer( const query = new URLSearchParams(body.query); if (answer) return json(answer(query)); if (!knows(query.get('client_id') ?? '', query.get('redirect_uri'))) return json({ type: 'invalid', error: 'invalid_client', description: 'Unknown client id' }); + if (!allows(query.get('client_id') ?? '', query.get('scope'))) return json({ type: 'redirect', location: scopeRefused(query.get('redirect_uri')) }); return json({ type: 'authenticated', location: `${query.get('redirect_uri')}?code=code-10&state=${query.get('state')}&iss=x`, @@ -153,6 +168,30 @@ describe('exchangePasswordForTokens', () => { expect(tokenCall(calls).has('client_secret')).toBe(false); }); + it('registers its own client on Stalwart 1.0.3, where bulwark-mobile lost its first-party access', async () => { + // 1.0.3 has no first-party clients: bulwark-mobile now allows only openid + // and offline_access and refuses the app's request with invalid_scope. + const calls = fakeServer('1.0', undefined, { 'bulwark-mobile': REGISTERED_REDIRECT_URI }, { 'bulwark-mobile': [] }); + const tokens = await exchangePasswordForTokens(BASE, 'u@example.org', 'pw', '123456'); + + expect(tokens.clientId).toBe('swc1.1'); + expect(authCalls(calls)).toHaveLength(1); + const register = JSON.parse(calls.find((c) => c.url.endsWith('/auth/register'))!.body!); + expect(register.scope).toContain('urn:stalwart:oauth:scope:security'); + expect(register.scope).not.toContain('urn:stalwart:oauth:scope:full'); + }); + + it('asks again when a remembered bulwark-mobile no longer allows the scopes, without a second password check failing', async () => { + fakeServer('1.0', undefined, { 'bulwark-mobile': REGISTERED_REDIRECT_URI }); + await exchangePasswordForTokens(BASE, 'u@example.org', 'pw', '123456'); + // The server is upgraded to 1.0.3 while the app keeps running. + const calls = fakeServer('1.0', undefined, { 'bulwark-mobile': REGISTERED_REDIRECT_URI }, { 'bulwark-mobile': [] }); + const tokens = await exchangePasswordForTokens(BASE, 'u@example.org', 'pw', '654321'); + + expect(tokens.clientId).toBe('swc1.1'); + expect(tokenCall(calls).get('client_id')).toBe('swc1.1'); + }); + it('keeps the registered client per server for the next sign-in', async () => { fakeServer('1.0'); await exchangePasswordForTokens(BASE, 'u@example.org', 'pw', '123456'); diff --git a/src/lib/oauth-native.ts b/src/lib/oauth-native.ts index 3a0a2e3b..4528c44c 100644 --- a/src/lib/oauth-native.ts +++ b/src/lib/oauth-native.ts @@ -14,9 +14,11 @@ import * as WebBrowser from 'expo-web-browser'; import { fetchWithoutForeignRedirect, followedOnlyAllowedRedirects, mayFollowRedirect, secureFetch } from './client-cert'; import { HANDOFF_REDIRECT_URI, HandoffError, HandoffCancelledError, type OAuthTokens } from './oauth'; import { + forgetLoginClient, generateCodeChallenge, generateCodeVerifier, resolveLoginClient, + STALWART_ACCOUNT_SCOPES, DEFAULT_CLIENT_ID, MOBILE_CLIENT_ID, REGISTERED_LOGOUT_REDIRECT_URI, @@ -142,7 +144,8 @@ const DATA_SCOPE_PREFIX = 'urn:ietf:params:oauth:scope:'; * advertises OAuth data scopes is Stalwart: 1.0 gives a client it doesn't * treat as first-party only what these name, and refuses a scope it doesn't * list (`email`, `profile`) with `invalid_scope`. For such a provider the - * scopes it doesn't list are dropped and every data scope it lists is added; + * scopes it doesn't list are dropped and every data scope it lists is added, + * with the account-settings scopes 1.0.3 lists (STALWART_ACCOUNT_SCOPES); * any other provider gets the usual OIDC set. */ export function pkceScopes(metadata: OAuthMetadata): string { @@ -151,7 +154,8 @@ export function pkceScopes(metadata: OAuthMetadata): string { if (!supported?.length) return wanted.join(' '); const kept = wanted.filter((s) => supported.includes(s)); const data = supported.filter((s) => typeof s === 'string' && s.startsWith(DATA_SCOPE_PREFIX)); - if (data.length) return [...new Set([...kept, ...data])].join(' '); + const account = STALWART_ACCOUNT_SCOPES.filter((s) => supported.includes(s)); + if (data.length) return [...new Set([...kept, ...data, ...account])].join(' '); return kept.join(' ') || supported.join(' '); } @@ -170,7 +174,10 @@ async function pkceClient(serverUrl: string, metadata: OAuthMetadata): Promise<{ const stalwart = !!metadata.scopes_supported?.some((s) => typeof s === 'string' && s.startsWith(DATA_SCOPE_PREFIX)); if (onServer || stalwart) { try { - return await resolveLoginClient(serverUrl, { registrationEndpoint: registrationEndpoint(metadata) }); + return await resolveLoginClient(serverUrl, { + registrationEndpoint: registrationEndpoint(metadata), + scope: pkceScopes(metadata), + }); } catch (err) { // Unreachable: say so, rather than let the browser show the server // refusing a client it doesn't know. @@ -232,7 +239,11 @@ export async function loginWithPkce( const cb = parseCallbackParams(result.url); const err = cb.get('error'); - if (err) throw new HandoffError(cb.get('error_description') || err); + if (err) { + // A remembered client the server no longer takes: the next try asks again. + if (!opts?.clientId && (err === 'invalid_client' || err === 'invalid_scope')) forgetLoginClient(serverUrl); + throw new HandoffError(cb.get('error_description') || err); + } if (cb.get('state') !== state) throw new HandoffError('State mismatch'); const code = cb.get('code'); if (!code) throw new HandoffError('Sign-in response missing authorization code'); diff --git a/src/lib/totp-login.ts b/src/lib/totp-login.ts index e6ef430e..b0b0fcd4 100644 --- a/src/lib/totp-login.ts +++ b/src/lib/totp-login.ts @@ -76,7 +76,20 @@ export const STALWART_DATA_SCOPES = [ 'urn:ietf:params:oauth:scope:files', ]; -export const STALWART_LOGIN_SCOPE = ['openid', 'offline_access', ...STALWART_DATA_SCOPES].join(' '); +/** + * Stalwart 1.0.3's scopes for the account's own settings (password, 2FA, app + * passwords, encryption keys), which until then only a first-party client + * reached. 1.0.3 dropped first-party clients, so `bulwark-mobile` too gets + * only what its scopes name. `urn:stalwart:oauth:scope:full` isn't asked for: + * only a client a superuser allowed it may, and a request naming a scope the + * client lacks is refused as a whole. Earlier servers skip these names. + */ +export const STALWART_ACCOUNT_SCOPES = [ + 'urn:stalwart:oauth:scope:security', + 'urn:stalwart:oauth:scope:settings', +]; + +export const STALWART_LOGIN_SCOPE = ['openid', 'offline_access', ...STALWART_DATA_SCOPES, ...STALWART_ACCOUNT_SCOPES].join(' '); export class LoginEndpointMissingError extends Error { constructor() { @@ -137,6 +150,11 @@ export function resetLoginClients(): void { loginClients.clear(); } +/** Drop the client remembered for `serverUrl`, which the server just refused. */ +export function forgetLoginClient(serverUrl: string): void { + loginClients.delete(trimBase(serverUrl)); +} + function rememberClient(base: string, client: LoginClient): void { if (!loginClients.has(base) && loginClients.size >= CLIENT_CACHE_MAX) { const oldest = loginClients.keys().next().value; @@ -209,14 +227,18 @@ async function registerPublicClient( /** * Stalwart 1.0's client check (GET /api/auth/client, which 0.16 doesn't - * have): does the server take `clientId` on `redirectUri`? `busy` for a - * server having a bad moment (5xx, 429), `none` where there is no such - * check; an unreachable server throws `NetworkError`. + * have): does the server take `clientId` on `redirectUri` asking for + * `scope`? A client registered without the scopes the app asks for + * (`bulwark-mobile` on 1.0.3, which dropped first-party clients) counts as + * unknown, since the server refuses the whole request with `invalid_scope`. + * `busy` for a server having a bad moment (5xx, 429), `none` where there is + * no such check; an unreachable server throws `NetworkError`. */ async function checkClient( base: string, clientId: string, redirectUri: string, + scope: string, signal?: AbortSignal, ): Promise<'known' | 'unknown' | 'none' | 'busy'> { const query = new URLSearchParams({ @@ -224,6 +246,7 @@ async function checkClient( client_id: clientId, redirect_uri: redirectUri, state: 'probe', + scope, code_challenge: 'A'.repeat(43), code_challenge_method: 'S256', }); @@ -249,7 +272,8 @@ async function checkClient( if (!response.ok) return 'none'; const body = (await response.json().catch(() => null)) as { error?: unknown } | null; if (!body || typeof body !== 'object') return 'none'; - // `invalid_client` (unknown id) or a redirect URI the client doesn't list. + // `invalid_client` (unknown id), a redirect URI the client doesn't list, + // or (as a `redirect` carrying `invalid_scope`) a scope it doesn't allow. return typeof body.error === 'string' && body.error ? 'unknown' : 'known'; } @@ -258,22 +282,25 @@ async function checkClient( * `bulwark-mobile` where the admin's OAuth setup registered it, else the * webmail's client if it takes the app's redirect URI, else a public client * the app registers itself, 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. + * metadata names one. Either of the first two counts only if it allows + * `scope`, the scopes the sign-in will ask for. 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, - opts?: { registrationEndpoint?: string; signal?: AbortSignal }, + opts?: { registrationEndpoint?: string; scope?: string; signal?: AbortSignal }, ): Promise { const base = trimBase(serverUrl); const cached = loginClients.get(base); if (cached) return cached; const fallback: LoginClient = { clientId: DEFAULT_CLIENT_ID, redirectUri: HANDOFF_REDIRECT_URI }; const signal = opts?.signal; + const scope = opts?.scope ?? STALWART_LOGIN_SCOPE; - const own = await checkClient(base, MOBILE_CLIENT_ID, REGISTERED_REDIRECT_URI, signal); + const own = await checkClient(base, MOBILE_CLIENT_ID, REGISTERED_REDIRECT_URI, scope, signal); if (own === 'busy') return fallback; if (own === 'known') { const client = { clientId: MOBILE_CLIENT_ID, redirectUri: REGISTERED_REDIRECT_URI }; @@ -281,7 +308,7 @@ export async function resolveLoginClient( return client; } if (own === 'unknown') { - const shared = await checkClient(base, DEFAULT_CLIENT_ID, HANDOFF_REDIRECT_URI, signal); + const shared = await checkClient(base, DEFAULT_CLIENT_ID, HANDOFF_REDIRECT_URI, scope, signal); if (shared === 'busy') return fallback; if (shared === 'unknown') { const endpoint = opts?.registrationEndpoint; @@ -502,10 +529,15 @@ export async function exchangePasswordForTokens( : await resolveLoginClient(base, { signal }); let outcome = await requestLoginCode(base, username, password, totp, client, challenge, signal); // A remembered client the server no longer knows (another server now - // answers at the address, or its key changed), or the default taken while - // the client check failed: ask again, once. 1.0 checks the client before - // the credentials, so this costs no failed sign-in. - if (outcome.kind === 'rejected' && outcome.error === 'invalid_client' && !opts?.clientId) { + // answers at the address, or its key changed) or no longer gives the + // scopes (bulwark-mobile after the upgrade to 1.0.3), or the default taken + // while the client check failed: ask again, once. 1.0 checks the client + // before the credentials, so this costs no failed sign-in. + if ( + outcome.kind === 'rejected' + && (outcome.error === 'invalid_client' || outcome.error === 'invalid_scope') + && !opts?.clientId + ) { loginClients.delete(base); const fresh = await resolveLoginClient(base, { signal }); if (fresh.clientId !== client.clientId) {