From 68794ebc9292cdedaa6a52d190e41e58f6edb1ba Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 9 Jun 2020 14:33:06 +0200 Subject: [PATCH] Emit an event for every disabled 2FA provider during cleanup Signed-off-by: Christoph Wurst --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + .../Db/ProviderUserAssignmentDao.php | 35 ++++++++++--- .../Authentication/TwoFactorAuth/Registry.php | 9 ++-- .../TwoFactorProviderDisabled.php | 52 +++++++++++++++++++ .../Db/ProviderUserAssignmentDaoTest.php | 25 +++++++-- .../TwoFactorAuth/RegistryTest.php | 11 +++- 7 files changed, 118 insertions(+), 16 deletions(-) create mode 100644 lib/public/Authentication/TwoFactorAuth/TwoFactorProviderDisabled.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 930a14df5d5..85fb3e89037 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -95,6 +95,7 @@ return array( 'OCP\\Authentication\\TwoFactorAuth\\IRegistry' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/IRegistry.php', 'OCP\\Authentication\\TwoFactorAuth\\RegistryEvent' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/RegistryEvent.php', 'OCP\\Authentication\\TwoFactorAuth\\TwoFactorException' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/TwoFactorException.php', + 'OCP\\Authentication\\TwoFactorAuth\\TwoFactorProviderDisabled' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/TwoFactorProviderDisabled.php', 'OCP\\AutoloadNotAllowedException' => $baseDir . '/lib/public/AutoloadNotAllowedException.php', 'OCP\\BackgroundJob' => $baseDir . '/lib/public/BackgroundJob.php', 'OCP\\BackgroundJob\\IJob' => $baseDir . '/lib/public/BackgroundJob/IJob.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 9539dbc1a4b..a0c6257c191 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -124,6 +124,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OCP\\Authentication\\TwoFactorAuth\\IRegistry' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/IRegistry.php', 'OCP\\Authentication\\TwoFactorAuth\\RegistryEvent' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/RegistryEvent.php', 'OCP\\Authentication\\TwoFactorAuth\\TwoFactorException' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/TwoFactorException.php', + 'OCP\\Authentication\\TwoFactorAuth\\TwoFactorProviderDisabled' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/TwoFactorProviderDisabled.php', 'OCP\\AutoloadNotAllowedException' => __DIR__ . '/../../..' . '/lib/public/AutoloadNotAllowedException.php', 'OCP\\BackgroundJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob.php', 'OCP\\BackgroundJob\\IJob' => __DIR__ . '/../../..' . '/lib/public/BackgroundJob/IJob.php', diff --git a/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php b/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php index 02e6863d1c4..bd8ff0353ee 100644 --- a/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php +++ b/lib/private/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDao.php @@ -29,6 +29,7 @@ namespace OC\Authentication\TwoFactorAuth\Db; use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; +use function array_map; /** * Data access object to query and assign (provider_id, uid, enabled) tuples of @@ -91,13 +92,35 @@ class ProviderUserAssignmentDao { } } - public function deleteByUser(string $uid) { - $qb = $this->conn->getQueryBuilder(); - - $deleteQuery = $qb->delete(self::TABLE_NAME) - ->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid))); - + /** + * Delete all provider states of a user and return the provider IDs + * + * @param string $uid + * + * @return int[] + */ + public function deleteByUser(string $uid): array { + $qb1 = $this->conn->getQueryBuilder(); + $selectQuery = $qb1->select('*') + ->from(self::TABLE_NAME) + ->where($qb1->expr()->eq('uid', $qb1->createNamedParameter($uid))); + $selectResult = $selectQuery->execute(); + $rows = $selectResult->fetchAll(); + $selectResult->closeCursor(); + + $qb2 = $this->conn->getQueryBuilder(); + $deleteQuery = $qb2 + ->delete(self::TABLE_NAME) + ->where($qb2->expr()->eq('uid', $qb2->createNamedParameter($uid))); $deleteQuery->execute(); + + return array_map(function (array $row) { + return [ + 'provider_id' => $row['provider_id'], + 'uid' => $row['uid'], + 'enabled' => 1 === (int) $row['enabled'], + ]; + }, $rows); } public function deleteAll(string $providerId) { diff --git a/lib/private/Authentication/TwoFactorAuth/Registry.php b/lib/private/Authentication/TwoFactorAuth/Registry.php index 97df2bd5311..2af8566d3e5 100644 --- a/lib/private/Authentication/TwoFactorAuth/Registry.php +++ b/lib/private/Authentication/TwoFactorAuth/Registry.php @@ -31,6 +31,7 @@ use OC\Authentication\TwoFactorAuth\Db\ProviderUserAssignmentDao; use OCP\Authentication\TwoFactorAuth\IProvider; use OCP\Authentication\TwoFactorAuth\IRegistry; use OCP\Authentication\TwoFactorAuth\RegistryEvent; +use OCP\Authentication\TwoFactorAuth\TwoFactorProviderDisabled; use OCP\EventDispatcher\IEventDispatcher; use OCP\IUser; @@ -66,11 +67,11 @@ class Registry implements IRegistry { $this->dispatcher->dispatch(self::EVENT_PROVIDER_DISABLED, $event); } - /** - * @todo evaluate if we should emit RegistryEvents for each of the deleted rows -> needs documentation - */ public function deleteUserData(IUser $user): void { - $this->assignmentDao->deleteByUser($user->getUID()); + foreach ($this->assignmentDao->deleteByUser($user->getUID()) as $provider) { + $event = new TwoFactorProviderDisabled($provider['provider_id']); + $this->dispatcher->dispatchTyped($event); + } } public function cleanUp(string $providerId) { diff --git a/lib/public/Authentication/TwoFactorAuth/TwoFactorProviderDisabled.php b/lib/public/Authentication/TwoFactorAuth/TwoFactorProviderDisabled.php new file mode 100644 index 00000000000..a0e111c59b3 --- /dev/null +++ b/lib/public/Authentication/TwoFactorAuth/TwoFactorProviderDisabled.php @@ -0,0 +1,52 @@ + + * + * @author 2020 Christoph Wurst + * + * @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 OCP\Authentication\TwoFactorAuth; + +use OCP\EventDispatcher\Event; + +/** + * @since 20.0.0 + */ +final class TwoFactorProviderDisabled extends Event { + + /** @var string */ + private $providerId; + + /** + * @since 20.0.0 + */ + public function __construct(string $providerId) { + parent::__construct(); + $this->providerId = $providerId; + } + + /** + * @since 20.0.0 + */ + public function getProviderId(): string { + return $this->providerId; + } +} diff --git a/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php b/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php index 1a21ee047df..7975108c59b 100644 --- a/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php +++ b/tests/lib/Authentication/TwoFactorAuth/Db/ProviderUserAssignmentDaoTest.php @@ -136,14 +136,29 @@ class ProviderUserAssignmentDaoTest extends TestCase { $this->dao->persist('twofactor_fail', 'user1', 1); $this->dao->persist('twofactor_u2f', 'user1', 1); $this->dao->persist('twofactor_fail', 'user2', 0); - $this->dao->persist('twofactor_u2f', 'user1', 0); - - $this->dao->deleteByUser('user1'); - + $this->dao->persist('twofactor_u2f', 'user2', 0); + + $deleted = $this->dao->deleteByUser('user1'); + + $this->assertEquals( + [ + [ + 'uid' => 'user1', + 'provider_id' => 'twofactor_fail', + 'enabled' => true, + ], + [ + 'uid' => 'user1', + 'provider_id' => 'twofactor_u2f', + 'enabled' => true, + ], + ], + $deleted + ); $statesUser1 = $this->dao->getState('user1'); $statesUser2 = $this->dao->getState('user2'); $this->assertCount(0, $statesUser1); - $this->assertCount(1, $statesUser2); + $this->assertCount(2, $statesUser2); } public function testDeleteAll() { diff --git a/tests/lib/Authentication/TwoFactorAuth/RegistryTest.php b/tests/lib/Authentication/TwoFactorAuth/RegistryTest.php index 49f4eaa7020..b0d0ef8efef 100644 --- a/tests/lib/Authentication/TwoFactorAuth/RegistryTest.php +++ b/tests/lib/Authentication/TwoFactorAuth/RegistryTest.php @@ -31,6 +31,7 @@ use OC\Authentication\TwoFactorAuth\Registry; use OCP\Authentication\TwoFactorAuth\IProvider; use OCP\Authentication\TwoFactorAuth\IRegistry; use OCP\Authentication\TwoFactorAuth\RegistryEvent; +use OCP\Authentication\TwoFactorAuth\TwoFactorProviderDisabled; use OCP\EventDispatcher\IEventDispatcher; use OCP\IUser; use PHPUnit\Framework\MockObject\MockObject; @@ -115,7 +116,15 @@ class RegistryTest extends TestCase { $user->expects($this->once())->method('getUID')->willReturn('user123'); $this->dao->expects($this->once()) ->method('deleteByUser') - ->with('user123'); + ->with('user123') + ->willReturn([ + [ + 'provider_id' => 'twofactor_u2f', + ] + ]); + $this->dispatcher->expects($this->once()) + ->method('dispatchTyped') + ->with(new TwoFactorProviderDisabled('twofactor_u2f')); $this->registry->deleteUserData($user); } -- 2.39.5