Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable26] Make OAuth2 authorization code expire #43019

Merged
merged 3 commits into from Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion apps/oauth2/appinfo/info.xml
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.14.1</version>
<version>1.14.2</version>
<licence>agpl</licence>
<author>Lukas Reschke</author>
<namespace>OAuth2</namespace>
Expand All @@ -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>
Expand Down
2 changes: 2 additions & 0 deletions apps/oauth2/composer/composer/autoload_classmap.php
Expand Up @@ -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',
Expand All @@ -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',
);
2 changes: 2 additions & 0 deletions apps/oauth2/composer/composer/autoload_static.php
Expand Up @@ -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',
Expand All @@ -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',
);

Expand Down
61 changes: 61 additions & 0 deletions apps/oauth2/lib/BackgroundJob/CleanupExpiredAuthorizationCode.php
@@ -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]);
}
}
}
79 changes: 61 additions & 18 deletions apps/oauth2/lib/Controller/OauthApiController.php
Expand Up @@ -27,6 +27,7 @@
*/
namespace OCA\OAuth2\Controller;

use DateTime;
use OC\Authentication\Exceptions\ExpiredTokenException;
use OC\Authentication\Exceptions\InvalidTokenException;
use OC\Authentication\Token\IProvider as TokenProvider;
Expand All @@ -39,12 +40,16 @@
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 */
Expand All @@ -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;
}
Expand All @@ -89,13 +96,17 @@ public function __construct(string $appName,
* @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 @@ -121,6 +132,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 = (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) {
Expand Down Expand Up @@ -181,13 +219,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()]);
Expand Down
10 changes: 10 additions & 0 deletions apps/oauth2/lib/Db/AccessToken.php
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 @@
protected $hashedCode;
/** @var string */
protected $encryptedToken;
/** @var int */
protected $codeCreatedAt;

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor Note

Property OCA\OAuth2\Db\AccessToken::$codeCreatedAt is not defined in constructor of OCA\OAuth2\Db\AccessToken or in any methods called in the constructor
/** @var int */
protected $tokenCount;

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor Note

Property OCA\OAuth2\Db\AccessToken::$tokenCount is not defined in constructor of OCA\OAuth2\Db\AccessToken or in any methods called in the constructor

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');
}
}
30 changes: 26 additions & 4 deletions apps/oauth2/lib/Db/AccessTokenMapper.php
Expand Up @@ -28,9 +28,12 @@
*/
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;

Expand All @@ -39,10 +42,9 @@
*/
class AccessTokenMapper extends QBMapper {

/**
* @param IDBConnection $db
*/
public function __construct(IDBConnection $db) {
public function __construct(
IDBConnection $db,
) {
parent::__construct($db, 'oauth2_access_tokens');
}

Expand Down Expand Up @@ -79,4 +81,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 = (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();
}
}