Update a credentials file format. Implement tests for login command.

Author: fa7ca7

src/cargo/util/config.rs

@@ -5,6 +5,7 @@ use std::collections::HashSet;
 use std::env;
 use std::fmt;
 use std::fs::{self, File};
+use std::io::SeekFrom;
 use std::io::prelude::*;
 use std::mem;
 use std::path::{Path, PathBuf};
@@ -429,28 +430,50 @@ impl Config {
             Ok(())
         }).chain_err(|| "Couldn't load Cargo configuration")?;
 
-        let mut map = match cfg {
-            CV::Table(map, _) => map,
+        self.load_credentials(&mut cfg)?;
+        match cfg {
+            CV::Table(map, _) => Ok(map),
             _ => unreachable!(),
-        };
+        }
+    }
 
+    fn load_credentials(&self, cfg: &mut ConfigValue) -> CargoResult<()> {
         let home_path = self.home_path.clone().into_path_unlocked();
-        let token = load_credentials(&home_path)?;
-        if let Some(t) = token {
-            if !t.is_empty() {
-                let mut registry = map.entry("registry".into())
-                                      .or_insert(CV::Table(HashMap::new(), PathBuf::from(".")));
-                match *registry {
-                    CV::Table(ref mut m, _) => {
-                        m.insert("token".into(),
-                                 CV::String(t, home_path.join("credentials")));
-                    }
-                    _ => unreachable!(),
-                }
-            }
+        let credentials = home_path.join("credentials");
+        if !fs::metadata(&credentials).is_ok() {
+            return Ok(());
         }
 
-        Ok(map)
+        let mut contents = String::new();
+        let mut file = File::open(&credentials)?;
+        file.read_to_string(&mut contents).chain_err(|| {
+            format!("failed to read configuration file `{}`", credentials.display())
+        })?;
+
+        let toml = cargo_toml::parse(&contents,
+                                     &credentials,
+                                     self).chain_err(|| {
+            format!("could not parse TOML configuration in `{}`", credentials.display())
+        })?;
+
+        let value = CV::from_toml(&credentials, toml).chain_err(|| {
+            format!("failed to load TOML configuration from `{}`", credentials.display())
+        })?;
+
+        let mut cfg = match *cfg {
+            CV::Table(ref mut map, _) => map,
+            _ => unreachable!(),
+        };
+
+        let mut registry = cfg.entry("registry".into())
+            .or_insert(CV::Table(HashMap::new(),
+                                 PathBuf::from(".")));
+        registry.merge(value).chain_err(|| {
+            format!("failed to merge configuration at `{}`",
+                          credentials.display())
+        })?;
+
+        Ok(())
     }
 
     /// Look for a path for `tool` in an environment variable or config path, but return `None`
@@ -567,6 +590,21 @@ impl ConfigValue {
         }
     }
 
+    fn into_toml(self) -> toml::Value {
+        match self {
+            CV::Boolean(s, _) => toml::Value::Boolean(s),
+            CV::String(s, _) => toml::Value::String(s),
+            CV::Integer(i, _) => toml::Value::Integer(i),
+            CV::List(l, _) => toml::Value::Array(l
+                                          .into_iter()
+                                          .map(|(s, _)| toml::Value::String(s))
+                                          .collect()),
+            CV::Table(l, _) => toml::Value::Table(l.into_iter()
+                                          .map(|(k, v)| (k, v.into_toml()))
+                                          .collect()),
+        }
+    }
+
     fn merge(&mut self, from: ConfigValue) -> CargoResult<()> {
         match (self, from) {
             (&mut CV::String(..), CV::String(..)) |
@@ -774,8 +812,21 @@ pub fn save_credentials(cfg: &Config,
                                    "credentials' config file")?
     };
 
-    file.write_all(token.as_bytes())?;
-    file.file().set_len(token.len() as u64)?;
+    let mut contents = String::new();
+    file.read_to_string(&mut contents).chain_err(|| {
+        format!("failed to read configuration file `{}`",
+                      file.path().display())
+    })?;
+    let mut toml = cargo_toml::parse(&contents, file.path(), cfg)?;
+    toml.as_table_mut()
+        .unwrap()
+        .insert("token".to_string(),
+                ConfigValue::String(token, file.path().to_path_buf()).into_toml());
+
+    let contents = toml.to_string();
+    file.seek(SeekFrom::Start(0))?;
+    file.write_all(contents.as_bytes())?;
+    file.file().set_len(contents.len() as u64)?;
     set_permissions(file.file(), 0o600)?;
 
     return Ok(());
@@ -796,19 +847,3 @@ pub fn save_credentials(cfg: &Config,
         Ok(())
     }
 }
-
-fn load_credentials(home: &PathBuf) -> CargoResult<Option<String>> {
-    let credentials = home.join("credentials");
-    if !fs::metadata(&credentials).is_ok() {
-        return Ok(None);
-    }
-
-    let mut token = String::new();
-    let mut file = File::open(&credentials)?;
-    file.read_to_string(&mut token).chain_err(|| {
-        format!("failed to read configuration file `{}`",
-                      credentials.display())
-    })?;
-
-    Ok(Some(token.trim().into()))
-}

tests/login.rs

@@ -0,0 +1,112 @@
+#[macro_use]
+extern crate cargotest;
+extern crate cargo;
+extern crate hamcrest;
+extern crate toml;
+
+use std::io::prelude::*;
+use std::fs::{self, File};
+
+use cargotest::cargo_process;
+use cargotest::support::execs;
+use cargotest::support::registry::registry;
+use cargotest::install::cargo_home;
+use hamcrest::{assert_that, existing_file, is_not};
+
+const TOKEN: &str = "test-token";
+const CONFIG_FILE: &str = r#"
+    [registry]
+    token = "api-token"
+"#;
+
+fn setup_old_credentials() {
+    let config = cargo_home().join("config");
+    t!(fs::create_dir_all(config.parent().unwrap()));
+    t!(t!(File::create(&config)).write_all(&CONFIG_FILE.as_bytes()));
+}
+
+fn setup_new_credentials() {
+    let config = cargo_home().join("credentials");
+    t!(fs::create_dir_all(config.parent().unwrap()));
+    t!(t!(File::create(&config)).write_all(br#"
+        token = "api-token"
+    "#));
+}
+
+fn check_host_token(toml: toml::Value) -> bool {
+    match toml {
+        toml::Value::Table(table) => match table.get("token") {
+            Some(v) => match v {
+                &toml::Value::String(ref token) => (token.as_str() == TOKEN),
+                _ => false,
+            },
+            None => false,
+        },
+        _ => false,
+    }
+}
+
+#[test]
+fn login_with_old_credentials() {
+    setup_old_credentials();
+
+    assert_that(cargo_process().arg("login")
+                .arg("--host").arg(registry().to_string()).arg(TOKEN),
+                execs().with_status(0));
+
+    let config = cargo_home().join("config");
+    assert_that(&config, existing_file());
+
+    let mut contents = String::new();
+    File::open(&config).unwrap().read_to_string(&mut contents).unwrap();
+    assert!(CONFIG_FILE == &contents);
+
+    let credentials = cargo_home().join("credentials");
+    assert_that(&credentials, existing_file());
+
+    contents.clear();
+    File::open(&credentials).unwrap().read_to_string(&mut contents).unwrap();
+    assert!(check_host_token(contents.parse().unwrap()));
+}
+
+#[test]
+fn login_with_new_credentials() {
+    setup_new_credentials();
+
+    assert_that(cargo_process().arg("login")
+                .arg("--host").arg(registry().to_string()).arg(TOKEN),
+                execs().with_status(0));
+
+    let config = cargo_home().join("config");
+    assert_that(&config, is_not(existing_file()));
+
+    let credentials = cargo_home().join("credentials");
+    assert_that(&credentials, existing_file());
+
+    let mut contents = String::new();
+    File::open(&credentials).unwrap().read_to_string(&mut contents).unwrap();
+    assert!(check_host_token(contents.parse().unwrap()));
+}
+
+#[test]
+fn login_with_old_and_new_credentials() {
+    setup_new_credentials();
+    login_with_old_credentials();
+}
+
+#[test]
+fn login_without_credentials() {
+    assert_that(cargo_process().arg("login")
+                .arg("--host").arg(registry().to_string()).arg(TOKEN),
+                execs().with_status(0));
+
+    let config = cargo_home().join("config");
+    assert_that(&config, is_not(existing_file()));
+
+    let credentials = cargo_home().join("credentials");
+    assert_that(&credentials, existing_file());
+
+    let mut contents = String::new();
+    File::open(&credentials).unwrap().read_to_string(&mut contents).unwrap();
+    assert!(check_host_token(contents.parse().unwrap()));
+}