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

@@ -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);

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']);
+  });
 });

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
+ * such client and gets one the app registers. Stalwart may name its
- * keeps the webmail's client id.
+ * 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);
+      return await resolveLoginClient(serverUrl, { registrationEndpoint: registrationEndpoint(metadata) });
-    } catch {
+    } 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`

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<LoginClient> {
+ * RFC 7591 registration as a public client, which 1.0 allows without
-  const url = `${base}/auth/register`;
+ * 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<LoginClient> {
   const t = withTimeout(CLIENT_CHECK_TIMEOUT_MS);
   let response: Response;
   try {
@@ -188,11 +192,16 @@ async function registerPublicClient(base: string): Promise<LoginClient> {
  * 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
+ * registers its own public client, at `registrationEndpoint` when the OAuth
- * `bulwark-webmail` as before. Unreachable: the default, uncached, and the
+ * metadata names one. Any other server, and 0.16, gets `bulwark-webmail` as
- * request that needs it fails on its own.
+ * 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<LoginClient> {
+export async function resolveLoginClient(
+  serverUrl: string,
+  opts?: { registrationEndpoint?: string },
+): Promise<LoginClient> {
   const base = trimBase(serverUrl);
   const cached = loginClients.get(base);
   if (cached) return cached;
@@ -214,13 +223,14 @@ export async function resolveLoginClient(serverUrl: string): Promise<LoginClient
       signal: t.signal,
       timeoutMs: CLIENT_CHECK_TIMEOUT_MS,
     });
-  } catch {
+  } catch (err) {
-    return fallback;
+    throw new NetworkError(err instanceof Error ? err.message : 'Server unreachable');
   } finally {
     t.done();
   }
   if (!response || typeof response.status !== 'number') return fallback;
   assertSameServer(url, response);
+  if (response.status >= 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<LoginClient
   }
   // `invalid_client` (unknown id) or a redirect URI the admin's client
   // doesn't list: either way the webmail's client isn't the app's to use.
-  if (typeof body.error === 'string' && body.error) return registerPublicClient(base);
+  if (typeof body.error === 'string' && body.error) {
+    const endpoint = opts?.registrationEndpoint;
+    return registerPublicClient(base, endpoint && /^https?:\/\//i.test(endpoint) ? endpoint : undefined);
+  }
   rememberClient(base, fallback);
   return fallback;
 }
@@ -404,9 +417,10 @@ export async function exchangePasswordForTokens(
     : await resolveLoginClient(base);
   let outcome = await requestLoginCode(base, username, password, totp, client, challenge);
   // A remembered client the server no longer knows (another server now
-  // answers at the address, or its key changed): ask again, once. 1.0 checks
+  // answers at the address, or its key changed), or the default taken while
-  // the client before the credentials, so this costs no failed sign-in.
+  // the client check failed: ask again, once. 1.0 checks the client before
-  if (outcome.kind === 'rejected' && outcome.error === 'invalid_client' && !opts?.clientId && loginClients.has(base)) {
+  // the credentials, so this costs no failed sign-in.
+  if (outcome.kind === 'rejected' && outcome.error === 'invalid_client' && !opts?.clientId) {
     loginClients.delete(base);
     const fresh = await resolveLoginClient(base);
     if (fresh.clientId !== client.clientId) {