Skip to content

Commit bc2d8f0

Browse files
printminion-coprintminion-co
committed
feat(login-flow-v2): Restrict allowed apps by user agent check
Enable via: ./occ config:system:set core.login_flow_v2.allowed_user_agents 0 --value '/Custom Foo Client/i' ./occ config:system:set core.login_flow_v2.allowed_user_agents 1 --value '/Custom Bar Client/i' if user agent string is unknown the template with "Access forbidden"-"Please use original client" will be displayed Signed-off-by: Misha M.-Kupriyanov <[email protected]>
1 parent 9a8c099 commit bc2d8f0

9 files changed

Lines changed: 199 additions & 2 deletions

File tree

‎config/config.sample.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2515,6 +2515,20 @@
25152515
'/^Microsoft-WebDAV-MiniRedir/', // Windows webdav drive
25162516
],
25172517

2518+
/**
2519+
* This option allows you to specify a list of allowed user agents for the Login Flow V2.
2520+
* If a user agent is not in this list, it will not be allowed to use the Login Flow V2.
2521+
* The user agents are defined using regular expressions.
2522+
*
2523+
* WARNING: only use this if you know what you are doing
2524+
*
2525+
* Example: Allow only the Nextcloud Android app to use the Login Flow V2
2526+
* 'core.login_flow_v2.allowed_user_agents' => ['/Nextcloud-android/i'],
2527+
*
2528+
* Defaults to an empty array.
2529+
*/
2530+
'core.login_flow_v2.allowed_user_agents' => [],
2531+
25182532
/**
25192533
* By default, there is on public pages a link shown that allows users to
25202534
* learn about the "simple sign up" - see https://nextcloud.com/signup/

‎core/Controller/ClientFlowLoginV2Controller.php‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
namespace OC\Core\Controller;
1010

1111
use OC\Core\Db\LoginFlowV2;
12+
use OC\Core\Exception\LoginFlowV2ClientForbiddenException;
1213
use OC\Core\Exception\LoginFlowV2NotFoundException;
1314
use OC\Core\ResponseDefinitions;
1415
use OC\Core\Service\LoginFlowV2Service;
@@ -109,6 +110,8 @@ public function showAuthPickerPage(string $user = '', int $direct = 0): Standalo
109110
$flow = $this->getFlowByLoginToken();
110111
} catch (LoginFlowV2NotFoundException $e) {
111112
return $this->loginTokenForbiddenResponse();
113+
} catch (LoginFlowV2ClientForbiddenException $e) {
114+
return $this->loginTokenForbiddenClientResponse();
112115
}
113116

114117
$stateToken = $this->random->generate(
@@ -152,6 +155,8 @@ public function grantPage(?string $stateToken, int $direct = 0): StandaloneTempl
152155
$flow = $this->getFlowByLoginToken();
153156
} catch (LoginFlowV2NotFoundException $e) {
154157
return $this->loginTokenForbiddenResponse();
158+
} catch (LoginFlowV2ClientForbiddenException $e) {
159+
return $this->loginTokenForbiddenClientResponse();
155160
}
156161

157162
/** @var IUser $user */
@@ -188,6 +193,8 @@ public function apptokenRedirect(?string $stateToken, string $user, string $pass
188193
$this->getFlowByLoginToken();
189194
} catch (LoginFlowV2NotFoundException $e) {
190195
return $this->loginTokenForbiddenResponse();
196+
} catch (LoginFlowV2ClientForbiddenException $e) {
197+
return $this->loginTokenForbiddenClientResponse();
191198
}
192199

193200
$loginToken = $this->session->get(self::TOKEN_NAME);
@@ -233,6 +240,8 @@ public function generateAppPassword(?string $stateToken): Response {
233240
$this->getFlowByLoginToken();
234241
} catch (LoginFlowV2NotFoundException $e) {
235242
return $this->loginTokenForbiddenResponse();
243+
} catch (LoginFlowV2ClientForbiddenException $e) {
244+
return $this->loginTokenForbiddenClientResponse();
236245
}
237246

238247
$loginToken = $this->session->get(self::TOKEN_NAME);
@@ -333,6 +342,7 @@ private function stateTokenForbiddenResponse(): StandaloneTemplateResponse {
333342
/**
334343
* @return LoginFlowV2
335344
* @throws LoginFlowV2NotFoundException
345+
* @throws LoginFlowV2ClientForbiddenException
336346
*/
337347
private function getFlowByLoginToken(): LoginFlowV2 {
338348
$currentToken = $this->session->get(self::TOKEN_NAME);
@@ -356,6 +366,19 @@ private function loginTokenForbiddenResponse(): StandaloneTemplateResponse {
356366
return $response;
357367
}
358368

369+
private function loginTokenForbiddenClientResponse(): StandaloneTemplateResponse {
370+
$response = new StandaloneTemplateResponse(
371+
$this->appName,
372+
'403',
373+
[
374+
'message' => $this->l10n->t('Please use original client'),
375+
],
376+
'guest'
377+
);
378+
$response->setStatus(Http::STATUS_FORBIDDEN);
379+
return $response;
380+
}
381+
359382
private function getServerPath(): string {
360383
$serverPostfix = '';
361384

‎core/Exception/LoginFlowV2ClientForbiddenException.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OC\Core\Exception;
10+
11+
class LoginFlowV2ClientForbiddenException extends \Exception {
12+
}

‎core/Service/LoginFlowV2Service.php‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use OC\Core\Data\LoginFlowV2Tokens;
1616
use OC\Core\Db\LoginFlowV2;
1717
use OC\Core\Db\LoginFlowV2Mapper;
18+
use OC\Core\Exception\LoginFlowV2ClientForbiddenException;
1819
use OC\Core\Exception\LoginFlowV2NotFoundException;
1920
use OCP\AppFramework\Db\DoesNotExistException;
2021
use OCP\AppFramework\Utility\ITimeFactory;
@@ -74,13 +75,33 @@ public function poll(string $pollToken): LoginFlowV2Credentials {
7475
* @param string $loginToken
7576
* @return LoginFlowV2
7677
* @throws LoginFlowV2NotFoundException
78+
* @throws LoginFlowV2ClientForbiddenException
7779
*/
7880
public function getByLoginToken(string $loginToken): LoginFlowV2 {
81+
/** @var LoginFlowV2|null $flow */
82+
$flow = null;
83+
7984
try {
80-
return $this->mapper->getByLoginToken($loginToken);
85+
$flow = $this->mapper->getByLoginToken($loginToken);
8186
} catch (DoesNotExistException $e) {
8287
throw new LoginFlowV2NotFoundException('Login token invalid');
8388
}
89+
90+
$allowedAgents = $this->config->getSystemValue('core.login_flow_v2.allowed_user_agents', []);
91+
92+
if (empty($allowedAgents)) {
93+
return $flow;
94+
}
95+
96+
$flowClient = $flow->getClientName();
97+
98+
foreach ($allowedAgents as $allowedAgent) {
99+
if (preg_match($allowedAgent, $flowClient) === 1) {
100+
return $flow;
101+
}
102+
}
103+
104+
throw new LoginFlowV2ClientForbiddenException('Client not allowed');
84105
}
85106

86107
/**

‎lib/composer/autoload.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@
1414
echo $err;
1515
}
1616
}
17-
throw new RuntimeException($err);
17+
trigger_error(
18+
$err,
19+
E_USER_ERROR
20+
);
1821
}
1922

2023
require_once __DIR__ . '/composer/autoload_real.php';

‎lib/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,6 +1371,7 @@
13711371
'OC\\Core\\Db\\ProfileConfigMapper' => $baseDir . '/core/Db/ProfileConfigMapper.php',
13721372
'OC\\Core\\Events\\BeforePasswordResetEvent' => $baseDir . '/core/Events/BeforePasswordResetEvent.php',
13731373
'OC\\Core\\Events\\PasswordResetEvent' => $baseDir . '/core/Events/PasswordResetEvent.php',
1374+
'OC\\Core\\Exception\\LoginFlowV2ClientForbiddenException' => $baseDir . '/core/Exception/LoginFlowV2ClientForbiddenException.php',
13741375
'OC\\Core\\Exception\\LoginFlowV2NotFoundException' => $baseDir . '/core/Exception/LoginFlowV2NotFoundException.php',
13751376
'OC\\Core\\Exception\\ResetPasswordException' => $baseDir . '/core/Exception/ResetPasswordException.php',
13761377
'OC\\Core\\Listener\\BeforeMessageLoggedEventListener' => $baseDir . '/core/Listener/BeforeMessageLoggedEventListener.php',

‎lib/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,6 +1412,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
14121412
'OC\\Core\\Db\\ProfileConfigMapper' => __DIR__ . '/../../..' . '/core/Db/ProfileConfigMapper.php',
14131413
'OC\\Core\\Events\\BeforePasswordResetEvent' => __DIR__ . '/../../..' . '/core/Events/BeforePasswordResetEvent.php',
14141414
'OC\\Core\\Events\\PasswordResetEvent' => __DIR__ . '/../../..' . '/core/Events/PasswordResetEvent.php',
1415+
'OC\\Core\\Exception\\LoginFlowV2ClientForbiddenException' => __DIR__ . '/../../..' . '/core/Exception/LoginFlowV2ClientForbiddenException.php',
14151416
'OC\\Core\\Exception\\LoginFlowV2NotFoundException' => __DIR__ . '/../../..' . '/core/Exception/LoginFlowV2NotFoundException.php',
14161417
'OC\\Core\\Exception\\ResetPasswordException' => __DIR__ . '/../../..' . '/core/Exception/ResetPasswordException.php',
14171418
'OC\\Core\\Listener\\BeforeMessageLoggedEventListener' => __DIR__ . '/../../..' . '/core/Listener/BeforeMessageLoggedEventListener.php',

‎tests/Core/Controller/ClientFlowLoginV2ControllerTest.php‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use OC\Core\Controller\ClientFlowLoginV2Controller;
1212
use OC\Core\Data\LoginFlowV2Credentials;
1313
use OC\Core\Db\LoginFlowV2;
14+
use OC\Core\Exception\LoginFlowV2ClientForbiddenException;
1415
use OC\Core\Exception\LoginFlowV2NotFoundException;
1516
use OC\Core\Service\LoginFlowV2Service;
1617
use OCP\AppFramework\Http;
@@ -56,6 +57,12 @@ protected function setUp(): void {
5657
$this->random = $this->createMock(ISecureRandom::class);
5758
$this->defaults = $this->createMock(Defaults::class);
5859
$this->l = $this->createMock(IL10N::class);
60+
$this->l
61+
->expects($this->any())
62+
->method('t')
63+
->willReturnCallback(function ($text, $parameters = []) {
64+
return vsprintf($text, $parameters);
65+
});
5966
$this->controller = new ClientFlowLoginV2Controller(
6067
'core',
6168
$this->request,
@@ -150,6 +157,22 @@ public function testShowAuthPickerInvalidLoginToken(): void {
150157
$this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus());
151158
}
152159

160+
public function testShowAuthPickerForbiddenUserClient() {
161+
$this->session->method('get')
162+
->with('client.flow.v2.login.token')
163+
->willReturn('loginToken');
164+
165+
$this->loginFlowV2Service->method('getByLoginToken')
166+
->with('loginToken')
167+
->willThrowException(new LoginFlowV2ClientForbiddenException());
168+
169+
$result = $this->controller->showAuthPickerPage();
170+
171+
$this->assertInstanceOf(Http\StandaloneTemplateResponse::class, $result);
172+
$this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus());
173+
$this->assertSame('Please use original client', $result->getParams()['message']);
174+
}
175+
153176
public function testShowAuthPickerValidLoginToken(): void {
154177
$this->session->method('get')
155178
->with('client.flow.v2.login.token')
@@ -206,6 +229,29 @@ public function testGrantPageInvalidLoginToken(): void {
206229
$this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus());
207230
}
208231

232+
public function testGrantPageForbiddenUserClient() {
233+
$this->session->method('get')
234+
->willReturnCallback(function ($name) {
235+
if ($name === 'client.flow.v2.state.token') {
236+
return 'stateToken';
237+
}
238+
if ($name === 'client.flow.v2.login.token') {
239+
return 'loginToken';
240+
}
241+
return null;
242+
});
243+
244+
$this->loginFlowV2Service->method('getByLoginToken')
245+
->with('loginToken')
246+
->willThrowException(new LoginFlowV2ClientForbiddenException());
247+
248+
$result = $this->controller->grantPage('stateToken');
249+
250+
$this->assertInstanceOf(Http\StandaloneTemplateResponse::class, $result);
251+
$this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus());
252+
$this->assertSame('Please use original client', $result->getParams()['message']);
253+
}
254+
209255
public function testGrantPageValid(): void {
210256
$this->session->method('get')
211257
->willReturnCallback(function ($name) {
@@ -266,6 +312,29 @@ public function testGenerateAppPassworInvalidLoginToken(): void {
266312
$this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus());
267313
}
268314

315+
public function testGenerateAppPasswordForbiddenUserClient() {
316+
$this->session->method('get')
317+
->willReturnCallback(function ($name) {
318+
if ($name === 'client.flow.v2.state.token') {
319+
return 'stateToken';
320+
}
321+
if ($name === 'client.flow.v2.login.token') {
322+
return 'loginToken';
323+
}
324+
return null;
325+
});
326+
327+
$this->loginFlowV2Service->method('getByLoginToken')
328+
->with('loginToken')
329+
->willThrowException(new LoginFlowV2ClientForbiddenException());
330+
331+
$result = $this->controller->generateAppPassword('stateToken');
332+
333+
$this->assertInstanceOf(Http\StandaloneTemplateResponse::class, $result);
334+
$this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus());
335+
$this->assertSame('Please use original client', $result->getParams()['message']);
336+
}
337+
269338
public function testGenerateAppPassworValid(): void {
270339
$this->session->method('get')
271340
->willReturnCallback(function ($name) {

‎tests/Core/Service/LoginFlowV2ServiceUnitTest.php‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
<?php
2+
23
/**
34
* SPDX-FileCopyrightText: 2021 Nextcloud GmbH and Nextcloud contributors
45
* SPDX-License-Identifier: AGPL-3.0-only
@@ -14,6 +15,7 @@
1415
use OC\Core\Data\LoginFlowV2Tokens;
1516
use OC\Core\Db\LoginFlowV2;
1617
use OC\Core\Db\LoginFlowV2Mapper;
18+
use OC\Core\Exception\LoginFlowV2ClientForbiddenException;
1719
use OC\Core\Exception\LoginFlowV2NotFoundException;
1820
use OC\Core\Service\LoginFlowV2Service;
1921
use OCP\AppFramework\Db\DoesNotExistException;
@@ -237,6 +239,57 @@ public function testGetByLoginTokenLoginTokenInvalid(): void {
237239
$this->subjectUnderTest->getByLoginToken('test_token');
238240
}
239241

242+
public function testGetByLoginTokenClientForbidden() {
243+
$this->expectException(LoginFlowV2ClientForbiddenException::class);
244+
$this->expectExceptionMessage('Client not allowed');
245+
246+
$allowedClients = [
247+
'/Custom Allowed Client/i'
248+
];
249+
250+
$this->config->expects($this->exactly(1))
251+
->method('getSystemValue')
252+
->willReturn($this->returnCallback(function ($key) use ($allowedClients) {
253+
// Note: \OCP\IConfig::getSystemValue returns either an array or string.
254+
return $key == 'core.login_flow_v2.allowed_user_agents' ? $allowedClients : '';
255+
}));
256+
257+
$loginFlowV2 = new LoginFlowV2();
258+
$loginFlowV2->setClientName('Rogue Curl Client/1.0');
259+
260+
$this->mapper->expects($this->once())
261+
->method('getByLoginToken')
262+
->willReturn($loginFlowV2);
263+
264+
$this->subjectUnderTest->getByLoginToken('test_token');
265+
}
266+
267+
public function testGetByLoginTokenClientAllowed() {
268+
$allowedClients = [
269+
'/Foo Allowed Client/i',
270+
'/Custom Allowed Client/i'
271+
];
272+
273+
$loginFlowV2 = new LoginFlowV2();
274+
$loginFlowV2->setClientName('Custom Allowed Client Curl Client/1.0');
275+
276+
$this->config->expects($this->exactly(1))
277+
->method('getSystemValue')
278+
->willReturn($this->returnCallback(function ($key) use ($allowedClients) {
279+
// Note: \OCP\IConfig::getSystemValue returns either an array or string.
280+
return $key == 'core.login_flow_v2.allowed_user_agents' ? $allowedClients : '';
281+
}));
282+
283+
$this->mapper->expects($this->once())
284+
->method('getByLoginToken')
285+
->willReturn($loginFlowV2);
286+
287+
$result = $this->subjectUnderTest->getByLoginToken('test_token');
288+
289+
$this->assertTrue($result instanceof LoginFlowV2);
290+
$this->assertEquals('Custom Allowed Client Curl Client/1.0', $result->getClientName());
291+
}
292+
240293
/*
241294
* Tests for startLoginFlow
242295
*/

0 commit comments

Comments
 (0)