fix: open a group mailbox notification's message in the group account #839

The notification only carried the local account it was posted for, so
tapping one for a message in a group or shared mailbox opened the
thread against the user's own JMAP account, where the message doesn't
exist.

The background task now passes the JMAP account the message was
fetched from as `jmapAccountId`. The native module stores it as an
intent extra, validated like the other ids, and hands it back with the
tap. `App.tsx` routes the open with it, the way the unified inbox does
for group messages. Notifications posted before this change have no
such extra and open against the account's own mail as before.

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),
   });
 }
 

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,

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"
 }

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<string, unknown>).BulwarkFcm = { showNotification };
+    (jmapClient.getStoredCredentials as ReturnType<typeof vi.fn>).mockResolvedValue({
+      serverUrl: 'https://mail.example.com',
+      username: 'alice',
+      password: 'secret',
+    });
+    emailGetAccount = null;
+    (secureFetch as ReturnType<typeof vi.fn>).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<string, unknown>).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<string, boolean> = {}): Email =>
     ({ id, threadId: 't', keywords, mailboxIds: {}, size: 0, receivedAt: '', hasAttachment: false } as Email);

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);

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,
     });

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