Skip to content

Commit

Permalink
[13.x] Improve performance, fix minor bugs, and add tests (#1783)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@hafezdivandari
hafezdivandari authored Sep 19, 2024
1 parent 6142f9c commit d16746c
Show file tree
Hide file tree
Showing 12 changed files with 351 additions and 116 deletions.
9 changes: 3 additions & 6 deletions .github/workflows/tests.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,13 @@ on:

jobs:
tests:
runs-on: ubuntu-22.04
runs-on: ubuntu-latest

strategy:
fail-fast: true
matrix:
php: [8.1, 8.2, 8.3]
laravel: [10, 11]
exclude:
- php: 8.1
laravel: 11
php: [8.2, 8.3]
laravel: [11]

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

Expand Down
8 changes: 4 additions & 4 deletions UPGRADE.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@

### Minimum PHP Version

PR: https://github.com/laravel/passport/pull/1734
PR: https://github.com/laravel/passport/pull/1734, https://github.com/laravel/passport/pull/1783

PHP 8.1 is now the minimum required version.
PHP 8.2 is now the minimum required version.

### Minimum Laravel Version

PR: https://github.com/laravel/passport/pull/1757
PR: https://github.com/laravel/passport/pull/1757, https://github.com/laravel/passport/pull/1783

Laravel 10.0 is now the minimum required version.
Laravel 11.14 is now the minimum required version.

### OAuth2 Server

Expand Down
28 changes: 14 additions & 14 deletions composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,31 @@
}
],
"require": {
"php": "^8.1",
"php": "^8.2",
"ext-json": "*",
"ext-openssl": "*",
"firebase/php-jwt": "^6.4",
"illuminate/auth": "^10.48.15|^11.14",
"illuminate/console": "^10.48.15|^11.14",
"illuminate/container": "^10.48.15|^11.14",
"illuminate/contracts": "^10.48.15|^11.14",
"illuminate/cookie": "^10.48.15|^11.14",
"illuminate/database": "^10.48.15|^11.14",
"illuminate/encryption": "^10.48.15|^11.14",
"illuminate/http": "^10.48.15|^11.14",
"illuminate/support": "^10.48.15|^11.14",
"illuminate/auth": "^11.14",
"illuminate/console": "^11.14",
"illuminate/container": "^11.14",
"illuminate/contracts": "^11.14",
"illuminate/cookie": "^11.14",
"illuminate/database": "^11.14",
"illuminate/encryption": "^11.14",
"illuminate/http": "^11.14",
"illuminate/support": "^11.14",
"lcobucci/jwt": "^5.0",
"league/oauth2-server": "^9.0",
"nyholm/psr7": "^1.5",
"phpseclib/phpseclib": "^3.0",
"symfony/console": "^6.0|^7.0",
"symfony/psr-http-message-bridge": "^6.0|^7.0"
"symfony/console": "^7.0",
"symfony/psr-http-message-bridge": "^7.0"
},
"require-dev": {
"mockery/mockery": "^1.0",
"orchestra/testbench": "^7.35|^8.14|^9.0",
"orchestra/testbench": "^9.0",
"phpstan/phpstan": "^1.10",
"phpunit/phpunit": "^9.3|^10.5|^11.0"
"phpunit/phpunit": "^10.5|^11.0"
},
"autoload": {
"psr-4": {
Expand Down
72 changes: 48 additions & 24 deletions src/AccessToken.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,24 @@

namespace Laravel\Passport;

use Illuminate\Contracts\Support\Arrayable;
use Illuminate\Contracts\Support\Jsonable;
use Illuminate\Support\Traits\ForwardsCalls;
use JsonSerializable;
use Psr\Http\Message\ServerRequestInterface;

/**
* @property string oauth_access_token_id
* @property string oauth_client_id
* @property string oauth_user_id
* @property string[] oauth_scopes
* @template TKey of string
* @template TValue
*
* @implements \Illuminate\Contracts\Support\Arrayable<TKey, TValue>
*
* @property string $oauth_access_token_id
* @property string $oauth_client_id
* @property string $oauth_user_id
* @property string[] $oauth_scopes
*/
class AccessToken
class AccessToken implements Arrayable, Jsonable, JsonSerializable
{
use ResolvesInheritedScopes, ForwardsCalls;

Expand All @@ -23,7 +31,7 @@ class AccessToken
/**
* All the attributes set on the access token instance.
*
* @var array<string, mixed>
* @var array<TKey, TValue>
*/
protected array $attributes = [];

Expand Down Expand Up @@ -52,21 +60,7 @@ public static function fromPsrRequest(ServerRequestInterface $request): static
*/
public function can(string $scope): bool
{
if (in_array('*', $this->oauth_scopes)) {
return true;
}

$scopes = Passport::$withInheritedScopes
? $this->resolveInheritedScopes($scope)
: [$scope];

foreach ($scopes as $scope) {
if (array_key_exists($scope, array_flip($this->oauth_scopes))) {
return true;
}
}

return false;
return in_array('*', $this->oauth_scopes) || $this->scopeExists($scope, $this->oauth_scopes);
}

/**
Expand All @@ -90,15 +84,45 @@ public function transient(): bool
*/
public function revoke(): bool
{
return Passport::token()->whereKey($this->oauth_access_token_id)->forceFill(['revoked' => true])->save();
return Passport::token()->newQuery()->whereKey($this->oauth_access_token_id)->update(['revoked' => true]);
}

/**
* Get the token instance.
*/
protected function getToken(): ?Token
{
return $this->token ??= Passport::token()->find($this->oauth_access_token_id);
return $this->token ??= Passport::token()->newQuery()->find($this->oauth_access_token_id);
}

/**
* Convert the access token instance to an array.
*
* @return array<TKey, TValue>
*/
public function toArray(): array
{
return $this->attributes;
}

/**
* Convert the object into something JSON serializable.
*
* @return array<TKey, TValue>
*/
public function jsonSerialize(): array
{
return $this->toArray();
}

/**
* Convert the access token instance to JSON.
*
* @param int $options
*/
public function toJson($options = 0): string
{
return json_encode($this->jsonSerialize(), $options);
}

/**
Expand All @@ -112,7 +136,7 @@ public function __isset(string $key): bool
/**
* Dynamically retrieve the value of an attribute.
*/
public function __get(string $key)
public function __get(string $key): mixed
{
if (array_key_exists($key, $this->attributes)) {
return $this->attributes[$key];
Expand Down
43 changes: 18 additions & 25 deletions src/Bridge/ScopeRepository.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace Laravel\Passport\Bridge;

use Illuminate\Support\Collection;
use Laravel\Passport\Client;
use Laravel\Passport\ClientRepository;
use Laravel\Passport\Passport;
use League\OAuth2\Server\Entities\ClientEntityInterface;
Expand All @@ -10,29 +12,19 @@

class ScopeRepository implements ScopeRepositoryInterface
{
/**
* The client repository.
*/
protected ClientRepository $clients;

/**
* Create a new scope repository.
*/
public function __construct(ClientRepository $clients)
public function __construct(protected ClientRepository $clients)
{
$this->clients = $clients;
}

/**
* {@inheritdoc}
*/
public function getScopeEntityByIdentifier(string $identifier): ?ScopeEntityInterface
{
if (Passport::hasScope($identifier)) {
return new Scope($identifier);
}

return null;
return Passport::hasScope($identifier) ? new Scope($identifier) : null;
}

/**
Expand All @@ -45,18 +37,19 @@ public function finalizeScopes(
string|null $userIdentifier = null,
?string $authCodeId = null
): array {
if (! in_array($grantType, ['password', 'personal_access', 'client_credentials'])) {
$scopes = collect($scopes)->reject(function ($scope) {
return trim($scope->getIdentifier()) === '*';
})->values()->all();
}

$client = $this->clients->findActive($clientEntity->getIdentifier());

return collect($scopes)->filter(function ($scope) {
return Passport::hasScope($scope->getIdentifier());
})->when($client, function ($scopes, $client) {
return $scopes->filter(fn ($scope) => $client->hasScope($scope->getIdentifier()));
})->values()->all();
return collect($scopes)
->unless(in_array($grantType, ['password', 'personal_access', 'client_credentials']),
fn (Collection $scopes): Collection => $scopes->reject(
fn (Scope $scope): bool => $scope->getIdentifier() === '*'
)
)
->filter(fn (Scope $scope): bool => Passport::hasScope($scope->getIdentifier()))
->when($this->clients->findActive($clientEntity->getIdentifier()),
fn (Collection $scopes, Client $client): Collection => $scopes->filter(
fn (Scope $scope): bool => $client->hasScope($scope->getIdentifier())
)
)
->values()
->all();
}
}
21 changes: 2 additions & 19 deletions src/Client.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,27 +192,10 @@ public function hasGrantType($grantType)

/**
* Determine whether the client has the given scope.
*
* @param string $scope
* @return bool
*/
public function hasScope($scope)
public function hasScope(string $scope): bool
{
if (! isset($this->attributes['scopes']) || ! is_array($this->scopes)) {
return true;
}

$scopes = Passport::$withInheritedScopes
? $this->resolveInheritedScopes($scope)
: [$scope];

foreach ($scopes as $scope) {
if (in_array($scope, $this->scopes)) {
return true;
}
}

return false;
return ! isset($this->attributes['scopes']) || $this->scopeExists($scope, $this->scopes);
}

/**
Expand Down
14 changes: 3 additions & 11 deletions src/ClientRepository.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,16 @@ class ClientRepository
{
/**
* Get a client by the given ID.
*
* @param int|string $id
* @return \Laravel\Passport\Client|null
*/
public function find($id)
public function find(string|int $id): ?Client
{
$client = Passport::client();

return $client->where($client->getKeyName(), $id)->first();
return once(fn () => Passport::client()->newQuery()->find($id));
}

/**
* Get an active client by the given ID.
*
* @param int|string $id
* @return \Laravel\Passport\Client|null
*/
public function findActive($id)
public function findActive(string|int $id): ?Client
{
$client = $this->find($id);

Expand Down
15 changes: 8 additions & 7 deletions src/Http/Controllers/AuthorizationController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,14 @@ protected function parseScopes($authRequest)
*/
protected function hasGrantedScopes($user, $client, $scopes)
{
return collect($scopes)->pluck('id')->diff(
$client->tokens()->where([
['user_id', '=', $user->getAuthIdentifier()],
['revoked', '=', false],
['expires_at', '>', Date::now()],
])->pluck('scopes')->flatten()
)->isEmpty();
$tokensScopes = $client->tokens()->where([
['user_id', '=', $user->getAuthIdentifier()],
['revoked', '=', false],
['expires_at', '>', Date::now()],
])->pluck('scopes');

return $tokensScopes->isNotEmpty() &&
collect($scopes)->pluck('id')->diff($tokensScopes->flatten())->isEmpty();
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/PassportServiceProvider.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ public function register()
->needs(StatefulGuard::class)
->give(fn () => Auth::guard(config('passport.guard', null)));

$this->app->singleton(ClientRepository::class);

$this->registerAuthorizationServer();
$this->registerJWTParser();
$this->registerResourceServer();
Expand Down
Loading

0 comments on commit d16746c

Please sign in to comment.