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 105c4442..2bcc8ff7 100644 --- a/android/app/src/main/java/com/anonymous/bulwarkmobile/BulwarkFcmModule.kt +++ b/android/app/src/main/java/com/anonymous/bulwarkmobile/BulwarkFcmModule.kt @@ -152,7 +152,9 @@ class BulwarkFcmModule(reactContext: ReactApplicationContext) .setSmallIcon(R.drawable.ic_notification) .setLargeIcon(largeIcon) .setContentTitle(title) - .setContentText(subject ?: preview ?: body) + // body is the subject with the "(No subject)" fallback; an empty + // subject is "" here, not null, so `subject ?: ...` showed a blank line. + .setContentText(body) .setStyle( NotificationCompat.BigTextStyle() .bigText(if (bigText.isNotBlank()) bigText else body) @@ -206,7 +208,10 @@ class BulwarkFcmModule(reactContext: ReactApplicationContext) PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE, ) builder.addAction( - NotificationCompat.Action.Builder(0, deleteLabel, deletePending).build() + NotificationCompat.Action.Builder(0, deleteLabel, deletePending) + // Android 12+: unlock first, so a locked phone can't delete mail. + .setAuthenticationRequired(true) + .build() ) // Action 3: Reply diff --git a/src/lib/__tests__/push-background-task.test.ts b/src/lib/__tests__/push-background-task.test.ts index dadfd7b8..676ff2e7 100644 --- a/src/lib/__tests__/push-background-task.test.ts +++ b/src/lib/__tests__/push-background-task.test.ts @@ -287,6 +287,32 @@ describe('pushBackgroundTask notifications', () => { ['Email/set', { accountId: 'jmap-primary', update: { m1: { mailboxIds: { 'trash-box': true } } } }, '0'], ]); }); + + it.each([ + ['has no trash mailbox', ['Mailbox/get', { list: [{ id: 'inbox', role: 'inbox' }] }, '0']], + ['cannot list mailboxes', ['error', { type: 'accountNotFound' }, '0']], + ])('never destroys from a notification when the account %s', async (_label, mailboxResponse) => { + const postCalls: any[] = []; + (secureFetch as ReturnType).mockImplementation(async (url: string, opts?: any) => { + if (opts?.body) postCalls.push(JSON.parse(opts.body)); + return { + ok: true, + json: async () => (url.endsWith('/.well-known/jmap') + ? { + apiUrl: 'https://mail.example.com/jmap/', + primaryAccounts: { 'urn:ietf:params:jmap:mail': 'jmap-primary' }, + accounts: { 'jmap-primary': {} }, + } + : { methodResponses: [mailboxResponse] }), + }; + }); + + await handleNotificationAction({ action: 'delete', emailId: 'm1', accountId: LOCAL, jmapAccountId: 'shared' }); + + expect(postCalls).toHaveLength(1); + expect(postCalls[0].methodCalls[0][0]).toBe('Mailbox/get'); + expect(postCalls[0].methodCalls[0][1].accountId).toBe('shared'); + }); }); describe('pushes for device sync (#34)', () => { diff --git a/src/lib/push-background-task.ts b/src/lib/push-background-task.ts index 56aa87ec..4afcad23 100644 --- a/src/lib/push-background-task.ts +++ b/src/lib/push-background-task.ts @@ -593,7 +593,7 @@ export async function handleNotificationAction(data: unknown): Promise { const emailAccountId = jmapAccountId ?? session.jmapAccountId; if (action === 'markRead') { - await jmapPost(session, [ + const res = await jmapPost(session, [ [ 'Email/set', { @@ -605,41 +605,42 @@ export async function handleNotificationAction(data: unknown): Promise { '0', ], ]); + assertEmailUpdated(res, emailId); } else if (action === 'delete') { const mbRes = await jmapPost(session, [ ['Mailbox/get', { accountId: emailAccountId, properties: ['id', 'role'] }, '0'], ]); - const [, mbBody] = mbRes[0] ?? []; - const mailboxes = ((mbBody?.list as Mailbox[]) ?? []); + const [mbName, mbBody] = mbRes[0] ?? []; + const mailboxes = mbName === 'Mailbox/get' ? ((mbBody?.list as Mailbox[]) ?? []) : []; const trash = mailboxes.find((m) => m.role === 'trash'); - - if (trash) { - await jmapPost(session, [ - [ - 'Email/set', - { - accountId: emailAccountId, - update: { - [emailId]: { mailboxIds: { [trash.id]: true } }, - }, - }, - '0', - ], - ]); - } else { - await jmapPost(session, [ - [ - 'Email/set', - { - accountId: emailAccountId, - destroy: [emailId], - }, - '0', - ], - ]); + // Never destroy from a notification: without a Trash (a shared + // account, or a Mailbox/get that failed) the message stays put. + if (!trash) { + console.warn('[push] no trash mailbox for notification delete', emailAccountId); + return; } + const res = await jmapPost(session, [ + [ + 'Email/set', + { + accountId: emailAccountId, + update: { + [emailId]: { mailboxIds: { [trash.id]: true } }, + }, + }, + '0', + ], + ]); + assertEmailUpdated(res, emailId); } } catch (err) { console.warn('[push] notification action failed', action, emailId, err); } } + +function assertEmailUpdated(responses: Array<[string, Record, string]>, emailId: string): void { + const [name, body] = responses[0] ?? []; + if (name !== 'Email/set' || !body?.updated || !(emailId in body.updated)) { + throw new Error(`Email/set did not update ${emailId}: ${JSON.stringify(body?.notUpdated?.[emailId] ?? body ?? null)}`); + } +}