Auto merge of #7708 - giraffate:fix_overwriting_credentials, r=ehuss

Fix overwriting alternate registry token

When executing `cargo login`, 2nd alternate registry token overwrites
1st alternate registry token.

Fixes #7701.

Author: bors

crates/cargo-test-support/src/registry.rs

@@ -15,49 +15,59 @@ use url::Url;
 /// initialized with a `config.json` file pointing to `dl_path` for downloads
 /// and `api_path` for uploads.
 pub fn registry_path() -> PathBuf {
-    paths::root().join("registry")
+    generate_path("registry")
 }
 pub fn registry_url() -> Url {
-    Url::from_file_path(registry_path()).ok().unwrap()
+    generate_url("registry")
 }
 /// Gets the path for local web API uploads. Cargo will place the contents of a web API
 /// request here. For example, `api/v1/crates/new` is the result of publishing a crate.
 pub fn api_path() -> PathBuf {
-    paths::root().join("api")
+    generate_path("api")
 }
 pub fn api_url() -> Url {
-    Url::from_file_path(api_path()).ok().unwrap()
+    generate_url("api")
 }
 /// Gets the path where crates can be downloaded using the web API endpoint. Crates
 /// should be organized as `{name}/{version}/download` to match the web API
 /// endpoint. This is rarely used and must be manually set up.
 pub fn dl_path() -> PathBuf {
-    paths::root().join("dl")
+    generate_path("dl")
 }
 pub fn dl_url() -> Url {
-    Url::from_file_path(dl_path()).ok().unwrap()
+    generate_url("dl")
 }
 /// Gets the alternative-registry version of `registry_path`.
 pub fn alt_registry_path() -> PathBuf {
-    paths::root().join("alternative-registry")
+    generate_path("alternative-registry")
 }
 pub fn alt_registry_url() -> Url {
-    Url::from_file_path(alt_registry_path()).ok().unwrap()
+    generate_url("alternative-registry")
 }
 /// Gets the alternative-registry version of `dl_path`.
 pub fn alt_dl_path() -> PathBuf {
-    paths::root().join("alt_dl")
+    generate_path("alt_dl")
 }
 pub fn alt_dl_url() -> String {
-    let base = Url::from_file_path(alt_dl_path()).ok().unwrap();
-    format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base)
+    generate_alt_dl_url("alt_dl")
 }
 /// Gets the alternative-registry version of `api_path`.
 pub fn alt_api_path() -> PathBuf {
-    paths::root().join("alt_api")
+    generate_path("alt_api")
 }
 pub fn alt_api_url() -> Url {
-    Url::from_file_path(alt_api_path()).ok().unwrap()
+    generate_url("alt_api")
+}
+
+pub fn generate_path(name: &str) -> PathBuf {
+    paths::root().join(name)
+}
+pub fn generate_url(name: &str) -> Url {
+    Url::from_file_path(generate_path(name)).ok().unwrap()
+}
+pub fn generate_alt_dl_url(name: &str) -> String {
+    let base = Url::from_file_path(generate_path(name)).ok().unwrap();
+    format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base)
 }
 
 /// A builder for creating a new package in a registry.
@@ -184,34 +194,36 @@ pub fn init() {
     ));
 
     // Initialize a new registry.
-    let _ = repo(&registry_path())
-        .file(
-            "config.json",
-            &format!(
-                r#"
-            {{"dl":"{}","api":"{}"}}
-        "#,
-                dl_url(),
-                api_url()
-            ),
-        )
-        .build();
-    fs::create_dir_all(api_path().join("api/v1/crates")).unwrap();
+    init_registry(
+        registry_path(),
+        dl_url().into_string(),
+        api_url(),
+        api_path(),
+    );
 
     // Initialize an alternative registry.
-    repo(&alt_registry_path())
+    init_registry(
+        alt_registry_path(),
+        alt_dl_url(),
+        alt_api_url(),
+        alt_api_path(),
+    );
+}
+
+pub fn init_registry(registry_path: PathBuf, dl_url: String, api_url: Url, api_path: PathBuf) {
+    // Initialize a new registry.
+    repo(&registry_path)
         .file(
             "config.json",
             &format!(
                 r#"
             {{"dl":"{}","api":"{}"}}
         "#,
-                alt_dl_url(),
-                alt_api_url()
+                dl_url, api_url
             ),
         )
         .build();
-    fs::create_dir_all(alt_api_path().join("api/v1/crates")).unwrap();
+    fs::create_dir_all(api_path.join("api/v1/crates")).unwrap();
 }
 
 impl Package {

src/cargo/util/config/mod.rs

@@ -1314,14 +1314,14 @@ pub fn save_credentials(cfg: &Config, token: String, registry: Option<String>) -
             .open_rw(filename, cfg, "credentials' config file")?
     };
 
-    let (key, value) = {
+    let (key, mut value) = {
         let key = "token".to_string();
         let value = ConfigValue::String(token, file.path().to_path_buf());
         let mut map = HashMap::new();
         map.insert(key, value);
         let table = CV::Table(map, file.path().to_path_buf());
 
-        if let Some(registry) = registry {
+        if let Some(registry) = registry.clone() {
             let mut map = HashMap::new();
             map.insert(registry, table);
             (
@@ -1352,6 +1352,12 @@ pub fn save_credentials(cfg: &Config, token: String, registry: Option<String>) -
             .insert("registry".into(), map.into());
     }
 
+    if let Some(_) = registry {
+        if let Some(table) = toml.as_table_mut().unwrap().remove("registries") {
+            let v = CV::from_toml(file.path(), table)?;
+            value.merge(v)?;
+        }
+    }
     toml.as_table_mut().unwrap().insert(key, value.into_toml());
 
     let contents = toml.to_string();

tests/testsuite/login.rs

@@ -1,17 +1,18 @@
 //! Tests for the `cargo login` command.
 
-use std::fs::{self, File};
+use std::fs::{self, File, OpenOptions};
 use std::io::prelude::*;
 use std::path::PathBuf;
 
 use cargo::core::Shell;
 use cargo::util::config::Config;
 use cargo_test_support::install::cargo_home;
 use cargo_test_support::registry::{self, registry_url};
-use cargo_test_support::{cargo_process, t};
+use cargo_test_support::{cargo_process, paths, t};
 use toml;
 
 const TOKEN: &str = "test-token";
+const TOKEN2: &str = "test-token2";
 const ORIGINAL_TOKEN: &str = "api-token";
 
 fn setup_new_credentials() {
@@ -169,20 +170,47 @@ fn new_credentials_is_used_instead_old() {
 #[cargo_test]
 fn registry_credentials() {
     registry::init();
+
+    let config = paths::home().join(".cargo/config");
+    let mut f = OpenOptions::new().append(true).open(config).unwrap();
+    t!(f.write_all(
+        format!(
+            r#"
+        [registries.alternative2]
+        index = '{}'
+    "#,
+            registry::generate_url("alternative2-registry")
+        )
+        .as_bytes(),
+    ));
+
+    registry::init_registry(
+        registry::generate_path("alternative2-registry"),
+        registry::generate_alt_dl_url("alt2_dl"),
+        registry::generate_url("alt2_api"),
+        registry::generate_path("alt2_api"),
+    );
     setup_new_credentials();
 
     let reg = "alternative";
 
-    cargo_process("login --registry")
-        .arg(reg)
-        .arg(TOKEN)
-        .arg("-Zunstable-options")
-        .masquerade_as_nightly_cargo()
-        .run();
+    cargo_process("login --registry").arg(reg).arg(TOKEN).run();
 
     // Ensure that we have not updated the default token
     assert!(check_token(ORIGINAL_TOKEN, None));
 
     // Also ensure that we get the new token for the registry
     assert!(check_token(TOKEN, Some(reg)));
+
+    let reg2 = "alternative2";
+    cargo_process("login --registry")
+        .arg(reg2)
+        .arg(TOKEN2)
+        .run();
+
+    // 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)));
 }