Merge pull request 'fix(notifications): notification replies re-check encryption at send time and use the right account (#299)' (#397) from fix/sw-reply-checks into lotus

src/app/pages/client/ClientNonUIFeatures.tsx

@@ -580,7 +580,8 @@ function MessageNotifications() {
                 // NotificationOptions typing.
                 actions: [{ action: 'reply', title: 'Reply', type: 'text', placeholder: 'Reply…' }],
               }),
-          data: { path: roomPath, roomId, threadId: threadId || undefined },
+          // userId: [Gitea #299] the SW replies as this account only.
+          data: { path: roomPath, roomId, threadId: threadId || undefined, userId: mx.getUserId() },
         },
         () => {
           window.focus();
@@ -588,7 +589,7 @@ function MessageNotifications() {
         },
       );
     },
-    [navigate, setToast, mDirects],
+    [navigate, setToast, mDirects, mx],
   );
 
   // N105: when a service-worker-owned notification is clicked, the SW focuses

src/index.tsx

@@ -43,7 +43,7 @@ if ('serviceWorker' in navigator && swProtocolOk) {
 
   const sendSessionToSW = () => {
     const session = getFallbackSession();
-    pushSessionToSW(session?.baseUrl, session?.accessToken);
+    pushSessionToSW(session?.baseUrl, session?.accessToken, session?.userId);
   };
 
   // The dev worker is an ES module (it imports workbox), so it must be

src/sw-session.ts

@@ -1,4 +1,4 @@
-export function pushSessionToSW(baseUrl?: string, accessToken?: string) {
+export function pushSessionToSW(baseUrl?: string, accessToken?: string, userId?: string) {
   if (!('serviceWorker' in navigator)) return;
   if (!navigator.serviceWorker.controller) return;
 
@@ -6,5 +6,6 @@ export function pushSessionToSW(baseUrl?: string, accessToken?: string) {
     type: 'setSession',
     accessToken,
     baseUrl,
+    userId,
   });
 }

src/sw.ts

@@ -2,7 +2,7 @@
 
 import { precacheAndRoute, type PrecacheEntry } from 'workbox-precaching';
 import { SHARE_TARGET_PATH, stashSharedForm } from './swShare';
-import { sendNotificationReply } from './swReply';
+import { pickReplySession, replyFailureText, sendNotificationReply } from './swReply';
 
 export type {};
 declare const self: ServiceWorkerGlobalScope & {
@@ -40,6 +40,8 @@ precacheAndRoute(self.__WB_MANIFEST);
 type SessionInfo = {
   accessToken: string;
   baseUrl: string;
+  /** [Gitea #299] whose session this is, so a notification reply uses its own account */
+  userId?: string;
 };
 
 /**
@@ -63,9 +65,13 @@ async function cleanupDeadClients() {
   });
 }
 
-function setSession(clientId: string, accessToken: string, baseUrl: string) {
+function setSession(clientId: string, accessToken: string, baseUrl: string, userId?: unknown) {
   if (typeof accessToken === 'string' && typeof baseUrl === 'string') {
-    sessions.set(clientId, { accessToken, baseUrl });
+    sessions.set(clientId, {
+      accessToken,
+      baseUrl,
+      userId: typeof userId === 'string' ? userId : undefined,
+    });
   } else {
     // Logout or invalid session
     sessions.delete(clientId);
@@ -130,10 +136,10 @@ self.addEventListener('message', (event: ExtendableMessageEvent) => {
   const client = event.source as Client | null;
   if (!client) return;
 
-  const { type, accessToken, baseUrl } = event.data || {};
+  const { type, accessToken, baseUrl, userId } = event.data || {};
 
   if (type === 'setSession') {
-    setSession(client.id, accessToken, baseUrl);
+    setSession(client.id, accessToken, baseUrl, userId);
     cleanupDeadClients();
   }
 });
@@ -155,15 +161,19 @@ self.addEventListener('notificationclick', (event: NotificationEvent) => {
     if (text) {
       event.waitUntil(
         sendNotificationReply(
-          Array.from(sessions.values()).pop(),
+          // [Gitea #299] The notification's own account, not the newest session.
+          pickReplySession(
+            Array.from(sessions.values()),
+            typeof data.userId === 'string' ? data.userId : undefined,
+          ),
           data.roomId,
           data.threadId,
           text,
-        ).then((ok) => {
-          if (!ok) {
+        ).then((result) => {
+          if (result !== 'sent') {
             // Let the user know it didn't go out, and keep their text reachable.
             return self.registration.showNotification('Reply not sent', {
-              body: `Couldn't send "${text.slice(0, 60)}" — open the app to retry.`,
+              body: replyFailureText(result, text),
               tag: `swreply-fail-${data.roomId}`,
               data: { path },
             });

src/swReply.test.ts

@@ -1,6 +1,12 @@
 import { test } from 'node:test';
 import assert from 'node:assert/strict';
-import { buildReplyContent, replySendUrl, sendNotificationReply } from './swReply';
+import {
+  buildReplyContent,
+  pickReplySession,
+  replyFailureText,
+  replySendUrl,
+  sendNotificationReply,
+} from './swReply';
 
 test('thread replies carry the m.thread relation, room replies do not', () => {
   assert.deepEqual(buildReplyContent('hi'), { msgtype: 'm.text', body: 'hi', 'm.mentions': {} });
@@ -19,40 +25,64 @@ test('send URL encodes the room id against the session base URL', () => {
   );
 });
 
-test('sendNotificationReply PUTs with the bearer token and reports ok / failure', async () => {
+test('sendNotificationReply checks encryption, then PUTs with the bearer token (#203, #299)', async () => {
   const calls: { url: string; init: RequestInit }[] = [];
-  const fetchOk = (async (url: string, init: RequestInit) => {
-    calls.push({ url, init });
-    return { ok: true } as Response;
-  }) as unknown as typeof fetch;
-  assert.equal(
-    await sendNotificationReply(
-      { accessToken: 'tok', baseUrl: 'https://hs' },
-      '!r:hs',
-      '$t',
-      'yo',
-      fetchOk,
-    ),
-    true,
-  );
-  assert.equal(calls[0].init.method, 'PUT');
-  assert.equal((calls[0].init.headers as Record<string, string>).Authorization, 'Bearer tok');
+  const fetchFor = (encStatus: number, sendOk = true) =>
+    (async (url: string, init: RequestInit = {}) => {
+      calls.push({ url, init });
+      if (url.includes('/state/m.room.encryption/')) {
+        return { ok: encStatus === 200, status: encStatus } as Response;
+      }
+      return { ok: sendOk, status: sendOk ? 200 : 500 } as Response;
+    }) as unknown as typeof fetch;
+  const session = { accessToken: 'tok', baseUrl: 'https://hs' };
+
+  assert.equal(await sendNotificationReply(session, '!r:hs', '$t', 'yo', fetchFor(404)), 'sent');
+  assert.match(calls[0].url, /\/rooms\/!r%3Ahs\/state\/m\.room\.encryption\/$/);
+  const put = calls[1];
+  assert.equal(put.init.method, 'PUT');
+  assert.equal((put.init.headers as Record<string, string>).Authorization, 'Bearer tok');
   assert.match(
-    calls[0].url,
+    put.url,
     /^https:\/\/hs\/_matrix\/client\/v3\/rooms\/!r%3Ahs\/send\/m\.room\.message\/swreply-/,
   );
-  assert.equal(JSON.parse(calls[0].init.body as string)['m.relates_to'].event_id, '$t');
+  assert.equal(JSON.parse(put.init.body as string)['m.relates_to'].event_id, '$t');
 
-  const fetchFail = (async () => ({ ok: false }) as Response) as unknown as typeof fetch;
+  // Encryption turned on since the notification was shown: nothing is sent.
+  calls.length = 0;
   assert.equal(
-    await sendNotificationReply(
-      { accessToken: 'tok', baseUrl: 'https://hs' },
-      '!r:hs',
-      undefined,
-      'yo',
-      fetchFail,
-    ),
-    false,
+    await sendNotificationReply(session, '!r:hs', undefined, 'yo', fetchFor(200)),
+    'encrypted',
+  );
+  assert.equal(calls.length, 1);
+  // Can't tell (server error): fail safe, nothing is sent.
+  calls.length = 0;
+  assert.equal(
+    await sendNotificationReply(session, '!r:hs', undefined, 'yo', fetchFor(500)),
+    'failed',
+  );
+  assert.equal(calls.length, 1);
+  assert.equal(
+    await sendNotificationReply(session, '!r:hs', undefined, 'yo', fetchFor(404, false)),
+    'failed',
+  );
+  assert.equal(
+    await sendNotificationReply(undefined, '!r:hs', undefined, 'yo', fetchFor(404)),
+    'no-session',
   );
-  assert.equal(await sendNotificationReply(undefined, '!r:hs', undefined, 'yo', fetchOk), false);
+});
+
+test('pickReplySession: the notification account only (#299)', () => {
+  const a = { accessToken: 'a', baseUrl: 'https://hs', userId: '@a:hs' };
+  const b = { accessToken: 'b', baseUrl: 'https://hs', userId: '@b:hs' };
+  assert.equal(pickReplySession([a, b], '@a:hs'), a);
+  assert.equal(pickReplySession([b], '@a:hs'), undefined, 'logged out of @a here');
+  assert.equal(pickReplySession([a], undefined), a, 'old notification, one session');
+  assert.equal(pickReplySession([a, b], undefined), undefined, 'old notification, ambiguous');
+});
+
+test('replyFailureText says why', () => {
+  assert.match(replyFailureText('encrypted', 'hi'), /now encrypted/);
+  assert.match(replyFailureText('no-session', 'hi'), /no longer signed in/);
+  assert.match(replyFailureText('failed', 'hi'), /retry/);
 });

src/swReply.ts

@@ -1,9 +1,42 @@
 /**
  * [Gitea #203] Build and send an inline reply typed into a notification's
  * text action. Lives outside sw.ts so it can be unit-tested; the SW passes in
- * its newest known session and the global fetch.
+ * the notification account's session and the global fetch.
  */
-export type SwReplySession = { accessToken: string; baseUrl: string };
+export type SwReplySession = { accessToken: string; baseUrl: string; userId?: string };
+
+export type SwReplyResult = 'sent' | 'failed' | 'encrypted' | 'no-session';
+
+/**
+ * [Gitea #299] The session to reply with: the notification's own account. A
+ * notification without a user id (shown before this change) may only use the
+ * session if it is the only one, so a reply never posts as another account.
+ */
+export const pickReplySession = (
+  sessions: SwReplySession[],
+  userId: string | undefined,
+): SwReplySession | undefined => {
+  if (userId) return sessions.filter((x) => x.userId === userId).pop();
+  return sessions.length === 1 ? sessions[0] : undefined;
+};
+
+/** What the "Reply not sent" notification says for each failure. */
+export const replyFailureText = (result: SwReplyResult, text: string): string => {
+  const quoted = `"${text.slice(0, 60)}"`;
+  if (result === 'encrypted') {
+    return `This room is now encrypted, so ${quoted} wasn't sent. Open the app to reply.`;
+  }
+  if (result === 'no-session') {
+    return `You're no longer signed in to that account here, so ${quoted} wasn't sent.`;
+  }
+  return `Couldn't send ${quoted}. Open the app to retry.`;
+};
+
+export const encryptionStateUrl = (baseUrl: string, roomId: string): string =>
+  new URL(
+    `/_matrix/client/v3/rooms/${encodeURIComponent(roomId)}/state/m.room.encryption/`,
+    baseUrl,
+  ).href;
 
 export const buildReplyContent = (text: string, threadId?: string): Record<string, unknown> => {
   const content: Record<string, unknown> = { msgtype: 'm.text', body: text, 'm.mentions': {} };
@@ -30,20 +63,24 @@ export async function sendNotificationReply(
   threadId: string | undefined,
   text: string,
   fetchFn: typeof fetch = fetch,
-): Promise<boolean> {
-  if (!session?.accessToken || !session.baseUrl) return false;
-  const txn = `swreply-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
+): Promise<SwReplyResult> {
+  if (!session?.accessToken || !session.baseUrl) return 'no-session';
+  const headers = { Authorization: `Bearer ${session.accessToken}` };
   try {
+    // [Gitea #299] The SW can't encrypt, and the reply action was offered when
+    // the notification was shown — possibly hours ago. Re-check now: only a
+    // definite "no encryption state" (404) lets the plain reply go out.
+    const enc = await fetchFn(encryptionStateUrl(session.baseUrl, roomId), { headers });
+    if (enc.ok) return 'encrypted';
+    if (enc.status !== 404) return 'failed';
+    const txn = `swreply-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
     const res = await fetchFn(replySendUrl(session.baseUrl, roomId, txn), {
       method: 'PUT',
-      headers: {
-        Authorization: `Bearer ${session.accessToken}`,
-        'Content-Type': 'application/json',
-      },
+      headers: { ...headers, 'Content-Type': 'application/json' },
       body: JSON.stringify(buildReplyContent(text, threadId)),
     });
-    return res.ok;
+    return res.ok ? 'sent' : 'failed';
   } catch {
-    return false;
+    return 'failed';
   }
 }