From f71249275cff81bd92255eff841f12516accc583 Mon Sep 17 00:00:00 2001 From: Hafez Divandari Date: Mon, 22 Jul 2024 18:36:02 +0330 Subject: [PATCH] [13.x] Fix user-token relations (#1773) * fix user-token relations * fix tests --- src/HasApiTokens.php | 24 ++++++++++------ src/PersonalAccessTokenFactory.php | 12 ++++---- src/Token.php | 4 ++- tests/Feature/AccessTokenControllerTest.php | 2 +- tests/Feature/HasApiTokensTest.php | 2 +- .../PersonalAccessTokenFactoryTest.php | 28 +++++++++++++------ 6 files changed, 45 insertions(+), 27 deletions(-) diff --git a/src/HasApiTokens.php b/src/HasApiTokens.php index f74e0565..c7b5742a 100644 --- a/src/HasApiTokens.php +++ b/src/HasApiTokens.php @@ -3,6 +3,7 @@ namespace Laravel\Passport; use Illuminate\Container\Container; +use LogicException; trait HasApiTokens { @@ -30,7 +31,18 @@ public function clients() */ public function tokens() { - return $this->hasMany(Passport::tokenModel(), 'user_id')->orderBy('created_at', 'desc'); + return $this->hasMany(Passport::tokenModel(), 'user_id') + ->where(function ($query) { + $query->whereHas('client', function ($query) { + $query->where(function ($query) { + $provider = $this->getProvider(); + + $query->when($provider === config('auth.guards.api.provider'), function ($query) { + $query->orWhereNull('provider'); + })->orWhere('provider', $provider); + }); + }); + }); } /** @@ -70,22 +82,18 @@ public function createToken($name, array $scopes = []) /** * Get the user provider name. - * - * @return string|null */ - public function getProvider(): ?string + public function getProvider(): string { $providers = collect(config('auth.guards'))->where('driver', 'passport')->pluck('provider')->all(); foreach (config('auth.providers') as $provider => $config) { - if (in_array($provider, $providers) - && (($config['driver'] === 'eloquent' && is_a($this, $config['model'])) - || ($config['driver'] === 'database' && $config['table'] === $this->getTable()))) { + if (in_array($provider, $providers) && $config['driver'] === 'eloquent' && is_a($this, $config['model'])) { return $provider; } } - return null; + throw new LogicException('Unable to determine authentication provider for this model from configuration.'); } /** diff --git a/src/PersonalAccessTokenFactory.php b/src/PersonalAccessTokenFactory.php index 97b2c695..ed825e00 100644 --- a/src/PersonalAccessTokenFactory.php +++ b/src/PersonalAccessTokenFactory.php @@ -65,10 +65,10 @@ public function __construct(AuthorizationServer $server, * @param mixed $userId * @param string $name * @param string[] $scopes - * @param string|null $provider + * @param string $provider * @return \Laravel\Passport\PersonalAccessTokenResult */ - public function make($userId, string $name, array $scopes = [], ?string $provider = null) + public function make($userId, string $name, array $scopes, string $provider) { $response = $this->dispatchRequestToAuthorizationServer( $this->createRequest($userId, $scopes, $provider) @@ -91,14 +91,12 @@ public function make($userId, string $name, array $scopes = [], ?string $provide * * @param mixed $userId * @param string[] $scopes - * @param string|null $provider + * @param string $provider * @return \Psr\Http\Message\ServerRequestInterface */ - protected function createRequest($userId, array $scopes, ?string $provider) + protected function createRequest($userId, array $scopes, string $provider) { - $config = $provider - ? config("passport.personal_access_client.$provider", config('passport.personal_access_client')) - : config('passport.personal_access_client'); + $config = config("passport.personal_access_client.$provider", config('passport.personal_access_client')); $client = isset($config['id']) ? $this->clients->findActive($config['id']) : null; diff --git a/src/Token.php b/src/Token.php index c78be4a9..a917eb49 100644 --- a/src/Token.php +++ b/src/Token.php @@ -68,11 +68,13 @@ public function refreshToken() /** * Get the user that the token belongs to. * + * @deprecated Will be removed in a future Laravel version. + * * @return \Illuminate\Database\Eloquent\Relations\BelongsTo */ public function user() { - $provider = config('auth.guards.api.provider'); + $provider = $this->client->provider ?: config('auth.guards.api.provider'); $model = config('auth.providers.'.$provider.'.model'); diff --git a/tests/Feature/AccessTokenControllerTest.php b/tests/Feature/AccessTokenControllerTest.php index 4e519bce..7f96ade3 100644 --- a/tests/Feature/AccessTokenControllerTest.php +++ b/tests/Feature/AccessTokenControllerTest.php @@ -145,7 +145,7 @@ public function testGettingAccessTokenWithPasswordGrant() $token = $this->app->make(PersonalAccessTokenFactory::class)->findAccessToken($decodedResponse); $this->assertInstanceOf(Token::class, $token); $this->assertFalse($token->revoked); - $this->assertTrue($token->user->is($user)); + $this->assertSame($user->getAuthIdentifier(), $token->user_id); $this->assertTrue($token->client->is($client)); $this->assertNull($token->name); $this->assertLessThanOrEqual(5, CarbonImmutable::now()->addSeconds($expiresInSeconds)->diffInSeconds($token->expires_at)); diff --git a/tests/Feature/HasApiTokensTest.php b/tests/Feature/HasApiTokensTest.php index b3b1b215..94d5e18a 100644 --- a/tests/Feature/HasApiTokensTest.php +++ b/tests/Feature/HasApiTokensTest.php @@ -16,7 +16,7 @@ public function testGetProvider() config([ 'auth.providers.admins' => ['driver' => 'eloquent', 'model' => AdminHasApiTokensStub::class], 'auth.guards.api-admins' => ['driver' => 'passport', 'provider' => 'admins'], - 'auth.providers.customers' => ['driver' => 'database', 'table' => 'customer_has_api_tokens_stubs'], + 'auth.providers.customers' => ['driver' => 'eloquent', 'model' => CustomerHasApiTokensStub::class], 'auth.guards.api-customers' => ['driver' => 'passport', 'provider' => 'customers'], ]); diff --git a/tests/Feature/PersonalAccessTokenFactoryTest.php b/tests/Feature/PersonalAccessTokenFactoryTest.php index 97d6997d..b23dfc2c 100644 --- a/tests/Feature/PersonalAccessTokenFactoryTest.php +++ b/tests/Feature/PersonalAccessTokenFactoryTest.php @@ -4,6 +4,7 @@ use Illuminate\Contracts\Hashing\Hasher; use Illuminate\Foundation\Auth\User as Authenticatable; +use Illuminate\Support\Facades\DB; use Laravel\Passport\Client; use Laravel\Passport\Database\Factories\ClientFactory; use Laravel\Passport\HasApiTokens; @@ -53,7 +54,7 @@ public function testIssueTokenWithDifferentProviders() config([ 'auth.providers.admins' => ['driver' => 'eloquent', 'model' => AdminProviderStub::class], 'auth.guards.api-admins' => ['driver' => 'passport', 'provider' => 'admins'], - 'auth.providers.customers' => ['driver' => 'database', 'table' => 'customer_provider_stubs'], + '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], @@ -80,6 +81,21 @@ public function testIssueTokenWithDifferentProviders() $this->assertInstanceOf(PersonalAccessTokenResult::class, $customerToken); $this->assertSame($customerClient->getKey(), $customerToken->token->client_id); $this->assertSame($customer->getAuthIdentifier(), $customerToken->token->user_id); + + DB::enableQueryLog(); + $userTokens = $user->tokens()->pluck('id')->all(); + $adminTokens = $admin->tokens()->pluck('id')->all(); + $customerTokens = $customer->tokens()->pluck('id')->all(); + DB::disableQueryLog(); + + $queries = DB::getRawQueryLog(); + $this->assertStringContainsString('and ("provider" is null or "provider" = \'users\')', $queries[0]['raw_query']); + $this->assertStringContainsString('and ("provider" = \'admins\')', $queries[1]['raw_query']); + $this->assertStringContainsString('and ("provider" = \'customers\')', $queries[2]['raw_query']); + + $this->assertEquals([$userToken->token->id], $userTokens); + $this->assertEquals([$adminToken->token->id], $adminTokens); + $this->assertEquals([$customerToken->token->id], $customerTokens); } } @@ -87,18 +103,12 @@ class AdminProviderStub extends Authenticatable { use HasApiTokens; - public function getAuthIdentifier() - { - return 'foo'; - } + protected $attributes = ['id' => 1]; } class CustomerProviderStub extends Authenticatable { use HasApiTokens; - public function getAuthIdentifier() - { - return 3; - } + protected $attributes = ['id' => 3]; }