diff --git a/src/api/account-security.ts b/src/api/account-security.ts index 01e3bb01..e0e41447 100644 --- a/src/api/account-security.ts +++ b/src/api/account-security.ts @@ -284,9 +284,40 @@ export async function removePublicKey(id: string): Promise { if (failure) throw new Error(failure.description || failure.type || 'Failed to remove public key'); } +/** + * The account's "Full name": the `description` of `x:AccountSettings`, which + * every user can read (webmail #900). Null when it is not set. + */ +export async function fetchAccountDisplayName(): Promise { + const accountId = jmapClient.accountId; + const responses = await send([['x:AccountSettings/get', { accountId, ids: ['singleton'] }, '0']]); + const result = resultFor<{ list?: Array<{ description?: string | null }> }>(responses, '0'); + const description = result.list?.[0]?.description; + return typeof description === 'string' && description.trim() ? description.trim() : null; +} + +// Stalwart only lets admins read their own principal with x:Account/get; +// everyone else is refused, every time. A refusal is remembered per account +// for the session, so the alias lookups don't keep asking. +const PRINCIPAL_REFUSALS = new Set(['forbidden', 'unknownMethod', 'accountNotSupportedByMethod']); +const principalRefused = new Set(); + +function principalKey(accountId: string): string { + return `${jmapClient.serverUrl ?? ''}|${accountId}`; +} + +/** Test hook. */ +export function resetPrincipalRefusals(): void { + principalRefused.clear(); +} + export async function fetchPrincipal(): Promise { const accountId = jmapClient.accountId; + const key = principalKey(accountId); + if (principalRefused.has(key)) throw new Error('forbidden'); const responses = await send([['x:Account/get', { accountId, ids: [accountId] }, '0']]); + const refusal = responses.find((r) => r[2] === '0' && r[0] === 'error')?.[1] as { type?: string } | undefined; + if (refusal?.type && PRINCIPAL_REFUSALS.has(refusal.type)) principalRefused.add(key); const result = resultFor<{ list?: Array<{ description?: string | null; diff --git a/src/components/email/CalendarInvitationBanner.tsx b/src/components/email/CalendarInvitationBanner.tsx index ba47a99a..4479c36f 100644 --- a/src/components/email/CalendarInvitationBanner.tsx +++ b/src/components/email/CalendarInvitationBanner.tsx @@ -68,11 +68,10 @@ export function CalendarInvitationBanner({ email, jmapAccountId }: Props) { const subscriptions = useCalendarSubscriptionsStore((s) => s.subscriptions); const importEvents = useCalendarStore((s) => s.importEvents); const rsvpEvent = useCalendarStore((s) => s.rsvpEvent); - // Login address + identities + aliases, so invitations addressed to an - // alias still show the RSVP buttons. - const currentUserEmails = useUserCalendarAddresses(); - const attachment = React.useMemo(() => findCalendarAttachment(email), [email]); + // Login address + identities + aliases, so invitations addressed to an + // alias still show the RSVP buttons. Only an invitation looks the aliases up. + const currentUserEmails = useUserCalendarAddresses(!!attachment && enabled); const [state, setState] = React.useState('loading'); const [event, setEvent] = React.useState | null>(null); diff --git a/src/lib/__tests__/calendar-user-addresses.test.ts b/src/lib/__tests__/calendar-user-addresses.test.ts new file mode 100644 index 00000000..2812642a --- /dev/null +++ b/src/lib/__tests__/calendar-user-addresses.test.ts @@ -0,0 +1,86 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// x:Account/get (the principal, for the account aliases) is refused to +// everyone but admins: it used to go out on every start and on the first +// message opened, invitation or not (PF6). + +const request = vi.fn(); +vi.mock('../../api/jmap-client', () => ({ + jmapClient: { + isConnected: true, + accountId: 'acc-1', + serverUrl: 'https://mail.example.com', + currentSession: { apiUrl: 'https://mail.example.com/jmap/' }, + hasAccountCapability: () => true, + request: (...args: unknown[]) => request(...args), + }, +})); + +// Run effects straight away, like a mount. +vi.mock('react', () => ({ + default: { + useEffect: (effect: () => void) => { effect(); }, + useReducer: () => [0, () => undefined], + useMemo: (fn: () => T) => fn(), + }, +})); + +vi.mock('../../stores/account-store', () => ({ + useAccountStore: (select: (s: unknown) => unknown) => select({ getActiveAccount: () => ({ email: 'me@example.com' }) }), +})); +vi.mock('../../stores/settings-store', () => ({ + useSettingsStore: (select: (s: unknown) => unknown) => select({ identities: [] }), +})); + +import { useUserCalendarAddresses, resetUserCalendarAddressCache } from '../calendar-user-addresses'; +import { fetchAccountDisplayName, fetchPrincipal, resetPrincipalRefusals } from '../../api/account-security'; + +const forbidden = { methodResponses: [['error', { type: 'forbidden' }, '0']] }; + +beforeEach(() => { + request.mockReset(); + resetUserCalendarAddressCache(); + resetPrincipalRefusals(); +}); + +describe('account aliases for calendar invitations', () => { + it('does not look the aliases up for a message without an invitation', () => { + expect(useUserCalendarAddresses(false)).toEqual(['me@example.com']); + expect(request).not.toHaveBeenCalled(); + }); + + it('asks once, and never again this session once refused', async () => { + request.mockResolvedValue(forbidden); + useUserCalendarAddresses(true); + await new Promise((r) => setTimeout(r, 0)); + expect(request).toHaveBeenCalledTimes(1); + expect(request.mock.calls[0][0]).toEqual([['x:Account/get', { accountId: 'acc-1', ids: ['acc-1'] }, '0']]); + // Every later view needing them (another invitation, the calendar) reuses that. + useUserCalendarAddresses(true); + + // Even with the alias cache gone, the refusal is remembered. + resetUserCalendarAddressCache(); + useUserCalendarAddresses(true); + await expect(fetchPrincipal()).rejects.toThrow('forbidden'); + expect(request).toHaveBeenCalledTimes(1); + }); + + it('keeps asking after a failure that is not a refusal', async () => { + request.mockRejectedValueOnce(new Error('offline')); + await expect(fetchPrincipal()).rejects.toThrow('offline'); + request.mockResolvedValueOnce({ methodResponses: [['x:Account/get', { list: [{ name: 'me@example.com', aliases: { a: { name: 'alias@example.com' } } }] }, '0']] }); + await expect(fetchPrincipal()).resolves.toMatchObject({ emails: ['me@example.com', 'alias@example.com'] }); + expect(request).toHaveBeenCalledTimes(2); + }); +}); + +describe('account display name', () => { + it('reads the full name from x:AccountSettings, which every user may read', async () => { + request.mockResolvedValue({ methodResponses: [['x:AccountSettings/get', { list: [{ description: ' Ada Lovelace ' }] }, '0']] }); + await expect(fetchAccountDisplayName()).resolves.toBe('Ada Lovelace'); + expect(request).toHaveBeenCalledWith( + [['x:AccountSettings/get', { accountId: 'acc-1', ids: ['singleton'] }, '0']], + expect.anything(), + ); + }); +}); diff --git a/src/lib/calendar-user-addresses.ts b/src/lib/calendar-user-addresses.ts index 0d751913..40e2ceaa 100644 --- a/src/lib/calendar-user-addresses.ts +++ b/src/lib/calendar-user-addresses.ts @@ -7,13 +7,14 @@ import { collectUserCalendarAddresses } from './calendar-participants'; // Account aliases come from x:Account/get (Stalwart's principal object) and // only change when an admin edits the account, so they're fetched once per -// JMAP account and remembered for the session. Failure (older server, no -// permission) simply leaves the list at login address + identities. +// JMAP account and remembered for the session, and only by a view that needs +// them. Failure (older server, no permission: only admins may read it) simply +// leaves the list at login address + identities. const aliasCache = new Map(); const aliasInFlight = new Map>(); const aliasListeners = new Set<() => void>(); -function loadAliases(accountId: string): Promise { +function fetchAliases(accountId: string): Promise { const cached = aliasCache.get(accountId); if (cached) return Promise.resolve(cached); const pending = aliasInFlight.get(accountId); @@ -42,20 +43,22 @@ export function resetUserCalendarAddressCache(): void { * login address, the sending identities and the account aliases. Used to * find "me" among an event's participants (RSVP), to detect alias-organized * events as the user's own, and as the organizer address of new invites. + * `loadAliases: false` skips the alias lookup (a message that carries no + * invitation). */ -export function useUserCalendarAddresses(): string[] { +export function useUserCalendarAddresses(loadAliases = true): string[] { const activeEmail = useAccountStore((s) => s.getActiveAccount()?.email ?? null); const identities = useSettingsStore((s) => s.identities); const [, bump] = React.useReducer((n: number) => n + 1, 0); const accountId = jmapClient.isConnected ? jmapClient.accountId : null; React.useEffect(() => { - if (!accountId) return; + if (!accountId || !loadAliases) return; if (aliasCache.has(accountId)) return; aliasListeners.add(bump); - void loadAliases(accountId); + void fetchAliases(accountId); return () => { aliasListeners.delete(bump); }; - }, [accountId]); + }, [accountId, loadAliases]); const aliases = accountId ? aliasCache.get(accountId) ?? [] : []; return React.useMemo( diff --git a/src/stores/__tests__/auth-store-display-name.test.ts b/src/stores/__tests__/auth-store-display-name.test.ts new file mode 100644 index 00000000..37fa186b --- /dev/null +++ b/src/stores/__tests__/auth-store-display-name.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect, vi } from 'vitest'; + +// The display-name sync on every sign-in and start read the Stalwart +// principal with x:Account/get, which is refused to everyone but admins +// (PF6). The full name lives in x:AccountSettings, which every user reads. + +const request = vi.fn(async (calls: Array<[string, Record, string]>) => ({ + methodResponses: calls.map(([name, , id]) => { + if (name === 'Identity/get') return [name, { list: [{ id: 'i1', name: 'Old Name', email: 'user@example.com' }] }, id]; + if (name === 'x:AccountSettings/get') return [name, { list: [{ description: 'Ada Lovelace' }] }, id]; + return ['error', { type: 'forbidden' }, id]; + }), +})); + +vi.mock('../../api/jmap-client', () => ({ + jmapClient: { + connect: vi.fn(async () => ({ apiUrl: 'https://mail.example.com/jmap/' })), + snapshot: vi.fn(() => ({ session: null, credentials: null, accountId: null })), + restoreSnapshot: vi.fn(), + onAuthFailure: vi.fn(() => () => undefined), + onTokenRefresh: vi.fn(() => () => undefined), + hasAccountCapability: vi.fn(() => true), + request: (calls: Array<[string, Record, string]>) => request(calls), + accountId: 'acc-1', + isConnected: true, + currentSession: { apiUrl: 'https://mail.example.com/jmap/' }, + username: 'user@example.com', + serverUrl: 'https://mail.example.com', + }, + AuthenticationError: class AuthenticationError extends Error {}, + NetworkError: class NetworkError extends Error {}, +})); + +vi.mock('../../lib/push-notifications', () => ({ + teardownPushNotifications: vi.fn(async () => undefined), + teardownPushNotificationsForAccount: vi.fn(async () => undefined), +})); + +import { useAuthStore } from '../auth-store'; +import { useAccountStore } from '../account-store'; + +describe('display name sync on sign-in', () => { + it('reads the full name without asking for the principal', async () => { + await useAuthStore.getState().login('https://mail.example.com', 'user@example.com', 'pass'); + + await vi.waitFor(() => { + expect(useAccountStore.getState().getActiveAccount()?.displayName).toBe('Ada Lovelace'); + }); + const methods = request.mock.calls.flatMap(([calls]) => calls.map(([name]) => name)); + expect(methods).toContain('x:AccountSettings/get'); + expect(methods).not.toContain('x:Account/get'); + }); +}); diff --git a/src/stores/auth-store.ts b/src/stores/auth-store.ts index 423c5ef7..8c157802 100644 --- a/src/stores/auth-store.ts +++ b/src/stores/auth-store.ts @@ -2,7 +2,7 @@ import { create } from 'zustand'; import { jmapClient, AuthenticationError, NetworkError } from '../api/jmap-client'; import type { JMAPSession } from '../api/types'; import { getIdentities } from '../api/identity'; -import { fetchPrincipal, isStalwartSupported } from '../api/account-security'; +import { fetchAccountDisplayName, isStalwartSupported } from '../api/account-security'; import { useAccountStore } from './account-store'; import { useEmailStore } from './email-store'; import { useContactsStore } from './contacts-store'; @@ -153,9 +153,10 @@ async function revokeStoredRefreshToken(accountId: string): Promise { } // Refresh the registry's display name / address from the server (#900): the -// primary identity first, then the Stalwart principal's "Full name" when the -// account advertises the extension. Fire-and-forget; a failure keeps whatever -// the registry already had. +// primary identity first, then the Stalwart account's "Full name" when the +// account advertises the extension. Read from x:AccountSettings, which every +// user can read; x:Account/get is refused to everyone but admins. Fire-and- +// forget; a failure keeps whatever the registry already had. async function syncAccountDisplayName(accountId: string): Promise { try { const accountStore = useAccountStore.getState(); @@ -169,11 +170,8 @@ async function syncAccountDisplayName(accountId: string): Promise { if (primary?.name?.trim()) updates.displayName = primary.name.trim(); if (primary?.email && !entry.email.includes('@')) updates.email = primary.email; if (isStalwartSupported()) { - const principal = await fetchPrincipal().catch(() => null); - if (principal?.displayName?.trim()) updates.displayName = principal.displayName.trim(); - if (!updates.email && principal?.emails[0] && !entry.email.includes('@')) { - updates.email = principal.emails[0]; - } + const fullName = await fetchAccountDisplayName().catch(() => null); + if (fullName) updates.displayName = fullName; } if (Object.keys(updates).length === 0) return; if (useAccountStore.getState().getAccountById(accountId)) {