From bf4128e5dbf10011b8f5e6da1a2d6d187e7dcf1a Mon Sep 17 00:00:00 2001
From: Bojan Bogdanovic <info@bojanbogdanovic.nl>
Date: Mon, 30 Dec 2024 15:03:34 +0100
Subject: [PATCH 1/3] Limit the allowed scopes by default scopes via the
consumer for the client credentials grant type
---
simple_oauth.module | 2 +-
src/Repositories/ScopeRepository.php | 19 +++++++--
tests/src/Kernel/ClientCredentialsTest.php | 47 +++++++++++++++++++++-
3 files changed, 63 insertions(+), 5 deletions(-)
diff --git a/simple_oauth.module b/simple_oauth.module
index ebf7145..61349e1 100644
--- a/simple_oauth.module
+++ b/simple_oauth.module
@@ -95,7 +95,7 @@ function simple_oauth_entity_base_field_info(EntityTypeInterface $entity_type) {
$fields['scopes'] = BaseFieldDefinition::create('oauth2_scope_reference')
->setLabel(new TranslatableMarkup('Scopes'))
- ->setDescription(new TranslatableMarkup('Here you can select scopes that would be the default scopes when authorizing.'))
+ ->setDescription(new TranslatableMarkup('Selecting scopes here will be set as default when authorizing (so no need to specify the scope parameter on the token request), also the selection of scopes will limit the allowed scopes on the consumer/client.'))
->setDisplayOptions('view', [
'label' => 'inline',
'type' => 'oauth2_scope_reference_label',
diff --git a/src/Repositories/ScopeRepository.php b/src/Repositories/ScopeRepository.php
index 4cf5d4e..3824b4c 100644
--- a/src/Repositories/ScopeRepository.php
+++ b/src/Repositories/ScopeRepository.php
@@ -66,11 +66,24 @@ class ScopeRepository implements ScopeRepositoryInterface {
}
$default_scopes = [];
+ $allowed_scopes = [];
$client_drupal_entity = $clientEntity->getDrupalEntity();
if (!$client_drupal_entity->get('scopes')->isEmpty()) {
- $default_scopes = array_map(function (Oauth2ScopeInterface $scope) {
- return $this->scopeFactory($scope);
- }, $client_drupal_entity->get('scopes')->getScopes());
+ foreach ($client_drupal_entity->get('scopes')->getScopes() as $scope) {
+ $default_scope = $this->scopeFactory($scope);
+ $default_scopes[] = $default_scope;
+ $allowed_scopes[] = $default_scope->getIdentifier();
+ }
+
+ // Limit the scopes if default scopes are set on the consumer for the
+ // client credentials grant type.
+ if ($grantType === 'client_credentials') {
+ foreach ($scopes as $scope) {
+ if (!in_array($scope->getIdentifier(), $allowed_scopes)) {
+ throw OAuthServerException::invalidScope($scope->getIdentifier());
+ }
+ }
+ }
}
$finalized_scopes = !empty($scopes) ? $scopes : $default_scopes;
diff --git a/tests/src/Kernel/ClientCredentialsTest.php b/tests/src/Kernel/ClientCredentialsTest.php
index 965bbb1..61ee88f 100644
--- a/tests/src/Kernel/ClientCredentialsTest.php
+++ b/tests/src/Kernel/ClientCredentialsTest.php
@@ -4,6 +4,9 @@ namespace Drupal\Tests\simple_oauth\Kernel;
use Drupal\Component\Serialization\Json;
use Drupal\TestTools\Random;
+use Drupal\Core\Session\AccountInterface;
+use Drupal\simple_oauth\Entity\Oauth2Scope;
+use Drupal\simple_oauth\Oauth2ScopeInterface;
use Symfony\Component\HttpFoundation\Request;
/**
@@ -197,7 +200,6 @@ class ClientCredentialsTest extends AuthorizedRequestBase {
* @dataProvider publicClientCredentialsGrantProvider
*/
public function testPublicClientCredentialsGrant(bool $confidential, string|null $client_secret, string $error, int $code): void {
-
// Configure the client for the test.
$this->client
->set('confidential', $confidential)
@@ -218,4 +220,47 @@ class ClientCredentialsTest extends AuthorizedRequestBase {
$this->assertSame($error, $parsed_response['error'], sprintf('Correct error code %s', $error));
}
+ /**
+ * Test the not allowed scopes set on the client.
+ */
+ public function testNotAllowedScopes(): void {
+ $not_allowed_scope = Oauth2Scope::create([
+ 'name' => 'test:scope3',
+ 'description' => 'Test scope 3 description',
+ 'grant_types' => [
+ 'client_credentials' => [
+ 'status' => TRUE,
+ 'description' => 'Test scope 3 description client_credentials',
+ ],
+ ],
+ 'umbrella' => FALSE,
+ 'granularity_id' => Oauth2ScopeInterface::GRANULARITY_ROLE,
+ 'granularity_configuration' => [
+ 'role' => AccountInterface::AUTHENTICATED_ROLE,
+ ],
+ ]);
+ $not_allowed_scope->save();
+
+ $parameters = [
+ 'grant_type' => 'client_credentials',
+ 'client_id' => $this->client->getClientId(),
+ 'client_secret' => $this->clientSecret,
+ 'scope' => $not_allowed_scope->getName(),
+ ];
+ $request = Request::create($this->url->toString(), 'POST', $parameters);
+ $response = $this->httpKernel->handle($request);
+ $parsed_response = Json::decode((string) $response->getContent());
+
+ $this->assertEquals(400, $response->getStatusCode());
+ $this->assertEquals('The requested scope is invalid, unknown, or malformed', $parsed_response['message']);
+
+ $parameters['scope'] = "{$this->scope} {$not_allowed_scope->getName()}";
+ $request = Request::create($this->url->toString(), 'POST', $parameters);
+ $response = $this->httpKernel->handle($request);
+ $parsed_response = Json::decode((string) $response->getContent());
+
+ $this->assertEquals(400, $response->getStatusCode());
+ $this->assertEquals('The requested scope is invalid, unknown, or malformed', $parsed_response['message']);
+ }
+
}
--
GitLab
From 86674470421c396343986b97f59f0b86de4fc0df Mon Sep 17 00:00:00 2001
From: Bojan Bogdanovic <info@bojanbogdanovic.nl>
Date: Mon, 30 Dec 2024 15:25:14 +0100
Subject: [PATCH 2/3] Update assertion
---
tests/src/Kernel/ClientCredentialsTest.php | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/src/Kernel/ClientCredentialsTest.php b/tests/src/Kernel/ClientCredentialsTest.php
index 61ee88f..70b088c 100644
--- a/tests/src/Kernel/ClientCredentialsTest.php
+++ b/tests/src/Kernel/ClientCredentialsTest.php
@@ -252,7 +252,7 @@ class ClientCredentialsTest extends AuthorizedRequestBase {
$parsed_response = Json::decode((string) $response->getContent());
$this->assertEquals(400, $response->getStatusCode());
- $this->assertEquals('The requested scope is invalid, unknown, or malformed', $parsed_response['message']);
+ $this->assertEquals('The requested scope is invalid, unknown, or malformed', $parsed_response['error_description']);
$parameters['scope'] = "{$this->scope} {$not_allowed_scope->getName()}";
$request = Request::create($this->url->toString(), 'POST', $parameters);
@@ -260,7 +260,7 @@ class ClientCredentialsTest extends AuthorizedRequestBase {
$parsed_response = Json::decode((string) $response->getContent());
$this->assertEquals(400, $response->getStatusCode());
- $this->assertEquals('The requested scope is invalid, unknown, or malformed', $parsed_response['message']);
+ $this->assertEquals('The requested scope is invalid, unknown, or malformed', $parsed_response['error_description']);
}
}
--
GitLab
From a51210c1135b65e22b9a7a73d2d67dffa743b85c Mon Sep 17 00:00:00 2001
From: Bojan Bogdanovic <info@bojanbogdanovic.nl>
Date: Thu, 23 Jan 2025 14:13:54 +0100
Subject: [PATCH 3/3] Make scope parameter required in the token endpoint when
dealing with client_credentials and no default scopes are set
---
src/Controller/Oauth2Token.php | 12 ++++++++++++
tests/src/Kernel/ClientCredentialsTest.php | 13 ++++++++++---
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/src/Controller/Oauth2Token.php b/src/Controller/Oauth2Token.php
index c0f31e2..a3d8e6e 100644
--- a/src/Controller/Oauth2Token.php
+++ b/src/Controller/Oauth2Token.php
@@ -113,6 +113,8 @@ class Oauth2Token extends ControllerBase {
$server_request = $this->httpMessageFactory->createRequest($request);
$server_response = new Response();
$client_id = $request->get('client_id');
+ $grant_type = $request->get('grant_type');
+ $scopes = $request->get('scope');
$lock_key = $this->createLockKey($request);
@@ -135,6 +137,16 @@ class Oauth2Token extends ControllerBase {
}
$client_drupal_entity = $client_entity->getDrupalEntity();
+ // Omitting scopes is not allowed when dealing with client_credentials
+ // and no default scopes are set.
+ if (
+ $grant_type === 'client_credentials' &&
+ empty($scopes) &&
+ $client_drupal_entity->get('scopes')->isEmpty()
+ ) {
+ throw OAuthServerException::invalidRequest('scope');
+ }
+
// Respond to the incoming request and fill in the response.
$server = $this->authorizationServerFactory->get($client_drupal_entity);
$response = $server->respondToAccessTokenRequest($server_request, $server_response);
diff --git a/tests/src/Kernel/ClientCredentialsTest.php b/tests/src/Kernel/ClientCredentialsTest.php
index 70b088c..e57ac36 100644
--- a/tests/src/Kernel/ClientCredentialsTest.php
+++ b/tests/src/Kernel/ClientCredentialsTest.php
@@ -71,21 +71,26 @@ class ClientCredentialsTest extends AuthorizedRequestBase {
*/
public static function missingClientCredentialsProvider(): array {
return [
- 'grant_type' => [
+ [
'grant_type',
'unsupported_grant_type',
400,
],
- 'client_id' => [
+ [
'client_id',
'invalid_request',
400,
],
- 'client_secret' => [
+ [
'client_secret',
'invalid_client',
401,
],
+ [
+ 'scope',
+ 'invalid_request',
+ 400,
+ ],
];
}
@@ -102,6 +107,8 @@ class ClientCredentialsTest extends AuthorizedRequestBase {
'scope' => $this->scope,
];
unset($parameters[$key]);
+ $this->client->set('scopes', []);
+ $this->client->save();
$request = Request::create($this->url->toString(), 'POST', $parameters);
$response = $this->httpKernel->handle($request);
$parsed_response = Json::decode((string) $response->getContent());
--
GitLab