Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TokenGuard query token twice #382

Closed
halaei opened this issue May 20, 2017 · 33 comments
Closed

TokenGuard query token twice #382

halaei opened this issue May 20, 2017 · 33 comments

Comments

@halaei
Copy link
Contributor

halaei commented May 20, 2017

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
@rdgout
Copy link

rdgout commented Jul 12, 2017

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

@gbdematos
Copy link

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)

@sylouuu
Copy link

sylouuu commented Nov 27, 2017

Same issue here.

@Kolimar
Copy link

Kolimar commented Mar 23, 2018

still happening

@hudovisk
Copy link

hudovisk commented Apr 1, 2018

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(

@raphaelbadia
Copy link

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

@fannyfan414
Copy link

Same issue, always 2 duplicated request to database

@CrashXD
Copy link

CrashXD commented Aug 24, 2018

Same issue

@pnlinh-it
Copy link

Same issue!

@calvinmuller
Copy link

Same issue here

@driesvints
Copy link
Member

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

@tylik1
Copy link

tylik1 commented Oct 19, 2018

@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

@driesvints
Copy link
Member

@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.

@ankurk91
Copy link
Contributor

It can be easily seen in debugbar

passport-duplcate-query

@driesvints
Copy link
Member

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

@ankurk91
Copy link
Contributor

Thanks but you forgot to re open this.

@vatichild
Copy link

vatichild commented Feb 8, 2019

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

@ankurk91
Copy link
Contributor

ankurk91 commented Feb 8, 2019

Thanks @Vatia13 for insights

@Patskimoto
Copy link

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

@driesvints
Copy link
Member

@Patskimoto We're open to pull requests.

@msdavid1296
Copy link

is there any way to avoid these repeated queries?

@colighto
Copy link

colighto commented Aug 12, 2019

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

@driesvints
Copy link
Member

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.

@chadidi
Copy link

chadidi commented Oct 2, 2019

Same issue here using Laravel 5.8 and Laravel Passport 7.2

@driesvints
Copy link
Member

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.

@electron93
Copy link

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')
);

@francsiswanto
Copy link

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.

@0plus1
Copy link

0plus1 commented Jun 4, 2020

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.

@taftse
Copy link

taftse commented Jun 6, 2020

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

@overtrue
Copy link

overtrue commented Jun 9, 2020

@Miljan9602
Copy link

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

@driesvints
Copy link
Member

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

@laravel laravel locked as spam and limited conversation to collaborators Jun 12, 2020
@driesvints
Copy link
Member

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.

@driesvints driesvints pinned this issue Aug 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet