fix: show a plain new-mail notification instead of another login's message when two logins share a JMAP account id

This commit is contained in:
Linus Rath
2026-10-29 02:51:24 +01:00
parent 65f00640ba
commit 9ccb62286e
4 changed files with 212 additions and 32 deletions
+55 -2
View File
@@ -1,6 +1,47 @@
import { beforeEach, describe, expect, it } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import AsyncStorage from '@react-native-async-storage/async-storage';
import { iosTapPayload } from '../push-notifications';
import { Platform } from 'react-native';
vi.mock('../ios-push-shared', () => ({
apnsEnvironment: () => 'production',
writeIosPushAccount: vi.fn(async () => undefined),
removeIosPushAccount: vi.fn(async () => undefined),
}));
import { iosTapPayload, shareAccountWithExtension } from '../push-notifications';
import { removeIosPushAccount, writeIosPushAccount } from '../ios-push-shared';
// The extension's entry for a pushed account is keyed by the JMAP account id
// alone (NotificationService.swift reads bulwark_push_account__<id>).
describe('shareAccountWithExtension', () => {
const API = 'https://mail.example.org/jmap/';
beforeEach(async () => {
vi.clearAllMocks();
await AsyncStorage.clear();
(Platform as { OS: string }).OS = 'ios';
});
afterEach(() => {
(Platform as { OS: string }).OS = 'android';
});
it('shares the login whose push setup recorded the JMAP account id', async () => {
await AsyncStorage.setItem('push:jmapAccountIds:v1', JSON.stringify({ 'ada@mail.example.org': 'c' }));
await shareAccountWithExtension('ada@mail.example.org', 'c', API);
expect(writeIosPushAccount).toHaveBeenCalledWith('c', { credentialsKey: expect.any(String), apiUrl: API });
});
it('shares neither of two logins with the same JMAP account id, so the extension shows its placeholder', async () => {
await AsyncStorage.setItem('push:jmapAccountIds:v1', JSON.stringify({
'ada@mail.example.org': 'c',
'bea@other.example.net': 'c',
}));
await shareAccountWithExtension('bea@other.example.net', 'c', 'https://other.example.net/jmap/');
expect(writeIosPushAccount).not.toHaveBeenCalled();
expect(removeIosPushAccount).toHaveBeenCalledWith('c');
});
});
// A tapped iOS mail notification: the relay's ids arrive as the
// notification's data (APNs `body`), the notification extension adds the
@@ -35,6 +76,18 @@ describe('iosTapPayload', () => {
expect(await iosTapPayload(null)).toBeNull();
});
it('tells two logins with the same JMAP account id apart by the relay label, or opens nothing', async () => {
await AsyncStorage.setItem('push:jmapAccountIds:v1', JSON.stringify({
'ada@mail.example.org': 'c',
'bea@other.example.net': 'c',
}));
const push = { kind: 'jmap-email-push', accountId: 'c', emailIds: ['e1'] };
expect((await iosTapPayload({ ...push, accountLabel: 'bea' }))?.accountId).toBe('bea@other.example.net');
// Opening e1 in either login is a guess: the app opens as it is.
expect(await iosTapPayload({ ...push, accountLabel: 'carl' })).toBeNull();
expect(await iosTapPayload(push)).toBeNull();
});
it('does nothing for a push without a message id', async () => {
expect(await iosTapPayload({ kind: 'jmap-state-change', accountId: 'c', emailIds: [] })).toBeNull();
});
+52 -3
View File
@@ -70,17 +70,45 @@ describe('matchAccountsForPush', () => {
it('matches on the recorded JMAP account id first', () => {
const payload = parseRelayPushData({ accountId: 'jb', accountLabel: 'alice' });
expect(matchAccountsForPush(payload, accounts, { 'bob@mail.example.com': 'jb' }, registry))
.toEqual(['bob@mail.example.com']);
.toEqual({ accountIds: ['bob@mail.example.com'], ambiguous: false });
});
it('falls back to the relay accountLabel (username)', () => {
const payload = parseRelayPushData({ accountId: 'unknown', accountLabel: 'alice' });
expect(matchAccountsForPush(payload, accounts, {}, registry)).toEqual(['alice@mail.example.com']);
expect(matchAccountsForPush(payload, accounts, {}, registry).accountIds).toEqual(['alice@mail.example.com']);
});
it('checks every account when nothing matches', () => {
const payload = parseRelayPushData({ accountId: 'unknown', accountLabel: 'carol' });
expect(matchAccountsForPush(payload, accounts, {}, registry)).toEqual(accounts);
expect(matchAccountsForPush(payload, accounts, {}, registry)).toEqual({ accountIds: accounts, ambiguous: false });
});
describe('with two logins on different servers that share a JMAP account id', () => {
const logins = ['alice@one.example.com', 'bob@two.example.net'];
const ids = { 'alice@one.example.com': 'b', 'bob@two.example.net': 'b' };
const users = [
{ id: 'alice@one.example.com', username: 'alice' },
{ id: 'bob@two.example.net', username: 'bob' },
];
it('picks the login the relay label names', () => {
const payload = parseRelayPushData({ accountId: 'b', accountLabel: 'bob', emailIds: '["m1"]' });
expect(matchAccountsForPush(payload, logins, ids, users)).toEqual({ accountIds: ['bob@two.example.net'], ambiguous: false });
});
it('calls a push naming messages ambiguous when the label does not tell them apart', () => {
const sameName = [
{ id: 'alice@one.example.com', username: 'alice' },
{ id: 'alice@two.example.net', username: 'alice' },
];
const both = ['alice@one.example.com', 'alice@two.example.net'];
const map = { 'alice@one.example.com': 'b', 'alice@two.example.net': 'b' };
const payload = parseRelayPushData({ accountId: 'b', accountLabel: 'alice', emailIds: '["m1"]' });
expect(matchAccountsForPush(payload, both, map, sameName)).toEqual({ accountIds: both, ambiguous: true });
// Without ids each login only looks at its own newest mail.
const legacy = parseRelayPushData({ accountId: 'b', accountLabel: 'alice', changed: '{"b":{"Email":"s"}}' });
expect(matchAccountsForPush(legacy, both, map, sameName).ambiguous).toBe(false);
});
});
});
@@ -145,6 +173,27 @@ describe('pushBackgroundTask notifications', () => {
);
});
it("shows a plain notification, not another server's message, when two logins fit the push", async () => {
// A second login named alice, on another server whose account id is the same.
const SECOND = 'alice@other.example.net';
await AsyncStorage.setItem('push:accountIds:v1', JSON.stringify([LOCAL, SECOND]));
await AsyncStorage.setItem('push:jmapAccountIds:v1', JSON.stringify({ [LOCAL]: 'jmap-primary', [SECOND]: 'jmap-primary' }));
await pushBackgroundTask({
kind: 'jmap-email-push',
accountLabel: 'alice',
accountId: 'jmap-primary',
emailIds: JSON.stringify(['m1']),
});
expect(emailGetAccount).toBeNull();
expect(showNotification).toHaveBeenCalledTimes(1);
const shown = (showNotification.mock.calls[0] as unknown[])[0] as Record<string, unknown>;
expect(shown).toMatchObject({ title: 'New email', emailId: '', threadId: '' });
expect(shown.accountId).toBeUndefined();
expect(shown.subject).toBeUndefined();
});
it('tags the user\'s own mail with the primary account', async () => {
await pushBackgroundTask({
kind: 'jmap-email-push',
+68 -23
View File
@@ -19,6 +19,7 @@ import {
notifiedIdsKey,
pushSubscriptionDueForExtension,
pushSubscriptionNeedsExtension,
accountMatchesPushLabel,
readPushAccountIds,
readPushJmapAccountIds,
recordPushSubscriptionExpires,
@@ -114,10 +115,11 @@ interface ShowNotificationOptions {
emailId: string;
threadId: string;
subject?: string;
accountId: string;
// Left out for a notification that names no login (showPlainNotification).
accountId?: string;
// The JMAP account the message lives in - a group or shared mailbox's
// account rather than the user's own - so a tap opens it there.
jmapAccountId: string;
jmapAccountId?: string;
// Android notification group: one per account so the tray bundles
// several deliveries under a "+N more" summary instead of stacking them.
groupKey: string;
@@ -204,32 +206,48 @@ async function readAccountRegistry(): Promise<RegistryAccount[]> {
}
}
export interface PushAccountMatch {
accountIds: string[];
// The push names messages, and more than one login may be the one they
// were delivered to: see matchAccountsForPush.
ambiguous: boolean;
}
/**
* Pick the local account(s) a relay payload belongs to. Primary key is the
* JMAP account id recorded at setup; the relay's `accountLabel` (= username)
* is a weaker fallback. When neither matches - a payload from a build that
* predates the id map - every registered account is checked.
* is a weaker fallback, and also tells apart logins that share a JMAP account
* id. When neither matches - a payload from a build that predates the id map -
* every registered account is checked.
*
* JMAP account ids repeat across servers (Stalwart starts every server at the
* same few), and so do message ids. A push naming messages that still fits
* more than one login is `ambiguous`: each login would fetch those ids from
* its own server and could show another server's message, so the caller
* shows a plain notification instead. A push without ids makes each login
* look at its own newest mail, which is never another login's.
*/
export function matchAccountsForPush(
payload: RelayPushData,
pushAccountIds: string[],
jmapAccountIds: Record<string, string>,
registry: RegistryAccount[],
): string[] {
if (payload.jmapAccountId) {
const byJmapId = pushAccountIds.filter((id) => jmapAccountIds[id] === payload.jmapAccountId);
if (byJmapId.length > 0) return byJmapId;
): PushAccountMatch {
const byLabel = (ids: string[]) => (payload.accountLabel
? ids.filter((id) => accountMatchesPushLabel(id, payload.accountLabel!, registry.find((a) => a.id === id)?.username))
: []);
let accountIds = pushAccountIds;
const byJmapId = payload.jmapAccountId
? pushAccountIds.filter((id) => jmapAccountIds[id] === payload.jmapAccountId)
: [];
if (byJmapId.length > 0) {
const narrowed = byJmapId.length > 1 ? byLabel(byJmapId) : [];
accountIds = narrowed.length > 0 ? narrowed : byJmapId;
} else {
const labelled = byLabel(pushAccountIds);
if (labelled.length > 0) accountIds = labelled;
}
if (payload.accountLabel) {
const label = payload.accountLabel.toLowerCase();
const byLabel = pushAccountIds.filter((id) => {
const entry = registry.find((a) => a.id === id);
const username = entry?.username?.toLowerCase();
return username === label || id.toLowerCase().startsWith(`${label}@`);
});
if (byLabel.length > 0) return byLabel;
}
return pushAccountIds;
return { accountIds, ambiguous: accountIds.length > 1 && payload.emailIds.length > 0 };
}
async function readNotifiedIds(accountId: string): Promise<string[]> {
@@ -444,11 +462,12 @@ export async function pushBackgroundTask(data: unknown): Promise<void> {
const registry = await readAccountRegistry();
const jmapAccountIds = await readPushJmapAccountIds();
const accountsToCheck = matchAccountsForPush(payload, accountIds, jmapAccountIds, registry);
const match = matchAccountsForPush(payload, accountIds, jmapAccountIds, registry);
if (match.ambiguous) await showPlainNotification(payload);
for (const accountId of accountsToCheck) {
for (const accountId of match.accountIds) {
try {
await processAccountForPush(accountId, payload);
await processAccountForPush(accountId, payload, !match.ambiguous);
} catch (err) {
console.warn(
'[push] background check failed for account',
@@ -511,11 +530,13 @@ async function extendPushSubscription(accountId: string, session: DetachedSessio
if (extended?.expires) await recordPushSubscriptionExpires(accountId, extended.expires);
}
async function processAccountForPush(accountId: string, payload: RelayPushData): Promise<void> {
// `notify` is false for an ambiguous push (see matchAccountsForPush): each
// login it may belong to still keeps its subscription alive.
async function processAccountForPush(accountId: string, payload: RelayPushData, notify: boolean): Promise<void> {
const session = await openDetachedSession(accountId);
if (!session) return;
try {
await notifyForPush(accountId, session, payload);
if (notify) await notifyForPush(accountId, session, payload);
} finally {
await extendPushSubscription(accountId, session).catch(() => undefined);
}
@@ -600,6 +621,30 @@ async function notifyForPush(accountId: string, session: DetachedSession, payloa
await rememberNotifiedIds(accountId, ordered.map((e) => e.id).reverse());
}
/**
* New mail without saying which message: for a push more than one login may
* belong to. It carries no message and no login, so it has no actions and a
* tap opens the app as it is; empty ids, which the tap handler refuses, keep
* it from opening anything (NotificationTapStore).
*/
async function showPlainNotification(payload: RelayPushData): Promise<void> {
const native = NativeModules.BulwarkFcm as BulwarkFcmNative | undefined;
if (!native?.showNotification) return;
const locale = await notificationLocale();
const label = payload.accountLabel ?? '';
await native.showNotification({
notificationId: `mail-plain:${payload.jmapAccountId ?? ''}:${payload.emailIds[0] ?? ''}`,
title: translate(locale, 'notifications.new_email', 'New email'),
body: label,
initials: getEmailInitials(label, ''),
bgColorHex: hslToHex(generateEmailAvatarColor(label, '')),
emailId: '',
threadId: '',
groupKey: 'bulwark-mail:plain',
groupTitle: label || 'Bulwark',
});
}
function hslToHex(hsl: string): string {
const match = hsl.match(/hsl\(\s*(\d+)\s*,\s*(\d+)%\s*,\s*(\d+)%\s*\)/);
if (!match) return '#2563eb';
+37 -4
View File
@@ -1003,14 +1003,38 @@ async function registerWithRelayApns(params: {
* Tell the iOS notification extension how to reach an account: which stored
* credentials to use and the JMAP API URL. Without it the extension shows the
* relay's placeholder text.
*
* The extension finds the entry by the push's JMAP account id alone
* (NotificationService.swift), and those ids repeat across servers. While
* another login with push has the same id, neither gets an entry: the
* placeholder says there is new mail, where one login's credentials would
* fetch and show a message of the other's server. Run again for the login
* that is left once the other is gone (the next resync does).
*/
export async function shareAccountWithExtension(accountId: string, jmapAccountId: string | null): Promise<void> {
export async function shareAccountWithExtension(
accountId: string,
jmapAccountId: string | null,
apiUrl: string | undefined = jmapClient.currentSession?.apiUrl,
): Promise<void> {
if (Platform.OS !== 'ios' || !jmapAccountId) return;
const apiUrl = jmapClient.currentSession?.apiUrl;
const byLocal = await readPushJmapAccountIds();
if (Object.keys(byLocal).some((local) => local !== accountId && byLocal[local] === jmapAccountId)) {
await removeIosPushAccount(jmapAccountId).catch(() => undefined);
return;
}
if (!apiUrl) return;
await writeIosPushAccount(jmapAccountId, { credentialsKey: credentialsKey(accountId), apiUrl }).catch(() => undefined);
}
/**
* Whether a local account (username@host) is the login a relay label names.
* The label is the username the account registered with the relay.
*/
export function accountMatchesPushLabel(localAccountId: string, label: string, username?: string): boolean {
const wanted = label.toLowerCase();
return username?.toLowerCase() === wanted || localAccountId.toLowerCase().startsWith(`${wanted}@`);
}
async function registerWithRelayUnifiedPush(params: {
relayBaseUrl: string;
subscriptionId: string;
@@ -2054,9 +2078,18 @@ export async function iosTapPayload(data: unknown): Promise<NotificationTapPaylo
const emailId = typeof d.emailId === 'string' && d.emailId ? d.emailId : ids[0];
if (!emailId) return null;
// The local account the subscription belongs to: the one whose push
// setup recorded this JMAP account id.
// setup recorded this JMAP account id. Ids repeat across servers: with
// more than one such login the relay's label (the username) picks one, and
// when it can't, the app just opens rather than show another login's
// message on a guess.
const byLocal = await readPushJmapAccountIds();
const accountId = Object.keys(byLocal).find((local) => byLocal[local] === jmapAccountId);
let candidates = Object.keys(byLocal).filter((local) => byLocal[local] === jmapAccountId);
if (candidates.length > 1) {
const label = typeof d.accountLabel === 'string' ? d.accountLabel : '';
candidates = label ? candidates.filter((local) => accountMatchesPushLabel(local, label)) : [];
if (candidates.length !== 1) return null;
}
const accountId = candidates[0];
return {
emailId,
threadId: typeof d.threadId === 'string' ? d.threadId : '',