Skip to content

Commit 8b2a762

Browse files
rullzerrullzer
authored
Merge pull request #21427 from nextcloud/backport/21416/stable19
[stable19] Clean up auth tokens when user is deleted
2 parents a436df0 + 1022db8 commit 8b2a762

File tree

5 files changed

+193
-0
lines changed

5 files changed

+193
-0
lines changed

‎core/Application.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
use OC\Authentication\Listeners\RemoteWipeEmailListener;
3737
use OC\Authentication\Listeners\RemoteWipeNotificationsListener;
3838
use OC\Authentication\Listeners\UserDeletedStoreCleanupListener;
39+
use OC\Authentication\Listeners\UserDeletedTokenCleanupListener;
3940
use OC\Authentication\Notifications\Notifier as AuthenticationNotifier;
4041
use OC\Core\Notification\RemoveLinkSharesNotifier;
4142
use OC\DB\MissingColumnInformation;
@@ -197,5 +198,6 @@ function (GenericEvent $event) use ($container) {
197198
$eventDispatcher->addServiceListener(RemoteWipeFinished::class, RemoteWipeNotificationsListener::class);
198199
$eventDispatcher->addServiceListener(RemoteWipeFinished::class, RemoteWipeEmailListener::class);
199200
$eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedStoreCleanupListener::class);
201+
$eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedTokenCleanupListener::class);
200202
}
201203
}

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@
612612
'OC\\Authentication\\Listeners\\RemoteWipeEmailListener' => $baseDir . '/lib/private/Authentication/Listeners/RemoteWipeEmailListener.php',
613613
'OC\\Authentication\\Listeners\\RemoteWipeNotificationsListener' => $baseDir . '/lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php',
614614
'OC\\Authentication\\Listeners\\UserDeletedStoreCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedStoreCleanupListener.php',
615+
'OC\\Authentication\\Listeners\\UserDeletedTokenCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php',
615616
'OC\\Authentication\\LoginCredentials\\Credentials' => $baseDir . '/lib/private/Authentication/LoginCredentials/Credentials.php',
616617
'OC\\Authentication\\LoginCredentials\\Store' => $baseDir . '/lib/private/Authentication/LoginCredentials/Store.php',
617618
'OC\\Authentication\\Login\\ALoginCommand' => $baseDir . '/lib/private/Authentication/Login/ALoginCommand.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
641641
'OC\\Authentication\\Listeners\\RemoteWipeEmailListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/RemoteWipeEmailListener.php',
642642
'OC\\Authentication\\Listeners\\RemoteWipeNotificationsListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php',
643643
'OC\\Authentication\\Listeners\\UserDeletedStoreCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedStoreCleanupListener.php',
644+
'OC\\Authentication\\Listeners\\UserDeletedTokenCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php',
644645
'OC\\Authentication\\LoginCredentials\\Credentials' => __DIR__ . '/../../..' . '/lib/private/Authentication/LoginCredentials/Credentials.php',
645646
'OC\\Authentication\\LoginCredentials\\Store' => __DIR__ . '/../../..' . '/lib/private/Authentication/LoginCredentials/Store.php',
646647
'OC\\Authentication\\Login\\ALoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/ALoginCommand.php',

‎lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright 2020 Christoph Wurst <christoph@winzerhof-wurst.at>
7+
*
8+
* @author 2020 Christoph Wurst <christoph@winzerhof-wurst.at>
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+
namespace OC\Authentication\Listeners;
27+
28+
use OC\Authentication\Token\Manager;
29+
use OCP\EventDispatcher\Event;
30+
use OCP\EventDispatcher\IEventListener;
31+
use OCP\ILogger;
32+
use OCP\User\Events\UserDeletedEvent;
33+
use Throwable;
34+
35+
class UserDeletedTokenCleanupListener implements IEventListener {
36+
37+
/** @var Manager */
38+
private $manager;
39+
40+
/** @var ILogger */
41+
private $logger;
42+
43+
public function __construct(Manager $manager,
44+
ILogger $logger) {
45+
$this->manager = $manager;
46+
$this->logger = $logger;
47+
}
48+
49+
public function handle(Event $event): void {
50+
if (!($event instanceof UserDeletedEvent)) {
51+
// Unrelated
52+
return;
53+
}
54+
55+
/**
56+
* Catch any exception during this process as any failure here shouldn't block the
57+
* user deletion.
58+
*/
59+
try {
60+
$uid = $event->getUser()->getUID();
61+
$tokens = $this->manager->getTokenByUser($uid);
62+
foreach ($tokens as $token) {
63+
$this->manager->invalidateTokenById($uid, $token->getId());
64+
}
65+
} catch (Throwable $e) {
66+
$this->logger->logException($e, [
67+
'message' => 'Could not clean up auth tokens after user deletion: ' . $e->getMessage(),
68+
'error' => ILogger::ERROR,
69+
]);
70+
}
71+
}
72+
}

‎tests/lib/Authentication/Listeners/UserDeletedTokenCleanupListenerTest.php‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright 2020 Christoph Wurst <christoph@winzerhof-wurst.at>
7+
*
8+
* @author 2020 Christoph Wurst <christoph@winzerhof-wurst.at>
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+
namespace Test\Authentication\Listeners;
27+
28+
use Exception;
29+
use OC\Authentication\Listeners\UserDeletedTokenCleanupListener;
30+
use OC\Authentication\Token\IToken;
31+
use OC\Authentication\Token\Manager;
32+
use OCP\EventDispatcher\Event;
33+
use OCP\ILogger;
34+
use OCP\IUser;
35+
use OCP\User\Events\UserDeletedEvent;
36+
use PHPUnit\Framework\MockObject\MockObject;
37+
use Test\TestCase;
38+
39+
class UserDeletedTokenCleanupListenerTest extends TestCase {
40+
41+
42+
/** @var Manager|MockObject */
43+
private $manager;
44+
45+
/** @var ILogger|MockObject */
46+
private $logger;
47+
48+
/** @var UserDeletedTokenCleanupListener */
49+
private $listener;
50+
51+
protected function setUp(): void {
52+
parent::setUp();
53+
54+
$this->manager = $this->createMock(Manager::class);
55+
$this->logger = $this->createMock(ILogger::class);
56+
57+
$this->listener = new UserDeletedTokenCleanupListener(
58+
$this->manager,
59+
$this->logger
60+
);
61+
}
62+
63+
public function testHandleUnrelated(): void {
64+
$event = new Event();
65+
$this->manager->expects($this->never())->method('getTokenByUser');
66+
$this->logger->expects($this->never())->method('logException');
67+
68+
$this->listener->handle($event);
69+
}
70+
71+
public function testHandleWithErrors(): void {
72+
$user = $this->createMock(IUser::class);
73+
$user->method('getUID')->willReturn('user123');
74+
$event = new UserDeletedEvent($user);
75+
$exception = new Exception('nope');
76+
$this->manager->expects($this->once())
77+
->method('getTokenByUser')
78+
->with('user123')
79+
->willThrowException($exception);
80+
$this->logger->expects($this->once())
81+
->method('logException')
82+
->with($exception, $this->anything());
83+
84+
$this->listener->handle($event);
85+
}
86+
87+
public function testHandle(): void {
88+
$user = $this->createMock(IUser::class);
89+
$user->method('getUID')->willReturn('user123');
90+
$event = new UserDeletedEvent($user);
91+
$token1 = $this->createMock(IToken::class);
92+
$token1->method('getId')->willReturn(1);
93+
$token2 = $this->createMock(IToken::class);
94+
$token2->method('getId')->willReturn(2);
95+
$token3 = $this->createMock(IToken::class);
96+
$token3->method('getId')->willReturn(3);
97+
$this->manager->expects($this->once())
98+
->method('getTokenByUser')
99+
->with('user123')
100+
->willReturn([
101+
$token1,
102+
$token2,
103+
$token3,
104+
]);
105+
$this->manager->expects($this->exactly(3))
106+
->method('invalidateTokenById')
107+
->withConsecutive(
108+
['user123', 1],
109+
['user123', 2],
110+
['user123', 3]
111+
);
112+
$this->logger->expects($this->never())
113+
->method('logException');
114+
115+
$this->listener->handle($event);
116+
}
117+
}

0 commit comments

Comments
 (0)