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

Does not account for devise-jwt's revocation strategy #5

Closed
derekyau opened this issue Aug 8, 2021 · 5 comments
Closed

Does not account for devise-jwt's revocation strategy #5

derekyau opened this issue Aug 8, 2021 · 5 comments

Comments

@derekyau
Copy link

derekyau commented Aug 8, 2021

This gem works great! One thing that I noticed was that using standard devise-jwt and their revocation strategies, when I send a logout request (set via config.jwt.revocation_requests) it removes the token from the database based upon the strategy chosen.

After I installed devise-jwt-cookie the logout call was made, but the revocation strategy did not run. I'm newer to this, but taking a brief look at the source code for devise-jwt it seems to expect there to be an HTTP_AUTHORIZATION header such that the proper database entry can be revoked. As after you install this gem, we no longer send that header (and rather use cookies) it does not seem to know to revoke it.

For the time being, a quick workaround seems to be to add some middleware that injects it back in. Something like:

  def call(env)
    if Devise::JWT::Cookie::Middleware.new(app).token_should_be_revoked?(env)
      request = Rack::Request.new(env)
      env['HTTP_AUTHORIZATION'] = "Bearer #{request.cookies[Devise::JWT::Cookie.config.name]}"
    end
  
     status, headers, response = app.call(env)
    [status, headers, response]
  end

This does work, but I'm wondering whether there's a better way? Or whether we should build this into the library itself?

Thanks !

@scarhand
Copy link
Owner

Thanks for the feedback. It's been a while since I worked on this, and don't know from the top of my head how the revocation strategies work (or should work :)).

It could be that it just unsets/removes the cookie, but does not run the revocation strategy. I'd have to dig a bit deeper to find out what happens exactly. I'll set up a test project and I'll see if I can find out what happens. Could take some time though, I'm a bit wrapped up in other things at the moment.

@scarhand
Copy link
Owner

@derekyau which revocation strategy are you normally using? The Allowlist, or the Denylist?

@derekyau
Copy link
Author

@scarhand I'm using the Allowlist in my app. Yeah, after adding the header back in with that middleware hack it seems to work for now

Great work on this btw, I was shocked there wasn't a library for Rails that did httpOnly cookie based authentication, this was the perfect fit.

scarhand added a commit that referenced this issue Aug 23, 2021
…and before the rest of the middleware is called.

Related to #5

Bump version to 0.5.0
@scarhand
Copy link
Owner

I've made some changes that should add the header if the token should be revoked. It essentially does the same as your middleware work-around, but in the gem itself. Thanks for the hint :)

I've built and pushed a new version. Could you let me know if this solves the issue, and removes the need for the additional middleware?

@derekyau
Copy link
Author

@scarhand it works! Glad my middleware snippet helped :)

Awesome, issue solved

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

No branches or pull requests

2 participants