aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRichard Steinmetz <richard@steinmetz.cloud>2025-02-17 14:34:01 +0100
committerRichard Steinmetz <richard@steinmetz.cloud>2025-04-01 11:25:52 +0200
commit246da73a363c11d02eed69e80e76d7c9a9a04c7b (patch)
tree925d2a3109f1cd3327a0721380471c773d7d2b04
parentb03ffab5f0f39139c71cb2b8c370ca3f3d1ad391 (diff)
downloadnextcloud-server-246da73a363c11d02eed69e80e76d7c9a9a04c7b.tar.gz
nextcloud-server-246da73a363c11d02eed69e80e76d7c9a9a04c7b.zip
fix(oauth2): retain support for legacy ownCloud clientsfix/oauth2/retain-legacy-oc-client-support
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
-rw-r--r--apps/dav/appinfo/v1/webdav.php3
-rw-r--r--apps/dav/lib/Connector/Sabre/BearerAuth.php10
-rw-r--r--apps/dav/lib/Server.php3
-rw-r--r--apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php8
-rw-r--r--apps/oauth2/appinfo/info.xml4
-rw-r--r--apps/oauth2/composer/composer/autoload_classmap.php1
-rw-r--r--apps/oauth2/composer/composer/autoload_static.php1
-rw-r--r--apps/oauth2/lib/Command/ImportLegacyOcClient.php76
-rw-r--r--apps/oauth2/lib/Controller/LoginRedirectorController.php15
-rw-r--r--apps/oauth2/openapi.json9
-rw-r--r--apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php84
-rw-r--r--core/Controller/ClientFlowLoginController.php20
-rw-r--r--core/templates/loginflow/authpicker.php2
-rw-r--r--core/templates/loginflow/grant.php1
-rw-r--r--lib/private/Repair/Owncloud/MigrateOauthTables.php21
-rw-r--r--openapi.json9
-rw-r--r--tests/Core/Controller/ClientFlowLoginControllerTest.php10
17 files changed, 264 insertions, 13 deletions
diff --git a/apps/dav/appinfo/v1/webdav.php b/apps/dav/appinfo/v1/webdav.php
index fe47ba74652..baeae66bb20 100644
--- a/apps/dav/appinfo/v1/webdav.php
+++ b/apps/dav/appinfo/v1/webdav.php
@@ -61,7 +61,8 @@ $authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend);
$bearerAuthPlugin = new BearerAuth(
Server::get(IUserSession::class),
Server::get(ISession::class),
- Server::get(IRequest::class)
+ Server::get(IRequest::class),
+ Server::get(IConfig::class),
);
$authPlugin->addBackend($bearerAuthPlugin);
diff --git a/apps/dav/lib/Connector/Sabre/BearerAuth.php b/apps/dav/lib/Connector/Sabre/BearerAuth.php
index 07bb3e052a5..e189d8fa128 100644
--- a/apps/dav/lib/Connector/Sabre/BearerAuth.php
+++ b/apps/dav/lib/Connector/Sabre/BearerAuth.php
@@ -7,6 +7,7 @@ namespace OCA\DAV\Connector\Sabre;
use OCP\AppFramework\Http;
use OCP\Defaults;
+use OCP\IConfig;
use OCP\IRequest;
use OCP\ISession;
use OCP\IUserSession;
@@ -19,6 +20,7 @@ class BearerAuth extends AbstractBearer {
private IUserSession $userSession,
private ISession $session,
private IRequest $request,
+ private IConfig $config,
private string $principalPrefix = 'principals/users/',
) {
// setup realm
@@ -57,6 +59,14 @@ class BearerAuth extends AbstractBearer {
* @param ResponseInterface $response
*/
public function challenge(RequestInterface $request, ResponseInterface $response): void {
+ // Legacy ownCloud clients still authenticate via OAuth2
+ $enableOcClients = $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
+ $userAgent = $request->getHeader('User-Agent');
+ if ($enableOcClients && $userAgent !== null && str_contains($userAgent, 'mirall')) {
+ parent::challenge($request, $response);
+ return;
+ }
+
$response->setStatus(Http::STATUS_UNAUTHORIZED);
}
}
diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php
index a14b49c178e..9ea18c029c8 100644
--- a/apps/dav/lib/Server.php
+++ b/apps/dav/lib/Server.php
@@ -157,7 +157,8 @@ class Server {
$bearerAuthBackend = new BearerAuth(
\OCP\Server::get(IUserSession::class),
\OCP\Server::get(ISession::class),
- \OCP\Server::get(IRequest::class)
+ \OCP\Server::get(IRequest::class),
+ \OCP\Server::get(IConfig::class),
);
$authPlugin->addBackend($bearerAuthBackend);
// because we are throwing exceptions this plugin has to be the last one
diff --git a/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php b/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php
index 06c070454af..99c2a461557 100644
--- a/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/BearerAuthTest.php
@@ -7,10 +7,12 @@ namespace OCA\DAV\Tests\unit\Connector\Sabre;
use OC\User\Session;
use OCA\DAV\Connector\Sabre\BearerAuth;
+use OCP\IConfig;
use OCP\IRequest;
use OCP\ISession;
use OCP\IUser;
use OCP\IUserSession;
+use PHPUnit\Framework\MockObject\MockObject;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
use Test\TestCase;
@@ -28,17 +30,21 @@ class BearerAuthTest extends TestCase {
/** @var BearerAuth */
private $bearerAuth;
+ private IConfig&MockObject $config;
+
protected function setUp(): void {
parent::setUp();
$this->userSession = $this->createMock(Session::class);
$this->session = $this->createMock(ISession::class);
$this->request = $this->createMock(IRequest::class);
+ $this->config = $this->createMock(IConfig::class);
$this->bearerAuth = new BearerAuth(
$this->userSession,
$this->session,
- $this->request
+ $this->request,
+ $this->config,
);
}
diff --git a/apps/oauth2/appinfo/info.xml b/apps/oauth2/appinfo/info.xml
index 5e4994fafa8..6ab30292ab0 100644
--- a/apps/oauth2/appinfo/info.xml
+++ b/apps/oauth2/appinfo/info.xml
@@ -33,6 +33,10 @@
</post-migration>
</repair-steps>
+ <commands>
+ <command>OCA\OAuth2\Command\ImportLegacyOcClient</command>
+ </commands>
+
<settings>
<admin>OCA\OAuth2\Settings\Admin</admin>
</settings>
diff --git a/apps/oauth2/composer/composer/autoload_classmap.php b/apps/oauth2/composer/composer/autoload_classmap.php
index cba0ef43dfc..f5fc5bfe251 100644
--- a/apps/oauth2/composer/composer/autoload_classmap.php
+++ b/apps/oauth2/composer/composer/autoload_classmap.php
@@ -8,6 +8,7 @@ $baseDir = $vendorDir;
return array(
'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php',
'OCA\\OAuth2\\BackgroundJob\\CleanupExpiredAuthorizationCode' => $baseDir . '/../lib/BackgroundJob/CleanupExpiredAuthorizationCode.php',
+ 'OCA\\OAuth2\\Command\\ImportLegacyOcClient' => $baseDir . '/../lib/Command/ImportLegacyOcClient.php',
'OCA\\OAuth2\\Controller\\LoginRedirectorController' => $baseDir . '/../lib/Controller/LoginRedirectorController.php',
'OCA\\OAuth2\\Controller\\OauthApiController' => $baseDir . '/../lib/Controller/OauthApiController.php',
'OCA\\OAuth2\\Controller\\SettingsController' => $baseDir . '/../lib/Controller/SettingsController.php',
diff --git a/apps/oauth2/composer/composer/autoload_static.php b/apps/oauth2/composer/composer/autoload_static.php
index c4905bc96c4..c4843fa1990 100644
--- a/apps/oauth2/composer/composer/autoload_static.php
+++ b/apps/oauth2/composer/composer/autoload_static.php
@@ -23,6 +23,7 @@ class ComposerStaticInitOAuth2
public static $classMap = array (
'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php',
'OCA\\OAuth2\\BackgroundJob\\CleanupExpiredAuthorizationCode' => __DIR__ . '/..' . '/../lib/BackgroundJob/CleanupExpiredAuthorizationCode.php',
+ 'OCA\\OAuth2\\Command\\ImportLegacyOcClient' => __DIR__ . '/..' . '/../lib/Command/ImportLegacyOcClient.php',
'OCA\\OAuth2\\Controller\\LoginRedirectorController' => __DIR__ . '/..' . '/../lib/Controller/LoginRedirectorController.php',
'OCA\\OAuth2\\Controller\\OauthApiController' => __DIR__ . '/..' . '/../lib/Controller/OauthApiController.php',
'OCA\\OAuth2\\Controller\\SettingsController' => __DIR__ . '/..' . '/../lib/Controller/SettingsController.php',
diff --git a/apps/oauth2/lib/Command/ImportLegacyOcClient.php b/apps/oauth2/lib/Command/ImportLegacyOcClient.php
new file mode 100644
index 00000000000..acdc57cf991
--- /dev/null
+++ b/apps/oauth2/lib/Command/ImportLegacyOcClient.php
@@ -0,0 +1,76 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\OAuth2\Command;
+
+use OCA\OAuth2\Db\Client;
+use OCA\OAuth2\Db\ClientMapper;
+use OCP\IConfig;
+use OCP\Security\ICrypto;
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputArgument;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+class ImportLegacyOcClient extends Command {
+ private const ARGUMENT_CLIENT_ID = 'client-id';
+ private const ARGUMENT_CLIENT_SECRET = 'client-secret';
+
+ public function __construct(
+ private readonly IConfig $config,
+ private readonly ICrypto $crypto,
+ private readonly ClientMapper $clientMapper,
+ ) {
+ parent::__construct();
+ }
+
+ protected function configure(): void {
+ $this->setName('oauth2:import-legacy-oc-client');
+ $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.');
+ $this->addArgument(
+ self::ARGUMENT_CLIENT_ID,
+ InputArgument::REQUIRED,
+ 'Value of the "identifier" column',
+ );
+ $this->addArgument(
+ self::ARGUMENT_CLIENT_SECRET,
+ InputArgument::REQUIRED,
+ 'Value of the "secret" column',
+ );
+ }
+
+ public function isEnabled(): bool {
+ return $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
+ }
+
+ protected function execute(InputInterface $input, OutputInterface $output): int {
+ /** @var string $clientId */
+ $clientId = $input->getArgument(self::ARGUMENT_CLIENT_ID);
+
+ /** @var string $clientSecret */
+ $clientSecret = $input->getArgument(self::ARGUMENT_CLIENT_SECRET);
+
+ // Should not happen but just to be sure
+ if (empty($clientId) || empty($clientSecret)) {
+ return 1;
+ }
+
+ $hashedClientSecret = bin2hex($this->crypto->calculateHMAC($clientSecret));
+
+ $client = new Client();
+ $client->setName('ownCloud Desktop Client');
+ $client->setRedirectUri('http://localhost:*');
+ $client->setClientIdentifier($clientId);
+ $client->setSecret($hashedClientSecret);
+ $this->clientMapper->insert($client);
+
+ $output->writeln('<info>Client imported successfully</info>');
+ return 0;
+ }
+}
diff --git a/apps/oauth2/lib/Controller/LoginRedirectorController.php b/apps/oauth2/lib/Controller/LoginRedirectorController.php
index db233e7956e..7241b35cdcf 100644
--- a/apps/oauth2/lib/Controller/LoginRedirectorController.php
+++ b/apps/oauth2/lib/Controller/LoginRedirectorController.php
@@ -20,6 +20,7 @@ use OCP\AppFramework\Http\Attribute\UseSession;
use OCP\AppFramework\Http\RedirectResponse;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\IAppConfig;
+use OCP\IConfig;
use OCP\IL10N;
use OCP\IRequest;
use OCP\ISession;
@@ -45,6 +46,7 @@ class LoginRedirectorController extends Controller {
private IL10N $l,
private ISecureRandom $random,
private IAppConfig $appConfig,
+ private IConfig $config,
) {
parent::__construct($appName, $request);
}
@@ -55,6 +57,7 @@ class LoginRedirectorController extends Controller {
* @param string $client_id Client ID
* @param string $state State of the flow
* @param string $response_type Response type for the flow
+ * @param string $redirect_uri URI to redirect to after the flow (is only used for legacy ownCloud clients)
* @return TemplateResponse<Http::STATUS_OK, array{}>|RedirectResponse<Http::STATUS_SEE_OTHER, array{}>
*
* 200: Client not found
@@ -65,7 +68,8 @@ class LoginRedirectorController extends Controller {
#[UseSession]
public function authorize($client_id,
$state,
- $response_type): TemplateResponse|RedirectResponse {
+ $response_type,
+ string $redirect_uri = ''): TemplateResponse|RedirectResponse {
try {
$client = $this->clientMapper->getByIdentifier($client_id);
} catch (ClientNotFoundException $e) {
@@ -81,6 +85,13 @@ class LoginRedirectorController extends Controller {
return new RedirectResponse($url);
}
+ $enableOcClients = $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
+
+ $providedRedirectUri = '';
+ if ($enableOcClients && $client->getRedirectUri() === 'http://localhost:*') {
+ $providedRedirectUri = $redirect_uri;
+ }
+
$this->session->set('oauth.state', $state);
if (in_array($client->getName(), $this->appConfig->getValueArray('oauth2', 'skipAuthPickerApplications', []))) {
@@ -95,6 +106,7 @@ class LoginRedirectorController extends Controller {
[
'stateToken' => $stateToken,
'clientIdentifier' => $client->getClientIdentifier(),
+ 'providedRedirectUri' => $providedRedirectUri,
]
);
} else {
@@ -102,6 +114,7 @@ class LoginRedirectorController extends Controller {
'core.ClientFlowLogin.showAuthPickerPage',
[
'clientIdentifier' => $client->getClientIdentifier(),
+ 'providedRedirectUri' => $providedRedirectUri,
]
);
}
diff --git a/apps/oauth2/openapi.json b/apps/oauth2/openapi.json
index cdf52de3f5e..76cd2da3150 100644
--- a/apps/oauth2/openapi.json
+++ b/apps/oauth2/openapi.json
@@ -65,6 +65,15 @@
"schema": {
"type": "string"
}
+ },
+ {
+ "name": "redirect_uri",
+ "in": "query",
+ "description": "URI to redirect to after the flow (is only used for legacy ownCloud clients)",
+ "schema": {
+ "type": "string",
+ "default": ""
+ }
}
],
"responses": {
diff --git a/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php
index afa5aae4f07..1e090665735 100644
--- a/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php
+++ b/apps/oauth2/tests/Controller/LoginRedirectorControllerTest.php
@@ -13,6 +13,7 @@ use OCA\OAuth2\Exceptions\ClientNotFoundException;
use OCP\AppFramework\Http\RedirectResponse;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\IAppConfig;
+use OCP\IConfig;
use OCP\IL10N;
use OCP\IRequest;
use OCP\ISession;
@@ -32,6 +33,7 @@ class LoginRedirectorControllerTest extends TestCase {
private IL10N&MockObject $l;
private ISecureRandom&MockObject $random;
private IAppConfig&MockObject $appConfig;
+ private IConfig&MockObject $config;
private LoginRedirectorController $loginRedirectorController;
@@ -45,6 +47,7 @@ class LoginRedirectorControllerTest extends TestCase {
$this->l = $this->createMock(IL10N::class);
$this->random = $this->createMock(ISecureRandom::class);
$this->appConfig = $this->createMock(IAppConfig::class);
+ $this->config = $this->createMock(IConfig::class);
$this->loginRedirectorController = new LoginRedirectorController(
'oauth2',
@@ -55,6 +58,7 @@ class LoginRedirectorControllerTest extends TestCase {
$this->l,
$this->random,
$this->appConfig,
+ $this->config,
);
}
@@ -77,9 +81,15 @@ class LoginRedirectorControllerTest extends TestCase {
'core.ClientFlowLogin.showAuthPickerPage',
[
'clientIdentifier' => 'MyClientIdentifier',
+ 'providedRedirectUri' => '',
]
)
->willReturn('https://example.com/?clientIdentifier=foo');
+ $this->config
+ ->expects($this->once())
+ ->method('getSystemValueBool')
+ ->with('oauth2.enable_oc_clients', false)
+ ->willReturn(false);
$expected = new RedirectResponse('https://example.com/?clientIdentifier=foo');
$this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code'));
@@ -124,9 +134,15 @@ class LoginRedirectorControllerTest extends TestCase {
[
'stateToken' => 'MyStateToken',
'clientIdentifier' => 'MyClientIdentifier',
+ 'providedRedirectUri' => '',
]
)
->willReturn('https://example.com/?clientIdentifier=foo');
+ $this->config
+ ->expects($this->once())
+ ->method('getSystemValueBool')
+ ->with('oauth2.enable_oc_clients', false)
+ ->willReturn(false);
$expected = new RedirectResponse('https://example.com/?clientIdentifier=foo');
$this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code'));
@@ -150,6 +166,74 @@ class LoginRedirectorControllerTest extends TestCase {
$this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'wrongcode'));
}
+ public function testAuthorizeWithLegacyOcClient(): void {
+ $client = new Client();
+ $client->setClientIdentifier('MyClientIdentifier');
+ $client->setRedirectUri('http://localhost:*');
+ $this->clientMapper
+ ->expects($this->once())
+ ->method('getByIdentifier')
+ ->with('MyClientId')
+ ->willReturn($client);
+ $this->session
+ ->expects($this->once())
+ ->method('set')
+ ->with('oauth.state', 'MyState');
+ $this->urlGenerator
+ ->expects($this->once())
+ ->method('linkToRouteAbsolute')
+ ->with(
+ 'core.ClientFlowLogin.showAuthPickerPage',
+ [
+ 'clientIdentifier' => 'MyClientIdentifier',
+ 'providedRedirectUri' => 'http://localhost:30000',
+ ]
+ )
+ ->willReturn('https://example.com/?clientIdentifier=foo&providedRedirectUri=http://localhost:30000');
+ $this->config
+ ->expects($this->once())
+ ->method('getSystemValueBool')
+ ->with('oauth2.enable_oc_clients', false)
+ ->willReturn(true);
+
+ $expected = new RedirectResponse('https://example.com/?clientIdentifier=foo&providedRedirectUri=http://localhost:30000');
+ $this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code', 'http://localhost:30000'));
+ }
+
+ public function testAuthorizeNotForwardingUntrustedURIs(): void {
+ $client = new Client();
+ $client->setClientIdentifier('MyClientIdentifier');
+ $this->clientMapper
+ ->expects($this->once())
+ ->method('getByIdentifier')
+ ->with('MyClientId')
+ ->willReturn($client);
+ $this->session
+ ->expects($this->once())
+ ->method('set')
+ ->with('oauth.state', 'MyState');
+ $this->urlGenerator
+ ->expects($this->once())
+ ->method('linkToRouteAbsolute')
+ ->with(
+ 'core.ClientFlowLogin.showAuthPickerPage',
+ [
+ 'clientIdentifier' => 'MyClientIdentifier',
+ 'providedRedirectUri' => '',
+ ]
+ )
+ ->willReturn('https://example.com/?clientIdentifier=foo');
+ $this->config
+ ->expects($this->once())
+ ->method('getSystemValueBool')
+ ->with('oauth2.enable_oc_clients', false)
+ ->willReturn(false);
+
+ $expected = new RedirectResponse('https://example.com/?clientIdentifier=foo');
+ $this->assertEquals($expected, $this->loginRedirectorController->authorize('MyClientId', 'MyState', 'code', 'http://untrusted-uri.com'));
+ }
+
+
public function testClientNotFound(): void {
$clientNotFound = new ClientNotFoundException('could not find client test123', 0);
$this->clientMapper
diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php
index 8d84ac100fa..99074e6ff59 100644
--- a/core/Controller/ClientFlowLoginController.php
+++ b/core/Controller/ClientFlowLoginController.php
@@ -26,6 +26,7 @@ use OCP\Authentication\Exceptions\InvalidTokenException;
use OCP\Authentication\Token\IToken;
use OCP\Defaults;
use OCP\EventDispatcher\IEventDispatcher;
+use OCP\IConfig;
use OCP\IL10N;
use OCP\IRequest;
use OCP\ISession;
@@ -55,6 +56,7 @@ class ClientFlowLoginController extends Controller {
private ICrypto $crypto,
private IEventDispatcher $eventDispatcher,
private ITimeFactory $timeFactory,
+ private IConfig $config,
) {
parent::__construct($appName, $request);
}
@@ -89,7 +91,7 @@ class ClientFlowLoginController extends Controller {
#[NoCSRFRequired]
#[UseSession]
#[FrontpageRoute(verb: 'GET', url: '/login/flow')]
- public function showAuthPickerPage(string $clientIdentifier = '', string $user = '', int $direct = 0): StandaloneTemplateResponse {
+ public function showAuthPickerPage(string $clientIdentifier = '', string $user = '', int $direct = 0, string $providedRedirectUri = ''): StandaloneTemplateResponse {
$clientName = $this->getClientName();
$client = null;
if ($clientIdentifier !== '') {
@@ -142,6 +144,7 @@ class ClientFlowLoginController extends Controller {
'oauthState' => $this->session->get('oauth.state'),
'user' => $user,
'direct' => $direct,
+ 'providedRedirectUri' => $providedRedirectUri,
],
'guest'
);
@@ -161,6 +164,7 @@ class ClientFlowLoginController extends Controller {
string $stateToken = '',
string $clientIdentifier = '',
int $direct = 0,
+ string $providedRedirectUri = '',
): Response {
if (!$this->isValidToken($stateToken)) {
return $this->stateTokenForbiddenResponse();
@@ -197,6 +201,7 @@ class ClientFlowLoginController extends Controller {
'serverHost' => $this->getServerPath(),
'oauthState' => $this->session->get('oauth.state'),
'direct' => $direct,
+ 'providedRedirectUri' => $providedRedirectUri,
],
'guest'
);
@@ -211,6 +216,7 @@ class ClientFlowLoginController extends Controller {
public function generateAppPassword(
string $stateToken,
string $clientIdentifier = '',
+ string $providedRedirectUri = '',
): Response {
if (!$this->isValidToken($stateToken)) {
$this->session->remove(self::STATE_NAME);
@@ -270,7 +276,19 @@ class ClientFlowLoginController extends Controller {
$accessToken->setCodeCreatedAt($this->timeFactory->now()->getTimestamp());
$this->accessTokenMapper->insert($accessToken);
+ $enableOcClients = $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
+
$redirectUri = $client->getRedirectUri();
+ if ($enableOcClients && $redirectUri === 'http://localhost:*') {
+ // Sanity check untrusted redirect URI provided by the client first
+ if (!preg_match('/^http:\/\/localhost:[0-9]+$/', $providedRedirectUri)) {
+ $response = new Response();
+ $response->setStatus(Http::STATUS_FORBIDDEN);
+ return $response;
+ }
+
+ $redirectUri = $providedRedirectUri;
+ }
if (parse_url($redirectUri, PHP_URL_QUERY)) {
$redirectUri .= '&';
diff --git a/core/templates/loginflow/authpicker.php b/core/templates/loginflow/authpicker.php
index 94aace60cba..47e3113604d 100644
--- a/core/templates/loginflow/authpicker.php
+++ b/core/templates/loginflow/authpicker.php
@@ -31,7 +31,7 @@ $urlGenerator = $_['urlGenerator'];
<br/>
<p id="redirect-link">
- <form id="login-form" action="<?php p($urlGenerator->linkToRoute('core.ClientFlowLogin.grantPage', ['stateToken' => $_['stateToken'], 'clientIdentifier' => $_['clientIdentifier'], 'oauthState' => $_['oauthState'], 'user' => $_['user'], 'direct' => $_['direct']])) ?>" method="get">
+ <form id="login-form" action="<?php p($urlGenerator->linkToRoute('core.ClientFlowLogin.grantPage', ['stateToken' => $_['stateToken'], 'clientIdentifier' => $_['clientIdentifier'], 'oauthState' => $_['oauthState'], 'user' => $_['user'], 'direct' => $_['direct'], 'providedRedirectUri' => $_['providedRedirectUri']])) ?>" method="get">
<input type="submit" class="login primary icon-confirm-white" value="<?php p($l->t('Log in')) ?>" disabled>
</form>
</p>
diff --git a/core/templates/loginflow/grant.php b/core/templates/loginflow/grant.php
index dd2fb8d1a8e..6beafccc96e 100644
--- a/core/templates/loginflow/grant.php
+++ b/core/templates/loginflow/grant.php
@@ -35,6 +35,7 @@ $urlGenerator = $_['urlGenerator'];
<input type="hidden" name="requesttoken" value="<?php p($_['requesttoken']) ?>" />
<input type="hidden" name="stateToken" value="<?php p($_['stateToken']) ?>" />
<input type="hidden" name="oauthState" value="<?php p($_['oauthState']) ?>" />
+ <input type="hidden" name="providedRedirectUri" value="<?php p($_['providedRedirectUri']) ?>">
<?php if ($_['direct']) { ?>
<input type="hidden" name="direct" value="1" />
<?php } ?>
diff --git a/lib/private/Repair/Owncloud/MigrateOauthTables.php b/lib/private/Repair/Owncloud/MigrateOauthTables.php
index cd5087cb588..de26a907e02 100644
--- a/lib/private/Repair/Owncloud/MigrateOauthTables.php
+++ b/lib/private/Repair/Owncloud/MigrateOauthTables.php
@@ -15,6 +15,7 @@ use OCA\OAuth2\Db\AccessTokenMapper;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Authentication\Token\IToken;
use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\IConfig;
use OCP\Migration\IOutput;
use OCP\Migration\IRepairStep;
use OCP\Security\ICrypto;
@@ -29,6 +30,7 @@ class MigrateOauthTables implements IRepairStep {
private ISecureRandom $random,
private ITimeFactory $timeFactory,
private ICrypto $crypto,
+ private IConfig $config,
) {
}
@@ -169,7 +171,12 @@ class MigrateOauthTables implements IRepairStep {
$schema = new SchemaWrapper($this->db);
}
- $output->info('Delete clients (and their related access tokens) with the redirect_uri starting with oc:// or ending with *');
+ $enableOcClients = $this->config->getSystemValueBool('oauth2.enable_oc_clients', false);
+ if ($enableOcClients) {
+ $output->info('Delete clients (and their related access tokens) with the redirect_uri starting with oc://');
+ } else {
+ $output->info('Delete clients (and their related access tokens) with the redirect_uri starting with oc:// or ending with *');
+ }
// delete the access tokens
$qbDeleteAccessTokens = $this->db->getQueryBuilder();
@@ -178,10 +185,12 @@ class MigrateOauthTables implements IRepairStep {
->from('oauth2_clients')
->where(
$qbSelectClientId->expr()->iLike('redirect_uri', $qbDeleteAccessTokens->createNamedParameter('oc://%', IQueryBuilder::PARAM_STR))
- )
- ->orWhere(
+ );
+ if (!$enableOcClients) {
+ $qbSelectClientId->orWhere(
$qbSelectClientId->expr()->iLike('redirect_uri', $qbDeleteAccessTokens->createNamedParameter('%*', IQueryBuilder::PARAM_STR))
);
+ }
$qbDeleteAccessTokens->delete('oauth2_access_tokens')
->where(
@@ -194,10 +203,12 @@ class MigrateOauthTables implements IRepairStep {
$qbDeleteClients->delete('oauth2_clients')
->where(
$qbDeleteClients->expr()->iLike('redirect_uri', $qbDeleteClients->createNamedParameter('oc://%', IQueryBuilder::PARAM_STR))
- )
- ->orWhere(
+ );
+ if (!$enableOcClients) {
+ $qbDeleteClients->orWhere(
$qbDeleteClients->expr()->iLike('redirect_uri', $qbDeleteClients->createNamedParameter('%*', IQueryBuilder::PARAM_STR))
);
+ }
$qbDeleteClients->executeStatement();
// Migrate legacy refresh tokens from oc
diff --git a/openapi.json b/openapi.json
index a82c05fd331..c7ca00d5157 100644
--- a/openapi.json
+++ b/openapi.json
@@ -22147,6 +22147,15 @@
"schema": {
"type": "string"
}
+ },
+ {
+ "name": "redirect_uri",
+ "in": "query",
+ "description": "URI to redirect to after the flow (is only used for legacy ownCloud clients)",
+ "schema": {
+ "type": "string",
+ "default": ""
+ }
}
],
"responses": {
diff --git a/tests/Core/Controller/ClientFlowLoginControllerTest.php b/tests/Core/Controller/ClientFlowLoginControllerTest.php
index 1f4575208b8..591a3027e96 100644
--- a/tests/Core/Controller/ClientFlowLoginControllerTest.php
+++ b/tests/Core/Controller/ClientFlowLoginControllerTest.php
@@ -22,6 +22,7 @@ use OCP\AppFramework\Http\StandaloneTemplateResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Defaults;
use OCP\EventDispatcher\IEventDispatcher;
+use OCP\IConfig;
use OCP\IL10N;
use OCP\IRequest;
use OCP\ISession;
@@ -48,6 +49,7 @@ class ClientFlowLoginControllerTest extends TestCase {
private ICrypto&MockObject $crypto;
private IEventDispatcher&MockObject $eventDispatcher;
private ITimeFactory&MockObject $timeFactory;
+ private IConfig&MockObject $config;
private ClientFlowLoginController $clientFlowLoginController;
@@ -73,6 +75,7 @@ class ClientFlowLoginControllerTest extends TestCase {
$this->crypto = $this->createMock(ICrypto::class);
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
$this->timeFactory = $this->createMock(ITimeFactory::class);
+ $this->config = $this->createMock(IConfig::class);
$this->clientFlowLoginController = new ClientFlowLoginController(
'core',
@@ -89,6 +92,7 @@ class ClientFlowLoginControllerTest extends TestCase {
$this->crypto,
$this->eventDispatcher,
$this->timeFactory,
+ $this->config,
);
}
@@ -163,7 +167,8 @@ class ClientFlowLoginControllerTest extends TestCase {
'serverHost' => 'https://example.com',
'oauthState' => 'OauthStateToken',
'user' => '',
- 'direct' => 0
+ 'direct' => 0,
+ 'providedRedirectUri' => '',
],
'guest'
);
@@ -233,7 +238,8 @@ class ClientFlowLoginControllerTest extends TestCase {
'serverHost' => 'https://example.com',
'oauthState' => 'OauthStateToken',
'user' => '',
- 'direct' => 0
+ 'direct' => 0,
+ 'providedRedirectUri' => '',
],
'guest'
);