-
Notifications
You must be signed in to change notification settings - Fork 46
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
10353 Bug: Duplicate Cognito Accounts #5252
Merged
jtdevos
merged 11 commits into
ustaxcourt:staging
from
flexion:10353-bug-duplicate-cognito-users
Aug 23, 2024
Merged
10353 Bug: Duplicate Cognito Accounts #5252
jtdevos
merged 11 commits into
ustaxcourt:staging
from
flexion:10353-bug-duplicate-cognito-users
Aug 23, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jimlerza
approved these changes
Aug 22, 2024
jimlerza
approved these changes
Aug 22, 2024
web-client/src/views/CreatePetitionerAccount/CreatePetitionerAccountForm.tsx
Outdated
Show resolved
Hide resolved
jtdevos
requested changes
Aug 22, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a comment re: magic numbers - let me know if you disagree
Sure, I can pull this out into a constant. |
…n/ef-cms into 10353-bug-duplicate-cognito-users
jtdevos
approved these changes
Aug 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Cognito will throw an error with the message
Alias entry already exists for a different username
when we attempt to create two accounts with the same email nearly simultaneously. Unfortunately, Cognito still creates the duplicate account. We sometimes run into the error when a user quickly hits submit multiple times inCreatePetitionerAccount/CreatePetitionerAccountForm.tsx
.This PR fixes the issue on the front end by adding a debounce and deactivating the signup button. For all practical purposes, that should solve the issue.
I thought about how we might enforce no duplicate Cognito users on the back end, too, which would be ideal. I think the solution would go something like this: On getting the duplicate error message, we delete the duplicate account. This is easier said than done: because the request "fails," we don't get the duplicate Cognito user id with which to key off of. Instead, then, we would need to kick off some asynchronous task to get all users associated with the email and delete those without
custom:userId
or something (since our custom attributes--which we set after successfully signing up a user in Cognito--won't have been set for the failed Cognito request).Frankly, I don't think it is worth it. I am happy enough with the front-end fix and pointing out that the issue is, after all, due to a bug with Cognito. That said, if reviewers think that this back-end solution is worthwhile, I can set it up.