Merge pull request #1010 from iansltx/protect-client-entity-gets

Ensure unvalidated ClientEntity gets throw/emit if they return null

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Public clients can now use the Auth Code Grant (PR #938)
 - `isConfidential` getter added to `ClientEntity` to identify type of client (PR #938)
 - Function `validateClient()` added to validate clients which was previously performed by the `getClientEntity()` function (PR #938)
+- Add a new function to the AbstractGrant class called `getClientEntityOrFail()`. This is a wrapper around the `getClientEntity()` function that ensures we emit and throw an exception if the repo doesn't return a client entity. (PR #1010)
 
 ### Changed
 - Replace `convertToJWT()` interface with a more generic `__toString()` to improve extensibility; AccessTokenEntityInterface now requires `setPrivateKey(CryptKey $privateKey)` so `__toString()` has everything it needs to work (PR #874)

src/Grant/AbstractGrant.php

@@ -197,6 +197,33 @@ abstract class AbstractGrant implements GrantTypeInterface
         return $client;
     }
 
+    /**
+     * Wrapper around ClientRepository::getClientEntity() that ensures we emit
+     * an event and throw an exception if the repo doesn't return a client
+     * entity.
+     *
+     * This is a bit of defensive coding because the interface contract
+     * doesn't actually enforce non-null returns/exception-on-no-client so
+     * getClientEntity might return null. By contrast, this method will
+     * always either return a ClientEntityInterface or throw.
+     *
+     * @param string                 $clientId
+     * @param ServerRequestInterface $request
+     *
+     * @return ClientEntityInterface
+     */
+    protected function getClientEntityOrFail($clientId, ServerRequestInterface $request)
+    {
+        $client = $this->clientRepository->getClientEntity($clientId);
+
+        if ($client instanceof ClientEntityInterface === false) {
+            $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
+            throw OAuthServerException::invalidClient($request);
+        }
+
+        return $client;
+    }
+
     /**
      * Gets the client credentials from the request from the request body or
      * the Http Basic Authorization header

src/Grant/AuthCodeGrant.php

@@ -97,7 +97,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
     ) {
         list($clientId) = $this->getClientCredentials($request);
 
-        $client = $this->clientRepository->getClientEntity($clientId);
+        $client = $this->getClientEntityOrFail($clientId, $request);
 
         // Only validate the client if it is confidential
         if ($client->isConfidential()) {
@@ -113,11 +113,8 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
         try {
             $authCodePayload = json_decode($this->decrypt($encryptedAuthCode));
 
-
             $this->validateAuthorizationCode($authCodePayload, $client, $request);
 
-
-
             $scopes = $this->scopeRepository->finalizeScopes(
                 $this->validateScopes($authCodePayload->scopes),
                 $this->getIdentifier(),
@@ -252,12 +249,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
             throw OAuthServerException::invalidRequest('client_id');
         }
 
-        $client = $this->clientRepository->getClientEntity($clientId);
+        $client = $this->getClientEntityOrFail($clientId, $request);
-
-        if ($client instanceof ClientEntityInterface === false) {
-            $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
-            throw OAuthServerException::invalidClient($request);
-        }
 
         $redirectUri = $this->getQueryStringParameter('redirect_uri', $request);
 

src/Grant/ImplicitGrant.php

@@ -10,7 +10,6 @@
 namespace League\OAuth2\Server\Grant;
 
 use DateInterval;
-use League\OAuth2\Server\Entities\ClientEntityInterface;
 use League\OAuth2\Server\Entities\UserEntityInterface;
 use League\OAuth2\Server\Exception\OAuthServerException;
 use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface;
@@ -125,12 +124,7 @@ class ImplicitGrant extends AbstractAuthorizeGrant
             throw OAuthServerException::invalidRequest('client_id');
         }
 
-        $client = $this->clientRepository->getClientEntity($clientId);
+        $client = $this->getClientEntityOrFail($clientId, $request);
-
-        if ($client instanceof ClientEntityInterface === false) {
-            $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
-            throw OAuthServerException::invalidClient($request);
-        }
 
         $redirectUri = $this->getQueryStringParameter('redirect_uri', $request);