We shouldn't let you add someone as an owner without their consent

[Gankra]

This is a vector for abuse. We should require confirmation from the person being added before adding them, just like Github does for adding someone as an owner to a repo/team.

---

Implementation steps added by carols10cents:

The following tasks assume we'll need a web UI for this in order to support users with arbitrarily old versions of cargo. I don't think crates.io can have a state where this feature is "half on", and even if we did, I think we'd have to stay in that state forever because people are always going to be using older versions of cargo. I'm open to hearing ideas on this issue though!

Crates.io Deployable chunk 1: Listing invitations

• (Backend of deployable chunk 1 of owner invitations #1040) Add a new table, crate_owner_invitations or something, that has columns:
  • id of the user who ran cargo owner --add
  • id of the user who is being added
  • id of the crate involved
  • status of the invitiation the only invitations in this table should be pending invitations; accepted invitations can be removed from this table and records added to crate_owners instead; declined invitations can be removed from this table.
  • when this request was created
• Don't add records to this table on a cargo owner --add in this PR, add records to this table either manually with psql or in tests
• (Backend of deployable chunk 1 of owner invitations #1040) Make an API route that can list all of the currently logged in user's pending invitations
• (Frontend page for listing pending owner invites #1057) Make a frontend page that calls that API route and displays the currently logged in user's pending invitations (don't link to this page from anywhere yet though)

Crates.io Deployable chunk 2: Accepting invitations

Still not creating records in crate_owner_invitations when cargo owner --add is run

• Make an API route for the currently logged in user to accept an invitation for a particular crate
  • In a transaction:
    • Add a record to the crate_owners table
    • Remove the record from the crate_owner_invitations table
• On the frontend page that lists pending invitations, have a button that calls the accept API route (and displays success/error nicely, etc)

Crates.io Deployable chunk 3: Declining invitations

Still not creating records in crate_owner_invitations when cargo owner --add is run

• Make an API route for the currently logged in user to decline an invitation for a particular crate
  • Delete the crate_owner_invitations record
• On the frontend page that lists pending invitations, have a button that calls the decline API route (and displays success/error nicely, etc)

Crates.io Deployable chunk 4: Requiring approval for ownership

Assume no changes to cargo, because we can't change old cargo anyway!

• When someone runs cargo owner --add for a user (not a team):
  • Don't create a record in crate_owners, instead create a record in crate_owner_invitations
  • Hopefully send a response back from crates.io that tells the person running the command that the person being added should visit the list invitations page on crates.io to accept ownership (this might not be possible depending on what cargo does today though)
• To make the approval process discoverable as the person being added to the crate from old cargo, when someone runs cargo publish on a crate where there's a pending invitation to add them as an owner:
  • Fail the publish
  • Return an error message from crates.io that directs the user that they have to accept the ownership invitation first by going to the list invitations page, and then they can try publishing again
• Document this requirement in the cargo owner docs (which currently live in cargo's repo) split into Document that adding someone as an owner now invites them, not adds immediately cargo#4599
• Super nice would be some sort of notification that shows up anywhere on crates.io and lets you know you have outstanding ownership invitations and you should go to the invitation list page to approve or decline them (thank you for this idea @natboehm!!!) split into Add some sort of notification that you have invitations to own a crate #1116

Cargo deployable chunk 5: adding CLI support for accept/decline

split into rust-lang/cargo#4600

Crates.io deployable chunk 6: UI for adding someone as an owner

split into #1117

[Gankra]

Workflow sketch:

• old_owner does cargo owner --add new_owner
• backend creates an owner entry but as a new kind of owner: OwnerKind::Pending (this should ideally force us to audit all the places where we evaluate rights, yay enums)
• backend sends a confirmation email to new_owner
• new_owner clicks link to crates.io; maybe clicks a button on a page (while signed in)
• backend changes Pending to User

[Gankra]

Unclear what the process for teams should be. I think you need to be a member of a team to add it already? I don't really remember.

[sunjay]

Running cargo publish should tell the user that they are still pending verification. For teams, would it be feasible to keep people's verification status on a per team basis? i.e. you have publishing rights as long as you're on that team but you have to verify that you want those rights before you can actually publish.

Keeping that kind of information isn't bullet proof though because people can leave or be removed from teams so I don't know how you'd verify them again after that.

A future feature that would be nice would be something that would allow you to block people from requesting to add you. Or maybe that could be done for the time being by just leaving the invite pending?

We could streamline the consent process by asking them if they consent during cargo publish. That would look like this:

1. Owner gives me publishing rights
2. I go to publish the crate
3. Instead of being redirected to check my email or something, I can just answer yes to being an owner and having publishing rights
4. Publishing continues and I am never asked again

Also to consider when adding consent: how do people remove consent later? This needs to be thought about both from the perspective of the old owner and the new owner since either may want to remove publishing rights from new owner.

Sorry for just dropping ideas here but I wanted to make sure these things get considered.

[sunjay]

Also I just checked and my crates user page doesn't actually list crates published by the teams that I'm on, so maybe teams isn't as high of a priority for this? I can't think of a way right now that someone could abuse GitHub organizations to spam, annoy or abuse someone through crates.io.

[ashleygwilliams]

so, one large blocking issue here is that crates.io does not have guaranteed email information for users as GitHub does not reliably send it.

[sunjay]

@ashleygwilliams Hmm. If we ask for the consent at publish time as well as do the email confirmation, we don't necessarily need people's emails. We just won't list them as owners until they've consented.

Could also add a custom cargo command just to accept the invitation to be an owner.

[sunjay]

Also to address the email thing, when I was working with the passport-github package, I found out that they actually do a separate API call to fetch the email after authentication. Maybe that could work as a longer term solution for getting emails? It's not ideal, but it does seem to work.

[ashleygwilliams]

so i think the first step here is to improve the crates.io ACL to include and require an email address. i'm down for any strategy re: that, so @sunjay your suggestion seems good to me. backfill, however, is gonna be a large task- either requiring a lot of user effort or a kinda tricky migration.

the next thing is that, afaik, since we never reliably had email addresses, we don't have any mailer service... at all. so writing one of those and making sure it works for non critical things, is the next step. probably a welcome email or something.

these are two Very Large changes to make. once they are in place and stable, i think figuring out how to properly implement an access invite service would be the next step.

(full disclosure: i care about this issue a lot. but, even npm does not have this yet. i am also not happy about that, but it is a lot of work and it's work that needs to be done right.)

[carols10cents]

Natalie is working on the email ish #808 so emails are coming.

[sunjay]

By mailer service do you mean something that sends emails? What about using something existing like mailgun? The pricing is really great even at scale.

I care about this issue a lot too and would be happy to donate my time in a few weeks to making it happen. (I've never worked on this codebase so I can't do it alone!) I can help with the Rust backend stuff and probably the migration too. I haven't used Ember in about 3 years so I'm probably not going to be able to do any changes we need there.

Maybe we can even figure out a way to fetch emails lazily (and then store them) as we need to.

To prevent being spammy, it might be prudent to also ask people if we can email them at all.

At minimum I can help in the planning and figuring out to work this in and make sure the experience is right. Like you said, it's a lot of work and it's work that needs to be done right.

Edit: just saw @carols10cents comment. 🎉

[Gankra]

I think I kinda just like @sunjay's suggestion of hooking verification into the publish process? Absolutely no notification if you've been added as an owner (so there's no way to even spam someone with ownership requests), and you can claim ownership the moment it's actually relevant (when you need to publish).

We could do the same thing for owner --remove, in the case of an ownership transfer (and then it's kinda like an atomic transfer op, which is neat).

Presumably, if you're adding someone as an owner, you have another communication channel to inform them that you did this.

[sunjay]

If we go with just implementing this through the cargo commands then this mostly becomes a task of adding some things to the API and database, then modifying the appropriate cargo commands. What do you think @ashleygwilliams?

We could have something like cargo owner --accept crate-name or something as well as something to list all the crates someone is an owner of/has pending requests to be an owner of.

[Gankra]

Reflecting further on teams: I don't think we need to worry about them. You need to be on a team to add it (partly this is because Github is incredibly unreliable in acknowledging a team's existence unless you're on it), so basically the abuse vector is someone on a team is trying to harass the entire team?

Only case I can think of is someone rage quitting a community, and in that case that person already has plenty of gas to do worse things, i would think.

[ashleygwilliams]

i am coming around to this CLI solution. it's really elegant actually. i think people will eventually ask for email integration, but that gives us time to build all that shenanigans out. good call @sunjay !

[sunjay]

Thanks! I'll admit I felt apprehensive about sharing the idea in the first place given that I've never even touched the code in this repo. I'm happy to hear you both liking it. Thanks for being open to that. 😊

I can help with the cargo integration stuff in about a week or so. What needs to be done from the crates.io side? I think much of your initial comments still apply just without the email stuff.