Skip to content

Commit

Permalink
feat(login-flow-v2): Restrict allowed apps by user agent check
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
printminion-co committed Feb 4, 2025
1 parent c82bc0a commit b467dcc
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 1 deletion.
23 changes: 23 additions & 0 deletions core/Controller/ClientFlowLoginV2Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use OC\Core\Db\LoginFlowV2;
use OC\Core\Exception\LoginFlowV2NotFoundException;
use OC\Core\Exception\LoginFlowV2ClientForbiddenException;
use OC\Core\ResponseDefinitions;
use OC\Core\Service\LoginFlowV2Service;
use OCP\AppFramework\Controller;
Expand Down Expand Up @@ -107,6 +108,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(
Expand Down Expand Up @@ -150,6 +153,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 */
Expand Down Expand Up @@ -186,6 +191,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);
Expand Down Expand Up @@ -231,6 +238,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);
Expand Down Expand Up @@ -331,6 +340,7 @@ private function stateTokenForbiddenResponse(): StandaloneTemplateResponse {
/**
* @return LoginFlowV2
* @throws LoginFlowV2NotFoundException
* @throws LoginFlowV2ClientForbiddenException
*/
private function getFlowByLoginToken(): LoginFlowV2 {
$currentToken = $this->session->get(self::TOKEN_NAME);
Expand All @@ -354,6 +364,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 = '';

Expand Down
12 changes: 12 additions & 0 deletions core/Exception/LoginFlowV2ClientForbiddenException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2025 STRATO GmbH
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OC\Core\Exception;

class LoginFlowV2ClientForbiddenException extends \Exception {
}
23 changes: 22 additions & 1 deletion core/Service/LoginFlowV2Service.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use OC\Core\Data\LoginFlowV2Tokens;
use OC\Core\Db\LoginFlowV2;
use OC\Core\Db\LoginFlowV2Mapper;
use OC\Core\Exception\LoginFlowV2ClientForbiddenException;
use OC\Core\Exception\LoginFlowV2NotFoundException;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\AppFramework\Utility\ITimeFactory;
Expand Down Expand Up @@ -74,13 +75,33 @@ public function poll(string $pollToken): LoginFlowV2Credentials {
* @param string $loginToken
* @return LoginFlowV2
* @throws LoginFlowV2NotFoundException
* @throws LoginFlowV2ClientForbiddenException
*/
public function getByLoginToken(string $loginToken): LoginFlowV2 {
/** @var LoginFlowV2|null $flow */
$flow = null;

try {
return $this->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');
}

/**
Expand Down
69 changes: 69 additions & 0 deletions tests/Core/Controller/ClientFlowLoginV2ControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
52 changes: 52 additions & 0 deletions tests/Core/Service/LoginFlowV2ServiceUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use OC\Core\Data\LoginFlowV2Tokens;
use OC\Core\Db\LoginFlowV2;
use OC\Core\Db\LoginFlowV2Mapper;
use OC\Core\Exception\LoginFlowV2ClientForbiddenException;
use OC\Core\Exception\LoginFlowV2NotFoundException;
use OC\Core\Service\LoginFlowV2Service;
use OCP\AppFramework\Db\DoesNotExistException;
Expand Down Expand Up @@ -237,6 +238,57 @@ public function testGetByLoginTokenLoginTokenInvalid(): void {
$this->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
*/
Expand Down

0 comments on commit b467dcc

Please sign in to comment.