diff --git a/lib/Push.php b/lib/Push.php index 2ee3d8faf..85de6a0ad 100644 --- a/lib/Push.php +++ b/lib/Push.php @@ -29,6 +29,8 @@ use OCP\AppFramework\Http; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\Http\Client\IClientService; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\IDBConnection; use OCP\ILogger; @@ -53,12 +55,22 @@ class Push { private $userManager; /** @var IClientService */ protected $clientService; + /** @var ICache */ + protected $cache; /** @var ILogger */ protected $log; /** @var OutputInterface */ protected $output; - public function __construct(IDBConnection $connection, INotificationManager $notificationManager, IConfig $config, IProvider $tokenProvider, Manager $keyManager, IUserManager $userManager, IClientService $clientService, ILogger $log) { + public function __construct(IDBConnection $connection, + INotificationManager $notificationManager, + IConfig $config, + IProvider $tokenProvider, + Manager $keyManager, + IUserManager $userManager, + IClientService $clientService, + ICacheFactory $cacheFactory, + ILogger $log) { $this->db = $connection; $this->notificationManager = $notificationManager; $this->config = $config; @@ -66,6 +78,7 @@ public function __construct(IDBConnection $connection, INotificationManager $not $this->keyManager = $keyManager; $this->userManager = $userManager; $this->clientService = $clientService; + $this->cache = $cacheFactory->createDistributed('pushtokens'); $this->log = $log; } @@ -115,30 +128,42 @@ public function pushToDevice(int $id, INotification $notification, ?OutputInterf $this->printInfo('Public user key size: ' . strlen($userKey->getPublic())); $isTalkNotification = \in_array($notification->getApp(), ['spreed', 'talk', 'admin_notification_talk'], true); - $talkApps = array_filter($devices, function ($device) { + $talkDevices = array_filter($devices, function ($device) { return $device['apptype'] === 'talk'; }); - $hasTalkApps = !empty($talkApps); + $otherDevices = array_filter($devices, function ($device) { + return $device['apptype'] !== 'talk'; + }); + + $this->printInfo('Identified ' . count($talkDevices) . ' Talk devices and ' . count($otherDevices) . ' others.'); + + if (!$isTalkNotification) { + if (empty($otherDevices)) { + // We only send file notifications to the files app. + // If you don't have such a device, bye! + return; + } + $devices = $otherDevices; + } else { + if (empty($talkDevices)) { + // If you don't have a talk device, + // we fall back to the files app. + $devices = $otherDevices; + } else { + $devices = $talkDevices; + } + } + + // We don't push to devices that are older than 60 days + $maxAge = time() - 60 * 24 * 60 * 60; $pushNotifications = []; foreach ($devices as $device) { $this->printInfo(''); $this->printInfo('Device token:' . $device['token']); - if (!$isTalkNotification && $device['apptype'] === 'talk') { - // The iOS app can not kill notifications, - // therefor we should only send relevant notifications to the Talk - // app, so it does not pollute the notifications bar with useless - // notifications, especially when the Sync client app is also installed. - $this->printInfo('Skipping talk device for non-talk notification'); - continue; - } - if ($isTalkNotification && $hasTalkApps && $device['apptype'] !== 'talk') { - // Similar to the previous case, we also don't send Talk notifications - // to the Sync client app, when there is a Talk app installed. We only - // do this, when you don't have a Talk app on your device, so you still - // get the push notification. - $this->printInfo('Skipping other device for talk notification because a talk app is available'); + if (!$this->validateToken($device['token'], $maxAge)) { + // Token does not exist anymore continue; } @@ -150,10 +175,6 @@ public function pushToDevice(int $id, INotification $notification, ?OutputInterf $pushNotifications[$proxyServer] = []; } $pushNotifications[$proxyServer][] = $payload; - } catch (InvalidTokenException $e) { - // Token does not exist anymore, should drop the push device entry - $this->printInfo('InvalidTokenException is thrown'); - $this->deletePushToken($device['token']); } catch (\InvalidArgumentException $e) { // Failed to encrypt message for device: public key is invalid $this->deletePushToken($device['token']); @@ -174,9 +195,17 @@ public function pushDeleteToDevice(string $userId, int $notificationId): void { return; } + // We don't push to devices that are older than 60 days + $maxAge = time() - 60 * 24 * 60 * 60; + $userKey = $this->keyManager->getKey($user); $pushNotifications = []; foreach ($devices as $device) { + if (!$this->validateToken($device['token'], $maxAge)) { + // Token does not exist anymore + continue; + } + try { $payload = json_encode($this->encryptAndSignDelete($userKey, $device, $notificationId)); @@ -185,9 +214,6 @@ public function pushDeleteToDevice(string $userId, int $notificationId): void { $pushNotifications[$proxyServer] = []; } $pushNotifications[$proxyServer][] = $payload; - } catch (InvalidTokenException $e) { - // Token does not exist anymore, should drop the push device entry - $this->deletePushToken($device['token']); } catch (\InvalidArgumentException $e) { // Failed to encrypt message for device: public key is invalid $this->deletePushToken($device['token']); @@ -252,6 +278,26 @@ protected function sendNotificationsToProxies(array $pushNotifications): void { } } + protected function validateToken(int $tokenId, int $maxAge): bool { + $age = $this->cache->get('t' . $tokenId); + if ($age !== null) { + return $age > $maxAge; + } + + try { + // Check if the token is still valid... + $token = $this->tokenProvider->getTokenById($tokenId); + $this->cache->set('t' . $tokenId, $token->getLastCheck(), 600); + return $token->getLastCheck() > $maxAge; + } catch (InvalidTokenException $e) { + // Token does not exist anymore, should drop the push device entry + $this->printInfo('InvalidTokenException is thrown'); + $this->deletePushToken($tokenId); + $this->cache->set('t' . $tokenId, 0, 600); + return false; + } + } + /** * @param Key $userKey * @param array $device @@ -263,9 +309,6 @@ protected function sendNotificationsToProxies(array $pushNotifications): void { * @throws \InvalidArgumentException */ protected function encryptAndSign(Key $userKey, array $device, int $id, INotification $notification, bool $isTalkNotification): array { - // Check if the token is still valid... - $this->tokenProvider->getTokenById($device['token']); - $data = [ 'nid' => $id, 'app' => $notification->getApp(), @@ -348,9 +391,6 @@ protected function shortenJsonEncodedMultibyte(string $subject, int $dataLength) * @throws \InvalidArgumentException */ protected function encryptAndSignDelete(Key $userKey, array $device, int $id): array { - // Check if the token is still valid... - $this->tokenProvider->getTokenById($device['token']); - if ($id === 0) { $data = [ 'delete-all' => true, diff --git a/tests/Unit/PushTest.php b/tests/Unit/PushTest.php index d4481f5bf..57efa1408 100644 --- a/tests/Unit/PushTest.php +++ b/tests/Unit/PushTest.php @@ -31,6 +31,8 @@ use OCP\AppFramework\Http; use OCP\Http\Client\IClient; use OCP\Http\Client\IResponse; +use OCP\ICache; +use OCP\ICacheFactory; use OCP\IConfig; use OCP\Http\Client\IClientService; use OCP\IDBConnection; @@ -39,6 +41,7 @@ use OCP\IUserManager; use OCP\Notification\IManager as INotificationManager; use OCP\Notification\INotification; +use PHPUnit\Framework\MockObject\MockObject; /** * Class PushTest @@ -49,19 +52,23 @@ class PushTest extends TestCase { /** @var IDBConnection */ protected $db; - /** @var INotificationManager|\PHPUnit_Framework_MockObject_MockObject */ + /** @var INotificationManager|MockObject */ protected $notificationManager; - /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IConfig|MockObject */ protected $config; - /** @var IProvider|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IProvider|MockObject */ protected $tokenProvider; - /** @var Manager|\PHPUnit_Framework_MockObject_MockObject */ + /** @var Manager|MockObject */ protected $keyManager; - /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IUserManager|MockObject */ protected $userManager; - /** @var IClientService|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IClientService|MockObject */ protected $clientService; - /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ + /** @var ICacheFactory|MockObject */ + protected $cacheFactory; + /** @var ICache|MockObject */ + protected $cache; + /** @var ILogger|MockObject */ protected $logger; protected function setUp(): void { @@ -74,12 +81,18 @@ protected function setUp(): void { $this->keyManager = $this->createMock(Manager::class); $this->userManager = $this->createMock(IUserManager::class); $this->clientService = $this->createMock(IClientService::class); + $this->cacheFactory = $this->createMock(ICacheFactory::class); + $this->cache = $this->createMock(ICache::class); $this->logger = $this->createMock(ILogger::class); + + $this->cacheFactory->method('createDistributed') + ->with('pushtokens') + ->willReturn($this->cache); } /** * @param string[] $methods - * @return Push|\PHPUnit_Framework_MockObject_MockObject + * @return Push|MockObject */ protected function getPush(array $methods = []) { if (!empty($methods)) { @@ -92,6 +105,7 @@ protected function getPush(array $methods = []) { $this->keyManager, $this->userManager, $this->clientService, + $this->cacheFactory, $this->logger, ]) ->setMethods($methods) @@ -106,6 +120,7 @@ protected function getPush(array $methods = []) { $this->keyManager, $this->userManager, $this->clientService, + $this->cacheFactory, $this->logger ); } @@ -117,7 +132,7 @@ public function testPushToDeviceInvalidUser() { $this->clientService->expects($this->never()) ->method('newClient'); - /** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */ + /** @var INotification|MockObject$notification */ $notification = $this->createMock(INotification::class); $notification->expects($this->once()) ->method('getUser') @@ -138,13 +153,13 @@ public function testPushToDeviceNoDevices() { $this->clientService->expects($this->never()) ->method('newClient'); - /** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */ + /** @var INotification|MockObject $notification */ $notification = $this->createMock(INotification::class); $notification->expects($this->exactly(2)) ->method('getUser') ->willReturn('valid'); - /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ + /** @var IUser|MockObject $user */ $user = $this->createMock(IUser::class); $this->userManager->expects($this->once()) @@ -166,13 +181,13 @@ public function testPushToDeviceNotPrepared() { $this->clientService->expects($this->never()) ->method('newClient'); - /** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */ + /** @var INotification|MockObject $notification */ $notification = $this->createMock(INotification::class); $notification->expects($this->exactly(3)) ->method('getUser') ->willReturn('valid'); - /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ + /** @var IUser|MockObject $user */ $user = $this->createMock(IUser::class); $this->userManager->expects($this->once()) @@ -209,13 +224,13 @@ public function testPushToDeviceInvalidToken() { $this->clientService->expects($this->never()) ->method('newClient'); - /** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */ + /** @var INotification|MockObject $notification */ $notification = $this->createMock(INotification::class); $notification->expects($this->exactly(3)) ->method('getUser') ->willReturn('valid'); - /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ + /** @var IUser|MockObject $user */ $user = $this->createMock(IUser::class); $this->userManager->expects($this->once()) @@ -246,7 +261,7 @@ public function testPushToDeviceInvalidToken() { ->willReturnArgument(0); - /** @var Key|\PHPUnit_Framework_MockObject_MockObject $key */ + /** @var Key|MockObject $key */ $key = $this->createMock(Key::class); $this->keyManager->expects($this->once()) @@ -254,10 +269,13 @@ public function testPushToDeviceInvalidToken() { ->with($user) ->willReturn($key); - $push->expects($this->once()) - ->method('encryptAndSign') + $this->tokenProvider->expects($this->once()) + ->method('getTokenById') ->willThrowException(new InvalidTokenException()); + $push->expects($this->never()) + ->method('encryptAndSign'); + $push->expects($this->once()) ->method('deletePushToken') ->with(23); @@ -266,17 +284,17 @@ public function testPushToDeviceInvalidToken() { } public function testPushToDeviceEncryptionError() { - $push = $this->getPush(['getDevicesForUser', 'encryptAndSign', 'deletePushToken']); + $push = $this->getPush(['getDevicesForUser', 'encryptAndSign', 'deletePushToken', 'validateToken']); $this->clientService->expects($this->never()) ->method('newClient'); - /** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */ + /** @var INotification|MockObject $notification */ $notification = $this->createMock(INotification::class); $notification->expects($this->exactly(2)) ->method('getUser') ->willReturn('valid'); - /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ + /** @var IUser|MockObject $user */ $user = $this->createMock(IUser::class); $this->userManager->expects($this->once()) @@ -305,7 +323,7 @@ public function testPushToDeviceEncryptionError() { ->willReturnArgument(0); - /** @var Key|\PHPUnit_Framework_MockObject_MockObject $key */ + /** @var Key|MockObject $key */ $key = $this->createMock(Key::class); $this->keyManager->expects($this->once()) @@ -313,6 +331,10 @@ public function testPushToDeviceEncryptionError() { ->with($user) ->willReturn($key); + $push->expects($this->once()) + ->method('validateToken') + ->willReturn(true); + $push->expects($this->once()) ->method('encryptAndSign') ->willThrowException(new \InvalidArgumentException()); @@ -336,15 +358,15 @@ public function dataPushToDeviceSending() { * @param bool $isDebug */ public function testPushToDeviceSending($isDebug) { - $push = $this->getPush(['getDevicesForUser', 'encryptAndSign', 'deletePushToken']); + $push = $this->getPush(['getDevicesForUser', 'encryptAndSign', 'deletePushToken', 'validateToken']); - /** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */ + /** @var INotification|MockObject $notification */ $notification = $this->createMock(INotification::class); $notification->expects($this->exactly(3)) ->method('getUser') ->willReturn('valid'); - /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ + /** @var IUser|MockObject $user */ $user = $this->createMock(IUser::class); $this->userManager->expects($this->once()) @@ -399,7 +421,7 @@ public function testPushToDeviceSending($isDebug) { ->with($notification, 'ru') ->willReturnArgument(0); - /** @var Key|\PHPUnit_Framework_MockObject_MockObject $key */ + /** @var Key|MockObject $key */ $key = $this->createMock(Key::class); $this->keyManager->expects($this->once()) @@ -407,6 +429,10 @@ public function testPushToDeviceSending($isDebug) { ->with($user) ->willReturn($key); + $push->expects($this->exactly(5)) + ->method('validateToken') + ->willReturn(true); + $push->expects($this->exactly(5)) ->method('encryptAndSign') ->willReturn(['Payload']); @@ -414,7 +440,7 @@ public function testPushToDeviceSending($isDebug) { $push->expects($this->never()) ->method('deletePushToken'); - /** @var IClient|\PHPUnit_Framework_MockObject_MockObject $client */ + /** @var IClient|MockObject $client */ $client = $this->createMock(IClient::class); $this->clientService->expects($this->once()) @@ -438,7 +464,7 @@ public function testPushToDeviceSending($isDebug) { 'level' => ILogger::WARN, ]); - /** @var IResponse|\PHPUnit_Framework_MockObject_MockObject $response1 */ + /** @var IResponse|MockObject $response1 */ $response1 = $this->createMock(IResponse::class); $response1->expects($this->once()) ->method('getStatusCode') @@ -463,7 +489,7 @@ public function testPushToDeviceSending($isDebug) { 'app' => 'notifications', ]); - /** @var IResponse|\PHPUnit_Framework_MockObject_MockObject $response1 */ + /** @var IResponse|MockObject $response1 */ $response2 = $this->createMock(IResponse::class); $response2->expects($this->once()) ->method('getStatusCode') @@ -488,7 +514,7 @@ public function testPushToDeviceSending($isDebug) { 'app' => 'notifications', ]); - /** @var IResponse|\PHPUnit_Framework_MockObject_MockObject $response1 */ + /** @var IResponse|MockObject $response1 */ $response3 = $this->createMock(IResponse::class); $response3->expects($this->once()) ->method('getStatusCode') @@ -525,9 +551,9 @@ public function dataPushToDeviceTalkNotification() { * @param int $pushedDevice */ public function testPushToDeviceTalkNotification(array $deviceTypes, $isTalkNotification, $pushedDevice) { - $push = $this->getPush(['getDevicesForUser', 'encryptAndSign', 'deletePushToken']); + $push = $this->getPush(['getDevicesForUser', 'encryptAndSign', 'deletePushToken', 'validateToken']); - /** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */ + /** @var INotification|MockObject $notification */ $notification = $this->createMock(INotification::class); $notification->expects($this->exactly(3)) ->method('getUser') @@ -543,7 +569,7 @@ public function testPushToDeviceTalkNotification(array $deviceTypes, $isTalkNoti ->willReturn('notifications'); } - /** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */ + /** @var IUser|MockObject $user */ $user = $this->createMock(IUser::class); $this->userManager->expects($this->once()) @@ -577,7 +603,7 @@ public function testPushToDeviceTalkNotification(array $deviceTypes, $isTalkNoti ->with($notification, 'ru') ->willReturnArgument(0); - /** @var Key|\PHPUnit_Framework_MockObject_MockObject $key */ + /** @var Key|MockObject $key */ $key = $this->createMock(Key::class); $this->keyManager->expects($this->once()) @@ -586,25 +612,33 @@ public function testPushToDeviceTalkNotification(array $deviceTypes, $isTalkNoti ->willReturn($key); if ($pushedDevice === null) { + $push->expects($this->never()) + ->method('validateToken'); + $push->expects($this->never()) ->method('encryptAndSign'); $this->clientService->expects($this->never()) ->method('newClient'); } else { + + $push->expects($this->exactly(1)) + ->method('validateToken') + ->willReturn(true); + $push->expects($this->exactly(1)) ->method('encryptAndSign') ->with($this->anything(), $devices[$pushedDevice], $this->anything(), $this->anything(), $isTalkNotification) ->willReturn(['Payload']); - /** @var IClient|\PHPUnit_Framework_MockObject_MockObject $client */ + /** @var IClient|MockObject $client */ $client = $this->createMock(IClient::class); $this->clientService->expects($this->once()) ->method('newClient') ->willReturn($client); - /** @var IResponse|\PHPUnit_Framework_MockObject_MockObject $response1 */ + /** @var IResponse|MockObject $response1 */ $response = $this->createMock(IResponse::class); $response->expects($this->once()) ->method('getStatusCode')