From ebf0bce5fcc7c05117b48b95533f9cbab1d59e3f Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Wed, 4 Jun 2025 12:15:09 +0330 Subject: [PATCH 1/3] fix skipping authorization consent when no scope is requested --- .../Controllers/AuthorizationController.php | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Http/Controllers/AuthorizationController.php b/src/Http/Controllers/AuthorizationController.php index 7e1a5202..fe46dd0d 100644 --- a/src/Http/Controllers/AuthorizationController.php +++ b/src/Http/Controllers/AuthorizationController.php @@ -114,14 +114,25 @@ protected function parseScopes(AuthorizationRequestInterface $authRequest): arra */ protected function hasGrantedScopes(Authenticatable $user, Client $client, array $scopes): bool { - $tokensScopes = $client->tokens()->where([ + $activeTokens = $client->tokens()->where([ ['user_id', '=', $user->getAuthIdentifier()], ['revoked', '=', false], ['expires_at', '>', Date::now()], - ])->pluck('scopes')->flatten(); + ]); + + // If no specific scope is requested, we'll simply check whether the given + // user has any active tokens that grant access to the specified client + // In this case, comparing the granted scopes is no longer necessary. + if (empty($scopes)) { + return $activeTokens->exists(); + } - return $tokensScopes->isNotEmpty() && - collect($scopes)->pluck('id')->diff($tokensScopes)->isEmpty(); + // Otherwise, we list all previously granted scopes from the active tokens + // of the given user that authorize access to the specified client, and + // check whether the newly requested scopes are included in that set. + return collect($scopes)->pluck('id')->diff( + $activeTokens->pluck('scopes')->flatten() + )->isEmpty(); } /** From 7732ad58eff0ac6229a4d03a10e46c9a922dda41 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Wed, 4 Jun 2025 12:15:15 +0330 Subject: [PATCH 2/3] add test --- tests/Feature/AuthorizationCodeGrantTest.php | 46 ++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/Feature/AuthorizationCodeGrantTest.php b/tests/Feature/AuthorizationCodeGrantTest.php index 7a9cb31b..119431c4 100644 --- a/tests/Feature/AuthorizationCodeGrantTest.php +++ b/tests/Feature/AuthorizationCodeGrantTest.php @@ -163,6 +163,52 @@ public function testSkipsAuthorizationWhenHasGrantedScopes() $this->assertArrayHasKey('code', $params); } + public function testSkipsAuthorizationWhenHasActiveTokensAndEmptyScope() + { + $client = ClientFactory::new()->create(); + + $query = http_build_query([ + 'client_id' => $client->getKey(), + 'redirect_uri' => $redirect = $client->redirect_uris[0], + 'response_type' => 'code', + 'scope' => '', + 'state' => Str::random(40), + ]); + + $user = UserFactory::new()->create(); + $this->actingAs($user, 'web'); + $json = $this->get('/oauth/authorize?'.$query)->json(); + + $response = $this->post('/oauth/authorize', ['auth_token' => $json['authToken']]); + parse_str(parse_url($response->headers->get('Location'), PHP_URL_QUERY), $params); + + $this->post('/oauth/token', [ + 'grant_type' => 'authorization_code', + 'client_id' => $client->getKey(), + 'client_secret' => $client->plainSecret, + 'redirect_uri' => $redirect, + 'code' => $params['code'], + ])->assertOk(); + + $query = http_build_query([ + 'client_id' => $client->getKey(), + 'redirect_uri' => $redirect, + 'response_type' => 'code', + 'scope' => '', + 'state' => $state = Str::random(40), + ]); + + $response = $this->get('/oauth/authorize?'.$query); + $response->assertRedirect(); + + $location = $response->headers->get('Location'); + parse_str(parse_url($location, PHP_URL_QUERY), $params); + + $this->assertStringStartsWith($redirect.'?', $location); + $this->assertSame($state, $params['state']); + $this->assertArrayHasKey('code', $params); + } + public function testValidateAuthorizationRequest() { $client = ClientFactory::new()->create(); From eef8a48fd284e1b09205a2b5326e0a2e84978a09 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Wed, 4 Jun 2025 12:41:06 +0330 Subject: [PATCH 3/3] add more tests --- tests/Feature/AuthorizationCodeGrantTest.php | 45 ++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/Feature/AuthorizationCodeGrantTest.php b/tests/Feature/AuthorizationCodeGrantTest.php index 119431c4..178f2778 100644 --- a/tests/Feature/AuthorizationCodeGrantTest.php +++ b/tests/Feature/AuthorizationCodeGrantTest.php @@ -209,6 +209,51 @@ public function testSkipsAuthorizationWhenHasActiveTokensAndEmptyScope() $this->assertArrayHasKey('code', $params); } + public function testPromptConsentForNewScope() + { + $client = ClientFactory::new()->create(); + + $query = http_build_query([ + 'client_id' => $client->getKey(), + 'redirect_uri' => $redirect = $client->redirect_uris[0], + 'response_type' => 'code', + 'scope' => 'create read', + 'state' => Str::random(40), + ]); + + $user = UserFactory::new()->create(); + $this->actingAs($user, 'web'); + $json = $this->get('/oauth/authorize?'.$query)->json(); + + $response = $this->post('/oauth/authorize', ['auth_token' => $json['authToken']]); + parse_str(parse_url($response->headers->get('Location'), PHP_URL_QUERY), $params); + + $this->post('/oauth/token', [ + 'grant_type' => 'authorization_code', + 'client_id' => $client->getKey(), + 'client_secret' => $client->plainSecret, + 'redirect_uri' => $redirect, + 'code' => $params['code'], + ])->assertOk(); + + $query = http_build_query([ + 'client_id' => $client->getKey(), + 'redirect_uri' => $redirect, + 'response_type' => 'code', + 'scope' => 'create update', + 'state' => $state = Str::random(40), + ]); + + $response = $this->get('/oauth/authorize?'.$query); + + $response->assertOk(); + $response->assertSessionHas('authRequest'); + $response->assertSessionHas('authToken'); + $json = $response->json(); + $this->assertEqualsCanonicalizing(['client', 'user', 'scopes', 'request', 'authToken'], array_keys($json)); + $this->assertSame(collect(Passport::scopesFor(['create', 'update']))->toArray(), $json['scopes']); + } + public function testValidateAuthorizationRequest() { $client = ClientFactory::new()->create();