From 8a79632e7ceabec2c84a49e16cbd686b7bd76493 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sat, 18 May 2024 00:17:49 +0330 Subject: [PATCH 1/6] deny access if user is not authenticated --- src/Grant/AuthCodeGrant.php | 21 ++++++++++++++++---- src/Grant/ImplicitGrant.php | 38 ++++++++++++++++++++++++++++--------- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 8a24a8e95..29013118d 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -347,15 +347,15 @@ function ($method) { */ public function completeAuthorizationRequest(AuthorizationRequestInterface $authorizationRequest): ResponseTypeInterface { - if ($authorizationRequest->getUser() instanceof UserEntityInterface === false) { - throw new LogicException('An instance of UserEntityInterface should be set on the AuthorizationRequest'); - } - $finalRedirectUri = $authorizationRequest->getRedirectUri() ?? $this->getClientRedirectUri($authorizationRequest); // The user approved the client, redirect them back with an auth code if ($authorizationRequest->isAuthorizationApproved() === true) { + if ($authorizationRequest->getUser() instanceof UserEntityInterface === false) { + throw new LogicException('An instance of UserEntityInterface should be set on the AuthorizationRequest'); + } + $authCode = $this->issueAuthCode( $this->authCodeTTL, $authorizationRequest->getClient(), @@ -395,6 +395,19 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth return $response; } + // The user is not authenticated, redirect them back with an error + if (is_null($authorizationRequest->getUser())) { + throw OAuthServerException::accessDenied( + 'The user is not authenticated.', + $this->makeRedirectUri( + $finalRedirectUri, + [ + 'state' => $authorizationRequest->getState(), + ] + ) + ); + } + // The user denied the client, redirect them back with an error throw OAuthServerException::accessDenied( 'The user denied the request', diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 7723ab8b1..536aea28d 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -148,18 +148,15 @@ public function validateAuthorizationRequest(ServerRequestInterface $request): A */ public function completeAuthorizationRequest(AuthorizationRequestInterface $authorizationRequest): ResponseTypeInterface { - if ($authorizationRequest->getUser() instanceof UserEntityInterface === false) { - 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); // The user approved the client, redirect them back with an access token if ($authorizationRequest->isAuthorizationApproved() === true) { + if ($authorizationRequest->getUser() instanceof UserEntityInterface === false) { + throw new LogicException('An instance of UserEntityInterface should be set on the AuthorizationRequest'); + } + // Finalize the requested scopes $finalizedScopes = $this->scopeRepository->finalizeScopes( $authorizationRequest->getScopes(), @@ -192,6 +189,19 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth return $response; } + // The user is not authenticated, redirect them back with an error + if (is_null($authorizationRequest->getUser())) { + throw OAuthServerException::accessDenied( + 'The user is not authenticated.', + $this->makeRedirectUri( + $finalRedirectUri, + [ + 'state' => $authorizationRequest->getState(), + ] + ) + ); + } + // The user denied the client, redirect them back with an error throw OAuthServerException::accessDenied( 'The user denied the request', @@ -203,4 +213,14 @@ 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(); + } } From e96eb7762c4f245f5ab6dfe9d26f8cce0f64ff89 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sat, 18 May 2024 00:46:15 +0330 Subject: [PATCH 2/6] formatting --- src/Grant/AuthCodeGrant.php | 17 +++-------------- src/Grant/ImplicitGrant.php | 17 +++-------------- 2 files changed, 6 insertions(+), 28 deletions(-) diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 29013118d..c8e0a9d95 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -395,22 +395,11 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth return $response; } - // The user is not authenticated, redirect them back with an error - if (is_null($authorizationRequest->getUser())) { - throw OAuthServerException::accessDenied( - 'The user is not authenticated.', - $this->makeRedirectUri( - $finalRedirectUri, - [ - 'state' => $authorizationRequest->getState(), - ] - ) - ); - } - // The user denied the client, redirect them back with an error throw OAuthServerException::accessDenied( - 'The user denied the request', + is_null($authorizationRequest->getUser()) + ? 'The user denied the request' + : 'The user is not authenticated.', $this->makeRedirectUri( $finalRedirectUri, [ diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 536aea28d..45dbb2632 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -189,22 +189,11 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth return $response; } - // The user is not authenticated, redirect them back with an error - if (is_null($authorizationRequest->getUser())) { - throw OAuthServerException::accessDenied( - 'The user is not authenticated.', - $this->makeRedirectUri( - $finalRedirectUri, - [ - 'state' => $authorizationRequest->getState(), - ] - ) - ); - } - // The user denied the client, redirect them back with an error throw OAuthServerException::accessDenied( - 'The user denied the request', + is_null($authorizationRequest->getUser()) + ? 'The user denied the request' + : 'The user is not authenticated.', $this->makeRedirectUri( $finalRedirectUri, [ From c3e3cbe15c61f80cee146b636240632b8a9911de Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sat, 18 May 2024 00:52:00 +0330 Subject: [PATCH 3/6] formatting --- src/Grant/AbstractAuthorizeGrant.php | 10 ++++++++++ src/Grant/AuthCodeGrant.php | 10 ---------- src/Grant/ImplicitGrant.php | 10 ---------- 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/Grant/AbstractAuthorizeGrant.php b/src/Grant/AbstractAuthorizeGrant.php index 22b58c4e2..389873ead 100644 --- a/src/Grant/AbstractAuthorizeGrant.php +++ b/src/Grant/AbstractAuthorizeGrant.php @@ -35,4 +35,14 @@ protected function createAuthorizationRequest(): AuthorizationRequestInterface { return new AuthorizationRequest(); } + + /** + * Get the client redirect URI. + */ + protected function getClientRedirectUri(AuthorizationRequestInterface $authorizationRequest): string + { + return is_array($authorizationRequest->getClient()->getRedirectUri()) + ? $authorizationRequest->getClient()->getRedirectUri()[0] + : $authorizationRequest->getClient()->getRedirectUri(); + } } diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index c8e0a9d95..a4901da80 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -408,14 +408,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(); - } } diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 45dbb2632..6cc23067f 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -202,14 +202,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(); - } } From 7b1754d8722db457ea45bd61c9e42e22b0c37db1 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sat, 18 May 2024 00:54:41 +0330 Subject: [PATCH 4/6] formatting --- src/Grant/AuthCodeGrant.php | 4 ++-- src/Grant/ImplicitGrant.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index a4901da80..42f1abe6f 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -398,8 +398,8 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth // The user denied the client, redirect them back with an error throw OAuthServerException::accessDenied( is_null($authorizationRequest->getUser()) - ? 'The user denied the request' - : 'The user is not authenticated.', + ? 'The user is not authenticated.' + : 'The user denied the request', $this->makeRedirectUri( $finalRedirectUri, [ diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 6cc23067f..2f9df5d59 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -192,8 +192,8 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth // The user denied the client, redirect them back with an error throw OAuthServerException::accessDenied( is_null($authorizationRequest->getUser()) - ? 'The user denied the request' - : 'The user is not authenticated.', + ? 'The user is not authenticated.' + : 'The user denied the request', $this->makeRedirectUri( $finalRedirectUri, [ From 75e96e6ecb88fe0288e3293007be6bd21f928964 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 29 Aug 2024 17:58:33 +0330 Subject: [PATCH 5/6] formatting --- src/Grant/AuthCodeGrant.php | 4 +--- src/Grant/ImplicitGrant.php | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Grant/AuthCodeGrant.php b/src/Grant/AuthCodeGrant.php index 42f1abe6f..ac6967501 100644 --- a/src/Grant/AuthCodeGrant.php +++ b/src/Grant/AuthCodeGrant.php @@ -397,9 +397,7 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth // The user denied the client, redirect them back with an error throw OAuthServerException::accessDenied( - is_null($authorizationRequest->getUser()) - ? 'The user is not authenticated.' - : 'The user denied the request', + 'The user denied the request', $this->makeRedirectUri( $finalRedirectUri, [ diff --git a/src/Grant/ImplicitGrant.php b/src/Grant/ImplicitGrant.php index 2f9df5d59..02e4f4f5b 100644 --- a/src/Grant/ImplicitGrant.php +++ b/src/Grant/ImplicitGrant.php @@ -191,9 +191,7 @@ public function completeAuthorizationRequest(AuthorizationRequestInterface $auth // The user denied the client, redirect them back with an error throw OAuthServerException::accessDenied( - is_null($authorizationRequest->getUser()) - ? 'The user is not authenticated.' - : 'The user denied the request', + 'The user denied the request', $this->makeRedirectUri( $finalRedirectUri, [ From 9fd553d8ba938a4b948c3cc05c47bbcea980a6e8 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Thu, 29 Aug 2024 17:58:48 +0330 Subject: [PATCH 6/6] fix tests --- tests/Grant/AuthCodeGrantTest.php | 32 ++++++++++++++++++++++++++++++- tests/Grant/ImplicitGrantTest.php | 28 ++++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/tests/Grant/AuthCodeGrantTest.php b/tests/Grant/AuthCodeGrantTest.php index 6a6842661..c8541e03c 100644 --- a/tests/Grant/AuthCodeGrantTest.php +++ b/tests/Grant/AuthCodeGrantTest.php @@ -2186,6 +2186,14 @@ public function testRefreshTokenRepositoryFailToPersistUniqueNoInfiniteLoop(): v public function testCompleteAuthorizationRequestNoUser(): void { + $client = new ClientEntity(); + $client->setRedirectUri('http://foo/bar'); + + $authRequest = new AuthorizationRequest(); + $authRequest->setAuthorizationApproved(true); + $authRequest->setClient($client); + $authRequest->setGrantTypeId('authorization_code'); + $grant = new AuthCodeGrant( $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(), @@ -2194,7 +2202,29 @@ public function testCompleteAuthorizationRequestNoUser(): void $this->expectException(LogicException::class); - $grant->completeAuthorizationRequest(new AuthorizationRequest()); + $grant->completeAuthorizationRequest($authRequest); + } + + public function testCompleteAuthorizationRequestDeniedNoUser(): void + { + $client = new ClientEntity(); + $client->setRedirectUri('http://foo/bar'); + + $authRequest = new AuthorizationRequest(); + $authRequest->setAuthorizationApproved(false); + $authRequest->setClient($client); + $authRequest->setGrantTypeId('authorization_code'); + + $grant = new AuthCodeGrant( + $this->getMockBuilder(AuthCodeRepositoryInterface::class)->getMock(), + $this->getMockBuilder(RefreshTokenRepositoryInterface::class)->getMock(), + new DateInterval('PT10M') + ); + + $this->expectException(OAuthServerException::class); + $this->expectExceptionCode(9); + + $grant->completeAuthorizationRequest($authRequest); } public function testPublicClientAuthCodeRequestRejectedWhenCodeChallengeRequiredButNotGiven(): void diff --git a/tests/Grant/ImplicitGrantTest.php b/tests/Grant/ImplicitGrantTest.php index c2b943197..e5cbbbde7 100644 --- a/tests/Grant/ImplicitGrantTest.php +++ b/tests/Grant/ImplicitGrantTest.php @@ -395,10 +395,36 @@ public function testSetRefreshTokenRepository(): void public function testCompleteAuthorizationRequestNoUser(): void { + $client = new ClientEntity(); + $client->setRedirectUri('https://foo/bar'); + + $authRequest = new AuthorizationRequest(); + $authRequest->setAuthorizationApproved(true); + $authRequest->setClient($client); + $authRequest->setGrantTypeId('authorization_code'); + $grant = new ImplicitGrant(new DateInterval('PT10M')); $this->expectException(LogicException::class); - $grant->completeAuthorizationRequest(new AuthorizationRequest()); + $grant->completeAuthorizationRequest($authRequest); + } + + public function testCompleteAuthorizationRequestDeniedNoUser(): void + { + $client = new ClientEntity(); + $client->setRedirectUri('https://foo/bar'); + + $authRequest = new AuthorizationRequest(); + $authRequest->setAuthorizationApproved(false); + $authRequest->setClient($client); + $authRequest->setGrantTypeId('authorization_code'); + + $grant = new ImplicitGrant(new DateInterval('PT10M')); + + $this->expectException(OAuthServerException::class); + $this->expectExceptionCode(9); + + $grant->completeAuthorizationRequest($authRequest); } }