diff --git a/src/bin/login.rs b/src/bin/login.rs index 6e7841717b6..95d0052bfe0 100644 --- a/src/bin/login.rs +++ b/src/bin/login.rs @@ -40,26 +40,30 @@ pub fn execute(options: Options, config: &Config) -> CliResult { &options.flag_color, options.flag_frozen, options.flag_locked)?; - let token = match options.arg_token.clone() { - Some(token) => token, + + let host = match options.flag_host { + Some(host) => host, None => { let src = SourceId::crates_io(config)?; let mut src = RegistrySource::remote(&src, config); src.update()?; - let config = src.config()?.unwrap(); - let host = options.flag_host.clone().unwrap_or(config.api); - println!("please visit {}me and paste the API Token below", host); + src.config()?.unwrap().api + } + }; + + let token = match options.arg_token { + Some(token) => token, + None => { + println!("please visit {}me and paste the API Token below", &host); let mut line = String::new(); let input = io::stdin(); input.lock().read_line(&mut line).chain_err(|| { "failed to read stdin" })?; - line + line.trim().to_string() } }; - let token = token.trim().to_string(); - ops::registry_login(config, token)?; + ops::registry_login(config, token, host)?; Ok(()) } - diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs index 9f32d692dfe..f0073fe0913 100644 --- a/src/cargo/core/source.rs +++ b/src/cargo/core/source.rs @@ -232,7 +232,7 @@ impl SourceId { /// This is the main cargo registry by default, but it can be overridden in /// a `.cargo/config`. pub fn crates_io(config: &Config) -> CargoResult { - let cfg = ops::registry_configuration(config)?; + let cfg = ops::registry_configuration(config, "https://crates.io")?; let url = if let Some(ref index) = cfg.index { static WARNED: AtomicBool = ATOMIC_BOOL_INIT; if !WARNED.swap(true, SeqCst) { diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 78681a6904e..5b457e48ca1 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -177,25 +177,39 @@ fn transmit(config: &Config, } } -pub fn registry_configuration(config: &Config) -> CargoResult { - let index = config.get_string("registry.index")?.map(|p| p.val); - let token = config.get_string("registry.token")?.map(|p| p.val); - Ok(RegistryConfig { index: index, token: token }) +pub fn registry_configuration(config: &Config, + host: &str) -> CargoResult { + let mut index = None; + let mut token = None; + + if !host.is_empty() { + index = config.get_string(&format!("registry.{}.index", host))?; + token = config.get_string(&format!("registry.{}.token", host))?; + } + + // FIXME: Checking out for the values which were picked up from + // $CARGO_HOME/config. This section should be removed after all the users + // start to use $CARGO_HOME/credentials for token configuration. + if index.is_none() && token.is_none() { + index = config.get_string("registry.index")?; + token = config.get_string("registry.token")?; + } + + Ok(RegistryConfig { + index: index.map(|p| p.val), + token: token.map(|p| p.val) + }) } pub fn registry(config: &Config, token: Option, index: Option) -> CargoResult<(Registry, SourceId)> { // Parse all configuration options - let RegistryConfig { - token: token_config, - index: _index_config, - } = registry_configuration(config)?; - let token = token.or(token_config); let sid = match index { Some(index) => SourceId::for_registry(&index.to_url()?), None => SourceId::crates_io(config)?, }; + let api_host = { let mut src = RegistrySource::remote(&sid, config); src.update().chain_err(|| { @@ -203,6 +217,12 @@ pub fn registry(config: &Config, })?; (src.config()?).unwrap().api }; + + let RegistryConfig { + token: token_config, + index: _index_config, + } = registry_configuration(config, &api_host)?; + let token = token.or(token_config); let handle = http_handle(config)?; Ok((Registry::new_handle(api_host, token, handle), sid)) } @@ -281,15 +301,31 @@ pub fn http_timeout(config: &Config) -> CargoResult> { Ok(env::var("HTTP_TIMEOUT").ok().and_then(|s| s.parse().ok())) } -pub fn registry_login(config: &Config, token: String) -> CargoResult<()> { - let RegistryConfig { index: _, token: old_token } = registry_configuration(config)?; +pub fn registry_login(config: &Config, + token: String, + host: String) -> CargoResult<()> { + let host = match host.to_url()?.host_str() { + Some(h) => h.to_string(), + None => host, + }; + let host: String = host.chars() + .map(|x| match x { + '\\'|'/'|':'|'.'|'-' => '_', + _ => x, + }).collect(); + + let RegistryConfig { + index: _, + token: old_token + } = registry_configuration(config, &host)?; + if let Some(old_token) = old_token { if old_token == token { return Ok(()); } } - config::save_credentials(config, token) + config::save_credentials(config, token, host) } pub struct OwnersOptions { diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index b99285ce7a1..a4e94b08558 100644 --- a/src/cargo/util/config.rs +++ b/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,52 @@ 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 +592,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(..)) | @@ -767,15 +807,32 @@ fn walk_tree(pwd: &Path, mut walk: F) -> CargoResult<()> } pub fn save_credentials(cfg: &Config, - token: String) -> CargoResult<()> { + token: String, + host: String) -> CargoResult<()> { let mut file = { cfg.home_path.create_dir()?; cfg.home_path.open_rw(Path::new("credentials"), cfg, "credentials' config file")? }; - file.write_all(token.as_bytes())?; - file.file().set_len(token.len() as u64)?; + let mut map = HashMap::new(); + map.insert("token".to_string(), + ConfigValue::String(token, file.path().to_path_buf())); + + 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(host, CV::Table(map, 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 +853,3 @@ pub fn save_credentials(cfg: &Config, Ok(()) } } - -fn load_credentials(home: &PathBuf) -> CargoResult> { - 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())) -} diff --git a/tests/login.rs b/tests/login.rs new file mode 100644 index 00000000000..c0467d66317 --- /dev/null +++ b/tests/login.rs @@ -0,0 +1,143 @@ +#[macro_use] +extern crate cargotest; +extern crate cargo; +extern crate hamcrest; +extern crate toml; + +use std::io::prelude::*; +use std::fs::{self, File}; + +use cargo::util::to_url::ToUrl; +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(format!(r#" + [crates_io] + token = "api-token" + + [{key}] + token = "api-token" + "#, key = host_to_key(registry().to_string())) + .as_bytes())); +} + +fn host_to_key(host: String) -> String { + let host = match host.to_url().unwrap().host_str() { + Some(h) => h.to_string(), + None => host, + }; + + host.chars() + .map(|x| match x { + '\\'|'/'|':'|'.'|'-' => '_', + _ => x, + }).collect() +} + +fn check_host_token(mut toml: toml::Value, host_key: &str) -> bool { + for &key in [host_key, "token"].into_iter() { + match toml { + toml::Value::Table(table) => { + if let Some(v) = table.get(key) { + toml = v.clone(); + } else { + return false; + } + } + _ => break, + } + } + + match toml { + toml::Value::String(token) => (&token == TOKEN), + _ => 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(), + &host_to_key(registry().to_string()))); +} + +#[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(), + &host_to_key(registry().to_string()))); +} + +#[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(); + let toml: toml::Value = contents.parse().unwrap(); + + assert!(check_host_token(toml.clone(), + &host_to_key(registry().to_string()))); +} diff --git a/tests/registry.rs b/tests/registry.rs index b336827abbc..5983cdd95e0 100644 --- a/tests/registry.rs +++ b/tests/registry.rs @@ -581,7 +581,8 @@ fn dev_dependency_not_used() { fn login_with_no_cargo_dir() { let home = paths::home().join("new-home"); t!(fs::create_dir(&home)); - assert_that(cargo_process().arg("login").arg("foo").arg("-v"), + assert_that(cargo_process().arg("login").arg("--host").arg(registry().to_string()) + .arg("foo").arg("-v"), execs().with_status(0)); } @@ -590,11 +591,14 @@ fn login_with_differently_sized_token() { // Verify that the configuration file gets properly trunchated. let home = paths::home().join("new-home"); t!(fs::create_dir(&home)); - assert_that(cargo_process().arg("login").arg("lmaolmaolmao").arg("-v"), + assert_that(cargo_process().arg("login").arg("--host").arg(registry().to_string()) + .arg("lmaolmaolmao").arg("-v"), execs().with_status(0)); - assert_that(cargo_process().arg("login").arg("lmao").arg("-v"), + assert_that(cargo_process().arg("login").arg("--host").arg(registry().to_string()) + .arg("lmao").arg("-v"), execs().with_status(0)); - assert_that(cargo_process().arg("login").arg("lmaolmaolmao").arg("-v"), + assert_that(cargo_process().arg("login").arg("--host").arg(registry().to_string()) + .arg("lmaolmaolmao").arg("-v"), execs().with_status(0)); }