Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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

Merged
Emyrk merged 15 commits intomainfromstevenmasley/linked_id_fix
Feb 13, 2024

Conversation

Emyrk
Copy link
Member

@EmyrkEmyrk commentedFeb 12, 2024•
edited
Loading

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 previousGetUserLinkByLinkedID would fetch a deleted user, and hit this line:

coder/coderd/userauth.go

Lines 1728 to 1729 in4f0203d

// If the user was deleted, act as if no account link exists.
user= database.User{}

This would then instead make a new account. A simple fix is to ignore deleted acounts.

Future work

  • It is possible to have more than 1 user_link per account. This does exist on prod, and is a mistake imo.
  • It is possible to have multiple user_links with the samelinked_id because 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?

@EmyrkEmyrk changed the titlechore: creaet unit test to exercise failed email change bugchore: create unit test to exercise failed email change bugFeb 12, 2024
@EmyrkGraphite App
Copy link
MemberAuthor

Emyrk commentedFeb 12, 2024•
edited
Loading

Copy link
Member

@johnstcnjohnstcn left a 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:

  1. Delete user_links when a user is deleted
  2. 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.

@EmyrkEmyrk changed the titlechore: create unit test to exercise failed email change bugfix: do not query user_link for deleted accountsFeb 13, 2024
@EmyrkEmyrkforce-pushed thestevenmasley/linked_id_fix branch frome681b22 to2ca3ef9CompareFebruary 13, 2024 14:03
@EmyrkEmyrk changed the base branch fromstevenmasley/linked_id_bug tomainFebruary 13, 2024 14:03
@EmyrkEmyrk changed the base branch frommain tostevenmasley/linked_id_bugFebruary 13, 2024 14:07
@EmyrkEmyrkforce-pushed thestevenmasley/linked_id_fix branch from2ca3ef9 to65effdcCompareFebruary 13, 2024 14:07
@EmyrkEmyrkforce-pushed thestevenmasley/linked_id_bug branch from2883815 to78e90d6CompareFebruary 13, 2024 14:08
@EmyrkEmyrkforce-pushed thestevenmasley/linked_id_fix branch from65effdc to6f09151CompareFebruary 13, 2024 14:08
@EmyrkEmyrkforce-pushed thestevenmasley/linked_id_bug branch from78e90d6 to90e7164CompareFebruary 13, 2024 15:48
@EmyrkEmyrkforce-pushed thestevenmasley/linked_id_fix branch from6f09151 to24e767fCompareFebruary 13, 2024 15:48
Added to the migration that added the delete field to users
Base automatically changed fromstevenmasley/linked_id_bug tomainFebruary 13, 2024 18:06
@EmyrkEmyrkforce-pushed thestevenmasley/linked_id_fix branch from24e767f tob0c3cbbCompareFebruary 13, 2024 18:07
@EmyrkEmyrk merged commit5d483a7 intomainFeb 13, 2024
@EmyrkEmyrk deleted the stevenmasley/linked_id_fix branchFebruary 13, 2024 19:02
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsFeb 13, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@johnstcnjohnstcnjohnstcn approved these changes

Assignees

@EmyrkEmyrk

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GitHub Login: If a user changes their primary email on GitHub Coder creates a new account for them instead of updating the email on existing account
2 participants
@Emyrk@johnstcn
[8]ページ先頭
©2009-2025 Movatter.jp