make OAuth2 authorization code expire

Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
pull/43019/head
Julien Veyssier 4 months ago
parent 529b3d068f
commit 9805c0f137
No known key found for this signature in database
GPG Key ID: 4141FEE162030638

@ -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.14.1</version>
<version>1.14.2</version>
<licence>agpl</licence>
<author>Lukas Reschke</author>
<namespace>OAuth2</namespace>
@ -19,6 +19,10 @@
<nextcloud min-version="26" max-version="26"/>
</dependencies>
<background-jobs>
<job>OCA\OAuth2\BackgroundJob\CleanupExpiredAuthorizationCode</job>
</background-jobs>
<repair-steps>
<post-migration>
<step>OCA\OAuth2\Migration\SetTokenExpiration</step>

@ -0,0 +1,61 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2024 Julien Veyssier <julien-nc@posteo.net>
*
* @author Julien Veyssier <julien-nc@posteo.net>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
namespace OCA\OAuth2\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]);
}
}
}

@ -27,6 +27,7 @@ declare(strict_types=1);
*/
namespace OCA\OAuth2\Controller;
use DateTime;
use OC\Authentication\Exceptions\ExpiredTokenException;
use OC\Authentication\Exceptions\InvalidTokenException;
use OC\Authentication\Token\IProvider as TokenProvider;
@ -39,12 +40,16 @@ use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\DB\Exception;
use OCP\IRequest;
use OCP\Security\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;
/** @var AccessTokenMapper */
private $accessTokenMapper;
/** @var ClientMapper */
@ -56,29 +61,31 @@ class OauthApiController extends Controller {
/** @var ISecureRandom */
private $secureRandom;
/** @var ITimeFactory */
private $time;
private $timeFactory;
/** @var Throttler */
private $throttler;
/** @var LoggerInterface */
private $logger;
public function __construct(string $appName,
IRequest $request,
ICrypto $crypto,
AccessTokenMapper $accessTokenMapper,
ClientMapper $clientMapper,
TokenProvider $tokenProvider,
ISecureRandom $secureRandom,
ITimeFactory $time,
LoggerInterface $logger,
Throttler $throttler) {
public function __construct(
string $appName,
IRequest $request,
ICrypto $crypto,
AccessTokenMapper $accessTokenMapper,
ClientMapper $clientMapper,
TokenProvider $tokenProvider,
ISecureRandom $secureRandom,
ITimeFactory $timeFactory,
LoggerInterface $logger,
Throttler $throttler
) {
parent::__construct($appName, $request);
$this->crypto = $crypto;
$this->accessTokenMapper = $accessTokenMapper;
$this->clientMapper = $clientMapper;
$this->tokenProvider = $tokenProvider;
$this->secureRandom = $secureRandom;
$this->time = $time;
$this->timeFactory = $timeFactory;
$this->throttler = $throttler;
$this->logger = $logger;
}
@ -89,13 +96,17 @@ class OauthApiController extends Controller {
* @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') {
@ -121,6 +132,33 @@ class OauthApiController extends Controller {
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 = (new DateTime())->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) {
@ -181,13 +219,18 @@ class OauthApiController extends Controller {
);
// 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()]);

@ -34,6 +34,10 @@ use OCP\AppFramework\Db\Entity;
* @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 @@ class AccessToken extends Entity {
$this->addType('clientId', 'int');
$this->addType('hashedCode', 'string');
$this->addType('encryptedToken', 'string');
$this->addType('codeCreatedAt', 'int');
$this->addType('tokenCount', 'int');
}
}

@ -28,9 +28,12 @@ declare(strict_types=1);
*/
namespace OCA\OAuth2\Db;
use DateTime;
use OCA\OAuth2\Controller\OauthApiController;
use OCA\OAuth2\Exceptions\AccessTokenNotFoundException;
use OCP\AppFramework\Db\IMapperException;
use OCP\AppFramework\Db\QBMapper;
use OCP\DB\Exception;
use OCP\DB\QueryBuilder\IQueryBuilder;
use OCP\IDBConnection;
@ -39,10 +42,9 @@ use OCP\IDBConnection;
*/
class AccessTokenMapper extends QBMapper {
/**
* @param IDBConnection $db
*/
public function __construct(IDBConnection $db) {
public function __construct(
IDBConnection $db,
) {
parent::__construct($db, 'oauth2_access_tokens');
}
@ -79,4 +81,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 = (new DateTime())->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();
}
}

@ -0,0 +1,86 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright 2023, Julien Veyssier <julien-nc@posteo.net>
*
* @author Julien Veyssier <julien-nc@posteo.net>
*
* @license GNU AGPL version 3 or any later version
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
namespace OCA\OAuth2\Migration;
use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\DB\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();
}
}

@ -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 @@ class OauthApiControllerTest extends TestCase {
$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,7 +94,7 @@ class OauthApiControllerTest extends TestCase {
$this->clientMapper,
$this->tokenProvider,
$this->secureRandom,
$this->time,
$this->timeFactory,
$this->logger,
$this->throttler
);
@ -122,7 +122,90 @@ class OauthApiControllerTest extends TestCase {
$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 @@ class OauthApiControllerTest extends TestCase {
$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 @@ class OauthApiControllerTest extends TestCase {
* @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 @@ class OauthApiControllerTest extends TestCase {
$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 @@ class OauthApiControllerTest extends TestCase {
$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 @@ class OauthApiControllerTest extends TestCase {
'random72'
)->willReturn($appToken);
$this->time->method('getTime')
$this->timeFactory->method('getTime')
->willReturn(1000);
$this->tokenProvider->expects($this->once())
@ -333,7 +416,7 @@ class OauthApiControllerTest extends TestCase {
$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 @@ class OauthApiControllerTest extends TestCase {
'random72'
)->willReturn($appToken);
$this->time->method('getTime')
$this->timeFactory->method('getTime')
->willReturn(1000);
$this->tokenProvider->expects($this->once())
@ -433,7 +516,7 @@ class OauthApiControllerTest extends TestCase {
$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 @@ class OauthApiControllerTest extends TestCase {
'random72'
)->willReturn($appToken);
$this->time->method('getTime')
$this->timeFactory->method('getTime')
->willReturn(1000);
$this->tokenProvider->expects($this->once())

@ -31,6 +31,7 @@
*/
namespace OC\Core\Controller;
use DateTime;
use OC\Authentication\Events\AppPasswordCreatedEvent;
use OC\Authentication\Exceptions\InvalidTokenException;
use OC\Authentication\Exceptions\PasswordlessTokenException;
@ -305,6 +306,7 @@ class ClientFlowLoginController extends Controller {
$accessToken->setEncryptedToken($this->crypto->encrypt($token, $code));
$accessToken->setHashedCode(hash('sha512', $code));
$accessToken->setTokenId($generatedToken->getId());
$accessToken->setCodeCreatedAt((new DateTime())->getTimestamp());
$this->accessTokenMapper->insert($accessToken);
$redirectUri = $client->getRedirectUri();

Loading…
Cancel
Save