From 30cc37846defb0933bb70dc9ccf3c1b55fe34985 Mon Sep 17 00:00:00 2001 From: Evgen Druzhynin Date: Tue, 2 May 2017 22:15:05 +0300 Subject: [PATCH 1/6] Move API token into the separate file. Now the token is stored in ~/.cargo/credentials (with 600 permissions on unix-like systems). --- src/cargo/ops/registry.rs | 27 ++++--- src/cargo/util/config.rs | 152 ++++++++++++++++++++++++-------------- tests/publish.rs | 4 +- 3 files changed, 114 insertions(+), 69 deletions(-) diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 6496715631f..7bf9a0f08f7 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; @@ -69,6 +66,14 @@ pub fn publish(ws: &Workspace, opts: &PublishOpts) -> CargoResult<()> { opts.config.shell().status("Uploading", pkg.package_id().to_string())?; transmit(opts.config, pkg, tarball.file(), &mut registry, opts.dry_run)?; + if opts.config.is_token_in_main_config() { + let _ = opts.config + .shell() + .warn("API token detected in ~/.cargo/config under `registry.token`.\n \ + You should remove it and do `login` again in order to \ + save token in ~/.cargo/credentials"); + } + Ok(()) } @@ -285,16 +290,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..c99ec82c661 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -5,7 +5,6 @@ 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}; @@ -35,6 +34,10 @@ pub struct Config { extra_verbose: Cell, frozen: Cell, locked: Cell, + + // A temporary solution to point on an old configuration's usage. + // If it's true cargo will warn on it on publish. + token_in_main_config: Cell, } impl Config { @@ -52,6 +55,7 @@ impl Config { extra_verbose: Cell::new(false), frozen: Cell::new(false), locked: Cell::new(false), + token_in_main_config: Cell::new(false), } } @@ -404,14 +408,19 @@ impl Config { !self.frozen.get() && !self.locked.get() } + pub fn is_token_in_main_config(&self) -> bool { + self.token_in_main_config.get() + } + 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,11 +438,30 @@ impl Config { Ok(()) }).chain_err(|| "Couldn't load Cargo configuration")?; + self.token_in_main_config.set(check_token_in_main_config("registry.token".into(), &cfg)); - match cfg { - CV::Table(map, _) => Ok(map), + let mut map = match cfg { + CV::Table(map, _) => map, _ => unreachable!(), + }; + + 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!(), + } + } } + + Ok(map) } /// Look for a path for `tool` in an environment variable or config path, but return `None` @@ -651,21 +679,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 +750,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 +771,72 @@ 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); - let mut toml = cargo_toml::parse(&contents, file.path(), cfg)?; - toml.as_table_mut() - .unwrap() - .insert(key.to_string(), value.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(()) + + file.write_all(token.as_bytes())?; + file.file().set_len(token.len() as u64)?; + 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(()) + } +} + +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())) +} + +fn check_token_in_main_config(key: &str, cfg: &ConfigValue) -> bool { + let mut keys = key.split('.'); + let k = keys.next().unwrap(); + let keys: String = keys.map(String::from).collect(); + + match *cfg { + CV::Table(ref map, _) => { + match map.get(k) { + Some(ref v) => check_token_in_main_config(keys.as_str(), v), + None => return false, + } + } + CV::String(ref v, _) => !v.is_empty(), + _ => return false, + } } diff --git a/tests/publish.rs b/tests/publish.rs index 968281f6219..f514adf9c84 100644 --- a/tests/publish.rs +++ b/tests/publish.rs @@ -57,7 +57,7 @@ fn simple() { assert_that(p.cargo_process("publish").arg("--no-verify") .arg("--host").arg(registry().to_string()), - execs().with_status(0).with_stderr(&format!("\ + execs().with_status(0).with_stderr_contains(&format!("\ [UPDATING] registry `{reg}` [WARNING] manifest has no documentation, [..] See [..] @@ -371,7 +371,7 @@ fn dry_run() { assert_that(p.cargo_process("publish").arg("--dry-run") .arg("--host").arg(registry().to_string()), - execs().with_status(0).with_stderr(&format!("\ + execs().with_status(0).with_stderr_contains(&format!("\ [UPDATING] registry `[..]` [WARNING] manifest has no documentation, [..] See [..] From 7c515a0c4e73b504f51ffb546f9ab8ba525b9170 Mon Sep 17 00:00:00 2001 From: Evgen Druzhynin Date: Wed, 17 May 2017 16:15:11 +0300 Subject: [PATCH 2/6] Update login paragraph in crates-io docs. --- src/doc/crates-io.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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. From 903be77ae802f0db84d128e6ec5a9cfd42517108 Mon Sep 17 00:00:00 2001 From: Evgen Druzhynin Date: Sat, 27 May 2017 12:33:12 +0300 Subject: [PATCH 3/6] Remove warnings about old configuration on publish. --- src/cargo/ops/registry.rs | 8 -------- src/cargo/util/config.rs | 28 ---------------------------- tests/publish.rs | 4 ++-- 3 files changed, 2 insertions(+), 38 deletions(-) diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 7bf9a0f08f7..78681a6904e 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -66,14 +66,6 @@ pub fn publish(ws: &Workspace, opts: &PublishOpts) -> CargoResult<()> { opts.config.shell().status("Uploading", pkg.package_id().to_string())?; transmit(opts.config, pkg, tarball.file(), &mut registry, opts.dry_run)?; - if opts.config.is_token_in_main_config() { - let _ = opts.config - .shell() - .warn("API token detected in ~/.cargo/config under `registry.token`.\n \ - You should remove it and do `login` again in order to \ - save token in ~/.cargo/credentials"); - } - Ok(()) } diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index c99ec82c661..b99285ce7a1 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -34,10 +34,6 @@ pub struct Config { extra_verbose: Cell, frozen: Cell, locked: Cell, - - // A temporary solution to point on an old configuration's usage. - // If it's true cargo will warn on it on publish. - token_in_main_config: Cell, } impl Config { @@ -55,7 +51,6 @@ impl Config { extra_verbose: Cell::new(false), frozen: Cell::new(false), locked: Cell::new(false), - token_in_main_config: Cell::new(false), } } @@ -408,10 +403,6 @@ impl Config { !self.frozen.get() && !self.locked.get() } - pub fn is_token_in_main_config(&self) -> bool { - self.token_in_main_config.get() - } - pub fn load_values(&self) -> CargoResult> { let mut cfg = CV::Table(HashMap::new(), PathBuf::from(".")); @@ -438,8 +429,6 @@ impl Config { Ok(()) }).chain_err(|| "Couldn't load Cargo configuration")?; - self.token_in_main_config.set(check_token_in_main_config("registry.token".into(), &cfg)); - let mut map = match cfg { CV::Table(map, _) => map, _ => unreachable!(), @@ -823,20 +812,3 @@ fn load_credentials(home: &PathBuf) -> CargoResult> { Ok(Some(token.trim().into())) } - -fn check_token_in_main_config(key: &str, cfg: &ConfigValue) -> bool { - let mut keys = key.split('.'); - let k = keys.next().unwrap(); - let keys: String = keys.map(String::from).collect(); - - match *cfg { - CV::Table(ref map, _) => { - match map.get(k) { - Some(ref v) => check_token_in_main_config(keys.as_str(), v), - None => return false, - } - } - CV::String(ref v, _) => !v.is_empty(), - _ => return false, - } -} diff --git a/tests/publish.rs b/tests/publish.rs index f514adf9c84..968281f6219 100644 --- a/tests/publish.rs +++ b/tests/publish.rs @@ -57,7 +57,7 @@ fn simple() { assert_that(p.cargo_process("publish").arg("--no-verify") .arg("--host").arg(registry().to_string()), - execs().with_status(0).with_stderr_contains(&format!("\ + execs().with_status(0).with_stderr(&format!("\ [UPDATING] registry `{reg}` [WARNING] manifest has no documentation, [..] See [..] @@ -371,7 +371,7 @@ fn dry_run() { assert_that(p.cargo_process("publish").arg("--dry-run") .arg("--host").arg(registry().to_string()), - execs().with_status(0).with_stderr_contains(&format!("\ + execs().with_status(0).with_stderr(&format!("\ [UPDATING] registry `[..]` [WARNING] manifest has no documentation, [..] See [..] From 29960d2528e65208dacef21252556a2415f87ad7 Mon Sep 17 00:00:00 2001 From: Evgen Druzhynin Date: Thu, 1 Jun 2017 16:59:53 +0300 Subject: [PATCH 4/6] Credentials for multiple hosts. Now cargo looks for credentials and stores them in $CARGO_HOME/credentials. If credentials for requested host are not found cargo will try to get them from $CARGO_HOME/config, but it's temporary behavior. It should be disabled after users start to use a new config file). --- src/bin/login.rs | 22 +++--- src/cargo/core/source.rs | 2 +- src/cargo/ops/registry.rs | 60 ++++++++++++---- src/cargo/util/config.rs | 113 ++++++++++++++++++++---------- tests/login.rs | 143 ++++++++++++++++++++++++++++++++++++++ tests/registry.rs | 12 ++-- 6 files changed, 290 insertions(+), 62 deletions(-) create mode 100644 tests/login.rs 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)); } From aa17b61e5fc68c14b155a8a3841babc6bd1265bf Mon Sep 17 00:00:00 2001 From: Evgen Druzhynin Date: Fri, 9 Jun 2017 13:14:19 +0300 Subject: [PATCH 5/6] Revert "Credentials for multiple hosts." This reverts commit 29960d2528e65208dacef21252556a2415f87ad7. --- src/bin/login.rs | 22 +++--- src/cargo/core/source.rs | 2 +- src/cargo/ops/registry.rs | 60 ++++------------ src/cargo/util/config.rs | 113 ++++++++++-------------------- tests/login.rs | 143 -------------------------------------- tests/registry.rs | 12 ++-- 6 files changed, 62 insertions(+), 290 deletions(-) delete mode 100644 tests/login.rs diff --git a/src/bin/login.rs b/src/bin/login.rs index 95d0052bfe0..6e7841717b6 100644 --- a/src/bin/login.rs +++ b/src/bin/login.rs @@ -40,30 +40,26 @@ pub fn execute(options: Options, config: &Config) -> CliResult { &options.flag_color, options.flag_frozen, options.flag_locked)?; - - let host = match options.flag_host { - Some(host) => host, + let token = match options.arg_token.clone() { + Some(token) => token, None => { let src = SourceId::crates_io(config)?; let mut src = RegistrySource::remote(&src, config); src.update()?; - 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 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); let mut line = String::new(); let input = io::stdin(); input.lock().read_line(&mut line).chain_err(|| { "failed to read stdin" })?; - line.trim().to_string() + line } }; - ops::registry_login(config, token, host)?; + let token = token.trim().to_string(); + ops::registry_login(config, token)?; Ok(()) } + diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs index f0073fe0913..9f32d692dfe 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, "https://crates.io")?; + let cfg = ops::registry_configuration(config)?; 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 5b457e48ca1..78681a6904e 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -177,39 +177,25 @@ fn transmit(config: &Config, } } -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_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(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(|| { @@ -217,12 +203,6 @@ 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)) } @@ -301,31 +281,15 @@ 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, - 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)?; - +pub fn registry_login(config: &Config, token: String) -> CargoResult<()> { + let RegistryConfig { index: _, token: old_token } = registry_configuration(config)?; if let Some(old_token) = old_token { if old_token == token { return Ok(()); } } - config::save_credentials(config, token, host) + config::save_credentials(config, token) } pub struct OwnersOptions { diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index a4e94b08558..b99285ce7a1 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -5,7 +5,6 @@ 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}; @@ -430,52 +429,28 @@ impl Config { Ok(()) }).chain_err(|| "Couldn't load Cargo configuration")?; - self.load_credentials(&mut cfg)?; - match cfg { - CV::Table(map, _) => Ok(map), + let mut map = match cfg { + CV::Table(map, _) => 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 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 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(()) + Ok(map) } /// Look for a path for `tool` in an environment variable or config path, but return `None` @@ -592,21 +567,6 @@ 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(..)) | @@ -807,32 +767,15 @@ fn walk_tree(pwd: &Path, mut walk: F) -> CargoResult<()> } pub fn save_credentials(cfg: &Config, - token: String, - host: String) -> CargoResult<()> { + 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 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)?; + file.write_all(token.as_bytes())?; + file.file().set_len(token.len() as u64)?; set_permissions(file.file(), 0o600)?; return Ok(()); @@ -853,3 +796,19 @@ 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 deleted file mode 100644 index c0467d66317..00000000000 --- a/tests/login.rs +++ /dev/null @@ -1,143 +0,0 @@ -#[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 5983cdd95e0..b336827abbc 100644 --- a/tests/registry.rs +++ b/tests/registry.rs @@ -581,8 +581,7 @@ 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("--host").arg(registry().to_string()) - .arg("foo").arg("-v"), + assert_that(cargo_process().arg("login").arg("foo").arg("-v"), execs().with_status(0)); } @@ -591,14 +590,11 @@ 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("--host").arg(registry().to_string()) - .arg("lmaolmaolmao").arg("-v"), + assert_that(cargo_process().arg("login").arg("lmaolmaolmao").arg("-v"), execs().with_status(0)); - assert_that(cargo_process().arg("login").arg("--host").arg(registry().to_string()) - .arg("lmao").arg("-v"), + assert_that(cargo_process().arg("login").arg("lmao").arg("-v"), execs().with_status(0)); - assert_that(cargo_process().arg("login").arg("--host").arg(registry().to_string()) - .arg("lmaolmaolmao").arg("-v"), + assert_that(cargo_process().arg("login").arg("lmaolmaolmao").arg("-v"), execs().with_status(0)); } From 1ccdc8bb1d52eb2139163ee33dca7f5cd86d46cf Mon Sep 17 00:00:00 2001 From: Evgen Druzhynin Date: Fri, 9 Jun 2017 13:07:06 +0300 Subject: [PATCH 6/6] Update a credentials file format. Implement tests for login command. --- src/cargo/util/config.rs | 105 ++++++++++++++++++++++++------------ tests/login.rs | 112 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 35 deletions(-) create mode 100644 tests/login.rs diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index b99285ce7a1..fe828c10724 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,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> { - 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..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())); +}