Skip to content

Commit d16746c

Browse files
hafezdivandarihafezdivandari
authored
[13.x] Improve performance, fix minor bugs, and add tests (#1783)
* improve finalizing scopes * fix tests * enhance access token * make client repository singleton * fix dependencies * add tests * formatting * formatting * formatting * add tests for php84 * test * test * formatting * remove php 8.4 for now
1 parent 6142f9c commit d16746c

File tree

12 files changed

+351
-116
lines changed

12 files changed

+351
-116
lines changed

‎.github/workflows/tests.yml‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,13 @@ on:
1111

1212
jobs:
1313
tests:
14-
runs-on: ubuntu-22.04
14+
runs-on: ubuntu-latest
1515

1616
strategy:
1717
fail-fast: true
1818
matrix:
19-
php: [8.1, 8.2, 8.3]
20-
laravel: [10, 11]
21-
exclude:
22-
- php: 8.1
23-
laravel: 11
19+
php: [8.2, 8.3]
20+
laravel: [11]
2421

2522
name: PHP ${{ matrix.php }} - Laravel ${{ matrix.laravel }}
2623

‎UPGRADE.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@
66

77
### Minimum PHP Version
88

9-
PR: https://github.com/laravel/passport/pull/1734
9+
PR: https://github.com/laravel/passport/pull/1734, https://github.com/laravel/passport/pull/1783
1010

11-
PHP 8.1 is now the minimum required version.
11+
PHP 8.2 is now the minimum required version.
1212

1313
### Minimum Laravel Version
1414

15-
PR: https://github.com/laravel/passport/pull/1757
15+
PR: https://github.com/laravel/passport/pull/1757, https://github.com/laravel/passport/pull/1783
1616

17-
Laravel 10.0 is now the minimum required version.
17+
Laravel 11.14 is now the minimum required version.
1818

1919
### OAuth2 Server
2020

‎composer.json‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,31 +14,31 @@
1414
}
1515
],
1616
"require": {
17-
"php": "^8.1",
17+
"php": "^8.2",
1818
"ext-json": "*",
1919
"ext-openssl": "*",
2020
"firebase/php-jwt": "^6.4",
21-
"illuminate/auth": "^10.48.15|^11.14",
22-
"illuminate/console": "^10.48.15|^11.14",
23-
"illuminate/container": "^10.48.15|^11.14",
24-
"illuminate/contracts": "^10.48.15|^11.14",
25-
"illuminate/cookie": "^10.48.15|^11.14",
26-
"illuminate/database": "^10.48.15|^11.14",
27-
"illuminate/encryption": "^10.48.15|^11.14",
28-
"illuminate/http": "^10.48.15|^11.14",
29-
"illuminate/support": "^10.48.15|^11.14",
21+
"illuminate/auth": "^11.14",
22+
"illuminate/console": "^11.14",
23+
"illuminate/container": "^11.14",
24+
"illuminate/contracts": "^11.14",
25+
"illuminate/cookie": "^11.14",
26+
"illuminate/database": "^11.14",
27+
"illuminate/encryption": "^11.14",
28+
"illuminate/http": "^11.14",
29+
"illuminate/support": "^11.14",
3030
"lcobucci/jwt": "^5.0",
3131
"league/oauth2-server": "^9.0",
3232
"nyholm/psr7": "^1.5",
3333
"phpseclib/phpseclib": "^3.0",
34-
"symfony/console": "^6.0|^7.0",
35-
"symfony/psr-http-message-bridge": "^6.0|^7.0"
34+
"symfony/console": "^7.0",
35+
"symfony/psr-http-message-bridge": "^7.0"
3636
},
3737
"require-dev": {
3838
"mockery/mockery": "^1.0",
39-
"orchestra/testbench": "^7.35|^8.14|^9.0",
39+
"orchestra/testbench": "^9.0",
4040
"phpstan/phpstan": "^1.10",
41-
"phpunit/phpunit": "^9.3|^10.5|^11.0"
41+
"phpunit/phpunit": "^10.5|^11.0"
4242
},
4343
"autoload": {
4444
"psr-4": {

‎src/AccessToken.php‎

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,24 @@
22

33
namespace Laravel\Passport;
44

5+
use Illuminate\Contracts\Support\Arrayable;
6+
use Illuminate\Contracts\Support\Jsonable;
57
use Illuminate\Support\Traits\ForwardsCalls;
8+
use JsonSerializable;
69
use Psr\Http\Message\ServerRequestInterface;
710

811
/**
9-
* @property string oauth_access_token_id
10-
* @property string oauth_client_id
11-
* @property string oauth_user_id
12-
* @property string[] oauth_scopes
12+
* @template TKey of string
13+
* @template TValue
14+
*
15+
* @implements \Illuminate\Contracts\Support\Arrayable<TKey, TValue>
16+
*
17+
* @property string $oauth_access_token_id
18+
* @property string $oauth_client_id
19+
* @property string $oauth_user_id
20+
* @property string[] $oauth_scopes
1321
*/
14-
class AccessToken
22+
class AccessToken implements Arrayable, Jsonable, JsonSerializable
1523
{
1624
use ResolvesInheritedScopes, ForwardsCalls;
1725

@@ -23,7 +31,7 @@ class AccessToken
2331
/**
2432
* All the attributes set on the access token instance.
2533
*
26-
* @var array<string, mixed>
34+
* @var array<TKey, TValue>
2735
*/
2836
protected array $attributes = [];
2937

@@ -52,21 +60,7 @@ public static function fromPsrRequest(ServerRequestInterface $request): static
5260
*/
5361
public function can(string $scope): bool
5462
{
55-
if (in_array('*', $this->oauth_scopes)) {
56-
return true;
57-
}
58-
59-
$scopes = Passport::$withInheritedScopes
60-
? $this->resolveInheritedScopes($scope)
61-
: [$scope];
62-
63-
foreach ($scopes as $scope) {
64-
if (array_key_exists($scope, array_flip($this->oauth_scopes))) {
65-
return true;
66-
}
67-
}
68-
69-
return false;
63+
return in_array('*', $this->oauth_scopes) || $this->scopeExists($scope, $this->oauth_scopes);
7064
}
7165

7266
/**
@@ -90,15 +84,45 @@ public function transient(): bool
9084
*/
9185
public function revoke(): bool
9286
{
93-
return Passport::token()->whereKey($this->oauth_access_token_id)->forceFill(['revoked' => true])->save();
87+
return Passport::token()->newQuery()->whereKey($this->oauth_access_token_id)->update(['revoked' => true]);
9488
}
9589

9690
/**
9791
* Get the token instance.
9892
*/
9993
protected function getToken(): ?Token
10094
{
101-
return $this->token ??= Passport::token()->find($this->oauth_access_token_id);
95+
return $this->token ??= Passport::token()->newQuery()->find($this->oauth_access_token_id);
96+
}
97+
98+
/**
99+
* Convert the access token instance to an array.
100+
*
101+
* @return array<TKey, TValue>
102+
*/
103+
public function toArray(): array
104+
{
105+
return $this->attributes;
106+
}
107+
108+
/**
109+
* Convert the object into something JSON serializable.
110+
*
111+
* @return array<TKey, TValue>
112+
*/
113+
public function jsonSerialize(): array
114+
{
115+
return $this->toArray();
116+
}
117+
118+
/**
119+
* Convert the access token instance to JSON.
120+
*
121+
* @param int $options
122+
*/
123+
public function toJson($options = 0): string
124+
{
125+
return json_encode($this->jsonSerialize(), $options);
102126
}
103127

104128
/**
@@ -112,7 +136,7 @@ public function __isset(string $key): bool
112136
/**
113137
* Dynamically retrieve the value of an attribute.
114138
*/
115-
public function __get(string $key)
139+
public function __get(string $key): mixed
116140
{
117141
if (array_key_exists($key, $this->attributes)) {
118142
return $this->attributes[$key];

‎src/Bridge/ScopeRepository.php‎

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
namespace Laravel\Passport\Bridge;
44

5+
use Illuminate\Support\Collection;
6+
use Laravel\Passport\Client;
57
use Laravel\Passport\ClientRepository;
68
use Laravel\Passport\Passport;
79
use League\OAuth2\Server\Entities\ClientEntityInterface;
@@ -10,29 +12,19 @@
1012

1113
class ScopeRepository implements ScopeRepositoryInterface
1214
{
13-
/**
14-
* The client repository.
15-
*/
16-
protected ClientRepository $clients;
17-
1815
/**
1916
* Create a new scope repository.
2017
*/
21-
public function __construct(ClientRepository $clients)
18+
public function __construct(protected ClientRepository $clients)
2219
{
23-
$this->clients = $clients;
2420
}
2521

2622
/**
2723
* {@inheritdoc}
2824
*/
2925
public function getScopeEntityByIdentifier(string $identifier): ?ScopeEntityInterface
3026
{
31-
if (Passport::hasScope($identifier)) {
32-
return new Scope($identifier);
33-
}
34-
35-
return null;
27+
return Passport::hasScope($identifier) ? new Scope($identifier) : null;
3628
}
3729

3830
/**
@@ -45,18 +37,19 @@ public function finalizeScopes(
4537
string|null $userIdentifier = null,
4638
?string $authCodeId = null
4739
): array {
48-
if (! in_array($grantType, ['password', 'personal_access', 'client_credentials'])) {
49-
$scopes = collect($scopes)->reject(function ($scope) {
50-
return trim($scope->getIdentifier()) === '*';
51-
})->values()->all();
52-
}
53-
54-
$client = $this->clients->findActive($clientEntity->getIdentifier());
55-
56-
return collect($scopes)->filter(function ($scope) {
57-
return Passport::hasScope($scope->getIdentifier());
58-
})->when($client, function ($scopes, $client) {
59-
return $scopes->filter(fn ($scope) => $client->hasScope($scope->getIdentifier()));
60-
})->values()->all();
40+
return collect($scopes)
41+
->unless(in_array($grantType, ['password', 'personal_access', 'client_credentials']),
42+
fn (Collection $scopes): Collection => $scopes->reject(
43+
fn (Scope $scope): bool => $scope->getIdentifier() === '*'
44+
)
45+
)
46+
->filter(fn (Scope $scope): bool => Passport::hasScope($scope->getIdentifier()))
47+
->when($this->clients->findActive($clientEntity->getIdentifier()),
48+
fn (Collection $scopes, Client $client): Collection => $scopes->filter(
49+
fn (Scope $scope): bool => $client->hasScope($scope->getIdentifier())
50+
)
51+
)
52+
->values()
53+
->all();
6154
}
6255
}

‎src/Client.php‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -192,27 +192,10 @@ public function hasGrantType($grantType)
192192

193193
/**
194194
* Determine whether the client has the given scope.
195-
*
196-
* @param string $scope
197-
* @return bool
198195
*/
199-
public function hasScope($scope)
196+
public function hasScope(string $scope): bool
200197
{
201-
if (! isset($this->attributes['scopes']) || ! is_array($this->scopes)) {
202-
return true;
203-
}
204-
205-
$scopes = Passport::$withInheritedScopes
206-
? $this->resolveInheritedScopes($scope)
207-
: [$scope];
208-
209-
foreach ($scopes as $scope) {
210-
if (in_array($scope, $this->scopes)) {
211-
return true;
212-
}
213-
}
214-
215-
return false;
198+
return ! isset($this->attributes['scopes']) || $this->scopeExists($scope, $this->scopes);
216199
}
217200

218201
/**

‎src/ClientRepository.php‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,16 @@ class ClientRepository
1010
{
1111
/**
1212
* Get a client by the given ID.
13-
*
14-
* @param int|string $id
15-
* @return \Laravel\Passport\Client|null
1613
*/
17-
public function find($id)
14+
public function find(string|int $id): ?Client
1815
{
19-
$client = Passport::client();
20-
21-
return $client->where($client->getKeyName(), $id)->first();
16+
return once(fn () => Passport::client()->newQuery()->find($id));
2217
}
2318

2419
/**
2520
* Get an active client by the given ID.
26-
*
27-
* @param int|string $id
28-
* @return \Laravel\Passport\Client|null
2921
*/
30-
public function findActive($id)
22+
public function findActive(string|int $id): ?Client
3123
{
3224
$client = $this->find($id);
3325

‎src/Http/Controllers/AuthorizationController.php‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,13 +139,14 @@ protected function parseScopes($authRequest)
139139
*/
140140
protected function hasGrantedScopes($user, $client, $scopes)
141141
{
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();
142+
$tokensScopes = $client->tokens()->where([
143+
['user_id', '=', $user->getAuthIdentifier()],
144+
['revoked', '=', false],
145+
['expires_at', '>', Date::now()],
146+
])->pluck('scopes');
147+
148+
return $tokensScopes->isNotEmpty() &&
149+
collect($scopes)->pluck('id')->diff($tokensScopes->flatten())->isEmpty();
149150
}
150151

151152
/**

‎src/PassportServiceProvider.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ public function register()
115115
->needs(StatefulGuard::class)
116116
->give(fn () => Auth::guard(config('passport.guard', null)));
117117

118+
$this->app->singleton(ClientRepository::class);
119+
118120
$this->registerAuthorizationServer();
119121
$this->registerJWTParser();
120122
$this->registerResourceServer();

0 commit comments

Comments
 (0)