From 807f173dec7288945fca98548e80e43d3e401d12 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 20 Jun 2023 11:54:43 +0200 Subject: [PATCH 01/13] make oauth2 authorization code expire after 10 minutes Signed-off-by: Julien Veyssier --- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Controller/OauthApiController.php | 34 +++++++++-- apps/oauth2/lib/Db/AccessToken.php | 5 ++ .../Version011603Date20230620111039.php | 57 +++++++++++++++++++ .../Controller/OauthApiControllerTest.php | 6 +- core/Controller/ClientFlowLoginController.php | 3 + .../ClientFlowLoginControllerTest.php | 5 +- 8 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 apps/oauth2/lib/Migration/Version011603Date20230620111039.php diff --git a/apps/oauth2/composer/composer/autoload_classmap.php b/apps/oauth2/composer/composer/autoload_classmap.php index ffc00e254def3..77068f4e9c022 100644 --- a/apps/oauth2/composer/composer/autoload_classmap.php +++ b/apps/oauth2/composer/composer/autoload_classmap.php @@ -21,5 +21,6 @@ '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', ); diff --git a/apps/oauth2/composer/composer/autoload_static.php b/apps/oauth2/composer/composer/autoload_static.php index 759e4fc3b79a5..2fb8a4f17a9f1 100644 --- a/apps/oauth2/composer/composer/autoload_static.php +++ b/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', ); diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index af1205be0d758..443db314f2a40 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -39,6 +39,7 @@ 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 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 @@ public function __construct( 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 @@ public function __construct( * 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|JSONResponse * * 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 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client 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) { diff --git a/apps/oauth2/lib/Db/AccessToken.php b/apps/oauth2/lib/Db/AccessToken.php index 26c830c64adf1..99404850e4f42 100644 --- a/apps/oauth2/lib/Db/AccessToken.php +++ b/apps/oauth2/lib/Db/AccessToken.php @@ -34,6 +34,8 @@ * @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 @@ public function __construct() { $this->addType('clientId', 'int'); $this->addType('hashedCode', 'string'); $this->addType('encryptedToken', 'string'); + $this->addType('created_at', 'int'); } } diff --git a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php new file mode 100644 index 0000000000000..279bf6cffaa8e --- /dev/null +++ b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php @@ -0,0 +1,57 @@ + + * + * @author Julien Veyssier + * + * @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 . + * + */ +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; + } +} diff --git a/apps/oauth2/tests/Controller/OauthApiControllerTest.php b/apps/oauth2/tests/Controller/OauthApiControllerTest.php index a7dc35943f035..e8ee03cb6e46d 100644 --- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php +++ b/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 @@ protected function setUp(): void { $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 @@ protected function setUp(): void { $this->secureRandom, $this->time, $this->logger, - $this->throttler + $this->throttler, + $this->timeFactory ); } diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 3f92ad8cf3037..05321c9222a17 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -46,6 +46,7 @@ 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 @@ public function __construct( private AccessTokenMapper $accessTokenMapper, private ICrypto $crypto, private IEventDispatcher $eventDispatcher, + private ITimeFactory $timeFactory, ) { parent::__construct($appName, $request); } @@ -287,6 +289,7 @@ public function generateAppPassword(string $stateToken, $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(); diff --git a/tests/Core/Controller/ClientFlowLoginControllerTest.php b/tests/Core/Controller/ClientFlowLoginControllerTest.php index dfd3e629dcd4a..e340e81f342c7 100644 --- a/tests/Core/Controller/ClientFlowLoginControllerTest.php +++ b/tests/Core/Controller/ClientFlowLoginControllerTest.php @@ -31,6 +31,7 @@ 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 @@ protected function setUp(): void { $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 @@ protected function setUp(): void { $this->clientMapper, $this->accessTokenMapper, $this->crypto, - $this->eventDispatcher + $this->eventDispatcher, + $this->timeFactory ); } From 2995b0948f23c75fc145ace6355907f8afcd5c8c Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 20 Jun 2023 12:39:41 +0200 Subject: [PATCH 02/13] add tests for oauth2 authorization code expiration Signed-off-by: Julien Veyssier --- .../lib/Controller/OauthApiController.php | 2 +- .../Controller/OauthApiControllerTest.php | 70 +++++++++++++++++-- 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 443db314f2a40..d9646363f5fa8 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -48,7 +48,7 @@ class OauthApiController extends Controller { // the authorization code expires after 10 minutes - private const AUTHORIZATION_CODE_EXPIRES_AFTER = 10 * 60; + public const AUTHORIZATION_CODE_EXPIRES_AFTER = 10 * 60; public function __construct( string $appName, diff --git a/apps/oauth2/tests/Controller/OauthApiControllerTest.php b/apps/oauth2/tests/Controller/OauthApiControllerTest.php index e8ee03cb6e46d..f9db388713bc0 100644 --- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php +++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php @@ -126,7 +126,63 @@ public function testGetTokenInvalidCode() { $this->assertEquals($expected, $this->oauthApiController->getToken('authorization_code', 'invalidcode', null, null, null)); } - public function testGetTokenInvalidRefreshToken() { + public function testGetTokenExpiredCode() { + $tokenCreatedAt = 100; + $expiredSince = 123; + + $expected = new JSONResponse([ + 'error' => 'invalid_request', + ], Http::STATUS_BAD_REQUEST); + $expected->throttle(['invalid_request' => 'authorization_code_expired', 'expired_since' => $expiredSince]); + + $accessToken = new AccessToken(); + $accessToken->setClientId(42); + $accessToken->setCreatedAt($tokenCreatedAt); + + $this->accessTokenMapper->method('getByCode') + ->with('validcode') + ->willReturn($accessToken); + + $tsNow = $tokenCreatedAt + OauthApiController::AUTHORIZATION_CODE_EXPIRES_AFTER + $expiredSince; + $dateNow = (new \DateTimeImmutable())->setTimestamp($tsNow); + $this->timeFactory->method('now') + ->willReturn($dateNow); + + $this->assertEquals($expected, $this->oauthApiController->getToken('authorization_code', 'validcode', null, null, null)); + } + + public function testGetTokenClientDoesNotExist() { + // In this test, the token's authorization code is valid and has not expired + // and we check what happens when the associated Oauth client does not exist + $tokenCreatedAt = 100; + + $expected = new JSONResponse([ + 'error' => 'invalid_request', + ], Http::STATUS_BAD_REQUEST); + $expected->throttle(['invalid_request' => 'client not found', 'client_id' => 42]); + + $accessToken = new AccessToken(); + $accessToken->setClientId(42); + $accessToken->setCreatedAt($tokenCreatedAt); + + $this->accessTokenMapper->method('getByCode') + ->with('validcode') + ->willReturn($accessToken); + + // 'now' is before the token's authorization code expiration + $tsNow = $tokenCreatedAt + OauthApiController::AUTHORIZATION_CODE_EXPIRES_AFTER - 1; + $dateNow = (new \DateTimeImmutable())->setTimestamp($tsNow); + $this->timeFactory->method('now') + ->willReturn($dateNow); + + $this->clientMapper->method('getByUid') + ->with(42) + ->willThrowException(new ClientNotFoundException()); + + $this->assertEquals($expected, $this->oauthApiController->getToken('authorization_code', 'validcode', null, null, null)); + } + + public function testRefreshTokenInvalidRefreshToken() { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); @@ -139,7 +195,7 @@ public function testGetTokenInvalidRefreshToken() { $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'invalidrefresh', null, null)); } - public function testGetTokenClientDoesNotExist() { + public function testRefreshTokenClientDoesNotExist() { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); @@ -173,7 +229,7 @@ public function invalidClientProvider() { * @param string $clientId * @param string $clientSecret */ - public function testGetTokenInvalidClient($clientId, $clientSecret) { + public function testRefreshTokenInvalidClient($clientId, $clientSecret) { $expected = new JSONResponse([ 'error' => 'invalid_client', ], Http::STATUS_BAD_REQUEST); @@ -196,7 +252,7 @@ public function testGetTokenInvalidClient($clientId, $clientSecret) { $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', $clientId, $clientSecret)); } - public function testGetTokenInvalidAppToken() { + public function testRefreshTokenInvalidAppToken() { $expected = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); @@ -240,7 +296,7 @@ public function testGetTokenInvalidAppToken() { $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret')); } - public function testGetTokenValidAppToken() { + public function testRefreshTokenValidAppToken() { $accessToken = new AccessToken(); $accessToken->setClientId(42); $accessToken->setTokenId(1337); @@ -337,7 +393,7 @@ public function testGetTokenValidAppToken() { $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret')); } - public function testGetTokenValidAppTokenBasicAuth() { + public function testRefreshTokenValidAppTokenBasicAuth() { $accessToken = new AccessToken(); $accessToken->setClientId(42); $accessToken->setTokenId(1337); @@ -437,7 +493,7 @@ public function testGetTokenValidAppTokenBasicAuth() { $this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', null, null)); } - public function testGetTokenExpiredAppToken() { + public function testRefreshTokenExpiredAppToken() { $accessToken = new AccessToken(); $accessToken->setClientId(42); $accessToken->setTokenId(1337); From 7bba41099753ca3e28ae5ee22f2460e5cd989250 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 29 Aug 2023 11:57:00 +0200 Subject: [PATCH 03/13] cleanup access tokens that are still in authorization state and that have expired Signed-off-by: Julien Veyssier --- apps/oauth2/appinfo/info.xml | 6 +- .../CleanupExpiredAuthorizationCode.php | 61 +++++++++++++++++++ .../lib/Controller/OauthApiController.php | 5 ++ apps/oauth2/lib/Db/AccessToken.php | 5 ++ apps/oauth2/lib/Db/AccessTokenMapper.php | 31 ++++++++-- .../Version011603Date20230620111039.php | 24 ++++++++ 6 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php diff --git a/apps/oauth2/appinfo/info.xml b/apps/oauth2/appinfo/info.xml index bc31d12f161b6..fd17afbb11f73 100644 --- a/apps/oauth2/appinfo/info.xml +++ b/apps/oauth2/appinfo/info.xml @@ -5,7 +5,7 @@ OAuth 2.0 Allows OAuth2 compatible authentication from other web applications. The OAuth2 app allows administrators to configure the built-in authentication workflow to also allow OAuth2 compatible authentication from other web applications. - 1.16.2 + 1.16.3 agpl Lukas Reschke OAuth2 @@ -19,6 +19,10 @@ + + OCA\OAuth2\BackgroundJob\CleanupExpiredAuthorizationCode + + OCA\OAuth2\Migration\SetTokenExpiration diff --git a/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php b/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php new file mode 100644 index 0000000000000..00d014140e300 --- /dev/null +++ b/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php @@ -0,0 +1,61 @@ + + * + * @author Julien Veyssier + * + * @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 . + */ + + +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) { + try { + $this->accessTokenMapper->cleanupExpiredAuthorizationCode(); + } catch (Exception $e) { + $this->logger->warning('Failed to cleanup tokens with expired authorization code', ['exception' => $e]); + } + } +} diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index d9646363f5fa8..2ac492bd6acd6 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -194,6 +194,11 @@ public function getToken( $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()]); diff --git a/apps/oauth2/lib/Db/AccessToken.php b/apps/oauth2/lib/Db/AccessToken.php index 99404850e4f42..5fbfb87bc8b29 100644 --- a/apps/oauth2/lib/Db/AccessToken.php +++ b/apps/oauth2/lib/Db/AccessToken.php @@ -36,6 +36,8 @@ * @method void setHashedCode(string $token) * @method int getCreatedAt() * @method void setCreatedAt(int $createdAt) + * @method int getTokenCount() + * @method void setTokenCount(int $tokenCount) */ class AccessToken extends Entity { /** @var int */ @@ -48,6 +50,8 @@ class AccessToken extends Entity { protected $encryptedToken; /** @var int */ protected $createdAt; + /** @var int */ + protected $tokenCount; public function __construct() { $this->addType('id', 'int'); @@ -56,5 +60,6 @@ public function __construct() { $this->addType('hashedCode', 'string'); $this->addType('encryptedToken', 'string'); $this->addType('created_at', 'int'); + $this->addType('token_count', 'int'); } } diff --git a/apps/oauth2/lib/Db/AccessTokenMapper.php b/apps/oauth2/lib/Db/AccessTokenMapper.php index da351d5d4966f..c62fc4d2b5f70 100644 --- a/apps/oauth2/lib/Db/AccessTokenMapper.php +++ b/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('created_at', $qb->createNamedParameter($maxTokenCreationTs, IQueryBuilder::PARAM_INT))); + $qb->executeStatement(); + } } diff --git a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php index 279bf6cffaa8e..ddf2739b125d1 100644 --- a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php +++ b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php @@ -27,13 +27,16 @@ 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, ) { } @@ -43,15 +46,36 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt if ($schema->hasTable('oauth2_access_tokens')) { $table = $schema->getTable('oauth2_access_tokens'); + $dbChanged = false; + if (!$table->hasColumn('created_at') || !$table->hasColumn('token_count')) { + $dbChanged = true; + } if (!$table->hasColumn('created_at')) { $table->addColumn('created_at', Types::BIGINT, [ 'notnull' => true, 'default' => 0, ]); + } + if (!$table->hasColumn('token_count')) { + $table->addColumn('token_count', Types::BIGINT, [ + 'notnull' => true, + 'default' => 0, + ]); + } + if ($dbChanged) { return $schema; } } return null; } + + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + // 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(); + } } From 1ab45bad5d20a62161448c29eb1c3282c1813649 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 29 Aug 2023 12:12:36 +0200 Subject: [PATCH 04/13] refuse oauth authorization code if a token has already been delivered (active token) Signed-off-by: Julien Veyssier --- apps/oauth2/lib/Controller/OauthApiController.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 2ac492bd6acd6..d1eda92b22807 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -113,8 +113,18 @@ public function getToken( return $response; } - // check authorization code expiration 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(); $tokenCreatedAt = $accessToken->getCreatedAt(); if ($tokenCreatedAt < $now - self::AUTHORIZATION_CODE_EXPIRES_AFTER) { From 779e1d51ac1d50c5625a1cc403d732d74b364ccf Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 29 Aug 2023 12:13:25 +0200 Subject: [PATCH 05/13] delete oauth access token when receiving a code that has expired Signed-off-by: Julien Veyssier --- apps/oauth2/lib/Controller/OauthApiController.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index d1eda92b22807..ecf0062918b44 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -128,6 +128,9 @@ public function getToken( $now = $this->timeFactory->now()->getTimestamp(); $tokenCreatedAt = $accessToken->getCreatedAt(); if ($tokenCreatedAt < $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); From ddfc124767a211e4007c11a016633b33a3b1ca76 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 29 Aug 2023 12:37:30 +0200 Subject: [PATCH 06/13] add test for refusing to get an oauth token from a code when we're not in authorization state Signed-off-by: Julien Veyssier --- .../Controller/OauthApiControllerTest.php | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/apps/oauth2/tests/Controller/OauthApiControllerTest.php b/apps/oauth2/tests/Controller/OauthApiControllerTest.php index f9db388713bc0..2ff49b92fa76f 100644 --- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php +++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php @@ -151,6 +151,33 @@ public function testGetTokenExpiredCode() { $this->assertEquals($expected, $this->oauthApiController->getToken('authorization_code', 'validcode', null, null, null)); } + public function testGetTokenWithCodeForActiveToken() { + // if a token has already delivered oauth tokens, + // it should not be possible to get a new oauth token from a valid authorization code + $tokenCreatedAt = 100; + + $expected = new JSONResponse([ + 'error' => 'invalid_request', + ], Http::STATUS_BAD_REQUEST); + $expected->throttle(['invalid_request' => 'authorization_code_received_for_active_token']); + + $accessToken = new AccessToken(); + $accessToken->setClientId(42); + $accessToken->setCreatedAt($tokenCreatedAt); + $accessToken->setTokenCount(1); + + $this->accessTokenMapper->method('getByCode') + ->with('validcode') + ->willReturn($accessToken); + + $tsNow = $tokenCreatedAt + 1; + $dateNow = (new \DateTimeImmutable())->setTimestamp($tsNow); + $this->timeFactory->method('now') + ->willReturn($dateNow); + + $this->assertEquals($expected, $this->oauthApiController->getToken('authorization_code', 'validcode', null, null, null)); + } + public function testGetTokenClientDoesNotExist() { // In this test, the token's authorization code is valid and has not expired // and we check what happens when the associated Oauth client does not exist From e944980eb65893ce0ebeeb691bee85734ef8801a Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Fri, 1 Sep 2023 10:34:10 +0200 Subject: [PATCH 07/13] add db index on oauth2_access_tokens's (token_count, created_at) Signed-off-by: Julien Veyssier --- .../lib/Migration/Version011603Date20230620111039.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php index ddf2739b125d1..06cc4db5ab35b 100644 --- a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php +++ b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php @@ -47,20 +47,23 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt if ($schema->hasTable('oauth2_access_tokens')) { $table = $schema->getTable('oauth2_access_tokens'); $dbChanged = false; - if (!$table->hasColumn('created_at') || !$table->hasColumn('token_count')) { - $dbChanged = true; - } if (!$table->hasColumn('created_at')) { $table->addColumn('created_at', Types::BIGINT, [ 'notnull' => true, 'default' => 0, ]); + $dbChanged = true; } if (!$table->hasColumn('token_count')) { $table->addColumn('token_count', Types::BIGINT, [ 'notnull' => true, 'default' => 0, ]); + $dbChanged = true; + } + if (!$table->hasIndex('oauth2_tk_c_created_idx')) { + $table->addIndex(['token_count', 'created_at'], 'oauth2_tk_c_created_idx'); + $dbChanged = true; } if ($dbChanged) { return $schema; From c6da99474eca89cb991ca4c398c008088b324eec Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Wed, 13 Sep 2023 13:55:48 +0200 Subject: [PATCH 08/13] rename oauth2_access_token's created_at to code_created_at Signed-off-by: Julien Veyssier --- .../lib/Controller/OauthApiController.php | 6 +++--- apps/oauth2/lib/Db/AccessToken.php | 8 ++++---- apps/oauth2/lib/Db/AccessTokenMapper.php | 2 +- .../Version011603Date20230620111039.php | 6 +++--- .../Controller/OauthApiControllerTest.php | 18 +++++++++--------- core/Controller/ClientFlowLoginController.php | 2 +- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index ecf0062918b44..bb0f180bff991 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -126,15 +126,15 @@ public function getToken( // check authorization code expiration $now = $this->timeFactory->now()->getTimestamp(); - $tokenCreatedAt = $accessToken->getCreatedAt(); - if ($tokenCreatedAt < $now - self::AUTHORIZATION_CODE_EXPIRES_AFTER) { + $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 - $tokenCreatedAt; + $expiredSince = $now - self::AUTHORIZATION_CODE_EXPIRES_AFTER - $codeCreatedAt; $response->throttle(['invalid_request' => 'authorization_code_expired', 'expired_since' => $expiredSince]); return $response; } diff --git a/apps/oauth2/lib/Db/AccessToken.php b/apps/oauth2/lib/Db/AccessToken.php index 5fbfb87bc8b29..1a31b4c250449 100644 --- a/apps/oauth2/lib/Db/AccessToken.php +++ b/apps/oauth2/lib/Db/AccessToken.php @@ -34,8 +34,8 @@ * @method void setEncryptedToken(string $token) * @method string getHashedCode() * @method void setHashedCode(string $token) - * @method int getCreatedAt() - * @method void setCreatedAt(int $createdAt) + * @method int getCodeCreatedAt() + * @method void setCodeCreatedAt(int $createdAt) * @method int getTokenCount() * @method void setTokenCount(int $tokenCount) */ @@ -49,7 +49,7 @@ class AccessToken extends Entity { /** @var string */ protected $encryptedToken; /** @var int */ - protected $createdAt; + protected $codeCreatedAt; /** @var int */ protected $tokenCount; @@ -59,7 +59,7 @@ public function __construct() { $this->addType('clientId', 'int'); $this->addType('hashedCode', 'string'); $this->addType('encryptedToken', 'string'); - $this->addType('created_at', 'int'); + $this->addType('code_created_at', 'int'); $this->addType('token_count', 'int'); } } diff --git a/apps/oauth2/lib/Db/AccessTokenMapper.php b/apps/oauth2/lib/Db/AccessTokenMapper.php index c62fc4d2b5f70..0347a43d7c62d 100644 --- a/apps/oauth2/lib/Db/AccessTokenMapper.php +++ b/apps/oauth2/lib/Db/AccessTokenMapper.php @@ -99,7 +99,7 @@ public function cleanupExpiredAuthorizationCode(): void { $qb ->delete($this->tableName) ->where($qb->expr()->eq('token_count', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))) - ->andWhere($qb->expr()->lt('created_at', $qb->createNamedParameter($maxTokenCreationTs, IQueryBuilder::PARAM_INT))); + ->andWhere($qb->expr()->lt('code_created_at', $qb->createNamedParameter($maxTokenCreationTs, IQueryBuilder::PARAM_INT))); $qb->executeStatement(); } } diff --git a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php index 06cc4db5ab35b..02dd8a38aabf3 100644 --- a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php +++ b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php @@ -47,8 +47,8 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt if ($schema->hasTable('oauth2_access_tokens')) { $table = $schema->getTable('oauth2_access_tokens'); $dbChanged = false; - if (!$table->hasColumn('created_at')) { - $table->addColumn('created_at', Types::BIGINT, [ + if (!$table->hasColumn('code_created_at')) { + $table->addColumn('code_created_at', Types::BIGINT, [ 'notnull' => true, 'default' => 0, ]); @@ -62,7 +62,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $dbChanged = true; } if (!$table->hasIndex('oauth2_tk_c_created_idx')) { - $table->addIndex(['token_count', 'created_at'], 'oauth2_tk_c_created_idx'); + $table->addIndex(['token_count', 'code_created_at'], 'oauth2_tk_c_created_idx'); $dbChanged = true; } if ($dbChanged) { diff --git a/apps/oauth2/tests/Controller/OauthApiControllerTest.php b/apps/oauth2/tests/Controller/OauthApiControllerTest.php index 2ff49b92fa76f..eec38890e05b5 100644 --- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php +++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php @@ -127,7 +127,7 @@ public function testGetTokenInvalidCode() { } public function testGetTokenExpiredCode() { - $tokenCreatedAt = 100; + $codeCreatedAt = 100; $expiredSince = 123; $expected = new JSONResponse([ @@ -137,13 +137,13 @@ public function testGetTokenExpiredCode() { $accessToken = new AccessToken(); $accessToken->setClientId(42); - $accessToken->setCreatedAt($tokenCreatedAt); + $accessToken->setCodeCreatedAt($codeCreatedAt); $this->accessTokenMapper->method('getByCode') ->with('validcode') ->willReturn($accessToken); - $tsNow = $tokenCreatedAt + OauthApiController::AUTHORIZATION_CODE_EXPIRES_AFTER + $expiredSince; + $tsNow = $codeCreatedAt + OauthApiController::AUTHORIZATION_CODE_EXPIRES_AFTER + $expiredSince; $dateNow = (new \DateTimeImmutable())->setTimestamp($tsNow); $this->timeFactory->method('now') ->willReturn($dateNow); @@ -154,7 +154,7 @@ public function testGetTokenExpiredCode() { public function testGetTokenWithCodeForActiveToken() { // if a token has already delivered oauth tokens, // it should not be possible to get a new oauth token from a valid authorization code - $tokenCreatedAt = 100; + $codeCreatedAt = 100; $expected = new JSONResponse([ 'error' => 'invalid_request', @@ -163,14 +163,14 @@ public function testGetTokenWithCodeForActiveToken() { $accessToken = new AccessToken(); $accessToken->setClientId(42); - $accessToken->setCreatedAt($tokenCreatedAt); + $accessToken->setCodeCreatedAt($codeCreatedAt); $accessToken->setTokenCount(1); $this->accessTokenMapper->method('getByCode') ->with('validcode') ->willReturn($accessToken); - $tsNow = $tokenCreatedAt + 1; + $tsNow = $codeCreatedAt + 1; $dateNow = (new \DateTimeImmutable())->setTimestamp($tsNow); $this->timeFactory->method('now') ->willReturn($dateNow); @@ -181,7 +181,7 @@ public function testGetTokenWithCodeForActiveToken() { public function testGetTokenClientDoesNotExist() { // In this test, the token's authorization code is valid and has not expired // and we check what happens when the associated Oauth client does not exist - $tokenCreatedAt = 100; + $codeCreatedAt = 100; $expected = new JSONResponse([ 'error' => 'invalid_request', @@ -190,14 +190,14 @@ public function testGetTokenClientDoesNotExist() { $accessToken = new AccessToken(); $accessToken->setClientId(42); - $accessToken->setCreatedAt($tokenCreatedAt); + $accessToken->setCodeCreatedAt($codeCreatedAt); $this->accessTokenMapper->method('getByCode') ->with('validcode') ->willReturn($accessToken); // 'now' is before the token's authorization code expiration - $tsNow = $tokenCreatedAt + OauthApiController::AUTHORIZATION_CODE_EXPIRES_AFTER - 1; + $tsNow = $codeCreatedAt + OauthApiController::AUTHORIZATION_CODE_EXPIRES_AFTER - 1; $dateNow = (new \DateTimeImmutable())->setTimestamp($tsNow); $this->timeFactory->method('now') ->willReturn($dateNow); diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 05321c9222a17..0a073a586e42e 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -289,7 +289,7 @@ public function generateAppPassword(string $stateToken, $accessToken->setEncryptedToken($this->crypto->encrypt($token, $code)); $accessToken->setHashedCode(hash('sha512', $code)); $accessToken->setTokenId($generatedToken->getId()); - $accessToken->setCreatedAt($this->timeFactory->now()->getTimestamp()); + $accessToken->setCodeCreatedAt($this->timeFactory->now()->getTimestamp()); $this->accessTokenMapper->insert($accessToken); $redirectUri = $client->getRedirectUri(); From 32f984c52084c3d04f579a41b704e8d30c81ed96 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Wed, 4 Oct 2023 11:52:19 +0200 Subject: [PATCH 09/13] adjust oauth tests Signed-off-by: Julien Veyssier --- apps/oauth2/lib/Db/AccessToken.php | 4 ++-- apps/oauth2/tests/Db/AccessTokenMapperTest.php | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/oauth2/lib/Db/AccessToken.php b/apps/oauth2/lib/Db/AccessToken.php index 1a31b4c250449..e0da141af3fd6 100644 --- a/apps/oauth2/lib/Db/AccessToken.php +++ b/apps/oauth2/lib/Db/AccessToken.php @@ -59,7 +59,7 @@ public function __construct() { $this->addType('clientId', 'int'); $this->addType('hashedCode', 'string'); $this->addType('encryptedToken', 'string'); - $this->addType('code_created_at', 'int'); - $this->addType('token_count', 'int'); + $this->addType('codeCreatedAt', 'int'); + $this->addType('tokenCount', 'int'); } } diff --git a/apps/oauth2/tests/Db/AccessTokenMapperTest.php b/apps/oauth2/tests/Db/AccessTokenMapperTest.php index 013eb72919a28..b7de147963a10 100644 --- a/apps/oauth2/tests/Db/AccessTokenMapperTest.php +++ b/apps/oauth2/tests/Db/AccessTokenMapperTest.php @@ -25,6 +25,7 @@ use OCA\OAuth2\Db\AccessToken; use OCA\OAuth2\Db\AccessTokenMapper; +use OCP\AppFramework\Utility\ITimeFactory; use Test\TestCase; /** @@ -36,7 +37,7 @@ class AccessTokenMapperTest extends TestCase { protected function setUp(): void { parent::setUp(); - $this->accessTokenMapper = new AccessTokenMapper(\OC::$server->getDatabaseConnection()); + $this->accessTokenMapper = new AccessTokenMapper(\OC::$server->getDatabaseConnection(), \OC::$server->get(ITimeFactory::class)); } public function testGetByCode() { @@ -54,7 +55,7 @@ public function testGetByCode() { $this->accessTokenMapper->delete($token); } - + public function testDeleteByClientId() { $this->expectException(\OCA\OAuth2\Exceptions\AccessTokenNotFoundException::class); From d2bc483adf6e714c5960f07989e1247bb96f7190 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Wed, 4 Oct 2023 13:20:57 +0200 Subject: [PATCH 10/13] adjust oauth app Signed-off-by: Julien Veyssier --- .../lib/BackgroundJob/CleanupExpiredAuthorizationCode.php | 2 +- .../lib/Migration/Version011603Date20230620111039.php | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php b/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php index 00d014140e300..f4e08081ce6ed 100644 --- a/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php +++ b/apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php @@ -51,7 +51,7 @@ public function __construct( * @param mixed $argument * @inheritDoc */ - protected function run($argument) { + protected function run($argument): void { try { $this->accessTokenMapper->cleanupExpiredAuthorizationCode(); } catch (Exception $e) { diff --git a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php index 02dd8a38aabf3..b694963b0f75d 100644 --- a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php +++ b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php @@ -40,7 +40,7 @@ public function __construct( ) { } - public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) { + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { /** @var ISchemaWrapper $schema */ $schema = $schemaClosure(); @@ -51,6 +51,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $table->addColumn('code_created_at', Types::BIGINT, [ 'notnull' => true, 'default' => 0, + 'unsigned' => true, ]); $dbChanged = true; } @@ -58,6 +59,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt $table->addColumn('token_count', Types::BIGINT, [ 'notnull' => true, 'default' => 0, + 'unsigned' => true, ]); $dbChanged = true; } @@ -73,7 +75,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt return null; } - public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) { + 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(); From da63d3c27b6b1192468ca33a438b67902a80133a Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Wed, 4 Oct 2023 13:23:30 +0200 Subject: [PATCH 11/13] update autoload files Signed-off-by: Julien Veyssier --- apps/oauth2/composer/composer/autoload_classmap.php | 1 + apps/oauth2/composer/composer/autoload_static.php | 1 + 2 files changed, 2 insertions(+) diff --git a/apps/oauth2/composer/composer/autoload_classmap.php b/apps/oauth2/composer/composer/autoload_classmap.php index 77068f4e9c022..536342c075113 100644 --- a/apps/oauth2/composer/composer/autoload_classmap.php +++ b/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', diff --git a/apps/oauth2/composer/composer/autoload_static.php b/apps/oauth2/composer/composer/autoload_static.php index 2fb8a4f17a9f1..1dc659778a3d3 100644 --- a/apps/oauth2/composer/composer/autoload_static.php +++ b/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', From 98c8a465b0976dc39a0ab75cbeb625a79cadcaa3 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Wed, 4 Oct 2023 13:27:16 +0200 Subject: [PATCH 12/13] update OpenAPI specs Signed-off-by: Julien Veyssier --- apps/oauth2/openapi.json | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/apps/oauth2/openapi.json b/apps/oauth2/openapi.json index a9903ae3473e5..c745e73bf7938 100644 --- a/apps/oauth2/openapi.json +++ b/apps/oauth2/openapi.json @@ -121,40 +121,50 @@ "name": "code", "in": "query", "description": "Code of the flow", - "required": true, "schema": { - "type": "string" + "type": "string", + "nullable": true } }, { "name": "refresh_token", "in": "query", "description": "Refresh token", - "required": true, "schema": { - "type": "string" + "type": "string", + "nullable": true } }, { "name": "client_id", "in": "query", "description": "Client ID", - "required": true, "schema": { - "type": "string" + "type": "string", + "nullable": true } }, { "name": "client_secret", "in": "query", "description": "Client secret", - "required": true, "schema": { - "type": "string" + "type": "string", + "nullable": true } } ], "responses": { + "500": { + "description": "", + "content": { + "text/plain": { + "schema": { + "type": "string" + } + } + } + }, "200": { "description": "Token returned", "content": { From d56950a6c92635ca75dd9925ce5c4ad776291ea3 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Wed, 4 Oct 2023 13:51:05 +0200 Subject: [PATCH 13/13] adjust phpdoc types in OauthApiController Signed-off-by: Julien Veyssier --- apps/oauth2/lib/Controller/OauthApiController.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index bb0f180bff991..dfb952a0951c0 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -74,10 +74,10 @@ public function __construct( * Get a token * * @param string $grant_type Token type that should be granted - * @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 + * @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 * @throws Exception * @return JSONResponse|JSONResponse *