waiting-for-dev / warden-jwt_auth

JWT token authentication with warden
MIT License
113 stars 56 forks source link

Updating to 0.3.1 broke my tests #12

Closed viamin closed 6 years ago

viamin commented 6 years ago

Updating to warden-jwt_auth 0.3.1 (from 0.3.0) breaks my RSpec controller tests that use before(:each) { sign_in :admin_user }

I started seeing the error message: "you need to sign in or sign up before continuing". This is with rails 5.1.4 and devise 4.3.0.

warden-jwt_auth was updated when updating devise-jwt to 0.5.1 (from 0.5.0). devise-jwt 0.5.1 and warden-jwt_auth 0.3.0 work fine together and the error goes away.

waiting-for-dev commented 6 years ago

Hi @viamin ,

the only change from 0.3.0 to 0.3.1 is that it ensures that scopes meant to be authenticated via JWT are never fetched from the session. So I guess that if your tests are failing is because you were authenticating them from the session, which could be a security threat for your application.

viamin commented 6 years ago

Reading your comment in https://github.com/hassox/warden/pull/118 makes this a bit more clear. That comment seems to make the assumption, however, that if you use JwtAuthenticatable, then you only want to log in with JwtAuthenticatable. In my case, I've created a session using the devise test helpers (sign_in :user) and it seems like it's being logged out because the user can be logged in using JWT.

Wouldn't it be simpler to use config.skip_session_storage = [:http_auth, :jwt_authenticatable] (or something similar) in config/initializers/devise.rb to achieve this when using devise-jwt?

I'll admit I'm only using the devise helpers because I don't want to rewrite every controller test I have to include an authentication header (and the code to set it up).

waiting-for-dev commented 6 years ago

Sorry if I wasn't clear enough in the previous comment.

The problem is not that jwt_authenticatable module persists the user to the session. It doesn't do it, because its associated strategy returns false in the #persist? method. The user is persisted to the session by devise database_authenticatable, and then warden automatically takes it from there (it is its behaviour: if the user is present in the session it escapes any strategy).

Until the issue your referenced is solved, I prefer to do a heavy assumption because the alternative is to leave an open door to a security threat if the user doesn't configure well everything (and he probably is not being conscious of the threat).

Notice that your tests are in essence wrong because they are not testing your expected behaviour for the application. I usually wrap the needed headers in a helper and use them for every request.

waiting-for-dev commented 6 years ago

Closing it, but feel free to add more comments or suggest a better approach to the issue.

waiting-for-dev commented 6 years ago

Added a helper to authenticate request headers for a user. Released in devise-jwt 0.5.2. You can read it in its README