diff options
Diffstat (limited to 'apps/oauth2/lib')
| -rw-r--r-- | apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php | 4 | ||||
| -rw-r--r-- | apps/oauth2/lib/Command/ImportLegacyOcClient.php | 76 | ||||
| -rw-r--r-- | apps/oauth2/lib/Controller/LoginRedirectorController.php | 85 | ||||
| -rw-r--r-- | apps/oauth2/lib/Controller/OauthApiController.php | 19 | ||||
| -rw-r--r-- | apps/oauth2/lib/Controller/SettingsController.php | 8 | ||||
| -rw-r--r-- | apps/oauth2/lib/Db/AccessToken.php | 12 | ||||
| -rw-r--r-- | apps/oauth2/lib/Db/Client.php | 4 | ||||
| -rw-r--r-- | apps/oauth2/lib/Db/ClientMapper.php | 4 | ||||
| -rw-r--r-- | apps/oauth2/lib/Migration/SetTokenExpiration.php | 22 | ||||
| -rw-r--r-- | apps/oauth2/lib/Migration/Version011901Date20240829164356.php | 49 | ||||
| -rw-r--r-- | apps/oauth2/lib/Settings/Admin.php | 5 |
11 files changed, 217 insertions, 71 deletions
diff --git a/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php b/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php index 84d62ab6b45..b819a45ace2 100644 --- a/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php +++ b/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php @@ -12,7 +12,6 @@ namespace OCA\OAuth2\BackgroundJob; use OCA\OAuth2\Db\AccessTokenMapper; use OCP\AppFramework\Utility\ITimeFactory; -use OCP\BackgroundJob\IJob; use OCP\BackgroundJob\TimedJob; use OCP\DB\Exception; use Psr\Log\LoggerInterface; @@ -23,12 +22,11 @@ class CleanupExpiredAuthorizationCode extends TimedJob { ITimeFactory $timeFactory, private AccessTokenMapper $accessTokenMapper, private LoggerInterface $logger, - ) { parent::__construct($timeFactory); // 30 days $this->setInterval(60 * 60 * 24 * 30); - $this->setTimeSensitivity(IJob::TIME_INSENSITIVE); + $this->setTimeSensitivity(self::TIME_INSENSITIVE); } /** 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 f326b821a60..7241b35cdcf 100644 --- a/apps/oauth2/lib/Controller/LoginRedirectorController.php +++ b/apps/oauth2/lib/Controller/LoginRedirectorController.php @@ -8,27 +8,27 @@ declare(strict_types=1); */ namespace OCA\OAuth2\Controller; +use OC\Core\Controller\ClientFlowLoginController; use OCA\OAuth2\Db\ClientMapper; use OCA\OAuth2\Exceptions\ClientNotFoundException; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\OpenAPI; +use OCP\AppFramework\Http\Attribute\PublicPage; +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; use OCP\IURLGenerator; +use OCP\Security\ISecureRandom; +#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] class LoginRedirectorController extends Controller { - /** @var IURLGenerator */ - private $urlGenerator; - /** @var ClientMapper */ - private $clientMapper; - /** @var ISession */ - private $session; - /** @var IL10N */ - private $l; - /** * @param string $appName * @param IRequest $request @@ -37,37 +37,39 @@ class LoginRedirectorController extends Controller { * @param ISession $session * @param IL10N $l */ - public function __construct(string $appName, + public function __construct( + string $appName, IRequest $request, - IURLGenerator $urlGenerator, - ClientMapper $clientMapper, - ISession $session, - IL10N $l) { + private IURLGenerator $urlGenerator, + private ClientMapper $clientMapper, + private ISession $session, + private IL10N $l, + private ISecureRandom $random, + private IAppConfig $appConfig, + private IConfig $config, + ) { parent::__construct($appName, $request); - $this->urlGenerator = $urlGenerator; - $this->clientMapper = $clientMapper; - $this->session = $session; - $this->l = $l; } /** - * @PublicPage - * @NoCSRFRequired - * @UseSession - * * Authorize the user * * @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 * 303: Redirect to login URL */ + #[PublicPage] + #[NoCSRFRequired] + #[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) { @@ -83,14 +85,39 @@ 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); - $targetUrl = $this->urlGenerator->linkToRouteAbsolute( - 'core.ClientFlowLogin.showAuthPickerPage', - [ - 'clientIdentifier' => $client->getClientIdentifier(), - ] - ); + if (in_array($client->getName(), $this->appConfig->getValueArray('oauth2', 'skipAuthPickerApplications', []))) { + /** @see ClientFlowLoginController::showAuthPickerPage **/ + $stateToken = $this->random->generate( + 64, + ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER . ISecureRandom::CHAR_DIGITS + ); + $this->session->set(ClientFlowLoginController::STATE_NAME, $stateToken); + $targetUrl = $this->urlGenerator->linkToRouteAbsolute( + 'core.ClientFlowLogin.grantPage', + [ + 'stateToken' => $stateToken, + 'clientIdentifier' => $client->getClientIdentifier(), + 'providedRedirectUri' => $providedRedirectUri, + ] + ); + } else { + $targetUrl = $this->urlGenerator->linkToRouteAbsolute( + 'core.ClientFlowLogin.showAuthPickerPage', + [ + 'clientIdentifier' => $client->getClientIdentifier(), + 'providedRedirectUri' => $providedRedirectUri, + ] + ); + } return new RedirectResponse($targetUrl); } } diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 389bc73811b..11f17fda4bf 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -15,6 +15,10 @@ use OCA\OAuth2\Exceptions\AccessTokenNotFoundException; use OCA\OAuth2\Exceptions\ClientNotFoundException; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\BruteForceProtection; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\OpenAPI; +use OCP\AppFramework\Http\Attribute\PublicPage; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Authentication\Exceptions\ExpiredTokenException; @@ -26,6 +30,7 @@ use OCP\Security\ICrypto; use OCP\Security\ISecureRandom; use Psr\Log\LoggerInterface; +#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] class OauthApiController extends Controller { // the authorization code expires after 10 minutes public const AUTHORIZATION_CODE_EXPIRES_AFTER = 10 * 60; @@ -47,10 +52,6 @@ class OauthApiController extends Controller { } /** - * @PublicPage - * @NoCSRFRequired - * @BruteForceProtection(action=oauth2GetToken) - * * Get a token * * @param string $grant_type Token type that should be granted @@ -64,9 +65,12 @@ class OauthApiController extends Controller { * 200: Token returned * 400: Getting token is not possible */ + #[PublicPage] + #[NoCSRFRequired] + #[BruteForceProtection(action: 'oauth2GetToken')] public function getToken( string $grant_type, ?string $code, ?string $refresh_token, - ?string $client_id, ?string $client_secret + ?string $client_id, ?string $client_secret, ): JSONResponse { // We only handle two types @@ -136,7 +140,8 @@ class OauthApiController extends Controller { } try { - $storedClientSecret = $this->crypto->decrypt($client->getSecret()); + $storedClientSecretHash = $client->getSecret(); + $clientSecretHash = bin2hex($this->crypto->calculateHMAC($client_secret)); } catch (\Exception $e) { $this->logger->error('OAuth client secret decryption error', ['exception' => $e]); // we don't throttle here because it might not be a bruteforce attack @@ -145,7 +150,7 @@ class OauthApiController extends Controller { ], Http::STATUS_BAD_REQUEST); } // The client id and secret must match. Else we don't provide an access token! - if ($client->getClientIdentifier() !== $client_id || $storedClientSecret !== $client_secret) { + if ($client->getClientIdentifier() !== $client_id || $storedClientSecretHash !== $clientSecretHash) { $response = new JSONResponse([ 'error' => 'invalid_client', ], Http::STATUS_BAD_REQUEST); diff --git a/apps/oauth2/lib/Controller/SettingsController.php b/apps/oauth2/lib/Controller/SettingsController.php index 81e728bd693..9bd02c8a2cd 100644 --- a/apps/oauth2/lib/Controller/SettingsController.php +++ b/apps/oauth2/lib/Controller/SettingsController.php @@ -35,7 +35,7 @@ class SettingsController extends Controller { private IL10N $l, private IAuthTokenProvider $tokenProvider, private IUserManager $userManager, - private ICrypto $crypto + private ICrypto $crypto, ) { parent::__construct($appName, $request); } @@ -50,8 +50,8 @@ class SettingsController extends Controller { $client->setName($name); $client->setRedirectUri($redirectUri); $secret = $this->secureRandom->generate(64, self::validChars); - $encryptedSecret = $this->crypto->encrypt($secret); - $client->setSecret($encryptedSecret); + $hashedSecret = bin2hex($this->crypto->calculateHMAC($secret)); + $client->setSecret($hashedSecret); $client->setClientIdentifier($this->secureRandom->generate(64, self::validChars)); $client = $this->clientMapper->insert($client); @@ -69,7 +69,7 @@ class SettingsController extends Controller { public function deleteClient(int $id): JSONResponse { $client = $this->clientMapper->getByUid($id); - $this->userManager->callForAllUsers(function (IUser $user) use ($client) { + $this->userManager->callForSeenUsers(function (IUser $user) use ($client): void { $this->tokenProvider->invalidateTokensOfUser($user->getUID(), $client->getName()); }); diff --git a/apps/oauth2/lib/Db/AccessToken.php b/apps/oauth2/lib/Db/AccessToken.php index 543d512e0ce..34adc4f4797 100644 --- a/apps/oauth2/lib/Db/AccessToken.php +++ b/apps/oauth2/lib/Db/AccessToken.php @@ -1,4 +1,5 @@ <?php + /** * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later @@ -6,6 +7,7 @@ namespace OCA\OAuth2\Db; use OCP\AppFramework\Db\Entity; +use OCP\DB\Types; /** * @method int getTokenId() @@ -36,12 +38,12 @@ class AccessToken extends Entity { protected $tokenCount; public function __construct() { - $this->addType('id', 'int'); - $this->addType('tokenId', 'int'); - $this->addType('clientId', 'int'); + $this->addType('id', Types::INTEGER); + $this->addType('tokenId', Types::INTEGER); + $this->addType('clientId', Types::INTEGER); $this->addType('hashedCode', 'string'); $this->addType('encryptedToken', 'string'); - $this->addType('codeCreatedAt', 'int'); - $this->addType('tokenCount', 'int'); + $this->addType('codeCreatedAt', Types::INTEGER); + $this->addType('tokenCount', Types::INTEGER); } } diff --git a/apps/oauth2/lib/Db/Client.php b/apps/oauth2/lib/Db/Client.php index c48ca4edfc0..8fce0040c96 100644 --- a/apps/oauth2/lib/Db/Client.php +++ b/apps/oauth2/lib/Db/Client.php @@ -1,4 +1,5 @@ <?php + /** * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later @@ -6,6 +7,7 @@ namespace OCA\OAuth2\Db; use OCP\AppFramework\Db\Entity; +use OCP\DB\Types; /** * @method string getClientIdentifier() @@ -28,7 +30,7 @@ class Client extends Entity { protected $secret; public function __construct() { - $this->addType('id', 'int'); + $this->addType('id', Types::INTEGER); $this->addType('name', 'string'); $this->addType('redirectUri', 'string'); $this->addType('clientIdentifier', 'string'); diff --git a/apps/oauth2/lib/Db/ClientMapper.php b/apps/oauth2/lib/Db/ClientMapper.php index dc19c93c4e1..c5ca2989d0f 100644 --- a/apps/oauth2/lib/Db/ClientMapper.php +++ b/apps/oauth2/lib/Db/ClientMapper.php @@ -41,7 +41,7 @@ class ClientMapper extends QBMapper { try { $client = $this->findEntity($qb); } catch (IMapperException $e) { - throw new ClientNotFoundException('could not find client '.$clientIdentifier, 0, $e); + throw new ClientNotFoundException('could not find client ' . $clientIdentifier, 0, $e); } return $client; } @@ -61,7 +61,7 @@ class ClientMapper extends QBMapper { try { $client = $this->findEntity($qb); } catch (IMapperException $e) { - throw new ClientNotFoundException('could not find client with id '.$id, 0, $e); + throw new ClientNotFoundException('could not find client with id ' . $id, 0, $e); } return $client; } diff --git a/apps/oauth2/lib/Migration/SetTokenExpiration.php b/apps/oauth2/lib/Migration/SetTokenExpiration.php index 5077e74be87..dc925e26bb2 100644 --- a/apps/oauth2/lib/Migration/SetTokenExpiration.php +++ b/apps/oauth2/lib/Migration/SetTokenExpiration.php @@ -18,21 +18,11 @@ use OCP\Migration\IRepairStep; class SetTokenExpiration implements IRepairStep { - /** @var IDBConnection */ - private $connection; - - /** @var ITimeFactory */ - private $time; - - /** @var TokenProvider */ - private $tokenProvider; - - public function __construct(IDBConnection $connection, - ITimeFactory $timeFactory, - TokenProvider $tokenProvider) { - $this->connection = $connection; - $this->time = $timeFactory; - $this->tokenProvider = $tokenProvider; + public function __construct( + private IDBConnection $connection, + private ITimeFactory $time, + private TokenProvider $tokenProvider, + ) { } public function getName(): string { @@ -44,7 +34,7 @@ class SetTokenExpiration implements IRepairStep { $qb->select('*') ->from('oauth2_access_tokens'); - $cursor = $qb->execute(); + $cursor = $qb->executeQuery(); while ($row = $cursor->fetch()) { $token = AccessToken::fromRow($row); diff --git a/apps/oauth2/lib/Migration/Version011901Date20240829164356.php b/apps/oauth2/lib/Migration/Version011901Date20240829164356.php new file mode 100644 index 00000000000..20f5754bf11 --- /dev/null +++ b/apps/oauth2/lib/Migration/Version011901Date20240829164356.php @@ -0,0 +1,49 @@ +<?php + +declare(strict_types=1); + +/** + * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +namespace OCA\OAuth2\Migration; + +use Closure; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; +use OCP\Security\ICrypto; + +class Version011901Date20240829164356 extends SimpleMigrationStep { + + public function __construct( + private IDBConnection $connection, + private ICrypto $crypto, + ) { + } + + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + $qbUpdate = $this->connection->getQueryBuilder(); + $qbUpdate->update('oauth2_clients') + ->set('secret', $qbUpdate->createParameter('updateSecret')) + ->where( + $qbUpdate->expr()->eq('id', $qbUpdate->createParameter('updateId')) + ); + + $qbSelect = $this->connection->getQueryBuilder(); + $qbSelect->select('id', 'secret') + ->from('oauth2_clients'); + $req = $qbSelect->executeQuery(); + while ($row = $req->fetch()) { + $id = $row['id']; + $storedEncryptedSecret = $row['secret']; + $secret = $this->crypto->decrypt($storedEncryptedSecret); + $hashedSecret = bin2hex($this->crypto->calculateHMAC($secret)); + $qbUpdate->setParameter('updateSecret', $hashedSecret, IQueryBuilder::PARAM_STR); + $qbUpdate->setParameter('updateId', $id, IQueryBuilder::PARAM_INT); + $qbUpdate->executeStatement(); + } + $req->closeCursor(); + } +} diff --git a/apps/oauth2/lib/Settings/Admin.php b/apps/oauth2/lib/Settings/Admin.php index eae10098a59..93b6b7bcc3f 100644 --- a/apps/oauth2/lib/Settings/Admin.php +++ b/apps/oauth2/lib/Settings/Admin.php @@ -12,7 +12,6 @@ use OCA\OAuth2\Db\ClientMapper; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Services\IInitialState; use OCP\IURLGenerator; -use OCP\Security\ICrypto; use OCP\Settings\ISettings; use Psr\Log\LoggerInterface; @@ -22,7 +21,6 @@ class Admin implements ISettings { private IInitialState $initialState, private ClientMapper $clientMapper, private IURLGenerator $urlGenerator, - private ICrypto $crypto, private LoggerInterface $logger, ) { } @@ -33,13 +31,12 @@ class Admin implements ISettings { foreach ($clients as $client) { try { - $secret = $this->crypto->decrypt($client->getSecret()); $result[] = [ 'id' => $client->getId(), 'name' => $client->getName(), 'redirectUri' => $client->getRedirectUri(), 'clientId' => $client->getClientIdentifier(), - 'clientSecret' => $secret, + 'clientSecret' => '', ]; } catch (\Exception $e) { $this->logger->error('[Settings] OAuth client secret decryption error', ['exception' => $e]); |