diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 6496715631f..78681a6904e 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -1,8 +1,6 @@ -use std::collections::HashMap; use std::env; use std::fs::{self, File}; use std::iter::repeat; -use std::path::PathBuf; use std::time::Duration; use curl::easy::{Easy, SslOpt}; @@ -19,10 +17,9 @@ use core::dependency::Kind; use core::manifest::ManifestMetadata; use ops; use sources::{RegistrySource}; -use util::config; +use util::config::{self, Config}; use util::paths; use util::ToUrl; -use util::config::{Config, ConfigValue, Location}; use util::errors::{CargoError, CargoResult, CargoResultExt}; use util::important_paths::find_root_manifest_for_wd; @@ -285,16 +282,14 @@ pub fn http_timeout(config: &Config) -> CargoResult> { } pub fn registry_login(config: &Config, token: String) -> CargoResult<()> { - let RegistryConfig { index, token: _ } = registry_configuration(config)?; - let mut map = HashMap::new(); - let p = config.cwd().to_path_buf(); - if let Some(index) = index { - map.insert("index".to_string(), ConfigValue::String(index, p.clone())); + let RegistryConfig { index: _, token: old_token } = registry_configuration(config)?; + if let Some(old_token) = old_token { + if old_token == token { + return Ok(()); + } } - map.insert("token".to_string(), ConfigValue::String(token, p)); - config::set_config(config, Location::Global, "registry", - ConfigValue::Table(map, PathBuf::from("."))) + config::save_credentials(config, token) } pub struct OwnersOptions { diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index afaacae0674..fe828c10724 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -407,11 +407,12 @@ impl Config { pub fn load_values(&self) -> CargoResult> { let mut cfg = CV::Table(HashMap::new(), PathBuf::from(".")); - walk_tree(&self.cwd, |mut file, path| { + walk_tree(&self.cwd, |path| { let mut contents = String::new(); + let mut file = File::open(&path)?; file.read_to_string(&mut contents).chain_err(|| { format!("failed to read configuration file `{}`", - path.display()) + path.display()) })?; let toml = cargo_toml::parse(&contents, &path, @@ -429,13 +430,52 @@ impl Config { Ok(()) }).chain_err(|| "Couldn't load Cargo configuration")?; - + 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 credentials = home_path.join("credentials"); + if !fs::metadata(&credentials).is_ok() { + return Ok(()); + } + + 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` /// if it's not present. fn maybe_get_tool(&self, tool: &str) -> CargoResult> { @@ -550,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(..)) | @@ -651,21 +706,6 @@ impl ConfigValue { wanted, self.desc(), key, self.definition_path().display()).into()) } - - 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()), - } - } } impl Definition { @@ -737,17 +777,14 @@ pub fn homedir(cwd: &Path) -> Option { } fn walk_tree(pwd: &Path, mut walk: F) -> CargoResult<()> - where F: FnMut(File, &Path) -> CargoResult<()> + where F: FnMut(&Path) -> CargoResult<()> { let mut stash: HashSet = HashSet::new(); for current in paths::ancestors(pwd) { let possible = current.join(".cargo").join("config"); if fs::metadata(&possible).is_ok() { - let file = File::open(&possible)?; - - walk(file, &possible)?; - + walk(&possible)?; stash.insert(possible); } } @@ -761,40 +798,52 @@ fn walk_tree(pwd: &Path, mut walk: F) -> CargoResult<()> })?; let config = home.join("config"); if !stash.contains(&config) && fs::metadata(&config).is_ok() { - let file = File::open(&config)?; - walk(file, &config)?; + walk(&config)?; } Ok(()) } -pub fn set_config(cfg: &Config, - loc: Location, - key: &str, - value: ConfigValue) -> CargoResult<()> { - // TODO: There are a number of drawbacks here - // - // 1. Project is unimplemented - // 2. This blows away all comments in a file - // 3. This blows away the previous ordering of a file. - let mut file = match loc { - Location::Global => { - cfg.home_path.create_dir()?; - cfg.home_path.open_rw(Path::new("config"), cfg, - "the global config file")? - } - Location::Project => unimplemented!(), +pub fn save_credentials(cfg: &Config, + token: String) -> CargoResult<()> { + let mut file = { + cfg.home_path.create_dir()?; + cfg.home_path.open_rw(Path::new("credentials"), cfg, + "credentials' config file")? }; + let mut contents = String::new(); - let _ = file.read_to_string(&mut contents); + 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(key.to_string(), value.into_toml()); + .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)?; - Ok(()) + set_permissions(file.file(), 0o600)?; + + return Ok(()); + + #[cfg(unix)] + fn set_permissions(file: & File, mode: u32) -> CargoResult<()> { + use std::os::unix::fs::PermissionsExt; + + let mut perms = file.metadata()?.permissions(); + perms.set_mode(mode); + file.set_permissions(perms)?; + Ok(()) + } + + #[cfg(not(unix))] + #[allow(unused)] + fn set_permissions(file: & File, mode: u32) -> CargoResult<()> { + Ok(()) + } } diff --git a/src/doc/crates-io.md b/src/doc/crates-io.md index 8e9a5269b95..ba4d2b7dab0 100644 --- a/src/doc/crates-io.md +++ b/src/doc/crates-io.md @@ -21,7 +21,8 @@ $ cargo login abcdefghijklmnopqrstuvwxyz012345 ``` This command will inform Cargo of your API token and store it locally in your -`~/.cargo/config`. Note that this token is a **secret** and should not be shared +`~/.cargo/credentials` (previously it was `~/.cargo/config`). +Note that this token is a **secret** and should not be shared with anyone else. If it leaks for any reason, you should regenerate it immediately. diff --git a/tests/login.rs b/tests/login.rs new file mode 100644 index 00000000000..306b4e0d9ea --- /dev/null +++ b/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())); +}