Skip to content

IONOS(login-flow-v2): reject unknown api clients by user agent #98

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
printminion-co merged 2 commits into from
Feb 7, 2025
Merged

IONOS(login-flow-v2): reject unknown api clients by user agent #98

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a649748
IONOS(login-flow-v2): reject unknown api clients by user agent
printminion-co Jan 30, 2025
759bece
IONOS(config): update submodule (reject unknown api clients by user a…
printminion-co Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion IONOS
Open in desktop
Submodule IONOS updated from 35650b to 9decc9
23 changes: 23 additions & 0 deletions core/Controller/ClientFlowLoginV2Controller.php
View file
Open in desktop
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 @@ -106,6 +107,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 @@ -149,6 +152,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 @@ -185,6 +190,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 @@ -230,6 +237,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 @@ -329,6 +338,7 @@ private function stateTokenForbiddenResponse(): StandaloneTemplateResponse {
/**
* @return LoginFlowV2
* @throws LoginFlowV2NotFoundException
* @throws LoginFlowV2ClientForbiddenException
*/
private function getFlowByLoginToken(): LoginFlowV2 {
$currentToken = $this->session->get(self::TOKEN_NAME);
Expand All @@ -352,6 +362,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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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() {
$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() {
$this->session->method('get')
->with('client.flow.v2.login.token')
Expand Down Expand Up @@ -206,6 +229,29 @@ public function testGrantPageInvalidLoginToken() {
$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() {
$this->session->method('get')
->willReturnCallback(function ($name) {
Expand Down Expand Up @@ -266,6 +312,29 @@ public function testGenerateAppPassworInvalidLoginToken() {
$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() {
$this->session->method('get')
->willReturnCallback(function ($name) {
Expand Down
52 changes: 52 additions & 0 deletions tests/Core/Service/LoginFlowV2ServiceUnitTest.php
View file
Open in desktop
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() {
$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
Loading