Skip to content

Commit 15e9f45

Browse files
st3inyst3iny
committed
fix(oauth2): retain support for legacy ownCloud clients
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
1 parent c50698a commit 15e9f45

File tree

16 files changed

+254
-18
lines changed

16 files changed

+254
-18
lines changed

‎apps/dav/appinfo/v1/webdav.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@
4444
$bearerAuthPlugin = new \OCA\DAV\Connector\Sabre\BearerAuth(
4545
\OC::$server->getUserSession(),
4646
\OC::$server->getSession(),
47-
\OC::$server->getRequest()
47+
\OC::$server->getRequest(),
48+
\OC::$server->getConfig(),
4849
);
4950
$authPlugin->addBackend($bearerAuthPlugin);
5051

‎apps/dav/lib/Connector/Sabre/BearerAuth.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
namespace OCA\DAV\Connector\Sabre;
77

8+
use OCP\IConfig;
89
use OCP\IRequest;
910
use OCP\ISession;
1011
use OCP\IUserSession;
@@ -16,15 +17,18 @@ class BearerAuth extends AbstractBearer {
1617
private IUserSession $userSession;
1718
private ISession $session;
1819
private IRequest $request;
20+
private IConfig $config;
1921
private string $principalPrefix;
2022

2123
public function __construct(IUserSession $userSession,
2224
ISession $session,
2325
IRequest $request,
26+
IConfig $config,
2427
$principalPrefix = 'principals/users/') {
2528
$this->userSession = $userSession;
2629
$this->session = $session;
2730
$this->request = $request;
31+
$this->config = $config;
2832
$this->principalPrefix = $principalPrefix;
2933

3034
// setup realm
@@ -63,6 +67,14 @@ public function validateBearerToken($bearerToken) {
6367
* @param ResponseInterface $response
6468
*/
6569
public function challenge(RequestInterface $request, ResponseInterface $response): void {
70+
// Legacy ownCloud clients still authenticate via OAuth2
71+
$enableOcClients = $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
72+
$userAgent = $request->getHeader('User-Agent');
73+
if ($enableOcClients && $userAgent !== null && str_contains($userAgent, 'mirall')) {
74+
parent::challenge($request, $response);
75+
return;
76+
}
77+
6678
$response->setStatus(401);
6779
}
6880
}

‎apps/dav/lib/Server.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,8 @@ public function __construct(IRequest $request, string $baseUri) {
128128
$bearerAuthBackend = new BearerAuth(
129129
\OC::$server->getUserSession(),
130130
\OC::$server->getSession(),
131-
\OC::$server->getRequest()
131+
\OC::$server->getRequest(),
132+
\OC::$server->getConfig(),
132133
);
133134
$authPlugin->addBackend($bearerAuthBackend);
134135
// because we are throwing exceptions this plugin has to be the last one

‎apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@
66
namespace OCA\DAV\Tests\unit\Connector\Sabre;
77

88
use OCA\DAV\Connector\Sabre\BearerAuth;
9+
use OCP\IConfig;
910
use OCP\IRequest;
1011
use OCP\ISession;
1112
use OCP\IUser;
1213
use OCP\IUserSession;
14+
use PHPUnit\Framework\MockObject\MockObject;
1315
use Sabre\HTTP\RequestInterface;
1416
use Sabre\HTTP\ResponseInterface;
1517
use Test\TestCase;
@@ -27,17 +29,21 @@ class BearerAuthTest extends TestCase {
2729
/** @var BearerAuth */
2830
private $bearerAuth;
2931

32+
private IConfig&MockObject $config;
33+
3034
protected function setUp(): void {
3135
parent::setUp();
3236

3337
$this->userSession = $this->createMock(\OC\User\Session::class);
3438
$this->session = $this->createMock(ISession::class);
3539
$this->request = $this->createMock(IRequest::class);
40+
$this->config = $this->createMock(IConfig::class);
3641

3742
$this->bearerAuth = new BearerAuth(
3843
$this->userSession,
3944
$this->session,
40-
$this->request
45+
$this->request,
46+
$this->config,
4147
);
4248
}
4349

‎apps/oauth2/appinfo/info.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@
3333
</post-migration>
3434
</repair-steps>
3535

36+
<commands>
37+
<command>OCA\OAuth2\Command\ImportLegacyOcClient</command>
38+
</commands>
39+
3640
<settings>
3741
<admin>OCA\OAuth2\Settings\Admin</admin>
3842
</settings>

‎apps/oauth2/composer/composer/autoload_classmap.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
return array(
99
'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php',
1010
'OCA\\OAuth2\\BackgroundJob\\CleanupExpiredAuthorizationCode' => $baseDir . '/../lib/BackgroundJob/CleanupExpiredAuthorizationCode.php',
11+
'OCA\\OAuth2\\Command\\ImportLegacyOcClient' => $baseDir . '/../lib/Command/ImportLegacyOcClient.php',
1112
'OCA\\OAuth2\\Controller\\LoginRedirectorController' => $baseDir . '/../lib/Controller/LoginRedirectorController.php',
1213
'OCA\\OAuth2\\Controller\\OauthApiController' => $baseDir . '/../lib/Controller/OauthApiController.php',
1314
'OCA\\OAuth2\\Controller\\SettingsController' => $baseDir . '/../lib/Controller/SettingsController.php',

‎apps/oauth2/composer/composer/autoload_static.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ class ComposerStaticInitOAuth2
2323
public static $classMap = array (
2424
'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php',
2525
'OCA\\OAuth2\\BackgroundJob\\CleanupExpiredAuthorizationCode' => __DIR__ . '/..' . '/../lib/BackgroundJob/CleanupExpiredAuthorizationCode.php',
26+
'OCA\\OAuth2\\Command\\ImportLegacyOcClient' => __DIR__ . '/..' . '/../lib/Command/ImportLegacyOcClient.php',
2627
'OCA\\OAuth2\\Controller\\LoginRedirectorController' => __DIR__ . '/..' . '/../lib/Controller/LoginRedirectorController.php',
2728
'OCA\\OAuth2\\Controller\\OauthApiController' => __DIR__ . '/..' . '/../lib/Controller/OauthApiController.php',
2829
'OCA\\OAuth2\\Controller\\SettingsController' => __DIR__ . '/..' . '/../lib/Controller/SettingsController.php',

‎apps/oauth2/lib/Command/ImportLegacyOcClient.php‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
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+
10+
namespace OCA\OAuth2\Command;
11+
12+
use OCA\OAuth2\Db\Client;
13+
use OCA\OAuth2\Db\ClientMapper;
14+
use OCP\IConfig;
15+
use OCP\Security\ICrypto;
16+
use Symfony\Component\Console\Command\Command;
17+
use Symfony\Component\Console\Input\InputArgument;
18+
use Symfony\Component\Console\Input\InputInterface;
19+
use Symfony\Component\Console\Output\OutputInterface;
20+
21+
class ImportLegacyOcClient extends Command {
22+
private const ARGUMENT_CLIENT_ID = 'client-id';
23+
private const ARGUMENT_CLIENT_SECRET = 'client-secret';
24+
25+
public function __construct(
26+
private readonly IConfig $config,
27+
private readonly ICrypto $crypto,
28+
private readonly ClientMapper $clientMapper,
29+
) {
30+
parent::__construct();
31+
}
32+
33+
protected function configure(): void {
34+
$this->setName('oauth2:import-legacy-oc-client');
35+
$this->setDescription('This command is only required to be run on instances which were migrated from ownCloud without the oauth2.enable_oc_clients system config! Import a legacy Oauth2 client from an ownCloud instance and migrate it. The data is expected to be straight out of the database table oc_oauth2_clients.');
36+
$this->addArgument(
37+
self::ARGUMENT_CLIENT_ID,
38+
InputArgument::REQUIRED,
39+
'Value of the "identifier" column',
40+
);
41+
$this->addArgument(
42+
self::ARGUMENT_CLIENT_SECRET,
43+
InputArgument::REQUIRED,
44+
'Value of the "secret" column',
45+
);
46+
}
47+
48+
public function isEnabled(): bool {
49+
return $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
50+
}
51+
52+
protected function execute(InputInterface $input, OutputInterface $output): int {
53+
/** @var string $clientId */
54+
$clientId = $input->getArgument(self::ARGUMENT_CLIENT_ID);
55+
56+
/** @var string $clientSecret */
57+
$clientSecret = $input->getArgument(self::ARGUMENT_CLIENT_SECRET);
58+
59+
// Should not happen but just to be sure
60+
if (empty($clientId) || empty($clientSecret)) {
61+
return 1;
62+
}
63+
64+
$hashedClientSecret = bin2hex($this->crypto->calculateHMAC($clientSecret));
65+
66+
$client = new Client();
67+
$client->setName('ownCloud Desktop Client');
68+
$client->setRedirectUri('http://localhost:*');
69+
$client->setClientIdentifier($clientId);
70+
$client->setSecret($hashedClientSecret);
71+
$this->clientMapper->insert($client);
72+
73+
$output->writeln('<info>Client imported successfully</info>');
74+
return 0;
75+
}
76+
}

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use OCP\AppFramework\Http\Attribute\UseSession;
1818
use OCP\AppFramework\Http\RedirectResponse;
1919
use OCP\AppFramework\Http\TemplateResponse;
20+
use OCP\IConfig;
2021
use OCP\IL10N;
2122
use OCP\IRequest;
2223
use OCP\ISession;
@@ -31,6 +32,8 @@ class LoginRedirectorController extends Controller {
3132
private $session;
3233
/** @var IL10N */
3334
private $l;
35+
/** @var IConfig */
36+
private $config;
3437

3538
/**
3639
* @param string $appName
@@ -39,18 +42,21 @@ class LoginRedirectorController extends Controller {
3942
* @param ClientMapper $clientMapper
4043
* @param ISession $session
4144
* @param IL10N $l
45+
* @param IConfig $config
4246
*/
4347
public function __construct(string $appName,
4448
IRequest $request,
4549
IURLGenerator $urlGenerator,
4650
ClientMapper $clientMapper,
4751
ISession $session,
48-
IL10N $l) {
52+
IL10N $l,
53+
IConfig $config) {
4954
parent::__construct($appName, $request);
5055
$this->urlGenerator = $urlGenerator;
5156
$this->clientMapper = $clientMapper;
5257
$this->session = $session;
5358
$this->l = $l;
59+
$this->config = $config;
5460
}
5561

5662
/**
@@ -59,6 +65,7 @@ public function __construct(string $appName,
5965
* @param string $client_id Client ID
6066
* @param string $state State of the flow
6167
* @param string $response_type Response type for the flow
68+
* @param string $redirect_uri URI to redirect to after the flow (is only used for legacy ownCloud clients)
6269
* @return TemplateResponse<Http::STATUS_OK, array{}>|RedirectResponse<Http::STATUS_SEE_OTHER, array{}>
6370
*
6471
* 200: Client not found
@@ -69,7 +76,8 @@ public function __construct(string $appName,
6976
#[UseSession]
7077
public function authorize($client_id,
7178
$state,
72-
$response_type): TemplateResponse|RedirectResponse {
79+
$response_type,
80+
string $redirect_uri = ''): TemplateResponse|RedirectResponse {
7381
try {
7482
$client = $this->clientMapper->getByIdentifier($client_id);
7583
} catch (ClientNotFoundException $e) {
@@ -85,6 +93,13 @@ public function authorize($client_id,
8593
return new RedirectResponse($url);
8694
}
8795

96+
$enableOcClients = $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
97+
98+
$providedRedirectUri = '';
99+
if ($enableOcClients && $client->getRedirectUri() === 'http://localhost:*') {
100+
$providedRedirectUri = $redirect_uri;
101+
}
102+
88103
$this->session->set('oauth.state', $state);
89104

90105
$targetUrl = $this->urlGenerator->linkToRouteAbsolute(

‎apps/oauth2/openapi.json‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,15 @@
6565
"schema": {
6666
"type": "string"
6767
}
68+
},
69+
{
70+
"name": "redirect_uri",
71+
"in": "query",
72+
"description": "URI to redirect to after the flow (is only used for legacy ownCloud clients)",
73+
"schema": {
74+
"type": "string",
75+
"default": ""
76+
}
6877
}
6978
],
7079
"responses": {

0 commit comments

Comments
 (0)