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

Consuming API with JavaScript with no valid session keeps returning users data until refreshing the page #909

Closed
hafezd opened this issue Dec 15, 2018 · 12 comments
Labels

Comments

@hafezd
Copy link

hafezd commented Dec 15, 2018

On Laravel 5.5 and Passport 4.0 with Consuming API With JavaScript.

I am trying to logout other devices when user changes password. On the app, I listen for change password event and revoke all refresh/access token of the user (except the current one) and \Illuminate\Session\Middleware\AuthenticateSession added to take care of sessions (I also tested removing all user's sessions from the db);
But the passport cookie is still valid and returns user data until page refresh, It is not enough to delete the cookie on logout event (Issue #85, PR #160) because in a case that user is logged out async and has no active session and no valid access token still can use the API with the passport cookie.

There should be a way to revoke transient tokens, invalidate passport cookies or maybe we should compare csrf of cookie with user's active sessions csrf.

@driesvints
Copy link
Member

Heya, unfortunately we don't support this version anymore. Can you please try to upgrade to the latest version and see if your problem persists? If so feel free to reply and we'll try to have a look.

@hafezd
Copy link
Author

hafezd commented Dec 17, 2018

@driesvints I can confirm that this problem persists on the latest version. As I said, the API still works (using passport cookie) after removing all user sessions and tokens from the db until manually refreshing the page.

@driesvints
Copy link
Member

How is it possible that API calls can be made with tokens which are revoked?

@driesvints driesvints reopened this Dec 17, 2018
@hafezd
Copy link
Author

hafezd commented Dec 19, 2018

@driesvints Steps to reproduce:

  1. Add the CreateFreshApiToken middleware to your web middleware group.
  2. Login to the app and get redirected to home
    • on this step, X-CSRF-TOKEN is being set.
    • Laravel passport cookie is being created.
    • and Api works as expected
  3. Delete all user sessions and revoke all access/refresh tokens
  4. Unexpectedly, API still works (because Passport compares cookie's token with headers X-CSRF-TOKEN and verifies it, although it is not a valid CSRF anymore and its session doesn't exists anymore)

protected function validCsrf($token, $request)
{
return isset($token['csrf']) && hash_equals(
$token['csrf'], (string) $request->header('X-CSRF-TOKEN')
);
}

@driesvints
Copy link
Member

@hafezdivandari can you please create a test app which reproduces this problem with the most minimal setup? I'd like to take a look at how you setup routes, controllers, middleware, etc.

@MovingGifts
Copy link

@hafezdivandari I thought we couldn't revoke tokens for a user created by CreateFreshApiToken, but only ones created by the Password Grant. Are you able to find a solution to your problem, really having a hard time understanding this.

@driesvints
Copy link
Member

Closing this issue because it's inactive, already solved, old or not relevant anymore. Feel free to reply if you're still experiencing this issue.

@jthunt
Copy link

jthunt commented Feb 15, 2019

FYI.. this is still an issue and came up on a security assessment of ours.

Reproduced doing the following:

  1. Setup passport and have routes protected by auth:api
  2. Use CreateFreshApiToken middleware
  3. Login. Record your CSRF token and laravel_token cookie.
  4. Log out (which should invalid the session and laravel_token)
  5. Using your CSRF token and laravel_token cookie in Postman still allows you to access the protected API route.

The issue is Passport is only checking the CSRF token against the laravel_token. It isn't validating if the laravel_token is still validate for a session in the sessions table.

At least that is my understanding of the problem here.

@driesvints
Copy link
Member

@jthunt hmm, I see. I'll try to look at this throughly when I find some time. Appreciating any help in the meantime.

@matt-allan
Copy link
Contributor

The laravel_token cookie is a JWT. Because we don't create an actual access token in the database for transient tokens there is nothing else to check against - if the JWT signature is valid and it hasn't expired yet it's considered valid.

This is really just one of the downsides of JWTs and there isn't much you can do. Any solution you can come up with ends up reinventing sessions. You can use the jti claim to assign a unique ID to each JWT and store either a whitelist or a blacklist but that sounds an awful lot like a session to me 😆.

All you can really do is set a short expiration time and delete the cookie on logout.

The expiration for the JWT is currently set to config('session.lifetime'). If you lower that value the JWT will expire faster, so i.e. it would only be valid for 5 minutes after logout.

It might be a good idea to make this configurable separately. It's OK if the session lifetime is longer because you can destroy it server side. The JWT on the other hand can't be revoked.

IMO it would be a good idea to update the docs to call this out, because it wasn't clear to me that the CreateFreshApiTokens tokens couldn't be revoked until I read the code.

If CreateFreshApiTokens used an actual client and created an actual token it would be possible to revoke the session, but that is a pretty major change.

@matt-allan
Copy link
Contributor

It's worth mentioning that even if you don't use the CreateFreshApiToken middleware a route is registered by default that allows creating the JWT, even if a JWT wasn't initially issued by the middleware.

@driesvints
Copy link
Member

Finally had time to look at this issue again. Thanks for clarifying so clearly @matt-allan. I've sent in a PR to the docs: laravel/docs#5993

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

No branches or pull requests

5 participants