From 491852b521828e4856ddbb5b620afcfd83d05af4 Mon Sep 17 00:00:00 2001
From: sephster <andrew@noexceptions.io>
Date: Mon, 13 Aug 2018 21:47:53 +0100
Subject: [PATCH] Move code challenge check to auth code request

---
 src/Grant/AuthCodeGrant.php       |  8 +++----
 tests/Grant/AuthCodeGrantTest.php | 40 +++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php
index 6e38bc09..407b98cf 100644
--- a/src/Grant/AuthCodeGrant.php
+++ b/src/Grant/AuthCodeGrant.php
@@ -151,7 +151,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
 
         // Validate code challenge
         if (!empty($authCodePayload->code_challenge)) {
-	        $codeVerifier = $this->getRequestParameter('code_verifier', $request, null);
+            $codeVerifier = $this->getRequestParameter('code_verifier', $request, null);
 
             if ($codeVerifier === null) {
                 throw OAuthServerException::invalidRequest('code_verifier');
@@ -193,8 +193,6 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
                     );
                 // @codeCoverageIgnoreEnd
             }
-        } else if ($this->requireCodeChallengeForPublicClients && !$client->isConfidential()) {
-            throw OAuthServerException::invalidRequest('code_challenge', 'Code challenge must be provided for public clients');
         }
 
         // Issue and persist access + refresh tokens
@@ -296,7 +294,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
 
         $authorizationRequest->setScopes($scopes);
 
-	$codeChallenge = $this->getQueryStringParameter('code_challenge', $request);
+        $codeChallenge = $this->getQueryStringParameter('code_challenge', $request);
 
         if ($codeChallenge !== null) {
             $codeChallengeMethod = $this->getQueryStringParameter('code_challenge_method', $request, 'plain');
@@ -319,6 +317,8 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
 
             $authorizationRequest->setCodeChallenge($codeChallenge);
             $authorizationRequest->setCodeChallengeMethod($codeChallengeMethod);
+        } else if ($this->requireCodeChallengeForPublicClients && !$client->isConfidential()) {
+            throw OAuthServerException::invalidRequest('code_challenge', 'Code challenge must be provided for public clients');
         }
 
         return $authorizationRequest;
diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php
index fc986568..564389e2 100644
--- a/tests/Grant/AuthCodeGrantTest.php
+++ b/tests/Grant/AuthCodeGrantTest.php
@@ -1784,4 +1784,44 @@ class AuthCodeGrantTest extends TestCase
 
         $grant->completeAuthorizationRequest(new AuthorizationRequest());
     }
+
+    public function testPublicClientAuthCodeRequestRejectedWhenCodeChallengeRequiredButNotGiven()
+    {
+        $client = new ClientEntity();
+        $client->setRedirectUri('http://foo/bar');
+
+        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
+
+        $scope = new ScopeEntity();
+        $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
+        $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scope);
+
+        $grant = new AuthCodeGrant(
+            $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
+            $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
+            new \DateInterval('PT10M')
+        );
+
+        $grant->setClientRepository($clientRepositoryMock);
+        $grant->setScopeRepository($scopeRepositoryMock);
+        $grant->setDefaultScope(self::DEFAULT_SCOPE);
+
+        $request = new ServerRequest(
+            [],
+            [],
+            null,
+            null,
+            'php://input',
+            [],
+            [],
+            [
+                'response_type' => 'code',
+                'client_id'     => 'foo',
+                'redirect_uri'  => 'http://foo/bar',
+            ]
+        );
+
+        $this->assertInstanceOf(AuthorizationRequest::class, $grant->validateAuthorizationRequest($request));
+    }
 }