From 3bb50c1fc7a7e73db576bec299c46e3215dda602 Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Sun, 30 Jun 2024 04:27:45 +0330
Subject: [PATCH 01/12] always validate the client
---
examples/src/Repositories/ClientRepository.php | 5 ++++-
src/Grant/AuthCodeGrant.php | 9 +--------
src/Grant/ClientCredentialsGrant.php | 7 +------
3 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/examples/src/Repositories/ClientRepository.php b/examples/src/Repositories/ClientRepository.php
index 0b19d57d7..047fe77f7 100644
--- a/examples/src/Repositories/ClientRepository.php
+++ b/examples/src/Repositories/ClientRepository.php
@@ -59,7 +59,10 @@ public function validateClient($clientIdentifier, $clientSecret, $grantType): bo
return false;
}
- if (password_verify($clientSecret, $clients[$clientIdentifier]['secret']) === false) {
+ if (
+ $clients[$clientIdentifier]['is_confidential'] === true
+ && password_verify($clientSecret, $clients[$clientIdentifier]['secret']) === false
+ ) {
return false;
}
diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php
index 8a24a8e95..1a4a1587e 100644
--- a/src/Grant/AuthCodeGrant.php
+++ b/src/Grant/AuthCodeGrant.php
@@ -96,14 +96,7 @@ public function respondToAccessTokenRequest(
ResponseTypeInterface $responseType,
DateInterval $accessTokenTTL
): ResponseTypeInterface {
- list($clientId) = $this->getClientCredentials($request);
-
- $client = $this->getClientEntityOrFail($clientId, $request);
-
- // Only validate the client if it is confidential
- if ($client->isConfidential()) {
- $this->validateClient($request);
- }
+ $client = $this->validateClient($request);
$encryptedAuthCode = $this->getRequestParameter('code', $request);
diff --git a/src/Grant/ClientCredentialsGrant.php b/src/Grant/ClientCredentialsGrant.php
index bee6abaa1..a24266c4c 100644
--- a/src/Grant/ClientCredentialsGrant.php
+++ b/src/Grant/ClientCredentialsGrant.php
@@ -34,9 +34,7 @@ public function respondToAccessTokenRequest(
ResponseTypeInterface $responseType,
DateInterval $accessTokenTTL
): ResponseTypeInterface {
- list($clientId) = $this->getClientCredentials($request);
-
- $client = $this->getClientEntityOrFail($clientId, $request);
+ $client = $this->validateClient($request);
if (!$client->isConfidential()) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
@@ -44,9 +42,6 @@ public function respondToAccessTokenRequest(
throw OAuthServerException::invalidClient($request);
}
- // Validate request
- $this->validateClient($request);
-
$scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope));
// Finalize the requested scopes
From 07871d831a58d0fb1910c949bfb20572cd6bc4f7 Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Sun, 30 Jun 2024 04:28:08 +0330
Subject: [PATCH 02/12] pass grant type to getClientEntity
---
examples/src/Repositories/ClientRepository.php | 2 +-
src/Grant/AbstractGrant.php | 2 +-
src/Repositories/ClientRepositoryInterface.php | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/examples/src/Repositories/ClientRepository.php b/examples/src/Repositories/ClientRepository.php
index 047fe77f7..8dcb0af7b 100644
--- a/examples/src/Repositories/ClientRepository.php
+++ b/examples/src/Repositories/ClientRepository.php
@@ -28,7 +28,7 @@ class ClientRepository implements ClientRepositoryInterface
/**
* {@inheritdoc}
*/
- public function getClientEntity(string $clientIdentifier): ?ClientEntityInterface
+ public function getClientEntity(string $clientIdentifier, ?string $grantType): ?ClientEntityInterface
{
$client = new ClientEntity();
diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php
index ea0064c3b..1d36d01d9 100644
--- a/src/Grant/AbstractGrant.php
+++ b/src/Grant/AbstractGrant.php
@@ -182,7 +182,7 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity
*/
protected function getClientEntityOrFail(string $clientId, ServerRequestInterface $request): ClientEntityInterface
{
- $client = $this->clientRepository->getClientEntity($clientId);
+ $client = $this->clientRepository->getClientEntity($clientId, $this->getIdentifier());
if ($client instanceof ClientEntityInterface === false) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
diff --git a/src/Repositories/ClientRepositoryInterface.php b/src/Repositories/ClientRepositoryInterface.php
index 63134ca9d..2d8e27336 100644
--- a/src/Repositories/ClientRepositoryInterface.php
+++ b/src/Repositories/ClientRepositoryInterface.php
@@ -22,7 +22,7 @@ interface ClientRepositoryInterface extends RepositoryInterface
/**
* Get a client.
*/
- public function getClientEntity(string $clientIdentifier): ?ClientEntityInterface;
+ public function getClientEntity(string $clientIdentifier, ?string $grantType): ?ClientEntityInterface;
/**
* Validate a client's secret.
From de0507459bc7571dd8a50f7cfa87b2889fc45455 Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Thu, 29 Aug 2024 17:21:31 +0330
Subject: [PATCH 03/12] fix tests
---
tests/Grant/AuthCodeGrantTest.php | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php
index 6a6842661..5dd8e429a 100644
--- a/tests/Grant/AuthCodeGrantTest.php
+++ b/tests/Grant/AuthCodeGrantTest.php
@@ -623,6 +623,7 @@ public function testRespondToAccessTokenRequestUsingHttpBasicAuth(): void
$client->setIdentifier('foo');
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
+ $clientRepositoryMock->method('validateClient')->willReturn(true);
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn(new ScopeEntity());
@@ -686,6 +687,7 @@ public function testRespondToAccessTokenRequestForPublicClient(): void
$client->setRedirectUri(self::REDIRECT_URI);
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
+ $clientRepositoryMock->method('validateClient')->willReturn(true);
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeEntity = new ScopeEntity();
@@ -751,6 +753,7 @@ public function testRespondToAccessTokenRequestNullRefreshToken(): void
$client->setRedirectUri(self::REDIRECT_URI);
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
+ $clientRepositoryMock->method('validateClient')->willReturn(true);
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeEntity = new ScopeEntity();
@@ -1187,6 +1190,7 @@ public function testRespondToAccessTokenRequestWithRefreshTokenInsteadOfAuthCode
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
+ $clientRepositoryMock->method('validateClient')->willReturn(true);
$grant = new AuthCodeGrant(
$this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
@@ -1276,6 +1280,7 @@ public function testRespondToAccessTokenRequestExpiredCode(): void
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
+ $clientRepositoryMock->method('validateClient')->willReturn(true);
$grant = new AuthCodeGrant(
$this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
@@ -1980,6 +1985,7 @@ public function testRefreshTokenRepositoryUniqueConstraintCheck(): void
$client->setRedirectUri(self::REDIRECT_URI);
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
+ $clientRepositoryMock->method('validateClient')->willReturn(true);
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeEntity = new ScopeEntity();
@@ -2055,6 +2061,7 @@ public function testRefreshTokenRepositoryFailToPersist(): void
$client->setRedirectUri(self::REDIRECT_URI);
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
+ $clientRepositoryMock->method('validateClient')->willReturn(true);
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeEntity = new ScopeEntity();
@@ -2123,6 +2130,7 @@ public function testRefreshTokenRepositoryFailToPersistUniqueNoInfiniteLoop(): v
$client->setRedirectUri(self::REDIRECT_URI);
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
+ $clientRepositoryMock->method('validateClient')->willReturn(true);
$scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
$scopeEntity = new ScopeEntity();
From f7b9acbdcd693565108c269fcc34c69a280c7638 Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Tue, 1 Oct 2024 17:08:55 +0330
Subject: [PATCH 04/12] add ClientEntityInterface::hasGrantType()
---
.../src/Repositories/ClientRepository.php | 2 +-
src/Entities/ClientEntityInterface.php | 5 +++
src/Entities/Traits/ClientTrait.php | 8 +++++
src/Grant/AbstractGrant.php | 8 +++--
.../ClientRepositoryInterface.php | 2 +-
tests/Grant/AbstractGrantTest.php | 1 +
tests/Grant/AuthCodeGrantTest.php | 32 ++++++++++++++-----
tests/Grant/DeviceCodeGrantTest.php | 4 ++-
tests/Grant/PasswordGrantTest.php | 4 ++-
tests/Grant/RefreshTokenGrantTest.php | 12 +++++--
10 files changed, 61 insertions(+), 17 deletions(-)
diff --git a/examples/src/Repositories/ClientRepository.php b/examples/src/Repositories/ClientRepository.php
index 8dcb0af7b..047fe77f7 100644
--- a/examples/src/Repositories/ClientRepository.php
+++ b/examples/src/Repositories/ClientRepository.php
@@ -28,7 +28,7 @@ class ClientRepository implements ClientRepositoryInterface
/**
* {@inheritdoc}
*/
- public function getClientEntity(string $clientIdentifier, ?string $grantType): ?ClientEntityInterface
+ public function getClientEntity(string $clientIdentifier): ?ClientEntityInterface
{
$client = new ClientEntity();
diff --git a/src/Entities/ClientEntityInterface.php b/src/Entities/ClientEntityInterface.php
index f3838b11c..138e831f7 100644
--- a/src/Entities/ClientEntityInterface.php
+++ b/src/Entities/ClientEntityInterface.php
@@ -38,4 +38,9 @@ public function getRedirectUri(): string|array;
* Returns true if the client is confidential.
*/
public function isConfidential(): bool;
+
+ /**
+ * Returns true if the client handles the given grant type.
+ */
+ public function hasGrantType(string $grantType): bool;
}
diff --git a/src/Entities/Traits/ClientTrait.php b/src/Entities/Traits/ClientTrait.php
index b179cfac4..0b4327c35 100644
--- a/src/Entities/Traits/ClientTrait.php
+++ b/src/Entities/Traits/ClientTrait.php
@@ -52,4 +52,12 @@ public function isConfidential(): bool
{
return $this->isConfidential;
}
+
+ /**
+ * Returns true if the client handles the given grant type.
+ */
+ public function hasGrantType(string $grantType): bool
+ {
+ return true;
+ }
}
diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php
index 1d36d01d9..54aaed965 100644
--- a/src/Grant/AbstractGrant.php
+++ b/src/Grant/AbstractGrant.php
@@ -182,13 +182,17 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity
*/
protected function getClientEntityOrFail(string $clientId, ServerRequestInterface $request): ClientEntityInterface
{
- $client = $this->clientRepository->getClientEntity($clientId, $this->getIdentifier());
+ $client = $this->clientRepository->getClientEntity($clientId);
if ($client instanceof ClientEntityInterface === false) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
throw OAuthServerException::invalidClient($request);
}
+ if (!$client->hasGrantType($this->getIdentifier())) {
+ throw OAuthServerException::invalidGrant();
+ }
+
return $client;
}
@@ -486,7 +490,7 @@ protected function issueRefreshToken(AccessTokenEntityInterface $accessToken): ?
{
$refreshToken = $this->refreshTokenRepository->getNewRefreshToken();
- if ($refreshToken === null) {
+ if ($refreshToken === null || !$accessToken->getClient()->hasGrantType('refresh_token')) {
return null;
}
diff --git a/src/Repositories/ClientRepositoryInterface.php b/src/Repositories/ClientRepositoryInterface.php
index 2d8e27336..63134ca9d 100644
--- a/src/Repositories/ClientRepositoryInterface.php
+++ b/src/Repositories/ClientRepositoryInterface.php
@@ -22,7 +22,7 @@ interface ClientRepositoryInterface extends RepositoryInterface
/**
* Get a client.
*/
- public function getClientEntity(string $clientIdentifier, ?string $grantType): ?ClientEntityInterface;
+ public function getClientEntity(string $clientIdentifier): ?ClientEntityInterface;
/**
* Validate a client's secret.
diff --git a/tests/Grant/AbstractGrantTest.php b/tests/Grant/AbstractGrantTest.php
index adfb880be..7e2193e87 100644
--- a/tests/Grant/AbstractGrantTest.php
+++ b/tests/Grant/AbstractGrantTest.php
@@ -398,6 +398,7 @@ public function testIssueRefreshToken(): void
$issueRefreshTokenMethod->setAccessible(true);
$accessToken = new AccessTokenEntity();
+ $accessToken->setClient(new ClientEntity());
/** @var RefreshTokenEntityInterface $refreshToken */
$refreshToken = $issueRefreshTokenMethod->invoke($grantMock, $accessToken);
diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php
index 5dd8e429a..cab22a927 100644
--- a/tests/Grant/AuthCodeGrantTest.php
+++ b/tests/Grant/AuthCodeGrantTest.php
@@ -565,7 +565,9 @@ public function testRespondToAccessTokenRequest(): void
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
@@ -630,7 +632,9 @@ public function testRespondToAccessTokenRequestUsingHttpBasicAuth(): void
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
$refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity());
@@ -695,7 +699,9 @@ public function testRespondToAccessTokenRequestForPublicClient(): void
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
@@ -831,7 +837,9 @@ public function testRespondToAccessTokenRequestCodeChallengePlain(): void
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
@@ -905,7 +913,9 @@ public function testRespondToAccessTokenRequestCodeChallengeS256(): void
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
@@ -1993,7 +2003,9 @@ public function testRefreshTokenRepositoryUniqueConstraintCheck(): void
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
@@ -2069,7 +2081,9 @@ public function testRefreshTokenRepositoryFailToPersist(): void
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
@@ -2138,7 +2152,9 @@ public function testRefreshTokenRepositoryFailToPersistUniqueNoInfiniteLoop(): v
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php
index 396ea760f..40249bb39 100644
--- a/tests/Grant/DeviceCodeGrantTest.php
+++ b/tests/Grant/DeviceCodeGrantTest.php
@@ -347,7 +347,9 @@ public function testRespondToAccessTokenRequest(): void
$clientRepositoryMock->method('validateClient')->willReturn(true);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
diff --git a/tests/Grant/PasswordGrantTest.php b/tests/Grant/PasswordGrantTest.php
index 8c60a8c78..011a9605e 100644
--- a/tests/Grant/PasswordGrantTest.php
+++ b/tests/Grant/PasswordGrantTest.php
@@ -46,7 +46,9 @@ public function testRespondToRequest(): void
$clientRepositoryMock->method('validateClient')->willReturn(true);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$userRepositoryMock = $this->getMockBuilder(UserRepositoryInterface::class)->getMock();
diff --git a/tests/Grant/RefreshTokenGrantTest.php b/tests/Grant/RefreshTokenGrantTest.php
index b37001a80..4a4adfb25 100644
--- a/tests/Grant/RefreshTokenGrantTest.php
+++ b/tests/Grant/RefreshTokenGrantTest.php
@@ -59,7 +59,9 @@ public function testRespondToRequest(): void
$scopeRepositoryMock->method('finalizeScopes')->willReturn([$scopeEntity]);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->expects(self::once())->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
@@ -181,7 +183,9 @@ public function testRespondToReducedScopes(): void
$clientRepositoryMock->method('validateClient')->willReturn(true);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
@@ -567,10 +571,12 @@ public function testRespondToRequestFinalizeScopes(): void
->with($scopes, $grant->getIdentifier(), $client)
->willReturn($finalizedScopes);
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
$accessTokenRepositoryMock
->method('getNewToken')
->with($client, $finalizedScopes)
- ->willReturn(new AccessTokenEntity());
+ ->willReturn($accessToken);
$oldRefreshToken = json_encode(
[
From 3f36fe5ebfad5a976505e1e078c5523b5285621e Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Sat, 5 Oct 2024 00:39:00 +0330
Subject: [PATCH 05/12] use unauthorized_client error
---
src/Exception/OAuthServerException.php | 11 ++++++++++-
src/Grant/AbstractGrant.php | 2 +-
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/Exception/OAuthServerException.php b/src/Exception/OAuthServerException.php
index 9eff92456..ff1b2fb47 100644
--- a/src/Exception/OAuthServerException.php
+++ b/src/Exception/OAuthServerException.php
@@ -252,8 +252,17 @@ public static function slowDown(string $hint = '', Throwable $previous = null):
}
/**
+ * Unauthorized client error.
+ */
+ public static function unauthorizedClient(?string $hint = null): static
{
- return $this->errorType;
+ return new static(
+ 'The authenticated client is not authorized to use this authorization grant type.',
+ 14,
+ 'unauthorized_client',
+ 400,
+ $hint
+ );
}
/**
diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php
index 54aaed965..af13c3e34 100644
--- a/src/Grant/AbstractGrant.php
+++ b/src/Grant/AbstractGrant.php
@@ -190,7 +190,7 @@ protected function getClientEntityOrFail(string $clientId, ServerRequestInterfac
}
if (!$client->hasGrantType($this->getIdentifier())) {
- throw OAuthServerException::invalidGrant();
+ throw OAuthServerException::unauthorizedClient();
}
return $client;
From 9d88ce9bd514308f6953f6098f97ff43ce2de983 Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Fri, 18 Oct 2024 13:50:30 +0330
Subject: [PATCH 06/12] validate confidential clients
---
examples/src/Repositories/ClientRepository.php | 5 +----
src/Grant/AbstractGrant.php | 11 ++++++++---
tests/Grant/AbstractGrantTest.php | 1 +
tests/Grant/AuthCodeGrantTest.php | 10 ++++++++--
tests/Grant/PasswordGrantTest.php | 10 +++++++++-
tests/Grant/RefreshTokenGrantTest.php | 10 ++++++++--
6 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/examples/src/Repositories/ClientRepository.php b/examples/src/Repositories/ClientRepository.php
index 047fe77f7..0b19d57d7 100644
--- a/examples/src/Repositories/ClientRepository.php
+++ b/examples/src/Repositories/ClientRepository.php
@@ -59,10 +59,7 @@ public function validateClient($clientIdentifier, $clientSecret, $grantType): bo
return false;
}
- if (
- $clients[$clientIdentifier]['is_confidential'] === true
- && password_verify($clientSecret, $clients[$clientIdentifier]['secret']) === false
- ) {
+ if (password_verify($clientSecret, $clients[$clientIdentifier]['secret']) === false) {
return false;
}
diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php
index af13c3e34..8249edfae 100644
--- a/src/Grant/AbstractGrant.php
+++ b/src/Grant/AbstractGrant.php
@@ -151,12 +151,13 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity
{
[$clientId, $clientSecret] = $this->getClientCredentials($request);
- if ($this->clientRepository->validateClient($clientId, $clientSecret, $this->getIdentifier()) === false) {
+ $client = $this->getClientEntityOrFail($clientId, $request);
+
+ if ($client->isConfidential() && $this->clientRepository->validateClient($clientId, $clientSecret, $this->getIdentifier()) === false) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
throw OAuthServerException::invalidClient($request);
}
- $client = $this->getClientEntityOrFail($clientId, $request);
// If a redirect URI is provided ensure it matches what is pre-registered
$redirectUri = $this->getRequestParameter('redirect_uri', $request);
@@ -488,9 +489,13 @@ protected function issueAuthCode(
*/
protected function issueRefreshToken(AccessTokenEntityInterface $accessToken): ?RefreshTokenEntityInterface
{
+ if (!$accessToken->getClient()->hasGrantType('refresh_token')) {
+ return null;
+ }
+
$refreshToken = $this->refreshTokenRepository->getNewRefreshToken();
- if ($refreshToken === null || !$accessToken->getClient()->hasGrantType('refresh_token')) {
+ if ($refreshToken === null) {
return null;
}
diff --git a/tests/Grant/AbstractGrantTest.php b/tests/Grant/AbstractGrantTest.php
index 7e2193e87..652150bd6 100644
--- a/tests/Grant/AbstractGrantTest.php
+++ b/tests/Grant/AbstractGrantTest.php
@@ -424,6 +424,7 @@ public function testIssueNullRefreshToken(): void
$issueRefreshTokenMethod->setAccessible(true);
$accessToken = new AccessTokenEntity();
+ $accessToken->setClient(new ClientEntity());
self::assertNull($issueRefreshTokenMethod->invoke($grantMock, $accessToken));
}
diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php
index dcd02267e..0d9aaf02e 100644
--- a/tests/Grant/AuthCodeGrantTest.php
+++ b/tests/Grant/AuthCodeGrantTest.php
@@ -687,8 +687,11 @@ public function testRespondToAccessTokenRequestWithDefaultRedirectUri(): void
$scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity);
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
@@ -886,8 +889,11 @@ public function testRespondToAccessTokenRequestNullRefreshToken(): void
$scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity);
$scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0);
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
diff --git a/tests/Grant/PasswordGrantTest.php b/tests/Grant/PasswordGrantTest.php
index 011a9605e..5c91c94f9 100644
--- a/tests/Grant/PasswordGrantTest.php
+++ b/tests/Grant/PasswordGrantTest.php
@@ -93,8 +93,11 @@ public function testRespondToRequestNullRefreshToken(): void
$clientRepositoryMock->method('getClientEntity')->willReturn($client);
$clientRepositoryMock->method('validateClient')->willReturn(true);
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf();
$userRepositoryMock = $this->getMockBuilder(UserRepositoryInterface::class)->getMock();
@@ -169,9 +172,14 @@ public function testRespondToRequestMissingPassword(): void
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
+ $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
+ $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn(new ScopeEntity());
+
$grant = new PasswordGrant($userRepositoryMock, $refreshTokenRepositoryMock);
$grant->setClientRepository($clientRepositoryMock);
$grant->setAccessTokenRepository($accessTokenRepositoryMock);
+ $grant->setDefaultScope(self::DEFAULT_SCOPE);
+ $grant->setScopeRepository($scopeRepositoryMock);
$serverRequest = (new ServerRequest())->withParsedBody([
'client_id' => 'foo',
diff --git a/tests/Grant/RefreshTokenGrantTest.php b/tests/Grant/RefreshTokenGrantTest.php
index 421a7551c..b642a9761 100644
--- a/tests/Grant/RefreshTokenGrantTest.php
+++ b/tests/Grant/RefreshTokenGrantTest.php
@@ -127,8 +127,11 @@ public function testRespondToRequestNullRefreshToken(): void
$scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity);
$scopeRepositoryMock->method('finalizeScopes')->willReturn([$scopeEntity]);
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->expects(self::once())->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
@@ -630,8 +633,11 @@ public function testRevokedRefreshToken(): void
$scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity);
$scopeRepositoryMock->method('finalizeScopes')->willReturn([$scopeEntity]);
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessToken);
$accessTokenRepositoryMock->expects(self::once())->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
From 34d83aa0b3b02980f18bfffa78cfc76b203f80bb Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Tue, 22 Oct 2024 20:00:33 +0330
Subject: [PATCH 07/12] require client_secret for confidential clients
---
src/Grant/AbstractGrant.php | 12 +++++++++---
tests/Grant/AuthCodeGrantTest.php | 12 +++++++-----
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php
index 8249edfae..92f76739f 100644
--- a/src/Grant/AbstractGrant.php
+++ b/src/Grant/AbstractGrant.php
@@ -153,10 +153,16 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity
$client = $this->getClientEntityOrFail($clientId, $request);
- if ($client->isConfidential() && $this->clientRepository->validateClient($clientId, $clientSecret, $this->getIdentifier()) === false) {
- $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
+ if ($client->isConfidential()) {
+ if ($clientSecret === '') {
+ throw OAuthServerException::invalidRequest('client_secret');
+ }
- throw OAuthServerException::invalidClient($request);
+ if ($this->clientRepository->validateClient($clientId, $clientSecret, $this->getIdentifier()) === false) {
+ $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
+
+ throw OAuthServerException::invalidClient($request);
+ }
}
// If a redirect URI is provided ensure it matches what is pre-registered
diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php
index 0d9aaf02e..8fa37505c 100644
--- a/tests/Grant/AuthCodeGrantTest.php
+++ b/tests/Grant/AuthCodeGrantTest.php
@@ -649,6 +649,7 @@ public function testRespondToAccessTokenRequest(): void
[
'grant_type' => 'authorization_code',
'client_id' => 'foo',
+ 'client_secret' => 'bar',
'redirect_uri' => self::REDIRECT_URI,
'code' => $this->cryptStub->doEncrypt(
json_encode([
@@ -722,6 +723,7 @@ public function testRespondToAccessTokenRequestWithDefaultRedirectUri(): void
[
'grant_type' => 'authorization_code',
'client_id' => 'foo',
+ 'client_secret' => 'bar',
'code' => $this->cryptStub->doEncrypt(
json_encode([
'auth_code_id' => uniqid(),
@@ -997,6 +999,7 @@ public function testRespondToAccessTokenRequestCodeChallengePlain(): void
[
'grant_type' => 'authorization_code',
'client_id' => 'foo',
+ 'client_secret' => 'bar',
'redirect_uri' => self::REDIRECT_URI,
'code_verifier' => self::CODE_VERIFIER,
'code' => $this->cryptStub->doEncrypt(
@@ -1073,6 +1076,7 @@ public function testRespondToAccessTokenRequestCodeChallengeS256(): void
[
'grant_type' => 'authorization_code',
'client_id' => 'foo',
+ 'client_secret' => 'bar',
'redirect_uri' => self::REDIRECT_URI,
'code_verifier' => self::CODE_VERIFIER,
'code' => $this->cryptStub->doEncrypt(
@@ -1556,6 +1560,7 @@ public function testRespondToAccessTokenRequestRevokedCode(): void
[
'grant_type' => 'authorization_code',
'client_id' => 'foo',
+ 'client_secret' => 'bar',
'redirect_uri' => self::REDIRECT_URI,
'code' => $this->cryptStub->doEncrypt(
json_encode([
@@ -1620,6 +1625,7 @@ public function testRespondToAccessTokenRequestClientMismatch(): void
[
'grant_type' => 'authorization_code',
'client_id' => 'foo',
+ 'client_secret' => 'bar',
'redirect_uri' => self::REDIRECT_URI,
'code' => $this->cryptStub->doEncrypt(
json_encode([
@@ -1683,6 +1689,7 @@ public function testRespondToAccessTokenRequestBadCodeEncryption(): void
[
'grant_type' => 'authorization_code',
'client_id' => 'foo',
+ 'client_secret' => 'bar',
'redirect_uri' => self::REDIRECT_URI,
'code' => 'sdfsfsd',
]
@@ -1702,7 +1709,6 @@ public function testRespondToAccessTokenRequestBadCodeVerifierPlain(): void
$client->setIdentifier('foo');
$client->setRedirectUri(self::REDIRECT_URI);
- $client->setConfidential();
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
@@ -1777,7 +1783,6 @@ public function testRespondToAccessTokenRequestBadCodeVerifierS256(): void
$client->setIdentifier('foo');
$client->setRedirectUri(self::REDIRECT_URI);
- $client->setConfidential();
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
@@ -1852,7 +1857,6 @@ public function testRespondToAccessTokenRequestMalformedCodeVerifierS256WithInva
$client->setIdentifier('foo');
$client->setRedirectUri(self::REDIRECT_URI);
- $client->setConfidential();
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
@@ -1927,7 +1931,6 @@ public function testRespondToAccessTokenRequestMalformedCodeVerifierS256WithInva
$client->setIdentifier('foo');
$client->setRedirectUri(self::REDIRECT_URI);
- $client->setConfidential();
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
@@ -2002,7 +2005,6 @@ public function testRespondToAccessTokenRequestMissingCodeVerifier(): void
$client->setIdentifier('foo');
$client->setRedirectUri(self::REDIRECT_URI);
- $client->setConfidential();
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
From c4c73629851785ce342e696134dd590c93db30d4 Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Sat, 9 Nov 2024 23:34:40 +0330
Subject: [PATCH 08/12] redirect uri is required on auth code
---
src/Grant/AbstractGrant.php | 11 ++---------
src/Grant/AuthCodeGrant.php | 2 ++
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php
index 92f76739f..32479638a 100644
--- a/src/Grant/AbstractGrant.php
+++ b/src/Grant/AbstractGrant.php
@@ -165,13 +165,6 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity
}
}
- // If a redirect URI is provided ensure it matches what is pre-registered
- $redirectUri = $this->getRequestParameter('redirect_uri', $request);
-
- if ($redirectUri !== null) {
- $this->validateRedirectUri($redirectUri, $client, $request);
- }
-
return $client;
}
@@ -233,13 +226,13 @@ protected function getClientCredentials(ServerRequestInterface $request): array
* @throws OAuthServerException
*/
protected function validateRedirectUri(
- string $redirectUri,
+ ?string $redirectUri,
ClientEntityInterface $client,
ServerRequestInterface $request
): void {
$validator = new RedirectUriValidator($client->getRedirectUri());
- if (!$validator->validateRedirectUri($redirectUri)) {
+ if (is_null($redirectUri) || !$validator->validateRedirectUri($redirectUri)) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
throw OAuthServerException::invalidClient($request);
}
diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php
index 0b857671a..c5fa25479 100644
--- a/src/Grant/AuthCodeGrant.php
+++ b/src/Grant/AuthCodeGrant.php
@@ -98,6 +98,8 @@ public function respondToAccessTokenRequest(
): ResponseTypeInterface {
$client = $this->validateClient($request);
+ $this->validateRedirectUri($this->getRequestParameter('redirect_uri', $request), $client, $request);
+
$encryptedAuthCode = $this->getRequestParameter('code', $request);
if ($encryptedAuthCode === null) {
From cd2f0bdbdfa9a82e3b96e71f49674cf3f18a186c Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Sat, 9 Nov 2024 23:47:16 +0330
Subject: [PATCH 09/12] fix tests
---
src/Grant/AbstractGrant.php | 4 +-
src/Grant/AuthCodeGrant.php | 2 -
tests/Grant/AbstractGrantTest.php | 78 -------------------------------
3 files changed, 2 insertions(+), 82 deletions(-)
diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php
index 32479638a..18e34c1a3 100644
--- a/src/Grant/AbstractGrant.php
+++ b/src/Grant/AbstractGrant.php
@@ -226,13 +226,13 @@ protected function getClientCredentials(ServerRequestInterface $request): array
* @throws OAuthServerException
*/
protected function validateRedirectUri(
- ?string $redirectUri,
+ string $redirectUri,
ClientEntityInterface $client,
ServerRequestInterface $request
): void {
$validator = new RedirectUriValidator($client->getRedirectUri());
- if (is_null($redirectUri) || !$validator->validateRedirectUri($redirectUri)) {
+ if (!$validator->validateRedirectUri($redirectUri)) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
throw OAuthServerException::invalidClient($request);
}
diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php
index c5fa25479..0b857671a 100644
--- a/src/Grant/AuthCodeGrant.php
+++ b/src/Grant/AuthCodeGrant.php
@@ -98,8 +98,6 @@ public function respondToAccessTokenRequest(
): ResponseTypeInterface {
$client = $this->validateClient($request);
- $this->validateRedirectUri($this->getRequestParameter('redirect_uri', $request), $client, $request);
-
$encryptedAuthCode = $this->getRequestParameter('code', $request);
if ($encryptedAuthCode === null) {
diff --git a/tests/Grant/AbstractGrantTest.php b/tests/Grant/AbstractGrantTest.php
index 652150bd6..e46cd0419 100644
--- a/tests/Grant/AbstractGrantTest.php
+++ b/tests/Grant/AbstractGrantTest.php
@@ -265,84 +265,6 @@ public function testValidateClientInvalidClientSecret(): void
$validateClientMethod->invoke($grantMock, $serverRequest, true, true);
}
- public function testValidateClientInvalidRedirectUri(): void
- {
- $client = new ClientEntity();
- $client->setRedirectUri('http://foo/bar');
- $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
- $clientRepositoryMock->method('getClientEntity')->willReturn($client);
-
- /** @var AbstractGrant $grantMock */
- $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
- $grantMock->setClientRepository($clientRepositoryMock);
-
- $abstractGrantReflection = new ReflectionClass($grantMock);
-
- $serverRequest = (new ServerRequest())->withParsedBody([
- 'client_id' => 'foo',
- 'redirect_uri' => 'http://bar/foo',
- ]);
-
- $validateClientMethod = $abstractGrantReflection->getMethod('validateClient');
- $validateClientMethod->setAccessible(true);
-
- $this->expectException(OAuthServerException::class);
-
- $validateClientMethod->invoke($grantMock, $serverRequest, true, true);
- }
-
- public function testValidateClientInvalidRedirectUriArray(): void
- {
- $client = new ClientEntity();
- $client->setRedirectUri(['http://foo/bar']);
- $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
- $clientRepositoryMock->method('getClientEntity')->willReturn($client);
-
- /** @var AbstractGrant $grantMock */
- $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
- $grantMock->setClientRepository($clientRepositoryMock);
-
- $abstractGrantReflection = new ReflectionClass($grantMock);
-
- $serverRequest = (new ServerRequest())->withParsedBody([
- 'client_id' => 'foo',
- 'redirect_uri' => 'http://bar/foo',
- ]);
-
- $validateClientMethod = $abstractGrantReflection->getMethod('validateClient');
- $validateClientMethod->setAccessible(true);
-
- $this->expectException(OAuthServerException::class);
-
- $validateClientMethod->invoke($grantMock, $serverRequest, true, true);
- }
-
- public function testValidateClientMalformedRedirectUri(): void
- {
- $client = new ClientEntity();
- $client->setRedirectUri('http://foo/bar');
- $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
- $clientRepositoryMock->method('getClientEntity')->willReturn($client);
-
- /** @var AbstractGrant $grantMock */
- $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
- $grantMock->setClientRepository($clientRepositoryMock);
-
- $abstractGrantReflection = new ReflectionClass($grantMock);
-
- $serverRequest = (new ServerRequest())->withParsedBody([
- 'client_id' => 'foo',
- 'redirect_uri' => ['not', 'a', 'string'],
- ]);
-
- $validateClientMethod = $abstractGrantReflection->getMethod('validateClient');
- $validateClientMethod->setAccessible(true);
-
- $this->expectException(OAuthServerException::class);
-
- $validateClientMethod->invoke($grantMock, $serverRequest, true, true);
- }
-
public function testValidateClientBadClient(): void
{
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
From 4998c4a2f2afaa17cc644a666eeb4f7a69bb9663 Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Tue, 19 Nov 2024 12:51:46 +0330
Subject: [PATCH 10/12] fix tests
---
tests/Grant/AuthCodeGrantTest.php | 1 +
tests/Grant/DeviceCodeGrantTest.php | 6 +++---
tests/Grant/RefreshTokenGrantTest.php | 4 +++-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php
index f30b8a093..393359cf0 100644
--- a/tests/Grant/AuthCodeGrantTest.php
+++ b/tests/Grant/AuthCodeGrantTest.php
@@ -1745,6 +1745,7 @@ public function testRespondToAccessTokenRequestNoEncryptionKey(): void
[
'grant_type' => 'authorization_code',
'client_id' => 'foo',
+ 'client_secret' => 'bar',
'redirect_uri' => self::REDIRECT_URI,
'code' => 'badCode',
]
diff --git a/tests/Grant/DeviceCodeGrantTest.php b/tests/Grant/DeviceCodeGrantTest.php
index cdbb843e4..c02c1de08 100644
--- a/tests/Grant/DeviceCodeGrantTest.php
+++ b/tests/Grant/DeviceCodeGrantTest.php
@@ -301,7 +301,7 @@ public function testDeviceAuthorizationResponse(): void
$server->setDefaultScope(self::DEFAULT_SCOPE);
$serverRequest = (new ServerRequest())->withParsedBody([
- 'client_id' => 'foo',
+ 'client_id' => 'foo',
]);
$deviceCodeGrant = new DeviceCodeGrant(
@@ -698,8 +698,8 @@ public function testIssueAccessDeniedError(): void
$grant->completeDeviceAuthorizationRequest($deviceCode->getIdentifier(), '1', false);
$serverRequest = (new ServerRequest())->withParsedBody([
- 'client_id' => 'foo',
- 'device_code' => $deviceCode->getIdentifier(),
+ 'client_id' => 'foo',
+ 'device_code' => $deviceCode->getIdentifier(),
]);
$responseType = new StubResponseType();
diff --git a/tests/Grant/RefreshTokenGrantTest.php b/tests/Grant/RefreshTokenGrantTest.php
index 1d8c23a27..1fe1eadab 100644
--- a/tests/Grant/RefreshTokenGrantTest.php
+++ b/tests/Grant/RefreshTokenGrantTest.php
@@ -784,7 +784,9 @@ public function testRespondToRequestWithIntUserId(): void
$scopeRepositoryMock->method('finalizeScopes')->willReturn([$scopeEntity]);
$accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
- $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
+ $accessTokenEntity = new AccessTokenEntity();
+ $accessTokenEntity->setClient($client);
+ $accessTokenRepositoryMock->method('getNewToken')->willReturn($accessTokenEntity);
$accessTokenRepositoryMock->expects(self::once())->method('persistNewAccessToken')->willReturnSelf();
$refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
From cc3a24497be85368c24b321020f14b9544e85e7a Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Wed, 4 Dec 2024 21:53:52 +0330
Subject: [PATCH 11/12] bypass bc breaking change and add tests
---
src/Entities/ClientEntityInterface.php | 6 ++-
src/Entities/Traits/ClientTrait.php | 4 +-
src/Grant/AbstractGrant.php | 13 ++++++-
tests/Grant/AbstractGrantTest.php | 53 ++++++++++++++++++++++++++
4 files changed, 70 insertions(+), 6 deletions(-)
diff --git a/src/Entities/ClientEntityInterface.php b/src/Entities/ClientEntityInterface.php
index 138e831f7..beba39d6b 100644
--- a/src/Entities/ClientEntityInterface.php
+++ b/src/Entities/ClientEntityInterface.php
@@ -40,7 +40,9 @@ public function getRedirectUri(): string|array;
public function isConfidential(): bool;
/**
- * Returns true if the client handles the given grant type.
+ * Returns true if the client supports the given grant type.
+ *
+ * To be added in a future major release.
*/
- public function hasGrantType(string $grantType): bool;
+ // public function supportsGrantType(string $grantType): bool;
}
diff --git a/src/Entities/Traits/ClientTrait.php b/src/Entities/Traits/ClientTrait.php
index 0b4327c35..ada53fa5a 100644
--- a/src/Entities/Traits/ClientTrait.php
+++ b/src/Entities/Traits/ClientTrait.php
@@ -54,9 +54,9 @@ public function isConfidential(): bool
}
/**
- * Returns true if the client handles the given grant type.
+ * Returns true if the client supports the given grant type.
*/
- public function hasGrantType(string $grantType): bool
+ public function supportsGrantType(string $grantType): bool
{
return true;
}
diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php
index c2e67cb28..7c27e95c5 100644
--- a/src/Grant/AbstractGrant.php
+++ b/src/Grant/AbstractGrant.php
@@ -189,13 +189,22 @@ protected function getClientEntityOrFail(string $clientId, ServerRequestInterfac
throw OAuthServerException::invalidClient($request);
}
- if (!$client->hasGrantType($this->getIdentifier())) {
+ if ($this->supportsGrantType($client, $this->getIdentifier()) === false) {
throw OAuthServerException::unauthorizedClient();
}
return $client;
}
+ /**
+ * Returns true if the given client is authorized to use the given grant type.
+ */
+ protected function supportsGrantType(ClientEntityInterface $client, string $grantType): bool
+ {
+ return method_exists($client, 'supportsGrantType') === false
+ || $client->supportsGrantType($grantType) === true;
+ }
+
/**
* Gets the client credentials from the request from the request body or
* the Http Basic Authorization header
@@ -488,7 +497,7 @@ protected function issueAuthCode(
*/
protected function issueRefreshToken(AccessTokenEntityInterface $accessToken): ?RefreshTokenEntityInterface
{
- if (!$accessToken->getClient()->hasGrantType('refresh_token')) {
+ if ($this->supportsGrantType($accessToken->getClient(), 'refresh_token') === false) {
return null;
}
diff --git a/tests/Grant/AbstractGrantTest.php b/tests/Grant/AbstractGrantTest.php
index e46cd0419..c13c2d2e5 100644
--- a/tests/Grant/AbstractGrantTest.php
+++ b/tests/Grant/AbstractGrantTest.php
@@ -289,6 +289,32 @@ public function testValidateClientBadClient(): void
$validateClientMethod->invoke($grantMock, $serverRequest, true);
}
+ public function testUnauthorizedClient(): void
+ {
+ $client = $this->getMockBuilder(ClientEntity::class)->getMock();
+ $client->method('supportsGrantType')->willReturn(false);
+
+ $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+ $clientRepositoryMock
+ ->expects(self::once())
+ ->method('getClientEntity')
+ ->with('foo')
+ ->willReturn($client);
+
+ /** @var AbstractGrant $grantMock */
+ $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
+ $grantMock->setClientRepository($clientRepositoryMock);
+
+ $abstractGrantReflection = new ReflectionClass($grantMock);
+
+ $getClientEntityOrFailMethod = $abstractGrantReflection->getMethod('getClientEntityOrFail');
+ $getClientEntityOrFailMethod->setAccessible(true);
+
+ $this->expectException(OAuthServerException::class);
+
+ $getClientEntityOrFailMethod->invoke($grantMock, 'foo', new ServerRequest());
+ }
+
public function testCanRespondToRequest(): void
{
$grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
@@ -350,6 +376,33 @@ public function testIssueNullRefreshToken(): void
self::assertNull($issueRefreshTokenMethod->invoke($grantMock, $accessToken));
}
+ public function testIssueNullRefreshTokenUnauthorizedClient(): void
+ {
+ $client = $this->getMockBuilder(ClientEntity::class)->getMock();
+ $client
+ ->expects(self::once())
+ ->method('supportsGrantType')
+ ->with('refresh_token')
+ ->willReturn(false);
+
+ $refreshTokenRepoMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
+ $refreshTokenRepoMock->expects(self::never())->method('getNewRefreshToken');
+
+ /** @var AbstractGrant $grantMock */
+ $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
+ $grantMock->setRefreshTokenTTL(new DateInterval('PT1M'));
+ $grantMock->setRefreshTokenRepository($refreshTokenRepoMock);
+
+ $abstractGrantReflection = new ReflectionClass($grantMock);
+ $issueRefreshTokenMethod = $abstractGrantReflection->getMethod('issueRefreshToken');
+ $issueRefreshTokenMethod->setAccessible(true);
+
+ $accessToken = new AccessTokenEntity();
+ $accessToken->setClient($client);
+
+ self::assertNull($issueRefreshTokenMethod->invoke($grantMock, $accessToken));
+ }
+
public function testIssueAccessToken(): void
{
$accessTokenRepoMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
From db44e64bf8d15cfe766a6e31300ee11ec1c99aef Mon Sep 17 00:00:00 2001
From: Hafez Divandari <hafezdivandari@gmail.com>
Date: Wed, 4 Dec 2024 21:56:59 +0330
Subject: [PATCH 12/12] formatting
---
src/Entities/ClientEntityInterface.php | 2 +-
tests/Grant/AbstractGrantTest.php | 52 +++++++++++++-------------
2 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/src/Entities/ClientEntityInterface.php b/src/Entities/ClientEntityInterface.php
index beba39d6b..b0050ecea 100644
--- a/src/Entities/ClientEntityInterface.php
+++ b/src/Entities/ClientEntityInterface.php
@@ -39,7 +39,7 @@ public function getRedirectUri(): string|array;
*/
public function isConfidential(): bool;
- /**
+ /*
* Returns true if the client supports the given grant type.
*
* To be added in a future major release.
diff --git a/tests/Grant/AbstractGrantTest.php b/tests/Grant/AbstractGrantTest.php
index c13c2d2e5..5711af103 100644
--- a/tests/Grant/AbstractGrantTest.php
+++ b/tests/Grant/AbstractGrantTest.php
@@ -289,32 +289,6 @@ public function testValidateClientBadClient(): void
$validateClientMethod->invoke($grantMock, $serverRequest, true);
}
- public function testUnauthorizedClient(): void
- {
- $client = $this->getMockBuilder(ClientEntity::class)->getMock();
- $client->method('supportsGrantType')->willReturn(false);
-
- $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
- $clientRepositoryMock
- ->expects(self::once())
- ->method('getClientEntity')
- ->with('foo')
- ->willReturn($client);
-
- /** @var AbstractGrant $grantMock */
- $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
- $grantMock->setClientRepository($clientRepositoryMock);
-
- $abstractGrantReflection = new ReflectionClass($grantMock);
-
- $getClientEntityOrFailMethod = $abstractGrantReflection->getMethod('getClientEntityOrFail');
- $getClientEntityOrFailMethod->setAccessible(true);
-
- $this->expectException(OAuthServerException::class);
-
- $getClientEntityOrFailMethod->invoke($grantMock, 'foo', new ServerRequest());
- }
-
public function testCanRespondToRequest(): void
{
$grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
@@ -553,4 +527,30 @@ public function testCompleteAuthorizationRequest(): void
$grantMock->completeAuthorizationRequest(new AuthorizationRequest());
}
+
+ public function testUnauthorizedClient(): void
+ {
+ $client = $this->getMockBuilder(ClientEntity::class)->getMock();
+ $client->method('supportsGrantType')->willReturn(false);
+
+ $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+ $clientRepositoryMock
+ ->expects(self::once())
+ ->method('getClientEntity')
+ ->with('foo')
+ ->willReturn($client);
+
+ /** @var AbstractGrant $grantMock */
+ $grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
+ $grantMock->setClientRepository($clientRepositoryMock);
+
+ $abstractGrantReflection = new ReflectionClass($grantMock);
+
+ $getClientEntityOrFailMethod = $abstractGrantReflection->getMethod('getClientEntityOrFail');
+ $getClientEntityOrFailMethod->setAccessible(true);
+
+ $this->expectException(OAuthServerException::class);
+
+ $getClientEntityOrFailMethod->invoke($grantMock, 'foo', new ServerRequest());
+ }
}