Auto merge of #11949 - ehuss:logout-default, r=epage

Use registry.default for login/logout

This changes `cargo login` and `cargo logout` to use the registry configured at `registry.default` as the registry instead of crates.io. For `cargo login`, this was an unintentional regression from #6466. The documentation has always stated that it will use the default registry.

This makes the command more in line with other registry-involving commands. There are still some inconsistencies.

These commands use the default if not specified:

* `cargo init`
* `cargo new`
* `cargo owner`
* `cargo search`
* `cargo yank`
* `cargo publish` uses the default, but will also look at the `publish` field `Cargo.toml` and use that if the registry is not specified.

These commands would always use crates.io if `--registry` is not specified:

* `cargo login`
* `cargo logout`
* `cargo install`

I'm a bit uncertain how to proceed, since this is technically a breaking change particularly if someone has scripted it. I suspect that the number of users that use `registry.default` is very small, and those that script `cargo login` are even smaller, and thus the intersection is probably small or nonexistent. However, there is some risk here.

Author: bors

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::<String>("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),

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::<String>("registry").map(String::as_str),
-    )?;
+    let registry = args.registry(config)?;
+    ops::registry_logout(config, registry.as_deref())?;
     Ok(())
 }

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,
                         }));

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<String>,
     /// Url where the user could log in.
     pub login_url: Option<Url>,
     /// 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,
         }

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]
@@ -367,3 +373,32 @@ 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 _alternative = RegistryBuilder::new().alternative().build();
+    let cargo_home = paths::home().join(".cargo");
+    cargo_util::paths::append(
+        &cargo_home.join("config"),
+        br#"
+            [registry]
+            default = "alternative"
+        "#,
+    )
+    .unwrap();
+
+    cargo_process("login")
+        .arg("a-new-token")
+        .with_stderr(
+            "\
+[UPDATING] `alternative` index
+[LOGIN] token for `alternative` saved
+",
+        )
+        .run();
+
+    check_token(None, None);
+    check_token(Some("a-new-token"), Some("alternative"));
+}

tests/testsuite/logout.rs

@@ -1,9 +1,9 @@
 //! Tests for the `cargo logout` command.
 
-use cargo_test_support::install::cargo_home;
+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};
-use std::fs;
 
 #[cargo_test]
 fn gated() {
@@ -21,32 +21,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 +38,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,11 +48,11 @@ 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]
-fn default_registry() {
+fn default_registry_unconfigured() {
     let registry = registry::init();
     simple_logout_test(&registry, None, "", "<https://crates.io/me>");
 }
@@ -89,4 +66,57 @@ fn other_registry() {
         "--registry alternative",
         "the `alternative` website",
     );
+    // 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 `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 the `dummy-registry` website \
+    and follow the instructions there.
+",
+        )
+        .run();
+    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 `dummy-registry`")
+        .run();
 }

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();
+}

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};