From f189e0bab66081e4bb271d7c804511333391ae3e Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Tue, 25 Aug 2026 18:45:35 +0200 Subject: [PATCH] fix: refresh account display name from the Stalwart principal on login/restore/switch #900 Stalwart seeds the default identity name from the principal "Full name" only once, and addAccount never rewrote displayName after first login, so an admin rename never showed up in Bulwark. Read the live name via x:Account/get through the passthrough (falling back to the identity name elsewhere) and prefer the synced registry name in account settings. --- components/settings/account-settings.tsx | 5 +- lib/__tests__/stalwart-principal.test.ts | 62 ++++++++++++ lib/stalwart/jmap-passthrough.ts | 20 +++- lib/stalwart/principal.ts | 50 ++++++++++ .../__tests__/auth-store-display-name.test.ts | 94 +++++++++++++++++++ stores/auth-store.ts | 53 ++++++++++- 6 files changed, 280 insertions(+), 4 deletions(-) create mode 100644 lib/__tests__/stalwart-principal.test.ts create mode 100644 lib/stalwart/principal.ts create mode 100644 stores/__tests__/auth-store-display-name.test.ts diff --git a/components/settings/account-settings.tsx b/components/settings/account-settings.tsx index 4845191cc..2571f08c2 100644 --- a/components/settings/account-settings.tsx +++ b/components/settings/account-settings.tsx @@ -54,7 +54,10 @@ export function AccountSettings() { const draggedIndexRef = useRef(null); const quotaPercentage = quota && quota.total > 0 ? Math.min(Math.round((quota.used / quota.total) * 100), 100) : 0; - const displayName = primaryIdentity?.name || account?.displayName || (isDemoMode ? 'Demo User' : undefined); + // `account.displayName` is refreshed from the server on every login/restore + // (Stalwart principal "Full name" when available - #900); the identity name + // is only a fallback until that entry exists. + const displayName = account?.displayName || primaryIdentity?.name || (isDemoMode ? 'Demo User' : undefined); const email = primaryIdentity?.email || account?.email || username; const max = getMaxAccounts(); diff --git a/lib/__tests__/stalwart-principal.test.ts b/lib/__tests__/stalwart-principal.test.ts new file mode 100644 index 000000000..fa738e7a4 --- /dev/null +++ b/lib/__tests__/stalwart-principal.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('@/lib/stalwart/jmap-passthrough', () => ({ + stalwartJmap: vi.fn(), + requireResult: (responses: Array<[string, unknown, string]>, method: string) => { + const match = responses.find((r) => r[0] === method); + if (!match) throw new Error('missing ' + method); + return match[1]; + }, +})); + +import { fetchPrincipalDisplayName } from '@/lib/stalwart/principal'; +import { stalwartJmap } from '@/lib/stalwart/jmap-passthrough'; + +const mockedJmap = stalwartJmap as unknown as ReturnType; + +const fakeClient = (opts: { stalwart?: boolean; accountId?: string } = {}) => + ({ + hasAccountCapability: (cap: string) => (opts.stalwart ?? true) && cap === 'urn:stalwart:jmap', + getAccountId: () => opts.accountId ?? 'acc-1', + }) as never; + +describe('fetchPrincipalDisplayName (#900)', () => { + beforeEach(() => { + mockedJmap.mockReset(); + }); + + it('reads the live principal description via x:Account/get on the given slot', async () => { + mockedJmap.mockResolvedValueOnce([ + ['x:Account/get', { list: [{ id: 'acc-1', description: ' Renamed User ' }] }, '0'], + ]); + + const name = await fetchPrincipalDisplayName(fakeClient(), 2); + + expect(name).toBe('Renamed User'); + expect(mockedJmap).toHaveBeenCalledWith( + [['x:Account/get', { accountId: 'acc-1', ids: ['acc-1'] }, '0']], + { slot: 2 }, + ); + }); + + it('skips the lookup entirely on non-Stalwart servers', async () => { + const name = await fetchPrincipalDisplayName(fakeClient({ stalwart: false })); + + expect(name).toBeNull(); + expect(mockedJmap).not.toHaveBeenCalled(); + }); + + it('returns null for an empty description so callers keep their fallback', async () => { + mockedJmap.mockResolvedValueOnce([ + ['x:Account/get', { list: [{ id: 'acc-1', description: '' }] }, '0'], + ]); + + expect(await fetchPrincipalDisplayName(fakeClient())).toBeNull(); + }); + + it('swallows passthrough errors (e.g. forbidden for this user)', async () => { + mockedJmap.mockRejectedValueOnce(new Error('forbidden')); + + await expect(fetchPrincipalDisplayName(fakeClient())).resolves.toBeNull(); + }); +}); diff --git a/lib/stalwart/jmap-passthrough.ts b/lib/stalwart/jmap-passthrough.ts index aeed3b85b..6261bc419 100644 --- a/lib/stalwart/jmap-passthrough.ts +++ b/lib/stalwart/jmap-passthrough.ts @@ -18,15 +18,31 @@ function buildError(message: string, status: number, methodError?: StalwartJmapE return err; } +export interface StalwartJmapOptions { + /** + * Cookie slot whose stored credentials the passthrough should use. + * Defaults to the active account's slot; pass it explicitly when calling + * on behalf of an account that is not (yet) active, e.g. during a + * background multi-account restore. + */ + slot?: number; +} + /** * Send a JMAP request to Stalwart via the server-side passthrough. * The passthrough injects the stored basic-auth header so credentials * stay in an httpOnly cookie. */ -export async function stalwartJmap(methodCalls: JmapMethodCall[]): Promise { +export async function stalwartJmap( + methodCalls: JmapMethodCall[], + options: StalwartJmapOptions = {}, +): Promise { + const slotHeaders = typeof options.slot === 'number' + ? { 'X-JMAP-Cookie-Slot': String(options.slot) } + : getActiveAccountSlotHeaders(); const response = await apiFetch('/api/account/stalwart/jmap', { method: 'POST', - headers: { ...getActiveAccountSlotHeaders(), 'Content-Type': 'application/json' }, + headers: { ...slotHeaders, 'Content-Type': 'application/json' }, body: JSON.stringify({ using: STALWART_JMAP_USING, methodCalls }), }); diff --git a/lib/stalwart/principal.ts b/lib/stalwart/principal.ts new file mode 100644 index 000000000..1da382baa --- /dev/null +++ b/lib/stalwart/principal.ts @@ -0,0 +1,50 @@ +import type { IJMAPClient } from '@/lib/jmap/client-interface'; +import { stalwartJmap, requireResult } from '@/lib/stalwart/jmap-passthrough'; +import { debug } from '@/lib/debug'; + +export const STALWART_JMAP_CAPABILITY = 'urn:stalwart:jmap'; + +interface PrincipalGetResponse { + list?: Array<{ description?: string | null }>; +} + +/** + * Live "Full name" of the authenticated Stalwart principal (#900). + * + * Stalwart seeds the default JMAP Identity's `name` from the principal + * description only once, when the identity is first created. An admin + * renaming the user afterwards never propagates to `Identity/get`, so the + * identity name is a stale snapshot. The current value is only exposed by + * the management method `x:Account/get`, which goes through the server-side + * passthrough (the browser never holds the credentials). + * + * `slot` addresses the account's cookie slot explicitly so the lookup works + * for accounts that are being restored in the background, not just the + * active one. + * + * Returns null when the server is not Stalwart, the call is not permitted + * for this user, or the principal has no description - callers fall back to + * the identity name in that case. + */ +export async function fetchPrincipalDisplayName( + client: IJMAPClient, + slot?: number, +): Promise { + if (!client.hasAccountCapability?.(STALWART_JMAP_CAPABILITY)) return null; + + try { + const accountId = client.getAccountId(); + const responses = await stalwartJmap( + [['x:Account/get', { accountId, ids: [accountId] }, '0']], + { slot }, + ); + const result = requireResult(responses, 'x:Account/get'); + const description = result.list?.[0]?.description; + const name = typeof description === 'string' ? description.trim() : ''; + return name || null; + } catch (error) { + // Not fatal: the cached display name (or the identity name) stays in use. + debug.warn('auth', 'Failed to read principal display name:', error); + return null; + } +} diff --git a/stores/__tests__/auth-store-display-name.test.ts b/stores/__tests__/auth-store-display-name.test.ts new file mode 100644 index 000000000..40aef4126 --- /dev/null +++ b/stores/__tests__/auth-store-display-name.test.ts @@ -0,0 +1,94 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('@/lib/stalwart/principal', () => ({ + fetchPrincipalDisplayName: vi.fn(), +})); + +import { syncAccountDisplayName } from '../auth-store'; +import { useAccountStore } from '../account-store'; +import { fetchPrincipalDisplayName } from '@/lib/stalwart/principal'; + +const mockedPrincipal = fetchPrincipalDisplayName as unknown as ReturnType; + +const SERVER = 'https://mail.example.com'; +const client = {} as never; + +function seedAccount(displayName = 'Old Name', label = displayName): string { + const store = useAccountStore.getState(); + const id = store.addAccount({ + label, + serverUrl: SERVER, + username: 'user@example.com', + authMode: 'basic', + rememberMe: true, + displayName, + email: 'user@example.com', + lastLoginAt: 0, + isConnected: true, + hasError: false, + isDefault: true, + }); + return id; +} + +// Issue #900: the registry entry is written once by addAccount and never +// refreshed, so a Stalwart admin renaming the user ("Full name") was never +// reflected in the account switcher / settings header. +describe('syncAccountDisplayName (#900)', () => { + beforeEach(() => { + mockedPrincipal.mockReset(); + useAccountStore.setState({ accounts: [], activeAccountId: null, defaultAccountId: null }); + }); + + it('replaces the cached name with the live Stalwart principal name', async () => { + const id = seedAccount('Old Name'); + mockedPrincipal.mockResolvedValueOnce('New Name'); + + await syncAccountDisplayName(id, client, 'Old Name'); + + const account = useAccountStore.getState().getAccountById(id)!; + expect(account.displayName).toBe('New Name'); + expect(account.label).toBe('New Name'); + expect(mockedPrincipal).toHaveBeenCalledWith(client, account.cookieSlot); + }); + + it('falls back to the identity name when the principal is unavailable', async () => { + const id = seedAccount('Old Name'); + mockedPrincipal.mockResolvedValueOnce(null); + + await syncAccountDisplayName(id, client, 'Identity Name'); + + expect(useAccountStore.getState().getAccountById(id)!.displayName).toBe('Identity Name'); + }); + + it('keeps the cached name when neither source yields one', async () => { + const id = seedAccount('Old Name'); + mockedPrincipal.mockResolvedValueOnce(null); + + await syncAccountDisplayName(id, client, ' '); + + expect(useAccountStore.getState().getAccountById(id)!.displayName).toBe('Old Name'); + }); + + it('leaves a diverged label alone', async () => { + const id = seedAccount('Old Name', 'Work'); + mockedPrincipal.mockResolvedValueOnce('New Name'); + + await syncAccountDisplayName(id, client); + + const account = useAccountStore.getState().getAccountById(id)!; + expect(account.displayName).toBe('New Name'); + expect(account.label).toBe('Work'); + }); + + it('is a no-op for unknown accounts and for unchanged names', async () => { + await syncAccountDisplayName('missing', client, 'Whatever'); + expect(mockedPrincipal).not.toHaveBeenCalled(); + + const id = seedAccount('Same'); + mockedPrincipal.mockResolvedValueOnce('Same'); + const before = useAccountStore.getState().getAccountById(id); + await syncAccountDisplayName(id, client); + expect(useAccountStore.getState().getAccountById(id)).toBe(before); + }); +}); diff --git a/stores/auth-store.ts b/stores/auth-store.ts index 90f90a22e..19a993b56 100644 --- a/stores/auth-store.ts +++ b/stores/auth-store.ts @@ -9,7 +9,8 @@ import { useVacationStore } from './vacation-store'; import { useCalendarStore } from './calendar-store'; import { useFilterStore } from './filter-store'; import { useSettingsStore } from './settings-store'; -import { useAccountStore } from './account-store'; +import { useAccountStore, type AccountEntry } from './account-store'; +import { fetchPrincipalDisplayName } from '@/lib/stalwart/principal'; import { fetchConfig } from '@/hooks/use-config'; import { debug } from '@/lib/debug'; import { generateAccountId } from '@/lib/account-utils'; @@ -131,6 +132,43 @@ function getClientRateLimitState(client: IJMAPClient | null): Pick { + const account = useAccountStore.getState().getAccountById(accountId); + if (!account) return; + + const name = (await fetchPrincipalDisplayName(client, account.cookieSlot)) + || fallbackName?.trim() + || ''; + if (!name) return; + + // Re-read: the entry may have changed (or been removed) during the request. + const current = useAccountStore.getState().getAccountById(accountId); + if (!current || current.displayName === name) return; + + const updates: Partial = { displayName: name }; + // `label` is seeded from the same value and never edited separately, so + // keep it in step unless it has diverged for some other reason. + if (!current.label || current.label === current.displayName) { + updates.label = name; + } + useAccountStore.getState().updateAccount(accountId, updates); +} + async function syncStalwartAuthContext( serverUrl: string, username: string, @@ -722,6 +760,7 @@ export const useAuthStore = create()( isDefault: accountStore.accounts.length === 0, }); accountStore.setActiveAccount(accountId); + void syncAccountDisplayName(accountId, client, primaryIdentity?.name); // Update account entry in case it already existed (addAccount is a no-op for existing accounts) accountStore.updateAccount(accountId, { @@ -948,6 +987,7 @@ export const useAuthStore = create()( accountStore.setActiveAccount(accountId); await syncStalwartAuthContext(serverUrl, username, client.getAuthHeader(), slot); + void syncAccountDisplayName(accountId, client, primaryIdentity?.name); set({ isAuthenticated: true, @@ -1087,6 +1127,7 @@ export const useAuthStore = create()( accountStore.setActiveAccount(accountId); await syncStalwartAuthContext(ssoServerUrl, username, client.getAuthHeader(), slot); + void syncAccountDisplayName(accountId, client, primaryIdentity?.name); set({ isAuthenticated: true, @@ -1606,6 +1647,7 @@ export const useAuthStore = create()( debug.error(`Failed to load data for ${accountId}:`, err); } } + void syncAccountDisplayName(accountId, targetClient, get().primaryIdentity?.name); // Sync settings fetchConfig().then(config => { @@ -1709,6 +1751,7 @@ export const useAuthStore = create()( scheduleRefresh(expires_in, get().refreshAccessToken, account.id); await contextSync; accountStore.updateAccount(account.id, { isConnected: true, hasError: false }); + void syncAccountDisplayName(account.id, client); } else if (res.status >= 500) { throw new TransientAuthError('Token refresh failed', res.status); } else { @@ -1725,6 +1768,7 @@ export const useAuthStore = create()( clients.set(account.id, client); await contextSync; accountStore.updateAccount(account.id, { isConnected: true, hasError: false }); + void syncAccountDisplayName(account.id, client); } else if (res.status >= 500) { throw new TransientAuthError('Session restore failed', res.status); } else { @@ -1924,6 +1968,7 @@ export const useAuthStore = create()( const { identities, primaryIdentity } = loadIdentities(await client.getIdentities(), state.username || ''); initializeFeatureStores(client); + void syncAccountDisplayName(accountId, client, primaryIdentity?.name); set({ isAuthenticated: true, @@ -1994,6 +2039,7 @@ export const useAuthStore = create()( const { identities, primaryIdentity } = loadIdentities(await client.getIdentities(), username); initializeFeatureStores(client); + void syncAccountDisplayName(accountId, client, primaryIdentity?.name); set({ isAuthenticated: true, @@ -2054,6 +2100,11 @@ export const useAuthStore = create()( const identities = identityState.identities; const primaryIdentity = identities[0] ?? null; set({ identities, primaryIdentity }); + + const { activeAccountId, client } = get(); + if (activeAccountId && client) { + void syncAccountDisplayName(activeAccountId, client, primaryIdentity?.name); + } }, refreshIdentities: async () => {