Emit an event for every disabled 2FA provider during cleanup

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 930a14df5d5c9aa105f615fa266f6ec08f52c944..85fb3e8903793691ec364cd0052c8105eba86b49 100644 (file)
--- 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 9539dbc1a4bc088a25c0098ab9fc74e0b853f16e..a0c6257c191936618832686bb44f059b3fe899eb 100644 (file)
--- 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 02e6863d1c41213026461693ff6c85abee50fa49..bd8ff0353eee4095dde4f2e8666c2ee4698c9469 100644 (file)
--- 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 97df2bd53119b571c70b4c731a126017fb27be46..2af8566d3e51c3bf3cc7c03c3fb42746271faffa 100644 (file)
--- 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 (file)
index 0000000..a0e111c
--- /dev/null
+++ b/lib/public/Authentication/TwoFactorAuth/TwoFactorProviderDisabled.php
@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright 2020 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @author 2020 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ */
+
+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 1a21ee047df3035480daa1478de21c7a6ef7514c..7975108c59bbf59d2097fcc966c3c27489640b51 100644 (file)
--- 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 49f4eaa7020a9a78a3888de895f324e5d25cb9b8..b0d0ef8efef2433cc1a2fc6ef1794f728452dd9b 100644 (file)
--- 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);
        }