Skip to content
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

Cargo owner --add requiring invitations deployment plan #4537

Closed
natboehm opened this issue Sep 25, 2017 · 2 comments · Fixed by #4551
Closed

Cargo owner --add requiring invitations deployment plan #4537

natboehm opened this issue Sep 25, 2017 · 2 comments · Fixed by #4551

Comments

@natboehm
Copy link
Contributor

I'm currently working on an issue on Crates.io regarding requiring a user's consent to be added as an owner of a crate. I'm at deployable chunk 4, requiring approval for ownership, and we've come to a point where the result of altering the owner_add function on the crates.io side to require a user to accept an invitation before being added as an owner of a crate might be confusing for users.

The problem is that currently the messages conveying the result of adding an owner are hardcoded into Cargo, and only the success/error results come from Crates.io. When cargo owner --add is run, Cargo displays the message "adding owner to a crate" and displays an error message indicating the user was not added if something went wrong in adding the owner to the crate. If we require a user to accept an owner invitation, it would be better to have a message on the Cargo end stating that your invitation has been submitted and the invitee must now accept their owner invitation, but that the user was not yet added as an owner. Due to the process of deploying something to Cargo, if implemented now on Crates.io, the feature could be live but this message would not be available for another few weeks on stable. If a user never updates Cargo, it may never become available. @carols10cents and I came up with a couple options to try to implement this as soon as possible on Crates.io while making the transition as smooth as possible on the Cargo side.

Option 1

Add a new route, something calling a new function owner_invite on Crates.io, and change the route on Cargo to this new route. Two routes would be available on the crates.io backend, one calling owner_invite and the other still for owner_add. Eventually deprecate owner_add on Crates.io. This option could possibly be confusing for users in having two different experiences when adding an owner to a crate - some of the time it might go through, and some of the time they might have to accept an invitation. This also defeats the point of having to invite users as it would still be possible to add owners without permission.

Option 2

Let older versions of Cargo lie about adding a user. Change the current message on the Cargo side, but if a user has not upgraded Cargo, they will never get the message indicating that the user they invited must accept to be added as an owner.

Option 3

Return an 'error' from Crates.io. Since Cargo does use error text, use the error for now to indicate that the user has not been added as an owner. The error in this case would be used to transfer a sort of success, that the user was sent an invitation, but that they must still accept to be added as an owner. This is misleading as an error did not actually occur, it is intended that a user cannot just be added to be a crate owner. It would also be difficult to distinguish between actual errors and this 'error'.

Another thought was to combine options 1 and 3. The old route, owner_add would return an error and another function owner_invite would be added which returns ok/errors normally.

None of these options sound particularly appealing, is there anything else that can be done about this? Are any of the options preferred?

@alexcrichton
Copy link
Member

I'd personally be in favor of Option 2, we could backport a fix to Cargo to just display a message that crates.io responds with and deploy it in just a few weeks (if we do it soon in like 2-3 weeks!). Once the current stable Cargo "does the right thing" I'm not too worried about any historical Cargo "lying" about having someone added

@natboehm
Copy link
Contributor Author

Okay, thanks! I'm content going with Option 2, of the three it seems like the easiest solution.

bors added a commit that referenced this issue Sep 30, 2017
Owner invite messages

This PR addresses issue #4537, the plan for `cargo owner --add` requiring invitations in Cargo and the encompassing issue [#924](rust-lang/crates.io#924), requiring an invite to add someone as an owner in Crates.io.

Regarding the Cargo issue, we went with Option 2, changing the `add_owners` function to decode a struct sent from Crates containing a `boolean` and `String`, the `boolean` being the response status and `String` being the success message. This may sound redundant however we concluded that using both of these fields were necessary to support older versions of Cargo - if we changed Crates.io to only return the `String` message on success this would likely break systems using the older version of `add_owner` expecting a response containing a `boolean`. Matching this schema, `add_owners` on the Crates.io side will soon return a struct containing a `boolean` and `String`, and instead of adding a new crate owner to the database will add a crate owner invite. If successful, `modify_owners` now prints the message sent from Crates.io instead of the old hardcoded message.

Resolves #4537
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants