diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e83e9999..282bccee 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -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 }} diff --git a/UPGRADE.md b/UPGRADE.md index c26ef418..fdf9f9e6 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -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 diff --git a/composer.json b/composer.json index 2dcb1cad..2ca2b791 100644 --- a/composer.json +++ b/composer.json @@ -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": { diff --git a/src/AccessToken.php b/src/AccessToken.php index 33c54a59..328baf80 100644 --- a/src/AccessToken.php +++ b/src/AccessToken.php @@ -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 + * + * @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; @@ -23,7 +31,7 @@ class AccessToken /** * All the attributes set on the access token instance. * - * @var array + * @var array */ protected array $attributes = []; @@ -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); } /** @@ -90,7 +84,7 @@ 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]); } /** @@ -98,7 +92,37 @@ public function revoke(): bool */ 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 + */ + public function toArray(): array + { + return $this->attributes; + } + + /** + * Convert the object into something JSON serializable. + * + * @return array + */ + 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); } /** @@ -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]; diff --git a/src/Bridge/ScopeRepository.php b/src/Bridge/ScopeRepository.php index 308f67d0..0cf73f73 100644 --- a/src/Bridge/ScopeRepository.php +++ b/src/Bridge/ScopeRepository.php @@ -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; @@ -10,17 +12,11 @@ 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; } /** @@ -28,11 +24,7 @@ public function __construct(ClientRepository $clients) */ public function getScopeEntityByIdentifier(string $identifier): ?ScopeEntityInterface { - if (Passport::hasScope($identifier)) { - return new Scope($identifier); - } - - return null; + return Passport::hasScope($identifier) ? new Scope($identifier) : null; } /** @@ -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(); } } diff --git a/src/Client.php b/src/Client.php index 3c3e2d4d..9eea7aa5 100644 --- a/src/Client.php +++ b/src/Client.php @@ -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); } /** diff --git a/src/ClientRepository.php b/src/ClientRepository.php index c0f3b066..3fb3b501 100644 --- a/src/ClientRepository.php +++ b/src/ClientRepository.php @@ -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); diff --git a/src/Http/Controllers/AuthorizationController.php b/src/Http/Controllers/AuthorizationController.php index 87491199..4cd76dd0 100644 --- a/src/Http/Controllers/AuthorizationController.php +++ b/src/Http/Controllers/AuthorizationController.php @@ -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(); } /** diff --git a/src/PassportServiceProvider.php b/src/PassportServiceProvider.php index f2cce1c9..9c96f22b 100644 --- a/src/PassportServiceProvider.php +++ b/src/PassportServiceProvider.php @@ -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(); diff --git a/src/ResolvesInheritedScopes.php b/src/ResolvesInheritedScopes.php index e7658a16..b277cf4d 100644 --- a/src/ResolvesInheritedScopes.php +++ b/src/ResolvesInheritedScopes.php @@ -4,13 +4,32 @@ trait ResolvesInheritedScopes { + /** + * Determine if a scope exists or inherited in the given array. + * + * @param string[] $haystack + */ + protected function scopeExists(string $scope, array $haystack): bool + { + $scopes = Passport::$withInheritedScopes + ? $this->resolveInheritedScopes($scope) + : [$scope]; + + foreach ($scopes as $scope) { + if (in_array($scope, $haystack)) { + return true; + } + } + + return false; + } + /** * Resolve all possible scopes. * - * @param string $scope - * @return array + * @return string[] */ - protected function resolveInheritedScopes($scope) + protected function resolveInheritedScopes(string $scope): array { $parts = explode(':', $scope); diff --git a/tests/Feature/AuthorizationCodeGrantTest.php b/tests/Feature/AuthorizationCodeGrantTest.php new file mode 100644 index 00000000..cb76f463 --- /dev/null +++ b/tests/Feature/AuthorizationCodeGrantTest.php @@ -0,0 +1,224 @@ + 'Create', + 'read' => 'Read', + 'update' => 'Update', + 'delete' => 'Delete', + ]); + + Passport::authorizationView(fn ($params) => $params); + } + + public function testIssueAccessToken() + { + $client = ClientFactory::new()->create(); + + $query = http_build_query([ + 'client_id' => $client->getKey(), + 'redirect_uri' => $redirect = $client->redirect_uris[0], + 'response_type' => 'code', + 'scope' => 'create read', + 'state' => $state = Str::random(40), + ]); + + $user = UserFactory::new()->create(); + $this->actingAs($user, 'web'); + + $response = $this->get('/oauth/authorize?'.$query); + + $response->assertOk(); + $response->assertSessionHas('authRequest'); + $response->assertSessionHas('authToken'); + $json = $response->json(); + $this->assertEqualsCanonicalizing(['client', 'user', 'scopes', 'request', 'authToken'], array_keys($json)); + $this->assertSame(collect(Passport::scopesFor(['create', 'read']))->toArray(), $json['scopes']); + + $response = $this->post('/oauth/authorize', ['auth_token' => $json['authToken']]); + $response->assertRedirect(); + $response->assertSessionMissing(['deviceCode', 'authToken']); + + $location = $response->headers->get('Location'); + parse_str(parse_url($location, PHP_URL_QUERY), $params); + + $this->assertStringStartsWith($redirect.'?', $location); + $this->assertSame($state, $params['state']); + $this->assertArrayHasKey('code', $params); + + $response = $this->post('/oauth/token', [ + 'grant_type' => 'authorization_code', + 'client_id' => $client->getKey(), + 'client_secret' => $client->plainSecret, + 'redirect_uri' => $redirect, + 'code' => $params['code'], + ]); + + $response->assertOk(); + $json = $response->json(); + $this->assertArrayHasKey('access_token', $json); + $this->assertArrayHasKey('refresh_token', $json); + $this->assertSame('Bearer', $json['token_type']); + $this->assertSame(31536000, $json['expires_in']); + + Route::get('/foo', fn (Request $request) => $request->user()->token()->toJson()) + ->middleware('auth:api'); + + $json = $this->withToken($json['access_token'], $json['token_type'])->get('/foo')->json(); + + $this->assertSame($client->getKey(), $json['oauth_client_id']); + $this->assertEquals($user->getAuthIdentifier(), $json['oauth_user_id']); + $this->assertSame(['create', 'read'], $json['oauth_scopes']); + } + + public function testDenyAuthorization() + { + $client = ClientFactory::new()->create(); + + $query = http_build_query([ + 'client_id' => $client->getKey(), + 'redirect_uri' => $redirect = $client->redirect_uris[0], + 'response_type' => 'code', + 'scope' => '', + 'state' => $state = Str::random(40), + ]); + + $user = UserFactory::new()->create(); + $this->actingAs($user, 'web'); + $json = $this->get('/oauth/authorize?'.$query)->json(); + + $response = $this->delete('/oauth/authorize', ['auth_token' => $json['authToken']]); + $response->assertRedirect(); + $response->assertSessionMissing(['deviceCode', 'authToken']); + + $location = $response->headers->get('Location'); + parse_str(parse_url($location, PHP_URL_QUERY), $params); + + $this->assertStringStartsWith($redirect.'?', $location); + $this->assertSame($state, $params['state']); + $this->assertSame('access_denied', $params['error']); + $this->assertArrayHasKey('error_description', $params); + } + + public function testSkipsAuthorizationWhenHasGrantedScopes() + { + $client = ClientFactory::new()->create(); + + $query = http_build_query([ + 'client_id' => $client->getKey(), + 'redirect_uri' => $redirect = $client->redirect_uris[0], + 'response_type' => 'code', + 'scope' => 'create read update', + 'state' => Str::random(40), + ]); + + $user = UserFactory::new()->create(); + $this->actingAs($user, 'web'); + $json = $this->get('/oauth/authorize?'.$query)->json(); + + $response = $this->post('/oauth/authorize', ['auth_token' => $json['authToken']]); + parse_str(parse_url($response->headers->get('Location'), PHP_URL_QUERY), $params); + + $this->post('/oauth/token', [ + 'grant_type' => 'authorization_code', + 'client_id' => $client->getKey(), + 'client_secret' => $client->plainSecret, + 'redirect_uri' => $redirect, + 'code' => $params['code'], + ]); + + $query = http_build_query([ + 'client_id' => $client->getKey(), + 'redirect_uri' => $redirect, + 'response_type' => 'code', + 'scope' => 'create read', + 'state' => $state = Str::random(40), + ]); + + $response = $this->get('/oauth/authorize?'.$query); + $response->assertRedirect(); + + $location = $response->headers->get('Location'); + parse_str(parse_url($location, PHP_URL_QUERY), $params); + + $this->assertStringStartsWith($redirect.'?', $location); + $this->assertSame($state, $params['state']); + $this->assertArrayHasKey('code', $params); + } + + public function testValidateAuthorizationRequest() + { + $client = ClientFactory::new()->create(); + + $query = http_build_query([ + 'client_id' => $client->getKey(), + 'redirect_uri' => fake()->url(), + 'response_type' => 'code', + 'scope' => '', + 'state' => Str::random(40), + ]); + + $json = $this->get('/oauth/authorize?'.$query)->json(); + $this->assertSame('invalid_client', $json['error']); + $this->assertArrayHasKey('error_description', $json); + } + + public function testRedirectGuestUser() + { + Route::get('/foo', fn () => '')->name('login'); + + $client = ClientFactory::new()->create(); + + $query = http_build_query([ + 'client_id' => $client->getKey(), + 'redirect_uri' => $client->redirect_uris[0], + 'response_type' => 'code', + ]); + + $response = $this->get('/oauth/authorize?'.$query); + $response->assertSessionHas('promptedForLogin', true); + $response->assertRedirectToRoute('login'); + } + + public function testPromptNone() + { + $client = ClientFactory::new()->create(); + + $query = http_build_query([ + 'client_id' => $client->getKey(), + 'redirect_uri' => $redirect = $client->redirect_uris[0], + 'response_type' => 'code', + 'state' => $state = Str::random(40), + 'prompt' => 'none', + ]); + + $this->actingAs(UserFactory::new()->create(), 'web'); + $response = $this->get('/oauth/authorize?'.$query); + $response->assertRedirect(); + + $location = $response->headers->get('Location'); + parse_str(parse_url($location, PHP_URL_QUERY), $params); + + $this->assertStringStartsWith($redirect.'?', $location); + $this->assertSame($state, $params['state']); + $this->assertSame('access_denied', $params['error']); + $this->assertArrayHasKey('error_description', $params); + } +} diff --git a/tests/Unit/AuthorizationControllerTest.php b/tests/Unit/AuthorizationControllerTest.php index cb925834..a653e16a 100644 --- a/tests/Unit/AuthorizationControllerTest.php +++ b/tests/Unit/AuthorizationControllerTest.php @@ -59,7 +59,7 @@ public function test_authorization_view_is_presented() $clients = m::mock(ClientRepository::class); $clients->shouldReceive('find')->with(1)->andReturn($client = m::mock(Client::class)); $client->shouldReceive('skipsAuthorization')->andReturn(false); - $client->shouldReceive('tokens->where->pluck->flatten')->andReturn(collect()); + $client->shouldReceive('tokens->where->pluck')->andReturn(collect()); $user->shouldReceive('getAuthIdentifier')->andReturn(1); @@ -139,7 +139,7 @@ public function test_request_is_approved_if_valid_token_exists() $client->shouldReceive('skipsAuthorization')->andReturn(false); $client->shouldReceive('getKey')->andReturn(1); - $client->shouldReceive('tokens->where->pluck->flatten')->andReturn(collect(['scope-1'])); + $client->shouldReceive('tokens->where->pluck')->andReturn(collect([['scope-1']])); $this->assertSame('approved', $controller->authorize( m::mock(ServerRequestInterface::class), $request, $clients @@ -275,7 +275,7 @@ public function test_authorization_denied_if_request_has_prompt_equals_to_none() $clients->shouldReceive('find')->with(1)->andReturn($client = m::mock(Client::class)); $client->shouldReceive('skipsAuthorization')->andReturn(false); $client->shouldReceive('getKey')->andReturn(1); - $client->shouldReceive('tokens->where->pluck->flatten')->andReturn(collect()); + $client->shouldReceive('tokens->where->pluck')->andReturn(collect()); $controller->authorize( m::mock(ServerRequestInterface::class), $request, $clients