laravel/passport

TokenGuard query token twice

halaei opened this issue ยท 34 comments

The following sql statement is queried twice during a call to TokenGuard::authenticateViaBearerToken() function:

select * from `oauth_access_tokens` where `oauth_access_tokens`.`id` = ? limit 1

I also see this happening on my API using Laravel Passport (v2.0.11)

Finally found someone with the same "problem", it's driving me nuts to figure out why this is happening.

I have a simple app to test it, installed passport and made vue make a request to some route with an authorization header. The following queries are executed, in this order:

select * from `oauth_access_tokens` where `oauth_access_tokens`.`id` = '[...]' limit 1
select * from `users` where `id` = '1' and `users`.`deleted_at` is null limit 1
select * from `oauth_access_tokens` where `oauth_access_tokens`.`id` = '[...]' limit 1
select * from `oauth_clients` where `oauth_clients`.`id` = '2' limit 1

(removed the token for presentation purposes)

Same issue here.

still happening

I am having the same issue here,
the first query is coming from a check to see if the token is revoked:

$psr = $this->server->validateAuthenticatedRequest($psr);

Then the oauth_access_tokens is queried again right after:

$token = $this->tokens->find(

Same issue here, the fun part is that it's happening in the same try block

Same issue, always 2 duplicated request to database

Same issue

Same issue!

Same issue here

Since this is an optimization rather than something blocking, feel free to send in a PR to solve this problem.

@driesvints this is not optimization, it certainly looks like a bug. For me Auth::user(); calls the query twice, on every single Ajax request i make.
Can you please reopen, even if it won't be solved now?

select * from "oauth_access_tokens" where "id" = token_id' limit 1

Backtrace:

First request:

15. /vendor/laravel/passport/src/TokenRepository.php:32
16. /vendor/laravel/passport/src/TokenRepository.php:108
17. /vendor/laravel/passport/src/Bridge/AccessTokenRepository.php:87
18. /vendor/league/oauth2-server/src/AuthorizationValidators/BearerTokenValidator.php:85
19. /vendor/league/oauth2-server/src/ResourceServer.php:84
20. /vendor/laravel/passport/src/Guards/TokenGuard.php:109
21. /vendor/laravel/passport/src/Guards/TokenGuard.php:89
22. /vendor/laravel/passport/src/PassportServiceProvider.php:272
25. /vendor/laravel/framework/src/Illuminate/Auth/AuthManager.php:292

Second request:

15. /vendor/laravel/passport/src/TokenRepository.php:32
16. /vendor/laravel/passport/src/Guards/TokenGuard.php:126
17. /vendor/laravel/passport/src/Guards/TokenGuard.php:89
18. /vendor/laravel/passport/src/PassportServiceProvider.php:272
21. /vendor/laravel/framework/src/Illuminate/Auth/AuthManager.php:292
22. /vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php:223

@tylik1 this isn't exactly blocking or causing issues. If you can optimize the process in a clean way which reduces the queries to one feel free to send in a PR.

It can be easily seen in debugbar

passport-duplcate-query

Okay. I'll re-open this but mark it as an enhancement for now.

Thanks but you forgot to re open this.

That's because laravel passport uses their own revoke check and second one is from league/oauth2-server required package. I think laravel passport should execute their check if token is revoked or find some other solution for it. I`ll show u both calls thet are in /passport/src/Guards/TokenGuard.php line 152

if ($this->clients->revoked($clientId)) { return; }

image

and /league/oauth2-server/src/AuthorizationValidators/BearerTokenValidator.php line 88

if ($this->accessTokenRepository->isAccessTokenRevoked($token->getClaim('jti'))) { throw OAuthServerException::accessDenied('Access token has been revoked'); }

image

image

Thanks @Vatia13 for insights

Just did an audit of my SQL queries and notice this is still happening as of Laravel 5.8 - any progress on this?

@Patskimoto We're open to pull requests.

is there any way to avoid these repeated queries?

in mysql the function uuid() generates a unique string key for primary keys that are defined as varchar or char it could also be called using a simple query like select uuid() string size could be 35 more details are provided about this function on this https://dev.mysql.com/doc/refman/8.0/en/miscellaneous-functions.html#function_uuid although there are equivalent functions on laravel that implements this like \Illuminate\Support\Str::uuid() and there is another package here as well https://packagist.org/packages/dyrynda/laravel-model-uuid

Can anyone please pinpoint the exact locations where these double calls are made in the current Passport code? Things have changed a bit since and I can't find them. And not inside the TokenRepository I mean. Where the calls to the repository are made.

Same issue here using Laravel 5.8 and Laravel Passport 7.2

Please stop reporting this or asking when this will be fixed. We need to pinpoint the exact locations where the calls are made (like I've asked above). The comments above don't help to solve the issue at hand.

Can anyone please pinpoint the exact locations where these double calls are made in the current Passport code? Things have changed a bit since and I can't find them. And not inside the TokenRepository I mean. Where the calls to the repository are made.

@driesvints

L127, begins in authenticateViaBearerToken($request)

protected function authenticateViaBearerToken($request)
{
if (! $psr = $this->getPsrRequestViaBearerToken($request)) {
return;
}

L175, calls TokenRepository::isAccessTokenRevoked($id) and then TokenRepository::find($id)

\League\OAuth2\Server\ResourceServer::validateAuthenticatedRequest($request)
\League\OAuth2\Server\AuthorizationValidators\BearerTokenValidator::validateAuthorization($request)

protected function getPsrRequestViaBearerToken($request)
{
// First, we will convert the Symfony request to a PSR-7 implementation which will
// be compatible with the base OAuth2 library. The Symfony bridge can perform a
// conversion for us to a Zend Diactoros implementation of the PSR-7 request.
$psr = (new DiactorosFactory)->createRequest($request);
try {
return $this->server->validateAuthenticatedRequest($psr);
} catch (OAuthServerException $e) {
$request->headers->set('Authorization', '', true);

L145, continues in authenticateViaBearerToken($request), calls TokenRepository::find($id)

// Next, we will assign a token instance to this user which the developers may use
// to determine if the token has a given scope, etc. This will be useful during
// authorization such as within the developer's Laravel model policy classes.
$token = $this->tokens->find(
$psr->getAttribute('oauth_access_token_id')
);

I need solution for this issue also. I'm still using Passport 5.0 on Laravel 5.6 installed on my client server, and there is no way to upgrade into latest version. I hope somebody can help. Thank you.

I am currently running a large scale application and this has a severe performance impact as they are all single transactions against the DB.. ended up writing my own TokenRepository to curb this issue. While I agree this isn't a bug, it's a deal breaker on large scale applications.

@0plus1 how about sharing how you solved it or sending a pull request

This bug still happens... Laravel: v7.14.1 and Passport: v8.5.0

I'm gonna close this to prevent more "this is still a bug" comments. Welcoming prs.

Since nobody has attempted any more prs I'm closing this. We're still welcoming prs if anyone wants to take up work on this.

Great news all. It seems there's finally coming a fix for this one in Passport v13: #1751