Merge pull request #43020 from nextcloud/backport/40766/stable27

[stable27] Make OAuth2 authorization code expire

Author: blizzz

apps/oauth2/appinfo/info.xml

@@ -5,7 +5,7 @@
 	<name>OAuth 2.0</name>
 	<summary>Allows OAuth2 compatible authentication from other web applications.</summary>
 	<description>The OAuth2 app allows administrators to configure the built-in authentication workflow to also allow OAuth2 compatible authentication from other web applications.</description>
-	<version>1.15.1</version>
+	<version>1.15.2</version>
 	<licence>agpl</licence>
 	<author>Lukas Reschke</author>
 	<namespace>OAuth2</namespace>
@@ -19,6 +19,10 @@
 		<nextcloud min-version="27" max-version="27"/>
 	</dependencies>
 
+	<background-jobs>
+		<job>OCA\OAuth2\BackgroundJob\CleanupExpiredAuthorizationCode</job>
+	</background-jobs>
+
 	<repair-steps>
 		<post-migration>
 			<step>OCA\OAuth2\Migration\SetTokenExpiration</step>

apps/oauth2/composer/composer/autoload_classmap.php

@@ -7,6 +7,7 @@
 
 return array(
     'Composer\\InstalledVersions' => $vendorDir . '/composer/InstalledVersions.php',
+    'OCA\\OAuth2\\BackgroundJob\\CleanupExpiredAuthorizationCode' => $baseDir . '/../lib/BackgroundJob/CleanupExpiredAuthorizationCode.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',
@@ -20,5 +21,6 @@
     'OCA\\OAuth2\\Migration\\Version010401Date20181207190718' => $baseDir . '/../lib/Migration/Version010401Date20181207190718.php',
     'OCA\\OAuth2\\Migration\\Version010402Date20190107124745' => $baseDir . '/../lib/Migration/Version010402Date20190107124745.php',
     'OCA\\OAuth2\\Migration\\Version011601Date20230522143227' => $baseDir . '/../lib/Migration/Version011601Date20230522143227.php',
+    'OCA\\OAuth2\\Migration\\Version011603Date20230620111039' => $baseDir . '/../lib/Migration/Version011603Date20230620111039.php',
     'OCA\\OAuth2\\Settings\\Admin' => $baseDir . '/../lib/Settings/Admin.php',
 );

apps/oauth2/composer/composer/autoload_static.php

@@ -22,6 +22,7 @@ class ComposerStaticInitOAuth2
 
     public static $classMap = array (
         'Composer\\InstalledVersions' => __DIR__ . '/..' . '/composer/InstalledVersions.php',
+        'OCA\\OAuth2\\BackgroundJob\\CleanupExpiredAuthorizationCode' => __DIR__ . '/..' . '/../lib/BackgroundJob/CleanupExpiredAuthorizationCode.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',
@@ -35,6 +36,7 @@ class ComposerStaticInitOAuth2
         'OCA\\OAuth2\\Migration\\Version010401Date20181207190718' => __DIR__ . '/..' . '/../lib/Migration/Version010401Date20181207190718.php',
         'OCA\\OAuth2\\Migration\\Version010402Date20190107124745' => __DIR__ . '/..' . '/../lib/Migration/Version010402Date20190107124745.php',
         'OCA\\OAuth2\\Migration\\Version011601Date20230522143227' => __DIR__ . '/..' . '/../lib/Migration/Version011601Date20230522143227.php',
+        'OCA\\OAuth2\\Migration\\Version011603Date20230620111039' => __DIR__ . '/..' . '/../lib/Migration/Version011603Date20230620111039.php',
         'OCA\\OAuth2\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php',
     );
 

apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php

@@ -0,0 +1,61 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2024 Julien Veyssier <julien-nc@posteo.net>
+ *
+ * @author Julien Veyssier <julien-nc@posteo.net>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+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;
+
+class CleanupExpiredAuthorizationCode extends TimedJob {
+
+	public function __construct(
+		ITimeFactory $timeFactory,
+		private AccessTokenMapper $accessTokenMapper,
+		private LoggerInterface $logger,
+
+	) {
+		parent::__construct($timeFactory);
+		// 30 days
+		$this->setInterval(60 * 60 * 24 * 30);
+		$this->setTimeSensitivity(IJob::TIME_INSENSITIVE);
+	}
+
+	/**
+	 * @param mixed $argument
+	 * @inheritDoc
+	 */
+	protected function run($argument): void {
+		try {
+			$this->accessTokenMapper->cleanupExpiredAuthorizationCode();
+		} catch (Exception $e) {
+			$this->logger->warning('Failed to cleanup tokens with expired authorization code', ['exception' => $e]);
+		}
+	}
+}

apps/oauth2/lib/Controller/OauthApiController.php

@@ -39,12 +39,15 @@
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\DB\Exception;
 use OCP\IRequest;
 use OCP\Security\ICrypto;
 use OCP\Security\ISecureRandom;
 use Psr\Log\LoggerInterface;
 
 class OauthApiController extends Controller {
+	// the authorization code expires after 10 minutes
+	public const AUTHORIZATION_CODE_EXPIRES_AFTER = 10 * 60;
 
 	public function __construct(
 		string $appName,
@@ -54,9 +57,9 @@ public function __construct(
 		private ClientMapper $clientMapper,
 		private TokenProvider $tokenProvider,
 		private ISecureRandom $secureRandom,
-		private ITimeFactory $time,
+		private ITimeFactory $timeFactory,
 		private LoggerInterface $logger,
-		private Throttler $throttler
+		private Throttler $throttler,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -67,13 +70,17 @@ public function __construct(
 	 * @BruteForceProtection(action=oauth2GetToken)
 	 *
 	 * @param string $grant_type
-	 * @param string $code
-	 * @param string $refresh_token
-	 * @param string $client_id
-	 * @param string $client_secret
+	 * @param string|null $code
+	 * @param string|null $refresh_token
+	 * @param string|null $client_id
+	 * @param string|null $client_secret
 	 * @return JSONResponse
+	 * @throws Exception
 	 */
-	public function getToken($grant_type, $code, $refresh_token, $client_id, $client_secret): JSONResponse {
+	public function getToken(
+		string $grant_type, ?string $code, ?string $refresh_token,
+		?string $client_id, ?string $client_secret
+	): JSONResponse {
 
 		// We only handle two types
 		if ($grant_type !== 'authorization_code' && $grant_type !== 'refresh_token') {
@@ -99,6 +106,33 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client
 			return $response;
 		}
 
+		if ($grant_type === 'authorization_code') {
+			// check this token is in authorization code state
+			$deliveredTokenCount = $accessToken->getTokenCount();
+			if ($deliveredTokenCount > 0) {
+				$response = new JSONResponse([
+					'error' => 'invalid_request',
+				], Http::STATUS_BAD_REQUEST);
+				$response->throttle(['invalid_request' => 'authorization_code_received_for_active_token']);
+				return $response;
+			}
+
+			// check authorization code expiration
+			$now = $this->timeFactory->now()->getTimestamp();
+			$codeCreatedAt = $accessToken->getCodeCreatedAt();
+			if ($codeCreatedAt < $now - self::AUTHORIZATION_CODE_EXPIRES_AFTER) {
+				// we know this token is not useful anymore
+				$this->accessTokenMapper->delete($accessToken);
+
+				$response = new JSONResponse([
+					'error' => 'invalid_request',
+				], Http::STATUS_BAD_REQUEST);
+				$expiredSince = $now - self::AUTHORIZATION_CODE_EXPIRES_AFTER - $codeCreatedAt;
+				$response->throttle(['invalid_request' => 'authorization_code_expired', 'expired_since' => $expiredSince]);
+				return $response;
+			}
+		}
+
 		try {
 			$client = $this->clientMapper->getByUid($accessToken->getClientId());
 		} catch (ClientNotFoundException $e) {
@@ -159,13 +193,18 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client
 		);
 
 		// Expiration is in 1 hour again
-		$appToken->setExpires($this->time->getTime() + 3600);
+		$appToken->setExpires($this->timeFactory->getTime() + 3600);
 		$this->tokenProvider->updateToken($appToken);
 
 		// Generate a new refresh token and encrypt the new apptoken in the DB
 		$newCode = $this->secureRandom->generate(128, ISecureRandom::CHAR_ALPHANUMERIC);
 		$accessToken->setHashedCode(hash('sha512', $newCode));
 		$accessToken->setEncryptedToken($this->crypto->encrypt($newToken, $newCode));
+		// increase the number of delivered oauth token
+		// this helps with cleaning up DB access token when authorization code has expired
+		// and it never delivered any oauth token
+		$tokenCount = $accessToken->getTokenCount();
+		$accessToken->setTokenCount($tokenCount + 1);
 		$this->accessTokenMapper->update($accessToken);
 
 		$this->throttler->resetDelay($this->request->getRemoteAddress(), 'login', ['user' => $appToken->getUID()]);

apps/oauth2/lib/Db/AccessToken.php

@@ -34,6 +34,10 @@
  * @method void setEncryptedToken(string $token)
  * @method string getHashedCode()
  * @method void setHashedCode(string $token)
+ * @method int getCodeCreatedAt()
+ * @method void setCodeCreatedAt(int $createdAt)
+ * @method int getTokenCount()
+ * @method void setTokenCount(int $tokenCount)
  */
 class AccessToken extends Entity {
 	/** @var int */
@@ -44,12 +48,18 @@ class AccessToken extends Entity {
 	protected $hashedCode;
 	/** @var string */
 	protected $encryptedToken;
+	/** @var int */
+	protected $codeCreatedAt;
+	/** @var int */
+	protected $tokenCount;
 
 	public function __construct() {
 		$this->addType('id', 'int');
 		$this->addType('tokenId', 'int');
 		$this->addType('clientId', 'int');
 		$this->addType('hashedCode', 'string');
 		$this->addType('encryptedToken', 'string');
+		$this->addType('codeCreatedAt', 'int');
+		$this->addType('tokenCount', 'int');
 	}
 }

apps/oauth2/lib/Db/AccessTokenMapper.php

@@ -28,9 +28,12 @@
  */
 namespace OCA\OAuth2\Db;
 
+use OCA\OAuth2\Controller\OauthApiController;
 use OCA\OAuth2\Exceptions\AccessTokenNotFoundException;
 use OCP\AppFramework\Db\IMapperException;
 use OCP\AppFramework\Db\QBMapper;
+use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\DB\Exception;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 
@@ -39,10 +42,10 @@
  */
 class AccessTokenMapper extends QBMapper {
 
-	/**
-	 * @param IDBConnection $db
-	 */
-	public function __construct(IDBConnection $db) {
+	public function __construct(
+		IDBConnection $db,
+		private ITimeFactory $timeFactory,
+	) {
 		parent::__construct($db, 'oauth2_access_tokens');
 	}
 
@@ -79,4 +82,24 @@ public function deleteByClientId(int $id) {
 			->where($qb->expr()->eq('client_id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)));
 		$qb->executeStatement();
 	}
+
+	/**
+	 * Delete access tokens that have an expired authorization code
+	 * -> those that are old enough
+	 * and which never delivered any oauth token (still in authorization state)
+	 *
+	 * @return void
+	 * @throws Exception
+	 */
+	public function cleanupExpiredAuthorizationCode(): void {
+		$now = $this->timeFactory->now()->getTimestamp();
+		$maxTokenCreationTs = $now - OauthApiController::AUTHORIZATION_CODE_EXPIRES_AFTER;
+
+		$qb = $this->db->getQueryBuilder();
+		$qb
+			->delete($this->tableName)
+			->where($qb->expr()->eq('token_count', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)))
+			->andWhere($qb->expr()->lt('code_created_at', $qb->createNamedParameter($maxTokenCreationTs, IQueryBuilder::PARAM_INT)));
+		$qb->executeStatement();
+	}
 }

apps/oauth2/lib/Migration/Version011603Date20230620111039.php

@@ -0,0 +1,86 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright 2023, Julien Veyssier <julien-nc@posteo.net>
+ *
+ * @author Julien Veyssier <julien-nc@posteo.net>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+namespace OCA\OAuth2\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\QueryBuilder\IQueryBuilder;
+use OCP\DB\Types;
+use OCP\IDBConnection;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version011603Date20230620111039 extends SimpleMigrationStep {
+
+	public function __construct(
+		private IDBConnection $connection,
+	) {
+	}
+
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		if ($schema->hasTable('oauth2_access_tokens')) {
+			$table = $schema->getTable('oauth2_access_tokens');
+			$dbChanged = false;
+			if (!$table->hasColumn('code_created_at')) {
+				$table->addColumn('code_created_at', Types::BIGINT, [
+					'notnull' => true,
+					'default' => 0,
+					'unsigned' => true,
+				]);
+				$dbChanged = true;
+			}
+			if (!$table->hasColumn('token_count')) {
+				$table->addColumn('token_count', Types::BIGINT, [
+					'notnull' => true,
+					'default' => 0,
+					'unsigned' => true,
+				]);
+				$dbChanged = true;
+			}
+			if (!$table->hasIndex('oauth2_tk_c_created_idx')) {
+				$table->addIndex(['token_count', 'code_created_at'], 'oauth2_tk_c_created_idx');
+				$dbChanged = true;
+			}
+			if ($dbChanged) {
+				return $schema;
+			}
+		}
+
+		return null;
+	}
+
+	public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
+		// we consider that existing access_tokens have already produced at least one oauth token
+		// which prevents cleaning them up
+		$qbUpdate = $this->connection->getQueryBuilder();
+		$qbUpdate->update('oauth2_access_tokens')
+			->set('token_count', $qbUpdate->createNamedParameter(1, IQueryBuilder::PARAM_INT));
+		$qbUpdate->executeStatement();
+	}
+}