Merge pull request #1420 from hafezdivandari/always-validate-client

Validate confidential clients and determine if the client handles the grant type

Author: Sephster

src/Entities/ClientEntityInterface.php

@@ -38,4 +38,11 @@ public function getRedirectUri(): string|array;
      * Returns true if the client is confidential.
      */
     public function isConfidential(): bool;
+
+    /*
+     * Returns true if the client supports the given grant type.
+     *
+     * To be added in a future major release.
+     */
+    // public function supportsGrantType(string $grantType): bool;
 }

src/Entities/Traits/ClientTrait.php

@@ -52,4 +52,12 @@ public function isConfidential(): bool
     {
         return $this->isConfidential;
     }
+
+    /**
+     * Returns true if the client supports the given grant type.
+     */
+    public function supportsGrantType(string $grantType): bool
+    {
+        return true;
+    }
 }

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
+        );
     }
 
     /**

src/Grant/AbstractGrant.php

@@ -151,18 +151,18 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity
     {
         [$clientId, $clientSecret] = $this->getClientCredentials($request);
 
-        if ($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);
+        if ($client->isConfidential()) {
+            if ($clientSecret === '') {
+                throw OAuthServerException::invalidRequest('client_secret');
+            }
 
-        if ($redirectUri !== null) {
-            $this->validateRedirectUri($redirectUri, $client, $request);
+            if ($this->clientRepository->validateClient($clientId, $clientSecret, $this->getIdentifier()) === false) {
+                $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
+
+                throw OAuthServerException::invalidClient($request);
+            }
         }
 
         return $client;
@@ -189,9 +189,22 @@ protected function getClientEntityOrFail(string $clientId, ServerRequestInterfac
             throw OAuthServerException::invalidClient($request);
         }
 
+        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
@@ -484,6 +497,10 @@ protected function issueAuthCode(
      */
     protected function issueRefreshToken(AccessTokenEntityInterface $accessToken): ?RefreshTokenEntityInterface
     {
+        if ($this->supportsGrantType($accessToken->getClient(), 'refresh_token') === false) {
+            return null;
+        }
+
         $refreshToken = $this->refreshTokenRepository->getNewRefreshToken();
 
         if ($refreshToken === null) {

src/Grant/AuthCodeGrant.php

@@ -97,14 +97,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);
 

src/Grant/ClientCredentialsGrant.php

@@ -34,19 +34,14 @@ 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));
 
             throw OAuthServerException::invalidClient($request);
         }
 
-        // Validate request
-        $this->validateClient($request);
-
         $scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope));
 
         // Finalize the requested scopes

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();
@@ -398,6 +320,7 @@ public function testIssueRefreshToken(): void
         $issueRefreshTokenMethod->setAccessible(true);
 
         $accessToken = new AccessTokenEntity();
+        $accessToken->setClient(new ClientEntity());
 
         /** @var RefreshTokenEntityInterface $refreshToken */
         $refreshToken = $issueRefreshTokenMethod->invoke($grantMock, $accessToken);
@@ -423,6 +346,34 @@ public function testIssueNullRefreshToken(): void
         $issueRefreshTokenMethod->setAccessible(true);
 
         $accessToken = new AccessTokenEntity();
+        $accessToken->setClient(new ClientEntity());
+        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));
     }
 
@@ -576,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());
+    }
 }