Skip to content

Commit 1b878e7

Browse files
Merge pull request #1261 from nextcloud/debt/noid/cleanup
Add user deletion cleanup
2 parents ccb0de1 + 534f973 commit 1b878e7

File tree

6 files changed

+186
-3
lines changed

6 files changed

+186
-3
lines changed

appinfo/info.xml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<name>Two-Factor TOTP Provider</name>
66
<summary>TOTP two-factor provider</summary>
77
<description>A Two-Factor-Auth Provider for TOTP (RFC 6238)</description>
8-
<version>6.4.0-alpha.1</version>
8+
<version>6.4.0-alpha.2</version>
99
<licence>agpl</licence>
1010
<author>Christoph Wurst</author>
1111
<namespace>TwoFactorTOTP</namespace>
@@ -23,6 +23,9 @@
2323
<two-factor-providers>
2424
<provider>OCA\TwoFactorTOTP\Provider\TotpProvider</provider>
2525
</two-factor-providers>
26+
<commands>
27+
<command>OCA\TwoFactorTOTP\Command\CleanUp</command>
28+
</commands>
2629
<activity>
2730
<settings>
2831
<setting>OCA\TwoFactorTOTP\Activity\Setting</setting>

lib/AppInfo/Application.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@
2727
use OCA\TwoFactorTOTP\Event\StateChanged;
2828
use OCA\TwoFactorTOTP\Listener\StateChangeActivity;
2929
use OCA\TwoFactorTOTP\Listener\StateChangeRegistryUpdater;
30+
use OCA\TwoFactorTOTP\Listener\UserDeleted;
3031
use OCA\TwoFactorTOTP\Service\ITotp;
3132
use OCA\TwoFactorTOTP\Service\Totp;
3233
use OCP\AppFramework\App;
3334
use OCP\AppFramework\Bootstrap\IBootContext;
3435
use OCP\AppFramework\Bootstrap\IBootstrap;
3536
use OCP\AppFramework\Bootstrap\IRegistrationContext;
37+
use OCP\User\Events\UserDeletedEvent;
3638

3739
class Application extends App implements IBootstrap {
3840
public const APP_ID = 'twofactor_totp';
@@ -49,6 +51,7 @@ public function register(IRegistrationContext $context): void {
4951
$context->registerEventListener(StateChanged::class, StateChangeActivity::class);
5052
$context->registerEventListener(StateChanged::class, StateChangeRegistryUpdater::class);
5153
$context->registerEventListener(DisabledByAdmin::class, StateChangeActivity::class);
54+
$context->registerEventListener(UserDeletedEvent::class, UserDeleted::class);
5255
}
5356

5457
public function boot(IBootContext $context): void {

lib/Command/CleanUp.php

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright Copyright (c) 2022 Daniel Kesselberg <mail@danielkesselberg.de>
7+
*
8+
* @author Daniel Kesselberg <mail@danielkesselberg.de>
9+
*
10+
* @license AGPL-3.0-or-later
11+
*
12+
* This code is free software: you can redistribute it and/or modify
13+
* it under the terms of the GNU Affero General Public License, version 3,
14+
* as published by the Free Software Foundation.
15+
*
16+
* This program is distributed in the hope that it will be useful,
17+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
18+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
19+
* GNU Affero General Public License for more details.
20+
*
21+
* You should have received a copy of the GNU Affero General Public License, version 3,
22+
* along with this program. If not, see <http://www.gnu.org/licenses/>
23+
*
24+
*/
25+
26+
namespace OCA\TwoFactorTOTP\Command;
27+
28+
use OCA\TwoFactorTOTP\Db\TotpSecretMapper;
29+
use OCP\DB\Exception;
30+
use OCP\IDBConnection;
31+
use OCP\IUserManager;
32+
use Symfony\Component\Console\Command\Command;
33+
use Symfony\Component\Console\Input\InputInterface;
34+
use Symfony\Component\Console\Output\OutputInterface;
35+
use Symfony\Component\Console\Style\SymfonyStyle;
36+
37+
class CleanUp extends Command {
38+
/** @var IDBConnection */
39+
private $db;
40+
41+
/** @var IUserManager */
42+
private $userManager;
43+
44+
/** @var TotpSecretMapper */
45+
private $totpSecretMapper;
46+
47+
public function __construct(
48+
IDBConnection $db,
49+
IUserManager $userManager,
50+
TotpSecretMapper $totpSecretMapper
51+
) {
52+
parent::__construct();
53+
54+
$this->db = $db;
55+
$this->userManager = $userManager;
56+
$this->totpSecretMapper = $totpSecretMapper;
57+
}
58+
59+
protected function configure(): void {
60+
$this
61+
->setName('twofactor_totp:cleanup')
62+
->setDescription('Remove orphaned totp secrets');
63+
}
64+
65+
protected function execute(InputInterface $input, OutputInterface $output): int {
66+
$io = new SymfonyStyle($input, $output);
67+
$io->title('Remove totp secrets for deleted users');
68+
69+
foreach ($this->findUserIds() as $userId) {
70+
if ($this->userManager->userExists($userId) === false) {
71+
try {
72+
$io->text('Delete secret for uid "' . $userId . '"');
73+
$this->totpSecretMapper->deleteSecretByUserId($userId);
74+
} catch (Exception $e) {
75+
$io->caution('Error deleting secret: ' . $e->getMessage());
76+
}
77+
}
78+
}
79+
80+
$io->success('Orphaned totp secrets removed.');
81+
82+
$io->text('Thank you for using Two-Factor TOTP!');
83+
return 0;
84+
}
85+
86+
/**
87+
* @throws Exception
88+
*/
89+
private function findUserIds(): array {
90+
$userIds = [];
91+
92+
$qb = $this->db->getQueryBuilder()
93+
->selectDistinct('user_id')
94+
->from($this->totpSecretMapper->getTableName());
95+
96+
$result = $qb->executeQuery();
97+
98+
while ($row = $result->fetch()) {
99+
$userIds[] = $row['user_id'];
100+
}
101+
102+
$result->closeCursor();
103+
104+
return $userIds;
105+
}
106+
}

lib/Db/TotpSecretMapper.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use Doctrine\DBAL\Statement;
2727
use OCP\AppFramework\Db\DoesNotExistException;
2828
use OCP\AppFramework\Db\QBMapper;
29+
use OCP\DB\Exception;
2930
use OCP\DB\QueryBuilder\IQueryBuilder;
3031
use OCP\IDBConnection;
3132
use OCP\IUser;
@@ -61,4 +62,16 @@ public function getSecret(IUser $user): TotpSecret {
6162
}
6263
return TotpSecret::fromRow($row);
6364
}
65+
66+
/**
67+
* @param string $uid
68+
* @throws Exception
69+
*/
70+
public function deleteSecretByUserId(string $uid): void {
71+
$qb = $this->db->getQueryBuilder();
72+
73+
$qb->delete($this->getTableName())
74+
->where($qb->expr()->eq('user_id', $qb->createNamedParameter($uid)));
75+
$qb->executeStatement();
76+
}
6477
}

lib/Listener/UserDeleted.php

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright Copyright (c) 2022 Daniel Kesselberg <mail@danielkesselberg.de>
7+
*
8+
* @author Daniel Kesselberg <mail@danielkesselberg.de>
9+
*
10+
* @license GNU AGPL version 3 or any later version
11+
*
12+
* This program is free software: you can redistribute it and/or modify
13+
* it under the terms of the GNU Affero General Public License as
14+
* published by the Free Software Foundation, either version 3 of the
15+
* License, or (at your option) any later version.
16+
*
17+
* This program is distributed in the hope that it will be useful,
18+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
19+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20+
* GNU Affero General Public License for more details.
21+
*
22+
* You should have received a copy of the GNU Affero General Public License
23+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
24+
*
25+
*/
26+
27+
namespace OCA\TwoFactorTOTP\Listener;
28+
29+
use OCA\TwoFactorTOTP\Db\TotpSecretMapper;
30+
use OCP\DB\Exception;
31+
use OCP\EventDispatcher\Event;
32+
use OCP\EventDispatcher\IEventListener;
33+
use OCP\User\Events\UserDeletedEvent;
34+
use Psr\Log\LoggerInterface;
35+
36+
class UserDeleted implements IEventListener {
37+
38+
/** @var TotpSecretMapper */
39+
private $totpSecretMapper;
40+
41+
/** @var LoggerInterface */
42+
private $logger;
43+
44+
public function __construct(TotpSecretMapper $totpSecretMapper, LoggerInterface $logger) {
45+
$this->totpSecretMapper = $totpSecretMapper;
46+
$this->logger = $logger;
47+
}
48+
49+
public function handle(Event $event): void {
50+
if ($event instanceof UserDeletedEvent) {
51+
try {
52+
$this->totpSecretMapper->deleteSecretByUserId($event->getUser()->getUID());
53+
} catch (Exception $e) {
54+
$this->logger->warning($e->getMessage(), ['uid' => $event->getUser()->getUID()]);
55+
}
56+
}
57+
}
58+
}

tests/Acceptance/TOTPAcceptanceTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public function testEnableTOTP(): void {
7070
// Log in
7171
$this->webDriver->findElement(WebDriverBy::id('user'))->sendKeys($this->user->getUID());
7272
$this->webDriver->findElement(WebDriverBy::id('password'))->sendKeys('password');
73-
$this->webDriver->findElement(WebDriverBy::cssSelector('form[name=login] input[type=submit]'))->click();
73+
$this->webDriver->findElement(WebDriverBy::cssSelector('form[name=login] [type=submit]'))->click();
7474

7575
// Go to personal settings and TOTP settings
7676
$this->webDriver->get('http://localhost:8080/index.php/settings/user/security');
@@ -156,7 +156,7 @@ public function testLoginShouldFailWithWrongOTP(): void {
156156
// Log in
157157
$this->webDriver->findElement(WebDriverBy::id('user'))->sendKeys($this->user->getUID());
158158
$this->webDriver->findElement(WebDriverBy::id('password'))->sendKeys('password');
159-
$this->webDriver->findElement(WebDriverBy::cssSelector('form[name=login] input[type=submit]'))->click();
159+
$this->webDriver->findElement(WebDriverBy::cssSelector('form[name=login] [type=submit]'))->click();
160160

161161
$this->webDriver->wait(20, 200)->until(function (WebDriver $driver) {
162162
try {

0 commit comments

Comments
 (0)