Skip to content

Commit 00afe49

Browse files
blizzzblizzz
authored
Merge pull request #37230 from nextcloud/backport/36033/stable26
[stable26] invalidate existing tokens when deleting an oauth client
2 parents d12699a + 40dcc08 commit 00afe49

8 files changed

Lines changed: 175 additions & 8 deletions

File tree

‎apps/oauth2/lib/Controller/SettingsController.php‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
*/
3131
namespace OCA\OAuth2\Controller;
3232

33+
use OCP\Authentication\Token\IProvider as IAuthTokenProvider;
3334
use OCA\OAuth2\Db\AccessTokenMapper;
3435
use OCA\OAuth2\Db\Client;
3536
use OCA\OAuth2\Db\ClientMapper;
@@ -38,6 +39,8 @@
3839
use OCP\AppFramework\Http\JSONResponse;
3940
use OCP\IL10N;
4041
use OCP\IRequest;
42+
use OCP\IUser;
43+
use OCP\IUserManager;
4144
use OCP\Security\ISecureRandom;
4245

4346
class SettingsController extends Controller {
@@ -49,21 +52,30 @@ class SettingsController extends Controller {
4952
private $accessTokenMapper;
5053
/** @var IL10N */
5154
private $l;
52-
55+
/** @var IAuthTokenProvider */
56+
private $tokenProvider;
57+
/**
58+
* @var IUserManager
59+
*/
60+
private $userManager;
5361
public const validChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
5462

5563
public function __construct(string $appName,
5664
IRequest $request,
5765
ClientMapper $clientMapper,
5866
ISecureRandom $secureRandom,
5967
AccessTokenMapper $accessTokenMapper,
60-
IL10N $l
68+
IL10N $l,
69+
IAuthTokenProvider $tokenProvider,
70+
IUserManager $userManager
6171
) {
6272
parent::__construct($appName, $request);
6373
$this->secureRandom = $secureRandom;
6474
$this->clientMapper = $clientMapper;
6575
$this->accessTokenMapper = $accessTokenMapper;
6676
$this->l = $l;
77+
$this->tokenProvider = $tokenProvider;
78+
$this->userManager = $userManager;
6779
}
6880

6981
public function addClient(string $name,
@@ -92,6 +104,11 @@ public function addClient(string $name,
92104

93105
public function deleteClient(int $id): JSONResponse {
94106
$client = $this->clientMapper->getByUid($id);
107+
108+
$this->userManager->callForSeenUsers(function (IUser $user) use ($client) {
109+
$this->tokenProvider->invalidateTokensOfUser($user->getUID(), $client->getName());
110+
});
111+
95112
$this->accessTokenMapper->deleteByClientId($id);
96113
$this->clientMapper->delete($client);
97114
return new JSONResponse([]);

‎apps/oauth2/tests/Controller/SettingsControllerTest.php‎

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
*/
2727
namespace OCA\OAuth2\Tests\Controller;
2828

29+
use OC\Authentication\Token\IToken;
30+
use OCP\Authentication\Token\IProvider as IAuthTokenProvider;
2931
use OCA\OAuth2\Controller\SettingsController;
3032
use OCA\OAuth2\Db\AccessTokenMapper;
3133
use OCA\OAuth2\Db\Client;
@@ -34,9 +36,14 @@
3436
use OCP\AppFramework\Http\JSONResponse;
3537
use OCP\IL10N;
3638
use OCP\IRequest;
39+
use OCP\IUser;
40+
use OCP\IUserManager;
3741
use OCP\Security\ISecureRandom;
3842
use Test\TestCase;
3943

44+
/**
45+
* @group DB
46+
*/
4047
class SettingsControllerTest extends TestCase {
4148
/** @var IRequest|\PHPUnit\Framework\MockObject\MockObject */
4249
private $request;
@@ -46,8 +53,14 @@ class SettingsControllerTest extends TestCase {
4653
private $secureRandom;
4754
/** @var AccessTokenMapper|\PHPUnit\Framework\MockObject\MockObject */
4855
private $accessTokenMapper;
56+
/** @var IAuthTokenProvider|\PHPUnit\Framework\MockObject\MockObject */
57+
private $authTokenProvider;
58+
/** @var IUserManager|\PHPUnit\Framework\MockObject\MockObject */
59+
private $userManager;
4960
/** @var SettingsController */
5061
private $settingsController;
62+
/** @var IL10N|\PHPUnit\Framework\MockObject\MockObject */
63+
private $l;
5164

5265
protected function setUp(): void {
5366
parent::setUp();
@@ -56,18 +69,22 @@ protected function setUp(): void {
5669
$this->clientMapper = $this->createMock(ClientMapper::class);
5770
$this->secureRandom = $this->createMock(ISecureRandom::class);
5871
$this->accessTokenMapper = $this->createMock(AccessTokenMapper::class);
59-
$l = $this->createMock(IL10N::class);
60-
$l->method('t')
72+
$this->authTokenProvider = $this->createMock(IAuthTokenProvider::class);
73+
$this->userManager = $this->createMock(IUserManager::class);
74+
$this->l = $this->createMock(IL10N::class);
75+
$this->l->method('t')
6176
->willReturnArgument(0);
62-
6377
$this->settingsController = new SettingsController(
6478
'oauth2',
6579
$this->request,
6680
$this->clientMapper,
6781
$this->secureRandom,
6882
$this->accessTokenMapper,
69-
$l
83+
$this->l,
84+
$this->authTokenProvider,
85+
$this->userManager
7086
);
87+
7188
}
7289

7390
public function testAddClient() {
@@ -113,6 +130,26 @@ public function testAddClient() {
113130
}
114131

115132
public function testDeleteClient() {
133+
134+
$userManager = \OC::$server->getUserManager();
135+
// count other users in the db before adding our own
136+
$count = 0;
137+
$function = function (IUser $user) use (&$count) {
138+
if ($user->getLastLogin() > 0) {
139+
$count++;
140+
}
141+
};
142+
$userManager->callForAllUsers($function);
143+
$user1 = $userManager->createUser('test101', 'test101');
144+
$user1->updateLastLoginTimestamp();
145+
$tokenProviderMock = $this->getMockBuilder(IAuthTokenProvider::class)->getMock();
146+
147+
// expect one call per user and ensure the correct client name
148+
$tokenProviderMock
149+
->expects($this->exactly($count + 1))
150+
->method('invalidateTokensOfUser')
151+
->with($this->isType('string'), 'My Client Name');
152+
116153
$client = new Client();
117154
$client->setId(123);
118155
$client->setName('My Client Name');
@@ -129,12 +166,26 @@ public function testDeleteClient() {
129166
->method('deleteByClientId')
130167
->with(123);
131168
$this->clientMapper
169+
->expects($this->once())
132170
->method('delete')
133171
->with($client);
134172

135-
$result = $this->settingsController->deleteClient(123);
173+
$settingsController = new SettingsController(
174+
'oauth2',
175+
$this->request,
176+
$this->clientMapper,
177+
$this->secureRandom,
178+
$this->accessTokenMapper,
179+
$this->l,
180+
$tokenProviderMock,
181+
$userManager
182+
);
183+
184+
$result = $settingsController->deleteClient(123);
136185
$this->assertInstanceOf(JSONResponse::class, $result);
137186
$this->assertEquals([], $result->getData());
187+
188+
$user1->delete();
138189
}
139190

140191
public function testInvalidRedirectUri() {

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@
9393
'OCP\\Authentication\\IProvideUserSecretBackend' => $baseDir . '/lib/public/Authentication/IProvideUserSecretBackend.php',
9494
'OCP\\Authentication\\LoginCredentials\\ICredentials' => $baseDir . '/lib/public/Authentication/LoginCredentials/ICredentials.php',
9595
'OCP\\Authentication\\LoginCredentials\\IStore' => $baseDir . '/lib/public/Authentication/LoginCredentials/IStore.php',
96+
'OCP\\Authentication\\Token\\IProvider' => $baseDir . '/lib/public/Authentication/Token/IProvider.php',
9697
'OCP\\Authentication\\TwoFactorAuth\\ALoginSetupController' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/ALoginSetupController.php',
9798
'OCP\\Authentication\\TwoFactorAuth\\IActivatableAtLogin' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/IActivatableAtLogin.php',
9899
'OCP\\Authentication\\TwoFactorAuth\\IActivatableByAdmin' => $baseDir . '/lib/public/Authentication/TwoFactorAuth/IActivatableByAdmin.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
126126
'OCP\\Authentication\\IProvideUserSecretBackend' => __DIR__ . '/../../..' . '/lib/public/Authentication/IProvideUserSecretBackend.php',
127127
'OCP\\Authentication\\LoginCredentials\\ICredentials' => __DIR__ . '/../../..' . '/lib/public/Authentication/LoginCredentials/ICredentials.php',
128128
'OCP\\Authentication\\LoginCredentials\\IStore' => __DIR__ . '/../../..' . '/lib/public/Authentication/LoginCredentials/IStore.php',
129+
'OCP\\Authentication\\Token\\IProvider' => __DIR__ . '/../../..' . '/lib/public/Authentication/Token/IProvider.php',
129130
'OCP\\Authentication\\TwoFactorAuth\\ALoginSetupController' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/ALoginSetupController.php',
130131
'OCP\\Authentication\\TwoFactorAuth\\IActivatableAtLogin' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/IActivatableAtLogin.php',
131132
'OCP\\Authentication\\TwoFactorAuth\\IActivatableByAdmin' => __DIR__ . '/../../..' . '/lib/public/Authentication/TwoFactorAuth/IActivatableByAdmin.php',

‎lib/private/Authentication/Token/Manager.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@
3232
use OC\Authentication\Exceptions\InvalidTokenException;
3333
use OC\Authentication\Exceptions\PasswordlessTokenException;
3434
use OC\Authentication\Exceptions\WipeTokenException;
35+
use OCP\Authentication\Token\IProvider as OCPIProvider;
3536

36-
class Manager implements IProvider {
37+
class Manager implements IProvider, OCPIProvider {
3738
/** @var PublicKeyTokenProvider */
3839
private $publicKeyTokenProvider;
3940

@@ -239,4 +240,13 @@ public function markPasswordInvalid(IToken $token, string $tokenId) {
239240
public function updatePasswords(string $uid, string $password) {
240241
$this->publicKeyTokenProvider->updatePasswords($uid, $password);
241242
}
243+
244+
public function invalidateTokensOfUser(string $uid, ?string $clientName) {
245+
$tokens = $this->getTokenByUser($uid);
246+
foreach ($tokens as $token) {
247+
if ($clientName === null || ($token->getName() === $clientName)) {
248+
$this->invalidateTokenById($uid, $token->getId());
249+
}
250+
}
251+
}
242252
}

‎lib/private/Server.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@
163163
use OCP\Accounts\IAccountManager;
164164
use OCP\App\IAppManager;
165165
use OCP\Authentication\LoginCredentials\IStore;
166+
use OCP\Authentication\Token\IProvider as OCPIProvider;
166167
use OCP\BackgroundJob\IJobList;
167168
use OCP\Collaboration\AutoComplete\IManager;
168169
use OCP\Collaboration\Reference\IReferenceManager;
@@ -550,6 +551,7 @@ public function __construct($webRoot, \OC\Config $config) {
550551
});
551552
$this->registerAlias(IStore::class, Store::class);
552553
$this->registerAlias(IProvider::class, Authentication\Token\Manager::class);
554+
$this->registerAlias(OCPIProvider::class, Authentication\Token\Manager::class);
553555

554556
$this->registerService(\OC\User\Session::class, function (Server $c) {
555557
$manager = $c->get(IUserManager::class);

‎lib/public/Authentication/Token/IProvider.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright Copyright (c) 2022 Artur Neumann <artur@jankaritech.com>
7+
*
8+
* @author Artur Neumann <artur@jankaritech.com>
9+
*
10+
* @license AGPL-3.0
11+
*
12+
* This code is free software: you can redistribute it and/or modify
13+
* it under the terms of the GNU Affero General Public License, version 3,
14+
* as published by the Free Software Foundation.
15+
*
16+
* This program is distributed in the hope that it will be useful,
17+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
18+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
19+
* GNU Affero General Public License for more details.
20+
*
21+
* You should have received a copy of the GNU Affero General Public License, version 3,
22+
* along with this program. If not, see <http://www.gnu.org/licenses/>
23+
*
24+
*/
25+
namespace OCP\Authentication\Token;
26+
27+
/**
28+
* @since 26.0.3
29+
*/
30+
interface IProvider {
31+
/**
32+
* invalidates all tokens of a specific user
33+
* if a client name is given only tokens of that client will be invalidated
34+
*
35+
* @param string $uid
36+
* @param string|null $clientName
37+
* @since 26.0.3
38+
* @return void
39+
*/
40+
public function invalidateTokensOfUser(string $uid, ?string $clientName);
41+
}

‎tests/lib/Authentication/Token/ManagerTest.php‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,4 +355,48 @@ public function testUpdatePasswords() {
355355

356356
$this->manager->updatePasswords('uid', 'pass');
357357
}
358+
359+
public function testInvalidateTokensOfUserNoClientName() {
360+
$t1 = new PublicKeyToken();
361+
$t2 = new PublicKeyToken();
362+
$t1->setId(123);
363+
$t2->setId(456);
364+
365+
$this->publicKeyTokenProvider
366+
->expects($this->once())
367+
->method('getTokenByUser')
368+
->with('theUser')
369+
->willReturn([$t1, $t2]);
370+
$this->publicKeyTokenProvider
371+
->expects($this->exactly(2))
372+
->method('invalidateTokenById')
373+
->withConsecutive(
374+
['theUser', 123],
375+
['theUser', 456],
376+
);
377+
$this->manager->invalidateTokensOfUser('theUser', null);
378+
}
379+
380+
public function testInvalidateTokensOfUserClientNameGiven() {
381+
$t1 = new PublicKeyToken();
382+
$t2 = new PublicKeyToken();
383+
$t3 = new PublicKeyToken();
384+
$t1->setId(123);
385+
$t1->setName('Firefox session');
386+
$t2->setId(456);
387+
$t2->setName('My Client Name');
388+
$t3->setId(789);
389+
$t3->setName('mobile client');
390+
391+
$this->publicKeyTokenProvider
392+
->expects($this->once())
393+
->method('getTokenByUser')
394+
->with('theUser')
395+
->willReturn([$t1, $t2, $t3]);
396+
$this->publicKeyTokenProvider
397+
->expects($this->once())
398+
->method('invalidateTokenById')
399+
->with('theUser', 456);
400+
$this->manager->invalidateTokensOfUser('theUser', 'My Client Name');
401+
}
358402
}

0 commit comments

Comments
 (0)