Fix owner removal for duplicate logins

[Turbo87]

When users or teams on GitHub are deleted and recreated they might exist multiple times within our own database with the same login. When owners are deleted, only the newest matching login is considered for deletion though, which makes it impossible to delete older users/teams.

This commit fixes the problem by deleting all owners with a matching login instead.

Unfortunately, diesel does not support UPDATE queries with JOINs, so this has to be a "raw" SQL query with bindings instead…

Fixes #2736, and AFAICT also fixes #1205

Related:

• Bad gateway error when wanting to remove access for a team #1205
• Can't remove invalid users from crate owners  #2736
• Can't remove deleted organizations from crate owners #1818
• can't remove deleted team from crate owners #1512
• Allow removing deleted organizations from crate owners #7051

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.08%. Comparing base (1765dd1) to head (431026d).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9930      +/-   ##
==========================================
+ Coverage   89.07%   89.08%   +0.01%     
==========================================
  Files         290      290              
  Lines       30060    30145      +85     
==========================================
+ Hits        26776    26855      +79     
- Misses       3284     3290       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[carols10cents]

Wait, what about this case:

• User foo, github ID 1 that is Person A has logged in to crates.io
• Person A renames their github account to bar but does NOT log in to crates.io
• Person B creates a new github account named foo with github ID 2 and logs in to crates.io
• Person A logs in to crates.io with their account that's now named bar but still has github ID 1, I believe the current behavior is crates.io updates the gh_login and everything Just Works because the gh_id matches

I think what you're saying is this PR would delete the record for Person A when Person B logs in??? So when Person A logs in again, their account/ownership stuff would be gone?

[carols10cents]

OH WAIT I think I'm confused. I read this line:

which makes it impossible to delete older users/teams.

and thought the records in the users table were being deleteed, but you're talking about the crate_owners table. Carry on, I think.

[Turbo87]

and thought the records in the users table were being deleted, but you're talking about the crate_owners table

yep, exactly :)

[eth3lbert]

LGTM on first glance :)

[eth3lbert]

Ah, and should we add some tests for the team-related parts?

[Turbo87]

should we add some tests for the team-related parts?

I've added #9943 just now, but it looks like there is no fix needed for that issue since it already works as intended.

[eth3lbert]

Wait, what about this case:

* User `foo`, github ID 1 that is Person A has logged in to crates.io

* Person A renames their github account to `bar` but does NOT log in to crates.io

* Person B creates a new github account named `foo` with github ID 2 and logs in to crates.io

* Person A logs in to crates.io with their account that's now named `bar` but still has github ID 1, I believe the current behavior is crates.io updates the gh_login and everything Just Works because the gh_id matches

I think what you're saying is this PR would delete the record for Person A when Person B logs in??? So when Person A logs in again, their account/ownership stuff would be gone?

If I understand correctly, in this case, Person B is not the owner of the crate, and they also do not have permission to change the owner. So, there should not be an issue here.

[eth3lbert]

The regression test confirms that the issue has been fixed as expected. The SQL part leverages bind, which should prevent any injection attacks.
Nice work, thanks 👍