From: Christoph Wurst Date: Mon, 15 Jun 2020 14:09:39 +0000 (+0200) Subject: Clean up auth tokens when user is deleted X-Git-Tag: v18.0.7RC1~40^2 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=8e3d47a215fb2e3f2915e90046b86a947d975284;p=nextcloud-server.git Clean up auth tokens when user is deleted Signed-off-by: Christoph Wurst --- diff --git a/core/Application.php b/core/Application.php index 879ac1cff74..b0a86b40a88 100644 --- a/core/Application.php +++ b/core/Application.php @@ -37,6 +37,7 @@ use OC\Authentication\Listeners\RemoteWipeActivityListener; use OC\Authentication\Listeners\RemoteWipeEmailListener; use OC\Authentication\Listeners\RemoteWipeNotificationsListener; use OC\Authentication\Listeners\UserDeletedStoreCleanupListener; +use OC\Authentication\Listeners\UserDeletedTokenCleanupListener; use OC\Authentication\Notifications\Notifier as AuthenticationNotifier; use OC\Core\Notification\RemoveLinkSharesNotifier; use OC\DB\MissingIndexInformation; @@ -175,6 +176,7 @@ class Application extends App { $eventDispatcher->addServiceListener(RemoteWipeFinished::class, RemoteWipeNotificationsListener::class); $eventDispatcher->addServiceListener(RemoteWipeFinished::class, RemoteWipeEmailListener::class); $eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedStoreCleanupListener::class); + $eventDispatcher->addServiceListener(UserDeletedEvent::class, UserDeletedTokenCleanupListener::class); } } diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 998fe176967..800cec8a1de 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -605,6 +605,7 @@ return array( 'OC\\Authentication\\Listeners\\RemoteWipeEmailListener' => $baseDir . '/lib/private/Authentication/Listeners/RemoteWipeEmailListener.php', 'OC\\Authentication\\Listeners\\RemoteWipeNotificationsListener' => $baseDir . '/lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php', 'OC\\Authentication\\Listeners\\UserDeletedStoreCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedStoreCleanupListener.php', + 'OC\\Authentication\\Listeners\\UserDeletedTokenCleanupListener' => $baseDir . '/lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php', 'OC\\Authentication\\LoginCredentials\\Credentials' => $baseDir . '/lib/private/Authentication/LoginCredentials/Credentials.php', 'OC\\Authentication\\LoginCredentials\\Store' => $baseDir . '/lib/private/Authentication/LoginCredentials/Store.php', 'OC\\Authentication\\Login\\ALoginCommand' => $baseDir . '/lib/private/Authentication/Login/ALoginCommand.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 7dd0e15e51d..749f43a2eb9 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -634,6 +634,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c 'OC\\Authentication\\Listeners\\RemoteWipeEmailListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/RemoteWipeEmailListener.php', 'OC\\Authentication\\Listeners\\RemoteWipeNotificationsListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/RemoteWipeNotificationsListener.php', 'OC\\Authentication\\Listeners\\UserDeletedStoreCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedStoreCleanupListener.php', + 'OC\\Authentication\\Listeners\\UserDeletedTokenCleanupListener' => __DIR__ . '/../../..' . '/lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php', 'OC\\Authentication\\LoginCredentials\\Credentials' => __DIR__ . '/../../..' . '/lib/private/Authentication/LoginCredentials/Credentials.php', 'OC\\Authentication\\LoginCredentials\\Store' => __DIR__ . '/../../..' . '/lib/private/Authentication/LoginCredentials/Store.php', 'OC\\Authentication\\Login\\ALoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/ALoginCommand.php', diff --git a/lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php b/lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php new file mode 100644 index 00000000000..d6238eb5ac8 --- /dev/null +++ b/lib/private/Authentication/Listeners/UserDeletedTokenCleanupListener.php @@ -0,0 +1,72 @@ + + * + * @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 OC\Authentication\Listeners; + +use OC\Authentication\Token\Manager; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\ILogger; +use OCP\User\Events\UserDeletedEvent; +use Throwable; + +class UserDeletedTokenCleanupListener implements IEventListener { + + /** @var Manager */ + private $manager; + + /** @var ILogger */ + private $logger; + + public function __construct(Manager $manager, + ILogger $logger) { + $this->manager = $manager; + $this->logger = $logger; + } + + public function handle(Event $event): void { + if (!($event instanceof UserDeletedEvent)) { + // Unrelated + return; + } + + /** + * Catch any exception during this process as any failure here shouldn't block the + * user deletion. + */ + try { + $uid = $event->getUser()->getUID(); + $tokens = $this->manager->getTokenByUser($uid); + foreach ($tokens as $token) { + $this->manager->invalidateTokenById($uid, $token->getId()); + } + } catch (Throwable $e) { + $this->logger->logException($e, [ + 'message' => 'Could not clean up auth tokens after user deletion: ' . $e->getMessage(), + 'error' => ILogger::ERROR, + ]); + } + } +} diff --git a/tests/lib/Authentication/Listeners/UserDeletedTokenCleanupListenerTest.php b/tests/lib/Authentication/Listeners/UserDeletedTokenCleanupListenerTest.php new file mode 100644 index 00000000000..672a044ce01 --- /dev/null +++ b/tests/lib/Authentication/Listeners/UserDeletedTokenCleanupListenerTest.php @@ -0,0 +1,117 @@ + + * + * @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 Test\Authentication\Listeners; + +use Exception; +use OC\Authentication\Listeners\UserDeletedTokenCleanupListener; +use OC\Authentication\Token\IToken; +use OC\Authentication\Token\Manager; +use OCP\EventDispatcher\Event; +use OCP\ILogger; +use OCP\IUser; +use OCP\User\Events\UserDeletedEvent; +use PHPUnit\Framework\MockObject\MockObject; +use Test\TestCase; + +class UserDeletedTokenCleanupListenerTest extends TestCase { + + + /** @var Manager|MockObject */ + private $manager; + + /** @var ILogger|MockObject */ + private $logger; + + /** @var UserDeletedTokenCleanupListener */ + private $listener; + + protected function setUp(): void { + parent::setUp(); + + $this->manager = $this->createMock(Manager::class); + $this->logger = $this->createMock(ILogger::class); + + $this->listener = new UserDeletedTokenCleanupListener( + $this->manager, + $this->logger + ); + } + + public function testHandleUnrelated(): void { + $event = new Event(); + $this->manager->expects($this->never())->method('getTokenByUser'); + $this->logger->expects($this->never())->method('logException'); + + $this->listener->handle($event); + } + + public function testHandleWithErrors(): void { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('user123'); + $event = new UserDeletedEvent($user); + $exception = new Exception('nope'); + $this->manager->expects($this->once()) + ->method('getTokenByUser') + ->with('user123') + ->willThrowException($exception); + $this->logger->expects($this->once()) + ->method('logException') + ->with($exception, $this->anything()); + + $this->listener->handle($event); + } + + public function testHandle(): void { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('user123'); + $event = new UserDeletedEvent($user); + $token1 = $this->createMock(IToken::class); + $token1->method('getId')->willReturn(1); + $token2 = $this->createMock(IToken::class); + $token2->method('getId')->willReturn(2); + $token3 = $this->createMock(IToken::class); + $token3->method('getId')->willReturn(3); + $this->manager->expects($this->once()) + ->method('getTokenByUser') + ->with('user123') + ->willReturn([ + $token1, + $token2, + $token3, + ]); + $this->manager->expects($this->exactly(3)) + ->method('invalidateTokenById') + ->withConsecutive( + ['user123', 1], + ['user123', 2], + ['user123', 3] + ); + $this->logger->expects($this->never()) + ->method('logException'); + + $this->listener->handle($event); + } +}