mirror of
https://github.com/bulwarkmail/native.git
synced 2026-10-31 16:06:32 +00:00
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:
@@ -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();
|
||||
});
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -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 : '',
|
||||
|
||||
Reference in New Issue
Block a user