upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-05-28 04:32:30 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 54

Merge pull request #59767 from nextcloud/enh/noid/safer-oauth2-gettoken

Browse source 
Wrap oauth2 token rotation in a transaction
This commit is contained in:
Christoph Wurst 2026-04-23 14:33:11 +02:00 committed by GitHub
commit 1a2712132a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 225 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

67
apps/oauth2/lib/Controller/OauthApiController.php
View file

@ -24,6 +24,7 @@ use OCP\AppFramework\Utility\ITimeFactory;
use OCP\Authentication\Exceptions\ExpiredTokenException;
use OCP\Authentication\Exceptions\InvalidTokenException;
use OCP\DB\Exception;
use OCP\IDBConnection;
use OCP\IRequest;
use OCP\Security\Bruteforce\IThrottler;
use OCP\Security\ICrypto;
@ -47,6 +48,7 @@ class OauthApiController extends Controller {
private LoggerInterface $logger,
private IThrottler $throttler,
private ITimeFactory $timeFactory,
private IDBConnection $db,
) {
parent::__construct($appName, $request);
}
@ -177,27 +179,52 @@ class OauthApiController extends Controller {
// Rotate the apptoken (so the old one becomes invalid basically)
$newToken = $this->secureRandom->generate(72, ISecureRandom::CHAR_ALPHANUMERIC);
$appToken = $this->tokenProvider->rotate(
$appToken,
$decryptedToken,
$newToken
);
// Expiration is in 1 hour again
$appToken->setExpires($this->time->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);
$newEncryptedToken = $this->crypto->encrypt($newToken, $newCode);
$redeemedThrottleReason = $grant_type === 'authorization_code'
? 'authorization_code_already_redeemed'
: 'refresh_token_already_redeemed';
$this->db->beginTransaction();
try {
$updatedRows = $this->accessTokenMapper->rotateToken(
$accessToken->getId(),
$code,
$newCode,
$newEncryptedToken,
$grant_type === 'authorization_code',
);
if ($updatedRows !== 1) {
$this->db->rollBack();
$response = new JSONResponse([
'error' => 'invalid_request',
], Http::STATUS_BAD_REQUEST);
$response->throttle(['invalid_request' => $redeemedThrottleReason]);
return $response;
}
$appToken = $this->tokenProvider->rotate(
$appToken,
$decryptedToken,
$newToken
);
// Expiration is in 1 hour again
$appToken->setExpires($this->time->getTime() + 3600);
$this->tokenProvider->updateToken($appToken);
$this->db->commit();
} catch (\Throwable $e) {
if ($this->db->inTransaction()) {
$this->db->rollBack();
}
// rotate() and updateToken() write the auth token to the cache,
// so if we are past rotate() we must invalidate the new token
$this->tokenProvider->invalidateToken($newToken);
throw $e;
}
$this->throttler->resetDelay($this->request->getRemoteAddress(), 'login', ['user' => $appToken->getUID()]);

27
apps/oauth2/lib/Db/AccessTokenMapper.php
View file

@ -82,4 +82,31 @@ class AccessTokenMapper extends QBMapper {
->andWhere($qb->expr()->lt('code_created_at', $qb->createNamedParameter($maxTokenCreationTs, IQueryBuilder::PARAM_INT)));
$qb->executeStatement();
}
/**
* Rotate an access token only if it still matches the caller's previously-read state.
*
* @param int $id
* @param string $oldCode
* @param string $newCode
* @param string $encryptedToken
* @param bool $expectAuthorizationCodeState Require the token to still be unused
* @return int Number of updated rows
*/
public function rotateToken(int $id, string $oldCode, string $newCode, string $encryptedToken, bool $expectAuthorizationCodeState): int {
$qb = $this->db->getQueryBuilder();
$qb
->update($this->tableName)
->set('hashed_code', $qb->createNamedParameter(hash('sha512', $newCode)))
->set('encrypted_token', $qb->createNamedParameter($encryptedToken))
->set('token_count', $qb->createFunction('token_count + 1'))
->where($qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)))
->andWhere($qb->expr()->eq('hashed_code', $qb->createNamedParameter(hash('sha512', $oldCode))));
if ($expectAuthorizationCodeState) {
$qb->andWhere($qb->expr()->eq('token_count', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)));
}
return $qb->executeStatement();
}
}

170
apps/oauth2/tests/Controller/OauthApiControllerTest.php
View file

@ -20,6 +20,7 @@ use OCA\OAuth2\Exceptions\ClientNotFoundException;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IDBConnection;
use OCP\IRequest;
use OCP\Security\Bruteforce\IThrottler;
use OCP\Security\ICrypto;
@ -53,6 +54,8 @@ class OauthApiControllerTest extends TestCase {
private $logger;
/** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
private $timeFactory;
/** @var IDBConnection|\PHPUnit\Framework\MockObject\MockObject */
private $db;
/** @var OauthApiController */
private $oauthApiController;
@ -69,6 +72,7 @@ class OauthApiControllerTest extends TestCase {
$this->throttler = $this->createMock(IThrottler::class);
$this->logger = $this->createMock(LoggerInterface::class);
$this->timeFactory = $this->createMock(ITimeFactory::class);
$this->db = $this->createMock(IDBConnection::class);
$this->oauthApiController = new OauthApiController(
'oauth2',
@ -81,7 +85,8 @@ class OauthApiControllerTest extends TestCase {
$this->time,
$this->logger,
$this->throttler,
$this->timeFactory
$this->timeFactory,
$this->db,
);
}
@ -316,6 +321,7 @@ class OauthApiControllerTest extends TestCase {
public function testRefreshTokenValidAppToken(): void {
$accessToken = new AccessToken();
$accessToken->setId(21);
$accessToken->setClientId(42);
$accessToken->setTokenId(1337);
$accessToken->setEncryptedToken('encryptedToken');
@ -367,6 +373,18 @@ class OauthApiControllerTest extends TestCase {
$this->time->method('getTime')
->willReturn(1000);
$this->db->expects($this->once())
->method('beginTransaction');
$this->db->expects($this->once())
->method('commit');
$this->db->expects($this->never())
->method('rollBack');
$this->tokenProvider->expects($this->never())
->method('invalidateToken');
$this->tokenProvider->expects($this->once())
->method('updateToken')
->with(
@ -380,13 +398,14 @@ class OauthApiControllerTest extends TestCase {
->willReturn('newEncryptedToken');
$this->accessTokenMapper->expects($this->once())
->method('update')
->method('rotateToken')
->with(
$this->callback(function (AccessToken $token) {
return $token->getHashedCode() === hash('sha512', 'random128')
&& $token->getEncryptedToken() === 'newEncryptedToken';
})
);
21,
'validrefresh',
'random128',
'newEncryptedToken',
false,
)->willReturn(1);
$expected = new JSONResponse([
'access_token' => 'random72',
@ -412,6 +431,7 @@ class OauthApiControllerTest extends TestCase {
public function testRefreshTokenValidAppTokenBasicAuth(): void {
$accessToken = new AccessToken();
$accessToken->setId(21);
$accessToken->setClientId(42);
$accessToken->setTokenId(1337);
$accessToken->setEncryptedToken('encryptedToken');
@ -463,6 +483,18 @@ class OauthApiControllerTest extends TestCase {
$this->time->method('getTime')
->willReturn(1000);
$this->db->expects($this->once())
->method('beginTransaction');
$this->db->expects($this->once())
->method('commit');
$this->db->expects($this->never())
->method('rollBack');
$this->tokenProvider->expects($this->never())
->method('invalidateToken');
$this->tokenProvider->expects($this->once())
->method('updateToken')
->with(
@ -476,13 +508,14 @@ class OauthApiControllerTest extends TestCase {
->willReturn('newEncryptedToken');
$this->accessTokenMapper->expects($this->once())
->method('update')
->method('rotateToken')
->with(
$this->callback(function (AccessToken $token) {
return $token->getHashedCode() === hash('sha512', 'random128')
&& $token->getEncryptedToken() === 'newEncryptedToken';
})
);
21,
'validrefresh',
'random128',
'newEncryptedToken',
false,
)->willReturn(1);
$expected = new JSONResponse([
'access_token' => 'random72',
@ -511,6 +544,7 @@ class OauthApiControllerTest extends TestCase {
public function testRefreshTokenExpiredAppToken(): void {
$accessToken = new AccessToken();
$accessToken->setId(21);
$accessToken->setClientId(42);
$accessToken->setTokenId(1337);
$accessToken->setEncryptedToken('encryptedToken');
@ -562,6 +596,18 @@ class OauthApiControllerTest extends TestCase {
$this->time->method('getTime')
->willReturn(1000);
$this->db->expects($this->once())
->method('beginTransaction');
$this->db->expects($this->once())
->method('commit');
$this->db->expects($this->never())
->method('rollBack');
$this->tokenProvider->expects($this->never())
->method('invalidateToken');
$this->tokenProvider->expects($this->once())
->method('updateToken')
->with(
@ -575,13 +621,14 @@ class OauthApiControllerTest extends TestCase {
->willReturn('newEncryptedToken');
$this->accessTokenMapper->expects($this->once())
->method('update')
->method('rotateToken')
->with(
$this->callback(function (AccessToken $token) {
return $token->getHashedCode() === hash('sha512', 'random128')
&& $token->getEncryptedToken() === 'newEncryptedToken';
})
);
21,
'validrefresh',
'random128',
'newEncryptedToken',
false,
)->willReturn(1);
$expected = new JSONResponse([
'access_token' => 'random72',
@ -604,4 +651,89 @@ class OauthApiControllerTest extends TestCase {
$this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret'));
}
public function testRefreshTokenRedeemedConcurrently(): void {
$expected = new JSONResponse([
'error' => 'invalid_request',
], Http::STATUS_BAD_REQUEST);
$expected->throttle(['invalid_request' => 'refresh_token_already_redeemed']);
$accessToken = new AccessToken();
$accessToken->setId(21);
$accessToken->setClientId(42);
$accessToken->setTokenId(1337);
$accessToken->setEncryptedToken('encryptedToken');
$this->accessTokenMapper->method('getByCode')
->with('validrefresh')
->willReturn($accessToken);
$client = new Client();
$client->setClientIdentifier('clientId');
$client->setSecret(bin2hex('hashedClientSecret'));
$this->clientMapper->method('getByUid')
->with(42)
->willReturn($client);
$this->crypto
->method('decrypt')
->with('encryptedToken')
->willReturn('decryptedToken');
$this->crypto
->method('calculateHMAC')
->with('clientSecret')
->willReturn('hashedClientSecret');
$appToken = new PublicKeyToken();
$appToken->setUid('userId');
$this->tokenProvider->method('getTokenById')
->with(1337)
->willReturn($appToken);
$this->secureRandom->method('generate')
->willReturnCallback(function ($len) {
return 'random' . $len;
});
$this->tokenProvider->expects($this->never())
->method('rotate');
$this->time->method('getTime')
->willReturn(1000);
$this->tokenProvider->expects($this->never())
->method('updateToken');
$this->crypto->method('encrypt')
->with('random72', 'random128')
->willReturn('newEncryptedToken');
$this->db->expects($this->once())
->method('beginTransaction');
$this->db->expects($this->never())
->method('commit');
$this->db->expects($this->once())
->method('rollBack');
$this->tokenProvider->expects($this->never())
->method('invalidateToken');
$this->accessTokenMapper->expects($this->once())
->method('rotateToken')
->with(
21,
'validrefresh',
'random128',
'newEncryptedToken',
false,
)->willReturn(0);
$this->throttler->expects($this->never())
->method('resetDelay');
$this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret'));
}
}