Skip to content

Can't remove invalid users from crate owners #2736

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

Closed
carols10cents opened this issue Aug 31, 2020 · 4 comments · Fixed by #9930
Closed

Can't remove invalid users from crate owners #2736

carols10cents opened this issue Aug 31, 2020 · 4 comments · Fixed by #9930
Labels
A-accounts C-bug 🐞 Category: unintended, undesired behavior

Comments

@carols10cents
Copy link
Member

This is similar to #1818 but for users.

What happened

  • A user had a GitHub account named, let's say, foo.
  • Another user someone_else added them as an owner of a crate
  • foo deleted their GitHub account (but crates.io has no real knowledge of this)
  • foo recreated their GitHub account with the same username (because it was still available), but in this situation GitHub assigns them a new ID
  • When foo now logs in to crates.io, it's a different account than their old foo crates.io account because of the new GitHub ID (and if it wasn't, this would be a security problem)
  • The new foo account is NOT an owner of the crate
  • someone_else can't add the new foo account as an owner, nor can they remove the old foo as an owner :(

What should happen

  • Removing an owner, whether it's valid/current or not, should always work (if performed by another valid owner, etc)
  • Once that removal works, it should be possible to add the new account as an owner
@carols10cents carols10cents added C-bug 🐞 Category: unintended, undesired behavior A-accounts labels Aug 31, 2020
@sgrif
Copy link
Contributor

sgrif commented Aug 31, 2020

This is slightly different framing, but almost the same as #1585

@sgrif
Copy link
Contributor

sgrif commented Aug 31, 2020

#1586 would also let us manually resolve this problem when we're aware of it (but that PR does not currently automatically mark users as deleted in the cases where we could detect it -- which in this case we could have). It needs some work to get up to date, but is something I can update if that approach is considered worthwhile

@dmntk
Copy link

dmntk commented Sep 7, 2023

I am currently having this problem with my crates. I would love to contribute to crates.io and fix it, but I have no idea where to start. Is there any possibility, that someone points the starting source code where to start looking for the functionality that removes an owner? Thanks in advance for help!

@EngosSoftware
Copy link

Maybe here in src/models/krate.rs:

 pub fn owner_remove(
        &self,
        app: &App,
        conn: &mut PgConnection,
        req_user: &User,
        login: &str,
    ) -> AppResult<()> {
        let owner = Owner::find_or_create_by_login(app, conn, req_user, login)?;

        let target = crate_owners::table.find((self.id(), owner.id(), owner.kind()));
        diesel::update(target)
            .set(crate_owners::deleted.eq(true))
            .execute(conn)?;
        Ok(())
    }

???

Turbo87 added a commit to Turbo87/crates.io that referenced this issue Nov 14, 2024
Turbo87 added a commit to Turbo87/crates.io that referenced this issue Nov 14, 2024
Turbo87 added a commit to Turbo87/crates.io that referenced this issue Nov 15, 2024
Turbo87 added a commit to Turbo87/crates.io that referenced this issue Nov 15, 2024
Turbo87 added a commit to Turbo87/crates.io that referenced this issue Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-accounts C-bug 🐞 Category: unintended, undesired behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants