diff --git a/UPGRADE.md b/UPGRADE.md index 0421175e..0d5e70e8 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -53,13 +53,13 @@ When authenticating users via bearer tokens, the `User` model's `token` method n ### Personal Access Client Table and Model Removal -PR: https://github.com/laravel/passport/pull/1749 +PR: https://github.com/laravel/passport/pull/1749, https://github.com/laravel/passport/pull/1780 Passport's `oauth_personal_access_clients` table has been redundant and unnecessary for several release cycles. Therefore, this release of Passport no longer interacts with this table or its corresponding model. If you wish, you may create a migration that drops this table: Schema::drop('oauth_personal_access_clients'); -In addition, the `Laravel\Passport\PersonalAccessClient` model, `Passport::$personalAccessClientModel` property, `Passport::usePersonalAccessClientModel()`, `Passport::personalAccessClientModel()`, and `Passport::personalAccessClient()` methods have been removed. +In addition, the `passport.personal_access_client` configuration value, `Laravel\Passport\PersonalAccessClient` model, `Passport::$personalAccessClientModel` property, `Passport::usePersonalAccessClientModel()`, `Passport::personalAccessClientModel()`, and `Passport::personalAccessClient()` methods have been removed. ## Upgrading To 12.0 From 11.x diff --git a/config/passport.php b/config/passport.php index dbdbfed1..c3b41b31 100644 --- a/config/passport.php +++ b/config/passport.php @@ -43,20 +43,4 @@ 'connection' => env('PASSPORT_CONNECTION'), - /* - |-------------------------------------------------------------------------- - | Personal Access Client - |-------------------------------------------------------------------------- - | - | If you enable client hashing, you should set the personal access client - | ID and unhashed secret within your environment file. The values will - | get used while issuing fresh personal access tokens to your users. - | - */ - - 'personal_access_client' => [ - 'id' => env('PASSPORT_PERSONAL_ACCESS_CLIENT_ID'), - 'secret' => env('PASSPORT_PERSONAL_ACCESS_CLIENT_SECRET'), - ], - ]; diff --git a/src/Bridge/ClientRepository.php b/src/Bridge/ClientRepository.php index 2ed2b45d..fe114f5f 100644 --- a/src/Bridge/ClientRepository.php +++ b/src/Bridge/ClientRepository.php @@ -36,17 +36,7 @@ public function getClientEntity(string $clientIdentifier): ?ClientEntityInterfac { $record = $this->clients->findActive($clientIdentifier); - if (! $record) { - return null; - } - - return new Client( - $clientIdentifier, - $record->name, - $record->redirect_uris, - $record->confidential(), - $record->provider - ); + return $record ? $this->fromClientModel($record) : null; } /** @@ -81,4 +71,28 @@ protected function verifySecret(string $clientSecret, string $storedHash): bool { return $this->hasher->check($clientSecret, $storedHash); } + + /** + * Get the personal access client for the given provider. + */ + public function getPersonalAccessClientEntity(string $provider): ?ClientEntityInterface + { + return $this->fromClientModel( + $this->clients->personalAccessClient($provider) + ); + } + + /** + * Create a new client entity from the given client model instance. + */ + protected function fromClientModel(ClientModel $model): ClientEntityInterface + { + return new Client( + $model->getKey(), + $model->name, + $model->redirect_uris, + $model->confidential(), + $model->provider + ); + } } diff --git a/src/Bridge/PersonalAccessGrant.php b/src/Bridge/PersonalAccessGrant.php index 4b23b7aa..2b1d8163 100644 --- a/src/Bridge/PersonalAccessGrant.php +++ b/src/Bridge/PersonalAccessGrant.php @@ -5,7 +5,6 @@ use DateInterval; use League\OAuth2\Server\Exception\OAuthServerException; use League\OAuth2\Server\Grant\AbstractGrant; -use League\OAuth2\Server\RequestEvent; use League\OAuth2\Server\ResponseTypes\ResponseTypeInterface; use Psr\Http\Message\ServerRequestInterface; @@ -20,20 +19,17 @@ public function respondToAccessTokenRequest( DateInterval $accessTokenTTL ): ResponseTypeInterface { // Validate request - $client = $this->validateClient($request); - - if (! $client->isConfidential()) { - $this->getEmitter()->emit(new RequestEvent(RequestEvent::CLIENT_AUTHENTICATION_FAILED, $request)); + if (! $userIdentifier = $this->getRequestParameter('user_id', $request)) { + throw OAuthServerException::invalidRequest('user_id'); + } - throw OAuthServerException::invalidClient($request); + if (! $provider = $this->getRequestParameter('provider', $request)) { + throw OAuthServerException::invalidRequest('provider'); } - $scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope)); - $userIdentifier = $this->getRequestParameter('user_id', $request); + $client = $this->clientRepository->getPersonalAccessClientEntity($provider); - if (! $userIdentifier) { - throw OAuthServerException::invalidRequest('user_id'); - } + $scopes = $this->validateScopes($this->getRequestParameter('scope', $request, $this->defaultScope)); // Finalize the requested scopes $scopes = $this->scopeRepository->finalizeScopes( diff --git a/src/ClientRepository.php b/src/ClientRepository.php index 61ecab47..7b71755b 100644 --- a/src/ClientRepository.php +++ b/src/ClientRepository.php @@ -4,6 +4,7 @@ use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Support\Str; +use RuntimeException; class ClientRepository { @@ -76,6 +77,29 @@ public function activeForUser($userId) })->values(); } + /* + * Get the latest active personal access client for the given user provider. + * + * @throws \RuntimeException + */ + public function personalAccessClient(string $provider): Client + { + return Passport::client() + ->where('revoked', false) + ->whereNull('user_id') + ->where(function ($query) use ($provider) { + $query->when($provider === config('auth.guards.api.provider'), function ($query) { + $query->orWhereNull('provider'); + })->orWhere('provider', $provider); + }) + ->latest() + ->get() + ->first(fn (Client $client) => $client->hasGrantType('personal_access')) + ?? throw new RuntimeException( + "Personal access client not found for '$provider' user provider. Please create one." + ); + } + /** * Store a new client. * diff --git a/src/Console/ClientCommand.php b/src/Console/ClientCommand.php index a783dedb..428647e8 100644 --- a/src/Console/ClientCommand.php +++ b/src/Console/ClientCommand.php @@ -68,19 +68,13 @@ protected function createPersonalAccessClient(ClientRepository $clients) $provider = $this->option('provider') ?: $this->choice( 'Which user provider should this client use to retrieve users?', - array_keys(config('auth.providers')), + collect(config('auth.guards'))->where('driver', 'passport')->pluck('provider')->all(), config('auth.guards.api.provider') ); - $client = $clients->createPersonalAccessGrantClient($name, $provider); + $clients->createPersonalAccessGrantClient($name, $provider); $this->components->info('Personal access client created successfully.'); - - if (! config('passport.personal_access_client')) { - $this->components->info('Next, define the `PASSPORT_PERSONAL_ACCESS_CLIENT_ID` and `PASSPORT_PERSONAL_ACCESS_CLIENT_SECRET` environment variables using the values below.'); - } - - $this->outputClientDetails($client); } /** @@ -98,7 +92,7 @@ protected function createPasswordClient(ClientRepository $clients) $provider = $this->option('provider') ?: $this->choice( 'Which user provider should this client use to retrieve users?', - array_keys(config('auth.providers')), + collect(config('auth.guards'))->where('driver', 'passport')->pluck('provider')->all(), config('auth.guards.api.provider') ); diff --git a/src/PersonalAccessTokenFactory.php b/src/PersonalAccessTokenFactory.php index ed825e00..bedfde47 100644 --- a/src/PersonalAccessTokenFactory.php +++ b/src/PersonalAccessTokenFactory.php @@ -7,7 +7,6 @@ use Nyholm\Psr7\Response; use Nyholm\Psr7\ServerRequest; use Psr\Http\Message\ServerRequestInterface; -use RuntimeException; class PersonalAccessTokenFactory { @@ -18,13 +17,6 @@ class PersonalAccessTokenFactory */ protected $server; - /** - * The client repository instance. - * - * @var \Laravel\Passport\ClientRepository - */ - protected $clients; - /** * The token repository instance. * @@ -43,20 +35,17 @@ class PersonalAccessTokenFactory * Create a new personal access token factory instance. * * @param \League\OAuth2\Server\AuthorizationServer $server - * @param \Laravel\Passport\ClientRepository $clients * @param \Laravel\Passport\TokenRepository $tokens * @param \Lcobucci\JWT\Parser $jwt * @return void */ public function __construct(AuthorizationServer $server, - ClientRepository $clients, TokenRepository $tokens, JwtParser $jwt) { $this->jwt = $jwt; $this->tokens = $tokens; $this->server = $server; - $this->clients = $clients; } /** @@ -96,20 +85,9 @@ public function make($userId, string $name, array $scopes, string $provider) */ protected function createRequest($userId, array $scopes, string $provider) { - $config = config("passport.personal_access_client.$provider", config('passport.personal_access_client')); - - $client = isset($config['id']) ? $this->clients->findActive($config['id']) : null; - - if (! $client || ($client->provider && $client->provider !== $provider)) { - throw new RuntimeException( - 'Personal access client not found. Please create one and set the `PASSPORT_PERSONAL_ACCESS_CLIENT_ID` and `PASSPORT_PERSONAL_ACCESS_CLIENT_SECRET` environment variables.' - ); - } - return (new ServerRequest('POST', 'not-important'))->withParsedBody([ 'grant_type' => 'personal_access', - 'client_id' => $config['id'] ?? null, - 'client_secret' => $config['secret'] ?? null, + 'provider' => $provider, 'user_id' => $userId, 'scope' => implode(' ', $scopes), ]); diff --git a/tests/Feature/AccessTokenControllerTest.php b/tests/Feature/AccessTokenControllerTest.php index 7f96ade3..fde77b30 100644 --- a/tests/Feature/AccessTokenControllerTest.php +++ b/tests/Feature/AccessTokenControllerTest.php @@ -269,49 +269,6 @@ public function testGettingCustomResponseType() $this->assertArrayHasKey('id_token', $decodedResponse); $this->assertSame('foo_bar_open_id_token', $decodedResponse['id_token']); } - - public function testPersonalAccessTokenRequestIsDisabled() - { - $user = UserFactory::new()->create([ - 'email' => 'foo@gmail.com', - 'password' => $this->app->make(Hasher::class)->make('foobar123'), - ]); - - /** @var Client $client */ - $client = ClientFactory::new()->asPersonalAccessTokenClient()->create(); - - config([ - 'passport.personal_access_client.id' => $client->getKey(), - 'passport.personal_access_client.secret' => $client->plainSecret, - ]); - - $response = $this->post( - '/oauth/token', - [ - 'grant_type' => 'personal_access', - 'client_id' => $client->getKey(), - 'client_secret' => $client->plainSecret, - 'user_id' => $user->getKey(), - 'scope' => '', - ] - ); - - $response->assertStatus(400); - - $decodedResponse = $response->decodeResponseJson()->json(); - - $this->assertArrayNotHasKey('token_type', $decodedResponse); - $this->assertArrayNotHasKey('expires_in', $decodedResponse); - $this->assertArrayNotHasKey('access_token', $decodedResponse); - - $this->assertArrayHasKey('error', $decodedResponse); - $this->assertSame('unsupported_grant_type', $decodedResponse['error']); - $this->assertArrayHasKey('error_description', $decodedResponse); - - $token = $user->createToken('test'); - - $this->assertInstanceOf(\Laravel\Passport\PersonalAccessTokenResult::class, $token); - } } class IdTokenResponse extends \League\OAuth2\Server\ResponseTypes\BearerTokenResponse diff --git a/tests/Feature/PersonalAccessTokenFactoryTest.php b/tests/Feature/PersonalAccessGrantTest.php similarity index 82% rename from tests/Feature/PersonalAccessTokenFactoryTest.php rename to tests/Feature/PersonalAccessGrantTest.php index b23dfc2c..ed53edce 100644 --- a/tests/Feature/PersonalAccessTokenFactoryTest.php +++ b/tests/Feature/PersonalAccessGrantTest.php @@ -2,7 +2,6 @@ namespace Laravel\Passport\Tests\Feature; -use Illuminate\Contracts\Hashing\Hasher; use Illuminate\Foundation\Auth\User as Authenticatable; use Illuminate\Support\Facades\DB; use Laravel\Passport\Client; @@ -13,25 +12,17 @@ use Orchestra\Testbench\Concerns\WithLaravelMigrations; use Workbench\Database\Factories\UserFactory; -class PersonalAccessTokenFactoryTest extends PassportTestCase +class PersonalAccessGrantTest extends PassportTestCase { use WithLaravelMigrations; public function testIssueToken() { - $user = UserFactory::new()->create([ - 'email' => 'foo@gmail.com', - 'password' => $this->app->make(Hasher::class)->make('foobar123'), - ]); + $user = UserFactory::new()->create(); /** @var Client $client */ $client = ClientFactory::new()->asPersonalAccessTokenClient()->create(); - config([ - 'passport.personal_access_client.id' => $client->getKey(), - 'passport.personal_access_client.secret' => $client->plainSecret, - ]); - Passport::tokensCan([ 'foo' => 'Do foo', 'bar' => 'Do bar', @@ -56,9 +47,6 @@ public function testIssueTokenWithDifferentProviders() 'auth.guards.api-admins' => ['driver' => 'passport', 'provider' => 'admins'], 'auth.providers.customers' => ['driver' => 'eloquent', 'model' => CustomerProviderStub::class], 'auth.guards.api-customers' => ['driver' => 'passport', 'provider' => 'customers'], - 'passport.personal_access_client' => ['id' => $client->getKey(), 'secret' => $client->plainSecret], - 'passport.personal_access_client.admins' => ['id' => $adminClient->getKey(), 'secret' => $adminClient->plainSecret], - 'passport.personal_access_client.customers' => ['id' => $customerClient->getKey(), 'secret' => $customerClient->plainSecret], ]); $user = UserFactory::new()->create(); @@ -97,6 +85,28 @@ public function testIssueTokenWithDifferentProviders() $this->assertEquals([$adminToken->token->id], $adminTokens); $this->assertEquals([$customerToken->token->id], $customerTokens); } + + public function testPersonalAccessTokenRequestIsDisabled() + { + $user = UserFactory::new()->create(); + $client = ClientFactory::new()->asPersonalAccessTokenClient()->create(); + + $response = $this->post('/oauth/token', [ + 'grant_type' => 'personal_access', + 'provider' => $user->getProvider(), + 'user_id' => $user->getKey(), + 'scope' => '', + ]); + + $response->assertStatus(400); + $json = $response->json(); + + $this->assertSame('unsupported_grant_type', $json['error']); + $this->assertArrayHasKey('error_description', $json); + $this->assertArrayNotHasKey('access_token', $json); + + $this->assertInstanceOf(PersonalAccessTokenResult::class, $user->createToken('test')); + } } class AdminProviderStub extends Authenticatable diff --git a/tests/Unit/BridgeClientRepositoryTest.php b/tests/Unit/BridgeClientRepositoryTest.php index 12a0477c..eb127832 100644 --- a/tests/Unit/BridgeClientRepositoryTest.php +++ b/tests/Unit/BridgeClientRepositoryTest.php @@ -210,6 +210,7 @@ public function test_without_grant_types() class BridgeClientRepositoryTestClientStub extends \Laravel\Passport\Client { protected $attributes = [ + 'id' => 1, 'name' => 'Client', 'redirect_uris' => '["http://localhost"]', 'secret' => '$2y$10$WgqU4wQpfsARCIQk.nPSOOiNkrMpPVxQiLCFUt8comvQwh1z6WFMG',