flexion / ef-cms

An Electronic Filing / Case Management System.
23 stars 10 forks source link

BUG: Duplicate Cognito Accounts created #10353

Closed mmarcotte closed 2 months ago

mmarcotte commented 6 months ago

Describe the Bug

Due to a bug in Cognito, multiple accounts can be created with the exact same casing of Email. This has occurred twice in Production and reportedly on test as well.

It appears that our API is receiving more than one nearly simultaneous requests, resulting in multiple simultaneous requests to Cognito with the same information. Interestingly, Cognito returns an error for the second request it received, but it still creates a Cognito account!

Screenshot 2024-04-19 at 2 31 57 PM

This is a hard to replicate bug. I've tried clicking quickly, but the frontend only triggered one request.

Also, it appears that the successfully created user is able to proceed normally -- confirm their account and create a case.

Is this bug related to #10459?

Business Impact/Reason for Severity

In which environment did you see this bug?

Prod, Test

Who were you logged in as?

N/A

What were you doing when you discovered this bug? (Using the application, demoing, smoke tests, testing other functionality, etc.)

Deduping Cognito accounts

To Reproduce

Steps to reproduce the behavior:

  1. Trigger near simultaneous calls to /auth/account/create with the same information. You may have to do this with a script rather than through the UI. We were unable to replicate it using the UI. Unclear how users are making this happen.
  2. See multiple Cognito accounts in our User Pool with the exact same Email Address (same casing as well)

Expected Behavior

In the event that multiple near simultaneous requests are received, the API should handle these appropriately and have the 2nd request handled fail while the first one succeeds. Only one Cognito account with the same Email address should be created.

Actual Behavior

It appears that the 2nd request fails with a 400, but the Cognito account is still created.

Cause of Bug, If Known

Cognito bug :( Unclear how our client is triggering duplicative calls.

Process for Logging a Bug:

Severity Definition:

Definition of Ready for Bugs(Created 10-4-21)

Definition used: A failure or flaw in the system which produces an incorrect or undesired result that deviates from the expected result or behavior. (Note: Expected results are use cases that have been documented in past user stories as acceptance criteria and test cases, and do not include strange behavior unrelated to use cases.)

The following criteria must be met in order for the development team to begin work on the bug.

The bug must:

Process: If the unexpected results are new use cases that have been identified, but not yet built, new acceptance criteria and test cases should be captured in a new user story and prioritized by the product owner.

If the Court is not able to reproduce the bug, add the “Unable to reproduce” tag. This will provide visibility into the type of support that may be needed by the Court. In the event that the Court cannot reproduce the bug, the Court will work with Flexion to communicate what type of troubleshooting help may be needed.

Definition of Done (Updated 4-14-21)

Product Owner

Engineering

ttlenard commented 2 months ago

@mwindo came up with this script to put in the console and after we did this, we were able to recreate the issue we saw on Prod:

const button = document.querySelector('[data-testid="petitioner-account-creation-submit-button"]');
function clickButtonMultipleTimes(button, times) {
  for (let i = 0; i < times; i++) {
    button.click();
  }
}
clickButtonMultipleTimes(button, 2);

Here is a screen grab of the logs on test:

image.png

Here are the logs for the real user on prod:

image.png
ttlenard commented 2 months ago

I was able to recreate this issue without the script. I updated my mouse settings so that the double-click speed was set to fast:

image.png

This resulted in duplicate cognito accounts created.

image.png

What if we just made it so that the continue button was deactivated after the first click? Would this be a viable solution?

image.png
Mwindo commented 2 months ago

I think Tenille's suggestion above to disable the button is a good practical front-end solution. (EDIT: Along with debouncing, just in case the user clicks before the button is disabled.) The back-end solution seems less straightforward given that we are just relying on Cognito and not a database (which would allow us to use some sort of lock/mutex or transaction logic). We could maybe do a cleanup job at the end of sign up to check for and merge duplicate Cognito accounts?

While we will want the back-end to enforce the no-duplicates policy somehow, I believe the front-end solution should, for all practical purposes, "fix" the issue.

Mwindo commented 2 months ago

10459 is definitely related to this bug. I am still trying to determine if there are aspects of #10459 that this bug does not explain.

Here is what happens:

  1. The user clicks quickly and sends multiple requests to our signUpUserInteractor. Those that we catch as duplicates before sending a request to Cognito we flag with the error messages User exists, email unconfirmed or User already exists. In these cases, we do not create duplicate accounts in Cognito. However, if the first account has not yet been fully created in Cognito before we check that the second is a duplicate, we proceed to signUp the second, duplicate account.
  2. This signUp call will make a request to add the second, duplicate Cognito user. This request will respond with an error: Alias entry already exists for a different username. Great! Except it will still create a user anyway. (See https://stackoverflow.com/questions/50730759/user-pool-allows-two-users-with-same-email-despite-configuration.)
  3. This failing call (that nevertheless creates a Cognito user :/) occurs before the request to add the custom id attribute. Therefore, this duplicate account will not have the custom id attribute, just as 10459 reports. Likewise, this failing request will prevent us from sending a second email (i.e., these lines are never hit), just like Tenille experienced when testing (multiple duplicate accounts, but only one email confirmation sent).

When I cross-check the file of users without custom id attributes attached to 10459 with the Alias entry already exists for a different username logs in OpenSearch, 7 data points (accounting for differences in time zones) match. For example, this May 20 entry in OpenSearch matches an entry in the file (for Username 2824cfec-917f-4f6c-872d-89d1ac267054):

Screenshot 2024-08-14 at 1.05.06 PM.png

There are 15 total hits on OpenSearch, and there are 44 users without custom id in the file attached to 10459. My suspicion is that the 8 entries in OpenSearch that are missing from the file correspond to duplicate Cognito users we have deleted. (Extra evidence for this suspicion comes from the fact that these entries are all earlier; in other words, these 8 OpenSearch requests probably correspond to Cognito accounts we have already "deduped," and therefore they do not show up in the file.)

However, this still does not explain why there are 37 users without custom id unaccounted for in the 10459 file.

I believe the only reason a user wouldn't have a custom id is if either the signUp request or adminUpdateUserAttributes request failed in signUp.ts. I have tried to find any suspicious-looking Cognito error messages to indicate other sorts of failures beyond Alias entry already exists for a different username, but I don't see any:

Screenshot 2024-08-14 at 2.07.49 PM.png
Mwindo commented 2 months ago

I have done two things since the last update:

  1. Run the script from #10459 (https://github.com/ustaxcourt/ef-cms/blob/27edcfcf619a577c8f78c23d50420cbbd2434c7e/scripts/run-once-scripts/verify-cognito-required-attributes.ts) on test and confirmed that the missing ids are exactly those corresponding to Tenille's tests where Alias entry already exists for a different username occurred.
  2. Tried to match timestamps from the file in #10459 to events in OpenSearch. For those 7 data points that overlap (mentioned in the previous comment), I find (as expected) Alias entry already exists for a different username in OpenSearch. For other timestamps in the file, I do not see any suggestive data in OpenSearch.

This suggests that the discrepancy results from activity outside of the app, i.e., running manual scripts or behind-the-scenes jobs might be leading to more users without custom user ids (beyond those users whose lack of custom id is explained by duplicate Cognito accounts).

Mwindo commented 2 months ago

After discussion with @jimlerza and @mmarcotte (thank you!), it seems that there was indeed activity outside the app that led to some users not getting custom:userIds: the old Cognito signup page. So only part of #10459 is explained by this bug; the other part is explained by users running into errors signing up via the old signup page.

ttlenard commented 2 months ago

Testing Feedback:

I have my mouse set to high speed for double clicks as indicated above.
If I double click the create account button multiple times, I see that the button re-activates and I can click create account again.
An error will flash on the screen, but it is so quick I haven't been able to get a screen grab of it, and then it goes to this page. Notice that there are a few errors, unbeknownst to the user.

image.png

Seems maybe a better user experience would be to deactivate the button entirely?