Skip to content

We should have a way to mark that a user has been deleted #1585

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.

Sign up for GitHub

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

Jump to bottom
Open
sgrif opened this issue Dec 27, 2018 · 6 comments
Open

We should have a way to mark that a user has been deleted #1585

sgrif opened this issue Dec 27, 2018 · 6 comments
Labels
C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear

Comments

@sgrif
Copy link
Contributor

sgrif commented Dec 27, 2018

When a user deletes their account (or renames it and never logs back into crates.io), we are never able to find out about it. We continue to show that github username's avatar, and links to the github account with that name, even if someone else has created a new account with the same name.

We should have a flag we can set to mark that a user has been deleted, which shows the avatar of @ghost, and either removes all links to their github account, or links to @ghost. This does not affect repository links, which are just URLs given to us in Cargo.toml, and not coupled to Github in any way.

I do not think we should try to automate this, but we should have the ability to do this manually when we receive reports.

The main blocker here is how do we link to that user once we know they're deleted. We currently use gh_login as the identifier in /users, we will need to do something else in this case.
sgrif added a commit to sgrif/crates.io that referenced this issue Dec 27, 2018
@sgrif
Add the ability to flag a user as deleted in GH
7343dd7 
When a user deletes their account in GitHub we don't receive any
notification that this has happened. If another user creates an account
with the same name, we will link to this new user on any crates owned
by the old user, which can make it appear that the new user is an owner
and lead to social engineering situations we don't want to permit.

This isn't something we can reasonably automate detection of, but when
we do find out that it's happened, we can now manually flag a user. We
*can* detect this when a new user is created with the same gh_login as
an existing user, which will be handled in a followup PR.

Fixes rust-lang#1585
sgrif added a commit to sgrif/crates.io that referenced this issue Dec 27, 2018
@sgrif
Add the ability to flag a user as deleted in GH
500051a 
When a user deletes their account in GitHub we don't receive any
notification that this has happened. If another user creates an account
with the same name, we will link to this new user on any crates owned
by the old user, which can make it appear that the new user is an owner
and lead to social engineering situations we don't want to permit.

The front-end is pretty coupled to the (false) concept that a user
always has a GH login, and that we can display it. Ideally I'd like to
move what we currently call "login" to the "id" field, but we have
at least 2 places today where we use the actual user id in the URL, so
we'll have to change those before we can make that happen. For now I've
just introduced a `display_name` field (which is confusingly named since
it's usually the same as `login`, not `name`, but I couldn't come up
with a better name).

When a user deletes their account (or renames it, we have no way of
knowing which one happened), we will no longer report their old github
name, and will instead report `deleted_123` where 123 is *our* user ID
for them. This is then used in the users route to look them up, instead
of their old GH login, which may be reused by another user. Since we
don't necessarily want to allow crawling of user profiles, this can only
be used to access deleted accounts. Attempting to visit
/users/deleted_123 for a user ID that isn't deleted will 404.

Additionally, since we can't generally tell if a user deleted their
account or renamed it, logging back into crates.io (which will update
gh_login with the new value) automatically marks them as undeleted.

This isn't something we can reasonably automate detection of, but when
we do find out that it's happened, we can now manually flag a user. We
*can* detect this when a new user is created with the same gh_login as
an existing user, which will be handled in a followup PR.

Fixes rust-lang#1585
@sgrif sgrif mentioned this issue Dec 27, 2018
Closed
@carols10cents
Copy link
Member

I don't understand why we need this, could you explain the benefit?

@sgrif
Copy link
Contributor Author

sgrif commented Dec 27, 2018

We continue to show that github username's avatar, and links to the github account with that name, even if someone else has created a new account with the same name.

This leads to confusion at best, and is a vector for social engineering attacks at worst. We're implying that some user who does not own a crate does

@sgrif sgrif mentioned this issue Dec 27, 2018
Open
@carols10cents
Copy link
Member

This leads to confusion at best, and is a vector for social engineering attacks at worst. We're implying that some user who does not own a crate does

So github user carols10cents owns crate foo. github user carols10cents deletes her github account. malicious user warols10cents registers now available name carols10cents and logs in to crates.io. Per this test, crates.io/users/carols10cents now links to warols10cents' carols10cents account and displays the crates that warols10cents has published. warols10cents doesn't have access to the crates carols10cents published.

Can you clarify what the social engineering is that warols10cents would have the ability and motivation to do?

@sgrif
Copy link
Contributor Author

sgrif commented Jan 1, 2019 via email

@carols10cents
Copy link
Member

You're missing that carols10cents still owns crates. Their crates.io account still exists. We still need to link to it. Right now we're literally just linking to a different account, implying they are the same. You're correct that if you're paying attention you'll notice that they aren't, but we shouldn't be putting that burden on our users to notice.

The "confusing" aspect I see, it's the "social engineering" bit that I'm still not getting. I do think we should fix it, I just don't see the urgency that "social engineering attacks" prompts.

@sgrif
Copy link
Contributor Author

sgrif commented Jan 3, 2019

I think I may have communicated poorly. I was not attempting to imply urgency, or that there's an obvious attack vector that is being exploited right now (if there were this would not have been opened publicly). By "confusing at best, and a vector for social engineering at worst" I was just trying to say that we should fix this no matter what, because it's definitely confusing and I think there is the possibility of someone using this for nefarious purposes by impersonating another user.

@carols10cents carols10cents added the C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear label Nov 11, 2019
@sgrif sgrif mentioned this issue Aug 31, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add the ability to flag a user as deleted in GH sgrif/crates.io
2 participants
@carols10cents @sgrif