From da981618fd10d6547774fa9645401edb17ffd2d2 Mon Sep 17 00:00:00 2001 From: Christian Fasching Date: Wed, 16 Sep 2026 13:58:03 +0200 Subject: [PATCH] [OAuth] Give a request without scope the resource's declared scopes An omitted scope passed through as an empty set, so the consent screen said nothing was requested while the token still reached the resource. Co-Authored-By: Claude Opus 5 --- .../06_OAuth_Server.md | 4 ++- .../07_OAuth_Protected_Applications.md | 8 +++-- .../Server/Grant/LoopbackAuthCodeGrant.php | 36 ++++++++++++++++--- .../Grant/LoopbackAuthCodeGrantTest.php | 26 +++++++++++--- .../Server/ResourceBindingLifecycleTest.php | 25 ++++++++++--- 5 files changed, 84 insertions(+), 15 deletions(-) diff --git a/doc/02_Installation_and_Configuration/06_OAuth_Server.md b/doc/02_Installation_and_Configuration/06_OAuth_Server.md index 7f20c9d17..b0599e62f 100644 --- a/doc/02_Installation_and_Configuration/06_OAuth_Server.md +++ b/doc/02_Installation_and_Configuration/06_OAuth_Server.md @@ -354,7 +354,9 @@ for the full rules. `scopes_supported` does two jobs. It caps what a token for that resource may carry, so a client asking for more is narrowed to the intersection and one asking **only** for scopes the resource does not declare is -refused with `invalid_scope`. And it is **how a scope comes to exist at all**: the server's catalogue, which +refused with `invalid_scope`. A client naming no scope at all is given every scope the resource declares, so +the consent screen always shows what the token will carry. And it is **how a scope comes to exist at all**: +the server's catalogue, which the authorization endpoint accepts, dynamic clients may register and the metadata advertises, is the union of the `scopes_supported` of every registered resource. There is nothing else to declare, and nothing that can disagree with it. diff --git a/doc/04_Development_Details/07_OAuth_Protected_Applications.md b/doc/04_Development_Details/07_OAuth_Protected_Applications.md index 0d8a3c66d..c047378c0 100644 --- a/doc/04_Development_Details/07_OAuth_Protected_Applications.md +++ b/doc/04_Development_Details/07_OAuth_Protected_Applications.md @@ -241,8 +241,8 @@ document 404s, its scopes vanish from the catalogue, and a client requesting its declared. If your resource is missing, check the tag first. The scopes are not decoration. They cap what a token for this resource may carry, a client asking for more is -narrowed to them before consent is shown, and they are how a scope comes to exist at all: the server's -catalogue is the union of what every resource supports. +narrowed to them before consent is shown, a client asking for none is given all of them, and they are how a +scope comes to exist at all: the server's catalogue is the union of what every resource supports. Providers are read lazily and only once. Symfony's tagged iterator does not instantiate anything until the registry is first read, and the registry memoises what it resolved, so a request touching neither OAuth nor @@ -377,6 +377,10 @@ on the token and reported back, but nothing compares a granted scope against an is therefore an upper bound the server maintains, not a check anyone performs: treat a scope as a label shown at consent time, not a guarantee, and enforce it yourself if your operations differ in privilege. +Check for your scope even when every operation has the same privilege. The authorization request always leads to +a consent screen listing your scopes, but a client refreshing a token may ask for fewer scopes than were granted, +including none, and no screen is shown then. Requiring your scope keeps such a token from reaching your resource. + ## Related - [OAuth 2.1 Authorization Server](../02_Installation_and_Configuration/06_OAuth_Server.md) - enabling and diff --git a/src/OAuth/Server/Grant/LoopbackAuthCodeGrant.php b/src/OAuth/Server/Grant/LoopbackAuthCodeGrant.php index 97750f760..604207960 100644 --- a/src/OAuth/Server/Grant/LoopbackAuthCodeGrant.php +++ b/src/OAuth/Server/Grant/LoopbackAuthCodeGrant.php @@ -127,13 +127,20 @@ private function narrowToResource(AuthorizationRequestInterface $request, string $supported = $this->resourceRegistry->get($resource)->scopesSupported ?? []; $requested = $request->getScopes(); - // A resource that declares no scopes constrains nothing, and a request that names - // none has nothing to narrow. Neither is an error: both are reachable today, and - // refusing them would turn working clients away over a token nobody checks. - if ($supported === [] || $requested === []) { + // A resource that declares no scopes constrains nothing. + if ($supported === []) { return $requested; } + // RFC 6749 section 3.3: a request that omits `scope` is processed with a default + // value or refused. Refusing would turn away clients that work today, so the default + // is everything the resource declares. Passing the empty set through instead issued a + // token with no scopes at all, whose consent screen told the user that nothing was + // requested while the token still reached the resource. + if ($requested === []) { + return $this->scopesOf($supported); + } + $narrowed = array_values( array_filter( $requested, @@ -163,6 +170,27 @@ private function narrowToResource(AuthorizationRequestInterface $request, string return $narrowed; } + /** + * The catalogue is derived from the same resources, so every declared scope resolves; + * the null check only keeps the return type honest. + * + * @param list $identifiers + * + * @return ScopeEntityInterface[] + */ + private function scopesOf(array $identifiers): array + { + $scopes = []; + foreach ($identifiers as $identifier) { + $scope = $this->scopeRepository->getScopeEntityByIdentifier($identifier); + if ($scope !== null) { + $scopes[] = $scope; + } + } + + return $scopes; + } + /** * The redirect league itself would have used for an `invalid_scope`, so a refusal * raised here reaches the client the same way rather than as a bare response. diff --git a/tests/Unit/OAuth/Server/Grant/LoopbackAuthCodeGrantTest.php b/tests/Unit/OAuth/Server/Grant/LoopbackAuthCodeGrantTest.php index a3a192197..3ede42c15 100644 --- a/tests/Unit/OAuth/Server/Grant/LoopbackAuthCodeGrantTest.php +++ b/tests/Unit/OAuth/Server/Grant/LoopbackAuthCodeGrantTest.php @@ -409,12 +409,30 @@ public function testScopesAreNarrowedToWhatTheResourceSupports(): void /** * The production configuration: no default scope is ever set, so a client that names - * none arrives with an empty list. Narrowing must leave it alone rather than read it - * as "asked for nothing this resource supports" and refuse a working client. + * none arrives with an empty list. It is still accepted, but given everything the + * resource declares: passing the empty set through issued a token whose consent screen + * said nothing was requested while the token still reached the resource. */ - public function testARequestNamingNoScopeIsAccepted(): void + public function testARequestNamingNoScopeIsGivenEveryScopeTheResourceDeclares(): void { - $authRequest = $this->grant(defaultScope: '', supportedScopes: ['mcp:read']) + $authRequest = $this->grant(defaultScope: '', supportedScopes: ['mcp:read', 'mcp:write']) + ->validateAuthorizationRequest($this->authorizeRequest([ + 'code_challenge' => self::CODE_CHALLENGE, + 'code_challenge_method' => 'S256', + 'resource' => self::KNOWN_RESOURCE, + 'scope' => null, + ])); + + $this->assertSame(['mcp:read', 'mcp:write'], $this->scopeIdentifiers($authRequest)); + } + + /** + * Without declared scopes there is no default to apply, so the request keeps its empty + * list rather than being refused. + */ + public function testARequestNamingNoScopeForAResourceDeclaringNoneStaysEmpty(): void + { + $authRequest = $this->grant(defaultScope: '', supportedScopes: []) ->validateAuthorizationRequest($this->authorizeRequest([ 'code_challenge' => self::CODE_CHALLENGE, 'code_challenge_method' => 'S256', diff --git a/tests/Unit/OAuth/Server/ResourceBindingLifecycleTest.php b/tests/Unit/OAuth/Server/ResourceBindingLifecycleTest.php index 4a43dcfdf..4048adf12 100644 --- a/tests/Unit/OAuth/Server/ResourceBindingLifecycleTest.php +++ b/tests/Unit/OAuth/Server/ResourceBindingLifecycleTest.php @@ -124,6 +124,19 @@ public function testTheAudienceSurvivesIssuanceAndRefresh(): void $this->assertSame([self::RESOURCE], $this->claims($refreshed['access_token'])->claims()->get('aud')); } + /** + * A client that omits `scope` gets the resource's declared scopes on the token, not an + * empty set. Driven end to end because the scopes travel from the authorization request + * through the encrypted code to the JWT claim, and the claim is what an application reads. + */ + public function testARequestWithoutScopeIsIssuedTheScopesTheResourceDeclares(): void + { + $issued = $this->exchange($this->authorize(null)); + + $this->assertSame(self::SCOPE, $this->claims($issued['access_token'])->claims()->get('scope')); + $this->assertSame(self::SCOPE, $issued['scope'] ?? null); + } + /** * league validates a refresh token from its own encrypted payload and reads an unknown * identifier as "not revoked", so nothing but this refuses a token whose record is gone. @@ -181,17 +194,21 @@ private function expectRefusal(callable $call): void /** * Runs the authorization leg and returns the authorization code it redirects with. */ - private function authorize(): string + private function authorize(?string $scope = self::SCOPE): string { - $request = (new ServerRequest('GET', self::ISSUER . '/pimcore-oauth/authorize'))->withQueryParams([ + $query = [ 'client_id' => self::CLIENT_ID, 'response_type' => 'code', 'redirect_uri' => self::REDIRECT_URI, - 'scope' => self::SCOPE, 'resource' => self::RESOURCE, 'code_challenge' => $this->codeChallenge(), 'code_challenge_method' => 'S256', - ]); + ]; + if ($scope !== null) { + $query['scope'] = $scope; + } + + $request = (new ServerRequest('GET', self::ISSUER . '/pimcore-oauth/authorize'))->withQueryParams($query); $authorizationRequest = $this->server->validateAuthorizationRequest($request); $authorizationRequest->setUser(new UserEntity('21'));