-
Notifications
You must be signed in to change notification settings - Fork 29
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
Usage of warden to store sessions #7
Comments
hey @ArneZsng thank you for the issue and the pr. I agree something is wrong here, but I don't think removing warden is the solution. warden allows us to interact with signed in users through devise / other warden apps, and it is in devise_token_auth. The goal with that is to not have users sign in again when first signing in to front end JS apps. Could you elaborate on how you are serving your front end code (ng-token-auth, j-toker, whatever)? Is it being accessed by a point where the user is already signed in to the rails app through devise? |
@ArneZsng @mcordell My issue was that if I first sign in with User1, then sign out of User1. Then sign in with User2, I will still be recognized as signed in as User1. Though the initial return of the header is correct(User2 still got the correct uid after sign in), but next requests authorized by grape_devise_token_auth, the server will return the wrong header(with the uid and token of previous signed in User1) This issue was also raised at the devise_token_auth repository. lynndylanhurley/devise_token_auth#375 Thanks |
@mcordell Thank you for looking into this. We are using a React.js single page app for the frontend with no specific auth framework. (No j-toker as we are not relying on jQuery) The problem I am having with the current implementation is the same as @ianchen06 described: However, when I call a grape endpoint authenticated by grape_devise_token_auth in between, User1 stays logged in, for example: This error is caused because the new client header cannot be found in the tokens for User1 who is still considered logged in. As per my understanding, devise_token_auth is using Warden only to check if an active user is already logged in (via devise for example) but does not set a user who authenticated via the API in Warden. Thus my proposal to remove Warden entirely. However, if Warden is relied on in other parts of the application, we should try to find a way for the user to only exist within a request and to end the Warden session after each request. Can you clarify what you mean by "The goal with that is to not have users sign in again when first signing in to front end JS apps."? |
@ArneZsng and @ianchen06 if you do what is proposed in that thread and pin devise_token_auth to 1.3.1, does that resolve your problem? I'm having trouble reproducing the problem in the test app with:
|
I do not believe that is the case, at least not in the current code. Yes, it checks for the devise user, here. But then if the token is valid, it signs the user into devise/warden here. Which is using this devise method. Whats confusing is the arguments, All that is to say, I think the intent is not to store the resource in the session. So, I changed the code here . @ArneZsng and @ianchen06 can you try that branch ( @ArneZsng if the above does not resolve can you paste into this thread:
Thank you both for the help, I appreciate it especially since I am having trouble reproducing this. |
Having the same issue with the Ionic App,
|
my work around is to delete the cookie session key on response back, everywhere in Rails and in Grape, not ideal, but yea.. |
@mcordell :+1 on your spike |
@justinsoong so the |
@mcordell I'm having the exact same issue as described above. I've tried the warden-spike branch and I'm running into an issue where authenticate_user! is giving me an "ArgumentError Exception: wrong number of arguments (3 for 1..2)". Any thoughts? |
I've resolved my issue for the time being by adding a before action of |
Previously, GDTA was storing the authenticated user in the session. This is likely not what was desired as evidenced by issue #7. Therefore, this commit changes warden to only sign in a user for that request. Since the user is not stored in session, the user has to be pulled from warden (rather than the warden through the session).
@dhosterman I fixed the warden-spike branch, could you try it out (without your above fix). It should resolve the 3 for 1..2 error and hopefully the larger issue at hand. I rebased the branch so you will likely need an update with bundler. If someone will confirm that the issue is fixed I'll bump the version and merge it into master. |
@mcordell Thanks for looking into this and the update! I can confirm that the 3 for 1..2 error has been resolved, but I'm still getting the same behavior with regards to users. I've confirmed I received the changes and ran a bundle update on the gem to ensure the application was using the newest code. Sorry! |
No problem, new tact. @dhosterman I've added a new commit to warden-spike to disable the warden existing user checking. Bundle update again from that branch. And then where you have your GrapeDeviseTokenAuth.setup! do |config|
config.ignore_existing_warden_users = true
end Let me know if that fixes it, thanks! |
@mcordell Thank you for the fix. |
Bumped to 0.1.3 and released. Thanks all |
I think I have to reopen this issue (or create a new one regarding this problem). After calling devise_token_auth's validate_token, an error is thrown when I call any grape endpoint with validation due to valid? being undefined for Hash as warden.user(:user) is a hash representation of the user object. NoMethodError (undefined method My guess is that though using the spike with storing in warden being turned off, a user is stored in Warden anyway by devise_token_auth's validate_token call. |
I am a little bit confused by the warden logic in this gem. From my understanding, each request in devise_token_auth is authenticated individually by its request headers. As I don't have a deeper understanding of Warden, this might be complete BS, so any help is appreciated:
In my example, I am using a Grape endpoint that needs to be available in a "light" version if the user is not logged in and will return more information if the user is logged in.
Thus, I played around with the request headers and noticed that the user is constantly logged in after the first log in, no matter if I changed the request headers or not. Is this the intended behaviour by devise_token_auth, as the "native" devise_token_auth endpoints behave differently?
I therefore removed the warden logic from this gem to authenticate each request by its header parameters individually. The changes can be found in #6.
Please help me to find out if this breaks stuff (like batch requests for example) and there is a much simpler solution to my problem or if this could actually make sense.
The text was updated successfully, but these errors were encountered: