Skip to content

Commit 61644b3

Browse files
hafezdivandarihafezdivandari
authored
[13.x] Fix skipping consent prompt (#1777)
* fix skip auth if user already consented * fix tests * formatting * formatting * formatting * formatting
1 parent 76820f7 commit 61644b3

File tree

3 files changed

+37
-57
lines changed

3 files changed

+37
-57
lines changed

‎src/Client.php

+3-2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Laravel\Passport;
44

5+
use Illuminate\Contracts\Auth\Authenticatable;
56
use Illuminate\Database\Eloquent\Casts\Attribute;
67
use Illuminate\Database\Eloquent\Factories\HasFactory;
78
use Illuminate\Database\Eloquent\Model;
@@ -163,9 +164,9 @@ public function firstParty()
163164
/**
164165
* Determine if the client should skip the authorization prompt.
165166
*
166-
* @return bool
167+
* @param \Laravel\Passport\Scope[] $scopes
167168
*/
168-
public function skipsAuthorization()
169+
public function skipsAuthorization(Authenticatable $user, array $scopes): bool
169170
{
170171
return false;
171172
}

‎src/Http/Controllers/AuthorizationController.php

+14-15
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@
44

55
use Illuminate\Contracts\Auth\StatefulGuard;
66
use Illuminate\Http\Request;
7+
use Illuminate\Support\Facades\Date;
78
use Illuminate\Support\Str;
89
use Laravel\Passport\Bridge\User;
910
use Laravel\Passport\ClientRepository;
1011
use Laravel\Passport\Contracts\AuthorizationViewResponse;
1112
use Laravel\Passport\Exceptions\AuthenticationException;
1213
use Laravel\Passport\Passport;
13-
use Laravel\Passport\TokenRepository;
1414
use League\OAuth2\Server\AuthorizationServer;
1515
use League\OAuth2\Server\Exception\OAuthServerException;
1616
use Nyholm\Psr7\Response as Psr7Response;
@@ -64,22 +64,18 @@ public function __construct(AuthorizationServer $server,
6464
* @param \Psr\Http\Message\ServerRequestInterface $psrRequest
6565
* @param \Illuminate\Http\Request $request
6666
* @param \Laravel\Passport\ClientRepository $clients
67-
* @param \Laravel\Passport\TokenRepository $tokens
6867
* @return \Illuminate\Http\Response|\Laravel\Passport\Contracts\AuthorizationViewResponse
6968
*/
70-
public function authorize(ServerRequestInterface $psrRequest,
71-
Request $request,
72-
ClientRepository $clients,
73-
TokenRepository $tokens)
69+
public function authorize(ServerRequestInterface $psrRequest, Request $request, ClientRepository $clients)
7470
{
7571
$authRequest = $this->withErrorHandling(function () use ($psrRequest) {
7672
return $this->server->validateAuthorizationRequest($psrRequest);
7773
});
7874

7975
if ($this->guard->guest()) {
8076
return $request->get('prompt') === 'none'
81-
? $this->denyRequest($authRequest)
82-
: $this->promptForLogin($request);
77+
? $this->denyRequest($authRequest)
78+
: $this->promptForLogin($request);
8379
}
8480

8581
if ($request->get('prompt') === 'login' &&
@@ -98,7 +94,7 @@ public function authorize(ServerRequestInterface $psrRequest,
9894
$client = $clients->find($authRequest->getClient()->getIdentifier());
9995

10096
if ($request->get('prompt') !== 'consent' &&
101-
($client->skipsAuthorization() || $this->hasValidToken($tokens, $user, $client, $scopes))) {
97+
($client->skipsAuthorization($user, $scopes) || $this->hasGrantedScopes($user, $client, $scopes))) {
10298
return $this->approveRequest($authRequest, $user);
10399
}
104100

@@ -134,19 +130,22 @@ protected function parseScopes($authRequest)
134130
}
135131

136132
/**
137-
* Determine if a valid token exists for the given user, client, and scopes.
133+
* Determine if the given user has already granted the client access to the scopes.
138134
*
139-
* @param \Laravel\Passport\TokenRepository $tokens
140135
* @param \Illuminate\Contracts\Auth\Authenticatable $user
141136
* @param \Laravel\Passport\Client $client
142137
* @param array $scopes
143138
* @return bool
144139
*/
145-
protected function hasValidToken($tokens, $user, $client, $scopes)
140+
protected function hasGrantedScopes($user, $client, $scopes)
146141
{
147-
$token = $tokens->findValidToken($user, $client);
148-
149-
return $token && $token->scopes === collect($scopes)->pluck('id')->all();
142+
return collect($scopes)->pluck('id')->diff(
143+
$client->tokens()->where([
144+
['user_id', '=', $user->getAuthIdentifier()],
145+
['revoked', '=', false],
146+
['expires_at', '>', Date::now()],
147+
])->pluck('scopes')->flatten()
148+
)->isEmpty();
150149
}
151150

152151
/**

‎tests/Unit/AuthorizationControllerTest.php

+20-40
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Laravel\Passport\Tests\Unit;
44

5+
use Illuminate\Contracts\Auth\Authenticatable;
56
use Illuminate\Contracts\Auth\StatefulGuard;
67
use Illuminate\Http\Request;
78
use Laravel\Passport\Bridge\Scope;
@@ -12,8 +13,6 @@
1213
use Laravel\Passport\Http\Controllers\AuthorizationController;
1314
use Laravel\Passport\Http\Responses\AuthorizationViewResponse;
1415
use Laravel\Passport\Passport;
15-
use Laravel\Passport\Token;
16-
use Laravel\Passport\TokenRepository;
1716
use League\OAuth2\Server\AuthorizationServer;
1817
use League\OAuth2\Server\Exception\OAuthServerException as LeagueException;
1918
use League\OAuth2\Server\RequestTypes\AuthorizationRequest;
@@ -44,7 +43,7 @@ public function test_authorization_view_is_presented()
4443
$controller = new AuthorizationController($server, $guard, $response);
4544

4645
$guard->shouldReceive('guest')->andReturn(false);
47-
$guard->shouldReceive('user')->andReturn($user = m::mock());
46+
$guard->shouldReceive('user')->andReturn($user = m::mock(Authenticatable::class));
4847
$server->shouldReceive('validateAuthorizationRequest')->andReturn($authRequest = m::mock(AuthorizationRequestInterface::class));
4948

5049
$request = m::mock(Request::class);
@@ -60,9 +59,9 @@ public function test_authorization_view_is_presented()
6059
$clients = m::mock(ClientRepository::class);
6160
$clients->shouldReceive('find')->with(1)->andReturn($client = m::mock(Client::class));
6261
$client->shouldReceive('skipsAuthorization')->andReturn(false);
62+
$client->shouldReceive('tokens->where->pluck->flatten')->andReturn(collect());
6363

64-
$tokens = m::mock(TokenRepository::class);
65-
$tokens->shouldReceive('findValidToken')->with($user, $client)->andReturnNull();
64+
$user->shouldReceive('getAuthIdentifier')->andReturn(1);
6665

6766
$response->shouldReceive('withParameters')->once()->andReturnUsing(function ($data) use ($client, $user, $request) {
6867
$this->assertEquals($client, $data['client']);
@@ -74,7 +73,7 @@ public function test_authorization_view_is_presented()
7473
});
7574

7675
$this->assertSame('view', $controller->authorize(
77-
m::mock(ServerRequestInterface::class), $request, $clients, $tokens
76+
m::mock(ServerRequestInterface::class), $request, $clients
7877
));
7978
}
8079

@@ -93,12 +92,11 @@ public function test_authorization_exceptions_are_handled()
9392
$request->shouldReceive('session')->andReturn($session = m::mock());
9493

9594
$clients = m::mock(ClientRepository::class);
96-
$tokens = m::mock(TokenRepository::class);
9795

9896
$this->expectException(OAuthServerException::class);
9997

10098
$controller->authorize(
101-
m::mock(ServerRequestInterface::class), $request, $clients, $tokens
99+
m::mock(ServerRequestInterface::class), $request, $clients
102100
);
103101
}
104102

@@ -115,7 +113,7 @@ public function test_request_is_approved_if_valid_token_exists()
115113
$controller = new AuthorizationController($server, $guard, $response);
116114

117115
$guard->shouldReceive('guest')->andReturn(false);
118-
$guard->shouldReceive('user')->andReturn($user = m::mock());
116+
$guard->shouldReceive('user')->andReturn($user = m::mock(Authenticatable::class));
119117
$psrResponse = new Response();
120118
$psrResponse->getBody()->write('approved');
121119
$server->shouldReceive('validateAuthorizationRequest')
@@ -140,15 +138,11 @@ public function test_request_is_approved_if_valid_token_exists()
140138
$clients->shouldReceive('find')->with(1)->andReturn($client = m::mock(Client::class));
141139

142140
$client->shouldReceive('skipsAuthorization')->andReturn(false);
143-
144-
$tokens = m::mock(TokenRepository::class);
145-
$tokens->shouldReceive('findValidToken')
146-
->with($user, $client)
147-
->andReturn($token = m::mock(Token::class));
148-
$token->shouldReceive('getAttribute')->with('scopes')->andReturn(['scope-1']);
141+
$client->shouldReceive('getKey')->andReturn(1);
142+
$client->shouldReceive('tokens->where->pluck->flatten')->andReturn(collect(['scope-1']));
149143

150144
$this->assertSame('approved', $controller->authorize(
151-
m::mock(ServerRequestInterface::class), $request, $clients, $tokens
145+
m::mock(ServerRequestInterface::class), $request, $clients
152146
)->getContent());
153147
}
154148

@@ -165,7 +159,7 @@ public function test_request_is_approved_if_client_can_skip_authorization()
165159
$controller = new AuthorizationController($server, $guard, $response);
166160

167161
$guard->shouldReceive('guest')->andReturn(false);
168-
$guard->shouldReceive('user')->andReturn($user = m::mock());
162+
$guard->shouldReceive('user')->andReturn($user = m::mock(Authenticatable::class));
169163
$psrResponse = new Response();
170164
$psrResponse->getBody()->write('approved');
171165
$server->shouldReceive('validateAuthorizationRequest')
@@ -191,13 +185,8 @@ public function test_request_is_approved_if_client_can_skip_authorization()
191185

192186
$client->shouldReceive('skipsAuthorization')->andReturn(true);
193187

194-
$tokens = m::mock(TokenRepository::class);
195-
$tokens->shouldReceive('findValidToken')
196-
->with($user, $client)
197-
->andReturnNull();
198-
199188
$this->assertSame('approved', $controller->authorize(
200-
m::mock(ServerRequestInterface::class), $request, $clients, $tokens
189+
m::mock(ServerRequestInterface::class), $request, $clients
201190
)->getContent());
202191
}
203192

@@ -232,9 +221,6 @@ public function test_authorization_view_is_presented_if_request_has_prompt_equal
232221
$clients->shouldReceive('find')->with(1)->andReturn($client = m::mock(Client::class));
233222
$client->shouldReceive('skipsAuthorization')->andReturn(false);
234223

235-
$tokens = m::mock(TokenRepository::class);
236-
$tokens->shouldNotReceive('findValidToken');
237-
238224
$response->shouldReceive('withParameters')->once()->andReturnUsing(function ($data) use ($client, $user, $request) {
239225
$this->assertEquals($client, $data['client']);
240226
$this->assertEquals($user, $data['user']);
@@ -245,7 +231,7 @@ public function test_authorization_view_is_presented_if_request_has_prompt_equal
245231
});
246232

247233
$this->assertSame('view', $controller->authorize(
248-
m::mock(ServerRequestInterface::class), $request, $clients, $tokens
234+
m::mock(ServerRequestInterface::class), $request, $clients
249235
));
250236
}
251237

@@ -264,7 +250,7 @@ public function test_authorization_denied_if_request_has_prompt_equals_to_none()
264250
$controller = new AuthorizationController($server, $guard, $response);
265251

266252
$guard->shouldReceive('guest')->andReturn(false);
267-
$guard->shouldReceive('user')->andReturn($user = m::mock());
253+
$guard->shouldReceive('user')->andReturn($user = m::mock(Authenticatable::class));
268254
$server->shouldReceive('validateAuthorizationRequest')
269255
->andReturn($authRequest = m::mock(AuthorizationRequest::class));
270256
$server->shouldReceive('completeAuthorizationRequest')
@@ -288,14 +274,11 @@ public function test_authorization_denied_if_request_has_prompt_equals_to_none()
288274
$clients = m::mock(ClientRepository::class);
289275
$clients->shouldReceive('find')->with(1)->andReturn($client = m::mock(Client::class));
290276
$client->shouldReceive('skipsAuthorization')->andReturn(false);
291-
292-
$tokens = m::mock(TokenRepository::class);
293-
$tokens->shouldReceive('findValidToken')
294-
->with($user, $client)
295-
->andReturnNull();
277+
$client->shouldReceive('getKey')->andReturn(1);
278+
$client->shouldReceive('tokens->where->pluck->flatten')->andReturn(collect());
296279

297280
$controller->authorize(
298-
m::mock(ServerRequestInterface::class), $request, $clients, $tokens
281+
m::mock(ServerRequestInterface::class), $request, $clients
299282
);
300283
}
301284

@@ -324,11 +307,10 @@ public function test_authorization_denied_if_unauthenticated_and_request_has_pro
324307
$authRequest->shouldReceive('getGrantTypeId')->andReturn('authorization_code');
325308

326309
$clients = m::mock(ClientRepository::class);
327-
$tokens = m::mock(TokenRepository::class);
328310

329311
try {
330312
$controller->authorize(
331-
m::mock(ServerRequestInterface::class), $request, $clients, $tokens
313+
m::mock(ServerRequestInterface::class), $request, $clients
332314
);
333315
} catch (\Laravel\Passport\Exceptions\OAuthServerException $e) {
334316
$this->assertStringStartsWith(
@@ -362,10 +344,9 @@ public function test_logout_and_prompt_login_if_request_has_prompt_equals_to_log
362344
$request->shouldReceive('get')->with('prompt')->andReturn('login');
363345

364346
$clients = m::mock(ClientRepository::class);
365-
$tokens = m::mock(TokenRepository::class);
366347

367348
$controller->authorize(
368-
m::mock(ServerRequestInterface::class), $request, $clients, $tokens
349+
m::mock(ServerRequestInterface::class), $request, $clients
369350
);
370351
}
371352

@@ -390,10 +371,9 @@ public function test_user_should_be_authenticated()
390371
$request->shouldReceive('get')->with('prompt')->andReturn(null);
391372

392373
$clients = m::mock(ClientRepository::class);
393-
$tokens = m::mock(TokenRepository::class);
394374

395375
$controller->authorize(
396-
m::mock(ServerRequestInterface::class), $request, $clients, $tokens
376+
m::mock(ServerRequestInterface::class), $request, $clients
397377
);
398378
}
399379
}

0 commit comments

Comments
 (0)