From 91d39bc742c9b30684a51214bc847072bb1b1c9c Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 9 Apr 2023 09:11:22 -0700 Subject: [PATCH 1/4] Share the check_token function between login and logout tests. --- tests/testsuite/login.rs | 28 +++++++++++++++++----------- tests/testsuite/logout.rs | 34 ++++++---------------------------- 2 files changed, 23 insertions(+), 39 deletions(-) diff --git a/tests/testsuite/login.rs b/tests/testsuite/login.rs index 19387aed57c..5cb1a8077af 100644 --- a/tests/testsuite/login.rs +++ b/tests/testsuite/login.rs @@ -27,14 +27,15 @@ fn setup_new_credentials_at(config: PathBuf) { )); } -fn check_token(expected_token: &str, registry: Option<&str>) -> bool { +/// Asserts whether or not the token is set to the given value for the given registry. +pub fn check_token(expected_token: Option<&str>, registry: Option<&str>) { let credentials = credentials_toml(); assert!(credentials.is_file()); let contents = fs::read_to_string(&credentials).unwrap(); let toml: toml::Table = contents.parse().unwrap(); - let token = match registry { + let actual_token = match registry { // A registry has been provided, so check that the token exists in a // table for the registry. Some(registry) => toml @@ -54,10 +55,15 @@ fn check_token(expected_token: &str, registry: Option<&str>) -> bool { }), }; - if let Some(token_val) = token { - token_val == expected_token - } else { - false + match (actual_token, expected_token) { + (None, None) => {} + (Some(actual), Some(expected)) => assert_eq!(actual, expected), + (None, Some(expected)) => { + panic!("expected `{registry:?}` to be `{expected}`, but was not set") + } + (Some(actual), None) => { + panic!("expected `{registry:?}` to be unset, but was set to `{actual}`") + } } } @@ -75,10 +81,10 @@ fn registry_credentials() { cargo_process("login --registry").arg(reg).arg(TOKEN).run(); // Ensure that we have not updated the default token - assert!(check_token(ORIGINAL_TOKEN, None)); + check_token(Some(ORIGINAL_TOKEN), None); // Also ensure that we get the new token for the registry - assert!(check_token(TOKEN, Some(reg))); + check_token(Some(TOKEN), Some(reg)); let reg2 = "alternative2"; cargo_process("login --registry") @@ -88,9 +94,9 @@ fn registry_credentials() { // Ensure not overwriting 1st alternate registry token with // 2nd alternate registry token (see rust-lang/cargo#7701). - assert!(check_token(ORIGINAL_TOKEN, None)); - assert!(check_token(TOKEN, Some(reg))); - assert!(check_token(TOKEN2, Some(reg2))); + check_token(Some(ORIGINAL_TOKEN), None); + check_token(Some(TOKEN), Some(reg)); + check_token(Some(TOKEN2), Some(reg2)); } #[cargo_test] diff --git a/tests/testsuite/logout.rs b/tests/testsuite/logout.rs index 78a9429c4e7..caefb04ecf3 100644 --- a/tests/testsuite/logout.rs +++ b/tests/testsuite/logout.rs @@ -1,9 +1,8 @@ //! Tests for the `cargo logout` command. -use cargo_test_support::install::cargo_home; +use super::login::check_token; use cargo_test_support::registry::TestRegistry; use cargo_test_support::{cargo_process, registry}; -use std::fs; #[cargo_test] fn gated() { @@ -21,32 +20,9 @@ the `cargo logout` command. .run(); } -/// Checks whether or not the token is set for the given token. -fn check_config_token(registry: Option<&str>, should_be_set: bool) { - let credentials = cargo_home().join("credentials.toml"); - let contents = fs::read_to_string(&credentials).unwrap(); - let toml: toml::Table = contents.parse().unwrap(); - if let Some(registry) = registry { - assert_eq!( - toml.get("registries") - .and_then(|registries| registries.get(registry)) - .and_then(|registry| registry.get("token")) - .is_some(), - should_be_set - ); - } else { - assert_eq!( - toml.get("registry") - .and_then(|registry| registry.get("token")) - .is_some(), - should_be_set - ); - } -} - fn simple_logout_test(registry: &TestRegistry, reg: Option<&str>, flag: &str, note: &str) { let msg = reg.unwrap_or("crates-io"); - check_config_token(reg, true); + check_token(Some(registry.token()), reg); let mut cargo = cargo_process(&format!("logout -Z unstable-options {}", flag)); if reg.is_none() { cargo.replace_crates_io(registry.index_url()); @@ -61,7 +37,7 @@ If you need to revoke the token, visit {note} and follow the instructions there. " )) .run(); - check_config_token(reg, false); + check_token(None, reg); let mut cargo = cargo_process(&format!("logout -Z unstable-options {}", flag)); if reg.is_none() { @@ -71,7 +47,7 @@ If you need to revoke the token, visit {note} and follow the instructions there. .masquerade_as_nightly_cargo(&["cargo-logout"]) .with_stderr(&format!("[LOGOUT] not currently logged in to `{msg}`")) .run(); - check_config_token(reg, false); + check_token(None, reg); } #[cargo_test] @@ -89,4 +65,6 @@ fn other_registry() { "--registry alternative", "the `alternative` website", ); + // It should not touch crates.io. + check_token(Some("sekrit"), None); } From 0e13f667c8b8fccd86756e19e403bcea07bd297d Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 9 Apr 2023 09:22:28 -0700 Subject: [PATCH 2/4] Add tests for registry.default for login/logout --- tests/testsuite/login.rs | 32 +++++++++++++++++++++++ tests/testsuite/logout.rs | 54 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 1 deletion(-) diff --git a/tests/testsuite/login.rs b/tests/testsuite/login.rs index 5cb1a8077af..ef951df95af 100644 --- a/tests/testsuite/login.rs +++ b/tests/testsuite/login.rs @@ -368,3 +368,35 @@ fn login_with_generate_asymmetric_token() { let credentials = fs::read_to_string(&credentials).unwrap(); assert!(credentials.contains("secret-key = \"k3.secret.")); } + +#[cargo_test] +fn default_registry_configured() { + // When registry.default is set, login should use that one when + // --registry is not used. + let cargo_home = paths::home().join(".cargo"); + cargo_home.mkdir_p(); + cargo_util::paths::write( + &cargo_home.join("config.toml"), + r#" + [registry] + default = "dummy-registry" + + [registries.dummy-registry] + index = "https://127.0.0.1/index" + "#, + ) + .unwrap(); + + cargo_process("login") + .arg("a-new-token") + .with_stderr( + "\ +[UPDATING] crates.io index +[LOGIN] token for `crates.io` saved +", + ) + .run(); + + check_token(Some("a-new-token"), None); + check_token(None, Some("alternative")); +} diff --git a/tests/testsuite/logout.rs b/tests/testsuite/logout.rs index caefb04ecf3..136e12e27cb 100644 --- a/tests/testsuite/logout.rs +++ b/tests/testsuite/logout.rs @@ -1,6 +1,7 @@ //! Tests for the `cargo logout` command. use super::login::check_token; +use cargo_test_support::paths::{self, CargoPathExt}; use cargo_test_support::registry::TestRegistry; use cargo_test_support::{cargo_process, registry}; @@ -51,7 +52,7 @@ If you need to revoke the token, visit {note} and follow the instructions there. } #[cargo_test] -fn default_registry() { +fn default_registry_unconfigured() { let registry = registry::init(); simple_logout_test(®istry, None, "", ""); } @@ -68,3 +69,54 @@ fn other_registry() { // It should not touch crates.io. check_token(Some("sekrit"), None); } + +#[cargo_test] +fn default_registry_configured() { + // When registry.default is set, logout should use that one when + // --registry is not used. + let cargo_home = paths::home().join(".cargo"); + cargo_home.mkdir_p(); + cargo_util::paths::write( + &cargo_home.join("config.toml"), + r#" + [registry] + default = "dummy-registry" + + [registries.dummy-registry] + index = "https://127.0.0.1/index" + "#, + ) + .unwrap(); + cargo_util::paths::write( + &cargo_home.join("credentials.toml"), + r#" + [registry] + token = "crates-io-token" + + [registries.dummy-registry] + token = "dummy-token" + "#, + ) + .unwrap(); + check_token(Some("dummy-token"), Some("dummy-registry")); + check_token(Some("crates-io-token"), None); + + cargo_process("logout -Zunstable-options") + .masquerade_as_nightly_cargo(&["cargo-logout"]) + .with_stderr( + "\ +[LOGOUT] token for `crates-io` has been removed from local storage +[NOTE] This does not revoke the token on the registry server. + If you need to revoke the token, visit \ + and follow the instructions there. +", + ) + .run(); + check_token(Some("dummy-token"), Some("dummy-registry")); + check_token(None, None); + + cargo_process("logout -Zunstable-options") + .masquerade_as_nightly_cargo(&["cargo-logout"]) + .with_stderr("[LOGOUT] not currently logged in to `crates-io`") + .run(); +} From 117eb4b33ff5cdd51586bf936f070d943db2efec Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 9 Apr 2023 09:30:36 -0700 Subject: [PATCH 3/4] Use registry.default for login/logout --- src/bin/cargo/commands/login.rs | 3 ++- src/bin/cargo/commands/logout.rs | 6 ++---- tests/testsuite/login.rs | 21 +++++++++------------ tests/testsuite/logout.rs | 10 +++++----- 4 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/bin/cargo/commands/login.rs b/src/bin/cargo/commands/login.rs index dac0457d9e0..1c8d3ae4cff 100644 --- a/src/bin/cargo/commands/login.rs +++ b/src/bin/cargo/commands/login.rs @@ -34,10 +34,11 @@ pub fn cli() -> Command { } pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { + let registry = args.registry(config)?; ops::registry_login( config, args.get_one::("token").map(|s| s.as_str().into()), - args.get_one("registry").map(String::as_str), + registry.as_deref(), args.flag("generate-keypair"), args.flag("secret-key"), args.get_one("key-subject").map(String::as_str), diff --git a/src/bin/cargo/commands/logout.rs b/src/bin/cargo/commands/logout.rs index bc16ee55ecf..f1dc49cba31 100644 --- a/src/bin/cargo/commands/logout.rs +++ b/src/bin/cargo/commands/logout.rs @@ -15,9 +15,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult { .cli_unstable() .fail_if_stable_command(config, "logout", 8933)?; } - ops::registry_logout( - config, - args.get_one::("registry").map(String::as_str), - )?; + let registry = args.registry(config)?; + ops::registry_logout(config, registry.as_deref())?; Ok(()) } diff --git a/tests/testsuite/login.rs b/tests/testsuite/login.rs index ef951df95af..022ec187862 100644 --- a/tests/testsuite/login.rs +++ b/tests/testsuite/login.rs @@ -373,16 +373,13 @@ fn login_with_generate_asymmetric_token() { fn default_registry_configured() { // When registry.default is set, login should use that one when // --registry is not used. + let _alternative = RegistryBuilder::new().alternative().build(); let cargo_home = paths::home().join(".cargo"); - cargo_home.mkdir_p(); - cargo_util::paths::write( - &cargo_home.join("config.toml"), - r#" + cargo_util::paths::append( + &cargo_home.join("config"), + br#" [registry] - default = "dummy-registry" - - [registries.dummy-registry] - index = "https://127.0.0.1/index" + default = "alternative" "#, ) .unwrap(); @@ -391,12 +388,12 @@ fn default_registry_configured() { .arg("a-new-token") .with_stderr( "\ -[UPDATING] crates.io index -[LOGIN] token for `crates.io` saved +[UPDATING] `alternative` index +[LOGIN] token for `alternative` saved ", ) .run(); - check_token(Some("a-new-token"), None); - check_token(None, Some("alternative")); + check_token(None, None); + check_token(Some("a-new-token"), Some("alternative")); } diff --git a/tests/testsuite/logout.rs b/tests/testsuite/logout.rs index 136e12e27cb..19f2cd9df56 100644 --- a/tests/testsuite/logout.rs +++ b/tests/testsuite/logout.rs @@ -105,18 +105,18 @@ fn default_registry_configured() { .masquerade_as_nightly_cargo(&["cargo-logout"]) .with_stderr( "\ -[LOGOUT] token for `crates-io` has been removed from local storage +[LOGOUT] token for `dummy-registry` has been removed from local storage [NOTE] This does not revoke the token on the registry server. - If you need to revoke the token, visit \ + If you need to revoke the token, visit the `dummy-registry` website \ and follow the instructions there. ", ) .run(); - check_token(Some("dummy-token"), Some("dummy-registry")); - check_token(None, None); + check_token(None, Some("dummy-registry")); + check_token(Some("crates-io-token"), None); cargo_process("logout -Zunstable-options") .masquerade_as_nightly_cargo(&["cargo-logout"]) - .with_stderr("[LOGOUT] not currently logged in to `crates-io`") + .with_stderr("[LOGOUT] not currently logged in to `dummy-registry`") .run(); } From a9e0b505d640bdb4b90952f4e69f169bdadafb16 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Mon, 10 Apr 2023 10:20:21 -0700 Subject: [PATCH 4/4] Update auth error message to specify args for `cargo login`. --- src/cargo/sources/registry/http_remote.rs | 1 + src/cargo/util/auth.rs | 10 ++- tests/testsuite/registry.rs | 82 +++++++++++++++++++++++ tests/testsuite/registry_auth.rs | 2 +- 4 files changed, 93 insertions(+), 2 deletions(-) diff --git a/src/cargo/sources/registry/http_remote.rs b/src/cargo/sources/registry/http_remote.rs index c0552734b33..ae89b629dfc 100644 --- a/src/cargo/sources/registry/http_remote.rs +++ b/src/cargo/sources/registry/http_remote.rs @@ -551,6 +551,7 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { if self.auth_required { return Poll::Ready(err.context(auth::AuthorizationError { sid: self.source_id.clone(), + default_registry: self.config.default_registry()?, login_url: self.login_url.clone(), reason: auth::AuthorizationErrorReason::TokenRejected, })); diff --git a/src/cargo/util/auth.rs b/src/cargo/util/auth.rs index f45adc6b745..f19acaebe08 100644 --- a/src/cargo/util/auth.rs +++ b/src/cargo/util/auth.rs @@ -347,6 +347,8 @@ impl fmt::Display for AuthorizationErrorReason { pub struct AuthorizationError { /// Url that was attempted pub sid: SourceId, + /// The `registry.default` config value. + pub default_registry: Option, /// Url where the user could log in. pub login_url: Option, /// Specific reason indicating what failed @@ -356,9 +358,14 @@ impl Error for AuthorizationError {} impl fmt::Display for AuthorizationError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if self.sid.is_crates_io() { + let args = if self.default_registry.is_some() { + " --registry crates-io" + } else { + "" + }; write!( f, - "{}, please run `cargo login`\nor use environment variable CARGO_REGISTRY_TOKEN", + "{}, please run `cargo login{args}`\nor use environment variable CARGO_REGISTRY_TOKEN", self.reason ) } else if let Some(name) = self.sid.alt_registry_key() { @@ -421,6 +428,7 @@ pub fn auth_token( Some(token) => Ok(token.expose()), None => Err(AuthorizationError { sid: sid.clone(), + default_registry: config.default_registry()?, login_url: login_url.cloned(), reason: AuthorizationErrorReason::TokenMissing, } diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 786d598d649..68a16d222b6 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -3192,3 +3192,85 @@ required by package `foo v0.0.1 ([ROOT]/foo)` ] ); } + +#[cargo_test] +fn default_auth_error() { + // Check for the error message for an authentication error when default is set. + let crates_io = RegistryBuilder::new().http_api().build(); + let _alternative = RegistryBuilder::new().http_api().alternative().build(); + + paths::home().join(".cargo/credentials.toml").rm_rf(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + license = "MIT" + description = "foo" + "#, + ) + .file("src/lib.rs", "") + .build(); + + // Test output before setting the default. + p.cargo("publish --no-verify") + .replace_crates_io(crates_io.index_url()) + .with_stderr( + "\ +[UPDATING] crates.io index +error: no token found, please run `cargo login` +or use environment variable CARGO_REGISTRY_TOKEN +", + ) + .with_status(101) + .run(); + + p.cargo("publish --no-verify --registry alternative") + .replace_crates_io(crates_io.index_url()) + .with_stderr( + "\ +[UPDATING] `alternative` index +error: no token found for `alternative`, please run `cargo login --registry alternative` +or use environment variable CARGO_REGISTRIES_ALTERNATIVE_TOKEN +", + ) + .with_status(101) + .run(); + + // Test the output with the default. + cargo_util::paths::append( + &cargo_home().join("config"), + br#" + [registry] + default = "alternative" + "#, + ) + .unwrap(); + + p.cargo("publish --no-verify") + .replace_crates_io(crates_io.index_url()) + .with_stderr( + "\ +[UPDATING] `alternative` index +error: no token found for `alternative`, please run `cargo login --registry alternative` +or use environment variable CARGO_REGISTRIES_ALTERNATIVE_TOKEN +", + ) + .with_status(101) + .run(); + + p.cargo("publish --no-verify --registry crates-io") + .replace_crates_io(crates_io.index_url()) + .with_stderr( + "\ +[UPDATING] crates.io index +error: no token found, please run `cargo login --registry crates-io` +or use environment variable CARGO_REGISTRY_TOKEN +", + ) + .with_status(101) + .run(); +} diff --git a/tests/testsuite/registry_auth.rs b/tests/testsuite/registry_auth.rs index 3989dc9ec2f..27d967b98fb 100644 --- a/tests/testsuite/registry_auth.rs +++ b/tests/testsuite/registry_auth.rs @@ -1,4 +1,4 @@ -//! Tests for normal registry dependencies. +//! Tests for registry authentication. use cargo_test_support::registry::{Package, RegistryBuilder}; use cargo_test_support::{project, Execs, Project};