Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate confidential clients and determine if the client handles the grant type #1420

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
7 changes: 7 additions & 0 deletions src/Entities/ClientEntityInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,11 @@ public function getRedirectUri(): string|array;
* Returns true if the client is confidential.
*/
public function isConfidential(): bool;

/*
* Returns true if the client supports the given grant type.
*
* To be added in a future major release.
*/
// public function supportsGrantType(string $grantType): bool;
}
8 changes: 8 additions & 0 deletions src/Entities/Traits/ClientTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,12 @@ public function isConfidential(): bool
{
return $this->isConfidential;
}

/**
* Returns true if the client supports the given grant type.
*/
public function supportsGrantType(string $grantType): bool
{
return true;
}
}
11 changes: 10 additions & 1 deletion src/Exception/OAuthServerException.php
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,17 @@ public static function slowDown(string $hint = '', ?Throwable $previous = null):
}

/**
* Unauthorized client error.
*/
public static function unauthorizedClient(?string $hint = null): static
{
return $this->errorType;
return new static(
'The authenticated client is not authorized to use this authorization grant type.',
14,
'unauthorized_client',
400,
$hint
);
}

/**
Expand Down
35 changes: 26 additions & 9 deletions src/Grant/AbstractGrant.php
Original file line number Diff line number Diff line change
Expand Up @@ -151,18 +151,18 @@ protected function validateClient(ServerRequestInterface $request): ClientEntity
{
[$clientId, $clientSecret] = $this->getClientCredentials($request);

if ($this->clientRepository->validateClient($clientId, $clientSecret, $this->getIdentifier()) === false) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));

throw OAuthServerException::invalidClient($request);
}
$client = $this->getClientEntityOrFail($clientId, $request);

// If a redirect URI is provided ensure it matches what is pre-registered
$redirectUri = $this->getRequestParameter('redirect_uri', $request);
if ($client->isConfidential()) {
if ($clientSecret === '') {
throw OAuthServerException::invalidRequest('client_secret');
}

if ($redirectUri !== null) {
$this->validateRedirectUri($redirectUri, $client, $request);
if ($this->clientRepository->validateClient($clientId, $clientSecret, $this->getIdentifier()) === false) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));

throw OAuthServerException::invalidClient($request);
}
}

return $client;
Expand All @@ -189,9 +189,22 @@ protected function getClientEntityOrFail(string $clientId, ServerRequestInterfac
throw OAuthServerException::invalidClient($request);
}

if ($this->supportsGrantType($client, $this->getIdentifier()) === false) {
throw OAuthServerException::unauthorizedClient();
}

return $client;
}

/**
* Returns true if the given client is authorized to use the given grant type.
*/
protected function supportsGrantType(ClientEntityInterface $client, string $grantType): bool
{
return method_exists($client, 'supportsGrantType') === false
|| $client->supportsGrantType($grantType) === true;
}

/**
* Gets the client credentials from the request from the request body or
* the Http Basic Authorization header
Expand Down Expand Up @@ -484,6 +497,10 @@ protected function issueAuthCode(
*/
protected function issueRefreshToken(AccessTokenEntityInterface $accessToken): ?RefreshTokenEntityInterface
{
if ($this->supportsGrantType($accessToken->getClient(), 'refresh_token') === false) {
return null;
}

$refreshToken = $this->refreshTokenRepository->getNewRefreshToken();

if ($refreshToken === null) {
Expand Down
9 changes: 1 addition & 8 deletions src/Grant/AuthCodeGrant.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,7 @@ public function respondToAccessTokenRequest(
ResponseTypeInterface $responseType,
DateInterval $accessTokenTTL
): ResponseTypeInterface {
list($clientId) = $this->getClientCredentials($request);

$client = $this->getClientEntityOrFail($clientId, $request);

// Only validate the client if it is confidential
if ($client->isConfidential()) {
$this->validateClient($request);
}
$client = $this->validateClient($request);

$encryptedAuthCode = $this->getRequestParameter('code', $request);

Expand Down
7 changes: 1 addition & 6 deletions src/Grant/ClientCredentialsGrant.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,14 @@ public function respondToAccessTokenRequest(
ResponseTypeInterface $responseType,
DateInterval $accessTokenTTL
): ResponseTypeInterface {
list($clientId) = $this->getClientCredentials($request);

$client = $this->getClientEntityOrFail($clientId, $request);
$client = $this->validateClient($request);

if (!$client->isConfidential()) {
$this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));

throw OAuthServerException::invalidClient($request);
}

// Validate request
$this->validateClient($request);

$scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope));

// Finalize the requested scopes
Expand Down
133 changes: 55 additions & 78 deletions tests/Grant/AbstractGrantTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -265,84 +265,6 @@ public function testValidateClientInvalidClientSecret(): void
$validateClientMethod->invoke($grantMock, $serverRequest, true, true);
}

public function testValidateClientInvalidRedirectUri(): void
{
$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())->withParsedBody([
'client_id' => 'foo',
'redirect_uri' => 'http://bar/foo',
]);

$validateClientMethod = $abstractGrantReflection->getMethod('validateClient');
$validateClientMethod->setAccessible(true);

$this->expectException(OAuthServerException::class);

$validateClientMethod->invoke($grantMock, $serverRequest, true, true);
}

public function testValidateClientInvalidRedirectUriArray(): void
{
$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())->withParsedBody([
'client_id' => 'foo',
'redirect_uri' => 'http://bar/foo',
]);

$validateClientMethod = $abstractGrantReflection->getMethod('validateClient');
$validateClientMethod->setAccessible(true);

$this->expectException(OAuthServerException::class);

$validateClientMethod->invoke($grantMock, $serverRequest, true, true);
}

public function testValidateClientMalformedRedirectUri(): void
{
$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())->withParsedBody([
'client_id' => 'foo',
'redirect_uri' => ['not', 'a', 'string'],
]);

$validateClientMethod = $abstractGrantReflection->getMethod('validateClient');
$validateClientMethod->setAccessible(true);

$this->expectException(OAuthServerException::class);

$validateClientMethod->invoke($grantMock, $serverRequest, true, true);
}

public function testValidateClientBadClient(): void
{
$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
Expand Down Expand Up @@ -398,6 +320,7 @@ public function testIssueRefreshToken(): void
$issueRefreshTokenMethod->setAccessible(true);

$accessToken = new AccessTokenEntity();
$accessToken->setClient(new ClientEntity());

/** @var RefreshTokenEntityInterface $refreshToken */
$refreshToken = $issueRefreshTokenMethod->invoke($grantMock, $accessToken);
Expand All @@ -423,6 +346,34 @@ public function testIssueNullRefreshToken(): void
$issueRefreshTokenMethod->setAccessible(true);

$accessToken = new AccessTokenEntity();
$accessToken->setClient(new ClientEntity());
self::assertNull($issueRefreshTokenMethod->invoke($grantMock, $accessToken));
}

public function testIssueNullRefreshTokenUnauthorizedClient(): void
{
$client = $this->getMockBuilder(ClientEntity::class)->getMock();
$client
->expects(self::once())
->method('supportsGrantType')
->with('refresh_token')
->willReturn(false);

$refreshTokenRepoMock = $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock();
$refreshTokenRepoMock->expects(self::never())->method('getNewRefreshToken');

/** @var AbstractGrant $grantMock */
$grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
$grantMock->setRefreshTokenTTL(new DateInterval('PT1M'));
$grantMock->setRefreshTokenRepository($refreshTokenRepoMock);

$abstractGrantReflection = new ReflectionClass($grantMock);
$issueRefreshTokenMethod = $abstractGrantReflection->getMethod('issueRefreshToken');
$issueRefreshTokenMethod->setAccessible(true);

$accessToken = new AccessTokenEntity();
$accessToken->setClient($client);

self::assertNull($issueRefreshTokenMethod->invoke($grantMock, $accessToken));
}

Expand Down Expand Up @@ -576,4 +527,30 @@ public function testCompleteAuthorizationRequest(): void

$grantMock->completeAuthorizationRequest(new AuthorizationRequest());
}

public function testUnauthorizedClient(): void
{
$client = $this->getMockBuilder(ClientEntity::class)->getMock();
$client->method('supportsGrantType')->willReturn(false);

$clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
$clientRepositoryMock
->expects(self::once())
->method('getClientEntity')
->with('foo')
->willReturn($client);

/** @var AbstractGrant $grantMock */
$grantMock = $this->getMockForAbstractClass(AbstractGrant::class);
$grantMock->setClientRepository($clientRepositoryMock);

$abstractGrantReflection = new ReflectionClass($grantMock);

$getClientEntityOrFailMethod = $abstractGrantReflection->getMethod('getClientEntityOrFail');
$getClientEntityOrFailMethod->setAccessible(true);

$this->expectException(OAuthServerException::class);

$getClientEntityOrFailMethod->invoke($grantMock, 'foo', new ServerRequest());
}
}
Loading
Loading