-
Notifications
You must be signed in to change notification settings - Fork 225
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
Rememberable race condition causing users to be incorrectly logged out #376
Comments
We have the same issue. After home page loads two widgets on it initiates two requests to the API endpoints which are piped through |
I have seen similar issues in one of my apps but have never taken the time to investigate. Thanks for raising this issue!! My first thought on a fix is to serialize the requests through a GenServer (Or Agent), ensuring that only one can be done at a time. I'll see if I can come up with a fix today. |
I just reviewed the code. First, the validation and generation of a new rememberable token IS an atomic action. It is done through the RememberableServer GenServer. Its a synchronous operation (call), but I don't think that's an important detail. So, I wonder if the issue is that the client is making simultaneous async requests so they are all landing with the same session. We had a similar issue with AJAX requests. For the AJAX case, we added code to bypass the token validation for AJAX requests. I'm at a bit of loss on how to solve the issue if we are getting async requests from the server. Some thoughts are:
@BobaFaux and @nmbrone what are your thoughts?? |
I also ran into a similar issue, where users were sporadically being logged off and seeing the invalid token message. After experimenting a bit, I was able to replicate the bug. When the initial session cookie expires or is deleted, authentication is handled by the coherence cookie, which causes the token to be reset. However, a new session cookie is never set, and the credential store is never updated when logging in with the coherence cookie. I don't know if this is a bug or by design, but it means that the token is reset with every request. If two requests are made simultaneously, the first will update the token, and the second will cause the user to be logged out because it is still using the old token. Making it so a new session cookie is set doesn't fix the race condition, but it does reduce the likelihood of a race condition happening because the token is reset much less often (and it doesn't require a database hit on every request). Fixing this so far seems to have fixed the issue for me. |
I believe I have found a race condition in the rememberable feature.
I have a pretty straightforward page which has links to 36 images on it. These images are not links to static images but are instead routed through the Phoenix itself (where I look up the image and return it). So my links look something like:
/for/123/image
/for/124/image
The routes for these links are piped through the browser pipeline, set up with the Coherence.Authentication.Session as specified in the docs so everything is being authenticated through Coherence.
When I log into my system for the first time everything is fine, selecting the remember me checkbox. When I close the browser and open the site again I appear to be logged in (the menus show logged in) but I am in fact logged out and if I refresh the page it shows I am logged out (menus show logged out).
It appears that the page request is working correctly and keeping me logged in. However, when processing one of the 36 image requests it is logging me out, I can see in the logs I get an "Invalid token. Potential Fraud." warning.
I believe what is happening is when requesting the images Coherence is validating the series_hash and token_hash to ensure it is valid, then updates these before returning the request. If a second image request starts before the first image request completes then it won't have the updated series_hash and token_hash and when validating it will fail, logging the user out as fraudulent.
I have worked around this for the moment by making another pipeline which doesn't include Coherence.Authentication.Session and piping my images only through this pipeline.
I believe you would also be able to have this same problem is someone refreshed multiple windows/tabs in their browser at the same time as well.
I think there are a couple of issues at the heart of the problem, the first being that the validating of series_hash/token_hash and updating them is not an atomic action. Second being the series_hash/token_hash is being changed when there could be another request being processed which will now have the incorrect keys.
At the moment I don't know where to get started on resolving this issue as I don't know enough detail on how the rememberable feature is implemented.
When I get a chance, I will try and put together an example repo which replicates this issue.
The text was updated successfully, but these errors were encountered: