From fc53d636f5d434218941b4d0fafbd06bd6b33dbb Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 10:47:06 +0000 Subject: [PATCH 01/13] Updated getClientEntity now just requires the client ID and the grant type --- src/Repositories/ClientRepositoryInterface.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Repositories/ClientRepositoryInterface.php b/src/Repositories/ClientRepositoryInterface.php index 3bec9452..eb88a6cd 100644 --- a/src/Repositories/ClientRepositoryInterface.php +++ b/src/Repositories/ClientRepositoryInterface.php @@ -19,12 +19,10 @@ interface ClientRepositoryInterface extends RepositoryInterface /** * Get a client * - * @param string $grantType The grant type used * @param string $clientIdentifier The client's identifier - * @param string|null $clientSecret The client's secret - * @param string|null $redirectUri The client's redirect URI + * @param string $grantType The grant type used * * @return \League\OAuth2\Server\Entities\Interfaces\ClientEntityInterface */ - public function getClientEntity($grantType, $clientIdentifier, $clientSecret = null, $redirectUri = null); + public function getClientEntity($clientIdentifier, $grantType); } From 0d8cb0d06f7811a3572ce595bb53d74c8defb665 Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 10:47:30 +0000 Subject: [PATCH 02/13] Fixes for RefreshTokenGrant --- src/Grant/RefreshTokenGrant.php | 11 +++- tests/Grant/RefreshTokenGrantTest.php | 82 +++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 tests/Grant/RefreshTokenGrantTest.php diff --git a/src/Grant/RefreshTokenGrant.php b/src/Grant/RefreshTokenGrant.php index b43792ee..665d60a4 100644 --- a/src/Grant/RefreshTokenGrant.php +++ b/src/Grant/RefreshTokenGrant.php @@ -12,6 +12,7 @@ namespace League\OAuth2\Server\Grant; use League\Event\Event; +use League\OAuth2\Server\Entities\ScopeEntity; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface; @@ -47,13 +48,17 @@ class RefreshTokenGrant extends AbstractGrant \DateInterval $accessTokenTTL ) { // Validate request - $client = $this->validateClient($request); + $client = $this->validateClient($request); $oldRefreshToken = $this->validateOldRefreshToken($request, $client->getIdentifier()); - $scopes = $this->validateScopes($request, $client); + $scopes = $this->validateScopes($request, $client); // If no new scopes are requested then give the access token the original session scopes if (count($scopes) === 0) { - $scopes = $oldRefreshToken['scopes']; + $scopes = array_map(function ($scopeId) { + $scope = new ScopeEntity(); + $scope->setIdentifier($scopeId); + return $scope; + }, $oldRefreshToken['scopes']); } else { // The OAuth spec says that a refreshed access token can have the original scopes or fewer so ensure // the request doesn't include any new scopes diff --git a/tests/Grant/RefreshTokenGrantTest.php b/tests/Grant/RefreshTokenGrantTest.php new file mode 100644 index 00000000..dc28b51f --- /dev/null +++ b/tests/Grant/RefreshTokenGrantTest.php @@ -0,0 +1,82 @@ +getMock(RefreshTokenRepositoryInterface::class); + + $grant = new RefreshTokenGrant($refreshTokenRepositoryMock); + $this->assertEquals('refresh_token', $grant->getIdentifier()); + } + + public function testRespondToRequest() + { + $client = new ClientEntity(); + $client->setIdentifier('foo'); + $client->setSecret('bar'); + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); + $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); + + $userRepositoryMock = $this->getMockBuilder(UserRepositoryInterface::class)->getMock(); + $userEntity = new UserEntity(); + $userRepositoryMock->method('getUserEntityByUserCredentials')->willReturn($userEntity); + + $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); + + $grant = new RefreshTokenGrant($refreshTokenRepositoryMock); + $grant->setClientRepository($clientRepositoryMock); + $grant->setAccessTokenRepository($accessTokenRepositoryMock); + $grant->setPathToPublicKey('file://'.__DIR__.'/../Utils/public.key'); + $grant->setPathToPrivateKey('file://'.__DIR__.'/../Utils/private.key'); + + $oldRefreshToken = KeyCrypt::encrypt( + json_encode( + [ + 'client_id' => 'foo', + 'refresh_token_id' => 'zyxwvu', + 'access_token_id' => 'abcdef', + 'scopes' => ['foo'], + 'user_id' => 123, + 'expire_time' => time() + 3600, + ] + ), + 'file://'.__DIR__.'/../Utils/private.key' + ); + + $serverRequest = new ServerRequest(); + $serverRequest = $serverRequest->withParsedBody( + [ + 'client_id' => 'foo', + 'client_secret' => 'bar', + 'refresh_token' => $oldRefreshToken, + ] + ); + + $responseType = new StubResponseType(); + $grant->respondToRequest($serverRequest, $responseType, new \DateInterval('PT5M')); + + $this->assertTrue($responseType->getAccessToken() instanceof AccessTokenEntityInterface); + $this->assertTrue($responseType->getRefreshToken() instanceof RefreshTokenEntityInterface); + } +} From fb77a78fb37c9e3af8ad47b0ee567b368887a9c4 Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 10:47:52 +0000 Subject: [PATCH 03/13] Added Password Grant test --- tests/Grant/PasswordGrantTest.php | 64 +++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 tests/Grant/PasswordGrantTest.php diff --git a/tests/Grant/PasswordGrantTest.php b/tests/Grant/PasswordGrantTest.php new file mode 100644 index 00000000..8dd7a48d --- /dev/null +++ b/tests/Grant/PasswordGrantTest.php @@ -0,0 +1,64 @@ +getMock(UserRepositoryInterface::class); + $refreshTokenRepositoryMock = $this->getMock(RefreshTokenRepositoryInterface::class); + + $grant = new PasswordGrant($userRepositoryMock, $refreshTokenRepositoryMock); + $this->assertEquals('password', $grant->getIdentifier()); + } + + public function testRespondToRequest() + { + $client = new ClientEntity(); + $client->setSecret('bar'); + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); + $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); + + $userRepositoryMock = $this->getMockBuilder(UserRepositoryInterface::class)->getMock(); + $userEntity = new UserEntity(); + $userRepositoryMock->method('getUserEntityByUserCredentials')->willReturn($userEntity); + + $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); + + $grant = new PasswordGrant($userRepositoryMock, $refreshTokenRepositoryMock); + $grant->setClientRepository($clientRepositoryMock); + $grant->setAccessTokenRepository($accessTokenRepositoryMock); + + $serverRequest = new ServerRequest(); + $serverRequest = $serverRequest->withParsedBody( + [ + 'client_id' => 'foo', + 'client_secret' => 'bar', + 'username' => 'foo', + 'password' => 'bar', + ] + ); + + $responseType = new StubResponseType(); + $grant->respondToRequest($serverRequest, $responseType, new \DateInterval('PT5M')); + + $this->assertTrue($responseType->getAccessToken() instanceof AccessTokenEntityInterface); + $this->assertTrue($responseType->getRefreshToken() instanceof RefreshTokenEntityInterface); + } +} From e808528cc854a436b5523ef854a05cb78f412c97 Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 10:47:59 +0000 Subject: [PATCH 04/13] Added test stubs --- tests/Stubs/StubResponseType.php | 60 ++++++++++++++++++++++++++++++++ tests/Stubs/UserEntity.php | 13 +++++++ 2 files changed, 73 insertions(+) create mode 100644 tests/Stubs/StubResponseType.php create mode 100644 tests/Stubs/UserEntity.php diff --git a/tests/Stubs/StubResponseType.php b/tests/Stubs/StubResponseType.php new file mode 100644 index 00000000..4d40b089 --- /dev/null +++ b/tests/Stubs/StubResponseType.php @@ -0,0 +1,60 @@ +accessToken; + } + + public function getRefreshToken() + { + return $this->refreshToken; + } + + /** + * @param \League\OAuth2\Server\Entities\Interfaces\AccessTokenEntityInterface $accessToken + */ + public function setAccessToken(AccessTokenEntityInterface $accessToken) + { + $this->accessToken = $accessToken; + } + + /** + * @param \League\OAuth2\Server\Entities\Interfaces\RefreshTokenEntityInterface $refreshToken + */ + public function setRefreshToken(RefreshTokenEntityInterface $refreshToken) + { + $this->refreshToken = $refreshToken; + } + + /** + * @param ServerRequestInterface $request + * + * @return ServerRequestInterface + */ + public function determineAccessTokenInHeader(ServerRequestInterface $request) + { + // TODO: Implement determineAccessTokenInHeader() method. + } + + /** + * @param ResponseInterface $response + * + * @return ResponseInterface + */ + public function generateHttpResponse(ResponseInterface $response) + { + // TODO: Implement generateHttpResponse() method. + } +} \ No newline at end of file diff --git a/tests/Stubs/UserEntity.php b/tests/Stubs/UserEntity.php new file mode 100644 index 00000000..4c157df5 --- /dev/null +++ b/tests/Stubs/UserEntity.php @@ -0,0 +1,13 @@ + Date: Thu, 18 Feb 2016 10:48:12 +0000 Subject: [PATCH 05/13] Updated ClientEntityInterface with additional methods --- .../Interfaces/ClientEntityInterface.php | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/Entities/Interfaces/ClientEntityInterface.php b/src/Entities/Interfaces/ClientEntityInterface.php index c0d0e812..2761c214 100644 --- a/src/Entities/Interfaces/ClientEntityInterface.php +++ b/src/Entities/Interfaces/ClientEntityInterface.php @@ -11,6 +11,7 @@ interface ClientEntityInterface /** * Set the client's identifier + * * @param $identifier */ public function setIdentifier($identifier); @@ -23,7 +24,42 @@ interface ClientEntityInterface /** * Set the client's name + * * @param string $name */ public function setName($name); + + /** + * @param string $secret + */ + public function setSecret($secret); + + /** + * Validate the secret provided by the client + * + * @param string $submittedSecret + * + * @return boolean + */ + public function validateSecret($submittedSecret); + + /** + * Set the client's redirect uri + * + * @param string $redirectUri + */ + public function setRedirectUri($redirectUri); + + /** + * Returns the registered redirect URI + * + * @return string + */ + public function getRedirectUri(); + + /** + * Returns true if the client is capable of keeping it's secrets secret + * @return boolean + */ + public function canKeepASecret(); } From 7f67000d531758129ce77e7d5beacc4fd4872224 Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 10:48:23 +0000 Subject: [PATCH 06/13] Provided implementation of new client entity methods --- src/Entities/Traits/ClientEntityTrait.php | 56 +++++++++++++++++++++-- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/src/Entities/Traits/ClientEntityTrait.php b/src/Entities/Traits/ClientEntityTrait.php index 80e61b93..d7665fcf 100644 --- a/src/Entities/Traits/ClientEntityTrait.php +++ b/src/Entities/Traits/ClientEntityTrait.php @@ -9,8 +9,17 @@ trait ClientEntityTrait protected $name; /** - * Get the client's name - * @return string + * @var string + */ + protected $secret; + + /** + * @var string + */ + protected $redirectUri; + + /** + * @inheritdoc */ public function getName() { @@ -18,11 +27,50 @@ trait ClientEntityTrait } /** - * Set the client's name - * @param string $name + * @inheritdoc */ public function setName($name) { $this->name = $name; } + + /** + * @inheritdoc + */ + public function canKeepASecret() + { + return $this->secret === null; + } + + /** + * @inheritdoc + */ + public function setSecret($secret) + { + $this->secret = $secret; + } + + /** + * @inheritdoc + */ + public function validateSecret($submittedSecret) + { + return strcmp((string) $submittedSecret, $this->secret) === 0; + } + + /** + * @inheritdoc + */ + public function setRedirectUri($redirectUri) + { + $this->redirectUri = $redirectUri; + } + + /** + * @inheritdoc + */ + public function getRedirectUri() + { + return $this->redirectUri; + } } From 3b36ae9000328ccba408b20752021cfbc51e7a6d Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 10:49:05 +0000 Subject: [PATCH 07/13] Rewrote validateClient method to progressively test client secret and redirect URI --- src/Grant/AbstractGrant.php | 41 ++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 5722357f..2139e6fa 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -130,17 +130,12 @@ abstract class AbstractGrant implements GrantTypeInterface * Validate the client * * @param \Psr\Http\Message\ServerRequestInterface $request - * @param bool $validateSecret - * @param bool $validateRedirectUri * * @return \League\OAuth2\Server\Entities\Interfaces\ClientEntityInterface * @throws \League\OAuth2\Server\Exception\OAuthServerException */ - protected function validateClient( - ServerRequestInterface $request, - $validateSecret = true, - $validateRedirectUri = false - ) { + protected function validateClient(ServerRequestInterface $request) + { $clientId = $this->getRequestParameter( 'client_id', $request, @@ -150,30 +145,34 @@ abstract class AbstractGrant implements GrantTypeInterface throw OAuthServerException::invalidRequest('client_id', null, '`%s` parameter is missing'); } + $client = $this->clientRepository->getClientEntity( + $clientId, + $this->getIdentifier() + ); + + if (!$client instanceof ClientEntityInterface) { + throw OAuthServerException::invalidClient(); + } + + // If the client is confidential require the client secret $clientSecret = $this->getRequestParameter( 'client_secret', $request, $this->getServerParameter('PHP_AUTH_PW', $request) ); - if (is_null($clientSecret) && $validateSecret === true) { + + if ($client->canKeepASecret() && is_null($clientSecret)) { throw OAuthServerException::invalidRequest('client_secret', null, '`%s` parameter is missing'); } - $redirectUri = $this->getRequestParameter('redirect_uri', $request, null); - if (is_null($redirectUri) && $validateRedirectUri === true) { - throw OAuthServerException::invalidRequest('redirect_uri', null, '`%s` parameter is missing'); + if ($client->canKeepASecret() && $client->validateSecret($clientSecret) === false) { + $this->getEmitter()->emit(new Event('client.authentication.failed', $request)); + throw OAuthServerException::invalidClient(); } - $client = $this->clientRepository->getClientEntity( - $clientId, - $clientSecret, - $redirectUri, - $this->getIdentifier() - ); - - if (!$client instanceof ClientEntityInterface) { - $this->getEmitter()->emit(new Event('client.authentication.failed', $request)); - + // If a redirect URI is provided ensure it matches what is pre-registered + $redirectUri = $this->getRequestParameter('redirect_uri', $request, null); + if ($redirectUri !== null && (strcmp($client->getRedirectUri(), $redirectUri) !== 0)) { throw OAuthServerException::invalidClient(); } From 73cd377c4bc127ffc17918aa90fa75859c8f81f2 Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 10:49:13 +0000 Subject: [PATCH 08/13] Added client credentials grant test --- tests/Grant/ClientCredentialsGrantTest.php | 48 ++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 tests/Grant/ClientCredentialsGrantTest.php diff --git a/tests/Grant/ClientCredentialsGrantTest.php b/tests/Grant/ClientCredentialsGrantTest.php new file mode 100644 index 00000000..52e51c9a --- /dev/null +++ b/tests/Grant/ClientCredentialsGrantTest.php @@ -0,0 +1,48 @@ +assertEquals('client_credentials', $grant->getIdentifier()); + } + + public function testRespondToRequest() + { + $client = new ClientEntity(); + $client->setSecret('bar'); + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); + $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); + + $grant = new ClientCredentialsGrant(); + $grant->setClientRepository($clientRepositoryMock); + $grant->setAccessTokenRepository($accessTokenRepositoryMock); + + $serverRequest = new ServerRequest(); + $serverRequest = $serverRequest->withParsedBody( + [ + 'client_id' => 'foo', + 'client_secret' => 'bar', + ] + ); + + $responseType = new StubResponseType(); + $grant->respondToRequest($serverRequest, $responseType, new \DateInterval('PT5M')); + + $this->assertTrue($responseType->getAccessToken() instanceof AccessTokenEntityInterface); + } +} From 704e1145685070f778093e1111ccc77dcc77cef1 Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 10:49:39 +0000 Subject: [PATCH 09/13] Updated AuthCodeGrant --- src/Grant/AuthCodeGrant.php | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 560b8ad1..cc0a1f81 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -89,26 +89,7 @@ class AuthCodeGrant extends AbstractGrant protected function respondToAuthorizationRequest( ServerRequestInterface $request ) { - $clientId = $this->getQueryStringParameter( - 'client_id', - $request, - $this->getServerParameter('PHP_AUTH_USER', $request) - ); - if (is_null($clientId)) { - throw OAuthServerException::invalidRequest('client_id', null, '`%s` parameter is missing'); - } - - $redirectUri = $this->getQueryStringParameter('redirect_uri', $request, null); - if (is_null($redirectUri)) { - throw OAuthServerException::invalidRequest('redirect_uri', null, '`%s` parameter is missing'); - } - - $client = $this->clientRepository->getClientEntity( - $clientId, - null, - $redirectUri, - $this->getIdentifier() - ); + $client = $this->validateClient($request); if ($client instanceof ClientEntityInterface === false) { $this->emitter->emit(new Event('client.authentication.failed', $request)); @@ -116,7 +97,7 @@ class AuthCodeGrant extends AbstractGrant throw OAuthServerException::invalidClient(); } - $scopes = $this->validateScopes($request, $client, $redirectUri); + $scopes = $this->validateScopes($request, $client, $client->getRedirectUri()); $queryString = http_build_query($request->getQueryParams()); $postbackUri = new Uri( sprintf( @@ -168,8 +149,9 @@ class AuthCodeGrant extends AbstractGrant // The user hasn't logged in yet so show a login form if ($userId === null) { $engine = new Engine(dirname($this->pathToLoginTemplate)); + $pathParts = explode(DIRECTORY_SEPARATOR, $this->pathToLoginTemplate); $html = $engine->render( - 'login_user', + end($pathParts), [ 'error' => $loginError, 'postback_uri' => (string) $postbackUri->withQuery($queryString), @@ -183,8 +165,9 @@ class AuthCodeGrant extends AbstractGrant // The user hasn't approved the client yet so show an authorize form if ($userId !== null && $userHasApprovedClient === null) { $engine = new Engine(dirname($this->pathToAuthorizeTemplate)); + $pathParts = explode(DIRECTORY_SEPARATOR, $this->pathToAuthorizeTemplate); $html = $engine->render( - 'authorize_client', + end($pathParts), [ 'client' => $client, 'scopes' => $scopes, @@ -212,7 +195,7 @@ class AuthCodeGrant extends AbstractGrant $stateParameter = $this->getQueryStringParameter('state', $request); - $redirectUri = new Uri($redirectUri); + $redirectUri = new Uri($client->getRedirectUri()); parse_str($redirectUri->getQuery(), $redirectPayload); if ($stateParameter !== null) { $redirectPayload['state'] = $stateParameter; @@ -263,6 +246,12 @@ class AuthCodeGrant extends AbstractGrant ResponseTypeInterface $responseType, DateInterval $accessTokenTTL ) { + // The redirect URI is required in this request + $redirectUri = $this->getQueryStringParameter('redirect_uri', $request, null); + if (is_null($redirectUri)) { + throw OAuthServerException::invalidRequest('redirect_uri', null, '`%s` parameter is missing'); + } + // Validate request $client = $this->validateClient($request); $encryptedAuthCode = $this->getRequestParameter('code', $request, null); From ad5b242d10b1718e4c312b6fcbb4cef27e86a45f Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 11:36:20 +0000 Subject: [PATCH 10/13] Updated AbstractGrantTest --- tests/Grant/AbstractGrantTest.php | 105 ++++++++++++++++++++++++++---- 1 file changed, 91 insertions(+), 14 deletions(-) diff --git a/tests/Grant/AbstractGrantTest.php b/tests/Grant/AbstractGrantTest.php index 30e0079c..92acf384 100644 --- a/tests/Grant/AbstractGrantTest.php +++ b/tests/Grant/AbstractGrantTest.php @@ -10,7 +10,6 @@ use League\OAuth2\Server\Entities\Interfaces\AuthCodeEntityInterface; use League\OAuth2\Server\Entities\Interfaces\RefreshTokenEntityInterface; use League\OAuth2\Server\Entities\ScopeEntity; use League\OAuth2\Server\Grant\AbstractGrant; -use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ClientRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; use Zend\Diactoros\ServerRequest; @@ -19,24 +18,45 @@ class AbstractGrantTest extends \PHPUnit_Framework_TestCase { public function testGetSet() { - $clientRepositoryMock = $this->getMock(ClientRepositoryInterface::class); - $accessTokenRepositoryMock = $this->getMock(AccessTokenRepositoryInterface::class); - $scopeRepositoryMock = $this->getMock(ScopeRepositoryInterface::class); + /** @var AbstractGrant $grantMock */ + $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); + $grantMock->setPathToPrivateKey('./private.key'); + $grantMock->setPathToPublicKey('./public.key'); + $grantMock->setEmitter(new Emitter()); + } + + public function testValidateClientPublic() + { + $client = new ClientEntity(); + + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + $clientRepositoryMock->method('getClientEntity')->willReturn($client); /** @var AbstractGrant $grantMock */ $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); $grantMock->setClientRepository($clientRepositoryMock); - $grantMock->setAccessTokenRepository($accessTokenRepositoryMock); - $grantMock->setScopeRepository($scopeRepositoryMock); - $grantMock->setPathToPrivateKey('./private.key'); - $grantMock->setPathToPublicKey('./public.key'); - $grantMock->setEmitter(new Emitter()); - $grantMock->setRefreshTokenTTL(new \DateInterval('PT1H')); + + $abstractGrantReflection = new \ReflectionClass($grantMock); + + $serverRequest = new ServerRequest(); + $serverRequest = $serverRequest->withParsedBody( + [ + 'client_id' => 'foo', + ] + ); + $validateClientMethod = $abstractGrantReflection->getMethod('validateClient'); + $validateClientMethod->setAccessible(true); + + $result = $validateClientMethod->invoke($grantMock, $serverRequest, true, true); + $this->assertEquals($client, $result); } - public function testValidateClient() + public function testValidateClientConfidential() { $client = new ClientEntity(); + $client->setSecret('bar'); + $client->setRedirectUri('http://foo/bar'); + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); $clientRepositoryMock->method('getClientEntity')->willReturn($client); @@ -89,6 +109,7 @@ class AbstractGrantTest extends \PHPUnit_Framework_TestCase public function testValidateClientMissingClientSecret() { $client = new ClientEntity(); + $client->setSecret('bar'); $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); $clientRepositoryMock->method('getClientEntity')->willReturn($client); @@ -112,11 +133,67 @@ class AbstractGrantTest extends \PHPUnit_Framework_TestCase /** * @expectedException \League\OAuth2\Server\Exception\OAuthServerException */ - public function testValidateClientMissingRedirectUri() + public function testValidateClientInvalidClientSecret() + { + $client = new ClientEntity(); + $client->setSecret('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(); + $serverRequest = $serverRequest->withParsedBody([ + 'client_id' => 'foo', + 'client_secret' => 'foo', + ]); + + $validateClientMethod = $abstractGrantReflection->getMethod('validateClient'); + $validateClientMethod->setAccessible(true); + + $validateClientMethod->invoke($grantMock, $serverRequest, true, true); + } + + /** + * @expectedException \League\OAuth2\Server\Exception\OAuthServerException + */ + public function testValidateClientInvalidRedirectUri() + { + $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(); + $serverRequest = $serverRequest->withParsedBody([ + 'client_id' => 'foo', + 'redirect_uri' => 'http://bar/foo' + ]); + + $validateClientMethod = $abstractGrantReflection->getMethod('validateClient'); + $validateClientMethod->setAccessible(true); + + $validateClientMethod->invoke($grantMock, $serverRequest, true, true); + } + + /** + * @expectedException \League\OAuth2\Server\Exception\OAuthServerException + */ + public function testValidateClientBadClient() { $client = new ClientEntity(); $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); - $clientRepositoryMock->method('getClientEntity')->willReturn($client); + $clientRepositoryMock->method('getClientEntity')->willReturn(null); /** @var AbstractGrant $grantMock */ $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); @@ -133,7 +210,7 @@ class AbstractGrantTest extends \PHPUnit_Framework_TestCase $validateClientMethod = $abstractGrantReflection->getMethod('validateClient'); $validateClientMethod->setAccessible(true); - $validateClientMethod->invoke($grantMock, $serverRequest, true, true); + $validateClientMethod->invoke($grantMock, $serverRequest, true); } public function testCanRespondToRequest() From 064eb85f4e7c697f2d565af710399317e81441d5 Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 12:07:23 +0000 Subject: [PATCH 11/13] AbstractGrant now handles persisting tokens --- src/Grant/AbstractGrant.php | 51 ++++++++++++++++++++++++++++ src/Grant/AuthCodeGrant.php | 17 ++-------- src/Grant/ClientCredentialsGrant.php | 1 - src/Grant/PasswordGrant.php | 9 +---- src/Grant/RefreshTokenGrant.php | 13 +++---- 5 files changed, 59 insertions(+), 32 deletions(-) diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 2139e6fa..142e930d 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -21,9 +21,12 @@ use League\OAuth2\Server\Entities\RefreshTokenEntity; use League\OAuth2\Server\Entities\ScopeEntity; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; +use League\OAuth2\Server\Repositories\AuthCodeRepositoryInterface; use League\OAuth2\Server\Repositories\ClientRepositoryInterface; +use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; use League\OAuth2\Server\Utils\SecureKey; +use OAuth2ServerExamples\Repositories\AuthCodeRepository; use Psr\Http\Message\ServerRequestInterface; /** @@ -55,6 +58,16 @@ abstract class AbstractGrant implements GrantTypeInterface */ protected $scopeRepository; + /** + * @var \League\OAuth2\Server\Repositories\AuthCodeRepositoryInterface + */ + private $authCodeRepository; + + /** + * @var \League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface + */ + private $refreshTokenRepository; + /** * @var string */ @@ -94,6 +107,22 @@ abstract class AbstractGrant implements GrantTypeInterface $this->scopeRepository = $scopeRepository; } + /** + * @param \League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface $refreshTokenRepository + */ + public function setRefreshTokenRepository(RefreshTokenRepositoryInterface $refreshTokenRepository) + { + $this->refreshTokenRepository = $refreshTokenRepository; + } + + /** + * @param \League\OAuth2\Server\Repositories\AuthCodeRepositoryInterface $authCodeRepository + */ + public function setAuthCodeRepository(AuthCodeRepositoryInterface $authCodeRepository) + { + $this->authCodeRepository = $authCodeRepository; + } + /** * @param string $pathToPrivateKey */ @@ -126,6 +155,22 @@ abstract class AbstractGrant implements GrantTypeInterface $this->refreshTokenTTL = $refreshTokenTTL; } + /** + * @return AuthCodeRepositoryInterface + */ + protected function getAuthCodeRepository() + { + return $this->authCodeRepository; + } + + /** + * @return RefreshTokenRepositoryInterface + */ + protected function getRefreshTokenRepository() + { + return $this->refreshTokenRepository; + } + /** * Validate the client * @@ -303,6 +348,8 @@ abstract class AbstractGrant implements GrantTypeInterface $accessToken->addScope($scope); } + $this->accessTokenRepository->persistNewAccessToken($accessToken); + return $accessToken; } @@ -336,6 +383,8 @@ abstract class AbstractGrant implements GrantTypeInterface $authCode->addScope($scope); } + $this->authCodeRepository->persistNewAuthCode($authCode); + return $authCode; } @@ -351,6 +400,8 @@ abstract class AbstractGrant implements GrantTypeInterface $refreshToken->setExpiryDateTime((new \DateTime())->add($this->refreshTokenTTL)); $refreshToken->setAccessToken($accessToken); + $this->refreshTokenRepository->persistNewRefreshToken($refreshToken); + return $refreshToken; } diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index cc0a1f81..13b8a984 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -23,10 +23,6 @@ class AuthCodeGrant extends AbstractGrant * @var \DateInterval */ private $authCodeTTL; - /** - * @var \League\OAuth2\Server\Repositories\AuthCodeRepositoryInterface - */ - private $authCodeRepository; /** * @var \League\OAuth2\Server\Repositories\UserRepositoryInterface @@ -43,10 +39,6 @@ class AuthCodeGrant extends AbstractGrant */ private $pathToAuthorizeTemplate; - /** - * @var \League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface - */ - private $refreshTokenRepository; /** * @param \League\OAuth2\Server\Repositories\AuthCodeRepositoryInterface $authCodeRepository @@ -64,8 +56,8 @@ class AuthCodeGrant extends AbstractGrant $pathToLoginTemplate = null, $pathToAuthorizeTemplate = null ) { - $this->authCodeRepository = $authCodeRepository; - $this->refreshTokenRepository = $refreshTokenRepository; + $this->setAuthCodeRepository($authCodeRepository); + $this->setRefreshTokenRepository($refreshTokenRepository); $this->userRepository = $userRepository; $this->authCodeTTL = $authCodeTTL; $this->pathToLoginTemplate = ($pathToLoginTemplate === null) @@ -209,7 +201,6 @@ class AuthCodeGrant extends AbstractGrant $redirectUri, $scopes ); - $this->authCodeRepository->persistNewAuthCode($authCode); $redirectPayload['code'] = KeyCrypt::encrypt( json_encode( @@ -267,7 +258,7 @@ class AuthCodeGrant extends AbstractGrant throw OAuthServerException::invalidRequest('code', 'Authorization code has expired'); } - if ($this->authCodeRepository->isAuthCodeRevoked($authCodePayload->auth_code_id) === true) { + if ($this->getAuthCodeRepository()->isAuthCodeRevoked($authCodePayload->auth_code_id) === true) { throw OAuthServerException::invalidRequest('code', 'Authorization code has been revoked'); } @@ -286,8 +277,6 @@ class AuthCodeGrant extends AbstractGrant $authCodePayload->scopes ); $refreshToken = $this->issueRefreshToken($accessToken); - $this->accessTokenRepository->persistNewAccessToken($accessToken); - $this->refreshTokenRepository->persistNewRefreshToken($refreshToken); // Inject tokens into response type $responseType->setAccessToken($accessToken); diff --git a/src/Grant/ClientCredentialsGrant.php b/src/Grant/ClientCredentialsGrant.php index cf6ce268..f5881e12 100644 --- a/src/Grant/ClientCredentialsGrant.php +++ b/src/Grant/ClientCredentialsGrant.php @@ -33,7 +33,6 @@ class ClientCredentialsGrant extends AbstractGrant // Issue and persist access token $accessToken = $this->issueAccessToken($accessTokenTTL, $client, $client->getIdentifier(), $scopes); - $this->accessTokenRepository->persistNewAccessToken($accessToken); // Inject access token into response type $responseType->setAccessToken($accessToken); diff --git a/src/Grant/PasswordGrant.php b/src/Grant/PasswordGrant.php index 00ffa149..0b30bfe2 100644 --- a/src/Grant/PasswordGrant.php +++ b/src/Grant/PasswordGrant.php @@ -29,11 +29,6 @@ class PasswordGrant extends AbstractGrant */ private $userRepository; - /** - * @var \League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface - */ - private $refreshTokenRepository; - /** * @param \League\OAuth2\Server\Repositories\UserRepositoryInterface $userRepository * @param \League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface $refreshTokenRepository @@ -43,7 +38,7 @@ class PasswordGrant extends AbstractGrant RefreshTokenRepositoryInterface $refreshTokenRepository ) { $this->userRepository = $userRepository; - $this->refreshTokenRepository = $refreshTokenRepository; + $this->setRefreshTokenRepository($refreshTokenRepository); $this->refreshTokenTTL = new \DateInterval('P1M'); } @@ -64,8 +59,6 @@ class PasswordGrant extends AbstractGrant // Issue and persist new tokens $accessToken = $this->issueAccessToken($accessTokenTTL, $client, $user->getIdentifier(), $scopes); $refreshToken = $this->issueRefreshToken($accessToken); - $this->accessTokenRepository->persistNewAccessToken($accessToken); - $this->refreshTokenRepository->persistNewRefreshToken($refreshToken); // Inject tokens into response $responseType->setAccessToken($accessToken); diff --git a/src/Grant/RefreshTokenGrant.php b/src/Grant/RefreshTokenGrant.php index 665d60a4..adaa2306 100644 --- a/src/Grant/RefreshTokenGrant.php +++ b/src/Grant/RefreshTokenGrant.php @@ -24,17 +24,12 @@ use Psr\Http\Message\ServerRequestInterface; */ class RefreshTokenGrant extends AbstractGrant { - /** - * @var \League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface - */ - private $refreshTokenRepository; - /** * @param \League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface $refreshTokenRepository */ public function __construct(RefreshTokenRepositoryInterface $refreshTokenRepository) { - $this->refreshTokenRepository = $refreshTokenRepository; + $this->setRefreshTokenRepository($refreshTokenRepository); $this->refreshTokenTTL = new \DateInterval('P1M'); } @@ -73,13 +68,13 @@ class RefreshTokenGrant extends AbstractGrant // Expire old tokens $this->accessTokenRepository->revokeAccessToken($oldRefreshToken['access_token_id']); - $this->refreshTokenRepository->revokeRefreshToken($oldRefreshToken['refresh_token_id']); + $this->getRefreshTokenRepository()->revokeRefreshToken($oldRefreshToken['refresh_token_id']); // Issue and persist new tokens $accessToken = $this->issueAccessToken($accessTokenTTL, $client, $oldRefreshToken['user_id'], $scopes); $refreshToken = $this->issueRefreshToken($accessToken); $this->accessTokenRepository->persistNewAccessToken($accessToken); - $this->refreshTokenRepository->persistNewRefreshToken($refreshToken); + $this->getRefreshTokenRepository()->persistNewRefreshToken($refreshToken); // Inject tokens into response $responseType->setAccessToken($accessToken); @@ -125,7 +120,7 @@ class RefreshTokenGrant extends AbstractGrant throw OAuthServerException::invalidRefreshToken('Token has expired'); } - if ($this->refreshTokenRepository->isRefreshTokenRevoked($refreshTokenData['refresh_token_id']) === true) { + if ($this->getRefreshTokenRepository()->isRefreshTokenRevoked($refreshTokenData['refresh_token_id']) === true) { throw OAuthServerException::invalidRefreshToken('Token has been revoked'); } From e8a01c3bcd3e5ff62bcd4db4e1ea09a910f3cd89 Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 12:07:36 +0000 Subject: [PATCH 12/13] Fix for logic --- src/Entities/Traits/ClientEntityTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Entities/Traits/ClientEntityTrait.php b/src/Entities/Traits/ClientEntityTrait.php index d7665fcf..bac3ccda 100644 --- a/src/Entities/Traits/ClientEntityTrait.php +++ b/src/Entities/Traits/ClientEntityTrait.php @@ -39,7 +39,7 @@ trait ClientEntityTrait */ public function canKeepASecret() { - return $this->secret === null; + return $this->secret !== null; } /** From 13baa0bb26ff8a70b49ea655d21b26f0e6d2b3de Mon Sep 17 00:00:00 2001 From: Alex Bilbie Date: Thu, 18 Feb 2016 12:07:50 +0000 Subject: [PATCH 13/13] Updated tests --- tests/Grant/AbstractGrantTest.php | 13 ++++++- tests/Grant/PasswordGrantTest.php | 1 + tests/Grant/RefreshTokenGrantTest.php | 1 + tests/ServerTest.php | 56 +++++++++++++++++++++++++++ tests/Stubs/StubResponseType.php | 3 +- 5 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 tests/ServerTest.php diff --git a/tests/Grant/AbstractGrantTest.php b/tests/Grant/AbstractGrantTest.php index 92acf384..cb19bf52 100644 --- a/tests/Grant/AbstractGrantTest.php +++ b/tests/Grant/AbstractGrantTest.php @@ -12,6 +12,9 @@ use League\OAuth2\Server\Entities\ScopeEntity; use League\OAuth2\Server\Grant\AbstractGrant; use League\OAuth2\Server\Repositories\ClientRepositoryInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; +use League\OAuth2\Server\Repositories\RefreshTokenRepositoryInterface; +use League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface; +use League\OAuth2\Server\Repositories\AuthCodeRepositoryInterface; use Zend\Diactoros\ServerRequest; class AbstractGrantTest extends \PHPUnit_Framework_TestCase @@ -191,7 +194,6 @@ class AbstractGrantTest extends \PHPUnit_Framework_TestCase */ public function testValidateClientBadClient() { - $client = new ClientEntity(); $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); $clientRepositoryMock->method('getClientEntity')->willReturn(null); @@ -228,9 +230,12 @@ class AbstractGrantTest extends \PHPUnit_Framework_TestCase public function testIssueRefreshToken() { + $refreshTokenRepoMock = $this->getMock(RefreshTokenRepositoryInterface::class); + /** @var AbstractGrant $grantMock */ $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); $grantMock->setRefreshTokenTTL(new \DateInterval('PT1M')); + $grantMock->setRefreshTokenRepository($refreshTokenRepoMock); $abstractGrantReflection = new \ReflectionClass($grantMock); $issueRefreshTokenMethod = $abstractGrantReflection->getMethod('issueRefreshToken'); @@ -246,8 +251,11 @@ class AbstractGrantTest extends \PHPUnit_Framework_TestCase public function testIssueAccessToken() { + $accessTokenRepoMock = $this->getMock(AccessTokenRepositoryInterface::class); + /** @var AbstractGrant $grantMock */ $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); + $grantMock->setAccessTokenRepository($accessTokenRepoMock); $abstractGrantReflection = new \ReflectionClass($grantMock); $issueAccessTokenMethod = $abstractGrantReflection->getMethod('issueAccessToken'); @@ -267,8 +275,11 @@ class AbstractGrantTest extends \PHPUnit_Framework_TestCase public function testIssueAuthCode() { + $authCodeRepoMock = $this->getMock(AuthCodeRepositoryInterface::class); + /** @var AbstractGrant $grantMock */ $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); + $grantMock->setAuthCodeRepository($authCodeRepoMock); $abstractGrantReflection = new \ReflectionClass($grantMock); $issueAuthCodeMethod = $abstractGrantReflection->getMethod('issueAuthCode'); diff --git a/tests/Grant/PasswordGrantTest.php b/tests/Grant/PasswordGrantTest.php index 8dd7a48d..c81e4c61 100644 --- a/tests/Grant/PasswordGrantTest.php +++ b/tests/Grant/PasswordGrantTest.php @@ -40,6 +40,7 @@ class PasswordGrantTest extends \PHPUnit_Framework_TestCase $userRepositoryMock->method('getUserEntityByUserCredentials')->willReturn($userEntity); $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); + $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf(); $grant = new PasswordGrant($userRepositoryMock, $refreshTokenRepositoryMock); $grant->setClientRepository($clientRepositoryMock); diff --git a/tests/Grant/RefreshTokenGrantTest.php b/tests/Grant/RefreshTokenGrantTest.php index dc28b51f..8bfe71c9 100644 --- a/tests/Grant/RefreshTokenGrantTest.php +++ b/tests/Grant/RefreshTokenGrantTest.php @@ -43,6 +43,7 @@ class RefreshTokenGrantTest extends \PHPUnit_Framework_TestCase $userRepositoryMock->method('getUserEntityByUserCredentials')->willReturn($userEntity); $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); + $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf(); $grant = new RefreshTokenGrant($refreshTokenRepositoryMock); $grant->setClientRepository($clientRepositoryMock); diff --git a/tests/ServerTest.php b/tests/ServerTest.php new file mode 100644 index 00000000..9a85c351 --- /dev/null +++ b/tests/ServerTest.php @@ -0,0 +1,56 @@ +getMock(ClientRepositoryInterface::class), + $this->getMock(AccessTokenRepositoryInterface::class), + $this->getMock(ScopeRepositoryInterface::class), + '', + '', + new StubResponseType() + ); + + $server->enableGrantType(new ClientCredentialsGrant(), new \DateInterval('PT1M')); + + $response = $server->respondToRequest(); + $this->assertTrue($response instanceof ResponseInterface); + $this->assertEquals(400, $response->getStatusCode()); + } + + public function testRespondToRequest() + { + $clientRepository = $this->getMock(ClientRepositoryInterface::class); + $clientRepository->method('getClientEntity')->willReturn(new ClientEntity()); + + $server = new Server( + $clientRepository, + $this->getMock(AccessTokenRepositoryInterface::class), + $this->getMock(ScopeRepositoryInterface::class), + '', + '', + new StubResponseType() + ); + + $server->enableGrantType(new ClientCredentialsGrant(), new \DateInterval('PT1M')); + + $_POST['grant_type'] = 'client_credentials'; + $_POST['client_id'] = 'foo'; + $_POST['client_secret'] = 'bar'; + $response = $server->respondToRequest(); + $this->assertEquals(200, $response->getStatusCode()); + } +} diff --git a/tests/Stubs/StubResponseType.php b/tests/Stubs/StubResponseType.php index 4d40b089..d62daf9b 100644 --- a/tests/Stubs/StubResponseType.php +++ b/tests/Stubs/StubResponseType.php @@ -7,6 +7,7 @@ use League\OAuth2\Server\Entities\Interfaces\RefreshTokenEntityInterface; use League\OAuth2\Server\ResponseTypes\AbstractResponseType; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; +use Zend\Diactoros\Response; class StubResponseType extends AbstractResponseType { @@ -55,6 +56,6 @@ class StubResponseType extends AbstractResponseType */ public function generateHttpResponse(ResponseInterface $response) { - // TODO: Implement generateHttpResponse() method. + return new Response(); } } \ No newline at end of file