laravel / passport

Laravel Passport provides OAuth2 server support to Laravel.
https://laravel.com/docs/passport
MIT License
3.28k stars 777 forks source link

Authorization by access token requires database lookup #858

Closed jasonlav closed 4 years ago

jasonlav commented 5 years ago

According to the OAuth 2.0 documentation, access tokens are self-encoded and do not require a database lookup.

When the access token expires, the application can use the refresh token to obtain a new access token. It can do this behind the scenes, and without the user’s involvement, so that it’s a seamless process to the user.

The main benefit of this approach is that the service can use self-encoded access tokens which can be verified without a database lookup. However, this means there is no way to expire those tokens directly, so instead, the tokens are issued with a short expiration time so that the application is forced to continually refresh them, giving the service a chance to revoke an application’s access if needed.

An password access token requires three database lookups to complete verification.

https://github.com/laravel/passport/blob/v7.0.2/src/Bridge/AccessTokenRepository.php#L85 https://github.com/laravel/passport/blob/v7.0.2/src/Guards/TokenGuard.php#L125 https://github.com/laravel/passport/blob/v7.0.2/src/Guards/TokenGuard.php#L134

This approach invalidates the purpose of having access and refresh tokens.

To my knowledge, there is no way to toggle this functionality within Passport. Is this a feature that Passport would consider?

driesvints commented 5 years ago

I don't really get what you mean. You say:

According to the OAuth 2.0 documentation, access tokens are self-encoded and do require a database lookup.

But the spec says:

The main benefit of this approach is that the service can use self-encoded access tokens which can be verified without a database lookup.

And Passport does perform database lookups (as you've pointed out). Did you mistype your first sentence? Did you mean that Passport shouldn't perform database checks?

jasonlav commented 5 years ago

Yes, that was a typo. I apologize for the confusion. I have updated the original post.

FlsZen commented 5 years ago

Perhaps adding an option in Passport.php so these additional checks can be disabled in boot() would be appropriate. This would be a good compliment to setting tokensExpireIn to a short-lived duration.

matt-allan commented 5 years ago

This issue wants Passport to stop looking up tokens in the DB and to trust the JWT, but #909 is because passport already does this for the cookie.

The linked documentation states that you can use self-encoded access tokens, it's not a requirement of using OAuth.

The big benefit of looking up the token in the database is they can be revoked. If Passport trusted the JWT than the JWT would remain valid until it expired. Since Passport defaults to only expiring tokens after 1 year this would be really bad for security.

You probably don't want someone to continue having access to the user's data for an entire year after they have changed their password, logged out, etc.

The next paragraph the linked article it states:

However, this means there is no way to expire those tokens directly, so instead, the tokens are issued with a short expiration time so that the application is forced to continually refresh them, giving the service a chance to revoke an application’s access if needed.

If you wanted to trust the JWT you would probably want to do something like:

It's important to point out that trusting the JWT means that if someone got ahold of the APP_KEY they could forge JWTs and pretend to be any user they want. The database lookup prevents that from being an issue, since they would need to have the APP_KEY and know the token ID of an active token for that user.

The guard still needs to fetch the user from the database as soon as the token is verified, so even if you did this you would still be making a database query. The JWT only makes sense IMO when the OAuth server has it's own database and you are trying to avoid making a HTTP request to the OAuth server for every request - you can use the public key to validate the JWT instead.

acmitch commented 4 years ago

I agree. That's the purpose of the signature, there is no major advantage in using JWT if you are going to couple yourself to the database with every call.

I think the larger concern, in general, is that the coupling to the database doesn't allow Passport to be used with microservices or outside the scope of a monolithic application. Using Passport, a developer should be able to create a micro-service that generates access tokens, an Authentication Server and multiple Resource Servers.

I think Passport should implement a secure endpoint for token validation, so developers are not limited to the guard for validation. Also, add support for a ResourceServer ServiceProvider for resource only applications. This provider should be configurable and allow the developer to use the new secure endpoint or verify the signature of the JWT with every incoming request.

driesvints commented 4 years ago

@acmitch we have no plans for that atm I'm afraid.

I agree with @matt-allan. The revoking of tokens functionality is crucial to Passport so closing this for now.