diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index e34436e5..c34e65cf 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -174,27 +174,24 @@ abstract class AbstractGrant implements GrantTypeInterface list($basicAuthUser, $basicAuthPassword) = $this->getBasicAuthCredentials($request); $clientId = $this->getRequestParameter('client_id', $request, $basicAuthUser); + if (is_null($clientId)) { throw OAuthServerException::invalidRequest('client_id'); } - // If the client is confidential require the client secret $clientSecret = $this->getRequestParameter('client_secret', $request, $basicAuthPassword); - $client = $this->clientRepository->getClientEntity( - $clientId, - $this->getIdentifier(), - $clientSecret, - true - ); - - if ($client instanceof ClientEntityInterface === false) { + if ($this->clientRepository->validateClient($clientId, $clientSecret) === false) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); + throw OAuthServerException::invalidClient($request); } + $client = $this->clientRepository->getClientEntity($clientId); + // If a redirect URI is provided ensure it matches what is pre-registered $redirectUri = $this->getRequestParameter('redirect_uri', $request, null); + if ($redirectUri !== null) { $this->validateRedirectUri($redirectUri, $client, $request); } diff --git a/src/Repositories/ClientRepositoryInterface.php b/src/Repositories/ClientRepositoryInterface.php index 9e863af8..d390a5f6 100644 --- a/src/Repositories/ClientRepositoryInterface.php +++ b/src/Repositories/ClientRepositoryInterface.php @@ -19,15 +19,11 @@ interface ClientRepositoryInterface extends RepositoryInterface /** * Get a client. * - * @param string $clientIdentifier The client's identifier - * @param null|string $grantType The grant type used (if sent) - * @param null|string $clientSecret The client's secret (if sent) - * @param bool $mustValidateSecret If true the client must attempt to validate the secret if the client - * is confidential + * @param string $clientIdentifier The client's identifier * * @return ClientEntityInterface */ - public function getClientEntity($clientIdentifier, $grantType = null, $clientSecret = null, $mustValidateSecret = true); + public function getClientEntity($clientIdentifier); /** * Check if a client is confidential. @@ -37,4 +33,21 @@ interface ClientRepositoryInterface extends RepositoryInterface * @return bool */ public function isClientConfidential($clientIdentifier); + + /** + * Validate a client's secret. + * + * @param string $clientIdentifier The client's identifier + * @param null|string $clientSecret The client's secret (if sent) + * + * @return bool + */ + public function validateClient($clientIdentifier, $clientSecret); + + /** + * Check if a client can use a grant type. + * + * @return bool + */ + public function canUseGrant($clientIdentifier, $grantType); } diff --git a/tests/Exception/OAuthServerExceptionTest.php b/tests/Exception/OAuthServerExceptionTest.php index cead7d6a..8efce51f 100644 --- a/tests/Exception/OAuthServerExceptionTest.php +++ b/tests/Exception/OAuthServerExceptionTest.php @@ -52,7 +52,7 @@ class OAuthServerExceptionTest extends TestCase private function issueInvalidClientException($serverRequest) { $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); - $clientRepositoryMock->method('getClientEntity')->willReturn(false); + $clientRepositoryMock->method('validateClient')->willReturn(false); $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); $grantMock->setClientRepository($clientRepositoryMock); diff --git a/tests/Grant/AbstractGrantTest.php b/tests/Grant/AbstractGrantTest.php index 64fde4f0..d6ddd4b1 100644 --- a/tests/Grant/AbstractGrantTest.php +++ b/tests/Grant/AbstractGrantTest.php @@ -175,7 +175,7 @@ class AbstractGrantTest extends TestCase public function testValidateClientMissingClientSecret() { $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); - $clientRepositoryMock->method('getClientEntity')->willReturn(null); + $clientRepositoryMock->method('validateClient')->willReturn(false); /** @var AbstractGrant $grantMock */ $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); @@ -200,7 +200,7 @@ class AbstractGrantTest extends TestCase public function testValidateClientInvalidClientSecret() { $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); - $clientRepositoryMock->method('getClientEntity')->willReturn(null); + $clientRepositoryMock->method('validateClient')->willReturn(false); /** @var AbstractGrant $grantMock */ $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); @@ -282,7 +282,7 @@ class AbstractGrantTest extends TestCase public function testValidateClientBadClient() { $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); - $clientRepositoryMock->method('getClientEntity')->willReturn(null); + $clientRepositoryMock->method('validateClient')->willReturn(false); /** @var AbstractGrant $grantMock */ $grantMock = $this->getMockForAbstractClass(AbstractGrant::class); diff --git a/tests/Middleware/AuthorizationServerMiddlewareTest.php b/tests/Middleware/AuthorizationServerMiddlewareTest.php index 99118736..dcfcfbd9 100644 --- a/tests/Middleware/AuthorizationServerMiddlewareTest.php +++ b/tests/Middleware/AuthorizationServerMiddlewareTest.php @@ -66,7 +66,7 @@ class AuthorizationServerMiddlewareTest extends TestCase public function testOAuthErrorResponse() { $clientRepository = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); - $clientRepository->method('getClientEntity')->willReturn(null); + $clientRepository->method('validateClient')->willReturn(false); $server = new AuthorizationServer( $clientRepository,