make oauth2 authorization code expire after 10 minutes

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>

apps/oauth2/composer/composer/autoload_classmap.php

@@ -21,5 +21,6 @@ return array(
     'OCA\\OAuth2\\Migration\\Version010402Date20190107124745' => $baseDir . '/../lib/Migration/Version010402Date20190107124745.php',
     'OCA\\OAuth2\\Migration\\Version011601Date20230522143227' => $baseDir . '/../lib/Migration/Version011601Date20230522143227.php',
     'OCA\\OAuth2\\Migration\\Version011602Date20230613160650' => $baseDir . '/../lib/Migration/Version011602Date20230613160650.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

@@ -36,6 +36,7 @@ class ComposerStaticInitOAuth2
         'OCA\\OAuth2\\Migration\\Version010402Date20190107124745' => __DIR__ . '/..' . '/../lib/Migration/Version010402Date20190107124745.php',
         'OCA\\OAuth2\\Migration\\Version011601Date20230522143227' => __DIR__ . '/..' . '/../lib/Migration/Version011601Date20230522143227.php',
         'OCA\\OAuth2\\Migration\\Version011602Date20230613160650' => __DIR__ . '/..' . '/../lib/Migration/Version011602Date20230613160650.php',
+        'OCA\\OAuth2\\Migration\\Version011603Date20230620111039' => __DIR__ . '/..' . '/../lib/Migration/Version011603Date20230620111039.php',
         'OCA\\OAuth2\\Settings\\Admin' => __DIR__ . '/..' . '/../lib/Settings/Admin.php',
     );
 

apps/oauth2/lib/Controller/OauthApiController.php

@@ -39,6 +39,7 @@ use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\DB\Exception;
 use OCP\IRequest;
 use OCP\Security\Bruteforce\IThrottler;
 use OCP\Security\ICrypto;
@@ -46,6 +47,8 @@ use OCP\Security\ISecureRandom;
 use Psr\Log\LoggerInterface;
 
 class OauthApiController extends Controller {
+	// the authorization code expires after 10 minutes
+	private const AUTHORIZATION_CODE_EXPIRES_AFTER = 10 * 60;
 
 	public function __construct(
 		string $appName,
@@ -57,7 +60,8 @@ class OauthApiController extends Controller {
 		private ISecureRandom $secureRandom,
 		private ITimeFactory $time,
 		private LoggerInterface $logger,
-		private IThrottler $throttler
+		private IThrottler $throttler,
+		private ITimeFactory $timeFactory,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -70,16 +74,20 @@ class OauthApiController extends Controller {
 	 * Get a token
 	 *
 	 * @param string $grant_type Token type that should be granted
-	 * @param string $code Code of the flow
-	 * @param string $refresh_token Refresh token
-	 * @param string $client_id Client ID
-	 * @param string $client_secret Client secret
+	 * @param string|null $code Code of the flow
+	 * @param string|null $refresh_token Refresh token
+	 * @param string|null $client_id Client ID
+	 * @param string|null $client_secret Client secret
+	 * @throws Exception
 	 * @return JSONResponse<Http::STATUS_OK, array{access_token: string, token_type: string, expires_in: int, refresh_token: string, user_id: string}, array{}>|JSONResponse<Http::STATUS_BAD_REQUEST, array{error: string}, array{}>
 	 *
 	 * 200: Token returned
 	 * 400: Getting token is not possible
 	 */
-	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') {
@@ -105,6 +113,20 @@ class OauthApiController extends Controller {
 			return $response;
 		}
 
+		// check authorization code expiration
+		if ($grant_type === 'authorization_code') {
+			$now = $this->timeFactory->now()->getTimestamp();
+			$tokenCreatedAt = $accessToken->getCreatedAt();
+			if ($tokenCreatedAt < $now - self::AUTHORIZATION_CODE_EXPIRES_AFTER) {
+				$response = new JSONResponse([
+					'error' => 'invalid_request',
+				], Http::STATUS_BAD_REQUEST);
+				$expiredSince = $now - self::AUTHORIZATION_CODE_EXPIRES_AFTER - $tokenCreatedAt;
+				$response->throttle(['invalid_request' => 'authorization_code_expired', 'expired_since' => $expiredSince]);
+				return $response;
+			}
+		}
+
 		try {
 			$client = $this->clientMapper->getByUid($accessToken->getClientId());
 		} catch (ClientNotFoundException $e) {

apps/oauth2/lib/Db/AccessToken.php

@@ -34,6 +34,8 @@ use OCP\AppFramework\Db\Entity;
  * @method void setEncryptedToken(string $token)
  * @method string getHashedCode()
  * @method void setHashedCode(string $token)
+ * @method int getCreatedAt()
+ * @method void setCreatedAt(int $createdAt)
  */
 class AccessToken extends Entity {
 	/** @var int */
@@ -44,6 +46,8 @@ class AccessToken extends Entity {
 	protected $hashedCode;
 	/** @var string */
 	protected $encryptedToken;
+	/** @var int */
+	protected $createdAt;
 
 	public function __construct() {
 		$this->addType('id', 'int');
@@ -51,5 +55,6 @@ class AccessToken extends Entity {
 		$this->addType('clientId', 'int');
 		$this->addType('hashedCode', 'string');
 		$this->addType('encryptedToken', 'string');
+		$this->addType('created_at', 'int');
 	}
 }

apps/oauth2/lib/Migration/Version011603Date20230620111039.php

@@ -0,0 +1,57 @@
+<?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\Types;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version011603Date20230620111039 extends SimpleMigrationStep {
+
+	public function __construct(
+	) {
+	}
+
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		if ($schema->hasTable('oauth2_access_tokens')) {
+			$table = $schema->getTable('oauth2_access_tokens');
+			if (!$table->hasColumn('created_at')) {
+				$table->addColumn('created_at', Types::BIGINT, [
+					'notnull' => true,
+					'default' => 0,
+				]);
+				return $schema;
+			}
+		}
+
+		return null;
+	}
+}

apps/oauth2/tests/Controller/OauthApiControllerTest.php

@@ -70,6 +70,8 @@ class OauthApiControllerTest extends TestCase {
 	private $throttler;
 	/** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */
 	private $logger;
+	/** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
+	private $timeFactory;
 	/** @var OauthApiController */
 	private $oauthApiController;
 
@@ -85,6 +87,7 @@ class OauthApiControllerTest extends TestCase {
 		$this->time = $this->createMock(ITimeFactory::class);
 		$this->throttler = $this->createMock(IThrottler::class);
 		$this->logger = $this->createMock(LoggerInterface::class);
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
 
 		$this->oauthApiController = new OauthApiController(
 			'oauth2',
@@ -96,7 +99,8 @@ class OauthApiControllerTest extends TestCase {
 			$this->secureRandom,
 			$this->time,
 			$this->logger,
-			$this->throttler
+			$this->throttler,
+			$this->timeFactory
 		);
 	}
 

core/Controller/ClientFlowLoginController.php

@@ -46,6 +46,7 @@ use OCP\AppFramework\Http\Attribute\IgnoreOpenAPI;
 use OCP\AppFramework\Http\Attribute\UseSession;
 use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Http\StandaloneTemplateResponse;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Defaults;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IL10N;
@@ -76,6 +77,7 @@ class ClientFlowLoginController extends Controller {
 		private AccessTokenMapper $accessTokenMapper,
 		private ICrypto $crypto,
 		private IEventDispatcher $eventDispatcher,
+		private ITimeFactory $timeFactory,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -287,6 +289,7 @@ class ClientFlowLoginController extends Controller {
 			$accessToken->setEncryptedToken($this->crypto->encrypt($token, $code));
 			$accessToken->setHashedCode(hash('sha512', $code));
 			$accessToken->setTokenId($generatedToken->getId());
+			$accessToken->setCreatedAt($this->timeFactory->now()->getTimestamp());
 			$this->accessTokenMapper->insert($accessToken);
 
 			$redirectUri = $client->getRedirectUri();

tests/Core/Controller/ClientFlowLoginControllerTest.php

@@ -31,6 +31,7 @@ use OCA\OAuth2\Db\Client;
 use OCA\OAuth2\Db\ClientMapper;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\StandaloneTemplateResponse;
+use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\Defaults;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IL10N;
@@ -95,6 +96,7 @@ class ClientFlowLoginControllerTest extends TestCase {
 		$this->accessTokenMapper = $this->createMock(AccessTokenMapper::class);
 		$this->crypto = $this->createMock(ICrypto::class);
 		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
+		$this->timeFactory = $this->createMock(ITimeFactory::class);
 
 		$this->clientFlowLoginController = new ClientFlowLoginController(
 			'core',
@@ -109,7 +111,8 @@ class ClientFlowLoginControllerTest extends TestCase {
 			$this->clientMapper,
 			$this->accessTokenMapper,
 			$this->crypto,
-			$this->eventDispatcher
+			$this->eventDispatcher,
+			$this->timeFactory
 		);
 	}