Skip to content

refactor(push): make encryption/signing more robust #2619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
joshtrichards wants to merge 7 commits into
base: master
Choose a base branch
from
+324 −51
Draft

refactor(push): make encryption/signing more robust #2619

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ecc7ed6
refactor(push): make encryption/signing more robust
joshtrichards Nov 1, 2025
a173f17
Add PushSigningException class for notifications
joshtrichards Nov 1, 2025
6323254
chore: fix namespace for PushSigningException
joshtrichards Nov 1, 2025
db30703
fix: catch sig exception and skip device
joshtrichards Nov 1, 2025
23d3a62
chore: make pushDeleteToDevice consistent with pushToDevice
joshtrichards Nov 1, 2025
2bc74ae
chore: tidy up
joshtrichards Nov 1, 2025
c6cf1a5
chore: Add tests for push to device signing errors
joshtrichards Nov 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
chore: Add tests for push to device signing errors 
Signed-off-by: Josh <[email protected]>
  • Loading branch information
@joshtrichards
joshtrichards authored Nov 1, 2025
commit c6cf1a5142aabb65a5077b710e0906698510a761
166 changes: 166 additions & 0 deletions tests/Unit/PushTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,172 @@ public function testPushToDeviceTalkNotification(array $deviceTypes, bool $isTal
$push->pushToDevice(200718, $notification);
}

public function testPushToDeviceSigningError(): void {
$push = $this->getPush(['createFakeUserObject', 'getDevicesForUser', 'encryptAndSign', 'deletePushToken', 'validateToken']);
$this->clientService->expects($this->never())
->method('newClient');

$this->config->expects($this->once())
->method('getSystemValueBool')
->with('has_internet_connection', true)
->willReturn(true);

/** @var INotification&MockObject $notification */
$notification = $this->createMock(INotification::class);
$notification
->method('getUser')
->willReturn('valid');
$user = $this->createMock(IUser::class);

$push->expects($this->once())
->method('createFakeUserObject')
->with('valid')
->willReturn($user);

$push->expects($this->once())
->method('getDevicesForUser')
->willReturn([[
'proxyserver' => 'proxyserver1',
'token' => 23,
'apptype' => 'other',
]]);

$this->l10nFactory
->method('getUserLanguage')
->with($user)
->willReturn('ru');

$this->notificationManager->expects($this->once())
->method('prepare')
->with($notification, 'ru')
->willReturnArgument(0);

$key = $this->createMock(Key::class);
$this->keyManager->expects($this->once())
->method('getKey')
->with($user)
->willReturn($key);

$push->expects($this->once())
->method('validateToken')
->willReturn(true);

$push->expects($this->once())
->method('encryptAndSign')
->willThrowException(new \OCA\Notifications\Exceptions\PushSigningException());

$push->expects($this->never())
->method('deletePushToken');

$push->pushToDevice(1971, $notification);
}

public function testPushDeleteToDeviceSigningError(): void {
$push = $this->getPush(['createFakeUserObject', 'getDevicesForUser', 'encryptAndSignDelete', 'deletePushToken', 'validateToken']);
$this->clientService->expects($this->never())
->method('newClient');

$this->config->expects($this->once())
->method('getSystemValueBool')
->with('has_internet_connection', true)
->willReturn(true);

$userId = 'valid';
$user = $this->createMock(IUser::class);
$push->expects($this->once())
->method('createFakeUserObject')
->with($userId)
->willReturn($user);

$push->expects($this->once())
->method('getDevicesForUser')
->willReturn([[
'proxyserver' => 'proxyserver1',
'token' => 23,
'apptype' => 'other',
]]);

$push->expects($this->once())
->method('validateToken')
->willReturn(true);

$key = $this->createMock(Key::class);
$this->keyManager->expects($this->once())
->method('getKey')
->with($user)
->willReturn($key);

$push->expects($this->once())
->method('encryptAndSignDelete')
->willThrowException(new \OCA\Notifications\Exceptions\PushSigningException());

$push->expects($this->never())
->method('deletePushToken');

$push->pushDeleteToDevice($userId, [42], 'other');
}

public function testPushDeleteToDeviceSigningErrorHaltsQueue(): void {
$push = $this->getPush([
'createFakeUserObject',
'getDevicesForUser',
'encryptAndSignDelete',
'deletePushToken',
'validateToken'
]);
$this->clientService->expects($this->never())
->method('newClient');

$this->config->expects($this->once())
->method('getSystemValueBool')
->with('has_internet_connection', true)
->willReturn(true);

$userId = 'valid';
$user = $this->createMock(\OCP\IUser::class);

$push->expects($this->once())
->method('createFakeUserObject')
->with($userId)
->willReturn($user);

$push->expects($this->once())
->method('getDevicesForUser')
->willReturn([[
'proxyserver' => 'proxyserver1',
'token' => 23,
'apptype' => 'other',
]]);

$push->expects($this->once())
->method('validateToken')
->willReturn(true);

$key = $this->createMock(\OC\Security\IdentityProof\Key::class);
$this->keyManager->expects($this->once())
->method('getKey')
->with($user)
->willReturn($key);

// Simulate queue of 3 notification IDs, but always throw signing error
$push->expects($this->once())
->method('encryptAndSignDelete')
->with($key, [
'proxyserver' => 'proxyserver1',
'token' => 23,
'apptype' => 'other',
], [101, 102, 103])
->willThrowException(new \OCA\Notifications\Exceptions\PushSigningException());

// Should NOT delete the token, nor call encryptAndSignDelete a second time
$push->expects($this->never())
->method('deletePushToken');

// Call function: expects no exception, but also expects no further queue processing
$push->pushDeleteToDevice($userId, [101, 102, 103], 'other');
// Consider adding assertions about $push->payloadsToSend
}

public static function dataValidateToken(): array {
return [
[1239999999, 1230000000, OCPIToken::WIPE_TOKEN, false],
Expand Down
Loading