From 1c29a01956f13348281d29f276c8c37c85ed93e3 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 5 Mar 2019 20:14:16 +0100 Subject: [PATCH 1/2] Clear backupcodes reminder if no 2FA is enabled Fixes #14125 Listen to 2FA disable event. If a provider is disabled for a user. We check if there are no more providers. If there are no more providers we Remove the backupcode reminder notification (if still present). Signed-off-by: Roeland Jago Douma --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/AppInfo/Application.php | 7 ++ .../lib/Listener/ProviderDisabled.php | 65 +++++++++++ .../Unit/Listener/ProviderDisabledTest.php | 110 ++++++++++++++++++ 5 files changed, 184 insertions(+) create mode 100644 apps/twofactor_backupcodes/lib/Listener/ProviderDisabled.php create mode 100644 apps/twofactor_backupcodes/tests/Unit/Listener/ProviderDisabledTest.php diff --git a/apps/twofactor_backupcodes/composer/composer/autoload_classmap.php b/apps/twofactor_backupcodes/composer/composer/autoload_classmap.php index c093a1112a..2038421a3a 100644 --- a/apps/twofactor_backupcodes/composer/composer/autoload_classmap.php +++ b/apps/twofactor_backupcodes/composer/composer/autoload_classmap.php @@ -17,6 +17,7 @@ return array( 'OCA\\TwoFactorBackupCodes\\Listener\\ActivityPublisher' => $baseDir . '/../lib/Listener/ActivityPublisher.php', 'OCA\\TwoFactorBackupCodes\\Listener\\ClearNotifications' => $baseDir . '/../lib/Listener/ClearNotifications.php', 'OCA\\TwoFactorBackupCodes\\Listener\\IListener' => $baseDir . '/../lib/Listener/IListener.php', + 'OCA\\TwoFactorBackupCodes\\Listener\\ProviderDisabled' => $baseDir . '/../lib/Listener/ProviderDisabled.php', 'OCA\\TwoFactorBackupCodes\\Listener\\ProviderEnabled' => $baseDir . '/../lib/Listener/ProviderEnabled.php', 'OCA\\TwoFactorBackupCodes\\Listener\\RegistryUpdater' => $baseDir . '/../lib/Listener/RegistryUpdater.php', 'OCA\\TwoFactorBackupCodes\\Migration\\CheckBackupCodes' => $baseDir . '/../lib/Migration/CheckBackupCodes.php', diff --git a/apps/twofactor_backupcodes/composer/composer/autoload_static.php b/apps/twofactor_backupcodes/composer/composer/autoload_static.php index baad28d711..d1f124a407 100644 --- a/apps/twofactor_backupcodes/composer/composer/autoload_static.php +++ b/apps/twofactor_backupcodes/composer/composer/autoload_static.php @@ -32,6 +32,7 @@ class ComposerStaticInitTwoFactorBackupCodes 'OCA\\TwoFactorBackupCodes\\Listener\\ActivityPublisher' => __DIR__ . '/..' . '/../lib/Listener/ActivityPublisher.php', 'OCA\\TwoFactorBackupCodes\\Listener\\ClearNotifications' => __DIR__ . '/..' . '/../lib/Listener/ClearNotifications.php', 'OCA\\TwoFactorBackupCodes\\Listener\\IListener' => __DIR__ . '/..' . '/../lib/Listener/IListener.php', + 'OCA\\TwoFactorBackupCodes\\Listener\\ProviderDisabled' => __DIR__ . '/..' . '/../lib/Listener/ProviderDisabled.php', 'OCA\\TwoFactorBackupCodes\\Listener\\ProviderEnabled' => __DIR__ . '/..' . '/../lib/Listener/ProviderEnabled.php', 'OCA\\TwoFactorBackupCodes\\Listener\\RegistryUpdater' => __DIR__ . '/..' . '/../lib/Listener/RegistryUpdater.php', 'OCA\\TwoFactorBackupCodes\\Migration\\CheckBackupCodes' => __DIR__ . '/..' . '/../lib/Migration/CheckBackupCodes.php', diff --git a/apps/twofactor_backupcodes/lib/AppInfo/Application.php b/apps/twofactor_backupcodes/lib/AppInfo/Application.php index f5d0139dbd..fc6c94d5b7 100644 --- a/apps/twofactor_backupcodes/lib/AppInfo/Application.php +++ b/apps/twofactor_backupcodes/lib/AppInfo/Application.php @@ -30,6 +30,7 @@ use OCA\TwoFactorBackupCodes\Event\CodesGenerated; use OCA\TwoFactorBackupCodes\Listener\ActivityPublisher; use OCA\TwoFactorBackupCodes\Listener\ClearNotifications; use OCA\TwoFactorBackupCodes\Listener\IListener; +use OCA\TwoFactorBackupCodes\Listener\ProviderDisabled; use OCA\TwoFactorBackupCodes\Listener\ProviderEnabled; use OCA\TwoFactorBackupCodes\Listener\RegistryUpdater; use OCA\TwoFactorBackupCodes\Notifications\Notifier; @@ -81,6 +82,12 @@ class Application extends App { $listener = $container->query(ProviderEnabled::class); $listener->handle($event); }); + + $eventDispatcher->addListener(IRegistry::EVENT_PROVIDER_DISABLED, function(RegistryEvent $event) use ($container) { + /** @var IListener $listener */ + $listener = $container->query(ProviderDisabled::class); + $listener->handle($event); + }); } public function registerNotification() { diff --git a/apps/twofactor_backupcodes/lib/Listener/ProviderDisabled.php b/apps/twofactor_backupcodes/lib/Listener/ProviderDisabled.php new file mode 100644 index 0000000000..835eb0394f --- /dev/null +++ b/apps/twofactor_backupcodes/lib/Listener/ProviderDisabled.php @@ -0,0 +1,65 @@ + + * + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + + +namespace OCA\TwoFactorBackupCodes\Listener; + +use OCA\TwoFactorBackupCodes\BackgroundJob\RememberBackupCodesJob; +use OCP\Authentication\TwoFactorAuth\IRegistry; +use OCP\Authentication\TwoFactorAuth\RegistryEvent; +use OCP\BackgroundJob\IJobList; +use Symfony\Component\EventDispatcher\Event; + +class ProviderDisabled implements IListener { + + /** @var IRegistry */ + private $registry; + + /** @var IJobList */ + private $jobList; + + public function __construct(IRegistry $registry, + IJobList $jobList) { + $this->registry = $registry; + $this->jobList = $jobList; + } + + public function handle(Event $event) { + if (!($event instanceof RegistryEvent)) { + return; + } + + $providers = $this->registry->getProviderStates($event->getUser()); + + // Loop over all providers. If all are disabled we remove the job + $state = array_reduce($providers, function (bool $carry, bool $enabled) { + return $carry || $enabled; + }, false); + + if ($state === false) { + $this->jobList->remove(RememberBackupCodesJob::class, ['uid' => $event->getUser()->getUID()]); + } + } + +} diff --git a/apps/twofactor_backupcodes/tests/Unit/Listener/ProviderDisabledTest.php b/apps/twofactor_backupcodes/tests/Unit/Listener/ProviderDisabledTest.php new file mode 100644 index 0000000000..1bd5a7ccab --- /dev/null +++ b/apps/twofactor_backupcodes/tests/Unit/Listener/ProviderDisabledTest.php @@ -0,0 +1,110 @@ + + * + * @author Roeland Jago Douma + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + + +namespace OCA\TwoFactorBackupCodes\Tests\Unit\Listener; + +use OCA\TwoFactorBackupCodes\BackgroundJob\RememberBackupCodesJob; +use OCA\TwoFactorBackupCodes\Listener\ProviderDisabled; +use OCP\Authentication\TwoFactorAuth\IRegistry; +use OCP\Authentication\TwoFactorAuth\RegistryEvent; +use OCP\BackgroundJob\IJobList; +use OCP\IUser; +use Symfony\Component\EventDispatcher\Event; +use Test\TestCase; + +class ProviderDisabledTest extends TestCase { + + /** @var IRegistry|\PHPUnit\Framework\MockObject\MockObject */ + private $registy; + + /** @var IJobList|\PHPUnit\Framework\MockObject\MockObject */ + private $jobList; + + /** @var ProviderDisabled */ + private $listener; + + protected function setUp() { + parent::setUp(); + + $this->registy = $this->createMock(IRegistry::class); + $this->jobList = $this->createMock(IJobList::class); + + $this->listener = new ProviderDisabled($this->registy, $this->jobList); + } + + public function testHandleGenericEvent() { + $event = $this->createMock(Event::class); + $this->jobList->expects($this->never()) + ->method($this->anything()); + + $this->listener->handle($event); + } + + public function testHandleStillActiveProvider() { + $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn('myUID'); + $event = $this->createMock(RegistryEvent::class); + $event->method('getUser') + ->willReturn($user); + + $this->registy->method('getProviderStates') + ->with($user) + ->willReturn([ + 'backup_codes' => false, + 'foo' => true, + ]); + + $this->jobList->expects($this->never()) + ->method($this->anything()); + + $this->listener->handle($event); + } + + public function testHandleNoActiveProvider() { + $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn('myUID'); + $event = $this->createMock(RegistryEvent::class); + $event->method('getUser') + ->willReturn($user); + + $this->registy->method('getProviderStates') + ->with($user) + ->willReturn([ + 'backup_codes' => false, + 'foo' => false, + ]); + + $this->jobList->expects($this->once()) + ->method('remove') + ->with( + $this->equalTo(RememberBackupCodesJob::class), + $this->equalTo(['uid' => 'myUID']) + ); + + $this->listener->handle($event); + } +} From 3d17ab0936c74c1a60360e06bb99fb7ce3cdc161 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Tue, 5 Mar 2019 20:21:37 +0100 Subject: [PATCH 2/2] Do not send notification if no active 2fa If the job is still present we should also not fire it off if there is not a single active 2FA provider. Signed-off-by: Roeland Jago Douma --- .../BackgroundJob/RememberBackupCodesJob.php | 10 +++++- .../RememberBackupCodesJobTest.php | 31 ++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/apps/twofactor_backupcodes/lib/BackgroundJob/RememberBackupCodesJob.php b/apps/twofactor_backupcodes/lib/BackgroundJob/RememberBackupCodesJob.php index b04a8c3a29..1a7f5aca3e 100644 --- a/apps/twofactor_backupcodes/lib/BackgroundJob/RememberBackupCodesJob.php +++ b/apps/twofactor_backupcodes/lib/BackgroundJob/RememberBackupCodesJob.php @@ -70,7 +70,15 @@ class RememberBackupCodesJob extends TimedJob { } $providers = $this->registry->getProviderStates($user); - if (isset($providers['backup_codes']) && $providers['backup_codes'] === true) { + $state2fa = array_reduce($providers, function(bool $carry, bool $state) { + return $carry || $state; + }, false); + + /* + * If no provider is active or if the backup codes are already generate + * we can remove the job + */ + if ($state2fa === false || (isset($providers['backup_codes']) && $providers['backup_codes'] === true)) { // Backup codes already generated lets remove this job $this->jobList->remove(self::class, $argument); return; diff --git a/apps/twofactor_backupcodes/tests/Unit/BackgroundJob/RememberBackupCodesJobTest.php b/apps/twofactor_backupcodes/tests/Unit/BackgroundJob/RememberBackupCodesJobTest.php index 0e23e032bd..fe68da8ebf 100644 --- a/apps/twofactor_backupcodes/tests/Unit/BackgroundJob/RememberBackupCodesJobTest.php +++ b/apps/twofactor_backupcodes/tests/Unit/BackgroundJob/RememberBackupCodesJobTest.php @@ -114,6 +114,34 @@ class RememberBackupCodesJobTest extends TestCase { $this->invokePrivate($this->job, 'run', [['uid' => 'validUID']]); } + public function testNoActiveProvider() { + $user = $this->createMock(IUser::class); + $user->method('getUID') + ->willReturn('validUID'); + $this->userManager->method('get') + ->with('validUID') + ->willReturn($user); + + $this->registry->method('getProviderStates') + ->with($user) + ->willReturn([ + 'backup_codes' => false, + 'foo' => false, + ]); + + $this->jobList->expects($this->once()) + ->method('remove') + ->with( + RememberBackupCodesJob::class, + ['uid' => 'validUID'] + ); + + $this->notificationManager->expects($this->never()) + ->method($this->anything()); + + $this->invokePrivate($this->job, 'run', [['uid' => 'validUID']]); + } + public function testNotificationSend() { $user = $this->createMock(IUser::class); $user->method('getUID') @@ -125,7 +153,8 @@ class RememberBackupCodesJobTest extends TestCase { $this->registry->method('getProviderStates') ->with($user) ->willReturn([ - 'backup_codes' => false + 'backup_codes' => false, + 'foo' => true, ]); $this->jobList->expects($this->never())