- Notifications
You must be signed in to change notification settings - Fork1.1k
fix: do not query user_link for deleted accounts#12112
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
Emyrk commentedFeb 12, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
This stack of pull requests is managed by Graphite.Learn more about stacking. |
johnstcn left a comment
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.
I think this patch is OK for now, but we should also:
- Delete user_links when a user is deleted
- Update the schema to ensure that this situation cannot occur in future.
We should create issues for the above change(s) and reference them in the unit test so we do not forget to do them.
Also: the PR title is incorrect as this is no longer just a chore but actually changes functionality.
e681b22 to2ca3ef9Compare2ca3ef9 to65effdcCompare2883815 to78e90d6Compare65effdc to6f09151Compare78e90d6 to90e7164Compare6f09151 to24e767fCompareAdded to the migration that added the delete field to users
Changing emails on github fails if another deleted user existswith the same link.
24e767f tob0c3cbbCompare
Uh oh!
There was an error while loading.Please reload this page.
Closes#10972
What this does
This solution is a patch, I think we need to create better unique constraints on the database to prevent other types of bugs in the future.
If you change your github email and log in, we link based on your Github UID. So your email is updated on Coder. This was failing if you previously deleted an account that was linked to the same github. The previous
GetUserLinkByLinkedIDwould fetch a deleted user, and hit this line:coder/coderd/userauth.go
Lines 1728 to 1729 in4f0203d
This would then instead make a new account. A simple fix is to ignore deleted acounts.
Future work
linked_idbecause of deleted users. I am not sure what the solution is here 🤔. When we delete an account, should we do something to the user_link?