From fc7e2da194937e556f5a8f74fa887161e45e8cc2 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Sat, 18 May 2024 01:35:01 +0330 Subject: [PATCH] [13.x] Support OAuth2 Server v9 (#1734) * support OAuth2 Server 9 * formatting * formatting * drop php 8.0 * use fqn * wip * wip * wip * formatting * formatting * wip * remove redundant attribute * fix tests * update upgrade guide * force re-run tests * wip * wip * add an entry on upgrade guide for oauth2 server * Update UPGRADE.md * Update UPGRADE.md * Update UPGRADE.md --------- Co-authored-by: Dries Vints Co-authored-by: Taylor Otwell --- .github/workflows/tests.yml | 6 +-- UPGRADE.md | 16 ++++++ composer.json | 15 +++--- database/factories/ClientFactory.php | 12 +++-- src/Bridge/AccessToken.php | 11 ++-- src/Bridge/AccessTokenRepository.php | 37 +++++-------- src/Bridge/AuthCodeRepository.php | 8 +-- src/Bridge/Client.php | 53 ++++--------------- src/Bridge/ClientRepository.php | 48 ++++++----------- src/Bridge/FormatsScopesForStorage.php | 11 ++-- src/Bridge/PersonalAccessGrant.php | 4 +- src/Bridge/RefreshTokenRepository.php | 20 +++---- src/Bridge/Scope.php | 19 ++----- src/Bridge/ScopeRepository.php | 25 ++++----- src/Bridge/User.php | 5 +- src/Bridge/UserRepository.php | 24 ++++----- src/Events/AccessTokenCreated.php | 4 +- src/Passport.php | 2 +- tests/Feature/AccessTokenControllerTest.php | 7 +-- tests/Unit/AuthorizationControllerTest.php | 7 ++- .../Unit/BridgeAccessTokenRepositoryTest.php | 8 +-- tests/Unit/BridgeClientRepositoryTest.php | 2 +- tests/Unit/BridgeScopeRepositoryTest.php | 14 ++++- .../CheckClientCredentialsForAnyScopeTest.php | 7 +-- tests/Unit/CheckClientCredentialsTest.php | 7 +-- .../Unit/DenyAuthorizationControllerTest.php | 4 +- tests/Unit/HandlesOAuthErrorsTest.php | 2 +- tests/Unit/HasApiTokensTest.php | 2 + tests/Unit/PersonalAccessTokenFactoryTest.php | 3 +- tests/Unit/TokenGuardTest.php | 13 ++--- 30 files changed, 173 insertions(+), 223 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7ca8da38a..6292d4602 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -16,13 +16,9 @@ jobs: strategy: fail-fast: true matrix: - php: ['8.0', 8.1, 8.2, 8.3] + php: [8.1, 8.2, 8.3] laravel: [9, 10, 11] exclude: - - php: '8.0' - laravel: 10 - - php: '8.0' - laravel: 11 - php: 8.1 laravel: 11 - php: 8.3 diff --git a/UPGRADE.md b/UPGRADE.md index c4d506ae5..6127c47ae 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -2,6 +2,20 @@ ## General Notes +## Upgrading To 13.0 From 12.x + +### Minimum PHP Version + +PR: https://github.com/laravel/passport/pull/1734 + +PHP 8.1 is now the minimum required version. + +### OAuth2 Server + +PR: https://github.com/laravel/passport/pull/1734 + +The `league/oauth2-server` Composer package which is utilized internally by Passport has been updated to 9.0, which adds additional types to method signatures. To ensure your application is compatible, you should review this package's complete [changelog](https://github.com/thephpleague/oauth2-server/blob/master/CHANGELOG.md#900---released-2024-05-13). + ## Upgrading To 12.0 From 11.x ### Migration Changes @@ -14,6 +28,8 @@ php artisan vendor:publish --tag=passport-migrations ### Password Grant Type +PR: https://github.com/laravel/passport/pull/1715 + The password grant type is disabled by default. You may enable it by calling the `enablePasswordGrant` method in the `boot` method of your application's `App\Providers\AppServiceProvider` class: ```php diff --git a/composer.json b/composer.json index 390a040f1..e8dad6784 100644 --- a/composer.json +++ b/composer.json @@ -14,8 +14,9 @@ } ], "require": { - "php": "^8.0", + "php": "^8.1", "ext-json": "*", + "ext-openssl": "*", "firebase/php-jwt": "^6.4", "illuminate/auth": "^9.21|^10.0|^11.0", "illuminate/console": "^9.21|^10.0|^11.0", @@ -26,18 +27,18 @@ "illuminate/encryption": "^9.21|^10.0|^11.0", "illuminate/http": "^9.21|^10.0|^11.0", "illuminate/support": "^9.21|^10.0|^11.0", - "lcobucci/jwt": "^4.3|^5.0", - "league/oauth2-server": "^8.5.3", + "lcobucci/jwt": "^5.0", + "league/oauth2-server": "^9.0", "nyholm/psr7": "^1.5", - "phpseclib/phpseclib": "^2.0|^3.0", + "phpseclib/phpseclib": "^3.0", "symfony/console": "^6.0|^7.0", - "symfony/psr-http-message-bridge": "^2.1|^6.0|^7.0" + "symfony/psr-http-message-bridge": "^6.0|^7.0" }, "require-dev": { "mockery/mockery": "^1.0", "orchestra/testbench": "^7.35|^8.14|^9.0", "phpstan/phpstan": "^1.10", - "phpunit/phpunit": "^9.3|^10.5" + "phpunit/phpunit": "^9.3|^10.5|^11.0" }, "autoload": { "psr-4": { @@ -66,6 +67,6 @@ "post-autoload-dump": "@prepare", "prepare": "@php vendor/bin/testbench package:discover --ansi" }, - "minimum-stability": "dev", + "minimum-stability": "stable", "prefer-stable": true } diff --git a/database/factories/ClientFactory.php b/database/factories/ClientFactory.php index e9f25b790..5089b748f 100644 --- a/database/factories/ClientFactory.php +++ b/database/factories/ClientFactory.php @@ -4,7 +4,6 @@ use Illuminate\Database\Eloquent\Factories\Factory; use Illuminate\Support\Str; -use Laravel\Passport\Client; use Laravel\Passport\Passport; /** @@ -13,11 +12,14 @@ class ClientFactory extends Factory { /** - * The name of the factory's corresponding model. + * Get the name of the model that is generated by the factory. * - * @var string + * @return class-string<\Illuminate\Database\Eloquent\Model> */ - protected $model = Client::class; + public function modelName() + { + return $this->model ?? Passport::clientModel(); + } /** * Define the model's default state. @@ -46,7 +48,7 @@ public function definition() protected function ensurePrimaryKeyIsSet(array $data) { if (Passport::clientUuids()) { - $keyName = (new $this->model)->getKeyName(); + $keyName = (new ($this->modelName()))->getKeyName(); $data[$keyName] = (string) Str::orderedUuid(); } diff --git a/src/Bridge/AccessToken.php b/src/Bridge/AccessToken.php index 77d37c52b..14e482cdf 100644 --- a/src/Bridge/AccessToken.php +++ b/src/Bridge/AccessToken.php @@ -15,14 +15,13 @@ class AccessToken implements AccessTokenEntityInterface /** * Create a new token instance. * - * @param string $userIdentifier - * @param array $scopes - * @param \League\OAuth2\Server\Entities\ClientEntityInterface $client - * @return void + * @param \League\OAuth2\Server\Entities\ScopeEntityInterface[] $scopes */ - public function __construct($userIdentifier, array $scopes, ClientEntityInterface $client) + public function __construct(string|null $userIdentifier, array $scopes, ClientEntityInterface $client) { - $this->setUserIdentifier($userIdentifier); + if (! is_null($userIdentifier)) { + $this->setUserIdentifier($userIdentifier); + } foreach ($scopes as $scope) { $this->addScope($scope); diff --git a/src/Bridge/AccessTokenRepository.php b/src/Bridge/AccessTokenRepository.php index 23572d760..cf918c738 100644 --- a/src/Bridge/AccessTokenRepository.php +++ b/src/Bridge/AccessTokenRepository.php @@ -17,24 +17,16 @@ class AccessTokenRepository implements AccessTokenRepositoryInterface /** * The token repository instance. - * - * @var \Laravel\Passport\TokenRepository */ - protected $tokenRepository; + protected TokenRepository $tokenRepository; /** * The event dispatcher instance. - * - * @var \Illuminate\Contracts\Events\Dispatcher */ - protected $events; + protected Dispatcher $events; /** * Create a new repository instance. - * - * @param \Laravel\Passport\TokenRepository $tokenRepository - * @param \Illuminate\Contracts\Events\Dispatcher $events - * @return void */ public function __construct(TokenRepository $tokenRepository, Dispatcher $events) { @@ -45,20 +37,23 @@ public function __construct(TokenRepository $tokenRepository, Dispatcher $events /** * {@inheritdoc} */ - public function getNewToken(ClientEntityInterface $clientEntity, array $scopes, $userIdentifier = null) - { + public function getNewToken( + ClientEntityInterface $clientEntity, + array $scopes, + string|null $userIdentifier = null + ): AccessTokenEntityInterface { return new Passport::$accessTokenEntity($userIdentifier, $scopes, $clientEntity); } /** * {@inheritdoc} */ - public function persistNewAccessToken(AccessTokenEntityInterface $accessTokenEntity) + public function persistNewAccessToken(AccessTokenEntityInterface $accessTokenEntity): void { $this->tokenRepository->create([ - 'id' => $accessTokenEntity->getIdentifier(), - 'user_id' => $accessTokenEntity->getUserIdentifier(), - 'client_id' => $accessTokenEntity->getClient()->getIdentifier(), + 'id' => $id = $accessTokenEntity->getIdentifier(), + 'user_id' => $userId = $accessTokenEntity->getUserIdentifier(), + 'client_id' => $clientId = $accessTokenEntity->getClient()->getIdentifier(), 'scopes' => $this->scopesToArray($accessTokenEntity->getScopes()), 'revoked' => false, 'created_at' => new DateTime, @@ -66,17 +61,13 @@ public function persistNewAccessToken(AccessTokenEntityInterface $accessTokenEnt 'expires_at' => $accessTokenEntity->getExpiryDateTime(), ]); - $this->events->dispatch(new AccessTokenCreated( - $accessTokenEntity->getIdentifier(), - $accessTokenEntity->getUserIdentifier(), - $accessTokenEntity->getClient()->getIdentifier() - )); + $this->events->dispatch(new AccessTokenCreated($id, $userId, $clientId)); } /** * {@inheritdoc} */ - public function revokeAccessToken($tokenId) + public function revokeAccessToken(string $tokenId): void { $this->tokenRepository->revokeAccessToken($tokenId); } @@ -84,7 +75,7 @@ public function revokeAccessToken($tokenId) /** * {@inheritdoc} */ - public function isAccessTokenRevoked($tokenId) + public function isAccessTokenRevoked(string $tokenId): bool { return $this->tokenRepository->isAccessTokenRevoked($tokenId); } diff --git a/src/Bridge/AuthCodeRepository.php b/src/Bridge/AuthCodeRepository.php index 2dfb626a9..fb0dc5294 100644 --- a/src/Bridge/AuthCodeRepository.php +++ b/src/Bridge/AuthCodeRepository.php @@ -13,7 +13,7 @@ class AuthCodeRepository implements AuthCodeRepositoryInterface /** * {@inheritdoc} */ - public function getNewAuthCode() + public function getNewAuthCode(): AuthCodeEntityInterface { return new AuthCode; } @@ -21,7 +21,7 @@ public function getNewAuthCode() /** * {@inheritdoc} */ - public function persistNewAuthCode(AuthCodeEntityInterface $authCodeEntity) + public function persistNewAuthCode(AuthCodeEntityInterface $authCodeEntity): void { $attributes = [ 'id' => $authCodeEntity->getIdentifier(), @@ -38,7 +38,7 @@ public function persistNewAuthCode(AuthCodeEntityInterface $authCodeEntity) /** * {@inheritdoc} */ - public function revokeAuthCode($codeId) + public function revokeAuthCode(string $codeId): void { Passport::authCode()->where('id', $codeId)->update(['revoked' => true]); } @@ -46,7 +46,7 @@ public function revokeAuthCode($codeId) /** * {@inheritdoc} */ - public function isAuthCodeRevoked($codeId) + public function isAuthCodeRevoked(string $codeId): bool { return Passport::authCode()->where('id', $codeId)->where('revoked', 1)->exists(); } diff --git a/src/Bridge/Client.php b/src/Bridge/Client.php index 70539ac80..50b129c90 100644 --- a/src/Bridge/Client.php +++ b/src/Bridge/Client.php @@ -4,63 +4,32 @@ use League\OAuth2\Server\Entities\ClientEntityInterface; use League\OAuth2\Server\Entities\Traits\ClientTrait; +use League\OAuth2\Server\Entities\Traits\EntityTrait; class Client implements ClientEntityInterface { - use ClientTrait; - - /** - * The client identifier. - * - * @var string - */ - protected $identifier; + use ClientTrait, EntityTrait; /** * The client's provider. - * - * @var string */ - public $provider; + public ?string $provider; /** * Create a new client instance. - * - * @param string $identifier - * @param string $name - * @param string $redirectUri - * @param bool $isConfidential - * @param string|null $provider - * @return void */ - public function __construct($identifier, $name, $redirectUri, $isConfidential = false, $provider = null) - { - $this->setIdentifier((string) $identifier); + public function __construct( + string $identifier, + string $name, + string $redirectUri, + bool $isConfidential = false, + ?string $provider = null + ) { + $this->setIdentifier($identifier); $this->name = $name; $this->isConfidential = $isConfidential; $this->redirectUri = explode(',', $redirectUri); $this->provider = $provider; } - - /** - * Get the client's identifier. - * - * @return string - */ - public function getIdentifier() - { - return (string) $this->identifier; - } - - /** - * Set the client's identifier. - * - * @param string $identifier - * @return void - */ - public function setIdentifier($identifier) - { - $this->identifier = $identifier; - } } diff --git a/src/Bridge/ClientRepository.php b/src/Bridge/ClientRepository.php index c3c9c2a78..1c21ce623 100644 --- a/src/Bridge/ClientRepository.php +++ b/src/Bridge/ClientRepository.php @@ -2,24 +2,21 @@ namespace Laravel\Passport\Bridge; +use Laravel\Passport\Client as ClientModel; use Laravel\Passport\ClientRepository as ClientModelRepository; use Laravel\Passport\Passport; +use League\OAuth2\Server\Entities\ClientEntityInterface; use League\OAuth2\Server\Repositories\ClientRepositoryInterface; class ClientRepository implements ClientRepositoryInterface { /** * The client model repository. - * - * @var \Laravel\Passport\ClientRepository */ - protected $clients; + protected ClientModelRepository $clients; /** * Create a new repository instance. - * - * @param \Laravel\Passport\ClientRepository $clients - * @return void */ public function __construct(ClientModelRepository $clients) { @@ -29,12 +26,12 @@ public function __construct(ClientModelRepository $clients) /** * {@inheritdoc} */ - public function getClientEntity($clientIdentifier) + public function getClientEntity(string $clientIdentifier): ?ClientEntityInterface { $record = $this->clients->findActive($clientIdentifier); if (! $record) { - return; + return null; } return new Client( @@ -49,7 +46,7 @@ public function getClientEntity($clientIdentifier) /** * {@inheritdoc} */ - public function validateClient($clientIdentifier, $clientSecret, $grantType) + public function validateClient(string $clientIdentifier, ?string $clientSecret, ?string $grantType): bool { // First, we will verify that the client exists and is authorized to create personal // access tokens. Generally personal access tokens are only generated by the user @@ -60,44 +57,31 @@ public function validateClient($clientIdentifier, $clientSecret, $grantType) return false; } - return ! $record->confidential() || $this->verifySecret((string) $clientSecret, $record->secret); + return ! $record->confidential() || $this->verifySecret($clientSecret, $record->secret); } /** * Determine if the given client can handle the given grant type. - * - * @param \Laravel\Passport\Client $record - * @param string $grantType - * @return bool */ - protected function handlesGrant($record, $grantType) + protected function handlesGrant(ClientModel $record, string $grantType): bool { if (! $record->hasGrantType($grantType)) { return false; } - switch ($grantType) { - case 'authorization_code': - return ! $record->firstParty(); - case 'personal_access': - return $record->personal_access_client && $record->confidential(); - case 'password': - return $record->password_client; - case 'client_credentials': - return $record->confidential(); - default: - return true; - } + return match ($grantType) { + 'authorization_code' => ! $record->firstParty(), + 'personal_access' => $record->personal_access_client && $record->confidential(), + 'password' => $record->password_client, + 'client_credentials' => $record->confidential(), + default => true, + }; } /** * Verify the client secret is valid. - * - * @param string $clientSecret - * @param string $storedHash - * @return bool */ - protected function verifySecret($clientSecret, $storedHash) + protected function verifySecret(string $clientSecret, string $storedHash): bool { return Passport::$hashesClientSecrets ? password_verify($clientSecret, $storedHash) diff --git a/src/Bridge/FormatsScopesForStorage.php b/src/Bridge/FormatsScopesForStorage.php index 98f1d4fe0..8474416b7 100644 --- a/src/Bridge/FormatsScopesForStorage.php +++ b/src/Bridge/FormatsScopesForStorage.php @@ -7,10 +7,9 @@ trait FormatsScopesForStorage /** * Format the given scopes for storage. * - * @param array $scopes - * @return string + * @param \League\OAuth2\Server\Entities\ScopeEntityInterface[] $scopes */ - public function formatScopesForStorage(array $scopes) + public function formatScopesForStorage(array $scopes): string { return json_encode($this->scopesToArray($scopes)); } @@ -18,10 +17,10 @@ public function formatScopesForStorage(array $scopes) /** * Get an array of scope identifiers for storage. * - * @param array $scopes - * @return array + * @param \League\OAuth2\Server\Entities\ScopeEntityInterface[] $scopes + * @return string[] */ - public function scopesToArray(array $scopes) + public function scopesToArray(array $scopes): array { return array_map(function ($scope) { return $scope->getIdentifier(); diff --git a/src/Bridge/PersonalAccessGrant.php b/src/Bridge/PersonalAccessGrant.php index 4eb5f869c..fd2d4aecb 100644 --- a/src/Bridge/PersonalAccessGrant.php +++ b/src/Bridge/PersonalAccessGrant.php @@ -16,7 +16,7 @@ public function respondToAccessTokenRequest( ServerRequestInterface $request, ResponseTypeInterface $responseType, DateInterval $accessTokenTTL - ) { + ): ResponseTypeInterface { // Validate request $client = $this->validateClient($request); $scopes = $this->validateScopes($this->getRequestParameter('scope', $request)); @@ -47,7 +47,7 @@ public function respondToAccessTokenRequest( /** * {@inheritdoc} */ - public function getIdentifier() + public function getIdentifier(): string { return 'personal_access'; } diff --git a/src/Bridge/RefreshTokenRepository.php b/src/Bridge/RefreshTokenRepository.php index 4bb7bd969..3a35b57cb 100644 --- a/src/Bridge/RefreshTokenRepository.php +++ b/src/Bridge/RefreshTokenRepository.php @@ -12,24 +12,16 @@ class RefreshTokenRepository implements RefreshTokenRepositoryInterface { /** * The refresh token repository instance. - * - * @var \Illuminate\Database\Connection */ - protected $refreshTokenRepository; + protected PassportRefreshTokenRepository $refreshTokenRepository; /** * The event dispatcher instance. - * - * @var \Illuminate\Contracts\Events\Dispatcher */ - protected $events; + protected Dispatcher $events; /** * Create a new repository instance. - * - * @param \Laravel\Passport\RefreshTokenRepository $refreshTokenRepository - * @param \Illuminate\Contracts\Events\Dispatcher $events - * @return void */ public function __construct(PassportRefreshTokenRepository $refreshTokenRepository, Dispatcher $events) { @@ -40,7 +32,7 @@ public function __construct(PassportRefreshTokenRepository $refreshTokenReposito /** * {@inheritdoc} */ - public function getNewRefreshToken() + public function getNewRefreshToken(): ?RefreshTokenEntityInterface { return new RefreshToken; } @@ -48,7 +40,7 @@ public function getNewRefreshToken() /** * {@inheritdoc} */ - public function persistNewRefreshToken(RefreshTokenEntityInterface $refreshTokenEntity) + public function persistNewRefreshToken(RefreshTokenEntityInterface $refreshTokenEntity): void { $this->refreshTokenRepository->create([ 'id' => $id = $refreshTokenEntity->getIdentifier(), @@ -63,7 +55,7 @@ public function persistNewRefreshToken(RefreshTokenEntityInterface $refreshToken /** * {@inheritdoc} */ - public function revokeRefreshToken($tokenId) + public function revokeRefreshToken(string $tokenId): void { $this->refreshTokenRepository->revokeRefreshToken($tokenId); } @@ -71,7 +63,7 @@ public function revokeRefreshToken($tokenId) /** * {@inheritdoc} */ - public function isRefreshTokenRevoked($tokenId) + public function isRefreshTokenRevoked(string $tokenId): bool { return $this->refreshTokenRepository->isRefreshTokenRevoked($tokenId); } diff --git a/src/Bridge/Scope.php b/src/Bridge/Scope.php index 50aaf02b9..4d77d6f35 100644 --- a/src/Bridge/Scope.php +++ b/src/Bridge/Scope.php @@ -4,30 +4,17 @@ use League\OAuth2\Server\Entities\ScopeEntityInterface; use League\OAuth2\Server\Entities\Traits\EntityTrait; +use League\OAuth2\Server\Entities\Traits\ScopeTrait; class Scope implements ScopeEntityInterface { - use EntityTrait; + use ScopeTrait, EntityTrait; /** * Create a new scope instance. - * - * @param string $name - * @return void */ - public function __construct($name) + public function __construct(string $name) { $this->setIdentifier($name); } - - /** - * Get the data that should be serialized to JSON. - * - * @return mixed - */ - #[\ReturnTypeWillChange] - public function jsonSerialize() - { - return $this->getIdentifier(); - } } diff --git a/src/Bridge/ScopeRepository.php b/src/Bridge/ScopeRepository.php index 05e0ac9a1..308f67d03 100644 --- a/src/Bridge/ScopeRepository.php +++ b/src/Bridge/ScopeRepository.php @@ -5,24 +5,20 @@ use Laravel\Passport\ClientRepository; use Laravel\Passport\Passport; use League\OAuth2\Server\Entities\ClientEntityInterface; +use League\OAuth2\Server\Entities\ScopeEntityInterface; use League\OAuth2\Server\Repositories\ScopeRepositoryInterface; class ScopeRepository implements ScopeRepositoryInterface { /** * The client repository. - * - * @var \Laravel\Passport\ClientRepository|null */ - protected ?ClientRepository $clients; + protected ClientRepository $clients; /** * Create a new scope repository. - * - * @param \Laravel\Passport\ClientRepository|null $clients - * @return void */ - public function __construct(?ClientRepository $clients = null) + public function __construct(ClientRepository $clients) { $this->clients = $clients; } @@ -30,27 +26,32 @@ public function __construct(?ClientRepository $clients = null) /** * {@inheritdoc} */ - public function getScopeEntityByIdentifier($identifier) + public function getScopeEntityByIdentifier(string $identifier): ?ScopeEntityInterface { if (Passport::hasScope($identifier)) { return new Scope($identifier); } + + return null; } /** * {@inheritdoc} */ public function finalizeScopes( - array $scopes, $grantType, - ClientEntityInterface $clientEntity, $userIdentifier = null) - { + array $scopes, + string $grantType, + ClientEntityInterface $clientEntity, + 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()); + $client = $this->clients->findActive($clientEntity->getIdentifier()); return collect($scopes)->filter(function ($scope) { return Passport::hasScope($scope->getIdentifier()); diff --git a/src/Bridge/User.php b/src/Bridge/User.php index 94ecfa113..afddf833f 100644 --- a/src/Bridge/User.php +++ b/src/Bridge/User.php @@ -11,11 +11,8 @@ class User implements UserEntityInterface /** * Create a new user instance. - * - * @param string|int $identifier - * @return void */ - public function __construct($identifier) + public function __construct(string $identifier) { $this->setIdentifier($identifier); } diff --git a/src/Bridge/UserRepository.php b/src/Bridge/UserRepository.php index b73404ad2..8be747a56 100644 --- a/src/Bridge/UserRepository.php +++ b/src/Bridge/UserRepository.php @@ -4,6 +4,7 @@ use Illuminate\Contracts\Hashing\Hasher; use League\OAuth2\Server\Entities\ClientEntityInterface; +use League\OAuth2\Server\Entities\UserEntityInterface; use League\OAuth2\Server\Repositories\UserRepositoryInterface; use RuntimeException; @@ -11,16 +12,11 @@ class UserRepository implements UserRepositoryInterface { /** * The hasher implementation. - * - * @var \Illuminate\Contracts\Hashing\Hasher */ - protected $hasher; + protected Hasher $hasher; /** * Create a new repository instance. - * - * @param \Illuminate\Contracts\Hashing\Hasher $hasher - * @return void */ public function __construct(Hasher $hasher) { @@ -30,8 +26,12 @@ public function __construct(Hasher $hasher) /** * {@inheritdoc} */ - public function getUserEntityByUserCredentials($username, $password, $grantType, ClientEntityInterface $clientEntity) - { + public function getUserEntityByUserCredentials( + string $username, + string $password, + string $grantType, + ClientEntityInterface $clientEntity + ): ?UserEntityInterface { $provider = $clientEntity->provider ?: config('auth.guards.api.provider'); if (is_null($model = config('auth.providers.'.$provider.'.model'))) { @@ -42,7 +42,7 @@ public function getUserEntityByUserCredentials($username, $password, $grantType, $user = (new $model)->findAndValidateForPassport($username, $password); if (! $user) { - return; + return null; } return new User($user->getAuthIdentifier()); @@ -55,13 +55,13 @@ public function getUserEntityByUserCredentials($username, $password, $grantType, } if (! $user) { - return; + return null; } elseif (method_exists($user, 'validateForPassportPasswordGrant')) { if (! $user->validateForPassportPasswordGrant($password)) { - return; + return null; } } elseif (! $this->hasher->check($password, $user->getAuthPassword())) { - return; + return null; } return new User($user->getAuthIdentifier()); diff --git a/src/Events/AccessTokenCreated.php b/src/Events/AccessTokenCreated.php index bcaf0d650..f40bf9ce2 100644 --- a/src/Events/AccessTokenCreated.php +++ b/src/Events/AccessTokenCreated.php @@ -14,7 +14,7 @@ class AccessTokenCreated /** * The ID of the user associated with the token. * - * @var string + * @var string|null */ public $userId; @@ -29,7 +29,7 @@ class AccessTokenCreated * Create a new event instance. * * @param string $tokenId - * @param string|int|null $userId + * @param string|null $userId * @param string $clientId * @return void */ diff --git a/src/Passport.php b/src/Passport.php index ccfe6455d..728cc20d9 100644 --- a/src/Passport.php +++ b/src/Passport.php @@ -40,7 +40,7 @@ class Passport * * @var string */ - public static $defaultScope; + public static $defaultScope = ''; /** * All of the scopes defined for the application. diff --git a/tests/Feature/AccessTokenControllerTest.php b/tests/Feature/AccessTokenControllerTest.php index 39d763983..215558e9a 100644 --- a/tests/Feature/AccessTokenControllerTest.php +++ b/tests/Feature/AccessTokenControllerTest.php @@ -96,8 +96,6 @@ public function testGettingAccessTokenWithClientCredentialsGrantInvalidClientSec $this->assertArrayHasKey('error_description', $decodedResponse); $this->assertSame('Client authentication failed', $decodedResponse['error_description']); $this->assertArrayNotHasKey('hint', $decodedResponse); - $this->assertArrayHasKey('message', $decodedResponse); - $this->assertSame('Client authentication failed', $decodedResponse['message']); $this->assertSame(0, Token::count()); } @@ -193,7 +191,6 @@ public function testGettingAccessTokenWithPasswordGrantWithInvalidPassword() $this->assertArrayHasKey('error', $decodedResponse); $this->assertSame('invalid_grant', $decodedResponse['error']); $this->assertArrayHasKey('error_description', $decodedResponse); - $this->assertArrayHasKey('message', $decodedResponse); $this->assertSame(0, Token::count()); } @@ -239,8 +236,6 @@ public function testGettingAccessTokenWithPasswordGrantWithInvalidClientSecret() $this->assertArrayHasKey('error_description', $decodedResponse); $this->assertSame('Client authentication failed', $decodedResponse['error_description']); $this->assertArrayNotHasKey('hint', $decodedResponse); - $this->assertArrayHasKey('message', $decodedResponse); - $this->assertSame('Client authentication failed', $decodedResponse['message']); $this->assertSame(0, Token::count()); } @@ -294,7 +289,7 @@ public function __construct($idToken) /** * @inheritdoc */ - protected function getExtraParams(\League\OAuth2\Server\Entities\AccessTokenEntityInterface $accessToken) + protected function getExtraParams(\League\OAuth2\Server\Entities\AccessTokenEntityInterface $accessToken): array { return [ 'id_token' => $this->idToken, diff --git a/tests/Unit/AuthorizationControllerTest.php b/tests/Unit/AuthorizationControllerTest.php index fe6e7f3ea..ac7fdb135 100644 --- a/tests/Unit/AuthorizationControllerTest.php +++ b/tests/Unit/AuthorizationControllerTest.php @@ -17,6 +17,7 @@ use League\OAuth2\Server\AuthorizationServer; use League\OAuth2\Server\Exception\OAuthServerException as LeagueException; use League\OAuth2\Server\RequestTypes\AuthorizationRequest; +use League\OAuth2\Server\RequestTypes\AuthorizationRequestInterface; use Mockery as m; use Nyholm\Psr7\Response; use PHPUnit\Framework\TestCase; @@ -44,7 +45,7 @@ public function test_authorization_view_is_presented() $guard->shouldReceive('guest')->andReturn(false); $guard->shouldReceive('user')->andReturn($user = m::mock()); - $server->shouldReceive('validateAuthorizationRequest')->andReturn($authRequest = m::mock()); + $server->shouldReceive('validateAuthorizationRequest')->andReturn($authRequest = m::mock(AuthorizationRequestInterface::class)); $request = m::mock(Request::class); $request->shouldReceive('session')->andReturn($session = m::mock()); @@ -269,7 +270,9 @@ public function test_authorization_denied_if_request_has_prompt_equals_to_none() $server->shouldReceive('completeAuthorizationRequest') ->with($authRequest, m::type(ResponseInterface::class)) ->once() - ->andThrow('League\OAuth2\Server\Exception\OAuthServerException'); + ->andReturnUsing(function () { + throw new \League\OAuth2\Server\Exception\OAuthServerException('', 0, ''); + }); $request = m::mock(Request::class); $request->shouldReceive('session')->andReturn($session = m::mock()); diff --git a/tests/Unit/BridgeAccessTokenRepositoryTest.php b/tests/Unit/BridgeAccessTokenRepositoryTest.php index aa99cd051..f8049d188 100644 --- a/tests/Unit/BridgeAccessTokenRepositoryTest.php +++ b/tests/Unit/BridgeAccessTokenRepositoryTest.php @@ -28,9 +28,9 @@ public function test_access_tokens_can_be_persisted() $events = m::mock(Dispatcher::class); $tokenRepository->shouldReceive('create')->once()->andReturnUsing(function ($array) use ($expiration) { - $this->assertSame(1, $array['id']); - $this->assertSame(2, $array['user_id']); - $this->assertSame('client-id', $array['client_id']); + $this->assertEquals(1, $array['id']); + $this->assertEquals(2, $array['user_id']); + $this->assertEquals('client-id', $array['client_id']); $this->assertEquals(['scopes'], $array['scopes']); $this->assertEquals(false, $array['revoked']); $this->assertInstanceOf(DateTime::class, $array['created_at']); @@ -63,6 +63,6 @@ public function test_can_get_new_access_token() $this->assertInstanceOf(AccessToken::class, $token); $this->assertEquals($client, $token->getClient()); $this->assertEquals($scopes, $token->getScopes()); - $this->assertSame($userIdentifier, $token->getUserIdentifier()); + $this->assertEquals($userIdentifier, $token->getUserIdentifier()); } } diff --git a/tests/Unit/BridgeClientRepositoryTest.php b/tests/Unit/BridgeClientRepositoryTest.php index 7c46fd73f..4b8b678c5 100644 --- a/tests/Unit/BridgeClientRepositoryTest.php +++ b/tests/Unit/BridgeClientRepositoryTest.php @@ -182,7 +182,7 @@ public function test_refresh_grant_is_prevented() } } -class BridgeClientRepositoryTestClientStub +class BridgeClientRepositoryTestClientStub extends \Laravel\Passport\Client { public $name = 'Client'; diff --git a/tests/Unit/BridgeScopeRepositoryTest.php b/tests/Unit/BridgeScopeRepositoryTest.php index e663bf89b..fb204ff53 100644 --- a/tests/Unit/BridgeScopeRepositoryTest.php +++ b/tests/Unit/BridgeScopeRepositoryTest.php @@ -44,7 +44,12 @@ public function test_invalid_scopes_are_removed_without_a_client_repository() 'scope-1' => 'description', ]); - $repository = new ScopeRepository(); + $clients = Mockery::mock(ClientRepository::class); + $clients->shouldReceive('findActive') + ->with('id') + ->andReturn(Mockery::mock(ClientModel::class)->makePartial()); + + $repository = new ScopeRepository($clients); $scopes = $repository->finalizeScopes( [$scope1 = new Scope('scope-1'), new Scope('scope-2')], 'client_credentials', new Client('id', 'name', 'http://localhost'), 1 @@ -148,7 +153,12 @@ public function test_superuser_scope_cant_be_applied_if_wrong_grant_without_a_cl 'scope-1' => 'description', ]); - $repository = new ScopeRepository(); + $clients = Mockery::mock(ClientRepository::class); + $clients->shouldReceive('findActive') + ->with('id') + ->andReturn(Mockery::mock(ClientModel::class)->makePartial()); + + $repository = new ScopeRepository($clients); $scopes = $repository->finalizeScopes( [$scope1 = new Scope('*')], 'refresh_token', new Client('id', 'name', 'http://localhost'), 1 diff --git a/tests/Unit/CheckClientCredentialsForAnyScopeTest.php b/tests/Unit/CheckClientCredentialsForAnyScopeTest.php index 807bf93d4..58ddd9f23 100644 --- a/tests/Unit/CheckClientCredentialsForAnyScopeTest.php +++ b/tests/Unit/CheckClientCredentialsForAnyScopeTest.php @@ -12,6 +12,7 @@ use League\OAuth2\Server\ResourceServer; use Mockery as m; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ServerRequestInterface; class CheckClientCredentialsForAnyScopeTest extends TestCase { @@ -23,7 +24,7 @@ protected function tearDown(): void public function test_request_is_passed_along_if_token_is_valid() { $resourceServer = m::mock(ResourceServer::class); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_user_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_access_token_id')->andReturn('token'); @@ -54,7 +55,7 @@ public function test_request_is_passed_along_if_token_is_valid() public function test_request_is_passed_along_if_token_has_any_required_scope() { $resourceServer = m::mock(ResourceServer::class); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_user_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_access_token_id')->andReturn('token'); @@ -109,7 +110,7 @@ public function test_exception_is_thrown_if_token_does_not_have_required_scope() $this->expectException('Laravel\Passport\Exceptions\MissingScopeException'); $resourceServer = m::mock(ResourceServer::class); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_user_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_access_token_id')->andReturn('token'); diff --git a/tests/Unit/CheckClientCredentialsTest.php b/tests/Unit/CheckClientCredentialsTest.php index 0a30b19ea..8c002a490 100644 --- a/tests/Unit/CheckClientCredentialsTest.php +++ b/tests/Unit/CheckClientCredentialsTest.php @@ -12,6 +12,7 @@ use League\OAuth2\Server\ResourceServer; use Mockery as m; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ServerRequestInterface; class CheckClientCredentialsTest extends TestCase { @@ -23,7 +24,7 @@ protected function tearDown(): void public function test_request_is_passed_along_if_token_is_valid() { $resourceServer = m::mock(ResourceServer::class); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_user_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_access_token_id')->andReturn('token'); @@ -54,7 +55,7 @@ public function test_request_is_passed_along_if_token_is_valid() public function test_request_is_passed_along_if_token_and_scope_are_valid() { $resourceServer = m::mock(ResourceServer::class); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_user_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_access_token_id')->andReturn('token'); @@ -108,7 +109,7 @@ public function test_exception_is_thrown_if_token_does_not_have_required_scopes( $this->expectException('Laravel\Passport\Exceptions\MissingScopeException'); $resourceServer = m::mock(ResourceServer::class); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_user_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_access_token_id')->andReturn('token'); diff --git a/tests/Unit/DenyAuthorizationControllerTest.php b/tests/Unit/DenyAuthorizationControllerTest.php index e2cd26f69..3497790ca 100644 --- a/tests/Unit/DenyAuthorizationControllerTest.php +++ b/tests/Unit/DenyAuthorizationControllerTest.php @@ -44,7 +44,9 @@ public function test_authorization_can_be_denied() $server->shouldReceive('completeAuthorizationRequest') ->with($authRequest, m::type(ResponseInterface::class)) - ->andThrow('League\OAuth2\Server\Exception\OAuthServerException'); + ->andReturnUsing(function () { + throw new \League\OAuth2\Server\Exception\OAuthServerException('', 0, ''); + }); $controller->deny($request); } diff --git a/tests/Unit/HandlesOAuthErrorsTest.php b/tests/Unit/HandlesOAuthErrorsTest.php index ed8dd1c1d..75bc43859 100644 --- a/tests/Unit/HandlesOAuthErrorsTest.php +++ b/tests/Unit/HandlesOAuthErrorsTest.php @@ -53,7 +53,7 @@ public function testShouldHandleOAuthServerException() $response = $e->render(new Request); $this->assertJsonStringEqualsJsonString( - '{"error":"fatal","error_description":"Error","message":"Error"}', + '{"error":"fatal","error_description":"Error"}', $response->getContent() ); } diff --git a/tests/Unit/HasApiTokensTest.php b/tests/Unit/HasApiTokensTest.php index 2abf68c95..68258a5b4 100644 --- a/tests/Unit/HasApiTokensTest.php +++ b/tests/Unit/HasApiTokensTest.php @@ -29,6 +29,8 @@ public function test_token_can_indicates_if_token_has_given_scope() public function test_token_can_be_created() { + $this->expectNotToPerformAssertions(); + $container = new Container; Container::setInstance($container); $container->instance(PersonalAccessTokenFactory::class, $factory = m::mock()); diff --git a/tests/Unit/PersonalAccessTokenFactoryTest.php b/tests/Unit/PersonalAccessTokenFactoryTest.php index c35127104..f20eaecef 100644 --- a/tests/Unit/PersonalAccessTokenFactoryTest.php +++ b/tests/Unit/PersonalAccessTokenFactoryTest.php @@ -16,6 +16,7 @@ use League\OAuth2\Server\AuthorizationServer; use Mockery as m; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ResponseInterface; class PersonalAccessTokenFactoryTest extends TestCase { @@ -34,7 +35,7 @@ public function test_access_token_can_be_created() $factory = new PersonalAccessTokenFactory($server, $clients, $tokens, $jwt); $clients->shouldReceive('personalAccessClient')->andReturn($client = new PersonalAccessTokenFactoryTestClientStub); - $server->shouldReceive('respondToAccessTokenRequest')->andReturn($response = m::mock()); + $server->shouldReceive('respondToAccessTokenRequest')->andReturn($response = m::mock(ResponseInterface::class)); $response->shouldReceive('getBody->__toString')->andReturn(json_encode([ 'access_token' => 'foo', ])); diff --git a/tests/Unit/TokenGuardTest.php b/tests/Unit/TokenGuardTest.php index 614393898..de2d4d234 100644 --- a/tests/Unit/TokenGuardTest.php +++ b/tests/Unit/TokenGuardTest.php @@ -20,6 +20,7 @@ use League\OAuth2\Server\ResourceServer; use Mockery as m; use PHPUnit\Framework\TestCase; +use Psr\Http\Message\ServerRequestInterface; class TokenGuardTest extends TestCase { @@ -42,7 +43,7 @@ public function test_user_can_be_pulled_via_bearer_token() $guard = new TokenGuard($resourceServer, $userProvider, $tokens, $clients, $encrypter, $request); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_user_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_access_token_id')->andReturn('token'); @@ -71,7 +72,7 @@ public function test_user_is_resolved_only_once() $guard = new TokenGuard($resourceServer, $userProvider, $tokens, $clients, $encrypter, $request); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_user_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_access_token_id')->andReturn('token'); @@ -137,7 +138,7 @@ public function test_null_is_returned_if_no_user_is_found() $guard = new TokenGuard($resourceServer, $userProvider, $tokens, $clients, $encrypter, $request); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_user_id')->andReturn(1); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $userProvider->shouldReceive('retrieveById')->with(1)->andReturn(null); @@ -444,7 +445,7 @@ public function test_client_can_be_pulled_via_bearer_token() $guard = new TokenGuard($resourceServer, $userProvider, $tokens, $clients, $encrypter, $request); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $clients->shouldReceive('findActive')->with(1)->andReturn(new TokenGuardTestClient); @@ -466,7 +467,7 @@ public function test_client_is_resolved_only_once() $guard = new TokenGuard($resourceServer, $userProvider, $tokens, $clients, $encrypter, $request); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $clients->shouldReceive('findActive')->with(1)->andReturn(new TokenGuardTestClient); @@ -521,7 +522,7 @@ public function test_null_is_returned_if_no_client_is_found() $guard = new TokenGuard($resourceServer, $userProvider, $tokens, $clients, $encrypter, $request); - $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock()); + $resourceServer->shouldReceive('validateAuthenticatedRequest')->andReturn($psr = m::mock(ServerRequestInterface::class)); $psr->shouldReceive('getAttribute')->with('oauth_client_id')->andReturn(1); $clients->shouldReceive('findActive')->with(1)->andReturn(null);