Skip to content

Commit

Permalink
Load credentials only when needed
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
giraffate committed Jan 7, 2020
1 parent 6e1ca92 commit 4b70f14
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 28 deletions.
2 changes: 2 additions & 0 deletions src/bin/cargo/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
29 changes: 24 additions & 5 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,6 @@ impl Config {
})
.chain_err(|| "could not load Cargo configuration")?;

self.load_credentials(&mut cfg)?;
match cfg {
CV::Table(map, _) => Ok(map),
_ => unreachable!(),
Expand Down Expand Up @@ -979,9 +978,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,
Expand All @@ -1006,7 +1004,28 @@ impl Config {
}
}

cfg.merge(value, true)?;
if let CV::Table(mut map, _) = value {
if map.contains_key("registry") {
if let Some(mut new_map) = self.values_mut()?.remove("registry") {
let token = map.remove("registry").unwrap();
new_map.merge(token, true)?;
self.values_mut()?.insert("registry".into(), new_map);
} else {
self.values_mut()?
.insert("registry".into(), map.remove("registry").unwrap());
}
}
if map.contains_key("registries") {
if let Some(mut new_map) = self.values_mut()?.remove("registries") {
let token = map.remove("registries").unwrap();
new_map.merge(token, true)?;
self.values_mut()?.insert("registries".into(), new_map);
} else {
self.values_mut()?
.insert("registries".into(), map.remove("registries").unwrap());
}
}
}

Ok(())
}
Expand Down
26 changes: 26 additions & 0 deletions tests/testsuite/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
26 changes: 3 additions & 23 deletions tests/testsuite/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
37 changes: 37 additions & 0 deletions tests/testsuite/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1261,3 +1261,40 @@ repository = "foo"
)],
);
}

#[cargo_test]
fn credentials_ambiguous_filename() {
registry::init();

let credentials_toml = paths::home().join(".cargo/credentials.toml");
File::create(&credentials_toml)
.unwrap()
.write_all(br#"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();
}

0 comments on commit 4b70f14

Please sign in to comment.