diff --git a/App.tsx b/App.tsx index dee0d125..7d60e871 100644 --- a/App.tsx +++ b/App.tsx @@ -20,6 +20,7 @@ import { addTokenRefreshListener, getInitialNotificationTap, getStoredRelayBaseUrl, + notificationTapJmapAccountId, resyncPushNotifications, teardownPushNotificationsForAccount, type NotificationTapPayload, @@ -87,6 +88,8 @@ async function navigateToNotificationTap(payload: NotificationTapPayload): Promi emailId: payload.emailId, threadId: payload.threadId, subject: payload.subject, + // A group mailbox's message lives under another JMAP account (#839). + jmapAccountId: notificationTapJmapAccountId(payload), }); } diff --git a/android/app/src/main/java/com/anonymous/bulwarkmobile/BulwarkFcmModule.kt b/android/app/src/main/java/com/anonymous/bulwarkmobile/BulwarkFcmModule.kt index e806a980..f55e86af 100644 --- a/android/app/src/main/java/com/anonymous/bulwarkmobile/BulwarkFcmModule.kt +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/BulwarkFcmModule.kt @@ -66,6 +66,7 @@ class BulwarkFcmModule(reactContext: ReactApplicationContext) val threadId = options.getString("threadId") val subject = options.takeIf { it.hasKey("subject") }?.getString("subject") val accountId = options.takeIf { it.hasKey("accountId") }?.getString("accountId") + val jmapAccountId = options.takeIf { it.hasKey("jmapAccountId") }?.getString("jmapAccountId") val groupKey = options.takeIf { it.hasKey("groupKey") }?.getString("groupKey") ?: accountId?.let { "bulwark-mail:$it" } val groupTitle = options.takeIf { it.hasKey("groupTitle") }?.getString("groupTitle") @@ -78,7 +79,7 @@ class BulwarkFcmModule(reactContext: ReactApplicationContext) ?: makeLetterAvatar(initials, bgColorHex) postNotification( notificationId, title, body, largeIcon, bgColorHex, - emailId, threadId, subject, accountId, groupKey, + emailId, threadId, subject, accountId, jmapAccountId, groupKey, ) if (groupKey != null) postGroupSummary(groupKey, groupTitle, bgColorHex, accountId) promise.resolve(null) @@ -107,6 +108,7 @@ class BulwarkFcmModule(reactContext: ReactApplicationContext) threadId: String?, subject: String?, accountId: String?, + jmapAccountId: String?, groupKey: String?, ) { val ctx = reactApplicationContext @@ -116,6 +118,7 @@ class BulwarkFcmModule(reactContext: ReactApplicationContext) if (threadId != null) putExtra(NotificationTapStore.EXTRA_THREAD_ID, threadId) if (subject != null) putExtra(NotificationTapStore.EXTRA_SUBJECT, subject) if (accountId != null) putExtra(NotificationTapStore.EXTRA_ACCOUNT_ID, accountId) + if (jmapAccountId != null) putExtra(NotificationTapStore.EXTRA_JMAP_ACCOUNT_ID, jmapAccountId) } val pending = PendingIntent.getActivity( ctx, diff --git a/android/app/src/main/java/com/anonymous/bulwarkmobile/NotificationTapStore.kt b/android/app/src/main/java/com/anonymous/bulwarkmobile/NotificationTapStore.kt index 6c1595be..baed3c73 100644 --- a/android/app/src/main/java/com/anonymous/bulwarkmobile/NotificationTapStore.kt +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/NotificationTapStore.kt @@ -40,13 +40,17 @@ object NotificationTapStore { // the navigation payload. val subject = extras.getString(EXTRA_SUBJECT)?.take(512) val accountId = extras.getString(EXTRA_ACCOUNT_ID)?.takeIf { ACCOUNT_ID_PATTERN.matches(it) } - val payload = TapPayload(emailId, threadId, subject, accountId) + // The JMAP account a group/shared mailbox's message lives in - a JMAP + // id like the email id. Optional: older notifications don't carry it. + val jmapAccountId = extras.getString(EXTRA_JMAP_ACCOUNT_ID)?.takeIf { ID_PATTERN.matches(it) } + val payload = TapPayload(emailId, threadId, subject, accountId, jmapAccountId) pending = payload // Clear so a subsequent activity lifecycle event doesn't replay this. extras.remove(EXTRA_EMAIL_ID) extras.remove(EXTRA_THREAD_ID) extras.remove(EXTRA_SUBJECT) extras.remove(EXTRA_ACCOUNT_ID) + extras.remove(EXTRA_JMAP_ACCOUNT_ID) return payload } @@ -55,12 +59,14 @@ object NotificationTapStore { val threadId: String, val subject: String?, val accountId: String?, + val jmapAccountId: String?, ) { fun toMap(): WritableMap = Arguments.createMap().apply { putString("emailId", emailId) putString("threadId", threadId) if (subject != null) putString("subject", subject) if (accountId != null) putString("accountId", accountId) + if (jmapAccountId != null) putString("jmapAccountId", jmapAccountId) } } @@ -68,4 +74,5 @@ object NotificationTapStore { const val EXTRA_THREAD_ID = "bulwark.notification.threadId" const val EXTRA_SUBJECT = "bulwark.notification.subject" const val EXTRA_ACCOUNT_ID = "bulwark.notification.accountId" + const val EXTRA_JMAP_ACCOUNT_ID = "bulwark.notification.jmapAccountId" } diff --git a/src/lib/__tests__/push-background-task.test.ts b/src/lib/__tests__/push-background-task.test.ts index 571973e0..45d48429 100644 --- a/src/lib/__tests__/push-background-task.test.ts +++ b/src/lib/__tests__/push-background-task.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; vi.mock('../../api/jmap-client', () => ({ jmapClient: { @@ -11,9 +11,13 @@ vi.mock('../client-cert', () => ({ secureFetch: vi.fn(async () => ({ ok: false, vi.mock('../oauth', () => ({ refreshOAuthAccessToken: vi.fn(async (t: unknown) => t) })); import AsyncStorage from '@react-native-async-storage/async-storage'; +import { NativeModules } from 'react-native'; +import { jmapClient } from '../../api/jmap-client'; +import { secureFetch } from '../client-cert'; import { matchAccountsForPush, parseRelayPushData, + pushBackgroundTask, selectNotifiableEmails, senderFaviconsAllowed, } from '../push-background-task'; @@ -78,6 +82,81 @@ describe('matchAccountsForPush', () => { }); }); +describe('pushBackgroundTask notifications', () => { + const LOCAL = 'alice@mail.example.com'; + const showNotification = vi.fn(async () => undefined); + let emailGetAccount: string | null = null; + + beforeEach(async () => { + vi.clearAllMocks(); + await AsyncStorage.clear(); + await AsyncStorage.setItem('push:accountIds:v1', JSON.stringify([LOCAL])); + await AsyncStorage.setItem('push:jmapAccountIds:v1', JSON.stringify({ [LOCAL]: 'jmap-primary' })); + (NativeModules as Record).BulwarkFcm = { showNotification }; + (jmapClient.getStoredCredentials as ReturnType).mockResolvedValue({ + serverUrl: 'https://mail.example.com', + username: 'alice', + password: 'secret', + }); + emailGetAccount = null; + (secureFetch as ReturnType).mockImplementation(async (url: string, init?: { body?: string }) => { + if (url.endsWith('/.well-known/jmap')) { + return { + ok: true, + json: async () => ({ + apiUrl: 'https://mail.example.com/jmap/', + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'jmap-primary' }, + accounts: { 'jmap-primary': {}, team: {} }, + }), + }; + } + const [[, args]] = JSON.parse(init?.body ?? '{}').methodCalls; + emailGetAccount = args.accountId; + return { + ok: true, + json: async () => ({ + methodResponses: [[ + 'Email/get', + { list: [{ id: 'm1', threadId: 't1', keywords: {}, subject: 'Hi', from: [{ email: 'bob@example.com' }] }] }, + '0', + ]], + }), + }; + }); + }); + + afterEach(() => { + delete (NativeModules as Record).BulwarkFcm; + }); + + it('tags a group mailbox notification with the JMAP account the message lives in (#839)', async () => { + await pushBackgroundTask({ + kind: 'jmap-email-push', + accountLabel: 'alice', + accountId: 'team', + emailIds: JSON.stringify(['m1']), + }); + + expect(emailGetAccount).toBe('team'); + expect(showNotification).toHaveBeenCalledWith( + expect.objectContaining({ emailId: 'm1', accountId: LOCAL, jmapAccountId: 'team' }), + ); + }); + + it('tags the user\'s own mail with the primary account', async () => { + await pushBackgroundTask({ + kind: 'jmap-email-push', + accountLabel: 'alice', + accountId: 'jmap-primary', + emailIds: JSON.stringify(['m1']), + }); + + expect(showNotification).toHaveBeenCalledWith( + expect.objectContaining({ emailId: 'm1', accountId: LOCAL, jmapAccountId: 'jmap-primary' }), + ); + }); +}); + describe('selectNotifiableEmails', () => { const email = (id: string, keywords: Record = {}): Email => ({ id, threadId: 't', keywords, mailboxIds: {}, size: 0, receivedAt: '', hasAttachment: false } as Email); diff --git a/src/lib/__tests__/push-notifications.test.ts b/src/lib/__tests__/push-notifications.test.ts index df8a5905..a324a717 100644 --- a/src/lib/__tests__/push-notifications.test.ts +++ b/src/lib/__tests__/push-notifications.test.ts @@ -73,6 +73,7 @@ import { deviceClientIdKey, disablePushForAccount, isValidRelayUrl, + notificationTapJmapAccountId, readPushAccountIds, readPushJmapAccountIds, PushSetupError, @@ -793,6 +794,22 @@ describe('teardownPushNotificationsForAccount', () => { }); }); +describe('notificationTapJmapAccountId', () => { + const tap = { emailId: 'm1', threadId: 't1', accountId: ACCOUNT_ID }; + + it('opens a group mailbox message against the group account', () => { + expect(notificationTapJmapAccountId({ ...tap, jmapAccountId: 'team' })).toBe('team'); + }); + + it('leaves the user\'s own mail on the default account', () => { + expect(notificationTapJmapAccountId({ ...tap, jmapAccountId: 'jmap-primary' })).toBeUndefined(); + }); + + it('keeps notifications posted before the field existed working', () => { + expect(notificationTapJmapAccountId(tap)).toBeUndefined(); + }); +}); + describe('isValidRelayUrl', () => { it('requires https except for loopback development hosts', () => { expect(isValidRelayUrl('https://relay.example.com')).toBe(true); diff --git a/src/lib/push-background-task.ts b/src/lib/push-background-task.ts index 8ca29175..65d51adb 100644 --- a/src/lib/push-background-task.ts +++ b/src/lib/push-background-task.ts @@ -81,6 +81,9 @@ interface ShowNotificationOptions { threadId: string; subject?: string; 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; // Android notification group: one per account so the tray bundles // several deliveries under a "+N more" summary instead of stacking them. groupKey: string; @@ -416,6 +419,7 @@ async function processAccountForPush(accountId: string, payload: RelayPushData): const alreadyNotified = await readNotifiedIds(accountId); let candidates: Email[]; + let emailAccountId = session.jmapAccountId; if (payload.emailIds.length > 0) { // EmailPush (or a relay that forwards ids): fetch exactly the delivered @@ -423,8 +427,8 @@ async function processAccountForPush(accountId: string, payload: RelayPushData): // to, in which case the ids live under that JMAP account. const fresh = payload.emailIds.filter((id) => !alreadyNotified.includes(id)); if (fresh.length === 0) return; - const targetAccount = payload.jmapAccountId ?? session.jmapAccountId; - candidates = await detachedGetEmails(session, targetAccount, fresh); + emailAccountId = payload.jmapAccountId ?? session.jmapAccountId; + candidates = await detachedGetEmails(session, emailAccountId, fresh); } else { // Legacy `jmap-state-change` payload without ids: look at the newest // unread inbox messages and notify the ones not shown before. The ring @@ -472,6 +476,7 @@ async function processAccountForPush(accountId: string, payload: RelayPushData): threadId: email.threadId, subject: email.subject ?? undefined, accountId, + jmapAccountId: emailAccountId, groupKey, groupTitle, }); diff --git a/src/lib/push-notifications.ts b/src/lib/push-notifications.ts index 9f60cf06..1a4a695d 100644 --- a/src/lib/push-notifications.ts +++ b/src/lib/push-notifications.ts @@ -1284,6 +1284,26 @@ export interface NotificationTapPayload { // Optional for back-compat: older notifications already on the system tray // won't carry this and will fall back to the active account on tap. accountId?: string; + // The JMAP account the message lives in: a group or shared mailbox's account + // when it isn't the user's own. Absent on older notifications. + jmapAccountId?: string; +} + +/** + * The JMAP account to open a tapped notification's message against, in the + * form the EmailThread route takes: the group or shared account it was + * delivered from, or undefined for the user's own mail - including + * notifications posted before the field existed (#839). + */ +export function notificationTapJmapAccountId(payload: NotificationTapPayload): string | undefined { + if (!payload.jmapAccountId) return undefined; + let primary: string | null = null; + try { + primary = jmapClient.accountId; + } catch { + primary = null; + } + return payload.jmapAccountId === primary ? undefined : payload.jmapAccountId; } // Returns - and clears - any pending "notification tap" that launched the app