From 9f8f2d27b6861ecde0b479db2a9dd3ef0d395d67 Mon Sep 17 00:00:00 2001 From: Artur Neumann Date: Fri, 11 Nov 2022 13:16:14 +0545 Subject: [PATCH] invalidate existing tokens when deleting an oauth client Signed-off-by: Artur Neumann --- .../lib/Controller/SettingsController.php | 28 ++++++++- .../Controller/SettingsControllerTest.php | 62 +++++++++++++++++-- 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/apps/oauth2/lib/Controller/SettingsController.php b/apps/oauth2/lib/Controller/SettingsController.php index 046e6d77041..872288064dd 100644 --- 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 216655190ae..6be8a4ee4b9 100644 --- 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() { -- 2.39.5