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 () => {