Skip to content

Commit

Permalink
[13.x] Fix user-token relations (#1773)
Browse files Browse the repository at this point in the history
* fix user-token relations

* fix tests
  • Loading branch information
hafezdivandari authored Jul 22, 2024
1 parent ef640ef commit f712492
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 27 deletions.
24 changes: 16 additions & 8 deletions src/HasApiTokens.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Laravel\Passport;

use Illuminate\Container\Container;
use LogicException;

trait HasApiTokens
{
Expand Down Expand Up @@ -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);
});
});
});
}

/**
Expand Down Expand Up @@ -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.');
}

/**
Expand Down
12 changes: 5 additions & 7 deletions src/PersonalAccessTokenFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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;

Expand Down
4 changes: 3 additions & 1 deletion src/Token.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
2 changes: 1 addition & 1 deletion tests/Feature/AccessTokenControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion tests/Feature/HasApiTokensTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
]);

Expand Down
28 changes: 19 additions & 9 deletions tests/Feature/PersonalAccessTokenFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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],
Expand All @@ -80,25 +81,34 @@ 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);
}
}

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];
}

0 comments on commit f712492

Please sign in to comment.