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

Possible Logout Issue #46

Closed
booleanbetrayal opened this issue Oct 14, 2014 · 3 comments
Closed

Possible Logout Issue #46

booleanbetrayal opened this issue Oct 14, 2014 · 3 comments

Comments

@booleanbetrayal
Copy link
Collaborator

Hey @lynndylanhurley -

Ran into an issue with ng-token-auth (0.0.23-beta2) / devise-token-auth (0.1.29) sometimes resulting in 401 unauthorized while hitting API routes while using changing tokens.

This seems to happen after a request errors, resulting in a non 200 OK response from the server. This only happens when a token has been re-generated in the after_action callback in the devise_token_auth set_user_by_token.rb concern.

The $httpInterceptor in ng-token-auth has appropriate request and response handlers, but no responseError handler. So, when an errored request (that is also updating the token) has its response hit the client, ng-token-auth never calls setAuthHeaders with the updated headers.

I'm not sure what's the most appropriate fix in this case. devise_token_auth could only update tokens if the response was successful, but I'm not sure that's the right thing to do or not. I'm leaning towards ng-token-auth essentially duplicating the response handler for responseError so it could be more agnostic. Thoughts?

@lynndylanhurley
Copy link
Owner

Thanks so much for the PR. Do you think that these tests provide adequate coverage for this issue?

@booleanbetrayal
Copy link
Collaborator Author

Looks good to me! As far as the server-side of things go, the more I think about it, the more I agree that error responses should still be rotating tokens (if configured). So I can't imagine anything needs to change in devise_token_auth unless you feel otherwise. Feel free to close unless you disagree.

Thanks @lynndylanhurley !

@lynndylanhurley
Copy link
Owner

I totally agree. Good work! 👍

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