Merge pull request #37231 from nextcloud/backport/36033/stable25

[stable25] invalidate existing tokens when deleting an oauth client

apps/oauth2/lib/Controller/SettingsController.php

@@ -30,6 +30,7 @@
  */
 namespace OCA\OAuth2\Controller;
 
+use OCP\Authentication\Token\IProvider as IAuthTokenProvider;
 use OCA\OAuth2\Db\AccessTokenMapper;
 use OCA\OAuth2\Db\Client;
 use OCA\OAuth2\Db\ClientMapper;
@@ -38,6 +39,8 @@
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IL10N;
 use OCP\IRequest;
+use OCP\IUser;
+use OCP\IUserManager;
 use OCP\Security\ISecureRandom;
 use OCP\Security\ICrypto;
 
@@ -50,9 +53,16 @@ class SettingsController extends Controller {
 	private $accessTokenMapper;
 	/** @var IL10N */
 	private $l;
+
 	/** @var ICrypto */
 	private $crypto;
 
+	/** @var IAuthTokenProvider */
+	private $tokenProvider;
+	/**
+	 * @var IUserManager
+	 */
+	private $userManager;
 	public const validChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
 
 	public function __construct(string $appName,
@@ -61,14 +71,18 @@ public function __construct(string $appName,
 								ISecureRandom $secureRandom,
 								AccessTokenMapper $accessTokenMapper,
 								IL10N $l,
-								ICrypto $crypto
+								ICrypto $crypto,
+								IAuthTokenProvider $tokenProvider,
+								IUserManager $userManager
 	) {
 		parent::__construct($appName, $request);
 		$this->secureRandom = $secureRandom;
 		$this->clientMapper = $clientMapper;
 		$this->accessTokenMapper = $accessTokenMapper;
 		$this->l = $l;
 		$this->crypto = $crypto;
+		$this->tokenProvider = $tokenProvider;
+		$this->userManager = $userManager;
 	}
 
 	public function addClient(string $name,
@@ -99,6 +113,11 @@ public function addClient(string $name,
 
 	public function deleteClient(int $id): JSONResponse {
 		$client = $this->clientMapper->getByUid($id);
+
+		$this->userManager->callForSeenUsers(function (IUser $user) use ($client) {
+			$this->tokenProvider->invalidateTokensOfUser($user->getUID(), $client->getName());
+		});
+
 		$this->accessTokenMapper->deleteByClientId($id);
 		$this->clientMapper->delete($client);
 		return new JSONResponse([]);

apps/oauth2/tests/Controller/SettingsControllerTest.php

@@ -26,6 +26,8 @@
  */
 namespace OCA\OAuth2\Tests\Controller;
 
+use OC\Authentication\Token\IToken;
+use OCP\Authentication\Token\IProvider as IAuthTokenProvider;
 use OCA\OAuth2\Controller\SettingsController;
 use OCA\OAuth2\Db\AccessTokenMapper;
 use OCA\OAuth2\Db\Client;
@@ -35,9 +37,14 @@
 use OCP\IL10N;
 use OCP\IRequest;
 use OCP\Security\ICrypto;
+use OCP\IUser;
+use OCP\IUserManager;
 use OCP\Security\ISecureRandom;
 use Test\TestCase;
 
+/**
+ * @group DB
+ */
 class SettingsControllerTest extends TestCase {
 	/** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */
 	private $request;
@@ -47,10 +54,16 @@ class SettingsControllerTest extends TestCase {
 	private $secureRandom;
 	/** @var AccessTokenMapper|\PHPUnit\Framework\MockObject\MockObject */
 	private $accessTokenMapper;
+	/** @var IAuthTokenProvider|\PHPUnit\Framework\MockObject\MockObject */
+	private $authTokenProvider;
+	/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
+	private $userManager;
 	/** @var SettingsController */
 	private $settingsController;
 	/** @var ICrypto|\PHPUnit\Framework\MockObject\MockObject */
 	private $crypto;
+	/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
+	private $l;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -59,8 +72,10 @@ protected function setUp(): void {
 		$this->clientMapper = $this->createMock(ClientMapper::class);
 		$this->secureRandom = $this->createMock(ISecureRandom::class);
 		$this->accessTokenMapper = $this->createMock(AccessTokenMapper::class);
-		$l = $this->createMock(IL10N::class);
-		$l->method('t')
+		$this->authTokenProvider = $this->createMock(IAuthTokenProvider::class);
+		$this->userManager = $this->createMock(IUserManager::class);
+		$this->l = $this->createMock(IL10N::class);
+		$this->l->method('t')
 			->willReturnArgument(0);
 		$this->crypto = $this->createMock(ICrypto::class);
 
@@ -70,9 +85,12 @@ protected function setUp(): void {
 			$this->clientMapper,
 			$this->secureRandom,
 			$this->accessTokenMapper,
-			$l,
-			$this->crypto
+			$this->l,
+			$this->crypto,
+			$this->authTokenProvider,
+			$this->userManager
 		);
+
 	}
 
 	public function testAddClient() {
@@ -123,6 +141,26 @@ public function testAddClient() {
 	}
 
 	public function testDeleteClient() {
+
+		$userManager = \OC::$server->getUserManager();
+		// count other users in the db before adding our own
+		$count = 0;
+		$function = function (IUser $user) use (&$count) {
+			if ($user->getLastLogin() > 0) {
+				$count++;
+			}
+		};
+		$userManager->callForAllUsers($function);
+		$user1 = $userManager->createUser('test101', 'test101');
+		$user1->updateLastLoginTimestamp();
+		$tokenProviderMock = $this->getMockBuilder(IAuthTokenProvider::class)->getMock();
+
+		// expect one call per user and ensure the correct client name
+		$tokenProviderMock
+			->expects($this->exactly($count + 1))
+			->method('invalidateTokensOfUser')
+			->with($this->isType('string'), 'My Client Name');
+
 		$client = new Client();
 		$client->setId(123);
 		$client->setName('My Client Name');
@@ -139,12 +177,27 @@ public function testDeleteClient() {
 			->method('deleteByClientId')
 			->with(123);
 		$this->clientMapper
+			->expects($this->once())
 			->method('delete')
 			->with($client);
 
-		$result = $this->settingsController->deleteClient(123);
+		$settingsController = new SettingsController(
+			'oauth2',
+			$this->request,
+			$this->clientMapper,
+			$this->secureRandom,
+			$this->accessTokenMapper,
+			$this->l,
+			$this->crypto,
+			$tokenProviderMock,
+			$userManager
+		);
+
+		$result = $settingsController->deleteClient(123);
 		$this->assertInstanceOf(JSONResponse::class, $result);
 		$this->assertEquals([], $result->getData());
+
+		$user1->delete();
 	}
 
 	public function testInvalidRedirectUri() {

lib/composer/composer/autoload_classmap.php

@@ -92,6 +92,7 @@
     'OCP\\Authentication\\IProvideUserSecretBackend' => $baseDir . '/lib/public/Authentication/IProvideUserSecretBackend.php',
     'OCP\\Authentication\\LoginCredentials\\ICredentials' => $baseDir . '/lib/public/Authentication/LoginCredentials/ICredentials.php',
     'OCP\\Authentication\\LoginCredentials\\IStore' => $baseDir . '/lib/public/Authentication/LoginCredentials/IStore.php',
+    'OCP\\Authentication\\Token\\IProvider' => $baseDir . '/lib/public/Authentication/Token/IProvider.php',
     'OCP\\Authentication\\TwoFactorAuth\\ALoginSetupController' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/ALoginSetupController.php',
     'OCP\\Authentication\\TwoFactorAuth\\IActivatableAtLogin' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/IActivatableAtLogin.php',
     'OCP\\Authentication\\TwoFactorAuth\\IActivatableByAdmin' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/IActivatableByAdmin.php',

lib/composer/composer/autoload_static.php

@@ -125,6 +125,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OCP\\Authentication\\IProvideUserSecretBackend' => __DIR__ . '/../../..' . '/lib/public/Authentication/IProvideUserSecretBackend.php',
         'OCP\\Authentication\\LoginCredentials\\ICredentials' => __DIR__ . '/../../..' . '/lib/public/Authentication/LoginCredentials/ICredentials.php',
         'OCP\\Authentication\\LoginCredentials\\IStore' => __DIR__ . '/../../..' . '/lib/public/Authentication/LoginCredentials/IStore.php',
+        'OCP\\Authentication\\Token\\IProvider' => __DIR__ . '/../../..' . '/lib/public/Authentication/Token/IProvider.php',
         'OCP\\Authentication\\TwoFactorAuth\\ALoginSetupController' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/ALoginSetupController.php',
         'OCP\\Authentication\\TwoFactorAuth\\IActivatableAtLogin' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/IActivatableAtLogin.php',
         'OCP\\Authentication\\TwoFactorAuth\\IActivatableByAdmin' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/IActivatableByAdmin.php',

lib/private/Authentication/Token/Manager.php

@@ -32,9 +32,9 @@
 use OC\Authentication\Exceptions\InvalidTokenException;
 use OC\Authentication\Exceptions\PasswordlessTokenException;
 use OC\Authentication\Exceptions\WipeTokenException;
+use OCP\Authentication\Token\IProvider as OCPIProvider;
 
-class Manager implements IProvider {
-
+class Manager implements IProvider, OCPIProvider {
 	/** @var PublicKeyTokenProvider */
 	private $publicKeyTokenProvider;
 
@@ -240,4 +240,13 @@ public function markPasswordInvalid(IToken $token, string $tokenId) {
 	public function updatePasswords(string $uid, string $password) {
 		$this->publicKeyTokenProvider->updatePasswords($uid, $password);
 	}
+
+	public function invalidateTokensOfUser(string $uid, ?string $clientName) {
+		$tokens = $this->getTokenByUser($uid);
+		foreach ($tokens as $token) {
+			if ($clientName === null || ($token->getName() === $clientName)) {
+				$this->invalidateTokenById($uid, $token->getId());
+			}
+		}
+	}
 }

lib/private/Server.php

@@ -161,6 +161,7 @@
 use OCP\Accounts\IAccountManager;
 use OCP\App\IAppManager;
 use OCP\Authentication\LoginCredentials\IStore;
+use OCP\Authentication\Token\IProvider as OCPIProvider;
 use OCP\BackgroundJob\IJobList;
 use OCP\Collaboration\AutoComplete\IManager;
 use OCP\Collaboration\Reference\IReferenceManager;
@@ -278,7 +279,6 @@
  * TODO: hookup all manager classes
  */
 class Server extends ServerContainer implements IServerContainer {
-
 	/** @var string */
 	private $webRoot;
 
@@ -547,6 +547,7 @@ public function __construct($webRoot, \OC\Config $config) {
 		});
 		$this->registerAlias(IStore::class, Store::class);
 		$this->registerAlias(IProvider::class, Authentication\Token\Manager::class);
+		$this->registerAlias(OCPIProvider::class, Authentication\Token\Manager::class);
 
 		$this->registerService(\OC\User\Session::class, function (Server $c) {
 			$manager = $c->get(IUserManager::class);

lib/public/Authentication/Token/IProvider.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2022 Artur Neumann <artur@jankaritech.com>
+ *
+ * @author Artur Neumann <artur@jankaritech.com>
+ *
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * 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, version 3,
+ * along with this program. If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+namespace OCP\Authentication\Token;
+
+/**
+ * @since 25.0.8
+ */
+interface IProvider {
+	/**
+	 * invalidates all tokens of a specific user
+	 * if a client name is given only tokens of that client will be invalidated
+	 *
+	 * @param string $uid
+	 * @param string|null $clientName
+	 * @since 25.0.8
+	 * @return void
+	 */
+	public function invalidateTokensOfUser(string $uid, ?string $clientName);
+}

tests/lib/Authentication/Token/ManagerTest.php

@@ -355,4 +355,48 @@ public function testUpdatePasswords() {
 
 		$this->manager->updatePasswords('uid', 'pass');
 	}
+
+	public function testInvalidateTokensOfUserNoClientName() {
+		$t1 = new PublicKeyToken();
+		$t2 = new PublicKeyToken();
+		$t1->setId(123);
+		$t2->setId(456);
+
+		$this->publicKeyTokenProvider
+			->expects($this->once())
+			->method('getTokenByUser')
+			->with('theUser')
+			->willReturn([$t1, $t2]);
+		$this->publicKeyTokenProvider
+			->expects($this->exactly(2))
+			->method('invalidateTokenById')
+			->withConsecutive(
+				['theUser', 123],
+				['theUser', 456],
+			);
+		$this->manager->invalidateTokensOfUser('theUser', null);
+	}
+
+	public function testInvalidateTokensOfUserClientNameGiven() {
+		$t1 = new PublicKeyToken();
+		$t2 = new PublicKeyToken();
+		$t3 = new PublicKeyToken();
+		$t1->setId(123);
+		$t1->setName('Firefox session');
+		$t2->setId(456);
+		$t2->setName('My Client Name');
+		$t3->setId(789);
+		$t3->setName('mobile client');
+
+		$this->publicKeyTokenProvider
+			->expects($this->once())
+			->method('getTokenByUser')
+			->with('theUser')
+			->willReturn([$t1, $t2, $t3]);
+		$this->publicKeyTokenProvider
+			->expects($this->once())
+			->method('invalidateTokenById')
+			->with('theUser', 456);
+		$this->manager->invalidateTokensOfUser('theUser', 'My Client Name');
+	}
 }