fix: refresh TOTP-minted tokens with the default client id when no OAuth client is configured #873

app/api/auth/token/route.ts

@@ -8,7 +8,7 @@ import {
   encodeCachedAccessToken,
   decodeCachedAccessToken,
 } from '@/lib/oauth/tokens';
-import { exchangeCodeForTokens, buildOAuthParams, getMetadata, getTokenEndpoint } from '@/lib/oauth/token-exchange';
+import { exchangeCodeForTokens, buildOAuthParams, getMetadata, getTokenEndpoint, DEFAULT_CLIENT_ID } from '@/lib/oauth/token-exchange';
 import { getCookieOptions } from '@/lib/oauth/cookie-config';
 import { MAX_ACCOUNT_SLOTS } from '@/lib/account-utils';
 
@@ -117,12 +117,15 @@ export async function PUT(request: NextRequest) {
       }
     }
 
-    const tokenEndpoint = await getTokenEndpoint(serverId);
+    // The refresh token may have been minted by the password+TOTP login route,
+    // which works without a configured OAuth client by falling back to the
+    // default client id - refreshing must fall back the same way (#873).
+    const tokenEndpoint = await getTokenEndpoint(serverId, { fallbackClientId: DEFAULT_CLIENT_ID });
 
     const params = buildOAuthParams({
       grant_type: 'refresh_token',
       refresh_token: refreshToken,
-    }, serverId);
+    }, serverId, { fallbackClientId: DEFAULT_CLIENT_ID });
 
     const tokenResponse = await fetch(tokenEndpoint, {
       method: 'POST',
@@ -185,9 +188,9 @@ export async function DELETE(request: NextRequest) {
         if (token) {
           // Best-effort revocation
           try {
-            const metadata = await getMetadata(slotServerId).catch(() => null);
+            const metadata = await getMetadata(slotServerId, { fallbackClientId: DEFAULT_CLIENT_ID }).catch(() => null);
             if (metadata?.revocation_endpoint) {
-              const params = buildOAuthParams({ token, token_type_hint: 'refresh_token' }, slotServerId);
+              const params = buildOAuthParams({ token, token_type_hint: 'refresh_token' }, slotServerId, { fallbackClientId: DEFAULT_CLIENT_ID });
               await fetch(metadata.revocation_endpoint, {
                 method: 'POST',
                 headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
@@ -208,7 +211,7 @@ export async function DELETE(request: NextRequest) {
     const cookieStore = await cookies();
     const refreshToken = cookieStore.get(cookieName)?.value;
     const slotServerId = cookieStore.get(refreshTokenServerCookieName(slot))?.value || null;
-    const metadata = await getMetadata(slotServerId).catch((err) => {
+    const metadata = await getMetadata(slotServerId, { fallbackClientId: DEFAULT_CLIENT_ID }).catch((err) => {
       logger.warn('Failed to discover OAuth metadata during logout', {
         error: err instanceof Error ? err.message : 'Unknown error',
       });
@@ -220,7 +223,7 @@ export async function DELETE(request: NextRequest) {
         const params = buildOAuthParams({
           token: refreshToken,
           token_type_hint: 'refresh_token',
-        }, slotServerId);
+        }, slotServerId, { fallbackClientId: DEFAULT_CLIENT_ID });
 
         try {
           const revocationResponse = await fetch(metadata.revocation_endpoint, {

app/api/auth/totp-token-exchange/route.ts

@@ -10,6 +10,7 @@ import { recordLogin } from '@/lib/telemetry/login-tracker';
 import { parseJmapServers, findServerByUrl, findServerById } from '@/lib/admin/jmap-servers';
 import { MAX_ACCOUNT_SLOTS } from '@/lib/account-utils';
 import { generateCodeVerifier, generateCodeChallenge } from '@/lib/oauth/pkce';
+import { DEFAULT_CLIENT_ID } from '@/lib/oauth/token-exchange';
 
 /**
  * Exchange a password + (optional) TOTP code for OAuth tokens.
@@ -31,12 +32,6 @@ import { generateCodeVerifier, generateCodeChallenge } from '@/lib/oauth/pkce';
  * the (≈30s) code in every request.
  */
 
-// Fallback OAuth client id used when no client is configured. Stalwart accepts
-// any client id unless `require_client_registration` is enabled (default off);
-// when it is enabled the admin must configure `oauthClientId` with this
-// redirect URI registered.
-const DEFAULT_CLIENT_ID = 'bulwark-webmail';
-
 interface LoginResult {
   type?: string;
   // The response keeps snake_case: only the LoginResponse variant *tags* are

lib/__tests__/oauth-token-exchange.test.ts

@@ -0,0 +1,69 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+
+vi.mock('@/lib/logger', () => ({
+  logger: { warn: vi.fn(), error: vi.fn(), debug: vi.fn(), info: vi.fn() },
+}));
+
+vi.mock('@/lib/oauth/discovery', () => ({
+  discoverOAuth: vi.fn(),
+}));
+
+vi.mock('@/lib/security/url-guard', () => ({
+  isPublicHttpUrl: vi.fn(),
+}));
+
+vi.mock('@/lib/read-file-env', () => ({
+  readFileEnv: () => '',
+}));
+
+// No admin config set - every lookup falls through to its default, so the
+// env vars stubbed per-test are the only configuration source.
+vi.mock('@/lib/admin/config-manager', () => ({
+  configManager: {
+    get: (_key: string, def: unknown) => def,
+    ensureLoaded: async () => {},
+  },
+}));
+
+import { getRequiredConfig, buildOAuthParams, DEFAULT_CLIENT_ID } from '@/lib/oauth/token-exchange';
+
+describe('token-exchange client id fallback (#873)', () => {
+  beforeEach(() => {
+    vi.stubEnv('JMAP_SERVER_URL', 'https://mail.example.com');
+    vi.stubEnv('OAUTH_CLIENT_ID', '');
+    vi.stubEnv('OAUTH_ISSUER_URL', '');
+    vi.stubEnv('OAUTH_CLIENT_SECRET', '');
+  });
+
+  afterEach(() => {
+    vi.unstubAllEnvs();
+  });
+
+  it('still fails loudly without a fallback so OAuth-initiating flows surface misconfiguration', () => {
+    expect(() => getRequiredConfig(null)).toThrow(/OAUTH_CLIENT_ID/);
+  });
+
+  it('uses the fallback client id when no client is configured', () => {
+    const config = getRequiredConfig(null, { fallbackClientId: DEFAULT_CLIENT_ID });
+    expect(config.clientId).toBe('bulwark-webmail');
+    // Discovery must target the mail server itself - that is where the TOTP
+    // login route minted the token.
+    expect(config.discoveryUrl).toBe('https://mail.example.com');
+  });
+
+  it('prefers a configured client id over the fallback', () => {
+    vi.stubEnv('OAUTH_CLIENT_ID', 'configured-client');
+    const config = getRequiredConfig(null, { fallbackClientId: DEFAULT_CLIENT_ID });
+    expect(config.clientId).toBe('configured-client');
+  });
+
+  it('builds refresh params with the fallback client id', () => {
+    const params = buildOAuthParams(
+      { grant_type: 'refresh_token', refresh_token: 'rt' },
+      null,
+      { fallbackClientId: DEFAULT_CLIENT_ID },
+    );
+    expect(params.get('client_id')).toBe('bulwark-webmail');
+    expect(params.get('grant_type')).toBe('refresh_token');
+  });
+});

lib/__tests__/oauth-token-route.test.ts

@@ -26,6 +26,7 @@ vi.mock('@/lib/oauth/token-exchange', () => ({
   buildOAuthParams: (...args: unknown[]) => buildOAuthParams(...args),
   getMetadata: vi.fn().mockResolvedValue(null),
   getTokenEndpoint: (...args: unknown[]) => getTokenEndpoint(...args),
+  DEFAULT_CLIENT_ID: 'bulwark-webmail',
 }));
 
 /** Minimal in-memory stand-in for the Next.js cookie store. */
@@ -218,6 +219,27 @@ describe('oauth token route - access token cache (#552)', () => {
     expect(cookieStore.deleted).not.toContain('jmap_rt');
   });
 
+  it('refreshes with the default client id fallback so TOTP sessions survive reloads (#873)', async () => {
+    cookieStore.set('jmap_rt', 'totp-minted-refresh-token');
+    fetchMock.mockResolvedValue({
+      ok: true,
+      json: async () => ({ access_token: 'fresh-token', expires_in: 1800 }),
+    });
+
+    const { status } = await callPut({ force: 'true' });
+
+    expect(status).toBe(200);
+    // Tokens minted by the TOTP login route exist even when no OAuth client is
+    // configured; the refresh must offer the same default client id instead of
+    // failing on the missing OAUTH_CLIENT_ID.
+    expect(getTokenEndpoint).toHaveBeenCalledWith(null, { fallbackClientId: 'bulwark-webmail' });
+    expect(buildOAuthParams).toHaveBeenCalledWith(
+      { grant_type: 'refresh_token', refresh_token: 'totp-minted-refresh-token' },
+      null,
+      { fallbackClientId: 'bulwark-webmail' },
+    );
+  });
+
   it('does not serve a cached token once the refresh token is gone', async () => {
     seedCachedToken('orphan-token', 1500);
 

lib/oauth/token-exchange.ts

@@ -6,6 +6,23 @@ import { readFileEnv } from '@/lib/read-file-env';
 import { configManager } from '@/lib/admin/config-manager';
 import { parseJmapServers, findServerById } from '@/lib/admin/jmap-servers';
 
+// Fallback OAuth client id used when no client is configured. The password+TOTP
+// login route mints tokens against the mail server's built-in OAuth with this
+// id (Stalwart accepts any client id unless `require_client_registration` is
+// enabled), so refreshing/revoking those tokens must fall back to the same id
+// instead of failing on the missing OAUTH_CLIENT_ID (#873).
+export const DEFAULT_CLIENT_ID = 'bulwark-webmail';
+
+export interface ClientConfigOptions {
+  /**
+   * Client id to use when none is configured. Only pass this for operations on
+   * tokens that may have been minted by the TOTP login fallback; flows that
+   * initiate OAuth (authorize URL, code exchange, SSO) must keep failing loudly
+   * so a misconfiguration surfaces at login rather than as a broken session.
+   */
+  fallbackClientId?: string;
+}
+
 // SSRF guard for OAuth discovery. When `oauthAllowPrivateEndpoints` is set,
 // the admin opts in to discovery resolving to RFC-1918 / loopback hosts —
 // required for split-DNS deployments where the JMAP server's public hostname
@@ -34,14 +51,14 @@ function getServerEntry(serverId?: string | null) {
   return findServerById(servers, serverId);
 }
 
-export function getRequiredConfig(serverId?: string | null) {
+export function getRequiredConfig(serverId?: string | null, options?: ClientConfigOptions) {
   const entry = getServerEntry(serverId);
 
   const globalClientId = configManager.get<string>('oauthClientId', '') || process.env.OAUTH_CLIENT_ID;
   const globalServerUrl = configManager.get<string>('jmapServerUrl', '') || process.env.JMAP_SERVER_URL || process.env.NEXT_PUBLIC_JMAP_SERVER_URL;
   const globalIssuerUrl = configManager.get<string>('oauthIssuerUrl', '') || process.env.OAUTH_ISSUER_URL;
 
-  const clientId = entry?.oauth?.clientId || globalClientId;
+  const clientId = entry?.oauth?.clientId || globalClientId || options?.fallbackClientId;
   const serverUrl = entry?.url || globalServerUrl;
   const issuerUrl = entry?.oauth?.issuerUrl || globalIssuerUrl;
 
@@ -61,8 +78,8 @@ function getClientSecret(serverId?: string | null): string {
   return getGlobalClientSecret();
 }
 
-export async function getTokenEndpoint(serverId?: string | null): Promise<string> {
-  const { discoveryUrl } = getRequiredConfig(serverId);
+export async function getTokenEndpoint(serverId?: string | null, options?: ClientConfigOptions): Promise<string> {
+  const { discoveryUrl } = getRequiredConfig(serverId, options);
   const metadata = await discoverOAuth(discoveryUrl, { validateEndpoint: getDiscoveryValidator() });
   if (!metadata?.token_endpoint) {
     throw new Error('OAuth token endpoint not found');
@@ -70,13 +87,13 @@ export async function getTokenEndpoint(serverId?: string | null): Promise<string
   return metadata.token_endpoint;
 }
 
-export async function getMetadata(serverId?: string | null): Promise<OAuthMetadata | null> {
-  const { discoveryUrl } = getRequiredConfig(serverId);
+export async function getMetadata(serverId?: string | null, options?: ClientConfigOptions): Promise<OAuthMetadata | null> {
+  const { discoveryUrl } = getRequiredConfig(serverId, options);
   return discoverOAuth(discoveryUrl, { validateEndpoint: getDiscoveryValidator() });
 }
 
-export function buildOAuthParams(base: Record<string, string>, serverId?: string | null): URLSearchParams {
-  const { clientId } = getRequiredConfig(serverId);
+export function buildOAuthParams(base: Record<string, string>, serverId?: string | null, options?: ClientConfigOptions): URLSearchParams {
+  const { clientId } = getRequiredConfig(serverId, options);
   const params = new URLSearchParams({ ...base, client_id: clientId });
   const secret = getClientSecret(serverId);
   if (secret) {