From ed4017dfb4d605943988c6686b088b41d1680110 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 13 Dec 2016 15:15:14 +0100 Subject: [PATCH] fix minor issues Signed-off-by: Christoph Wurst --- .../lib/Activity/GenericFilter.php | 2 +- .../lib/Activity/Provider.php | 2 +- .../lib/Service/BackupCodeStorage.php | 20 ++++++++++++---- .../tests/Unit/Activity/GenericFilterTest.php | 3 +-- .../Unit/Service/BackupCodeStorageTest.php | 8 ++++++- .../Authentication/TwoFactorAuth/Manager.php | 23 +++++++++++++++---- lib/private/Server.php | 2 +- 7 files changed, 45 insertions(+), 15 deletions(-) diff --git a/apps/twofactor_backupcodes/lib/Activity/GenericFilter.php b/apps/twofactor_backupcodes/lib/Activity/GenericFilter.php index f2b0ef6f23..27c0274b60 100644 --- a/apps/twofactor_backupcodes/lib/Activity/GenericFilter.php +++ b/apps/twofactor_backupcodes/lib/Activity/GenericFilter.php @@ -40,7 +40,7 @@ class GenericFilter implements IFilter { } public function allowedApps() { - return null; + return []; } public function filterTypes(array $types) { diff --git a/apps/twofactor_backupcodes/lib/Activity/Provider.php b/apps/twofactor_backupcodes/lib/Activity/Provider.php index db139a0e67..cfb16c9f8d 100644 --- a/apps/twofactor_backupcodes/lib/Activity/Provider.php +++ b/apps/twofactor_backupcodes/lib/Activity/Provider.php @@ -55,7 +55,7 @@ class Provider implements IProvider { switch ($event->getSubject()) { case 'codes_generated': - $event->setParsedSubject($l->t('You created backup codes for your account')); + $event->setParsedSubject($l->t('You created two-factor backup codes for your account')); $event->setIcon($this->urlGenerator->getAbsoluteURL($this->urlGenerator->imagePath('core', 'actions/password.svg'))); break; default: diff --git a/apps/twofactor_backupcodes/lib/Service/BackupCodeStorage.php b/apps/twofactor_backupcodes/lib/Service/BackupCodeStorage.php index d7bcfe0147..d9f75fa5a3 100644 --- a/apps/twofactor_backupcodes/lib/Service/BackupCodeStorage.php +++ b/apps/twofactor_backupcodes/lib/Service/BackupCodeStorage.php @@ -22,9 +22,11 @@ namespace OCA\TwoFactorBackupCodes\Service; +use Exception; use OCA\TwoFactorBackupCodes\Db\BackupCode; use OCA\TwoFactorBackupCodes\Db\BackupCodeMapper; use OCP\Activity\IManager; +use OCP\ILogger; use OCP\IUser; use OCP\Security\IHasher; use OCP\Security\ISecureRandom; @@ -43,11 +45,16 @@ class BackupCodeStorage { /** @var IManager */ private $activityManager; - public function __construct(BackupCodeMapper $mapper, ISecureRandom $random, IHasher $hasher, IManager $activityManager) { + /** @var ILogger */ + private $logger; + + public function __construct(BackupCodeMapper $mapper, ISecureRandom $random, IHasher $hasher, + IManager $activityManager, ILogger $logger) { $this->mapper = $mapper; $this->hasher = $hasher; $this->random = $random; $this->activityManager = $activityManager; + $this->logger = $logger; } /** @@ -89,9 +96,14 @@ class BackupCodeStorage { $activity->setApp('twofactor_backupcodes') ->setType('twofactor') ->setAuthor($user->getUID()) - ->setAffectedUser($user->getUID()); - $activity->setSubject($event); - $this->activityManager->publish($activity); + ->setAffectedUser($user->getUID()) + ->setSubject($event); + try { + $this->activityManager->publish($activity); + } catch (Exception $e) { + $this->logger->warning('could not publish backup code creation activity', ['app' => 'twofactor_backupcodes']); + $this->logger->logException($e, ['app' => 'twofactor_backupcodes']); + } } /** diff --git a/apps/twofactor_backupcodes/tests/Unit/Activity/GenericFilterTest.php b/apps/twofactor_backupcodes/tests/Unit/Activity/GenericFilterTest.php index a367a18fe9..ca24c0be64 100644 --- a/apps/twofactor_backupcodes/tests/Unit/Activity/GenericFilterTest.php +++ b/apps/twofactor_backupcodes/tests/Unit/Activity/GenericFilterTest.php @@ -26,7 +26,6 @@ use OCA\TwoFactorBackupCodes\Activity\GenericFilter; use OCP\IL10N; use OCP\IURLGenerator; use Test\TestCase; -use function returnValue; class GenericFilterTest extends TestCase { @@ -46,7 +45,7 @@ class GenericFilterTest extends TestCase { } public function testAllowedApps() { - $this->assertEquals(0, $this->filter->allowedApps()); + $this->assertEquals([], $this->filter->allowedApps()); } public function testFilterTypes() { diff --git a/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php b/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php index fbaac0220a..54738f7460 100644 --- a/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php +++ b/apps/twofactor_backupcodes/tests/Unit/Service/BackupCodeStorageTest.php @@ -27,6 +27,7 @@ use OCA\TwoFactorBackupCodes\Db\BackupCodeMapper; use OCA\TwoFactorBackupCodes\Service\BackupCodeStorage; use OCP\Activity\IEvent; use OCP\Activity\IManager; +use OCP\ILogger; use OCP\IUser; use OCP\Security\IHasher; use OCP\Security\ISecureRandom; @@ -46,6 +47,9 @@ class BackupCodeStorageTest extends TestCase { /** @var IManager|\PHPUnit_Framework_MockObject_MockObject */ private $activityManager; + /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ + private $logger; + /** @var BackupCodeStorage */ private $storage; @@ -58,7 +62,9 @@ class BackupCodeStorageTest extends TestCase { $this->random = $this->getMockBuilder(ISecureRandom::class)->getMock(); $this->hasher = $this->getMockBuilder(IHasher::class)->getMock(); $this->activityManager = $this->createMock(IManager::class); - $this->storage = new BackupCodeStorage($this->mapper, $this->random, $this->hasher, $this->activityManager); + $this->logger = $this->createMock(ILogger::class); + + $this->storage = new BackupCodeStorage($this->mapper, $this->random, $this->hasher, $this->activityManager, $this->logger); } public function testCreateCodes() { diff --git a/lib/private/Authentication/TwoFactorAuth/Manager.php b/lib/private/Authentication/TwoFactorAuth/Manager.php index 297e69fac9..1d0deada69 100644 --- a/lib/private/Authentication/TwoFactorAuth/Manager.php +++ b/lib/private/Authentication/TwoFactorAuth/Manager.php @@ -31,6 +31,7 @@ use OCP\Activity\IManager; use OCP\AppFramework\QueryException; use OCP\Authentication\TwoFactorAuth\IProvider; use OCP\IConfig; +use OCP\ILogger; use OCP\ISession; use OCP\IUser; @@ -53,16 +54,23 @@ class Manager { /** @var IManager */ private $activityManager; + /** @var ILogger */ + private $logger; + /** * @param AppManager $appManager * @param ISession $session * @param IConfig $config + * @param IManager $activityManager + * @param ILogger $logger */ - public function __construct(AppManager $appManager, ISession $session, IConfig $config, IManager $activityManager) { + public function __construct(AppManager $appManager, ISession $session, IConfig $config, IManager $activityManager, + ILogger $logger) { $this->appManager = $appManager; $this->session = $session; $this->config = $config; $this->activityManager = $activityManager; + $this->logger = $logger; } /** @@ -211,11 +219,16 @@ class Manager { private function publishEvent(IUser $user, $event, array $params) { $activity = $this->activityManager->generateEvent(); $activity->setApp('twofactor_generic') - ->setType('twofactor_generic') + ->setType('twofactor') ->setAuthor($user->getUID()) - ->setAffectedUser($user->getUID()); - $activity->setSubject($event, $params); - $this->activityManager->publish($activity); + ->setAffectedUser($user->getUID()) + ->setSubject($event, $params); + try { + $this->activityManager->publish($activity); + } catch (Exception $e) { + $this->logger->warning('could not publish backup code creation activity', ['app' => 'twofactor_backupcodes']); + $this->logger->logException($e, ['app' => 'twofactor_backupcodes']); + } } /** diff --git a/lib/private/Server.php b/lib/private/Server.php index d51ae7d865..6f4d4f066e 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -312,7 +312,7 @@ class Server extends ServerContainer implements IServerContainer { }); $this->registerService(\OC\Authentication\TwoFactorAuth\Manager::class, function (Server $c) { - return new \OC\Authentication\TwoFactorAuth\Manager($c->getAppManager(), $c->getSession(), $c->getConfig(), $c->getActivityManager()); + return new \OC\Authentication\TwoFactorAuth\Manager($c->getAppManager(), $c->getSession(), $c->getConfig(), $c->getActivityManager(), $c->getLogger()); }); $this->registerService('NavigationManager', function ($c) {