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

Authorization by access token requires database lookup #858

Closed
jasonlav opened this issue Oct 26, 2018 · 6 comments
Closed

Authorization by access token requires database lookup #858

jasonlav opened this issue Oct 26, 2018 · 6 comments

Comments

@jasonlav
Copy link

jasonlav commented Oct 26, 2018

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
Copy link
Member

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
Copy link
Author

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

@FlsZen
Copy link

FlsZen commented Oct 31, 2018

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
Copy link
Contributor

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:

  • Access tokens are good for 1 hour
  • Refresh tokens are good for 1 week
  • Request authorization trusts the JWT signature, it doesn't lookup tokens in the database
  • Using a refresh token does lookup the token in the database, so you can revoke them

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

acmitch commented Mar 24, 2020

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants