diff --git a/config/config.sample.php b/config/config.sample.php index ddc0deb79b904..ddd91d84a3efe 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -2515,6 +2515,20 @@ '/^Microsoft-WebDAV-MiniRedir/', // Windows webdav drive ], +/** + * This option allows you to specify a list of allowed user agents for the Login Flow V2. + * If a user agent is not in this list, it will not be allowed to use the Login Flow V2. + * The user agents are defined using regular expressions. + * + * WARNING: only use this if you know what you are doing + * + * Example: Allow only the Nextcloud Android app to use the Login Flow V2 + * 'core.login_flow_v2.allowed_user_agents' => ['/Nextcloud-android/i'], + * + * Defaults to an empty array. + */ +'core.login_flow_v2.allowed_user_agents' => [], + /** * By default, there is on public pages a link shown that allows users to * learn about the "simple sign up" - see https://nextcloud.com/signup/ diff --git a/core/Controller/ClientFlowLoginV2Controller.php b/core/Controller/ClientFlowLoginV2Controller.php index 1ce43c1993222..b4a7622161fe8 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -9,6 +9,7 @@ namespace OC\Core\Controller; use OC\Core\Db\LoginFlowV2; +use OC\Core\Exception\LoginFlowV2ClientForbiddenException; use OC\Core\Exception\LoginFlowV2NotFoundException; use OC\Core\ResponseDefinitions; use OC\Core\Service\LoginFlowV2Service; @@ -109,6 +110,8 @@ public function showAuthPickerPage(string $user = '', int $direct = 0): Standalo $flow = $this->getFlowByLoginToken(); } catch (LoginFlowV2NotFoundException $e) { return $this->loginTokenForbiddenResponse(); + } catch (LoginFlowV2ClientForbiddenException $e) { + return $this->loginTokenForbiddenClientResponse(); } $stateToken = $this->random->generate( @@ -152,6 +155,8 @@ public function grantPage(?string $stateToken, int $direct = 0): StandaloneTempl $flow = $this->getFlowByLoginToken(); } catch (LoginFlowV2NotFoundException $e) { return $this->loginTokenForbiddenResponse(); + } catch (LoginFlowV2ClientForbiddenException $e) { + return $this->loginTokenForbiddenClientResponse(); } /** @var IUser $user */ @@ -188,6 +193,8 @@ public function apptokenRedirect(?string $stateToken, string $user, string $pass $this->getFlowByLoginToken(); } catch (LoginFlowV2NotFoundException $e) { return $this->loginTokenForbiddenResponse(); + } catch (LoginFlowV2ClientForbiddenException $e) { + return $this->loginTokenForbiddenClientResponse(); } $loginToken = $this->session->get(self::TOKEN_NAME); @@ -233,6 +240,8 @@ public function generateAppPassword(?string $stateToken): Response { $this->getFlowByLoginToken(); } catch (LoginFlowV2NotFoundException $e) { return $this->loginTokenForbiddenResponse(); + } catch (LoginFlowV2ClientForbiddenException $e) { + return $this->loginTokenForbiddenClientResponse(); } $loginToken = $this->session->get(self::TOKEN_NAME); @@ -333,6 +342,7 @@ private function stateTokenForbiddenResponse(): StandaloneTemplateResponse { /** * @return LoginFlowV2 * @throws LoginFlowV2NotFoundException + * @throws LoginFlowV2ClientForbiddenException */ private function getFlowByLoginToken(): LoginFlowV2 { $currentToken = $this->session->get(self::TOKEN_NAME); @@ -356,6 +366,19 @@ private function loginTokenForbiddenResponse(): StandaloneTemplateResponse { return $response; } + private function loginTokenForbiddenClientResponse(): StandaloneTemplateResponse { + $response = new StandaloneTemplateResponse( + $this->appName, + '403', + [ + 'message' => $this->l10n->t('Please use original client'), + ], + 'guest' + ); + $response->setStatus(Http::STATUS_FORBIDDEN); + return $response; + } + private function getServerPath(): string { $serverPostfix = ''; diff --git a/core/Exception/LoginFlowV2ClientForbiddenException.php b/core/Exception/LoginFlowV2ClientForbiddenException.php new file mode 100644 index 0000000000000..e6ac6d9618ffe --- /dev/null +++ b/core/Exception/LoginFlowV2ClientForbiddenException.php @@ -0,0 +1,12 @@ +mapper->getByLoginToken($loginToken); + $flow = $this->mapper->getByLoginToken($loginToken); } catch (DoesNotExistException $e) { throw new LoginFlowV2NotFoundException('Login token invalid'); } + + $allowedAgents = $this->config->getSystemValue('core.login_flow_v2.allowed_user_agents', []); + + if (empty($allowedAgents)) { + return $flow; + } + + $flowClient = $flow->getClientName(); + + foreach ($allowedAgents as $allowedAgent) { + if (preg_match($allowedAgent, $flowClient) === 1) { + return $flow; + } + } + + throw new LoginFlowV2ClientForbiddenException('Client not allowed'); } /** diff --git a/lib/composer/autoload.php b/lib/composer/autoload.php index 7b1481e876c13..b3b39129e7a8c 100644 --- a/lib/composer/autoload.php +++ b/lib/composer/autoload.php @@ -14,7 +14,10 @@ echo $err; } } - throw new RuntimeException($err); + trigger_error( + $err, + E_USER_ERROR + ); } require_once __DIR__ . '/composer/autoload_real.php'; diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 1f3d9d3813bc0..3d36f50e9501d 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1371,6 +1371,7 @@ 'OC\\Core\\Db\\ProfileConfigMapper' => $baseDir . '/core/Db/ProfileConfigMapper.php', 'OC\\Core\\Events\\BeforePasswordResetEvent' => $baseDir . '/core/Events/BeforePasswordResetEvent.php', 'OC\\Core\\Events\\PasswordResetEvent' => $baseDir . '/core/Events/PasswordResetEvent.php', + 'OC\\Core\\Exception\\LoginFlowV2ClientForbiddenException' => $baseDir . '/core/Exception/LoginFlowV2ClientForbiddenException.php', 'OC\\Core\\Exception\\LoginFlowV2NotFoundException' => $baseDir . '/core/Exception/LoginFlowV2NotFoundException.php', 'OC\\Core\\Exception\\ResetPasswordException' => $baseDir . '/core/Exception/ResetPasswordException.php', 'OC\\Core\\Listener\\BeforeMessageLoggedEventListener' => $baseDir . '/core/Listener/BeforeMessageLoggedEventListener.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 8c87d90156dfe..7085d0b4a44ac 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1412,6 +1412,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Db\\ProfileConfigMapper' => __DIR__ . '/../../..' . '/core/Db/ProfileConfigMapper.php', 'OC\\Core\\Events\\BeforePasswordResetEvent' => __DIR__ . '/../../..' . '/core/Events/BeforePasswordResetEvent.php', 'OC\\Core\\Events\\PasswordResetEvent' => __DIR__ . '/../../..' . '/core/Events/PasswordResetEvent.php', + 'OC\\Core\\Exception\\LoginFlowV2ClientForbiddenException' => __DIR__ . '/../../..' . '/core/Exception/LoginFlowV2ClientForbiddenException.php', 'OC\\Core\\Exception\\LoginFlowV2NotFoundException' => __DIR__ . '/../../..' . '/core/Exception/LoginFlowV2NotFoundException.php', 'OC\\Core\\Exception\\ResetPasswordException' => __DIR__ . '/../../..' . '/core/Exception/ResetPasswordException.php', 'OC\\Core\\Listener\\BeforeMessageLoggedEventListener' => __DIR__ . '/../../..' . '/core/Listener/BeforeMessageLoggedEventListener.php', diff --git a/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php b/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php index 98c7821791db6..81de4cd92da8b 100644 --- a/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php +++ b/tests/Core/Controller/ClientFlowLoginV2ControllerTest.php @@ -11,6 +11,7 @@ use OC\Core\Controller\ClientFlowLoginV2Controller; use OC\Core\Data\LoginFlowV2Credentials; use OC\Core\Db\LoginFlowV2; +use OC\Core\Exception\LoginFlowV2ClientForbiddenException; use OC\Core\Exception\LoginFlowV2NotFoundException; use OC\Core\Service\LoginFlowV2Service; use OCP\AppFramework\Http; @@ -56,6 +57,12 @@ protected function setUp(): void { $this->random = $this->createMock(ISecureRandom::class); $this->defaults = $this->createMock(Defaults::class); $this->l = $this->createMock(IL10N::class); + $this->l + ->expects($this->any()) + ->method('t') + ->willReturnCallback(function ($text, $parameters = []) { + return vsprintf($text, $parameters); + }); $this->controller = new ClientFlowLoginV2Controller( 'core', $this->request, @@ -150,6 +157,22 @@ public function testShowAuthPickerInvalidLoginToken(): void { $this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus()); } + public function testShowAuthPickerForbiddenUserClient() { + $this->session->method('get') + ->with('client.flow.v2.login.token') + ->willReturn('loginToken'); + + $this->loginFlowV2Service->method('getByLoginToken') + ->with('loginToken') + ->willThrowException(new LoginFlowV2ClientForbiddenException()); + + $result = $this->controller->showAuthPickerPage(); + + $this->assertInstanceOf(Http\StandaloneTemplateResponse::class, $result); + $this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus()); + $this->assertSame('Please use original client', $result->getParams()['message']); + } + public function testShowAuthPickerValidLoginToken(): void { $this->session->method('get') ->with('client.flow.v2.login.token') @@ -206,6 +229,29 @@ public function testGrantPageInvalidLoginToken(): void { $this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus()); } + public function testGrantPageForbiddenUserClient() { + $this->session->method('get') + ->willReturnCallback(function ($name) { + if ($name === 'client.flow.v2.state.token') { + return 'stateToken'; + } + if ($name === 'client.flow.v2.login.token') { + return 'loginToken'; + } + return null; + }); + + $this->loginFlowV2Service->method('getByLoginToken') + ->with('loginToken') + ->willThrowException(new LoginFlowV2ClientForbiddenException()); + + $result = $this->controller->grantPage('stateToken'); + + $this->assertInstanceOf(Http\StandaloneTemplateResponse::class, $result); + $this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus()); + $this->assertSame('Please use original client', $result->getParams()['message']); + } + public function testGrantPageValid(): void { $this->session->method('get') ->willReturnCallback(function ($name) { @@ -266,6 +312,29 @@ public function testGenerateAppPassworInvalidLoginToken(): void { $this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus()); } + public function testGenerateAppPasswordForbiddenUserClient() { + $this->session->method('get') + ->willReturnCallback(function ($name) { + if ($name === 'client.flow.v2.state.token') { + return 'stateToken'; + } + if ($name === 'client.flow.v2.login.token') { + return 'loginToken'; + } + return null; + }); + + $this->loginFlowV2Service->method('getByLoginToken') + ->with('loginToken') + ->willThrowException(new LoginFlowV2ClientForbiddenException()); + + $result = $this->controller->generateAppPassword('stateToken'); + + $this->assertInstanceOf(Http\StandaloneTemplateResponse::class, $result); + $this->assertSame(Http::STATUS_FORBIDDEN, $result->getStatus()); + $this->assertSame('Please use original client', $result->getParams()['message']); + } + public function testGenerateAppPassworValid(): void { $this->session->method('get') ->willReturnCallback(function ($name) { diff --git a/tests/Core/Service/LoginFlowV2ServiceUnitTest.php b/tests/Core/Service/LoginFlowV2ServiceUnitTest.php index 1c31efee8f7c0..8118106c7228e 100644 --- a/tests/Core/Service/LoginFlowV2ServiceUnitTest.php +++ b/tests/Core/Service/LoginFlowV2ServiceUnitTest.php @@ -1,4 +1,5 @@ subjectUnderTest->getByLoginToken('test_token'); } + public function testGetByLoginTokenClientForbidden() { + $this->expectException(LoginFlowV2ClientForbiddenException::class); + $this->expectExceptionMessage('Client not allowed'); + + $allowedClients = [ + '/Custom Allowed Client/i' + ]; + + $this->config->expects($this->exactly(1)) + ->method('getSystemValue') + ->willReturn($this->returnCallback(function ($key) use ($allowedClients) { + // Note: \OCP\IConfig::getSystemValue returns either an array or string. + return $key == 'core.login_flow_v2.allowed_user_agents' ? $allowedClients : ''; + })); + + $loginFlowV2 = new LoginFlowV2(); + $loginFlowV2->setClientName('Rogue Curl Client/1.0'); + + $this->mapper->expects($this->once()) + ->method('getByLoginToken') + ->willReturn($loginFlowV2); + + $this->subjectUnderTest->getByLoginToken('test_token'); + } + + public function testGetByLoginTokenClientAllowed() { + $allowedClients = [ + '/Foo Allowed Client/i', + '/Custom Allowed Client/i' + ]; + + $loginFlowV2 = new LoginFlowV2(); + $loginFlowV2->setClientName('Custom Allowed Client Curl Client/1.0'); + + $this->config->expects($this->exactly(1)) + ->method('getSystemValue') + ->willReturn($this->returnCallback(function ($key) use ($allowedClients) { + // Note: \OCP\IConfig::getSystemValue returns either an array or string. + return $key == 'core.login_flow_v2.allowed_user_agents' ? $allowedClients : ''; + })); + + $this->mapper->expects($this->once()) + ->method('getByLoginToken') + ->willReturn($loginFlowV2); + + $result = $this->subjectUnderTest->getByLoginToken('test_token'); + + $this->assertTrue($result instanceof LoginFlowV2); + $this->assertEquals('Custom Allowed Client Curl Client/1.0', $result->getClientName()); + } + /* * Tests for startLoginFlow */