From 352f30d7a653889697e0a4437c66ce27f716891d Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 1 Oct 2026 10:39:56 +0200 Subject: [PATCH 1/2] fix(webpush): Correctly hide already read notifications from webpush Assisted-by: ClaudeCode:claude-opus-5.5 Signed-off-by: Joas Schilling --- docs/webpush-v2.md | 6 +++++- lib/Push.php | 24 ++++++++++++++------- lib/WebPushClient.php | 5 +++-- src/workers/service-worker.js | 26 ++++++++++++++++++++--- tests/Unit/PushTest.php | 36 +++++++++++++++++++++++++++++++- tests/Unit/WebPushClientTest.php | 2 ++ 6 files changed, 84 insertions(+), 15 deletions(-) diff --git a/docs/webpush-v2.md b/docs/webpush-v2.md index 6c249b27a..4aad7d47a 100644 --- a/docs/webpush-v2.md +++ b/docs/webpush-v2.md @@ -238,6 +238,8 @@ Notifications are filtered per subscription using the stored `appTypes` before s The notification `urgency` ([RFC 8030 §5.3](https://www.rfc-editor.org/rfc/rfc8030#section-5.3)) is derived from the app/type: Talk, two-factor and phonetrack notifications use `high`, everything else `normal`. +Content notifications and single delete notifications are sent with the `Topic` `nid{{nid}}` ([RFC 8030 §5.4](https://www.rfc-editor.org/rfc/rfc8030#section-5.4)). When a notification is removed before the push service delivered it (e.g. the browser was offline and the notification was read on another device), the delete replaces the pending notification, so the browser never shows it. + ### Normal content notification The web push payload uses the same shape as the [mobile push subject](push-v2.md#encrypted-subject-data), but it is delivered as clear (already-decrypted) JSON and may carry a longer `subject` (the payload budget is ~3000 bytes instead of ~240). If you are missing any information necessary to parse the notification, use the `nid` to fetch the full notification via the [OCS API](ocs-endpoint-v2.md). @@ -288,6 +290,8 @@ These notifications should not be shown to the user. Instead, delete pending not ### Silent delete notifications (multiple) +The server sends a single delete notification per `nid` to web push subscriptions, so the `Topic` replacement works. Clients should still handle this payload. + ```json { "nids" : [1337], @@ -318,7 +322,7 @@ These notifications should not be shown to the user. Instead, delete pending not The app ships a service worker (registered from `/apps/notifications/service-worker.js`) that handles the browser `push` event: 1. If a Nextcloud tab is open, the decrypted payload is forwarded to it via `postMessage` (`{ type: 'push', content }`) and the in-page UI handles it. If the payload carries an `activationToken`, the page performs the activation request instead. -2. If no tab is open and the payload has a `subject`, the service worker shows a background notification (first line of the subject becomes the title, the rest the body). +2. If no tab is open and the payload has a `subject`, the service worker shows a background notification (first line of the subject becomes the title, the rest the body) tagged `nid{{nid}}`. A delete payload arriving with no open tab closes the background notifications with the matching tags (all of them for `delete-all`). 3. If there is no displayable content (e.g. a delete payload arriving with no open tab) and the browser requires `userVisibleOnly`, a short-lived silent placeholder notification is shown so the browser does not unregister the subscription for staying silent. On `pushsubscriptionchange` the service worker asks the page to re-register (`{ type: 'pushEndpoint' }`), driving the renewal flow described above. diff --git a/lib/Push.php b/lib/Push.php index 4c6825080..0010e575c 100644 --- a/lib/Push.php +++ b/lib/Push.php @@ -421,7 +421,8 @@ public function webPushToDevice(int $id, IUser $user, array $devices, INotificat $device['ua_public'], $device['auth'], json_encode($data, JSON_THROW_ON_ERROR), - urgency: $urgency + urgency: $urgency, + topic: $this->getWebPushTopic($id), ); } catch (\JsonException $e) { $this->log->error('JSON error while encoding push notification: ' . $e->getMessage(), ['exception' => $e]); @@ -588,14 +589,13 @@ public function webPushDeleteToDevice(string $userId, IUser $user, array $device $this->log->error('JSON error while encoding push notification: ' . $e->getMessage(), ['exception' => $e]); } } else { - $temp = $notificationIds; - - while (!empty($temp)) { - $data = $this->encodeDeleteNotifs($temp); - $temp = $data['remaining']; + // Single deletes with the notification's topic replace the + // content push if it was not delivered yet (RFC 8030 §5.4) + foreach ($notificationIds ?? [] as $notificationId) { + $data = $this->encodeDeleteNotifs([$notificationId]); try { $payload = json_encode($data['data'], JSON_THROW_ON_ERROR); - $this->wpClient->enqueue($device['endpoint'], $device['ua_public'], $device['auth'], $payload); + $this->wpClient->enqueue($device['endpoint'], $device['ua_public'], $device['auth'], $payload, topic: $this->getWebPushTopic($notificationId)); } catch (\JsonException $e) { $this->log->error('JSON error while encoding push notification: ' . $e->getMessage(), ['exception' => $e]); } @@ -608,7 +608,7 @@ public function webPushDeleteToDevice(string $userId, IUser $user, array $device } if (!$this->deferPayloads) { - $this->sendNotificationsToProxies(); + $this->wpClient->flush(fn ($r) => $this->webPushCallback($r)); } } @@ -923,6 +923,14 @@ protected function encodeNotif(int $id, INotification $notification, int $maxLen return $data; } + /** + * Topic of the web push messages of a notification, so a delete replaces + * a not yet delivered notification at the push service (RFC 8030 §5.4) + */ + protected function getWebPushTopic(int $notificationId): string { + return 'nid' . $notificationId; + } + /** * @param ?int[] $ids * @return array diff --git a/lib/WebPushClient.php b/lib/WebPushClient.php index 403cd5c24..8669759b7 100644 --- a/lib/WebPushClient.php +++ b/lib/WebPushClient.php @@ -136,13 +136,14 @@ public function notify(string $endpoint, string $uaPublicKey, string $auth, stri * Queue one notification. [flush] needs to be called to actually send the notifications * @throws \ErrorException */ - public function enqueue(string $endpoint, string $uaPublicKey, string $auth, string $body, string $urgency = 'normal'): void { + public function enqueue(string $endpoint, string $uaPublicKey, string $auth, string $body, string $urgency = 'normal', ?string $topic = null): void { $c = $this->getClient(); $c->queueNotification( new Subscription($endpoint, $uaPublicKey, $auth, 'aes128gcm'), $body, options: [ - 'urgency' => $urgency + 'urgency' => $urgency, + 'topic' => $topic, ] ); } diff --git a/src/workers/service-worker.js b/src/workers/service-worker.js index e8420f06c..a272c7a28 100644 --- a/src/workers/service-worker.js +++ b/src/workers/service-worker.js @@ -6,16 +6,33 @@ 'use strict' /** - * @param {string} content received by Nextcloud, splitted to be shown in UI notification + * @param {object} content received by Nextcloud, the subject is splitted to be shown in UI notification */ function showBgNotification(content) { - const [title, ...msg] = content.split('\n') + const [title, ...msg] = content.subject.split('\n') const options = { body: msg.join('\n'), + tag: 'nid' + content.nid, } return self.registration.showNotification(title, options) } +/** + * Close the shown notifications of a delete payload + * + * @param {object} content received by Nextcloud + */ +function closeBgNotifications(content) { + let tags = [undefined] + if (content.delete) { + tags = ['nid' + content.nid] + } else if (content['delete-multiple']) { + tags = content.nids.map((nid) => 'nid' + nid) + } + return Promise.all(tags.map((tag) => self.registration.getNotifications({ tag }) + .then((notifications) => notifications.forEach((notification) => notification.close())))) +} + /** * For Chrom* and Apple (who followed Chrome) users: * We need to show a silent notification that we remove to avoid being unregistered, @@ -53,7 +70,10 @@ self.addEventListener('push', function(event) { // Here, the user has an active tab, we don't need to show a notification from the sw } else if (content.subject) { console.debug('No valid client to send notif - showing bg notif') - return showBgNotification(content.subject) + return showBgNotification(content) + } else if (content.delete || content['delete-multiple'] || content['delete-all']) { + console.debug('No valid client to send notif - closing bg notifs') + return closeBgNotifications(content).then(silentNotification) } else { console.warn('No valid client to send notif') return silentNotification() diff --git a/tests/Unit/PushTest.php b/tests/Unit/PushTest.php index c51c7537b..a4c6e0bcd 100644 --- a/tests/Unit/PushTest.php +++ b/tests/Unit/PushTest.php @@ -1213,7 +1213,8 @@ public function testWebPushToDeviceSending(bool $isRateLimited): void { ->method('deleteWebPushToken'); $this->wpClient->expects($this->exactly($isRateLimited ? 1 : 2)) - ->method('enqueue'); + ->method('enqueue') + ->with($this->anything(), self::EX_UA_PUBLIC, self::EX_AUTH, $this->anything(), 'normal', 'nid207787'); if ($isRateLimited) { $this->cache @@ -1233,6 +1234,39 @@ public function testWebPushToDeviceSending(bool $isRateLimited): void { $push->pushToDevice(207787, $notification); } + public static function dataWebPushDeleteToDevice(): array { + return [ + 'single' => [[42], [['{"nid":42,"delete":true}', 'nid42']]], + 'multiple' => [[42, 1337], [['{"nid":42,"delete":true}', 'nid42'], ['{"nid":1337,"delete":true}', 'nid1337']]], + 'all' => [null, [['{"delete-all":true}', null]]], + ]; + } + + #[DataProvider(methodName: 'dataWebPushDeleteToDevice')] + public function testWebPushDeleteToDevice(?array $notificationIds, array $expected): void { + $push = $this->getPush(['deleteWebPushToken']); + + $calls = []; + $this->wpClient->expects($this->exactly(count($expected))) + ->method('enqueue') + ->willReturnCallback(static function (string $endpoint, string $uaPublicKey, string $auth, string $body, string $urgency = 'normal', ?string $topic = null) use (&$calls): void { + $calls[] = [$body, $topic]; + }); + $this->wpClient->expects($this->once()) + ->method('flush'); + + $push->webPushDeleteToDevice('valid', $this->createStub(IUser::class), [ + [ + 'endpoint' => 'endpoint1', + 'ua_public' => self::EX_UA_PUBLIC, + 'auth' => self::EX_AUTH, + 'token' => 16, + ], + ], $notificationIds === null, $notificationIds); + + $this->assertSame($expected, $calls); + } + public static function dataFilterWebPushDeviceList(): array { return [ [false, 'all', 'myApp', false], diff --git a/tests/Unit/WebPushClientTest.php b/tests/Unit/WebPushClientTest.php index adce6ba17..450963680 100644 --- a/tests/Unit/WebPushClientTest.php +++ b/tests/Unit/WebPushClientTest.php @@ -105,6 +105,7 @@ public function testFlushSendsQueuedNotificationsWithTheNextcloudClient(): void $this->callback(static function (array $options): bool { return $options['http_errors'] === false && $options['headers']['Urgency'] === ['high'] + && $options['headers']['Topic'] === ['nid42'] && $options['headers']['Content-Encoding'] === ['aes128gcm'] && $options['body'] !== ''; }), @@ -120,6 +121,7 @@ public function testFlushSendsQueuedNotificationsWithTheNextcloudClient(): void self::$auth, '{"message":"test"}', 'high', + 'nid42', ); $reports = []; From 79a4537adf9dd9ef01828836573cd35b5f6b7a57 Mon Sep 17 00:00:00 2001 From: nextcloud-command Date: Thu, 1 Oct 2026 08:54:25 +0000 Subject: [PATCH 2/2] chore(assets): Recompile assets Signed-off-by: nextcloud-command --- js/service-worker.js | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/js/service-worker.js b/js/service-worker.js index e8420f06c..a272c7a28 100644 --- a/js/service-worker.js +++ b/js/service-worker.js @@ -6,16 +6,33 @@ 'use strict' /** - * @param {string} content received by Nextcloud, splitted to be shown in UI notification + * @param {object} content received by Nextcloud, the subject is splitted to be shown in UI notification */ function showBgNotification(content) { - const [title, ...msg] = content.split('\n') + const [title, ...msg] = content.subject.split('\n') const options = { body: msg.join('\n'), + tag: 'nid' + content.nid, } return self.registration.showNotification(title, options) } +/** + * Close the shown notifications of a delete payload + * + * @param {object} content received by Nextcloud + */ +function closeBgNotifications(content) { + let tags = [undefined] + if (content.delete) { + tags = ['nid' + content.nid] + } else if (content['delete-multiple']) { + tags = content.nids.map((nid) => 'nid' + nid) + } + return Promise.all(tags.map((tag) => self.registration.getNotifications({ tag }) + .then((notifications) => notifications.forEach((notification) => notification.close())))) +} + /** * For Chrom* and Apple (who followed Chrome) users: * We need to show a silent notification that we remove to avoid being unregistered, @@ -53,7 +70,10 @@ self.addEventListener('push', function(event) { // Here, the user has an active tab, we don't need to show a notification from the sw } else if (content.subject) { console.debug('No valid client to send notif - showing bg notif') - return showBgNotification(content.subject) + return showBgNotification(content) + } else if (content.delete || content['delete-multiple'] || content['delete-all']) { + console.debug('No valid client to send notif - closing bg notifs') + return closeBgNotifications(content).then(silentNotification) } else { console.warn('No valid client to send notif') return silentNotification()