Skip to content

Commit 3a3401f

Browse files
[13.x] Fix skipping authorization consent when no scopes are requested (#1825)
* fix skipping authorization consent when no scope is requested * add test * add more tests
1 parent 988779d commit 3a3401f

File tree

2 files changed

+106
-4
lines changed

2 files changed

+106
-4
lines changed

src/Http/Controllers/AuthorizationController.php

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,14 +114,25 @@ protected function parseScopes(AuthorizationRequestInterface $authRequest): arra
114114
*/
115115
protected function hasGrantedScopes(Authenticatable $user, Client $client, array $scopes): bool
116116
{
117-
$tokensScopes = $client->tokens()->where([
117+
$activeTokens = $client->tokens()->where([
118118
['user_id', '=', $user->getAuthIdentifier()],
119119
['revoked', '=', false],
120120
['expires_at', '>', Date::now()],
121-
])->pluck('scopes')->flatten();
121+
]);
122+
123+
// If no specific scope is requested, we'll simply check whether the given
124+
// user has any active tokens that grant access to the specified client
125+
// In this case, comparing the granted scopes is no longer necessary.
126+
if (empty($scopes)) {
127+
return $activeTokens->exists();
128+
}
122129

123-
return $tokensScopes->isNotEmpty() &&
124-
collect($scopes)->pluck('id')->diff($tokensScopes)->isEmpty();
130+
// Otherwise, we list all previously granted scopes from the active tokens
131+
// of the given user that authorize access to the specified client, and
132+
// check whether the newly requested scopes are included in that set.
133+
return collect($scopes)->pluck('id')->diff(
134+
$activeTokens->pluck('scopes')->flatten()
135+
)->isEmpty();
125136
}
126137

127138
/**

tests/Feature/AuthorizationCodeGrantTest.php

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,97 @@ public function testSkipsAuthorizationWhenHasGrantedScopes()
163163
$this->assertArrayHasKey('code', $params);
164164
}
165165

166+
public function testSkipsAuthorizationWhenHasActiveTokensAndEmptyScope()
167+
{
168+
$client = ClientFactory::new()->create();
169+
170+
$query = http_build_query([
171+
'client_id' => $client->getKey(),
172+
'redirect_uri' => $redirect = $client->redirect_uris[0],
173+
'response_type' => 'code',
174+
'scope' => '',
175+
'state' => Str::random(40),
176+
]);
177+
178+
$user = UserFactory::new()->create();
179+
$this->actingAs($user, 'web');
180+
$json = $this->get('/oauth/authorize?'.$query)->json();
181+
182+
$response = $this->post('/oauth/authorize', ['auth_token' => $json['authToken']]);
183+
parse_str(parse_url($response->headers->get('Location'), PHP_URL_QUERY), $params);
184+
185+
$this->post('/oauth/token', [
186+
'grant_type' => 'authorization_code',
187+
'client_id' => $client->getKey(),
188+
'client_secret' => $client->plainSecret,
189+
'redirect_uri' => $redirect,
190+
'code' => $params['code'],
191+
])->assertOk();
192+
193+
$query = http_build_query([
194+
'client_id' => $client->getKey(),
195+
'redirect_uri' => $redirect,
196+
'response_type' => 'code',
197+
'scope' => '',
198+
'state' => $state = Str::random(40),
199+
]);
200+
201+
$response = $this->get('/oauth/authorize?'.$query);
202+
$response->assertRedirect();
203+
204+
$location = $response->headers->get('Location');
205+
parse_str(parse_url($location, PHP_URL_QUERY), $params);
206+
207+
$this->assertStringStartsWith($redirect.'?', $location);
208+
$this->assertSame($state, $params['state']);
209+
$this->assertArrayHasKey('code', $params);
210+
}
211+
212+
public function testPromptConsentForNewScope()
213+
{
214+
$client = ClientFactory::new()->create();
215+
216+
$query = http_build_query([
217+
'client_id' => $client->getKey(),
218+
'redirect_uri' => $redirect = $client->redirect_uris[0],
219+
'response_type' => 'code',
220+
'scope' => 'create read',
221+
'state' => Str::random(40),
222+
]);
223+
224+
$user = UserFactory::new()->create();
225+
$this->actingAs($user, 'web');
226+
$json = $this->get('/oauth/authorize?'.$query)->json();
227+
228+
$response = $this->post('/oauth/authorize', ['auth_token' => $json['authToken']]);
229+
parse_str(parse_url($response->headers->get('Location'), PHP_URL_QUERY), $params);
230+
231+
$this->post('/oauth/token', [
232+
'grant_type' => 'authorization_code',
233+
'client_id' => $client->getKey(),
234+
'client_secret' => $client->plainSecret,
235+
'redirect_uri' => $redirect,
236+
'code' => $params['code'],
237+
])->assertOk();
238+
239+
$query = http_build_query([
240+
'client_id' => $client->getKey(),
241+
'redirect_uri' => $redirect,
242+
'response_type' => 'code',
243+
'scope' => 'create update',
244+
'state' => $state = Str::random(40),
245+
]);
246+
247+
$response = $this->get('/oauth/authorize?'.$query);
248+
249+
$response->assertOk();
250+
$response->assertSessionHas('authRequest');
251+
$response->assertSessionHas('authToken');
252+
$json = $response->json();
253+
$this->assertEqualsCanonicalizing(['client', 'user', 'scopes', 'request', 'authToken'], array_keys($json));
254+
$this->assertSame(collect(Passport::scopesFor(['create', 'update']))->toArray(), $json['scopes']);
255+
}
256+
166257
public function testValidateAuthorizationRequest()
167258
{
168259
$client = ClientFactory::new()->create();

0 commit comments

Comments
 (0)