From 3f19bf7660b3659cab2ba5345a930425423ff345 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 22 Jan 2024 12:34:47 +0100 Subject: [PATCH 1/2] make OAuth2 authorization code expire Signed-off-by: Julien Veyssier --- apps/oauth2/appinfo/info.xml | 6 +- .../CleanupExpiredAuthorizationCode.php | 61 ++++++++++ .../lib/Controller/OauthApiController.php | 55 +++++++-- apps/oauth2/lib/Db/AccessToken.php | 10 ++ apps/oauth2/lib/Db/AccessTokenMapper.php | 31 ++++- .../Version011603Date20230620111039.php | 86 ++++++++++++++ .../Controller/OauthApiControllerTest.php | 111 +++++++++++++++--- .../oauth2/tests/Db/AccessTokenMapperTest.php | 5 +- core/Controller/ClientFlowLoginController.php | 33 ++++-- .../ClientFlowLoginControllerTest.php | 7 +- 10 files changed, 362 insertions(+), 43 deletions(-) create mode 100644 apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php create mode 100644 apps/oauth2/lib/Migration/Version011603Date20230620111039.php diff --git a/apps/oauth2/appinfo/info.xml b/apps/oauth2/appinfo/info.xml index d1dc6aa316fab..716e9ecd362d0 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.15.1 + 1.15.2 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..9428424362868 --- /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): void { + 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 e07a2c2de1579..dea8bd5c83da1 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/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()]); diff --git a/apps/oauth2/lib/Db/AccessToken.php b/apps/oauth2/lib/Db/AccessToken.php index 26c830c64adf1..e0da141af3fd6 100644 --- a/apps/oauth2/lib/Db/AccessToken.php +++ b/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,6 +48,10 @@ 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'); @@ -51,5 +59,7 @@ public function __construct() { $this->addType('clientId', 'int'); $this->addType('hashedCode', 'string'); $this->addType('encryptedToken', 'string'); + $this->addType('codeCreatedAt', 'int'); + $this->addType('tokenCount', 'int'); } } diff --git a/apps/oauth2/lib/Db/AccessTokenMapper.php b/apps/oauth2/lib/Db/AccessTokenMapper.php index da351d5d4966f..0347a43d7c62d 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('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 new file mode 100644 index 0000000000000..b694963b0f75d --- /dev/null +++ b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php @@ -0,0 +1,86 @@ + + * + * @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\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(); + } +} diff --git a/apps/oauth2/tests/Controller/OauthApiControllerTest.php b/apps/oauth2/tests/Controller/OauthApiControllerTest.php index c65302532a937..6faef7c88f31f 100644 --- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php +++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php @@ -65,7 +65,7 @@ class OauthApiControllerTest extends TestCase { /** @var ISecureRandom|\PHPUnit\Framework\MockObject\MockObject */ private $secureRandom; /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ - private $time; + private $timeFactory; /** @var Throttler|\PHPUnit\Framework\MockObject\MockObject */ private $throttler; /** @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject */ @@ -82,7 +82,7 @@ protected function setUp(): void { $this->clientMapper = $this->createMock(ClientMapper::class); $this->tokenProvider = $this->createMock(TokenProvider::class); $this->secureRandom = $this->createMock(ISecureRandom::class); - $this->time = $this->createMock(ITimeFactory::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); $this->throttler = $this->createMock(Throttler::class); $this->logger = $this->createMock(LoggerInterface::class); @@ -94,9 +94,9 @@ protected function setUp(): void { $this->clientMapper, $this->tokenProvider, $this->secureRandom, - $this->time, + $this->timeFactory, $this->logger, - $this->throttler + $this->throttler, ); } @@ -122,7 +122,90 @@ public function testGetTokenInvalidCode() { $this->assertEquals($expected, $this->oauthApiController->getToken('authorization_code', 'invalidcode', null, null, null)); } - public function testGetTokenInvalidRefreshToken() { + public function testGetTokenExpiredCode() { + $codeCreatedAt = 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->setCodeCreatedAt($codeCreatedAt); + + $this->accessTokenMapper->method('getByCode') + ->with('validcode') + ->willReturn($accessToken); + + $tsNow = $codeCreatedAt + 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 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 + $codeCreatedAt = 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->setCodeCreatedAt($codeCreatedAt); + $accessToken->setTokenCount(1); + + $this->accessTokenMapper->method('getByCode') + ->with('validcode') + ->willReturn($accessToken); + + $tsNow = $codeCreatedAt + 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 + $codeCreatedAt = 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->setCodeCreatedAt($codeCreatedAt); + + $this->accessTokenMapper->method('getByCode') + ->with('validcode') + ->willReturn($accessToken); + + // 'now' is before the token's authorization code expiration + $tsNow = $codeCreatedAt + 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); @@ -135,7 +218,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); @@ -169,7 +252,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); @@ -192,7 +275,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); @@ -236,7 +319,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); @@ -287,7 +370,7 @@ public function testGetTokenValidAppToken() { 'random72' )->willReturn($appToken); - $this->time->method('getTime') + $this->timeFactory->method('getTime') ->willReturn(1000); $this->tokenProvider->expects($this->once()) @@ -333,7 +416,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); @@ -384,7 +467,7 @@ public function testGetTokenValidAppTokenBasicAuth() { 'random72' )->willReturn($appToken); - $this->time->method('getTime') + $this->timeFactory->method('getTime') ->willReturn(1000); $this->tokenProvider->expects($this->once()) @@ -433,7 +516,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); @@ -484,7 +567,7 @@ public function testGetTokenExpiredAppToken() { 'random72' )->willReturn($appToken); - $this->time->method('getTime') + $this->timeFactory->method('getTime') ->willReturn(1000); $this->tokenProvider->expects($this->once()) 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); diff --git a/core/Controller/ClientFlowLoginController.php b/core/Controller/ClientFlowLoginController.php index 85a793bd92b1f..3179b577df344 100644 --- a/core/Controller/ClientFlowLoginController.php +++ b/core/Controller/ClientFlowLoginController.php @@ -44,6 +44,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; @@ -68,22 +69,26 @@ class ClientFlowLoginController extends Controller { private AccessTokenMapper $accessTokenMapper; private ICrypto $crypto; private IEventDispatcher $eventDispatcher; + private ITimeFactory $timeFactory; public const STATE_NAME = 'client.flow.state.token'; - public function __construct(string $appName, - IRequest $request, - IUserSession $userSession, - IL10N $l10n, - Defaults $defaults, - ISession $session, - IProvider $tokenProvider, - ISecureRandom $random, - IURLGenerator $urlGenerator, - ClientMapper $clientMapper, - AccessTokenMapper $accessTokenMapper, - ICrypto $crypto, - IEventDispatcher $eventDispatcher) { + public function __construct( + string $appName, + IRequest $request, + IUserSession $userSession, + IL10N $l10n, + Defaults $defaults, + ISession $session, + IProvider $tokenProvider, + ISecureRandom $random, + IURLGenerator $urlGenerator, + ClientMapper $clientMapper, + AccessTokenMapper $accessTokenMapper, + ICrypto $crypto, + IEventDispatcher $eventDispatcher, + ITimeFactory $timeFactory + ) { parent::__construct($appName, $request); $this->userSession = $userSession; $this->l10n = $l10n; @@ -96,6 +101,7 @@ public function __construct(string $appName, $this->accessTokenMapper = $accessTokenMapper; $this->crypto = $crypto; $this->eventDispatcher = $eventDispatcher; + $this->timeFactory = $timeFactory; } private function getClientName(): string { @@ -305,6 +311,7 @@ public function generateAppPassword(string $stateToken, $accessToken->setEncryptedToken($this->crypto->encrypt($token, $code)); $accessToken->setHashedCode(hash('sha512', $code)); $accessToken->setTokenId($generatedToken->getId()); + $accessToken->setCodeCreatedAt($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..f8337d2fcaa6d 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; @@ -69,6 +70,8 @@ class ClientFlowLoginControllerTest extends TestCase { private $crypto; /** @var IEventDispatcher|\PHPUnit\Framework\MockObject\MockObject */ private $eventDispatcher; + /** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */ + private $timeFactory; /** @var ClientFlowLoginController */ @@ -95,6 +98,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 +113,8 @@ protected function setUp(): void { $this->clientMapper, $this->accessTokenMapper, $this->crypto, - $this->eventDispatcher + $this->eventDispatcher, + $this->timeFactory ); } From 8dada6a33400631a6b82cb7724228bd56ee21c38 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Mon, 22 Jan 2024 12:37:33 +0100 Subject: [PATCH 2/2] generate oauth2 autoloaders Signed-off-by: Julien Veyssier --- apps/oauth2/composer/composer/autoload_classmap.php | 2 ++ apps/oauth2/composer/composer/autoload_static.php | 2 ++ 2 files changed, 4 insertions(+) diff --git a/apps/oauth2/composer/composer/autoload_classmap.php b/apps/oauth2/composer/composer/autoload_classmap.php index 09cacb20335f9..b2f24b59c772a 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', @@ -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', ); diff --git a/apps/oauth2/composer/composer/autoload_static.php b/apps/oauth2/composer/composer/autoload_static.php index 1442093e32fb9..ad4983efbb9ab 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', @@ -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', );