diff --git a/apps/oauth2/lib/Controller/OauthApiController.php b/apps/oauth2/lib/Controller/OauthApiController.php index 835f2fdfd8c..b194674dfac 100644 --- a/apps/oauth2/lib/Controller/OauthApiController.php +++ b/apps/oauth2/lib/Controller/OauthApiController.php @@ -184,21 +184,9 @@ class OauthApiController extends Controller { $redeemedThrottleReason = $grant_type === 'authorization_code' ? 'authorization_code_already_redeemed' : 'refresh_token_already_redeemed'; - $tokenRotated = false; $this->db->beginTransaction(); try { - $appToken = $this->tokenProvider->rotate( - $appToken, - $decryptedToken, - $newToken - ); - $tokenRotated = true; - - // Expiration is in 1 hour again - $appToken->setExpires($this->time->getTime() + 3600); - $this->tokenProvider->updateToken($appToken); - $updatedRows = $this->accessTokenMapper->rotateToken( $accessToken->getId(), $code, @@ -209,8 +197,6 @@ class OauthApiController extends Controller { if ($updatedRows !== 1) { $this->db->rollBack(); - // tokenProvider->rotate() updates the auth token cache, so we have to clear the new token on rollback - $this->tokenProvider->invalidateToken($newToken); $response = new JSONResponse([ 'error' => 'invalid_request', ], Http::STATUS_BAD_REQUEST); @@ -218,16 +204,24 @@ class OauthApiController extends Controller { 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(); } - - if ($tokenRotated) { - // tokenProvider->rotate() updates the auth token cache, so we have to clear the new token on rollback - $this->tokenProvider->invalidateToken($newToken); - } + // 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; } diff --git a/apps/oauth2/tests/Controller/OauthApiControllerTest.php b/apps/oauth2/tests/Controller/OauthApiControllerTest.php index 82867a6d0a5..b528908de2c 100644 --- a/apps/oauth2/tests/Controller/OauthApiControllerTest.php +++ b/apps/oauth2/tests/Controller/OauthApiControllerTest.php @@ -696,20 +696,14 @@ class OauthApiControllerTest extends TestCase { return 'random' . $len; }); - $this->tokenProvider->expects($this->once()) - ->method('rotate') - ->with( - $appToken, - 'decryptedToken', - 'random72' - )->willReturn($appToken); + $this->tokenProvider->expects($this->never()) + ->method('rotate'); $this->time->method('getTime') ->willReturn(1000); - $this->tokenProvider->expects($this->once()) - ->method('updateToken') - ->with($this->isInstanceOf(PublicKeyToken::class)); + $this->tokenProvider->expects($this->never()) + ->method('updateToken'); $this->crypto->method('encrypt') ->with('random72', 'random128') @@ -721,16 +715,11 @@ class OauthApiControllerTest extends TestCase { $this->db->expects($this->never()) ->method('commit'); - $this->db->expects($this->exactly(2)) - ->method('inTransaction') - ->willReturnOnConsecutiveCalls(true, false); - $this->db->expects($this->once()) ->method('rollBack'); - $this->tokenProvider->expects($this->once()) - ->method('invalidateToken') - ->with('random72'); + $this->tokenProvider->expects($this->never()) + ->method('invalidateToken'); $this->accessTokenMapper->expects($this->once()) ->method('rotateToken')