API can return 401 status codes for valid sessions under load, forcing users to be logged out #7169
Assignees
Labels
Priority: 0 - Critical
Work on this right away.
Type: Bug
Fix something that isn't working as intended
Projects
Milestone
Comments
dianabarsan
added
Type: Bug
dianabarsan
changed the title
|
Around the time of this happening, I'm seeing lots of CouchDB fails in logs, for example: |
|
The problem may be in the authentication code where we respond with a 401 no matter what CouchDB responded, so even if it timed out, or CouchDB was unavailable, or pretty much anything we will log the users out. My vote would be your first idea, to only respond with a 401 if we know for sure that the user is logged out. |
|
How far back should we backport the fix? |
|
We support versions 3.10.x and above, so definitely that far. That also solves the issue for this particular production instance. I don't think we need to backport to 3.9.x at this stage. |
5 tasks
|
This is ready for AT on The changes that are implemented are:
So, online and offline users should continue to work as before, authentication should work as before. I still haven't succeeded to replicate overloading CouchDB in such a way that _session requests timeout - every time I upped the load high enough, I crashed my local CouchDB. |
|
Tested on local, basically checking if the following still work
Feel free to merge @dianabarsan |
dianabarsan
added a commit
that referenced
this issue
Jul 6, 2021
Updates API and Webapp to: - Only consider 401s from CouchDB to be valid missing authentication - Throw other _session errors without changes - Malformed _session responses result in a 500 - Don't send 401s for missing userCtx, instead send the actual authentication error or a 500. - Add a custom header to 401 responses that Webapp checks for, and only when matched it would log users out. (prevents man-in-the-middle from logging users out with 401s) #7169
dianabarsan
added a commit
that referenced
this issue
Jul 6, 2021
Updates API and Webapp to: - Only consider 401s from CouchDB to be valid missing authentication - Throw other _session errors without changes - Malformed _session responses result in a 500 - Don't send 401s for missing userCtx, instead send the actual authentication error or a 500. - Add a custom header to 401 responses that Webapp checks for, and only when matched it would log users out. (prevents man-in-the-middle from logging users out with 401s) #7169 (cherry picked from commit 2486dc5)
dianabarsan
added a commit
that referenced
this issue
Jul 6, 2021
Updates API and Webapp to: - Only consider 401s from CouchDB to be valid missing authentication - Throw other _session errors without changes - Malformed _session responses result in a 500 - Don't send 401s for missing userCtx, instead send the actual authentication error or a 500. - Add a custom header to 401 responses that Webapp checks for, and only when matched it would log users out. (prevents man-in-the-middle from logging users out with 401s) #7169 (cherry picked from commit 2486dc5)
dianabarsan
added a commit
that referenced
this issue
Jul 6, 2021
Updates API and Webapp to: - Only consider 401s from CouchDB to be valid missing authentication - Throw other _session errors without changes - Malformed _session responses result in a 500 - Don't send 401s for missing userCtx, instead send the actual authentication error or a 500. - Add a custom header to 401 responses that Webapp checks for, and only when matched it would log users out. (prevents man-in-the-middle from logging users out with 401s) #7169 (cherry picked from commit 2486dc5)
dianabarsan
added a commit
that referenced
this issue
Jul 6, 2021
* Prevent users being logged out incorrectly (#7171) Updates API and Webapp to: - Only consider 401s from CouchDB to be valid missing authentication - Throw other _session errors without changes - Malformed _session responses result in a 500 - Don't send 401s for missing userCtx, instead send the actual authentication error or a 500. - Add a custom header to 401 responses that Webapp checks for, and only when matched it would log users out. (prevents man-in-the-middle from logging users out with 401s) #7169 (cherry picked from commit 2486dc5) * Adds GitHub actions workflow to take over CI.
|
Merged to |
dianabarsan
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Requests that reach API and are forwarded (proxied) to CouchDB could result in 401 status codes even if the user has an active session.
Any 401 response status will trigger a logout in webapp based on: #3919 where the solution was to hook into all Pouch<->Couch fetch requests and navigate to login whenever a 401 is received. This was introduced in 3.6.0.
To Reproduce
No reproduction steps yet, this appears to have been a one-off (?) event on a production server (very high load).
Expected behavior
Given the logistical difficulties of users being logged out, we should make sure that the we mean it for sure before logging them out.
Logs
Logs are extensive, but I extracted some relevant logs for a single user. The logs are redacted to obfuscate the instance, the user's IP and the username:
The app/CouchDB were under such load that the simplest
/dbinforequest - which is just/for offline users and is directly forwarded to CouchDB here (https://github.com/medic/cht-core/blob/3.10.x/api/src/routing.js#L195) took 80 seconds to succeed.Some ideas:
/_sessionwhen a 401 is received, before deciding to definitively log a user out (deleting the userCtx cookie). Alternatively, the login script could also poll session before doing anything. The session endpoint will also populate the userCtx cookie if a valid session is found.Environment
Additional context
Over a period of a day, numerous users from a production instance were logged out.
The text was updated successfully, but these errors were encountered: