Fix "state" parameter of error responses

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Fixed
 - Auto-generated event emitter is now persisted. Previously, a new emitter was generated every time (PR #1428)
 - Fixed bug where you could not omit a redirect uri even if one had not been specified during the auth request (PR #1428)
+- Fixed bug where "state" parameter wasn't present on `invalid_scope` error response and wasn't on fragment part of `access_denied` redirect URI on Implicit grant (PR #1298) 
 
 ## [9.0.0] - released 2024-05-13
 ### Added

src/Grant/AbstractAuthorizeGrant.php

@@ -14,6 +14,7 @@
 
 namespace League\OAuth2\Server\Grant;
 
+use League\OAuth2\Server\Entities\ClientEntityInterface;
 use League\OAuth2\Server\RequestTypes\AuthorizationRequest;
 use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface;
 
@@ -35,4 +36,14 @@ protected function createAuthorizationRequest(): AuthorizationRequestInterface
     {
         return new AuthorizationRequest();
     }
+
+    /**
+     * Get the client redirect URI.
+     */
+    protected function getClientRedirectUri(ClientEntityInterface $client): string
+    {
+        return is_array($client->getRedirectUri())
+            ? $client->getRedirectUri()[0]
+            : $client->getRedirectUri();
+    }
 }

src/Grant/AuthCodeGrant.php

@@ -280,17 +280,16 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A
             throw OAuthServerException::invalidClient($request);
         }
 
-        $defaultClientRedirectUri = is_array($client->getRedirectUri())
-            ? $client->getRedirectUri()[0]
-            : $client->getRedirectUri();
+        $stateParameter = $this->getQueryStringParameter('state', $request);
 
         $scopes = $this->validateScopes(
             $this->getQueryStringParameter('scope', $request, $this->defaultScope),
-            $redirectUri ?? $defaultClientRedirectUri
+            $this->makeRedirectUri(
+                $redirectUri ?? $this->getClientRedirectUri($client),
+                $stateParameter !== null ? ['state' => $stateParameter] : []
+            )
         );
 
-        $stateParameter = $this->getQueryStringParameter('state', $request);
-
         $authorizationRequest = $this->createAuthorizationRequest();
         $authorizationRequest->setGrantTypeId($this->getIdentifier());
         $authorizationRequest->setClient($client);
@@ -354,7 +353,7 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth
         }
 
         $finalRedirectUri = $authorizationRequest->getRedirectUri()
-                          ?? $this->getClientRedirectUri($authorizationRequest);
+                          ?? $this->getClientRedirectUri($authorizationRequest->getClient());
 
         // The user approved the client, redirect them back with an auth code
         if ($authorizationRequest->isAuthorizationApproved() === true) {
@@ -408,14 +407,4 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth
             )
         );
     }
-
-    /**
-     * Get the client redirect URI if not set in the request.
-     */
-    private function getClientRedirectUri(AuthorizationRequestInterface $authorizationRequest): string
-    {
-        return is_array($authorizationRequest->getClient()->getRedirectUri())
-                ? $authorizationRequest->getClient()->getRedirectUri()[0]
-                : $authorizationRequest->getClient()->getRedirectUri();
-    }
 }

src/Grant/ImplicitGrant.php

@@ -111,24 +111,24 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A
         if ($redirectUri !== null) {
             $this->validateRedirectUri($redirectUri, $client, $request);
         } elseif (
-            is_array($client->getRedirectUri()) && count($client->getRedirectUri()) !== 1
-            || $client->getRedirectUri() === ''
+            $client->getRedirectUri() === '' ||
+            (is_array($client->getRedirectUri()) && count($client->getRedirectUri()) !== 1)
         ) {
             $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request));
             throw OAuthServerException::invalidClient($request);
-        } else {
-            $redirectUri = is_array($client->getRedirectUri())
-                ? $client->getRedirectUri()[0]
-                : $client->getRedirectUri();
         }
 
+        $stateParameter = $this->getQueryStringParameter('state', $request);
+
         $scopes = $this->validateScopes(
             $this->getQueryStringParameter('scope', $request, $this->defaultScope),
-            $redirectUri
+            $this->makeRedirectUri(
+                $redirectUri ?? $this->getClientRedirectUri($client),
+                $stateParameter !== null ? ['state' => $stateParameter] : [],
+                $this->queryDelimiter
+            )
         );
 
-        $stateParameter = $this->getQueryStringParameter('state', $request);
-
         $authorizationRequest = $this->createAuthorizationRequest();
         $authorizationRequest->setGrantTypeId($this->getIdentifier());
         $authorizationRequest->setClient($client);
@@ -152,11 +152,8 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth
             throw new LogicException('An instance of UserEntityInterface should be set on the AuthorizationRequest');
         }
 
-        $clientRegisteredRedirectUri = is_array($authorizationRequest->getClient()->getRedirectUri())
-                ? $authorizationRequest->getClient()->getRedirectUri()[0]
-                : $authorizationRequest->getClient()->getRedirectUri();
-
-        $finalRedirectUri = $authorizationRequest->getRedirectUri() ?? $clientRegisteredRedirectUri;
+        $finalRedirectUri = $authorizationRequest->getRedirectUri()
+                          ?? $this->getClientRedirectUri($authorizationRequest->getClient());
 
         // The user approved the client, redirect them back with an access token
         if ($authorizationRequest->isAuthorizationApproved() === true) {
@@ -199,7 +196,8 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth
                 $finalRedirectUri,
                 [
                     'state' => $authorizationRequest->getState(),
-                ]
+                ],
+                $this->queryDelimiter
             )
         );
     }

tests/Grant/AuthCodeGrantTest.php

@@ -469,6 +469,50 @@ public function testValidateAuthorizationRequestInvalidCodeChallengeMethod(): vo
         $grant->validateAuthorizationRequest($request);
     }
 
+    public function testValidateAuthorizationRequestInvalidScopes(): void
+    {
+        $client = new ClientEntity();
+        $client->setRedirectUri(self::REDIRECT_URI);
+        $client->setConfidential();
+
+        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
+
+        $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
+        $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn(null);
+
+        $grant = new AuthCodeGrant(
+            $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(),
+            $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(),
+            new DateInterval('PT10M')
+        );
+
+        $grant->setClientRepository($clientRepositoryMock);
+        $grant->setScopeRepository($scopeRepositoryMock);
+        $grant->setDefaultScope(self::DEFAULT_SCOPE);
+
+        $request = (new ServerRequest())->withQueryParams([
+            'response_type' => 'code',
+            'client_id' => 'foo',
+            'redirect_uri' => self::REDIRECT_URI,
+            'scope' => 'foo',
+            'state' => 'foo',
+        ]);
+
+        try {
+            $grant->validateAuthorizationRequest($request);
+        } catch (OAuthServerException $e) {
+            self::assertSame(5, $e->getCode());
+            self::assertSame('invalid_scope', $e->getErrorType());
+            self::assertSame('https://foo/bar?state=foo', $e->getRedirectUri());
+
+            return;
+        }
+
+        $this->expectException(OAuthServerException::class);
+        $this->expectExceptionCode(5);
+    }
+
     public function testCompleteAuthorizationRequest(): void
     {
         $client = new ClientEntity();
@@ -529,6 +573,7 @@ public function testCompleteAuthorizationRequestDenied(): void
         $authRequest->setClient($client);
         $authRequest->setGrantTypeId('authorization_code');
         $authRequest->setUser(new UserEntity());
+        $authRequest->setState('foo');
 
         $authCodeRepository = $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock();
         $authCodeRepository->method('getNewAuthCode')->willReturn(new AuthCodeEntity());
@@ -540,10 +585,18 @@ public function testCompleteAuthorizationRequestDenied(): void
         );
         $grant->setEncryptionKey($this->cryptStub->getKey());
 
+        try {
+            $grant->completeAuthorizationRequest($authRequest);
+        } catch (OAuthServerException $e) {
+            self::assertSame(9, $e->getCode());
+            self::assertSame('access_denied', $e->getErrorType());
+            self::assertSame('http://foo/bar?state=foo', $e->getRedirectUri());
+
+            return;
+        }
+
         $this->expectException(OAuthServerException::class);
         $this->expectExceptionCode(9);
-
-        $grant->completeAuthorizationRequest($authRequest);
     }
 
     public function testRespondToAccessTokenRequest(): void

tests/Grant/ImplicitGrantTest.php

@@ -95,7 +95,7 @@ public function testValidateAuthorizationRequest(): void
         $grant->setDefaultScope(self::DEFAULT_SCOPE);
 
         $request = (new ServerRequest())->withQueryParams([
-            'response_type' => 'code',
+            'response_type' => 'token',
             'client_id'     => 'foo',
             'redirect_uri'  => self::REDIRECT_URI,
         ]);
@@ -120,7 +120,7 @@ public function testValidateAuthorizationRequestRedirectUriArray(): void
         $grant->setDefaultScope(self::DEFAULT_SCOPE);
 
         $request = (new ServerRequest())->withQueryParams([
-            'response_type' => 'code',
+            'response_type' => 'token',
             'client_id' => 'foo',
             'redirect_uri' => self::REDIRECT_URI,
         ]);
@@ -135,7 +135,7 @@ public function testValidateAuthorizationRequestMissingClientId(): void
         $grant = new ImplicitGrant(new DateInterval('PT10M'));
         $grant->setClientRepository($clientRepositoryMock);
 
-        $request = (new ServerRequest())->withQueryParams(['response_type' => 'code']);
+        $request = (new ServerRequest())->withQueryParams(['response_type' => 'token']);
 
         $this->expectException(OAuthServerException::class);
         $this->expectExceptionCode(3);
@@ -152,7 +152,7 @@ public function testValidateAuthorizationRequestInvalidClientId(): void
         $grant->setClientRepository($clientRepositoryMock);
 
         $request = (new ServerRequest())->withQueryParams([
-            'response_type' => 'code',
+            'response_type' => 'token',
             'client_id'     => 'foo',
         ]);
 
@@ -173,7 +173,7 @@ public function testValidateAuthorizationRequestBadRedirectUriString(): void
         $grant->setClientRepository($clientRepositoryMock);
 
         $request = (new ServerRequest())->withQueryParams([
-            'response_type' => 'code',
+            'response_type' => 'token',
             'client_id' => 'foo',
             'redirect_uri' => 'http://bar',
         ]);
@@ -195,7 +195,7 @@ public function testValidateAuthorizationRequestBadRedirectUriArray(): void
         $grant->setClientRepository($clientRepositoryMock);
 
         $request = (new ServerRequest())->withQueryParams([
-            'response_type' => 'code',
+            'response_type' => 'token',
             'client_id'     => 'foo',
             'redirect_uri'  => 'http://bar',
         ]);
@@ -206,6 +206,45 @@ public function testValidateAuthorizationRequestBadRedirectUriArray(): void
         $grant->validateAuthorizationRequest($request);
     }
 
+    public function testValidateAuthorizationRequestInvalidScopes(): void
+    {
+        $client = new ClientEntity();
+        $client->setRedirectUri(self::REDIRECT_URI);
+
+        $clientRepositoryMock = $this->getMockBuilder(ClientRepositoryInterface::class)->getMock();
+        $clientRepositoryMock->method('getClientEntity')->willReturn($client);
+
+        $scopeRepositoryMock = $this->getMockBuilder(ScopeRepositoryInterface::class)->getMock();
+        $scopeRepositoryMock->method('getScopeEntityByIdentifier')->willReturn(null);
+
+        $grant = new ImplicitGrant(new DateInterval('PT10M'));
+
+        $grant->setClientRepository($clientRepositoryMock);
+        $grant->setScopeRepository($scopeRepositoryMock);
+        $grant->setDefaultScope(self::DEFAULT_SCOPE);
+
+        $request = (new ServerRequest())->withQueryParams([
+            'response_type' => 'token',
+            'client_id' => 'foo',
+            'redirect_uri' => self::REDIRECT_URI,
+            'scope' => 'foo',
+            'state' => 'foo',
+        ]);
+
+        try {
+            $grant->validateAuthorizationRequest($request);
+        } catch (OAuthServerException $e) {
+            self::assertSame(5, $e->getCode());
+            self::assertSame('invalid_scope', $e->getErrorType());
+            self::assertSame('https://foo/bar#state=foo', $e->getRedirectUri());
+
+            return;
+        }
+
+        $this->expectException(OAuthServerException::class);
+        $this->expectExceptionCode(5);
+    }
+
     public function testCompleteAuthorizationRequest(): void
     {
         $client = new ClientEntity();
@@ -248,6 +287,7 @@ public function testCompleteAuthorizationRequestDenied(): void
         $authRequest->setClient($client);
         $authRequest->setGrantTypeId('authorization_code');
         $authRequest->setUser(new UserEntity());
+        $authRequest->setState('foo');
 
         $accessTokenRepositoryMock = $this->getMockBuilder(AccessTokenRepositoryInterface::class)->getMock();
         $accessTokenRepositoryMock->method('getNewToken')->willReturn(new AccessTokenEntity());
@@ -261,10 +301,18 @@ public function testCompleteAuthorizationRequestDenied(): void
         $grant->setAccessTokenRepository($accessTokenRepositoryMock);
         $grant->setScopeRepository($scopeRepositoryMock);
 
+        try {
+            $grant->completeAuthorizationRequest($authRequest);
+        } catch (OAuthServerException $e) {
+            self::assertSame(9, $e->getCode());
+            self::assertSame('access_denied', $e->getErrorType());
+            self::assertSame('https://foo/bar#state=foo', $e->getRedirectUri());
+
+            return;
+        }
+
         $this->expectException(OAuthServerException::class);
         $this->expectExceptionCode(9);
-
-        $grant->completeAuthorizationRequest($authRequest);
     }
 
     public function testAccessTokenRepositoryUniqueConstraintCheck(): void