From 31b1dc3f4cc9308057ebbd8b19de6cd51bc2dab7 Mon Sep 17 00:00:00 2001 From: moskirathe Date: Fri, 28 Aug 2026 12:38:00 +0200 Subject: [PATCH 01/10] util: Add Rust `canon_username()` implementation The helper mirrors the PostgreSQL function so in-memory owner matching uses the same case folding and `-` to `_` normalization as database queries. --- src/util.rs | 1 + src/util/canon_username.rs | 65 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 src/util/canon_username.rs diff --git a/src/util.rs b/src/util.rs index 7ef863d203f..9661cc551a6 100644 --- a/src/util.rs +++ b/src/util.rs @@ -2,6 +2,7 @@ pub use self::io_util::{read_fill, read_le_u32}; pub use self::request_helpers::*; pub use crates_io_database::utils::token; +pub mod canon_username; pub mod diesel; pub mod errors; mod io_util; diff --git a/src/util/canon_username.rs b/src/util/canon_username.rs new file mode 100644 index 00000000000..d44b6def705 --- /dev/null +++ b/src/util/canon_username.rs @@ -0,0 +1,65 @@ +/// Replaces all instances of `-` with `_` in the given username +pub fn canon_username(username: &str) -> String { + username.replace("-", "_").to_lowercase() +} + +#[cfg(test)] +mod tests { + use super::*; + use crates_io_database::fns::canon_username as canon_username_sql; + use crates_io_test_db::TestDatabase; + use diesel_async::RunQueryDsl; + + const USERNAMES: &[(&str, &str)] = &[ + ("foo", "foo"), + ("Foo", "foo"), + ("FOO", "foo"), + ("foo-bar", "foo_bar"), + ("foo_bar", "foo_bar"), + ("Foo-Bar", "foo_bar"), + ("FOO-BAR", "foo_bar"), + ("foo-biz-bar", "foo_biz_bar"), + ("foo--bar", "foo__bar"), + ("-foo-", "_foo_"), + ("-", "_"), + ("user-2", "user_2"), + ("github:User-2", "github:user_2"), + ("", ""), + ]; + + #[test] + fn normalizes_case_and_separators() { + for &(input, expected) in USERNAMES { + assert_eq!(canon_username(input), expected); + } + } + + #[test] + fn usernames_differing_only_by_case_or_separator_match() { + assert_eq!(canon_username("foo-bar"), canon_username("foo_bar")); + assert_eq!(canon_username("Foo-Bar"), canon_username("fOO_bAR")); + assert_eq!(canon_username("user-2"), canon_username("USER_2")); + } + + #[test] + fn distinct_usernames_do_not_match() { + assert_ne!(canon_username("foobar"), canon_username("foo_bar")); + assert_ne!(canon_username("foo-bar"), canon_username("foo--bar")); + assert_ne!(canon_username("alice"), canon_username("alice2")); + } + + #[tokio::test] + async fn matches_the_canon_username_sql_implementation() { + let test_db = TestDatabase::new(); + let mut conn = test_db.async_connect().await; + + for &(input, _) in USERNAMES { + let from_sql: String = diesel::select(canon_username_sql(input)) + .get_result(&mut conn) + .await + .unwrap(); + + assert_eq!(canon_username(input), from_sql); + } + } +} From b92a3ba8f72d284e456a4d6dd3f6639523aafa7f Mon Sep 17 00:00:00 2001 From: moskirathe Date: Mon, 31 Aug 2026 13:51:52 +0200 Subject: [PATCH 02/10] controllers/owners: Support crates.io-prefixed additions The add-owner endpoint accepts `crates.io:username` arguments and resolves them using crates.io username canonicalization. Owner invitations now retain the selected username so responses identify the requested crates.io account without changing legacy unprefixed behavior. --- crates/crates_io_database/src/models/krate.rs | 2 +- crates/crates_io_database/src/models/owner.rs | 7 ++ packages/crates-io-api-client/schema.ts | 12 +- src/controllers/krate/owners.rs | 106 ++++++++++++++---- src/tests/routes/crates/owners/add.rs | 71 ++++++++++-- ..._openapi__openapi_internal_snapshot-2.snap | 10 +- ...egration__openapi__openapi_snapshot-2.snap | 10 +- src/tests/team.rs | 2 +- 8 files changed, 179 insertions(+), 41 deletions(-) diff --git a/crates/crates_io_database/src/models/krate.rs b/crates/crates_io_database/src/models/krate.rs index f18cf47a1e1..ef97bc6ce7d 100644 --- a/crates/crates_io_database/src/models/krate.rs +++ b/crates/crates_io_database/src/models/krate.rs @@ -265,7 +265,7 @@ impl Crate { pub enum NewOwnerInvite { /// The invitee was a [`User`], and they must accept the invite through the /// UI or via the provided invite token. - User(User, SecretString), + User(User, SecretString, String), /// The invitee was a [`Team`], and they were immediately added as an owner. Team(Team), diff --git a/crates/crates_io_database/src/models/owner.rs b/crates/crates_io_database/src/models/owner.rs index c95de392df9..7e55c161a6b 100644 --- a/crates/crates_io_database/src/models/owner.rs +++ b/crates/crates_io_database/src/models/owner.rs @@ -109,6 +109,13 @@ impl Owner { } } + pub fn username(&self) -> &str { + match self { + Owner::User(user) => &user.username, + Owner::Team(team) => &team.login, + } + } + pub fn id(&self) -> i32 { match self { Owner::User(user) => user.id, diff --git a/packages/crates-io-api-client/schema.ts b/packages/crates-io-api-client/schema.ts index 5dc0998cb99..1f9b81515c2 100644 --- a/packages/crates-io-api-client/schema.ts +++ b/packages/crates-io-api-client/schema.ts @@ -3293,9 +3293,13 @@ export interface operations { * * For users, use just the username (e.g., `"octocat"`). * For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). + * + * When adding an owner, use the `crates.io:username` prefix to explicitly + * select a crates.io username. * @example [ * "octocat", - * "github:rust-lang:owners" + * "github:rust-lang:owners", + * "crates.io:some_user" * ] */ owners: string[]; @@ -3358,9 +3362,13 @@ export interface operations { * * For users, use just the username (e.g., `"octocat"`). * For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). + * + * When adding an owner, use the `crates.io:username` prefix to explicitly + * select a crates.io username. * @example [ * "octocat", - * "github:rust-lang:owners" + * "github:rust-lang:owners", + * "crates.io:some_user" * ] */ owners: string[]; diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index cf9f6d38675..bb0700f2451 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -3,11 +3,13 @@ use crate::controllers::helpers::authorization::Rights; use crate::controllers::krate::CratePath; use crate::models::krate::OwnerRemoveError; -use crate::models::{Crate, Owner, PublicUser, Team, User}; +use crate::models::{Crate, Owner, PublicUser, Team, User, users_by_username}; use crate::models::{ CrateOwner, NewCrateOwnerInvitation, NewCrateOwnerInvitationOutcome, NewTeam, krate::NewOwnerInvite, token::EndpointScope, }; +use crate::schema::oauth_github; +use crate::util::canon_username::canon_username; use crate::util::errors::{AppResult, BoxedAppError, bad_request, custom, forbidden}; use crate::views::EncodableOwner; use crate::{App, app::AppState}; @@ -17,7 +19,7 @@ use chrono::Utc; use crates_io_encryption::TokenEncryption; use crates_io_github::{GitHubAuth, GitHubClient, GitHubError}; use diesel::prelude::*; -use diesel_async::{AsyncConnection, AsyncPgConnection}; +use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl}; use http::StatusCode; use http::request::Parts; use minijinja::context; @@ -190,21 +192,28 @@ pub async fn add_owners( let mut msgs = Vec::with_capacity(logins.len()); for login in &logins { - let login_test = - |owner: &Owner| owner.login().to_lowercase() == *login.to_lowercase(); + let parsed_login = Login::parse(login)?; + let login_test = |owner: &Owner| match parsed_login { + Login::GitHubTeam(_) | Login::Unprefixed(_) => { + owner.login().to_lowercase() == login.to_lowercase() + } + Login::CratesIo(username) => { + canon_username(owner.username()) == canon_username(username) + } + }; if owners.iter().any(login_test) { return Err(bad_request(format_args!("`{login}` is already an owner"))); } - match add_owner(&app, conn, user, &krate, login).await { + match add_owner(&app, conn, user, &krate, parsed_login).await { // A user was successfully invited, and they must accept // the invite, and a best-effort attempt should be made // to email them the invite token for one-click // acceptance. - Ok(NewOwnerInvite::User(invitee, token)) => { + Ok(NewOwnerInvite::User(invitee, token, username)) => { msgs.push(format!( "user {} has been invited to be an owner of crate {}", - invitee.gh_login, krate.name, + username, krate.name, )); if let Some(recipient) = invitee.verified_email(conn).await.ok().flatten() { @@ -319,7 +328,10 @@ pub struct ChangeOwnersRequest { /// /// For users, use just the username (e.g., `"octocat"`). /// For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). - #[schema(example = json!(["octocat", "github:rust-lang:owners"]))] + /// + /// When adding an owner, use the `crates.io:username` prefix to explicitly + /// select a crates.io username. + #[schema(example = json!(["octocat", "github:rust-lang:owners", "crates.io:some_user"]))] #[serde(alias = "users")] owners: Vec, } @@ -361,15 +373,35 @@ async fn add_owner( conn: &mut AsyncPgConnection, req_user: &User, krate: &Crate, - login: &str, + login: Login<'_>, ) -> Result { - match Login::parse(login)? { + match login { Login::GitHubTeam(team) => { let github = &*app.github; let encryption = &app.config.token_encryption; add_github_team_owner(github, conn, req_user, krate, team, encryption).await } - Login::Unprefixed(login) => invite_user_owner(app, conn, req_user, krate, login).await, + Login::CratesIo(username) => { + let user = find_user_by_username(conn, username) + .await + .optional()? + .ok_or_else(|| { + bad_request(format_args!( + "could not find user with crates.io username {username}" + )) + })?; + invite_user_owner(app, conn, req_user, user, username.to_owned(), krate).await + } + Login::Unprefixed(username) => { + let user = User::find_by_login(conn, username) + .await + .optional()? + .ok_or_else(|| { + bad_request(format_args!("could not find user with login `{username}`")) + })?; + let gh_login = user.gh_login.clone(); + invite_user_owner(app, conn, req_user, user, gh_login, krate).await + } } } @@ -377,6 +409,8 @@ async fn add_owner( enum Login<'a> { /// GitHub organization team, such as `github:rust-lang:owners`. GitHubTeam(GitHubTeamLogin<'a>), + /// crates.io user, such as `crates.io:octocat`. + CratesIo(&'a str), /// User login without a service prefix. Unprefixed(&'a str), } @@ -384,11 +418,37 @@ enum Login<'a> { impl<'a> Login<'a> { /// Parses an owner login. fn parse(login: &'a str) -> Result { - if !login.contains(':') { - return Ok(Self::Unprefixed(login)); + fn is_valid(value: &str, label: &str) -> Result { + if value.is_empty() { + return Err(bad_request(format_args!("{label} cannot be empty"))); + } + + if let Some(character) = value.chars().find( + |character| !matches!(character, 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_'), + ) { + return Err(bad_request(format_args!( + "{label} cannot contain special characters like {character}" + ))); + } + + Ok(true) } - GitHubTeamLogin::parse(login).map(Self::GitHubTeam) + match login.split(':').collect::>().as_slice() { + ["github", org, team] if is_valid(org, "organization")? && is_valid(team, "team")? => { + GitHubTeamLogin::parse(login).map(Self::GitHubTeam) + } + ["crates.io", username] if is_valid(username, "username")? => { + Ok(Self::CratesIo(username)) + } + ["github", _] => Err(bad_request( + "missing github team argument; format is github:org:team", + )), + [username] if is_valid(username, "username")? => Ok(Self::Unprefixed(username)), + _ => Err(bad_request( + "invalid argument. only github:org:team, crates.io:username and username are supported.", + )), + } } } @@ -423,14 +483,10 @@ async fn invite_user_owner( app: &App, conn: &mut AsyncPgConnection, req_user: &User, + user: User, + username: String, krate: &Crate, - login: &str, ) -> Result { - let user = User::find_by_login(conn, login) - .await - .optional()? - .ok_or_else(|| bad_request(format_args!("could not find user with login `{login}`")))?; - // Users are invited and must accept before being added let expires_at = Utc::now() + app.config.ownership_invitations_expiration; let invite = NewCrateOwnerInvitation { @@ -442,7 +498,7 @@ async fn invite_user_owner( match invite.create(conn).await? { NewCrateOwnerInvitationOutcome::InviteCreated { plaintext_token } => { - Ok(NewOwnerInvite::User(user, plaintext_token)) + Ok(NewOwnerInvite::User(user, plaintext_token, username)) } NewCrateOwnerInvitationOutcome::AlreadyExists => { Err(OwnerAddError::AlreadyInvited(Box::new(user))) @@ -450,6 +506,14 @@ async fn invite_user_owner( } } +async fn find_user_by_username(conn: &mut AsyncPgConnection, username: &str) -> QueryResult { + users_by_username(username) + .left_join(oauth_github::table) + .select(User::as_select()) + .first(conn) + .await +} + async fn add_github_team_owner( gh_client: &dyn GitHubClient, conn: &mut AsyncPgConnection, diff --git a/src/tests/routes/crates/owners/add.rs b/src/tests/routes/crates/owners/add.rs index c26d856534d..64d57e60104 100644 --- a/src/tests/routes/crates/owners/add.rs +++ b/src/tests/routes/crates/owners/add.rs @@ -2,7 +2,10 @@ use crate::OwnerResp; use crate::builders::{CrateBuilder, UserBuilder}; use crate::owners::expire_invitation; use crate::util::{RequestHelper, Response, TestApp}; -use crates_io::models::token::{CrateScope, EndpointScope}; +use crates_io::models::{ + CrateOwner, + token::{CrateScope, EndpointScope}, +}; use insta::assert_snapshot; // This is testing Cargo functionality! ! ! @@ -114,22 +117,22 @@ async fn unprefixed_github_login_separator_variant() { #[tokio::test(flavor = "multi_thread")] async fn crates_io_prefixed_username_verbatim() { let response = invite_distinct_login_user("crates.io:crates-user").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"user crates-user has been invited to be an owner of crate foo","ok":true}"#); } #[tokio::test(flavor = "multi_thread")] async fn crates_io_prefixed_username_case_insensitive() { let response = invite_distinct_login_user("crates.io:CRATES-USER").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"user CRATES-USER has been invited to be an owner of crate foo","ok":true}"#); } #[tokio::test(flavor = "multi_thread")] async fn crates_io_prefixed_username_separator_variant() { let response = invite_distinct_login_user("crates.io:crates_user").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"user crates_user has been invited to be an owner of crate foo","ok":true}"#); } #[tokio::test(flavor = "multi_thread")] @@ -157,7 +160,59 @@ async fn github_prefixed_login_separator_variant() { async fn crates_io_prefix_does_not_match_github_login() { let response = invite_distinct_login_user("crates.io:github-user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with crates.io username github-user"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn crates_io_prefixed_username_not_found() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "crates.io:nonexistent").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with crates.io username nonexistent"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn crates_io_prefixed_username_already_owner() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let user = app.db_new_user("user2").await; + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + + let response = cookie.add_named_owner("foo", "crates.io:user2").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"`crates.io:user2` is already an owner"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn crates_io_prefixed_username_empty() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "crates.io:").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot be empty"}]}"#); } #[tokio::test(flavor = "multi_thread")] diff --git a/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap b/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap index bea546fabff..382275ab360 100644 --- a/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap +++ b/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap @@ -3640,10 +3640,11 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` prefix to explicitly\nselect a crates.io username.", "example": [ "octocat", - "github:rust-lang:owners" + "github:rust-lang:owners", + "crates.io:some_user" ], "items": { "type": "string" @@ -3800,10 +3801,11 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` prefix to explicitly\nselect a crates.io username.", "example": [ "octocat", - "github:rust-lang:owners" + "github:rust-lang:owners", + "crates.io:some_user" ], "items": { "type": "string" diff --git a/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap b/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap index 055ad1a7778..5927a8fe5c9 100644 --- a/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap +++ b/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap @@ -2907,10 +2907,11 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` prefix to explicitly\nselect a crates.io username.", "example": [ "octocat", - "github:rust-lang:owners" + "github:rust-lang:owners", + "crates.io:some_user" ], "items": { "type": "string" @@ -3067,10 +3068,11 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` prefix to explicitly\nselect a crates.io username.", "example": [ "octocat", - "github:rust-lang:owners" + "github:rust-lang:owners", + "crates.io:some_user" ], "items": { "type": "string" diff --git a/src/tests/team.rs b/src/tests/team.rs index 806f8eb6434..d4363bea6f4 100644 --- a/src/tests/team.rs +++ b/src/tests/team.rs @@ -40,7 +40,7 @@ async fn not_github() { .add_named_owner("foo_not_github", "dropbox:foo:foo") .await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid argument. only github:org:team, crates.io:username and username are supported."}]}"#); } #[tokio::test(flavor = "multi_thread")] From 98025c35a4ee79dffe52084b74d5af7de0e50363 Mon Sep 17 00:00:00 2001 From: moskirathe Date: Fri, 28 Aug 2026 12:38:25 +0200 Subject: [PATCH 03/10] database: Index `oauth_github.login` case-insensitively The expression index supports queries using `lower(oauth_github.login)`. Concurrent creation and removal avoid blocking writes and therefore run outside a transaction. --- .../2026-07-18-120001-0000_add_oauth_github_login_index/down.sql | 1 + .../metadata.toml | 1 + .../2026-07-18-120001-0000_add_oauth_github_login_index/up.sql | 1 + 3 files changed, 3 insertions(+) create mode 100644 migrations/2026-07-18-120001-0000_add_oauth_github_login_index/down.sql create mode 100644 migrations/2026-07-18-120001-0000_add_oauth_github_login_index/metadata.toml create mode 100644 migrations/2026-07-18-120001-0000_add_oauth_github_login_index/up.sql diff --git a/migrations/2026-07-18-120001-0000_add_oauth_github_login_index/down.sql b/migrations/2026-07-18-120001-0000_add_oauth_github_login_index/down.sql new file mode 100644 index 00000000000..53a2c516a45 --- /dev/null +++ b/migrations/2026-07-18-120001-0000_add_oauth_github_login_index/down.sql @@ -0,0 +1 @@ +DROP INDEX CONCURRENTLY IF EXISTS index_oauth_github_login; diff --git a/migrations/2026-07-18-120001-0000_add_oauth_github_login_index/metadata.toml b/migrations/2026-07-18-120001-0000_add_oauth_github_login_index/metadata.toml new file mode 100644 index 00000000000..79e9221c1f2 --- /dev/null +++ b/migrations/2026-07-18-120001-0000_add_oauth_github_login_index/metadata.toml @@ -0,0 +1 @@ +run_in_transaction = false diff --git a/migrations/2026-07-18-120001-0000_add_oauth_github_login_index/up.sql b/migrations/2026-07-18-120001-0000_add_oauth_github_login_index/up.sql new file mode 100644 index 00000000000..17f7debfb0d --- /dev/null +++ b/migrations/2026-07-18-120001-0000_add_oauth_github_login_index/up.sql @@ -0,0 +1 @@ +CREATE INDEX CONCURRENTLY IF NOT EXISTS index_oauth_github_login ON oauth_github (lower(login)); From 99a27264e0f06f676fb2f743bf5d107209feff6c Mon Sep 17 00:00:00 2001 From: moskirathe Date: Fri, 28 Aug 2026 12:39:18 +0200 Subject: [PATCH 04/10] models/user: Resolve OAuth accounts by GitHub login `OauthGithub::find_by_login()` selects the matching account with the highest account ID when historical rows differ only by case. Joined `User` queries also expose the OAuth login required by owner resolution. --- crates/crates_io_database/src/models/user.rs | 68 +++++++++++++++++++ .../crates_io_test_utils/src/builders/user.rs | 3 +- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/crates/crates_io_database/src/models/user.rs b/crates/crates_io_database/src/models/user.rs index 10513bc5ceb..806f825a363 100644 --- a/crates/crates_io_database/src/models/user.rs +++ b/crates/crates_io_database/src/models/user.rs @@ -77,6 +77,10 @@ pub struct User { pub name: Option, pub gh_id: i32, pub gh_login: String, + // This is the same as gh_login, but reads from oauth_github instead. + // Can rename to `gh_login` or something more appropriate when gh_login is removed from this struct. + #[diesel(select_expression = oauth_github::login.nullable())] + pub gh_username: Option, #[diesel(select_expression = oauth_github::avatar.nullable())] pub gh_avatar: Option, #[diesel(select_expression = oauth_github::encrypted_token.nullable())] @@ -216,6 +220,20 @@ pub struct OauthGithub { pub user_id: i32, } +impl OauthGithub { + pub async fn find_by_login( + mut conn: &AsyncPgConnection, + login: &str, + ) -> QueryResult { + oauth_github::table + .filter(canon_username(oauth_github::login).eq(canon_username(login))) + .filter(oauth_github::account_id.ne(-1)) + .order(oauth_github::account_id.desc()) + .first(&mut conn) + .await + } +} + /// Represents a new crates.io user to GitHub user OAuth link to be inserted into the /// `oauth_github` table. #[derive(Insertable, Debug, Builder)] @@ -245,3 +263,53 @@ impl NewOauthGithub<'_> { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crates_io_test_db::TestDatabase; + + async fn insert_user( + conn: &AsyncPgConnection, + username: &str, + gh_login: &str, + gh_id: i32, + ) -> QueryResult { + let user_id = NewUser::builder() + .gh_id(gh_id) + .gh_login(gh_login) + .username(username) + .build() + .insert(conn) + .await?; + + NewOauthGithub::builder() + .account_id(gh_id as i64) + .encrypted_token(&[]) + .login(gh_login) + .user_id(user_id) + .build() + .insert(conn) + .await?; + + Ok(user_id) + } + + #[tokio::test] + async fn test_find_by_login_returns_highest_account_id_account() { + let test_db = TestDatabase::new(); + let conn = test_db.async_connect().await; + + insert_user(&conn, "alice", "alice", 100).await.unwrap(); + let user_id = insert_user(&conn, "alice", "Alice", 200).await.unwrap(); + + // case-insensitive checks + for login in ["alice", "Alice", "ALICE"] { + let user = OauthGithub::find_by_login(&conn, login).await.unwrap(); + + assert_eq!(user.account_id, 200); + assert_eq!(user.user_id, user_id); + assert_eq!(user.login, "Alice"); + } + } +} diff --git a/crates/crates_io_test_utils/src/builders/user.rs b/crates/crates_io_test_utils/src/builders/user.rs index 28f8b0c6274..746cf9c13aa 100644 --- a/crates/crates_io_test_utils/src/builders/user.rs +++ b/crates/crates_io_test_utils/src/builders/user.rs @@ -60,8 +60,9 @@ impl<'a> UserBuilder<'a> { pub fn build(self) -> User { User { id: 1, - gh_login: self.gh_login.into(), name: self.display_name.map(ToString::to_string), + gh_login: self.gh_login.into(), + gh_username: Some(self.gh_login.into()), gh_id: 123, gh_avatar: None, gh_encrypted_token: None, From 82f29412e59134b19f3cd14724b4adeda8092c98 Mon Sep 17 00:00:00 2001 From: moskirathe Date: Mon, 31 Aug 2026 13:58:42 +0200 Subject: [PATCH 05/10] controllers/owners: Support GitHub-prefixed additions The add-owner endpoint accepts `github:username` arguments and resolves them through linked OAuth accounts. Shared owner-argument validation distinguishes GitHub users from teams while both prefixed removal forms remain unsupported. --- crates/crates_io_database/src/models/owner.rs | 6 +- packages/crates-io-api-client/schema.ts | 14 +- src/controllers/krate/owners.rs | 46 +- src/tests/issues/issue1205.rs | 8 +- src/tests/routes/crates/owners/add.rs | 394 +++++++++++++++--- ..._openapi__openapi_internal_snapshot-2.snap | 10 +- ...egration__openapi__openapi_snapshot-2.snap | 10 +- src/tests/team.rs | 106 ++++- 8 files changed, 488 insertions(+), 106 deletions(-) diff --git a/crates/crates_io_database/src/models/owner.rs b/crates/crates_io_database/src/models/owner.rs index 7e55c161a6b..41ee0a85f27 100644 --- a/crates/crates_io_database/src/models/owner.rs +++ b/crates/crates_io_database/src/models/owner.rs @@ -102,10 +102,10 @@ impl Owner { } } - pub fn login(&self) -> &str { + pub fn gh_login(&self) -> Option<&str> { match self { - Owner::User(user) => &user.gh_login, - Owner::Team(team) => &team.login, + Owner::User(user) => user.gh_username.as_deref(), + Owner::Team(team) => Some(&team.login), } } diff --git a/packages/crates-io-api-client/schema.ts b/packages/crates-io-api-client/schema.ts index 1f9b81515c2..a4cfa57ace4 100644 --- a/packages/crates-io-api-client/schema.ts +++ b/packages/crates-io-api-client/schema.ts @@ -3294,12 +3294,13 @@ export interface operations { * For users, use just the username (e.g., `"octocat"`). * For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). * - * When adding an owner, use the `crates.io:username` prefix to explicitly - * select a crates.io username. + * When adding an owner, use the `crates.io:username` or `github:username` + * prefix to explicitly select the username's service. * @example [ * "octocat", * "github:rust-lang:owners", - * "crates.io:some_user" + * "crates.io:some_user", + * "github:other_user" * ] */ owners: string[]; @@ -3363,12 +3364,13 @@ export interface operations { * For users, use just the username (e.g., `"octocat"`). * For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). * - * When adding an owner, use the `crates.io:username` prefix to explicitly - * select a crates.io username. + * When adding an owner, use the `crates.io:username` or `github:username` + * prefix to explicitly select the username's service. * @example [ * "octocat", * "github:rust-lang:owners", - * "crates.io:some_user" + * "crates.io:some_user", + * "github:other_user" * ] */ owners: string[]; diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index bb0700f2451..2b7ceddaa5a 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -16,6 +16,7 @@ use crate::{App, app::AppState}; use crate::{auth::AuthCheck, email::EmailMessage}; use axum::Json; use chrono::Utc; +use crates_io_database::models::OauthGithub; use crates_io_encryption::TokenEncryption; use crates_io_github::{GitHubAuth, GitHubClient, GitHubError}; use diesel::prelude::*; @@ -194,12 +195,18 @@ pub async fn add_owners( for login in &logins { let parsed_login = Login::parse(login)?; let login_test = |owner: &Owner| match parsed_login { - Login::GitHubTeam(_) | Login::Unprefixed(_) => { - owner.login().to_lowercase() == login.to_lowercase() + Login::GitHubTeam(_) => { + canon_username(owner.username()) == canon_username(login) } + Login::GitHub(username) => owner + .gh_login() + .is_some_and(|login| canon_username(login) == canon_username(username)), Login::CratesIo(username) => { canon_username(owner.username()) == canon_username(username) } + Login::Unprefixed(_) => owner.gh_login().is_some_and(|owner_login| { + owner_login.to_lowercase() == login.to_lowercase() + }), }; if owners.iter().any(login_test) { return Err(bad_request(format_args!("`{login}` is already an owner"))); @@ -329,9 +336,9 @@ pub struct ChangeOwnersRequest { /// For users, use just the username (e.g., `"octocat"`). /// For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). /// - /// When adding an owner, use the `crates.io:username` prefix to explicitly - /// select a crates.io username. - #[schema(example = json!(["octocat", "github:rust-lang:owners", "crates.io:some_user"]))] + /// When adding an owner, use the `crates.io:username` or `github:username` + /// prefix to explicitly select the username's service. + #[schema(example = json!(["octocat", "github:rust-lang:owners", "crates.io:some_user", "github:other_user"]))] #[serde(alias = "users")] owners: Vec, } @@ -381,6 +388,18 @@ async fn add_owner( let encryption = &app.config.token_encryption; add_github_team_owner(github, conn, req_user, krate, team, encryption).await } + Login::GitHub(username) => { + let oauth = OauthGithub::find_by_login(conn, username) + .await + .optional()? + .ok_or_else(|| { + bad_request(format_args!( + "could not find user with github username {username}" + )) + })?; + let user = User::find(conn, oauth.user_id).await?; + invite_user_owner(app, conn, req_user, user, username, krate).await + } Login::CratesIo(username) => { let user = find_user_by_username(conn, username) .await @@ -390,7 +409,7 @@ async fn add_owner( "could not find user with crates.io username {username}" )) })?; - invite_user_owner(app, conn, req_user, user, username.to_owned(), krate).await + invite_user_owner(app, conn, req_user, user, username, krate).await } Login::Unprefixed(username) => { let user = User::find_by_login(conn, username) @@ -399,8 +418,7 @@ async fn add_owner( .ok_or_else(|| { bad_request(format_args!("could not find user with login `{username}`")) })?; - let gh_login = user.gh_login.clone(); - invite_user_owner(app, conn, req_user, user, gh_login, krate).await + invite_user_owner(app, conn, req_user, user, username, krate).await } } } @@ -409,6 +427,8 @@ async fn add_owner( enum Login<'a> { /// GitHub organization team, such as `github:rust-lang:owners`. GitHubTeam(GitHubTeamLogin<'a>), + /// GitHub user, such as `github:octocat`. + GitHub(&'a str), /// crates.io user, such as `crates.io:octocat`. CratesIo(&'a str), /// User login without a service prefix. @@ -438,15 +458,13 @@ impl<'a> Login<'a> { ["github", org, team] if is_valid(org, "organization")? && is_valid(team, "team")? => { GitHubTeamLogin::parse(login).map(Self::GitHubTeam) } + ["github", username] if is_valid(username, "username")? => Ok(Self::GitHub(username)), ["crates.io", username] if is_valid(username, "username")? => { Ok(Self::CratesIo(username)) } - ["github", _] => Err(bad_request( - "missing github team argument; format is github:org:team", - )), [username] if is_valid(username, "username")? => Ok(Self::Unprefixed(username)), _ => Err(bad_request( - "invalid argument. only github:org:team, crates.io:username and username are supported.", + "invalid argument. only github:org:team, github:username, crates.io:username and username are supported.", )), } } @@ -484,7 +502,7 @@ async fn invite_user_owner( conn: &mut AsyncPgConnection, req_user: &User, user: User, - username: String, + username: &str, krate: &Crate, ) -> Result { // Users are invited and must accept before being added @@ -498,7 +516,7 @@ async fn invite_user_owner( match invite.create(conn).await? { NewCrateOwnerInvitationOutcome::InviteCreated { plaintext_token } => { - Ok(NewOwnerInvite::User(user, plaintext_token, username)) + Ok(NewOwnerInvite::User(user, plaintext_token, username.into())) } NewCrateOwnerInvitationOutcome::AlreadyExists => { Err(OwnerAddError::AlreadyInvited(Box::new(user))) diff --git a/src/tests/issues/issue1205.rs b/src/tests/issues/issue1205.rs index 5d2cdc5d815..937aeda22d8 100644 --- a/src/tests/issues/issue1205.rs +++ b/src/tests/issues/issue1205.rs @@ -27,8 +27,8 @@ async fn test_issue_1205() -> anyhow::Result<()> { let owners = krate.owners(&conn).await?; assert_eq!(owners.len(), 2); - assert_eq!(owners[0].login(), "foo"); - assert_eq!(owners[1].login(), "github:rustaudio:owners"); + assert_eq!(owners[0].username(), "foo"); + assert_eq!(owners[1].username(), "github:rustaudio:owners"); let response = user .add_named_owner(CRATE_NAME, "github:rustaudio:cratesio-push") @@ -38,8 +38,8 @@ async fn test_issue_1205() -> anyhow::Result<()> { let owners = krate.owners(&conn).await?; assert_eq!(owners.len(), 2); - assert_eq!(owners[0].login(), "foo"); - assert_eq!(owners[1].login(), "github:rustaudio:cratesio-push"); + assert_eq!(owners[0].username(), "foo"); + assert_eq!(owners[1].username(), "github:rustaudio:cratesio-push"); let response = user .remove_named_owner(CRATE_NAME, "github:rustaudio:owners") diff --git a/src/tests/routes/crates/owners/add.rs b/src/tests/routes/crates/owners/add.rs index 64d57e60104..cd30af6bea4 100644 --- a/src/tests/routes/crates/owners/add.rs +++ b/src/tests/routes/crates/owners/add.rs @@ -1,11 +1,9 @@ use crate::OwnerResp; -use crate::builders::{CrateBuilder, UserBuilder}; +use crate::builders::{CrateBuilder, OauthGithubBuilder, UserBuilder}; use crate::owners::expire_invitation; use crate::util::{RequestHelper, Response, TestApp}; -use crates_io::models::{ - CrateOwner, - token::{CrateScope, EndpointScope}, -}; +use crates_io::models::CrateOwner; +use crates_io::models::token::{CrateScope, EndpointScope}; use insta::assert_snapshot; // This is testing Cargo functionality! ! ! @@ -104,7 +102,7 @@ async fn unprefixed_github_login_verbatim() { async fn unprefixed_github_login_case_insensitive() { let response = invite_distinct_login_user("GITHUB-USER").await; assert_snapshot!(response.status(), @"200 OK"); - assert_snapshot!(response.text(), @r#"{"msg":"user github-user has been invited to be an owner of crate foo","ok":true}"#); + assert_snapshot!(response.text(), @r#"{"msg":"user GITHUB-USER has been invited to be an owner of crate foo","ok":true}"#); } #[tokio::test(flavor = "multi_thread")] @@ -138,22 +136,22 @@ async fn crates_io_prefixed_username_separator_variant() { #[tokio::test(flavor = "multi_thread")] async fn github_prefixed_login_verbatim() { let response = invite_distinct_login_user("github:github-user").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"user github-user has been invited to be an owner of crate foo","ok":true}"#); } #[tokio::test(flavor = "multi_thread")] async fn github_prefixed_login_case_insensitive() { let response = invite_distinct_login_user("github:GITHUB-USER").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"user GITHUB-USER has been invited to be an owner of crate foo","ok":true}"#); } #[tokio::test(flavor = "multi_thread")] async fn github_prefixed_login_separator_variant() { let response = invite_distinct_login_user("github:github_user").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"user github_user has been invited to be an owner of crate foo","ok":true}"#); } #[tokio::test(flavor = "multi_thread")] @@ -163,63 +161,11 @@ async fn crates_io_prefix_does_not_match_github_login() { assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with crates.io username github-user"}]}"#); } -#[tokio::test(flavor = "multi_thread")] -async fn crates_io_prefixed_username_not_found() { - let (app, _, cookie) = TestApp::full().with_user().await; - let mut conn = app.db_conn().await; - - CrateBuilder::new("foo", cookie.as_model().id) - .expect_build(&mut conn) - .await; - - let response = cookie.add_named_owner("foo", "crates.io:nonexistent").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with crates.io username nonexistent"}]}"#); -} - -#[tokio::test(flavor = "multi_thread")] -async fn crates_io_prefixed_username_already_owner() { - let (app, _, cookie) = TestApp::full().with_user().await; - let mut conn = app.db_conn().await; - - let user = app.db_new_user("user2").await; - let krate = CrateBuilder::new("foo", cookie.as_model().id) - .expect_build(&mut conn) - .await; - - CrateOwner::builder() - .crate_id(krate.id) - .user_id(user.as_model().id) - .created_by(cookie.as_model().id) - .build() - .insert(&conn) - .await - .unwrap(); - - let response = cookie.add_named_owner("foo", "crates.io:user2").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"`crates.io:user2` is already an owner"}]}"#); -} - -#[tokio::test(flavor = "multi_thread")] -async fn crates_io_prefixed_username_empty() { - let (app, _, cookie) = TestApp::full().with_user().await; - let mut conn = app.db_conn().await; - - CrateBuilder::new("foo", cookie.as_model().id) - .expect_build(&mut conn) - .await; - - let response = cookie.add_named_owner("foo", "crates.io:").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot be empty"}]}"#); -} - #[tokio::test(flavor = "multi_thread")] async fn github_prefix_does_not_match_crates_io_username() { let response = invite_distinct_login_user("github:crates-user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with github username crates-user"}]}"#); } #[tokio::test(flavor = "multi_thread")] @@ -554,3 +500,321 @@ async fn no_invite_emails_for_txn_rollback() { // 9 emails to the good invitees should have been sent. assert_eq!(app.emails().await.len(), 9); } + +#[tokio::test(flavor = "multi_thread")] +async fn test_unsupported_disambiguation_prefix() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + app.db_new_user("user2").await; + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "gitlab:user2").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid argument. only github:org:team, github:username, crates.io:username and username are supported."}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_disambiguated_github_username_not_found() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "github:nonexistent").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with github username nonexistent"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_disambiguated_cratesio_username_not_found() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "crates.io:nonexistent").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with crates.io username nonexistent"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_already_owner_cratesio() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let user2 = app.db_new_user("user2").await; + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + // add existing owner + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user2.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + + let response = cookie.add_named_owner("foo", "crates.io:user2").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"`crates.io:user2` is already an owner"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_already_owner_github() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + // `user2` has matching crates.io username and GitHub login. + let user2 = app.db_new_user("user2").await; + OauthGithubBuilder::for_user(user2.as_model()) + .with_login("user2") + .insert(&conn) + .await; + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + // add existing owner + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user2.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + + let response = cookie.add_named_owner("foo", "github:user2").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"`github:user2` is already an owner"}]}"#); +} + +/// An existing owner whose crates.io username differs from their GitHub login +/// is still detected as "already an owner" when re-added via the `github:` +/// prefix, because the duplicate check matches `github:` logins against the +/// owner's GitHub login (not their crates.io username). +#[tokio::test(flavor = "multi_thread")] +async fn test_already_owner_github_mismatched_username() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let builder = UserBuilder::new() + .with_username("user2") + .with_gh_login("user2-gh"); + let user2 = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(user2.as_model()) + .with_login("user2-gh") + .insert(&conn) + .await; + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + // add existing owner + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user2.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + + let response = cookie.add_named_owner("foo", "github:user2-gh").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"`github:user2-gh` is already an owner"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_team_with_extra_component() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie + .add_named_owner("foo", "github:alice:team:extra") + .await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid argument. only github:org:team, github:username, crates.io:username and username are supported."}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_empty_org() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "github::team").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"organization cannot be empty"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_empty_org_with_extra_component() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "github::team:extra").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid argument. only github:org:team, github:username, crates.io:username and username are supported."}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_empty_team() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "github:org:").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"team cannot be empty"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_empty_github_username() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "github:").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot be empty"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_empty_cratesio_username() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "crates.io:").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot be empty"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_empty_login() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot be empty"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_single_colon() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", ":").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid argument. only github:org:team, github:username, crates.io:username and username are supported."}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_double_colon() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "::").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid argument. only github:org:team, github:username, crates.io:username and username are supported."}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_github_username_with_invalid_char() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "github:a&lice*").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot contain special characters like &"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_org_with_invalid_char() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "github:or&g:team").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"organization cannot contain special characters like &"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_team_with_invalid_char() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "github:org:te@m").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"team cannot contain special characters like @"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_unprefixed_login_with_invalid_char() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "a&lice").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot contain special characters like &"}]}"#); +} diff --git a/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap b/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap index 382275ab360..ed5c3ca095f 100644 --- a/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap +++ b/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap @@ -3640,11 +3640,12 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` prefix to explicitly\nselect a crates.io username.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` or `github:username`\nprefix to explicitly select the username's service.", "example": [ "octocat", "github:rust-lang:owners", - "crates.io:some_user" + "crates.io:some_user", + "github:other_user" ], "items": { "type": "string" @@ -3801,11 +3802,12 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` prefix to explicitly\nselect a crates.io username.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` or `github:username`\nprefix to explicitly select the username's service.", "example": [ "octocat", "github:rust-lang:owners", - "crates.io:some_user" + "crates.io:some_user", + "github:other_user" ], "items": { "type": "string" diff --git a/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap b/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap index 5927a8fe5c9..ddc7f24ba62 100644 --- a/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap +++ b/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap @@ -2907,11 +2907,12 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` prefix to explicitly\nselect a crates.io username.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` or `github:username`\nprefix to explicitly select the username's service.", "example": [ "octocat", "github:rust-lang:owners", - "crates.io:some_user" + "crates.io:some_user", + "github:other_user" ], "items": { "type": "string" @@ -3068,11 +3069,12 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` prefix to explicitly\nselect a crates.io username.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` or `github:username`\nprefix to explicitly select the username's service.", "example": [ "octocat", "github:rust-lang:owners", - "crates.io:some_user" + "crates.io:some_user", + "github:other_user" ], "items": { "type": "string" diff --git a/src/tests/team.rs b/src/tests/team.rs index d4363bea6f4..43e4b96b180 100644 --- a/src/tests/team.rs +++ b/src/tests/team.rs @@ -40,7 +40,7 @@ async fn not_github() { .add_named_owner("foo_not_github", "dropbox:foo:foo") .await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid argument. only github:org:team, crates.io:username and username are supported."}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid argument. only github:org:team, github:username, crates.io:username and username are supported."}]}"#); } #[tokio::test(flavor = "multi_thread")] @@ -59,7 +59,7 @@ async fn weird_name() { assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"organization cannot contain special characters like /"}]}"#); } -/// Tests adding team without second `:` +/// Resolved as a disambiguated username. #[tokio::test(flavor = "multi_thread")] async fn one_colon() { let (app, _, user, token) = TestApp::init().with_token().await; @@ -69,9 +69,9 @@ async fn one_colon() { .expect_build(&mut conn) .await; - let response = token.add_named_owner("foo_one_colon", "github:foo").await; + let response = token.add_named_owner("foo_one_colon", "github:user2").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with github username user2"}]}"#); } #[tokio::test(flavor = "multi_thread")] @@ -132,6 +132,100 @@ async fn add_renamed_team() -> anyhow::Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread")] +async fn too_many_colons() { + let (app, _, user, token) = TestApp::init().with_token().await; + let mut conn = app.db_conn().await; + CrateBuilder::new("foo_too_many_colons", user.as_model().id) + .expect_build(&mut conn) + .await; + + let response = token + .add_named_owner("foo_too_many_colons", "github:test:core:extra") + .await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid argument. only github:org:team, github:username, crates.io:username and username are supported."}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn empty_org() { + let (app, _, user, token) = TestApp::init().with_token().await; + let mut conn = app.db_conn().await; + CrateBuilder::new("foo_empty_org", user.as_model().id) + .expect_build(&mut conn) + .await; + + let response = token.add_named_owner("foo_empty_org", "github::core").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"organization cannot be empty"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn empty_team() { + let (app, _, user, token) = TestApp::init().with_token().await; + let mut conn = app.db_conn().await; + CrateBuilder::new("foo_empty_team", user.as_model().id) + .expect_build(&mut conn) + .await; + + let response = token + .add_named_owner("foo_empty_team", "github:test-org:") + .await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"team cannot be empty"}]}"#); +} + +/// Re-adding a team that is already an owner is rejected. +#[tokio::test(flavor = "multi_thread")] +async fn already_owner_team() { + let (app, _) = TestApp::init().empty().await; + let mut conn = app.db_conn().await; + let user = app.db_new_user("user-all-teams").await; + let token = user.db_new_token("arbitrary token name").await; + + CrateBuilder::new("foo_already_team", user.as_model().id) + .expect_build(&mut conn) + .await; + + token + .add_named_owner("foo_already_team", "github:test-org:core") + .await + .good(); + + let response = token + .add_named_owner("foo_already_team", "github:test-org:core") + .await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"`github:test-org:core` is already an owner"}]}"#); +} + +/// Removing a team owner works and is case-insensitive in the team name. +#[tokio::test(flavor = "multi_thread")] +async fn remove_team_case_insensitive() { + let (app, anon) = TestApp::init().empty().await; + let mut conn = app.db_conn().await; + let user = app.db_new_user("user-all-teams").await; + let token = user.db_new_token("arbitrary token name").await; + + CrateBuilder::new("foo_remove_team_case", user.as_model().id) + .expect_build(&mut conn) + .await; + + token + .add_named_owner("foo_remove_team_case", "github:test-org:core") + .await + .good(); + + let response = token + .remove_named_owner("foo_remove_team_case", "github:test-ORG:COre") + .await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); + + let json = anon.crate_owner_teams("foo_remove_team_case").await.good(); + assert_eq!(json.teams.len(), 0); +} + /// Tests adding team names with mixed case, when on the team #[tokio::test(flavor = "multi_thread")] async fn add_team_mixed_case() -> anyhow::Result<()> { @@ -153,7 +247,7 @@ async fn add_team_mixed_case() -> anyhow::Result<()> { let owners = krate.owners(&conn).await?; assert_eq!(owners.len(), 2); let owner = &owners[1]; - assert_eq!(owner.login(), owner.login().to_lowercase()); + assert_eq!(owner.username(), owner.username().to_lowercase()); let json = anon.crate_owner_teams("foo_mixed_case").await.good(); assert_eq!(json.teams.len(), 1); @@ -182,7 +276,7 @@ async fn add_team_as_org_owner() -> anyhow::Result<()> { let owners = krate.owners(&conn).await?; assert_eq!(owners.len(), 2); let owner = &owners[1]; - assert_eq!(owner.login(), owner.login().to_lowercase()); + assert_eq!(owner.username(), owner.username().to_lowercase()); let json = anon.crate_owner_teams("foo_org_owner").await.good(); assert_eq!(json.teams.len(), 1); From c3b7f02ad07c736dfda8af9861880e4aeb9dd880 Mon Sep 17 00:00:00 2001 From: moskirathe Date: Fri, 28 Aug 2026 12:44:43 +0200 Subject: [PATCH 06/10] models/krate: Add namespace-specific owner removal `Crate` provides separate removal queries for canonical crates.io usernames and linked GitHub logins. Owner routes select the query for the parsed user or team argument. --- crates/crates_io_database/src/models/krate.rs | 47 +++++++++++++++++-- src/controllers/krate/owners.rs | 25 +++++++++- src/tests/owners.rs | 5 +- src/tests/routes/crates/list.rs | 5 +- src/tests/routes/crates/owners/remove.rs | 22 ++++----- src/tests/routes/me/get.rs | 2 +- src/tests/routes/users/stats.rs | 2 +- src/tests/team.rs | 5 +- 8 files changed, 93 insertions(+), 20 deletions(-) diff --git a/crates/crates_io_database/src/models/krate.rs b/crates/crates_io_database/src/models/krate.rs index ef97bc6ce7d..8a6dcb00355 100644 --- a/crates/crates_io_database/src/models/krate.rs +++ b/crates/crates_io_database/src/models/krate.rs @@ -213,7 +213,8 @@ impl Crate { Ok(users.chain(teams).collect()) } - pub async fn owner_remove( + /// Remove owner given a crates.io username. + pub async fn owner_remove_with_username( &self, mut conn: &AsyncPgConnection, login: &str, @@ -225,7 +226,7 @@ impl Crate { CASE WHEN crate_owners.owner_kind = 1 THEN teams.login ELSE - users.gh_login + users.username END AS login FROM crate_owners LEFT JOIN teams @@ -243,7 +244,47 @@ impl Crate { WHERE crate_owners.crate_id = crate_owners_with_login.crate_id AND crate_owners.owner_id = crate_owners_with_login.owner_id AND crate_owners.owner_kind = crate_owners_with_login.owner_kind - AND lower(crate_owners_with_login.login) = lower($2);"#, + AND canon_username(crate_owners_with_login.login) = canon_username($2);"#, + ); + + let num_updated_rows = query + .bind::(self.id) + .bind::(login) + .execute(&mut conn) + .await?; + + if num_updated_rows == 0 { + return Err(OwnerRemoveError::not_found(login)); + } + + Ok(()) + } + + /// Remove owner given a GitHub username. + pub async fn owner_remove_with_gh_login( + &self, + mut conn: &AsyncPgConnection, + login: &str, + ) -> Result<(), OwnerRemoveError> { + let query = diesel::sql_query( + r#"WITH crate_owners_with_gh_login AS ( + SELECT + crate_owners.*, + login + FROM crate_owners + JOIN oauth_github + ON crate_owners.owner_id = oauth_github.user_id + AND crate_owners.owner_kind = 0 + WHERE crate_owners.crate_id = $1 + AND crate_owners.deleted = false + ) + UPDATE crate_owners + SET deleted = true + FROM crate_owners_with_gh_login + WHERE crate_owners.crate_id = crate_owners_with_gh_login.crate_id + AND crate_owners.owner_id = crate_owners_with_gh_login.owner_id + AND crate_owners.owner_kind = crate_owners_with_gh_login.owner_kind + AND canon_username(crate_owners_with_gh_login.login) = canon_username($2);"#, ); let num_updated_rows = query diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index 2b7ceddaa5a..fd2a0c4fb5d 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -309,7 +309,7 @@ pub async fn remove_owners( conn.transaction(async |conn| { for login in &body.owners { - krate.owner_remove(conn, login).await?; + remove_owner(&krate, conn, login).await?; } if User::owning(&krate, conn).await?.is_empty() { return Err(bad_request( @@ -497,6 +497,29 @@ impl<'a> GitHubTeamLogin<'a> { } } +async fn remove_owner( + krate: &Crate, + conn: &mut AsyncPgConnection, + login: &str, +) -> Result<(), BoxedAppError> { + match Login::parse(login)? { + Login::GitHubTeam(login) => krate.owner_remove_with_username(conn, login.login).await?, + Login::GitHub(_) => { + return Err(bad_request( + "missing github team argument; format is github:org:team", + )); + } + Login::CratesIo(_) => { + return Err(bad_request( + "unknown organization handler, only 'github:org:team' is supported", + )); + } + Login::Unprefixed(login) => krate.owner_remove_with_gh_login(conn, login).await?, + } + + Ok(()) +} + async fn invite_user_owner( app: &App, conn: &mut AsyncPgConnection, diff --git a/src/tests/owners.rs b/src/tests/owners.rs index b3f42d86c7e..ce8563f3459 100644 --- a/src/tests/owners.rs +++ b/src/tests/owners.rs @@ -393,7 +393,10 @@ async fn deleted_ownership_isnt_in_owner_user() { let krate = CrateBuilder::new("foo_my_packages", user.id) .expect_build(&mut conn) .await; - krate.owner_remove(&conn, &user.gh_login).await.unwrap(); + krate + .owner_remove_with_username(&conn, &user.username) + .await + .unwrap(); let json: UserResponse = anon .get("/api/v1/crates/foo_my_packages/owner_user") diff --git a/src/tests/routes/crates/list.rs b/src/tests/routes/crates/list.rs index 4cca0245f79..981e60b9032 100644 --- a/src/tests/routes/crates/list.rs +++ b/src/tests/routes/crates/list.rs @@ -1372,7 +1372,10 @@ async fn crates_by_user_id_not_including_deleted_owners() -> anyhow::Result<()> let krate = CrateBuilder::new("foo_my_packages", user.id) .expect_build(&mut conn) .await; - krate.owner_remove(&conn, "foo").await.unwrap(); + krate + .owner_remove_with_username(&conn, "foo") + .await + .unwrap(); for response in search_both_by_user_id(&anon, user.id).await { assert_eq!(response.crates.len(), 0); diff --git a/src/tests/routes/crates/owners/remove.rs b/src/tests/routes/crates/owners/remove.rs index 49f5ba3dec4..b55cbe6069e 100644 --- a/src/tests/routes/crates/owners/remove.rs +++ b/src/tests/routes/crates/owners/remove.rs @@ -174,16 +174,16 @@ async fn unprefixed_github_login_case_insensitive() { #[tokio::test(flavor = "multi_thread")] async fn unprefixed_github_login_separator_variant() { let (response, owner_count) = remove_distinct_login_user("github_user").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `github_user`"}]}"#); - assert_eq!(owner_count, 2); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); + assert_eq!(owner_count, 1); } #[tokio::test(flavor = "multi_thread")] async fn crates_io_prefixed_username_verbatim() { let (response, owner_count) = remove_distinct_login_user("crates.io:crates-user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `crates.io:crates-user`"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); assert_eq!(owner_count, 2); } @@ -191,7 +191,7 @@ async fn crates_io_prefixed_username_verbatim() { async fn crates_io_prefixed_username_case_insensitive() { let (response, owner_count) = remove_distinct_login_user("crates.io:CRATES-USER").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `crates.io:CRATES-USER`"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); assert_eq!(owner_count, 2); } @@ -199,7 +199,7 @@ async fn crates_io_prefixed_username_case_insensitive() { async fn crates_io_prefixed_username_separator_variant() { let (response, owner_count) = remove_distinct_login_user("crates.io:crates_user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `crates.io:crates_user`"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); assert_eq!(owner_count, 2); } @@ -207,7 +207,7 @@ async fn crates_io_prefixed_username_separator_variant() { async fn github_prefixed_login_verbatim() { let (response, owner_count) = remove_distinct_login_user("github:github-user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `github:github-user`"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); assert_eq!(owner_count, 2); } @@ -215,7 +215,7 @@ async fn github_prefixed_login_verbatim() { async fn github_prefixed_login_case_insensitive() { let (response, owner_count) = remove_distinct_login_user("github:GITHUB-USER").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `github:GITHUB-USER`"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); assert_eq!(owner_count, 2); } @@ -223,7 +223,7 @@ async fn github_prefixed_login_case_insensitive() { async fn github_prefixed_login_separator_variant() { let (response, owner_count) = remove_distinct_login_user("github:github_user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `github:github_user`"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); assert_eq!(owner_count, 2); } @@ -231,7 +231,7 @@ async fn github_prefixed_login_separator_variant() { async fn crates_io_prefix_does_not_match_github_login() { let (response, owner_count) = remove_distinct_login_user("crates.io:github-user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `crates.io:github-user`"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); assert_eq!(owner_count, 2); } @@ -239,7 +239,7 @@ async fn crates_io_prefix_does_not_match_github_login() { async fn github_prefix_does_not_match_crates_io_username() { let (response, owner_count) = remove_distinct_login_user("github:crates-user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `github:crates-user`"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); assert_eq!(owner_count, 2); } diff --git a/src/tests/routes/me/get.rs b/src/tests/routes/me/get.rs index fc87a080833..6355cd1e84e 100644 --- a/src/tests/routes/me/get.rs +++ b/src/tests/routes/me/get.rs @@ -62,7 +62,7 @@ async fn test_user_owned_crates_doesnt_include_deleted_ownership() { .expect_build(&mut conn) .await; krate - .owner_remove(&conn, &user_model.gh_login) + .owner_remove_with_username(&conn, &user_model.username) .await .unwrap(); diff --git a/src/tests/routes/users/stats.rs b/src/tests/routes/users/stats.rs index d29bf6a4af0..c20a63f180a 100644 --- a/src/tests/routes/users/stats.rs +++ b/src/tests/routes/users/stats.rs @@ -53,7 +53,7 @@ async fn user_total_downloads() -> anyhow::Result<()> { .execute(&mut conn) .await?; no_longer_my_krate - .owner_remove(&conn, &user.gh_login) + .owner_remove_with_username(&conn, &user.username) .await .unwrap(); diff --git a/src/tests/team.rs b/src/tests/team.rs index 43e4b96b180..ff53779a820 100644 --- a/src/tests/team.rs +++ b/src/tests/team.rs @@ -595,7 +595,10 @@ async fn crates_by_team_id_not_including_deleted_owners() -> anyhow::Result<()> .expect_build(&mut conn) .await; add_team_to_crate(&t, &krate, user.id, &mut conn).await?; - krate.owner_remove(&conn, &t.login).await.unwrap(); + krate + .owner_remove_with_username(&conn, &t.login) + .await + .unwrap(); let json = anon.search(&format!("team_id={}", t.id)).await; assert_eq!(json.crates.len(), 0); From ebb148e6c40a644956a1b245c008502b01139c92 Mon Sep 17 00:00:00 2001 From: moskirathe Date: Mon, 31 Aug 2026 14:05:13 +0200 Subject: [PATCH 07/10] controllers/owners: Support crates.io-prefixed removals The remove-owner endpoint accepts `crates.io:username` arguments and removes the current owner resolved through crates.io username canonicalization. GitHub-prefixed removal arguments remain rejected until their corresponding removal support is enabled. --- packages/crates-io-api-client/schema.ts | 10 ++- src/controllers/krate/owners.rs | 11 +-- src/tests/routes/crates/owners/remove.rs | 90 ++++++++++++++++--- ..._openapi__openapi_internal_snapshot-2.snap | 4 +- ...egration__openapi__openapi_snapshot-2.snap | 4 +- 5 files changed, 93 insertions(+), 26 deletions(-) diff --git a/packages/crates-io-api-client/schema.ts b/packages/crates-io-api-client/schema.ts index a4cfa57ace4..dee3e805a4b 100644 --- a/packages/crates-io-api-client/schema.ts +++ b/packages/crates-io-api-client/schema.ts @@ -3294,8 +3294,9 @@ export interface operations { * For users, use just the username (e.g., `"octocat"`). * For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). * - * When adding an owner, use the `crates.io:username` or `github:username` - * prefix to explicitly select the username's service. + * To explicitly select a crates.io username, use the `crates.io:username` + * prefix. When adding an owner, use the `github:username` prefix to + * explicitly select a GitHub username. * @example [ * "octocat", * "github:rust-lang:owners", @@ -3364,8 +3365,9 @@ export interface operations { * For users, use just the username (e.g., `"octocat"`). * For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). * - * When adding an owner, use the `crates.io:username` or `github:username` - * prefix to explicitly select the username's service. + * To explicitly select a crates.io username, use the `crates.io:username` + * prefix. When adding an owner, use the `github:username` prefix to + * explicitly select a GitHub username. * @example [ * "octocat", * "github:rust-lang:owners", diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index fd2a0c4fb5d..2a28c093a0a 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -336,8 +336,9 @@ pub struct ChangeOwnersRequest { /// For users, use just the username (e.g., `"octocat"`). /// For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). /// - /// When adding an owner, use the `crates.io:username` or `github:username` - /// prefix to explicitly select the username's service. + /// To explicitly select a crates.io username, use the `crates.io:username` + /// prefix. When adding an owner, use the `github:username` prefix to + /// explicitly select a GitHub username. #[schema(example = json!(["octocat", "github:rust-lang:owners", "crates.io:some_user", "github:other_user"]))] #[serde(alias = "users")] owners: Vec, @@ -509,11 +510,7 @@ async fn remove_owner( "missing github team argument; format is github:org:team", )); } - Login::CratesIo(_) => { - return Err(bad_request( - "unknown organization handler, only 'github:org:team' is supported", - )); - } + Login::CratesIo(username) => krate.owner_remove_with_username(conn, username).await?, Login::Unprefixed(login) => krate.owner_remove_with_gh_login(conn, login).await?, } diff --git a/src/tests/routes/crates/owners/remove.rs b/src/tests/routes/crates/owners/remove.rs index b55cbe6069e..0d857d0b146 100644 --- a/src/tests/routes/crates/owners/remove.rs +++ b/src/tests/routes/crates/owners/remove.rs @@ -1,5 +1,5 @@ use crate::OwnerResp; -use crate::builders::{CrateBuilder, UserBuilder}; +use crate::builders::{CrateBuilder, OauthGithubBuilder, UserBuilder}; use crate::util::{RequestHelper, Response, TestApp}; use crates_io::models::CrateOwner; use crates_io_github::{GitHubOrganization, GitHubTeam, GitHubTeamMembership, MockGitHubClient}; @@ -104,6 +104,74 @@ async fn test_remove_uppercase_user() { assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); } +#[tokio::test(flavor = "multi_thread")] +async fn test_remove_ambiguous_user_with_cratesio_prefix() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let builder = UserBuilder::new() + .with_username("alice") + .with_gh_login("alice-gh"); + let cratesio_alice = app.db_new_user_from_builder(builder).await; + let builder = UserBuilder::new() + .with_username("bob") + .with_gh_login("alice"); + let github_alice = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(github_alice.as_model()) + .with_login("alice") + .insert(&conn) + .await; + + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + for user in [&cratesio_alice, &github_alice] { + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + } + + let response = cookie.remove_named_owner("foo", "crates.io:alice").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_empty_cratesio_username() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.remove_named_owner("foo", "crates.io:").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot be empty"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_disambiguated_cratesio_username_not_found() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie + .remove_named_owner("foo", "crates.io:nonexistent") + .await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `nonexistent`"}]}"#); +} + async fn remove_distinct_login_user(login: &str) -> (Response, usize) { let (app, _, cookie) = TestApp::full().with_user().await; let mut conn = app.db_conn().await; @@ -182,25 +250,25 @@ async fn unprefixed_github_login_separator_variant() { #[tokio::test(flavor = "multi_thread")] async fn crates_io_prefixed_username_verbatim() { let (response, owner_count) = remove_distinct_login_user("crates.io:crates-user").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); - assert_eq!(owner_count, 2); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); + assert_eq!(owner_count, 1); } #[tokio::test(flavor = "multi_thread")] async fn crates_io_prefixed_username_case_insensitive() { let (response, owner_count) = remove_distinct_login_user("crates.io:CRATES-USER").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); - assert_eq!(owner_count, 2); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); + assert_eq!(owner_count, 1); } #[tokio::test(flavor = "multi_thread")] async fn crates_io_prefixed_username_separator_variant() { let (response, owner_count) = remove_distinct_login_user("crates.io:crates_user").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); - assert_eq!(owner_count, 2); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); + assert_eq!(owner_count, 1); } #[tokio::test(flavor = "multi_thread")] @@ -231,7 +299,7 @@ async fn github_prefixed_login_separator_variant() { async fn crates_io_prefix_does_not_match_github_login() { let (response, owner_count) = remove_distinct_login_user("crates.io:github-user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"unknown organization handler, only 'github:org:team' is supported"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `github-user`"}]}"#); assert_eq!(owner_count, 2); } diff --git a/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap b/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap index ed5c3ca095f..46521b1c690 100644 --- a/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap +++ b/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap @@ -3640,7 +3640,7 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` or `github:username`\nprefix to explicitly select the username's service.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo explicitly select a crates.io username, use the `crates.io:username`\nprefix. When adding an owner, use the `github:username` prefix to\nexplicitly select a GitHub username.", "example": [ "octocat", "github:rust-lang:owners", @@ -3802,7 +3802,7 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` or `github:username`\nprefix to explicitly select the username's service.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo explicitly select a crates.io username, use the `crates.io:username`\nprefix. When adding an owner, use the `github:username` prefix to\nexplicitly select a GitHub username.", "example": [ "octocat", "github:rust-lang:owners", diff --git a/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap b/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap index ddc7f24ba62..3c46574e9a7 100644 --- a/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap +++ b/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap @@ -2907,7 +2907,7 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` or `github:username`\nprefix to explicitly select the username's service.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo explicitly select a crates.io username, use the `crates.io:username`\nprefix. When adding an owner, use the `github:username` prefix to\nexplicitly select a GitHub username.", "example": [ "octocat", "github:rust-lang:owners", @@ -3069,7 +3069,7 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nWhen adding an owner, use the `crates.io:username` or `github:username`\nprefix to explicitly select the username's service.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo explicitly select a crates.io username, use the `crates.io:username`\nprefix. When adding an owner, use the `github:username` prefix to\nexplicitly select a GitHub username.", "example": [ "octocat", "github:rust-lang:owners", From 11657db347fbbe1396ef4716b41931275d347575 Mon Sep 17 00:00:00 2001 From: moskirathe Date: Mon, 31 Aug 2026 14:08:55 +0200 Subject: [PATCH 08/10] controllers/owners: Support GitHub-prefixed removals The remove-owner endpoint accepts `github:username` arguments and removes the current owner resolved through the GitHub login namespace. The owner request documentation now describes both service prefixes for additions and removals. --- crates/crates_io_database/src/models/owner.rs | 12 +- packages/crates-io-api-client/schema.ts | 10 +- src/controllers/krate/owners.rs | 18 +- src/tests/routes/crates/owners/remove.rs | 161 ++++++++++++++++-- ..._openapi__openapi_internal_snapshot-2.snap | 4 +- ...egration__openapi__openapi_snapshot-2.snap | 4 +- 6 files changed, 172 insertions(+), 37 deletions(-) diff --git a/crates/crates_io_database/src/models/owner.rs b/crates/crates_io_database/src/models/owner.rs index 41ee0a85f27..44fb4abbd0c 100644 --- a/crates/crates_io_database/src/models/owner.rs +++ b/crates/crates_io_database/src/models/owner.rs @@ -102,17 +102,17 @@ impl Owner { } } - pub fn gh_login(&self) -> Option<&str> { + pub fn username(&self) -> &str { match self { - Owner::User(user) => user.gh_username.as_deref(), - Owner::Team(team) => Some(&team.login), + Owner::User(user) => &user.username, + Owner::Team(team) => &team.login, } } - pub fn username(&self) -> &str { + pub fn gh_login(&self) -> Option<&str> { match self { - Owner::User(user) => &user.username, - Owner::Team(team) => &team.login, + Owner::User(user) => user.gh_username.as_deref(), + Owner::Team(team) => Some(&team.login), } } diff --git a/packages/crates-io-api-client/schema.ts b/packages/crates-io-api-client/schema.ts index dee3e805a4b..5d8c37a57d8 100644 --- a/packages/crates-io-api-client/schema.ts +++ b/packages/crates-io-api-client/schema.ts @@ -3294,9 +3294,8 @@ export interface operations { * For users, use just the username (e.g., `"octocat"`). * For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). * - * To explicitly select a crates.io username, use the `crates.io:username` - * prefix. When adding an owner, use the `github:username` prefix to - * explicitly select a GitHub username. + * To disambiguate between crates.io and GitHub usernames, use + * the `crates.io:username` or `github:username` prefix. * @example [ * "octocat", * "github:rust-lang:owners", @@ -3365,9 +3364,8 @@ export interface operations { * For users, use just the username (e.g., `"octocat"`). * For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). * - * To explicitly select a crates.io username, use the `crates.io:username` - * prefix. When adding an owner, use the `github:username` prefix to - * explicitly select a GitHub username. + * To disambiguate between crates.io and GitHub usernames, use + * the `crates.io:username` or `github:username` prefix. * @example [ * "octocat", * "github:rust-lang:owners", diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index 2a28c093a0a..8b0bf0d95dc 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -309,7 +309,8 @@ pub async fn remove_owners( conn.transaction(async |conn| { for login in &body.owners { - remove_owner(&krate, conn, login).await?; + let parsed_login = Login::parse(login)?; + remove_owner(&krate, conn, parsed_login).await?; } if User::owning(&krate, conn).await?.is_empty() { return Err(bad_request( @@ -336,9 +337,8 @@ pub struct ChangeOwnersRequest { /// For users, use just the username (e.g., `"octocat"`). /// For GitHub teams, use the format `github:org:team` (e.g., `"github:rust-lang:owners"`). /// - /// To explicitly select a crates.io username, use the `crates.io:username` - /// prefix. When adding an owner, use the `github:username` prefix to - /// explicitly select a GitHub username. + /// To disambiguate between crates.io and GitHub usernames, use + /// the `crates.io:username` or `github:username` prefix. #[schema(example = json!(["octocat", "github:rust-lang:owners", "crates.io:some_user", "github:other_user"]))] #[serde(alias = "users")] owners: Vec, @@ -501,15 +501,11 @@ impl<'a> GitHubTeamLogin<'a> { async fn remove_owner( krate: &Crate, conn: &mut AsyncPgConnection, - login: &str, + login: Login<'_>, ) -> Result<(), BoxedAppError> { - match Login::parse(login)? { + match login { Login::GitHubTeam(login) => krate.owner_remove_with_username(conn, login.login).await?, - Login::GitHub(_) => { - return Err(bad_request( - "missing github team argument; format is github:org:team", - )); - } + Login::GitHub(username) => krate.owner_remove_with_gh_login(conn, username).await?, Login::CratesIo(username) => krate.owner_remove_with_username(conn, username).await?, Login::Unprefixed(login) => krate.owner_remove_with_gh_login(conn, login).await?, } diff --git a/src/tests/routes/crates/owners/remove.rs b/src/tests/routes/crates/owners/remove.rs index 0d857d0b146..632300cc440 100644 --- a/src/tests/routes/crates/owners/remove.rs +++ b/src/tests/routes/crates/owners/remove.rs @@ -142,6 +142,88 @@ async fn test_remove_ambiguous_user_with_cratesio_prefix() { assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); } +#[tokio::test(flavor = "multi_thread")] +async fn test_remove_ambiguous_user_with_github_prefix() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let builder = UserBuilder::new() + .with_username("alice") + .with_gh_login("alice-gh"); + let cratesio_alice = app.db_new_user_from_builder(builder).await; + let builder = UserBuilder::new() + .with_username("bob") + .with_gh_login("alice"); + let github_alice = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(github_alice.as_model()) + .with_login("alice") + .insert(&conn) + .await; + + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + for user in [&cratesio_alice, &github_alice] { + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + } + + let response = cookie.remove_named_owner("foo", "github:alice").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_team_with_extra_component() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie + .remove_named_owner("foo", "github:alice:team:extra") + .await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid argument. only github:org:team, github:username, crates.io:username and username are supported."}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_empty_org() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.remove_named_owner("foo", "github::team").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"organization cannot be empty"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_empty_team() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.remove_named_owner("foo", "github:org:").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"team cannot be empty"}]}"#); +} + #[tokio::test(flavor = "multi_thread")] async fn test_reject_empty_cratesio_username() { let (app, _, cookie) = TestApp::full().with_user().await; @@ -156,6 +238,65 @@ async fn test_reject_empty_cratesio_username() { assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot be empty"}]}"#); } +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_empty_login() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.remove_named_owner("foo", "").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot be empty"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_reject_github_username_with_invalid_char() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.remove_named_owner("foo", "github:a&lice*").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot contain special characters like &"}]}"#); +} + +/// Test that an unsupported prefix (e.g. gitlab:) returns an error. +#[tokio::test(flavor = "multi_thread")] +async fn test_unsupported_disambiguation_prefix() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.remove_named_owner("foo", "gitlab:user2").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid argument. only github:org:team, github:username, crates.io:username and username are supported."}]}"#); +} + +/// Test that removing with nonexistent github username returns an error. +#[tokio::test(flavor = "multi_thread")] +async fn test_disambiguated_github_username_not_found() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.remove_named_owner("foo", "github:nonexistent").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `nonexistent`"}]}"#); +} + +/// Test that removing with nonexistent crates.io username returns an error. #[tokio::test(flavor = "multi_thread")] async fn test_disambiguated_cratesio_username_not_found() { let (app, _, cookie) = TestApp::full().with_user().await; @@ -274,25 +415,25 @@ async fn crates_io_prefixed_username_separator_variant() { #[tokio::test(flavor = "multi_thread")] async fn github_prefixed_login_verbatim() { let (response, owner_count) = remove_distinct_login_user("github:github-user").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); - assert_eq!(owner_count, 2); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); + assert_eq!(owner_count, 1); } #[tokio::test(flavor = "multi_thread")] async fn github_prefixed_login_case_insensitive() { let (response, owner_count) = remove_distinct_login_user("github:GITHUB-USER").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); - assert_eq!(owner_count, 2); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); + assert_eq!(owner_count, 1); } #[tokio::test(flavor = "multi_thread")] async fn github_prefixed_login_separator_variant() { let (response, owner_count) = remove_distinct_login_user("github:github_user").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); - assert_eq!(owner_count, 2); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); + assert_eq!(owner_count, 1); } #[tokio::test(flavor = "multi_thread")] @@ -307,7 +448,7 @@ async fn crates_io_prefix_does_not_match_github_login() { async fn github_prefix_does_not_match_crates_io_username() { let (response, owner_count) = remove_distinct_login_user("github:crates-user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"missing github team argument; format is github:org:team"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `crates-user`"}]}"#); assert_eq!(owner_count, 2); } diff --git a/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap b/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap index 46521b1c690..45a834ec9e5 100644 --- a/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap +++ b/src/tests/snapshots/integration__openapi__openapi_internal_snapshot-2.snap @@ -3640,7 +3640,7 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo explicitly select a crates.io username, use the `crates.io:username`\nprefix. When adding an owner, use the `github:username` prefix to\nexplicitly select a GitHub username.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo disambiguate between crates.io and GitHub usernames, use\nthe `crates.io:username` or `github:username` prefix.", "example": [ "octocat", "github:rust-lang:owners", @@ -3802,7 +3802,7 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo explicitly select a crates.io username, use the `crates.io:username`\nprefix. When adding an owner, use the `github:username` prefix to\nexplicitly select a GitHub username.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo disambiguate between crates.io and GitHub usernames, use\nthe `crates.io:username` or `github:username` prefix.", "example": [ "octocat", "github:rust-lang:owners", diff --git a/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap b/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap index 3c46574e9a7..8b8727ddd14 100644 --- a/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap +++ b/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap @@ -2907,7 +2907,7 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo explicitly select a crates.io username, use the `crates.io:username`\nprefix. When adding an owner, use the `github:username` prefix to\nexplicitly select a GitHub username.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo disambiguate between crates.io and GitHub usernames, use\nthe `crates.io:username` or `github:username` prefix.", "example": [ "octocat", "github:rust-lang:owners", @@ -3069,7 +3069,7 @@ expression: response.json() "schema": { "properties": { "owners": { - "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo explicitly select a crates.io username, use the `crates.io:username`\nprefix. When adding an owner, use the `github:username` prefix to\nexplicitly select a GitHub username.", + "description": "List of owner login names to add or remove.\n\nFor users, use just the username (e.g., `\"octocat\"`).\nFor GitHub teams, use the format `github:org:team` (e.g., `\"github:rust-lang:owners\"`).\n\nTo disambiguate between crates.io and GitHub usernames, use\nthe `crates.io:username` or `github:username` prefix.", "example": [ "octocat", "github:rust-lang:owners", From 51f30450270ceaa9c363b80bab2cfbef789f5ccb Mon Sep 17 00:00:00 2001 From: moskirathe Date: Fri, 28 Aug 2026 12:56:35 +0200 Subject: [PATCH 09/10] controllers/owners: Resolve unprefixed additions as crates.io users Unprefixed additions resolve canonical crates.io usernames and require the linked GitHub login to identify the same name. Ambiguous accounts receive explicit `crates.io:` and `github:` commands for selecting the intended user. --- src/controllers/krate/owners.rs | 53 ++++++-- src/tests/routes/crates/owners/add.rs | 186 +++++++++++++++++++++++++- 2 files changed, 221 insertions(+), 18 deletions(-) diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index 8b0bf0d95dc..69c425a0a52 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -194,6 +194,7 @@ pub async fn add_owners( let mut msgs = Vec::with_capacity(logins.len()); for login in &logins { let parsed_login = Login::parse(login)?; + let owner = resolve_unprefixed_login(conn, &parsed_login).await?; let login_test = |owner: &Owner| match parsed_login { Login::GitHubTeam(_) => { canon_username(owner.username()) == canon_username(login) @@ -201,18 +202,15 @@ pub async fn add_owners( Login::GitHub(username) => owner .gh_login() .is_some_and(|login| canon_username(login) == canon_username(username)), - Login::CratesIo(username) => { + Login::CratesIo(username) | Login::Unprefixed(username) => { canon_username(owner.username()) == canon_username(username) } - Login::Unprefixed(_) => owner.gh_login().is_some_and(|owner_login| { - owner_login.to_lowercase() == login.to_lowercase() - }), }; if owners.iter().any(login_test) { return Err(bad_request(format_args!("`{login}` is already an owner"))); } - match add_owner(&app, conn, user, &krate, parsed_login).await { + match add_owner(&app, conn, user, &krate, parsed_login, owner).await { // A user was successfully invited, and they must accept // the invite, and a best-effort attempt should be made // to email them the invite token for one-click @@ -374,6 +372,41 @@ fn render_owner_invite_email( ) } +/// Checks whether an unprefixed login is ambiguous. +/// +/// Prefixed logins return `None`. A successfully resolved unprefixed login +/// returns its user. +async fn resolve_unprefixed_login( + conn: &mut AsyncPgConnection, + login: &Login<'_>, +) -> Result, BoxedAppError> { + let Login::Unprefixed(username) = login else { + return Ok(None); + }; + + let Some(user) = find_user_by_username(conn, username).await.optional()? else { + return Err(bad_request(format_args!( + "could not find user with login `{username}`" + ))); + }; + + if let Some(gh_login) = &user.gh_username + && canon_username(gh_login) != canon_username(&user.username) + { + let error = format_args!( + "username `{username}` is possibly ambiguous. The crates.io account `{username}` is associated with GitHub user `{gh_login}`.\n\n\ + To confirm this is the account you want to add, please run one of the following:\n\n\ + $ cargo owner --add crates.io:{username}\n\ + $ cargo owner --add github:{gh_login}\n\n\ + If this is not the account you want to add, verify the crates.io username of the account you want.", + ); + + return Err(bad_request(error)); + } + + Ok(Some(user)) +} + /// Invites `login` as an owner of this crate, returning the created /// [`NewOwnerInvite`]. async fn add_owner( @@ -382,6 +415,7 @@ async fn add_owner( req_user: &User, krate: &Crate, login: Login<'_>, + owner: Option, ) -> Result { match login { Login::GitHubTeam(team) => { @@ -413,12 +447,9 @@ async fn add_owner( invite_user_owner(app, conn, req_user, user, username, krate).await } Login::Unprefixed(username) => { - let user = User::find_by_login(conn, username) - .await - .optional()? - .ok_or_else(|| { - bad_request(format_args!("could not find user with login `{username}`")) - })?; + let user = owner.ok_or_else(|| { + bad_request(format_args!("could not find user with login `{username}`")) + })?; invite_user_owner(app, conn, req_user, user, username, krate).await } } diff --git a/src/tests/routes/crates/owners/add.rs b/src/tests/routes/crates/owners/add.rs index cd30af6bea4..ae3eab40d63 100644 --- a/src/tests/routes/crates/owners/add.rs +++ b/src/tests/routes/crates/owners/add.rs @@ -74,35 +74,35 @@ async fn invite_distinct_login_user(login: &str) -> Response { async fn unprefixed_crates_io_username_verbatim() { let response = invite_distinct_login_user("crates-user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with login `crates-user`"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username `crates-user` is possibly ambiguous. The crates.io account `crates-user` is associated with GitHub user `github-user`.\n\nTo confirm this is the account you want to add, please run one of the following:\n\n$ cargo owner --add crates.io:crates-user\n$ cargo owner --add github:github-user\n\nIf this is not the account you want to add, verify the crates.io username of the account you want."}]}"#); } #[tokio::test(flavor = "multi_thread")] async fn unprefixed_crates_io_username_case_insensitive() { let response = invite_distinct_login_user("CRATES-USER").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with login `CRATES-USER`"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username `CRATES-USER` is possibly ambiguous. The crates.io account `CRATES-USER` is associated with GitHub user `github-user`.\n\nTo confirm this is the account you want to add, please run one of the following:\n\n$ cargo owner --add crates.io:CRATES-USER\n$ cargo owner --add github:github-user\n\nIf this is not the account you want to add, verify the crates.io username of the account you want."}]}"#); } #[tokio::test(flavor = "multi_thread")] async fn unprefixed_crates_io_username_separator_variant() { let response = invite_distinct_login_user("crates_user").await; assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with login `crates_user`"}]}"#); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username `crates_user` is possibly ambiguous. The crates.io account `crates_user` is associated with GitHub user `github-user`.\n\nTo confirm this is the account you want to add, please run one of the following:\n\n$ cargo owner --add crates.io:crates_user\n$ cargo owner --add github:github-user\n\nIf this is not the account you want to add, verify the crates.io username of the account you want."}]}"#); } #[tokio::test(flavor = "multi_thread")] async fn unprefixed_github_login_verbatim() { let response = invite_distinct_login_user("github-user").await; - assert_snapshot!(response.status(), @"200 OK"); - assert_snapshot!(response.text(), @r#"{"msg":"user github-user has been invited to be an owner of crate foo","ok":true}"#); + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with login `github-user`"}]}"#); } #[tokio::test(flavor = "multi_thread")] async fn unprefixed_github_login_case_insensitive() { let response = invite_distinct_login_user("GITHUB-USER").await; - assert_snapshot!(response.status(), @"200 OK"); - assert_snapshot!(response.text(), @r#"{"msg":"user GITHUB-USER has been invited to be an owner of crate foo","ok":true}"#); + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with login `GITHUB-USER`"}]}"#); } #[tokio::test(flavor = "multi_thread")] @@ -777,6 +777,178 @@ async fn test_reject_github_username_with_invalid_char() { assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username cannot contain special characters like &"}]}"#); } +#[tokio::test(flavor = "multi_thread")] +async fn test_ambiguous_username_error() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let builder = UserBuilder::new() + .with_username("user2") + .with_gh_login("user2-gh"); + let new_user = app.db_new_user_from_builder(builder).await; + + OauthGithubBuilder::for_user(new_user.as_model()) + .with_login("user2-gh") + .insert(&conn) + .await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "user2").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username `user2` is possibly ambiguous. The crates.io account `user2` is associated with GitHub user `user2-gh`.\n\nTo confirm this is the account you want to add, please run one of the following:\n\n$ cargo owner --add crates.io:user2\n$ cargo owner --add github:user2-gh\n\nIf this is not the account you want to add, verify the crates.io username of the account you want."}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_separator_variant_gh_login_is_not_ambiguous() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let builder = UserBuilder::new() + .with_username("user-2") + .with_gh_login("user_2"); + let new_user = app.db_new_user_from_builder(builder).await; + + OauthGithubBuilder::for_user(new_user.as_model()) + .with_login("user_2") + .insert(&conn) + .await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "user-2").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"user user-2 has been invited to be an owner of crate foo","ok":true}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_add_separator_variant_unprefixed_login() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + app.db_new_user("user-2").await; + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "user_2").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"user user_2 has been invited to be an owner of crate foo","ok":true}"#); +} + +/// Test that ambiguity is resolved before comparing against existing owners +#[tokio::test(flavor = "multi_thread")] +async fn test_shared_login_is_ambiguous_even_when_one_account_is_already_an_owner() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let builder = UserBuilder::new() + .with_username("alice") + .with_gh_login("alice-gh"); + let cratesio_alice = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(cratesio_alice.as_model()) + .with_login("alice-gh") + .insert(&conn) + .await; + + let builder = UserBuilder::new() + .with_username("bob") + .with_gh_login("alice"); + let github_alice = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(github_alice.as_model()) + .with_login("alice") + .insert(&conn) + .await; + + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + // The crates.io `alice` is already an owner, the GitHub `alice` is not. + CrateOwner::builder() + .crate_id(krate.id) + .user_id(cratesio_alice.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + + let response = cookie.add_named_owner("foo", "alice").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username `alice` is possibly ambiguous. The crates.io account `alice` is associated with GitHub user `alice-gh`.\n\nTo confirm this is the account you want to add, please run one of the following:\n\n$ cargo owner --add crates.io:alice\n$ cargo owner --add github:alice-gh\n\nIf this is not the account you want to add, verify the crates.io username of the account you want."}]}"#); + + let response = cookie.add_named_owner("foo", "crates.io:alice").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"`crates.io:alice` is already an owner"}]}"#); + + // …while disambiguating to the GitHub `alice` invites the other account. + let response = cookie.add_named_owner("foo", "github:alice").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"user alice has been invited to be an owner of crate foo","ok":true}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_already_owner_error() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + // The cookie user is already the owner of the crate + let response = cookie + .add_named_owner("foo", &cookie.as_model().gh_login) + .await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"`foo` is already an owner"}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_add_mixed_case_unprefixed_login() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + app.db_new_user("user2").await; + CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + let response = cookie.add_named_owner("foo", "USer2").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"user USer2 has been invited to be an owner of crate foo","ok":true}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_already_owner_unprefixed() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let user2 = app.db_new_user("user2").await; + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + // add existing owner + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user2.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + + let response = cookie.add_named_owner("foo", "user2").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"`user2` is already an owner"}]}"#); +} + #[tokio::test(flavor = "multi_thread")] async fn test_reject_org_with_invalid_char() { let (app, _, cookie) = TestApp::full().with_user().await; From 6836aea1905b6270266308698026de029a4233d1 Mon Sep 17 00:00:00 2001 From: moskirathe Date: Fri, 28 Aug 2026 12:58:52 +0200 Subject: [PATCH 10/10] controllers/owners: Resolve unprefixed removals across services Unprefixed removals compare the crate's current owners by canonical crates.io username and linked GitHub login. A sole match is removed, while distinct owners with the same supplied name produce an ambiguity error. --- crates/crates_io_database/src/models/krate.rs | 2 +- src/controllers/krate/owners.rs | 153 +++++----- src/tests/routes/crates/owners/remove.rs | 265 +++++++++++++++++- 3 files changed, 346 insertions(+), 74 deletions(-) diff --git a/crates/crates_io_database/src/models/krate.rs b/crates/crates_io_database/src/models/krate.rs index 8a6dcb00355..ed6fa2bb149 100644 --- a/crates/crates_io_database/src/models/krate.rs +++ b/crates/crates_io_database/src/models/krate.rs @@ -260,7 +260,7 @@ impl Crate { Ok(()) } - /// Remove owner given a GitHub username. + /// Remove owner given a github username. pub async fn owner_remove_with_gh_login( &self, mut conn: &AsyncPgConnection, diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index 69c425a0a52..ba0606760d0 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -195,17 +195,21 @@ pub async fn add_owners( for login in &logins { let parsed_login = Login::parse(login)?; let owner = resolve_unprefixed_login(conn, &parsed_login).await?; - let login_test = |owner: &Owner| match parsed_login { - Login::GitHubTeam(_) => { - canon_username(owner.username()) == canon_username(login) - } - Login::GitHub(username) => owner - .gh_login() - .is_some_and(|login| canon_username(login) == canon_username(username)), - Login::CratesIo(username) | Login::Unprefixed(username) => { - canon_username(owner.username()) == canon_username(username) + + let login_test = |owner: &Owner| -> bool { + match parsed_login { + Login::GitHubTeam(_) => { + canon_username(owner.username()) == canon_username(login) + } + Login::GitHub(username) => owner + .gh_login() + .is_some_and(|u| canon_username(u) == canon_username(username)), + Login::CratesIo(u) | Login::Unprefixed(u) => { + canon_username(owner.username()) == canon_username(u) + } } }; + if owners.iter().any(login_test) { return Err(bad_request(format_args!("`{login}` is already an owner"))); } @@ -308,7 +312,7 @@ pub async fn remove_owners( conn.transaction(async |conn| { for login in &body.owners { let parsed_login = Login::parse(login)?; - remove_owner(&krate, conn, parsed_login).await?; + remove_owner(&krate, conn, parsed_login, &owners).await? } if User::owning(&krate, conn).await?.is_empty() { return Err(bad_request( @@ -372,10 +376,9 @@ fn render_owner_invite_email( ) } -/// Checks whether an unprefixed login is ambiguous. +/// Check if an unprefixed login is ambiguous. /// -/// Prefixed logins return `None`. A successfully resolved unprefixed login -/// returns its user. +/// Returns `Ok(None)` for prefixed logins, and Ok(user) for a resolved unprefixed login. async fn resolve_unprefixed_login( conn: &mut AsyncPgConnection, login: &Login<'_>, @@ -407,8 +410,18 @@ async fn resolve_unprefixed_login( Ok(Some(user)) } +async fn find_user_by_username(conn: &mut AsyncPgConnection, username: &str) -> QueryResult { + users_by_username(username) + .left_join(oauth_github::table) + .select(User::as_select()) + .first(conn) + .await +} + /// Invites `login` as an owner of this crate, returning the created /// [`NewOwnerInvite`]. +/// +/// `owner` is the resolved login if the supplied login was unprefixed. passing it here to avoid a duplicate `find_by_username()` request to the database. async fn add_owner( app: &App, conn: &mut AsyncPgConnection, @@ -455,31 +468,80 @@ async fn add_owner( } } -/// Parsed owner login used by the owner endpoints. +async fn remove_owner( + krate: &Crate, + conn: &mut AsyncPgConnection, + login: Login<'_>, + owners: &[Owner], +) -> Result<(), BoxedAppError> { + match login { + Login::GitHubTeam(login) => krate.owner_remove_with_username(conn, login.login).await?, + Login::GitHub(username) => krate.owner_remove_with_gh_login(conn, username).await?, + Login::CratesIo(username) => krate.owner_remove_with_username(conn, username).await?, + Login::Unprefixed(username) => { + let cratesio_owner_to_remove = owners + .iter() + .find(|o| canon_username(o.username()) == canon_username(username)); + let github_owner_to_remove = owners.iter().find(|o| { + o.gh_login() + .is_some_and(|u| canon_username(u) == canon_username(username)) + }); + + // check if ambiguous. assumes usernames are unique on separate services. + if let Some(cratesio_owner) = cratesio_owner_to_remove + && let Some(github_owner) = github_owner_to_remove + && cratesio_owner.id() != github_owner.id() + { + let error = format_args!( + "username `{username}` is ambiguous. There are two owners of this crate with the username `{username}` on different services.\n\n\ + To confirm which owner you want to remove, please run one of the following:\n\n\ + $ cargo owner --remove crates.io:{username}\n\ + $ cargo owner --remove github:{username}\n\n\ + If this is not the account you want to remove, verify the crates.io username of the account you want.", + ); + + return Err(bad_request(error)); + } + + if cratesio_owner_to_remove.is_some() { + krate.owner_remove_with_username(conn, username).await? + } else if github_owner_to_remove.is_some() { + krate.owner_remove_with_gh_login(conn, username).await? + } else { + return Err(OwnerRemoveError::not_found(username).into()); + } + } + }; + Ok(()) +} + +/// Parsed login string representation enum Login<'a> { - /// GitHub organization team, such as `github:rust-lang:owners`. + /// GitHub organization team (e.g `github:org:team`). the original login is preserved as a convenience to avoid rebuilding it. GitHubTeam(GitHubTeamLogin<'a>), - /// GitHub user, such as `github:octocat`. + /// GitHub user (e.g. `github:username`). GitHub(&'a str), - /// crates.io user, such as `crates.io:octocat`. + /// crates.io user (`crates.io:username`). CratesIo(&'a str), - /// User login without a service prefix. + /// Unprefixed username (`username` without any prefix) Unprefixed(&'a str), } impl<'a> Login<'a> { /// Parses an owner login. fn parse(login: &'a str) -> Result { - fn is_valid(value: &str, label: &str) -> Result { - if value.is_empty() { + // sanitization + fn is_valid(s: &str, label: &str) -> Result { + if s.is_empty() { return Err(bad_request(format_args!("{label} cannot be empty"))); } - if let Some(character) = value.chars().find( - |character| !matches!(character, 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_'), - ) { + if let Some(c) = s + .chars() + .find(|c| !matches!(c, 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_')) + { return Err(bad_request(format_args!( - "{label} cannot contain special characters like {character}" + "{label} cannot contain special characters like {c}" ))); } @@ -529,21 +591,6 @@ impl<'a> GitHubTeamLogin<'a> { } } -async fn remove_owner( - krate: &Crate, - conn: &mut AsyncPgConnection, - login: Login<'_>, -) -> Result<(), BoxedAppError> { - match login { - Login::GitHubTeam(login) => krate.owner_remove_with_username(conn, login.login).await?, - Login::GitHub(username) => krate.owner_remove_with_gh_login(conn, username).await?, - Login::CratesIo(username) => krate.owner_remove_with_username(conn, username).await?, - Login::Unprefixed(login) => krate.owner_remove_with_gh_login(conn, login).await?, - } - - Ok(()) -} - async fn invite_user_owner( app: &App, conn: &mut AsyncPgConnection, @@ -571,14 +618,7 @@ async fn invite_user_owner( } } -async fn find_user_by_username(conn: &mut AsyncPgConnection, username: &str) -> QueryResult { - users_by_username(username) - .left_join(oauth_github::table) - .select(User::as_select()) - .first(conn) - .await -} - +/// Adds a parsed GitHub team as a crate owner. async fn add_github_team_owner( gh_client: &dyn GitHubClient, conn: &mut AsyncPgConnection, @@ -591,7 +631,7 @@ async fn add_github_team_owner( let team = create_or_update_github_team( gh_client, conn, - &login.login.to_lowercase(), + login.login, login.org, login.team, req_user, @@ -613,7 +653,7 @@ async fn add_github_team_owner( } /// Tries to create or update a GitHub Team. Assumes `org` and `team` are -/// correctly parsed out of the full `name`. `name` is passed as a +/// correctly parsed out of the full `login`. `login` is passed as a /// convenience to avoid rebuilding it. pub async fn create_or_update_github_team( gh_client: &dyn GitHubClient, @@ -624,21 +664,6 @@ pub async fn create_or_update_github_team( req_user: &User, encryption: &TokenEncryption, ) -> AppResult { - // GET orgs/:org/teams - // check that `team` is the `slug` in results, and grab its data - - // "sanitization" - fn is_allowed_char(c: char) -> bool { - matches!(c, 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_') - } - - if let Some(c) = org_name.chars().find(|c| !is_allowed_char(*c)) { - return Err(bad_request(format_args!( - "organization cannot contain special \ - characters like {c}" - ))); - } - let Some(token) = req_user.gh_encrypted_token.as_ref() else { return Err(bad_request( "Cannot add a GitHub team as an owner without a connected GitHub account", diff --git a/src/tests/routes/crates/owners/remove.rs b/src/tests/routes/crates/owners/remove.rs index 632300cc440..b32ab9ae110 100644 --- a/src/tests/routes/crates/owners/remove.rs +++ b/src/tests/routes/crates/owners/remove.rs @@ -104,6 +104,253 @@ async fn test_remove_uppercase_user() { assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); } +#[tokio::test(flavor = "multi_thread")] +async fn test_remove_ambiguous_user() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let builder = UserBuilder::new() + .with_username("alice") + .with_gh_login("alice-gh"); + let cratesio_alice = app.db_new_user_from_builder(builder).await; + let builder = UserBuilder::new() + .with_username("bob") + .with_gh_login("alice"); + let github_alice = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(github_alice.as_model()) + .with_login("alice") + .insert(&conn) + .await; + + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + for user in [&cratesio_alice, &github_alice] { + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + } + + let response = cookie.remove_named_owner("foo", "alice").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username `alice` is ambiguous. There are two owners of this crate with the username `alice` on different services.\n\nTo confirm which owner you want to remove, please run one of the following:\n\n$ cargo owner --remove crates.io:alice\n$ cargo owner --remove github:alice\n\nIf this is not the account you want to remove, verify the crates.io username of the account you want."}]}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_remove_ambiguous_user_differing_only_by_separator() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let builder = UserBuilder::new() + .with_username("alice-2") + .with_gh_login("alice-2-gh"); + let cratesio_alice = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(cratesio_alice.as_model()) + .with_login("alice-2-gh") + .insert(&conn) + .await; + + let builder = UserBuilder::new() + .with_username("bob") + .with_gh_login("alice_2"); + let github_alice = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(github_alice.as_model()) + .with_login("alice_2") + .insert(&conn) + .await; + + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + for user in [&cratesio_alice, &github_alice] { + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + } + + let response = cookie.remove_named_owner("foo", "alice-2").await; + assert_snapshot!(response.status(), @"400 Bad Request"); + assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"username `alice-2` is ambiguous. There are two owners of this crate with the username `alice-2` on different services.\n\nTo confirm which owner you want to remove, please run one of the following:\n\n$ cargo owner --remove crates.io:alice-2\n$ cargo owner --remove github:alice-2\n\nIf this is not the account you want to remove, verify the crates.io username of the account you want."}]}"#); + + // The suggested commands name the owners as they are stored, so both work. + let response = cookie.remove_named_owner("foo", "github:alice_2").await; + assert_snapshot!(response.status(), @"200 OK"); + + let response = cookie.remove_named_owner("foo", "crates.io:alice-2").await; + assert_snapshot!(response.status(), @"200 OK"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_remove_shared_login_when_only_cratesio_user_is_owner() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let builder = UserBuilder::new() + .with_username("alice") + .with_gh_login("alice-gh"); + let cratesio_alice = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(cratesio_alice.as_model()) + .with_login("alice-gh") + .insert(&conn) + .await; + + let builder = UserBuilder::new() + .with_username("bob") + .with_gh_login("alice"); + let github_alice = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(github_alice.as_model()) + .with_login("alice") + .insert(&conn) + .await; + + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + CrateOwner::builder() + .crate_id(krate.id) + .user_id(cratesio_alice.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + + let response = cookie.remove_named_owner("foo", "alice").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_remove_shared_login_when_only_github_user_is_owner() { + let (app, _, cookie) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let builder = UserBuilder::new() + .with_username("alice") + .with_gh_login("alice-gh"); + let cratesio_alice = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(cratesio_alice.as_model()) + .with_login("alice-gh") + .insert(&conn) + .await; + + let builder = UserBuilder::new() + .with_username("bob") + .with_gh_login("alice"); + let github_alice = app.db_new_user_from_builder(builder).await; + OauthGithubBuilder::for_user(github_alice.as_model()) + .with_login("alice") + .insert(&conn) + .await; + + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + + CrateOwner::builder() + .crate_id(krate.id) + .user_id(github_alice.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + + let response = cookie.remove_named_owner("foo", "alice").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_remove_unprefixed_non_ambiguous() { + let (app, _, cookie) = TestApp::full().with_user().await; + let user2 = app.db_new_user("user2").await; + let mut conn = app.db_conn().await; + + OauthGithubBuilder::for_user(user2.as_model()) + .with_login("user2") + .insert(&conn) + .await; + + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user2.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + + let response = cookie.remove_named_owner("foo", "user2").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_remove_unprefixed_username_only() { + let (app, _, cookie) = TestApp::full().with_user().await; + let builder = UserBuilder::new() + .with_username("user2") + .with_gh_login("user2-gh"); + let user2 = app.db_new_user_from_builder(builder).await; + let mut conn = app.db_conn().await; + + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user2.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + + let response = cookie.remove_named_owner("foo", "user2").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_remove_separator_variant_unprefixed() { + let (app, _, cookie) = TestApp::full().with_user().await; + let user2 = app.db_new_user("user-2").await; + let mut conn = app.db_conn().await; + + let krate = CrateBuilder::new("foo", cookie.as_model().id) + .expect_build(&mut conn) + .await; + CrateOwner::builder() + .crate_id(krate.id) + .user_id(user2.as_model().id) + .created_by(cookie.as_model().id) + .build() + .insert(&conn) + .await + .unwrap(); + + let response = cookie.remove_named_owner("foo", "user_2").await; + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); +} + #[tokio::test(flavor = "multi_thread")] async fn test_remove_ambiguous_user_with_cratesio_prefix() { let (app, _, cookie) = TestApp::full().with_user().await; @@ -343,25 +590,25 @@ async fn remove_distinct_login_user(login: &str) -> (Response, usize) #[tokio::test(flavor = "multi_thread")] async fn unprefixed_crates_io_username_verbatim() { let (response, owner_count) = remove_distinct_login_user("crates-user").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `crates-user`"}]}"#); - assert_eq!(owner_count, 2); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); + assert_eq!(owner_count, 1); } #[tokio::test(flavor = "multi_thread")] async fn unprefixed_crates_io_username_case_insensitive() { let (response, owner_count) = remove_distinct_login_user("CRATES-USER").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `CRATES-USER`"}]}"#); - assert_eq!(owner_count, 2); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); + assert_eq!(owner_count, 1); } #[tokio::test(flavor = "multi_thread")] async fn unprefixed_crates_io_username_separator_variant() { let (response, owner_count) = remove_distinct_login_user("crates_user").await; - assert_snapshot!(response.status(), @"400 Bad Request"); - assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find owner with login `crates_user`"}]}"#); - assert_eq!(owner_count, 2); + assert_snapshot!(response.status(), @"200 OK"); + assert_snapshot!(response.text(), @r#"{"msg":"owners successfully removed","ok":true}"#); + assert_eq!(owner_count, 1); } #[tokio::test(flavor = "multi_thread")]