Skip to content

Commit

Permalink
make OAuth2 authorization code expire
Browse files Browse the repository at this point in the history
Signed-off-by: Julien Veyssier <[email protected]>
  • Loading branch information
julien-nc committed Jan 22, 2024
1 parent 9282aa9 commit 5571a4e
Show file tree
Hide file tree
Showing 10 changed files with 359 additions and 35 deletions.
6 changes: 5 additions & 1 deletion apps/oauth2/appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<name>OAuth 2.0</name>
<summary>Allows OAuth2 compatible authentication from other web applications.</summary>
<description>The OAuth2 app allows administrators to configure the built-in authentication workflow to also allow OAuth2 compatible authentication from other web applications.</description>
<version>1.15.1</version>
<version>1.15.2</version>
<licence>agpl</licence>
<author>Lukas Reschke</author>
<namespace>OAuth2</namespace>
Expand All @@ -19,6 +19,10 @@
<nextcloud min-version="27" max-version="27"/>
</dependencies>

<background-jobs>
<job>OCA\OAuth2\BackgroundJob\CleanupExpiredAuthorizationCode</job>
</background-jobs>

<repair-steps>
<post-migration>
<step>OCA\OAuth2\Migration\SetTokenExpiration</step>
Expand Down
61 changes: 61 additions & 0 deletions apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2024 Julien Veyssier <[email protected]>
*
* @author Julien Veyssier <[email protected]>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/


namespace OCA\OAuth2\BackgroundJob;

use OCA\OAuth2\Db\AccessTokenMapper;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\BackgroundJob\IJob;
use OCP\BackgroundJob\TimedJob;
use OCP\DB\Exception;
use Psr\Log\LoggerInterface;

class CleanupExpiredAuthorizationCode extends TimedJob {

public function __construct(
ITimeFactory $timeFactory,
private AccessTokenMapper $accessTokenMapper,
private LoggerInterface $logger,

) {
parent::__construct($timeFactory);
// 30 days
$this->setInterval(60 * 60 * 24 * 30);
$this->setTimeSensitivity(IJob::TIME_INSENSITIVE);
}

/**
* @param mixed $argument
* @inheritDoc
*/
protected function run($argument): void {
try {
$this->accessTokenMapper->cleanupExpiredAuthorizationCode();
} catch (Exception $e) {
$this->logger->warning('Failed to cleanup tokens with expired authorization code', ['exception' => $e]);
}
}
}
52 changes: 46 additions & 6 deletions apps/oauth2/lib/Controller/OauthApiController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -56,7 +59,8 @@ public function __construct(
private ISecureRandom $secureRandom,
private ITimeFactory $time,
private LoggerInterface $logger,
private Throttler $throttler
private Throttler $throttler,
private ITimeFactory $timeFactory,
) {
parent::__construct($appName, $request);
}
Expand All @@ -67,13 +71,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') {
Expand All @@ -99,6 +107,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) {
Expand Down Expand Up @@ -166,6 +201,11 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client
$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()]);
Expand Down
10 changes: 10 additions & 0 deletions apps/oauth2/lib/Db/AccessToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -44,12 +48,18 @@ class AccessToken extends Entity {
protected $hashedCode;
/** @var string */
protected $encryptedToken;
/** @var int */
protected $codeCreatedAt;
/** @var int */
protected $tokenCount;

public function __construct() {
$this->addType('id', 'int');
$this->addType('tokenId', 'int');
$this->addType('clientId', 'int');
$this->addType('hashedCode', 'string');
$this->addType('encryptedToken', 'string');
$this->addType('codeCreatedAt', 'int');
$this->addType('tokenCount', 'int');
}
}
31 changes: 27 additions & 4 deletions apps/oauth2/lib/Db/AccessTokenMapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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');
}

Expand Down Expand Up @@ -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();
}
}
86 changes: 86 additions & 0 deletions apps/oauth2/lib/Migration/Version011603Date20230620111039.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright 2023, Julien Veyssier <[email protected]>
*
* @author Julien Veyssier <[email protected]>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\OAuth2\Migration;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\DB\Types;
use OCP\IDBConnection;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

class Version011603Date20230620111039 extends SimpleMigrationStep {

public function __construct(
private IDBConnection $connection,
) {
}

public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

if ($schema->hasTable('oauth2_access_tokens')) {
$table = $schema->getTable('oauth2_access_tokens');
$dbChanged = false;
if (!$table->hasColumn('code_created_at')) {
$table->addColumn('code_created_at', Types::BIGINT, [
'notnull' => true,
'default' => 0,
'unsigned' => true,
]);
$dbChanged = true;
}
if (!$table->hasColumn('token_count')) {
$table->addColumn('token_count', Types::BIGINT, [
'notnull' => true,
'default' => 0,
'unsigned' => true,
]);
$dbChanged = true;
}
if (!$table->hasIndex('oauth2_tk_c_created_idx')) {
$table->addIndex(['token_count', 'code_created_at'], 'oauth2_tk_c_created_idx');
$dbChanged = true;
}
if ($dbChanged) {
return $schema;
}
}

return null;
}

public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
// we consider that existing access_tokens have already produced at least one oauth token
// which prevents cleaning them up
$qbUpdate = $this->connection->getQueryBuilder();
$qbUpdate->update('oauth2_access_tokens')
->set('token_count', $qbUpdate->createNamedParameter(1, IQueryBuilder::PARAM_INT));
$qbUpdate->executeStatement();
}
}
Loading

0 comments on commit 5571a4e

Please sign in to comment.