invalidate existing tokens when deleting an oauth client

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

diff --git a/apps/oauth2/lib/Controller/SettingsController.php b/apps/oauth2/lib/Controller/SettingsController.php
index 046e6d7704188c3c04a1de3c9eaaef60c9c20b39..872288064ddf2842e8f198e514e627b7f9317fe3 100644 (file)
--- a/apps/oauth2/lib/Controller/SettingsController.php
+++ b/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([]);

diff --git a/apps/oauth2/tests/Controller/SettingsControllerTest.php b/apps/oauth2/tests/Controller/SettingsControllerTest.php
index 216655190ae5812a0d91a5795cfb98b0ba95d6b7..6be8a4ee4b98334adcf0195bd49436a571d4cc20 100644 (file)
--- a/apps/oauth2/tests/Controller/SettingsControllerTest.php
+++ b/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() {