invalidate existing tokens when deleting an oauth client

Signed-off-by: Artur Neumann <artur@jankaritech.com>

apps/oauth2/lib/Controller/SettingsController.php

@@ -30,6 +30,7 @@ declare(strict_types=1);
  */
 namespace OCA\OAuth2\Controller;
 
+use OC\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;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IL10N;
 use OCP\IRequest;
+use OCP\IUser;
+use OCP\IUserManager;
 use OCP\Security\ISecureRandom;
 
 class SettingsController extends Controller {
@@ -49,7 +52,12 @@ class SettingsController extends Controller {
 	private $accessTokenMapper;
 	/** @var IL10N */
 	private $l;
-
+	/** @var IAuthTokenProvider */
+	private $tokenProvider;
+	/**
+	 * @var IUserManager
+	 */
+	private $userManager;
 	public const validChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
 
 	public function __construct(string $appName,
@@ -57,13 +65,17 @@ class SettingsController extends Controller {
 								ClientMapper $clientMapper,
 								ISecureRandom $secureRandom,
 								AccessTokenMapper $accessTokenMapper,
-								IL10N $l
+								IL10N $l,
+								IAuthTokenProvider $tokenProvider,
+								IUserManager $userManager
 	) {
 		parent::__construct($appName, $request);
 		$this->secureRandom = $secureRandom;
 		$this->clientMapper = $clientMapper;
 		$this->accessTokenMapper = $accessTokenMapper;
 		$this->l = $l;
+		$this->tokenProvider = $tokenProvider;
+		$this->userManager = $userManager;
 	}
 
 	public function addClient(string $name,
@@ -92,6 +104,18 @@ class SettingsController extends Controller {
 
 	public function deleteClient(int $id): JSONResponse {
 		$client = $this->clientMapper->getByUid($id);
+
+		$this->userManager->callForAllUsers(function (IUser $user) use ($client) {
+			$tokens = $this->tokenProvider->getTokenByUser($user->getUID());
+			foreach ($tokens as $token) {
+				if ($token->getName() === $client->getName()) {
+					$this->tokenProvider->invalidateTokenById(
+						$user->getUID(), $token->getId()
+					);
+				}
+			}
+		});
+
 		$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 OC\Authentication\Token\IProvider as IAuthTokenProvider;
 use OCA\OAuth2\Controller\SettingsController;
 use OCA\OAuth2\Db\AccessTokenMapper;
 use OCA\OAuth2\Db\Client;
@@ -34,9 +36,14 @@ use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IL10N;
 use OCP\IRequest;
+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;
@@ -48,6 +55,8 @@ class SettingsControllerTest extends TestCase {
 	private $accessTokenMapper;
 	/** @var SettingsController */
 	private $settingsController;
+	/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
+	private $l;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -56,18 +65,20 @@ class SettingsControllerTest extends TestCase {
 		$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->l = $this->createMock(IL10N::class);
+		$this->l->method('t')
 			->willReturnArgument(0);
-
 		$this->settingsController = new SettingsController(
 			'oauth2',
 			$this->request,
 			$this->clientMapper,
 			$this->secureRandom,
 			$this->accessTokenMapper,
-			$l
+			$this->l,
+			$this->createMock(IAuthTokenProvider::class),
+			$this->createMock(IUserManager::class)
 		);
+
 	}
 
 	public function testAddClient() {
@@ -113,6 +124,34 @@ class SettingsControllerTest extends TestCase {
 	}
 
 	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) {
+			$count++;
+		};
+		$userManager->callForAllUsers($function);
+		$user1 = $userManager->createUser('test101', 'test101');
+		$tokenMocks[0] = $this->getMockBuilder(IToken::class)->getMock();
+		$tokenMocks[0]->method('getName')->willReturn('Firefox session');
+		$tokenMocks[0]->method('getId')->willReturn(1);
+		$tokenMocks[1] = $this->getMockBuilder(IToken::class)->getMock();
+		$tokenMocks[1]->method('getName')->willReturn('My Client Name');
+		$tokenMocks[1]->method('getId')->willReturn(2);
+		$tokenMocks[2] = $this->getMockBuilder(IToken::class)->getMock();
+		$tokenMocks[2]->method('getName')->willReturn('mobile client');
+		$tokenMocks[2]->method('getId')->willReturn(3);
+
+		$tokenProviderMock = $this->getMockBuilder(IAuthTokenProvider::class)->getMock();
+		$tokenProviderMock->method('getTokenByUser')->willReturn($tokenMocks);
+
+		// expect one call per user and make sure the correct tokeId is selected
+		$tokenProviderMock
+			->expects($this->exactly($count + 1))
+			->method('invalidateTokenById')
+			->with($this->isType('string'), 2);
+
 		$client = new Client();
 		$client->setId(123);
 		$client->setName('My Client Name');
@@ -132,9 +171,22 @@ class SettingsControllerTest extends TestCase {
 			->method('delete')
 			->with($client);
 
-		$result = $this->settingsController->deleteClient(123);
+		$settingsController = new SettingsController(
+			'oauth2',
+			$this->request,
+			$this->clientMapper,
+			$this->secureRandom,
+			$this->accessTokenMapper,
+			$this->l,
+			$tokenProviderMock,
+			$userManager
+		);
+
+		$result = $settingsController->deleteClient(123);
 		$this->assertInstanceOf(JSONResponse::class, $result);
 		$this->assertEquals([], $result->getData());
+
+		$user1->delete();
 	}
 
 	public function testInvalidRedirectUri() {