From 8fff9504351f26c446df4c9733cf9ed1f3a034aa 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 [skip ci] --- docs/webpush-v2.md | 6 +++++- lib/Push.php | 24 +++++++++++++++-------- lib/WebPushClient.php | 5 +++-- src/workers/service-worker.js | 26 ++++++++++++++++++++++--- tests/Unit/PushTest.php | 36 ++++++++++++++++++++++++++++++++++- 5 files changed, 82 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 b0d33ef4f..e39f4b2c4 100644 --- a/lib/Push.php +++ b/lib/Push.php @@ -419,7 +419,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]); @@ -586,14 +587,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]); } @@ -606,7 +606,7 @@ public function webPushDeleteToDevice(string $userId, IUser $user, array $device } if (!$this->deferPayloads) { - $this->sendNotificationsToProxies(); + $this->wpClient->flush(fn ($r) => $this->webPushCallback($r)); } } @@ -921,6 +921,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 a0774592d..cf2901ee7 100644 --- a/lib/WebPushClient.php +++ b/lib/WebPushClient.php @@ -124,13 +124,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 2c2da7372..39b996737 100644 --- a/tests/Unit/PushTest.php +++ b/tests/Unit/PushTest.php @@ -1216,7 +1216,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 @@ -1236,6 +1237,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], From f1724b56747c71852406d9318501ad5ffb6af2be Mon Sep 17 00:00:00 2001 From: nextcloud-command Date: Thu, 29 Oct 2026 08:30:01 +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()