diff --git a/CHANGELOG.md b/CHANGELOG.md index dfe3c16f..f74b4849 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `issueAccessToken()` in the Abstract Grant no longer sets access token client, user ID or scopes. These values should already have been set when calling `getNewToken()` (PR #919) - No longer need to enable PKCE with `enableCodeExchangeProof` flag. Any client sending a code challenge will initiate PKCE checks. (PR #938) - Function `getClientEntity()` no longer performs client validation (PR #938) +- Password Grant now returns an invalid_grant error instead of invalid_credentials if a user cannot be validated (PR #967) - Use `DateTimeImmutable()` instead of `DateTime()`, `time()` instead of `(new DateTime())->getTimeStamp()`, and `DateTime::getTimeStamp()` instead of `DateTime::format('U')` (PR #963) ### Removed diff --git a/src/Grant/AbstractGrant.php b/src/Grant/AbstractGrant.php index 30f5448c..d64c9697 100644 --- a/src/Grant/AbstractGrant.php +++ b/src/Grant/AbstractGrant.php @@ -173,15 +173,7 @@ abstract class AbstractGrant implements GrantTypeInterface */ protected function validateClient(ServerRequestInterface $request) { - list($basicAuthUser, $basicAuthPassword) = $this->getBasicAuthCredentials($request); - - $clientId = $this->getRequestParameter('client_id', $request, $basicAuthUser); - - if (is_null($clientId)) { - throw OAuthServerException::invalidRequest('client_id'); - } - - $clientSecret = $this->getRequestParameter('client_secret', $request, $basicAuthPassword); + list($clientId, $clientSecret) = $this->getClientCredentials($request); if ($this->clientRepository->validateClient($clientId, $clientSecret, $this->getIdentifier()) === false) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); @@ -201,6 +193,29 @@ abstract class AbstractGrant implements GrantTypeInterface return $client; } + /** + * Gets the client credentials from the request from the request body or + * the Http Basic Authorization header + * + * @param ServerRequestInterface $request + * + * @return array + */ + protected function getClientCredentials(ServerRequestInterface $request) + { + list($basicAuthUser, $basicAuthPassword) = $this->getBasicAuthCredentials($request); + + $clientId = $this->getRequestParameter('client_id', $request, $basicAuthUser); + + if (is_null($clientId)) { + throw OAuthServerException::invalidRequest('client_id'); + } + + $clientSecret = $this->getRequestParameter('client_secret', $request, $basicAuthPassword); + + return [$clientId, $clientSecret]; + } + /** * Validate redirectUri from the request. * If a redirect URI is provided ensure it matches what is pre-registered diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 9758744d..a26ea6ec 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -92,11 +92,7 @@ class AuthCodeGrant extends AbstractAuthorizeGrant ResponseTypeInterface $responseType, DateInterval $accessTokenTTL ) { - $clientId = $this->getRequestParameter('client_id', $request, null); - - if ($clientId === null) { - throw OAuthServerException::invalidRequest('client_id'); - } + list($clientId) = $this->getClientCredentials($request); $client = $this->clientRepository->getClientEntity($clientId); diff --git a/src/Grant/PasswordGrant.php b/src/Grant/PasswordGrant.php index 4b68ad44..7375d436 100644 --- a/src/Grant/PasswordGrant.php +++ b/src/Grant/PasswordGrant.php @@ -82,11 +82,13 @@ class PasswordGrant extends AbstractGrant protected function validateUser(ServerRequestInterface $request, ClientEntityInterface $client) { $username = $this->getRequestParameter('username', $request); + if (is_null($username)) { throw OAuthServerException::invalidRequest('username'); } $password = $this->getRequestParameter('password', $request); + if (is_null($password)) { throw OAuthServerException::invalidRequest('password'); } @@ -97,10 +99,11 @@ class PasswordGrant extends AbstractGrant $this->getIdentifier(), $client ); + if ($user instanceof UserEntityInterface === false) { $this->getEmitter()->emit(new RequestEvent(RequestEvent::USER_AUTHENTICATION_FAILED, $request)); - throw OAuthServerException::invalidCredentials(); + throw OAuthServerException::invalidGrant(); } return $user; diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php index ed702e8a..9b6f24d1 100644 --- a/tests/Grant/AuthCodeGrantTest.php +++ b/tests/Grant/AuthCodeGrantTest.php @@ -607,6 +607,71 @@ class AuthCodeGrantTest extends TestCase $this->assertInstanceOf(RefreshTokenEntityInterface::class, $response->getRefreshToken()); } + public function testRespondToAccessTokenRequestUsingHttpBasicAuth() + { + $client = new ClientEntity(); + $client->setIdentifier('foo'); + $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); + $clientRepositoryMock->method('getClientEntity')->willReturn($client); + + $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock(); + $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn(new ScopeEntity()); + $scopeRepositoryMock->method('finalizeScopes')->willReturnArgument(0); + + $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); + $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity()); + + $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); + $refreshTokenRepositoryMock->method('getNewRefreshToken')->willReturn(new RefreshTokenEntity()); + + $authCodeGrant = new AuthCodeGrant( + $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), + $refreshTokenRepositoryMock, + new \DateInterval('PT10M') + ); + + $authCodeGrant->setClientRepository($clientRepositoryMock); + $authCodeGrant->setScopeRepository($scopeRepositoryMock); + $authCodeGrant->setAccessTokenRepository($accessTokenRepositoryMock); + $authCodeGrant->setEncryptionKey($this->cryptStub->getKey()); + $authCodeGrant->setPrivateKey(new CryptKey('file://' . __DIR__ . '/../Stubs/private.key')); + + $request = new ServerRequest( + [], + [], + null, + 'POST', + 'php://input', + [ + 'Authorization' => 'Basic Zm9vOmJhcg==', + ], + [], + [], + [ + 'grant_type' => 'authorization_code', + 'redirect_uri' => 'http://foo/bar', + 'code' => $this->cryptStub->doEncrypt( + json_encode( + [ + 'auth_code_id' => uniqid(), + 'client_id' => 'foo', + 'expire_time' => time() + 3600, + 'user_id' => 123, + 'scopes' => ['foo'], + 'redirect_uri' => 'http://foo/bar', + ] + ) + ), + ] + ); + + /** @var StubResponseType $response */ + $response = $authCodeGrant->respondToAccessTokenRequest($request, new StubResponseType(), new \DateInterval('PT10M')); + + $this->assertInstanceOf(AccessTokenEntityInterface::class, $response->getAccessToken()); + $this->assertInstanceOf(RefreshTokenEntityInterface::class, $response->getRefreshToken()); + } + public function testRespondToAccessTokenRequestForPublicClient() { $client = new ClientEntity(); @@ -966,27 +1031,16 @@ class AuthCodeGrantTest extends TestCase public function testRespondToAccessTokenRequestExpiredCode() { - $client = new ClientEntity(); - $client->setIdentifier('foo'); - $client->setRedirectUri('http://foo/bar'); - $client->setConfidential(); $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock(); - $clientRepositoryMock->method('getClientEntity')->willReturn($client); - - $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock(); - $accessTokenRepositoryMock->method('persistNewAccessToken')->willReturnSelf(); - - $refreshTokenRepositoryMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(); - $refreshTokenRepositoryMock->method('persistNewRefreshToken')->willReturnSelf(); + $clientRepositoryMock->method('getClientEntity')->willReturn(new ClientEntity()); $grant = new AuthCodeGrant( $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(), new DateInterval('PT10M') ); + $grant->setClientRepository($clientRepositoryMock); - $grant->setAccessTokenRepository($accessTokenRepositoryMock); - $grant->setRefreshTokenRepository($refreshTokenRepositoryMock); $grant->setEncryptionKey($this->cryptStub->getKey()); $request = new ServerRequest( diff --git a/tests/Grant/PasswordGrantTest.php b/tests/Grant/PasswordGrantTest.php index 56a278df..88592339 100644 --- a/tests/Grant/PasswordGrantTest.php +++ b/tests/Grant/PasswordGrantTest.php @@ -146,6 +146,7 @@ class PasswordGrantTest extends TestCase /** * @expectedException \League\OAuth2\Server\Exception\OAuthServerException + * @expectedExceptionCode 10 */ public function testRespondToRequestBadCredentials() {