From 57d199b88962c4f462719a55eb72cdb373efc7aa Mon Sep 17 00:00:00 2001
From: Alex Bilbie <hello@alexbilbie.com>
Date: Fri, 16 Jun 2017 16:52:36 +0100
Subject: [PATCH] Stricter validation of code challenge value to match RFC 7636
 requirements

---
 src/Grant/AuthCodeGrant.php       |   7 ++
 tests/Grant/AuthCodeGrantTest.php | 113 +++++++++++++++++++++++++++++-
 2 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php
index df89400e..119c1f96 100644
--- a/src/Grant/AuthCodeGrant.php
+++ b/src/Grant/AuthCodeGrant.php
@@ -264,6 +264,13 @@ class AuthCodeGrant extends AbstractAuthorizeGrant
                 throw OAuthServerException::invalidRequest('code_challenge');
             }
 
+            if (preg_match("/^[A-Za-z0-9-._~]{43,128}$/", $codeChallenge) !== 1) {
+                throw OAuthServerException::invalidRequest(
+                    'code_challenge',
+                    'The code_challenge must be between 43 and 128 characters'
+                );
+            }
+
             $codeChallengeMethod = $this->getQueryStringParameter('code_challenge_method', $request, 'plain');
             if (in_array($codeChallengeMethod, ['plain', 'S256']) === false) {
                 throw OAuthServerException::invalidRequest(
diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php
index 498fdb4e..63c9042a 100644
--- a/tests/Grant/AuthCodeGrantTest.php
+++ b/tests/Grant/AuthCodeGrantTest.php
@@ -164,13 +164,124 @@ class AuthCodeGrantTest extends \PHPUnit_Framework_TestCase
                 'response_type'  => 'code',
                 'client_id'      => 'foo',
                 'redirect_uri'   => 'http://foo/bar',
-                'code_challenge' => 'FOOBAR',
+                'code_challenge' => str_repeat('A', 43),
             ]
         );
 
         $this->assertTrue($grant->validateAuthorizationRequest($request) instanceof AuthorizationRequest);
     }
 
+    /**
+     * @expectedException \League\OAuth2\Server\Exception\OAuthServerException
+     */
+    public function testValidateAuthorizationRequestCodeChallengeInvalidLengthTooShort()
+    {
+        $client = new ClientEntity();
+        $client->setRedirectUri('http://foo/bar');
+        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
+
+        $grant = new AuthCodeGrant(
+            $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
+            $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
+            new \DateInterval('PT10M')
+        );
+        $grant->enableCodeExchangeProof();
+        $grant->setClientRepository($clientRepositoryMock);
+
+        $request = new ServerRequest(
+            [],
+            [],
+            null,
+            null,
+            'php://input',
+            [],
+            [],
+            [
+                'response_type'  => 'code',
+                'client_id'      => 'foo',
+                'redirect_uri'   => 'http://foo/bar',
+                'code_challenge' => str_repeat('A', 42),
+            ]
+        );
+
+        $grant->validateAuthorizationRequest($request);
+    }
+
+    /**
+     * @expectedException \League\OAuth2\Server\Exception\OAuthServerException
+     */
+    public function testValidateAuthorizationRequestCodeChallengeInvalidLengthTooLong()
+    {
+        $client = new ClientEntity();
+        $client->setRedirectUri('http://foo/bar');
+        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
+
+        $grant = new AuthCodeGrant(
+            $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
+            $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
+            new \DateInterval('PT10M')
+        );
+        $grant->enableCodeExchangeProof();
+        $grant->setClientRepository($clientRepositoryMock);
+
+        $request = new ServerRequest(
+            [],
+            [],
+            null,
+            null,
+            'php://input',
+            [],
+            [],
+            [
+                'response_type'  => 'code',
+                'client_id'      => 'foo',
+                'redirect_uri'   => 'http://foo/bar',
+                'code_challenge' => str_repeat('A', 129),
+            ]
+        );
+
+        $grant->validateAuthorizationRequest($request);
+    }
+
+    /**
+     * @expectedException \League\OAuth2\Server\Exception\OAuthServerException
+     */
+    public function testValidateAuthorizationRequestCodeChallengeInvalidCharacters()
+    {
+        $client = new ClientEntity();
+        $client->setRedirectUri('http://foo/bar');
+        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
+
+        $grant = new AuthCodeGrant(
+            $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
+            $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
+            new \DateInterval('PT10M')
+        );
+        $grant->enableCodeExchangeProof();
+        $grant->setClientRepository($clientRepositoryMock);
+
+        $request = new ServerRequest(
+            [],
+            [],
+            null,
+            null,
+            'php://input',
+            [],
+            [],
+            [
+                'response_type'  => 'code',
+                'client_id'      => 'foo',
+                'redirect_uri'   => 'http://foo/bar',
+                'code_challenge' => str_repeat('A', 42) . '!',
+            ]
+        );
+
+        $grant->validateAuthorizationRequest($request);
+    }
+
     /**
      * @expectedException \League\OAuth2\Server\Exception\OAuthServerException
      * @expectedExceptionCode 3