From 7bba41099753ca3e28ae5ee22f2460e5cd989250 Mon Sep 17 00:00:00 2001 From: Julien Veyssier Date: Tue, 29 Aug 2023 11:57:00 +0200 Subject: 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 ++++++++++++++++++++++ apps/oauth2/lib/Controller/OauthApiController.php | 5 ++ apps/oauth2/lib/Db/AccessToken.php | 5 ++ apps/oauth2/lib/Db/AccessTokenMapper.php | 31 +++++++++-- .../Migration/Version011603Date20230620111039.php | 24 +++++++++ 6 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php (limited to 'apps') diff --git a/apps/oauth2/appinfo/info.xml b/apps/oauth2/appinfo/info.xml index bc31d12f161..fd17afbb11f 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 00000000000..00d014140e3 --- /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 d9646363f5f..2ac492bd6ac 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -194,6 +194,11 @@ class OauthApiController extends Controller { $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 99404850e4f..5fbfb87bc8b 100644 --- a/apps/oauth2/lib/Db/AccessToken.php +++ b/apps/oauth2/lib/Db/AccessToken.php @@ -36,6 +36,8 @@ use OCP\AppFramework\Db\Entity; * @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 @@ class AccessToken extends Entity { $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 da351d5d496..c62fc4d2b5f 100644 --- a/apps/oauth2/lib/Db/AccessTokenMapper.php +++ b/apps/oauth2/lib/Db/AccessTokenMapper.php @@ -28,9 +28,12 @@ declare(strict_types=1); */ 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 @@ use OCP\IDBConnection; */ 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 @@ class AccessTokenMapper extends QBMapper { ->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 279bf6cffaa..ddf2739b125 100644 --- a/apps/oauth2/lib/Migration/Version011603Date20230620111039.php +++ b/apps/oauth2/lib/Migration/Version011603Date20230620111039.php @@ -27,13 +27,16 @@ 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, ) { } @@ -43,15 +46,36 @@ class Version011603Date20230620111039 extends SimpleMigrationStep { 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(); + } } -- cgit v1.2.3