Skip to content

Commit 80d4c71

Browse files
committed
fix(webpush): recover gracefully from corrupted or mismatched VAPID keys
If the stored VAPID keys were encrypted with a different instance secret (e.g. after a migration or secret rotation), decrypting them throws an exception in WebPushClient::getVapid(). This caused the entire request to fail, even for unrelated operations like joining a Talk room (which triggers notification processing as a side effect). Catch any Throwable from getAppValueString() and treat it the same as missing keys: regenerate a fresh VAPID key pair and store it with the current secret. This self-heals the broken state transparently. Signed-off-by: Anna Larch <anna@nextcloud.com> AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 6ba332d commit 80d4c71

2 files changed

Lines changed: 81 additions & 2 deletions

File tree

lib/WebPushClient.php

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,14 @@ private function getClient(): WebPush {
6666
*/
6767
private function getVapid(): array {
6868
// Do not use lazy for now
69-
$publicKey = $this->appConfig->getAppValueString('webpush_vapid_pubkey');
70-
$privateKey = $this->appConfig->getAppValueString('webpush_vapid_privkey');
69+
try {
70+
$publicKey = $this->appConfig->getAppValueString('webpush_vapid_pubkey');
71+
$privateKey = $this->appConfig->getAppValueString('webpush_vapid_privkey');
72+
} catch (\Throwable) {
73+
// Decryption failed (e.g. mismatched instance secret), regenerate keys
74+
$publicKey = '';
75+
$privateKey = '';
76+
}
7177
if ($publicKey === '' || $privateKey === '') {
7278
/** @var array{publicKey: string, privateKey: string} $vapid */
7379
$vapid = VAPID::createVapidKeys();

tests/Unit/WebPushClientTest.php

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Notifications\Tests\Unit;
11+
12+
use OCA\Notifications\WebPushClient;
13+
use OCP\AppFramework\Services\IAppConfig;
14+
use PHPUnit\Framework\MockObject\MockObject;
15+
use Test\TestCase;
16+
17+
class WebPushClientTest extends TestCase {
18+
protected IAppConfig&MockObject $appConfig;
19+
20+
protected function setUp(): void {
21+
parent::setUp();
22+
$this->appConfig = $this->createMock(IAppConfig::class);
23+
}
24+
25+
public function testConstructSucceedsWhenVapidKeysAreStored(): void {
26+
$this->appConfig->method('getAppValueString')
27+
->willReturnMap([
28+
['webpush_vapid_pubkey', '', false, 'BCVxsr7N_eNgVRqvHtD0zTZsEc6-VV-JvLexhqUzORcxaOzi6-AYWXvTBHm4bjyPjs7Vd8pZGH6SRpkNtoIAiw'],
29+
['webpush_vapid_privkey', '', false, 'test-private-key'],
30+
]);
31+
32+
$this->appConfig->expects($this->never())->method('setAppValueString');
33+
34+
$client = new WebPushClient($this->appConfig);
35+
$this->assertInstanceOf(WebPushClient::class, $client);
36+
}
37+
38+
public function testConstructRegeneratesVapidKeysWhenDecryptionFails(): void {
39+
// Simulates the case where the stored VAPID keys were encrypted with a
40+
// different instance secret — getAppValueString throws during decryption.
41+
$this->appConfig->method('getAppValueString')
42+
->willThrowException(new \RuntimeException('HMAC does not match.'));
43+
44+
$this->appConfig->expects($this->exactly(2))
45+
->method('setAppValueString')
46+
->with($this->logicalOr(
47+
$this->equalTo('webpush_vapid_pubkey'),
48+
$this->equalTo('webpush_vapid_privkey'),
49+
));
50+
51+
// Must not throw — corrupted keys should be transparently regenerated
52+
$client = new WebPushClient($this->appConfig);
53+
$this->assertInstanceOf(WebPushClient::class, $client);
54+
}
55+
56+
public function testConstructRegeneratesVapidKeysWhenMissing(): void {
57+
$this->appConfig->method('getAppValueString')
58+
->willReturnMap([
59+
['webpush_vapid_pubkey', '', false, ''],
60+
['webpush_vapid_privkey', '', false, ''],
61+
]);
62+
63+
$this->appConfig->expects($this->exactly(2))
64+
->method('setAppValueString')
65+
->with($this->logicalOr(
66+
$this->equalTo('webpush_vapid_pubkey'),
67+
$this->equalTo('webpush_vapid_privkey'),
68+
));
69+
70+
$client = new WebPushClient($this->appConfig);
71+
$this->assertInstanceOf(WebPushClient::class, $client);
72+
}
73+
}

0 commit comments

Comments
 (0)