diff --git a/.travis.yml b/.travis.yml index 85474626..2684cac3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,6 @@ language: php +dist: trusty sudo: false cache: diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index dc880365..d1669b2f 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -134,6 +134,15 @@ class AuthCodeGrant extends AbstractAuthorizeGrant throw OAuthServerException::invalidRequest('code_verifier'); } + // Validate code_verifier according to RFC-7636 + // @see: https://tools.ietf.org/html/rfc7636#section-4.1 + if (preg_match('/^[A-Za-z0-9-._~]{43,128}$/', $codeVerifier) !== 1) { + throw OAuthServerException::invalidRequest( + 'code_verifier', + 'Code Verifier must follow the specifications of RFC-7636.' + ); + } + switch ($authCodePayload->code_challenge_method) { case 'plain': if (hash_equals($codeVerifier, $authCodePayload->code_challenge) === false) { @@ -270,13 +279,6 @@ 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'], true) === false) { throw OAuthServerException::invalidRequest( @@ -285,6 +287,15 @@ class AuthCodeGrant extends AbstractAuthorizeGrant ); } + // Validate code_challenge according to RFC-7636 + // @see: https://tools.ietf.org/html/rfc7636#section-4.2 + if (preg_match('/^[A-Za-z0-9-._~]{43,128}$/', $codeChallenge) !== 1) { + throw OAuthServerException::invalidRequest( + 'code_challenged', + 'Code challenge must follow the specifications of RFC-7636.' + ); + } + $authorizationRequest->setCodeChallenge($codeChallenge); $authorizationRequest->setCodeChallengeMethod($codeChallengeMethod); } diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php index 97206a76..6a319234 100644 --- a/tests/Grant/AuthCodeGrantTest.php +++ b/tests/Grant/AuthCodeGrantTest.php @@ -34,6 +34,10 @@ class AuthCodeGrantTest extends TestCase */ protected $cryptStub; + const CODE_VERIFIER = 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk'; + + const CODE_CHALLENGE = 'E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM'; + public function setUp() { $this->cryptStub = new CryptTraitStub; @@ -185,7 +189,7 @@ class AuthCodeGrantTest extends TestCase 'response_type' => 'code', 'client_id' => 'foo', 'redirect_uri' => 'http://foo/bar', - 'code_challenge' => str_repeat('A', 43), + 'code_challenge' => self::CODE_CHALLENGE, ] ); @@ -686,7 +690,7 @@ class AuthCodeGrantTest extends TestCase 'grant_type' => 'authorization_code', 'client_id' => 'foo', 'redirect_uri' => 'http://foo/bar', - 'code_verifier' => 'foobar', + 'code_verifier' => self::CODE_VERIFIER, 'code' => $this->cryptStub->doEncrypt( json_encode( [ @@ -696,7 +700,7 @@ class AuthCodeGrantTest extends TestCase 'user_id' => 123, 'scopes' => ['foo'], 'redirect_uri' => 'http://foo/bar', - 'code_challenge' => 'foobar', + 'code_challenge' => self::CODE_VERIFIER, 'code_challenge_method' => 'plain', ] ) @@ -744,10 +748,6 @@ class AuthCodeGrantTest extends TestCase $grant->setRefreshTokenRepository($refreshTokenRepositoryMock); $grant->setEncryptionKey($this->cryptStub->getKey()); - // [RFC 7636] Appendix B. Example for the S256 code_challenge_method - $codeVerifier = 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk'; - $codeChallenge = 'E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM'; - $request = new ServerRequest( [], [], @@ -761,7 +761,7 @@ class AuthCodeGrantTest extends TestCase 'grant_type' => 'authorization_code', 'client_id' => 'foo', 'redirect_uri' => 'http://foo/bar', - 'code_verifier' => $codeVerifier, + 'code_verifier' => self::CODE_VERIFIER, 'code' => $this->cryptStub->doEncrypt( json_encode( [ @@ -771,7 +771,7 @@ class AuthCodeGrantTest extends TestCase 'user_id' => 123, 'scopes' => ['foo'], 'redirect_uri' => 'http://foo/bar', - 'code_challenge' => $codeChallenge, + 'code_challenge' => self::CODE_CHALLENGE, 'code_challenge_method' => 'S256', ] ) @@ -1204,7 +1204,7 @@ class AuthCodeGrantTest extends TestCase 'grant_type' => 'authorization_code', 'client_id' => 'foo', 'redirect_uri' => 'http://foo/bar', - 'code_verifier' => 'nope', + 'code_verifier' => self::CODE_VERIFIER, 'code' => $this->cryptStub->doEncrypt( json_encode( [ @@ -1298,7 +1298,151 @@ class AuthCodeGrantTest extends TestCase /* @var StubResponseType $response */ $grant->respondToAccessTokenRequest($request, new StubResponseType(), new \DateInterval('PT10M')); } catch (OAuthServerException $e) { - $this->assertEquals($e->getHint(), 'Failed to verify `code_verifier`.'); + $this->assertEquals($e->getHint(), 'Code Verifier must follow the specifications of RFC-7636.'); + } + } + + public function testRespondToAccessTokenRequestMalformedCodeVerifierS256WithInvalidChars() + { + $client = new ClientEntity(); + $client->setIdentifier('foo'); + $client->setRedirectUri('http://foo/bar'); + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeEntity = new ScopeEntity(); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity); + $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); + + $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); + $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); + $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); + + $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); + $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf(); + $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity()); + + $grant = new AuthCodeGrant( + $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), + $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(), + new \DateInterval('PT10M') + ); + $grant->enableCodeExchangeProof(); + $grant->setClientRepository($clientRepositoryMock); + $grant->setAccessTokenRepository($accessTokenRepositoryMock); + $grant->setRefreshTokenRepository($refreshTokenRepositoryMock); + $grant->setScopeRepository($scopeRepositoryMock); + $grant->setEncryptionKey($this->cryptStub->getKey()); + + $request = new ServerRequest( + [], + [], + null, + 'POST', + 'php://input', + [], + [], + [], + [ + 'grant_type' => 'authorization_code', + 'client_id' => 'foo', + 'redirect_uri' => 'http://foo/bar', + 'code_verifier' => 'dqX7C-RbqjHYtytmhGTigKdZCXfxq-+xbsk9_GxUcaE', // Malformed code. Contains `+`. + 'code' => $this->cryptStub->doEncrypt( + json_encode( + [ + 'auth_code_id' => uniqid(), + 'expire_time' => time() + 3600, + 'client_id' => 'foo', + 'user_id' => 123, + 'scopes' => ['foo'], + 'redirect_uri' => 'http://foo/bar', + 'code_challenge' => self::CODE_CHALLENGE, + 'code_challenge_method' => 'S256', + ] + ) + ), + ] + ); + + try { + /* @var StubResponseType $response */ + $grant->respondToAccessTokenRequest($request, new StubResponseType(), new \DateInterval('PT10M')); + } catch (OAuthServerException $e) { + $this->assertEquals($e->getHint(), 'Code Verifier must follow the specifications of RFC-7636.'); + } + } + + public function testRespondToAccessTokenRequestMalformedCodeVerifierS256WithInvalidLength() + { + $client = new ClientEntity(); + $client->setIdentifier('foo'); + $client->setRedirectUri('http://foo/bar'); + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeEntity = new ScopeEntity(); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn($scopeEntity); + $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); + + $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); + $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); + $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); + + $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); + $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf(); + $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity()); + + $grant = new AuthCodeGrant( + $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), + $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(), + new \DateInterval('PT10M') + ); + $grant->enableCodeExchangeProof(); + $grant->setClientRepository($clientRepositoryMock); + $grant->setAccessTokenRepository($accessTokenRepositoryMock); + $grant->setRefreshTokenRepository($refreshTokenRepositoryMock); + $grant->setScopeRepository($scopeRepositoryMock); + $grant->setEncryptionKey($this->cryptStub->getKey()); + + $request = new ServerRequest( + [], + [], + null, + 'POST', + 'php://input', + [], + [], + [], + [ + 'grant_type' => 'authorization_code', + 'client_id' => 'foo', + 'redirect_uri' => 'http://foo/bar', + 'code_verifier' => 'dqX7C-RbqjHY', // Malformed code. Invalid length. + 'code' => $this->cryptStub->doEncrypt( + json_encode( + [ + 'auth_code_id' => uniqid(), + 'expire_time' => time() + 3600, + 'client_id' => 'foo', + 'user_id' => 123, + 'scopes' => ['foo'], + 'redirect_uri' => 'http://foo/bar', + 'code_challenge' => 'R7T1y1HPNFvs1WDCrx4lfoBS6KD2c71pr8OHvULjvv8', + 'code_challenge_method' => 'S256', + ] + ) + ), + ] + ); + + try { + /* @var StubResponseType $response */ + $grant->respondToAccessTokenRequest($request, new StubResponseType(), new \DateInterval('PT10M')); + } catch (OAuthServerException $e) { + $this->assertEquals($e->getHint(), 'Code Verifier must follow the specifications of RFC-7636.'); } }