Skip to content

Commit

Permalink
Fix for a possible race condition. thephpleague#1306
Browse files Browse the repository at this point in the history
  • Loading branch information
marcriemer committed May 21, 2024
1 parent 2ed9e5f commit ceb14dc
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 15 deletions.
44 changes: 29 additions & 15 deletions src/Grant/AuthCodeGrant.php
Original file line number Diff line number Diff line change
Expand Up @@ -141,23 +141,33 @@ public function respondToAccessTokenRequest(
$this->validateCodeChallenge($authCodePayload, $codeVerifier);
}

// Issue and persist new access token
$accessToken = $this->issueAccessToken($accessTokenTTL, $client, $authCodePayload->user_id, $scopes);
$this->getEmitter()->emit(new RequestAccessTokenEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request, $accessToken));
$responseType->setAccessToken($accessToken);

// Issue and persist new refresh token if given
$refreshToken = $this->issueRefreshToken($accessToken);
if ($this->authCodeRepository->lockAuthCode($authCodePayload->auth_code_id)) {

Check failure on line 144 in src/Grant/AuthCodeGrant.php

View workflow job for this annotation

GitHub Actions / Static Analysis (8.1, prefer-lowest, ubuntu-latest)

Method League\OAuth2\Server\Grant\AuthCodeGrant::respondToAccessTokenRequest() should return League\OAuth2\Server\ResponseTypes\ResponseTypeInterface but return statement is missing.

Check failure on line 144 in src/Grant/AuthCodeGrant.php

View workflow job for this annotation

GitHub Actions / Static Analysis (8.1, prefer-stable, ubuntu-latest)

Method League\OAuth2\Server\Grant\AuthCodeGrant::respondToAccessTokenRequest() should return League\OAuth2\Server\ResponseTypes\ResponseTypeInterface but return statement is missing.

Check failure on line 144 in src/Grant/AuthCodeGrant.php

View workflow job for this annotation

GitHub Actions / Static Analysis (8.2, prefer-lowest, ubuntu-latest)

Method League\OAuth2\Server\Grant\AuthCodeGrant::respondToAccessTokenRequest() should return League\OAuth2\Server\ResponseTypes\ResponseTypeInterface but return statement is missing.

Check failure on line 144 in src/Grant/AuthCodeGrant.php

View workflow job for this annotation

GitHub Actions / Static Analysis (8.2, prefer-stable, ubuntu-latest)

Method League\OAuth2\Server\Grant\AuthCodeGrant::respondToAccessTokenRequest() should return League\OAuth2\Server\ResponseTypes\ResponseTypeInterface but return statement is missing.

Check failure on line 144 in src/Grant/AuthCodeGrant.php

View workflow job for this annotation

GitHub Actions / Static Analysis (8.3, prefer-stable, ubuntu-latest)

Method League\OAuth2\Server\Grant\AuthCodeGrant::respondToAccessTokenRequest() should return League\OAuth2\Server\ResponseTypes\ResponseTypeInterface but return statement is missing.
try {
// Issue and persist new access token
$accessToken = $this->issueAccessToken($accessTokenTTL, $client, $authCodePayload->user_id, $scopes);
$this->getEmitter()->emit(new RequestAccessTokenEvent(RequestEvent::ACCESS_TOKEN_ISSUED, $request, $accessToken));
$responseType->setAccessToken($accessToken);

// Issue and persist new refresh token if given
$refreshToken = $this->issueRefreshToken($accessToken);

if ($refreshToken !== null) {
$this->getEmitter()->emit(new RequestRefreshTokenEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request, $refreshToken));
$responseType->setRefreshToken($refreshToken);
}

if ($refreshToken !== null) {
$this->getEmitter()->emit(new RequestRefreshTokenEvent(RequestEvent::REFRESH_TOKEN_ISSUED, $request, $refreshToken));
$responseType->setRefreshToken($refreshToken);
return $responseType;
} catch (Exception $e) {
$this->authCodeRepository->unlockAuthCode($authCodePayload->auth_code_id);
throw OAuthServerException::serverError(
'access_token',
$e
);
} finally {
// Revoke used auth code
$this->authCodeRepository->revokeAuthCode($authCodePayload->auth_code_id);
}
}

// Revoke used auth code
$this->authCodeRepository->revokeAuthCode($authCodePayload->auth_code_id);

return $responseType;
}

private function validateCodeChallenge(object $authCodePayload, ?string $codeVerifier): void
Expand Down Expand Up @@ -213,6 +223,10 @@ private function validateAuthorizationCode(
throw OAuthServerException::invalidGrant('Authorization code has been revoked');
}

if ($this->authCodeRepository->isAuthCodeLocked($authCodePayload->auth_code_id) === true) {
throw OAuthServerException::invalidGrant('Authorization code has been locked while an access code beeing issued');
}

if ($authCodePayload->client_id !== $client->getIdentifier()) {
throw OAuthServerException::invalidRequest('code', 'Authorization code was not issued to this client');
}
Expand Down
22 changes: 22 additions & 0 deletions src/Repositories/AuthCodeRepositoryInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,26 @@ public function persistNewAuthCode(AuthCodeEntityInterface $authCodeEntity): voi
public function revokeAuthCode(string $codeId): void;

public function isAuthCodeRevoked(string $codeId): bool;

/**
* Method locks an auth code in case an error occurs during the issuance of an access code.
*
* The storage engine should make this persistent immediately to prevent possible race conditions while issuing an access token.
*
* @param string $codeId
*
* @return void
*/
public function lockAuthCode(string $codeId): bool;

Check failure on line 43 in src/Repositories/AuthCodeRepositoryInterface.php

View workflow job for this annotation

GitHub Actions / Static Analysis (8.1, prefer-lowest, ubuntu-latest)

PHPDoc tag @return with type void is incompatible with native type bool.

Check failure on line 43 in src/Repositories/AuthCodeRepositoryInterface.php

View workflow job for this annotation

GitHub Actions / Static Analysis (8.1, prefer-stable, ubuntu-latest)

PHPDoc tag @return with type void is incompatible with native type bool.

Check failure on line 43 in src/Repositories/AuthCodeRepositoryInterface.php

View workflow job for this annotation

GitHub Actions / Static Analysis (8.2, prefer-lowest, ubuntu-latest)

PHPDoc tag @return with type void is incompatible with native type bool.

Check failure on line 43 in src/Repositories/AuthCodeRepositoryInterface.php

View workflow job for this annotation

GitHub Actions / Static Analysis (8.2, prefer-stable, ubuntu-latest)

PHPDoc tag @return with type void is incompatible with native type bool.

Check failure on line 43 in src/Repositories/AuthCodeRepositoryInterface.php

View workflow job for this annotation

GitHub Actions / Static Analysis (8.3, prefer-stable, ubuntu-latest)

PHPDoc tag @return with type void is incompatible with native type bool.

/**
* This method is used to make the auth code available again after an error occurred in the access code issuance process.
*
* @param string $codeId
*
* @return void
*/
public function unlockAuthCode(string $codeId): void;

public function isAuthCodeLocked(string $codeId): bool;
}
67 changes: 67 additions & 0 deletions tests/Grant/AuthCodeGrantTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,73 @@ public function testRespondToAccessTokenRequestRevokedCode(): void
}
}

public function testRespondToAccessTokenRequestLockedCode(): void
{
$client = new ClientEntity();

$client->setIdentifier('foo');
$client->setRedirectUri(self::REDIRECT_URI);
$client->setConfidential();

$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();

$clientRepositoryMock->method('getClientEntity')->willReturn($client);
$clientRepositoryMock->method('validateClient')->willReturn(true);

$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();

$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
$refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf();

$authCodeRepositoryMock = $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock();
$authCodeRepositoryMock->method('isAuthCodeLocked')->willReturn(true);

$grant = new AuthCodeGrant(
$authCodeRepositoryMock,
$this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
new DateInterval('PT10M')
);
$grant->setClientRepository($clientRepositoryMock);
$grant->setAccessTokenRepository($accessTokenRepositoryMock);
$grant->setRefreshTokenRepository($refreshTokenRepositoryMock);
$grant->setEncryptionKey($this->cryptStub->getKey());

$request = new ServerRequest(
[],
[],
null,
'POST',
'php://input',
[],
[],
[],
[
'grant_type' => 'authorization_code',
'client_id' => 'foo',
'redirect_uri' => self::REDIRECT_URI,
'code' => $this->cryptStub->doEncrypt(
json_encode([
'auth_code_id' => uniqid(),
'expire_time' => time() + 3600,
'client_id' => 'foo',
'user_id' => 123,
'scopes' => ['foo'],
'redirect_uri' => 'http://foo/bar',
], JSON_THROW_ON_ERROR)
),
]
);

try {
/* @var StubResponseType $response */
$grant->respondToAccessTokenRequest($request, new StubResponseType(), new DateInterval('PT10M'));
} catch (OAuthServerException $e) {
self::assertEquals($e->getHint(), 'Authorization code has been revoked');
self::assertEquals($e->getErrorType(), 'invalid_grant');
}
}

public function testRespondToAccessTokenRequestClientMismatch(): void
{
$client = new ClientEntity();
Expand Down

0 comments on commit ceb14dc

Please sign in to comment.