Auto merge of #7774 - giraffate:update_credentials, r=ehuss

Load credentials only when needed

Credentials are always loaded, even if these are not used. If
access to confidential files such as credentials is not given,
`cargo build` fails despite not using credentials.

Fixes #7624.

Author: bors

src/bin/cargo/commands/owner.rs

@@ -39,6 +39,8 @@ Explicitly named owners can also modify the set of owners, so take care!
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+    config.load_credentials()?;
+
     let registry = args.registry(config)?;
     let opts = OwnersOptions {
         krate: args.value_of("crate").map(|s| s.to_string()),

src/bin/cargo/commands/publish.rs

@@ -26,6 +26,8 @@ pub fn cli() -> App {
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+    config.load_credentials()?;
+
     let registry = args.registry(config)?;
     let ws = args.workspace(config)?;
     let index = args.index(config)?;

src/bin/cargo/commands/yank.rs

@@ -29,6 +29,8 @@ crates to be locked to any yanked version.
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
+    config.load_credentials()?;
+
     let registry = args.registry(config)?;
 
     ops::yank(

src/cargo/util/config/mod.rs

@@ -723,7 +723,6 @@ impl Config {
         })
         .chain_err(|| "could not load Cargo configuration")?;
 
-        self.load_credentials(&mut cfg)?;
         match cfg {
             CV::Table(map, _) => Ok(map),
             _ => unreachable!(),
@@ -956,9 +955,8 @@ impl Config {
         Ok(url)
     }
 
-    /// Loads credentials config from the credentials file into the `ConfigValue` object, if
-    /// present.
-    fn load_credentials(&self, cfg: &mut ConfigValue) -> CargoResult<()> {
+    /// Loads credentials config from the credentials file, if present.
+    pub fn load_credentials(&mut self) -> CargoResult<()> {
         let home_path = self.home_path.clone().into_path_unlocked();
         let credentials = match self.get_file_path(&home_path, "credentials", true)? {
             Some(credentials) => credentials,
@@ -983,7 +981,19 @@ impl Config {
             }
         }
 
-        cfg.merge(value, true)?;
+        if let CV::Table(map, _) = value {
+            let base_map = self.values_mut()?;
+            for (k, v) in map {
+                match base_map.entry(k) {
+                    Vacant(entry) => {
+                        entry.insert(v);
+                    }
+                    Occupied(mut entry) => {
+                        entry.get_mut().merge(v, true)?;
+                    }
+                }
+            }
+        }
 
         Ok(())
     }

tests/testsuite/build.rs

@@ -2237,6 +2237,32 @@ fn recompile_space_in_name() {
     foo.cargo("build").with_stdout("").run();
 }
 
+#[cfg(unix)]
+#[cargo_test]
+fn credentials_is_unreadable() {
+    use cargo_test_support::paths::home;
+    use std::os::unix::prelude::*;
+    let p = project()
+        .file("Cargo.toml", &basic_manifest("foo", "0.1.0"))
+        .file("src/lib.rs", "")
+        .build();
+
+    let credentials = home().join(".cargo/credentials");
+    t!(fs::create_dir_all(credentials.parent().unwrap()));
+    t!(t!(File::create(&credentials)).write_all(
+        br#"
+        [registry]
+        token = "api-token"
+    "#
+    ));
+    let stat = fs::metadata(credentials.as_path()).unwrap();
+    let mut perms = stat.permissions();
+    perms.set_mode(0o000);
+    fs::set_permissions(credentials, perms.clone()).unwrap();
+
+    p.cargo("build").run();
+}
+
 #[cfg(unix)]
 #[cargo_test]
 fn ignore_bad_directories() {

tests/testsuite/login.rs

@@ -111,28 +111,6 @@ fn credentials_work_with_extension() {
     assert!(check_token(TOKEN, None));
 }
 
-#[cargo_test]
-fn credentials_ambiguous_filename() {
-    registry::init();
-    setup_new_credentials();
-    setup_new_credentials_toml();
-
-    cargo_process("login --host")
-        .arg(registry_url().to_string())
-        .arg(TOKEN)
-        .with_stderr_contains(
-            "\
-[WARNING] Both `[..]/credentials` and `[..]/credentials.toml` exist. Using `[..]/credentials`
-",
-        )
-        .run();
-
-    // It should use the value from the one without the extension
-    // for backwards compatibility. check_token explicitly checks
-    // 'credentials' without the extension, which is what we want.
-    assert!(check_token(TOKEN, None));
-}
-
 #[cargo_test]
 fn login_with_old_and_new_credentials() {
     setup_new_credentials();
@@ -161,7 +139,9 @@ fn new_credentials_is_used_instead_old() {
         .arg(TOKEN)
         .run();
 
-    let config = Config::new(Shell::new(), cargo_home(), cargo_home());
+    let mut config = Config::new(Shell::new(), cargo_home(), cargo_home());
+    let _ = config.values();
+    let _ = config.load_credentials();
 
     let token = config.get_string("registry.token").unwrap().map(|p| p.val);
     assert_eq!(token.unwrap(), TOKEN);

tests/testsuite/main.rs

@@ -68,6 +68,7 @@ mod net_config;
 mod new;
 mod offline;
 mod out_dir;
+mod owner;
 mod package;
 mod patch;
 mod path;
@@ -106,6 +107,7 @@ mod verify_project;
 mod version;
 mod warn_on_failure;
 mod workspaces;
+mod yank;
 
 #[cargo_test]
 fn aaa_trigger_cross_compile_disabled_check() {

tests/testsuite/owner.rs

@@ -0,0 +1,50 @@
+//! Tests for the `cargo owner` command.
+
+use std::fs;
+
+use cargo_test_support::paths::CargoPathExt;
+use cargo_test_support::project;
+use cargo_test_support::registry::{self, api_path, registry_url};
+
+fn setup(name: &str) {
+    let dir = api_path().join(format!("api/v1/crates/{}", name));
+    dir.mkdir_p();
+    fs::write(
+        dir.join("owners"),
+        r#"{
+        "users": [
+            {
+                "id": 70,
+                "login": "github:rust-lang:core",
+                "name": "Core"
+            }
+        ]
+    }"#,
+    )
+    .unwrap();
+}
+
+#[cargo_test]
+fn simple_list() {
+    registry::init();
+    setup("foo");
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+            description = "foo"
+        "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    p.cargo("owner -l --index")
+        .arg(registry_url().to_string())
+        .run();
+}

tests/testsuite/publish.rs

@@ -1261,3 +1261,37 @@ repository = "foo"
         )],
     );
 }
+
+#[cargo_test]
+fn credentials_ambiguous_filename() {
+    registry::init();
+
+    let credentials_toml = paths::home().join(".cargo/credentials.toml");
+    fs::write(credentials_toml, r#"token = "api-token""#).unwrap();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+            description = "foo"
+        "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    p.cargo("publish --no-verify --index")
+        .arg(registry_url().to_string())
+        .with_stderr_contains(
+            "\
+[WARNING] Both `[..]/credentials` and `[..]/credentials.toml` exist. Using `[..]/credentials`
+",
+        )
+        .run();
+
+    validate_upload_foo();
+}

tests/testsuite/yank.rs

@@ -0,0 +1,38 @@
+//! Tests for the `cargo yank` command.
+
+use std::fs;
+
+use cargo_test_support::paths::CargoPathExt;
+use cargo_test_support::project;
+use cargo_test_support::registry::{self, api_path, registry_url};
+
+fn setup(name: &str, version: &str) {
+    let dir = api_path().join(format!("api/v1/crates/{}/{}", name, version));
+    dir.mkdir_p();
+    fs::write(dir.join("yank"), r#"{"ok": true}"#).unwrap();
+}
+
+#[cargo_test]
+fn simple() {
+    registry::init();
+    setup("foo", "0.0.1");
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+            license = "MIT"
+            description = "foo"
+        "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    p.cargo("yank --vers 0.0.1 --index")
+        .arg(registry_url().to_string())
+        .run();
+}