Skip to content

Commit 0224efb

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

File tree

16 files changed

+258
-17
lines changed

16 files changed

+258
-17
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@
6767
$bearerAuthPlugin = new \OCA\DAV\Connector\Sabre\BearerAuth(
6868
\OC::$server->getUserSession(),
6969
\OC::$server->getSession(),
70-
\OC::$server->getRequest()
70+
\OC::$server->getRequest(),
71+
\OC::$server->getConfig(),
7172
);
7273
$authPlugin->addBackend($bearerAuthPlugin);
7374

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
*/
2424
namespace OCA\DAV\Connector\Sabre;
2525

26+
use OCP\IConfig;
2627
use OCP\IRequest;
2728
use OCP\ISession;
2829
use OCP\IUserSession;
@@ -34,15 +35,18 @@ class BearerAuth extends AbstractBearer {
3435
private IUserSession $userSession;
3536
private ISession $session;
3637
private IRequest $request;
38+
private IConfig $config;
3739
private string $principalPrefix;
3840

3941
public function __construct(IUserSession $userSession,
4042
ISession $session,
4143
IRequest $request,
44+
IConfig $config,
4245
$principalPrefix = 'principals/users/') {
4346
$this->userSession = $userSession;
4447
$this->session = $session;
4548
$this->request = $request;
49+
$this->config = $config;
4650
$this->principalPrefix = $principalPrefix;
4751

4852
// setup realm
@@ -81,6 +85,14 @@ public function validateBearerToken($bearerToken) {
8185
* @param ResponseInterface $response
8286
*/
8387
public function challenge(RequestInterface $request, ResponseInterface $response): void {
88+
// Legacy ownCloud clients still authenticate via OAuth2
89+
$enableOcClients = $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
90+
$userAgent = $request->getHeader('User-Agent');
91+
if ($enableOcClients && $userAgent !== null && str_contains($userAgent, 'mirall')) {
92+
parent::challenge($request, $response);
93+
return;
94+
}
95+
8496
$response->setStatus(401);
8597
}
8698
}

‎apps/dav/lib/Server.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,8 @@ public function __construct(IRequest $request, string $baseUri) {
153153
$bearerAuthBackend = new BearerAuth(
154154
\OC::$server->getUserSession(),
155155
\OC::$server->getSession(),
156-
\OC::$server->getRequest()
156+
\OC::$server->getRequest(),
157+
\OC::$server->getConfig(),
157158
);
158159
$authPlugin->addBackend($bearerAuthBackend);
159160
// 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
@@ -25,10 +25,12 @@
2525
namespace OCA\DAV\Tests\unit\Connector\Sabre;
2626

2727
use OCA\DAV\Connector\Sabre\BearerAuth;
28+
use OCP\IConfig;
2829
use OCP\IRequest;
2930
use OCP\ISession;
3031
use OCP\IUser;
3132
use OCP\IUserSession;
33+
use PHPUnit\Framework\MockObject\MockObject;
3234
use Sabre\HTTP\RequestInterface;
3335
use Sabre\HTTP\ResponseInterface;
3436
use Test\TestCase;
@@ -46,17 +48,21 @@ class BearerAuthTest extends TestCase {
4648
/** @var BearerAuth */
4749
private $bearerAuth;
4850

51+
private IConfig&MockObject $config;
52+
4953
protected function setUp(): void {
5054
parent::setUp();
5155

5256
$this->userSession = $this->createMock(\OC\User\Session::class);
5357
$this->session = $this->createMock(ISession::class);
5458
$this->request = $this->createMock(IRequest::class);
59+
$this->config = $this->createMock(IConfig::class);
5560

5661
$this->bearerAuth = new BearerAuth(
5762
$this->userSession,
5863
$this->session,
59-
$this->request
64+
$this->request,
65+
$this->config,
6066
);
6167
}
6268

‎apps/oauth2/appinfo/info.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@
2929
</post-migration>
3030
</repair-steps>
3131

32+
<commands>
33+
<command>OCA\OAuth2\Command\ImportLegacyOcClient</command>
34+
</commands>
35+
3236
<settings>
3337
<admin>OCA\OAuth2\Settings\Admin</admin>
3438
</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
@@ -34,6 +34,7 @@
3434
use OCP\AppFramework\Http;
3535
use OCP\AppFramework\Http\RedirectResponse;
3636
use OCP\AppFramework\Http\TemplateResponse;
37+
use OCP\IConfig;
3738
use OCP\IL10N;
3839
use OCP\IRequest;
3940
use OCP\ISession;
@@ -48,6 +49,7 @@ class LoginRedirectorController extends Controller {
4849
private $session;
4950
/** @var IL10N */
5051
private $l;
52+
private IConfig $config;
5153

5254
/**
5355
* @param string $appName
@@ -56,18 +58,21 @@ class LoginRedirectorController extends Controller {
5658
* @param ClientMapper $clientMapper
5759
* @param ISession $session
5860
* @param IL10N $l
61+
* @param IConfig $l
5962
*/
6063
public function __construct(string $appName,
6164
IRequest $request,
6265
IURLGenerator $urlGenerator,
6366
ClientMapper $clientMapper,
6467
ISession $session,
65-
IL10N $l) {
68+
IL10N $l,
69+
IConfig $config) {
6670
parent::__construct($appName, $request);
6771
$this->urlGenerator = $urlGenerator;
6872
$this->clientMapper = $clientMapper;
6973
$this->session = $session;
7074
$this->l = $l;
75+
$this->config = $config;
7176
}
7277

7378
/**
@@ -80,14 +85,16 @@ public function __construct(string $appName,
8085
* @param string $client_id Client ID
8186
* @param string $state State of the flow
8287
* @param string $response_type Response type for the flow
88+
* @param string $redirect_uri URI to redirect to after the flow (is only used for legacy ownCloud clients)
8389
* @return TemplateResponse<Http::STATUS_OK, array{}>|RedirectResponse<Http::STATUS_SEE_OTHER, array{}>
8490
*
8591
* 200: Client not found
8692
* 303: Redirect to login URL
8793
*/
8894
public function authorize($client_id,
8995
$state,
90-
$response_type): TemplateResponse|RedirectResponse {
96+
$response_type,
97+
string $redirect_uri = ''): TemplateResponse|RedirectResponse {
9198
try {
9299
$client = $this->clientMapper->getByIdentifier($client_id);
93100
} catch (ClientNotFoundException $e) {
@@ -103,12 +110,20 @@ public function authorize($client_id,
103110
return new RedirectResponse($url);
104111
}
105112

113+
$enableOcClients = $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
114+
115+
$providedRedirectUri = '';
116+
if ($enableOcClients && $client->getRedirectUri() === 'http://localhost:*') {
117+
$providedRedirectUri = $redirect_uri;
118+
}
119+
106120
$this->session->set('oauth.state', $state);
107121

108122
$targetUrl = $this->urlGenerator->linkToRouteAbsolute(
109123
'core.ClientFlowLogin.showAuthPickerPage',
110124
[
111125
'clientIdentifier' => $client->getClientIdentifier(),
126+
'providedRedirectUri' => $providedRedirectUri,
112127
]
113128
);
114129
return new RedirectResponse($targetUrl);

‎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)