Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use registry.default for login/logout #11949

Merged
merged 4 commits into from
Apr 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/bin/cargo/commands/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,11 @@ pub fn cli() -> Command {
}

pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
let registry = args.registry(config)?;
ops::registry_login(
config,
args.get_one::<String>("token").map(|s| s.as_str().into()),
args.get_one("registry").map(String::as_str),
registry.as_deref(),
args.flag("generate-keypair"),
args.flag("secret-key"),
args.get_one("key-subject").map(String::as_str),
Expand Down
6 changes: 2 additions & 4 deletions src/bin/cargo/commands/logout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
.cli_unstable()
.fail_if_stable_command(config, "logout", 8933)?;
}
ops::registry_logout(
config,
args.get_one::<String>("registry").map(String::as_str),
)?;
let registry = args.registry(config)?;
ops::registry_logout(config, registry.as_deref())?;
Ok(())
}
1 change: 1 addition & 0 deletions src/cargo/sources/registry/http_remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
if self.auth_required {
return Poll::Ready(err.context(auth::AuthorizationError {
sid: self.source_id.clone(),
default_registry: self.config.default_registry()?,
login_url: self.login_url.clone(),
reason: auth::AuthorizationErrorReason::TokenRejected,
}));
Expand Down
10 changes: 9 additions & 1 deletion src/cargo/util/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,8 @@ impl fmt::Display for AuthorizationErrorReason {
pub struct AuthorizationError {
/// Url that was attempted
pub sid: SourceId,
/// The `registry.default` config value.
pub default_registry: Option<String>,
/// Url where the user could log in.
pub login_url: Option<Url>,
/// Specific reason indicating what failed
Expand All @@ -356,9 +358,14 @@ impl Error for AuthorizationError {}
impl fmt::Display for AuthorizationError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if self.sid.is_crates_io() {
let args = if self.default_registry.is_some() {
" --registry crates-io"
} else {
""
};
write!(
f,
"{}, please run `cargo login`\nor use environment variable CARGO_REGISTRY_TOKEN",
"{}, please run `cargo login{args}`\nor use environment variable CARGO_REGISTRY_TOKEN",
self.reason
)
} else if let Some(name) = self.sid.alt_registry_key() {
Expand Down Expand Up @@ -421,6 +428,7 @@ pub fn auth_token(
Some(token) => Ok(token.expose()),
None => Err(AuthorizationError {
sid: sid.clone(),
default_registry: config.default_registry()?,
login_url: login_url.cloned(),
reason: AuthorizationErrorReason::TokenMissing,
}
Expand Down
57 changes: 46 additions & 11 deletions tests/testsuite/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@ fn setup_new_credentials_at(config: PathBuf) {
));
}

fn check_token(expected_token: &str, registry: Option<&str>) -> bool {
/// Asserts whether or not the token is set to the given value for the given registry.
pub fn check_token(expected_token: Option<&str>, registry: Option<&str>) {
let credentials = credentials_toml();
assert!(credentials.is_file());

let contents = fs::read_to_string(&credentials).unwrap();
let toml: toml::Table = contents.parse().unwrap();

let token = match registry {
let actual_token = match registry {
// A registry has been provided, so check that the token exists in a
// table for the registry.
Some(registry) => toml
Expand All @@ -54,10 +55,15 @@ fn check_token(expected_token: &str, registry: Option<&str>) -> bool {
}),
};

if let Some(token_val) = token {
token_val == expected_token
} else {
false
match (actual_token, expected_token) {
(None, None) => {}
(Some(actual), Some(expected)) => assert_eq!(actual, expected),
(None, Some(expected)) => {
panic!("expected `{registry:?}` to be `{expected}`, but was not set")
}
(Some(actual), None) => {
panic!("expected `{registry:?}` to be unset, but was set to `{actual}`")
}
}
}

Expand All @@ -75,10 +81,10 @@ fn registry_credentials() {
cargo_process("login --registry").arg(reg).arg(TOKEN).run();

// Ensure that we have not updated the default token
assert!(check_token(ORIGINAL_TOKEN, None));
check_token(Some(ORIGINAL_TOKEN), None);

// Also ensure that we get the new token for the registry
assert!(check_token(TOKEN, Some(reg)));
check_token(Some(TOKEN), Some(reg));

let reg2 = "alternative2";
cargo_process("login --registry")
Expand All @@ -88,9 +94,9 @@ fn registry_credentials() {

// Ensure not overwriting 1st alternate registry token with
// 2nd alternate registry token (see rust-lang/cargo#7701).
assert!(check_token(ORIGINAL_TOKEN, None));
assert!(check_token(TOKEN, Some(reg)));
assert!(check_token(TOKEN2, Some(reg2)));
check_token(Some(ORIGINAL_TOKEN), None);
check_token(Some(TOKEN), Some(reg));
check_token(Some(TOKEN2), Some(reg2));
}

#[cargo_test]
Expand Down Expand Up @@ -362,3 +368,32 @@ fn login_with_generate_asymmetric_token() {
let credentials = fs::read_to_string(&credentials).unwrap();
assert!(credentials.contains("secret-key = \"k3.secret."));
}

#[cargo_test]
fn default_registry_configured() {
// When registry.default is set, login should use that one when
// --registry is not used.
let _alternative = RegistryBuilder::new().alternative().build();
let cargo_home = paths::home().join(".cargo");
cargo_util::paths::append(
&cargo_home.join("config"),
br#"
[registry]
default = "alternative"
"#,
)
.unwrap();

cargo_process("login")
.arg("a-new-token")
.with_stderr(
"\
[UPDATING] `alternative` index
[LOGIN] token for `alternative` saved
",
)
.run();

check_token(None, None);
check_token(Some("a-new-token"), Some("alternative"));
}
88 changes: 59 additions & 29 deletions tests/testsuite/logout.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
//! Tests for the `cargo logout` command.

use cargo_test_support::install::cargo_home;
use super::login::check_token;
use cargo_test_support::paths::{self, CargoPathExt};
use cargo_test_support::registry::TestRegistry;
use cargo_test_support::{cargo_process, registry};
use std::fs;

#[cargo_test]
fn gated() {
Expand All @@ -21,32 +21,9 @@ the `cargo logout` command.
.run();
}

/// Checks whether or not the token is set for the given token.
fn check_config_token(registry: Option<&str>, should_be_set: bool) {
let credentials = cargo_home().join("credentials.toml");
let contents = fs::read_to_string(&credentials).unwrap();
let toml: toml::Table = contents.parse().unwrap();
if let Some(registry) = registry {
assert_eq!(
toml.get("registries")
.and_then(|registries| registries.get(registry))
.and_then(|registry| registry.get("token"))
.is_some(),
should_be_set
);
} else {
assert_eq!(
toml.get("registry")
.and_then(|registry| registry.get("token"))
.is_some(),
should_be_set
);
}
}

fn simple_logout_test(registry: &TestRegistry, reg: Option<&str>, flag: &str, note: &str) {
let msg = reg.unwrap_or("crates-io");
check_config_token(reg, true);
check_token(Some(registry.token()), reg);
let mut cargo = cargo_process(&format!("logout -Z unstable-options {}", flag));
if reg.is_none() {
cargo.replace_crates_io(registry.index_url());
Expand All @@ -61,7 +38,7 @@ If you need to revoke the token, visit {note} and follow the instructions there.
"
))
.run();
check_config_token(reg, false);
check_token(None, reg);

let mut cargo = cargo_process(&format!("logout -Z unstable-options {}", flag));
if reg.is_none() {
Expand All @@ -71,11 +48,11 @@ If you need to revoke the token, visit {note} and follow the instructions there.
.masquerade_as_nightly_cargo(&["cargo-logout"])
.with_stderr(&format!("[LOGOUT] not currently logged in to `{msg}`"))
.run();
check_config_token(reg, false);
check_token(None, reg);
}

#[cargo_test]
fn default_registry() {
fn default_registry_unconfigured() {
let registry = registry::init();
simple_logout_test(&registry, None, "", "<https://crates.io/me>");
}
Expand All @@ -89,4 +66,57 @@ fn other_registry() {
"--registry alternative",
"the `alternative` website",
);
// It should not touch crates.io.
check_token(Some("sekrit"), None);
}

#[cargo_test]
fn default_registry_configured() {
// When registry.default is set, logout should use that one when
// --registry is not used.
let cargo_home = paths::home().join(".cargo");
cargo_home.mkdir_p();
cargo_util::paths::write(
&cargo_home.join("config.toml"),
r#"
[registry]
default = "dummy-registry"

[registries.dummy-registry]
index = "https://127.0.0.1/index"
"#,
)
.unwrap();
cargo_util::paths::write(
&cargo_home.join("credentials.toml"),
r#"
[registry]
token = "crates-io-token"

[registries.dummy-registry]
token = "dummy-token"
"#,
)
.unwrap();
check_token(Some("dummy-token"), Some("dummy-registry"));
check_token(Some("crates-io-token"), None);

cargo_process("logout -Zunstable-options")
.masquerade_as_nightly_cargo(&["cargo-logout"])
.with_stderr(
"\
[LOGOUT] token for `dummy-registry` has been removed from local storage
[NOTE] This does not revoke the token on the registry server.
If you need to revoke the token, visit the `dummy-registry` website \
and follow the instructions there.
",
)
.run();
check_token(None, Some("dummy-registry"));
check_token(Some("crates-io-token"), None);

cargo_process("logout -Zunstable-options")
.masquerade_as_nightly_cargo(&["cargo-logout"])
.with_stderr("[LOGOUT] not currently logged in to `dummy-registry`")
.run();
}
82 changes: 82 additions & 0 deletions tests/testsuite/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3192,3 +3192,85 @@ required by package `foo v0.0.1 ([ROOT]/foo)`
]
);
}

#[cargo_test]
fn default_auth_error() {
// Check for the error message for an authentication error when default is set.
let crates_io = RegistryBuilder::new().http_api().build();
let _alternative = RegistryBuilder::new().http_api().alternative().build();

paths::home().join(".cargo/credentials.toml").rm_rf();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
license = "MIT"
description = "foo"
"#,
)
.file("src/lib.rs", "")
.build();

// Test output before setting the default.
p.cargo("publish --no-verify")
.replace_crates_io(crates_io.index_url())
.with_stderr(
"\
[UPDATING] crates.io index
error: no token found, please run `cargo login`
or use environment variable CARGO_REGISTRY_TOKEN
",
)
.with_status(101)
.run();

p.cargo("publish --no-verify --registry alternative")
.replace_crates_io(crates_io.index_url())
.with_stderr(
"\
[UPDATING] `alternative` index
error: no token found for `alternative`, please run `cargo login --registry alternative`
or use environment variable CARGO_REGISTRIES_ALTERNATIVE_TOKEN
",
)
.with_status(101)
.run();

// Test the output with the default.
cargo_util::paths::append(
&cargo_home().join("config"),
br#"
[registry]
default = "alternative"
"#,
)
.unwrap();

p.cargo("publish --no-verify")
.replace_crates_io(crates_io.index_url())
.with_stderr(
"\
[UPDATING] `alternative` index
error: no token found for `alternative`, please run `cargo login --registry alternative`
or use environment variable CARGO_REGISTRIES_ALTERNATIVE_TOKEN
",
)
.with_status(101)
.run();

p.cargo("publish --no-verify --registry crates-io")
.replace_crates_io(crates_io.index_url())
.with_stderr(
"\
[UPDATING] crates.io index
error: no token found, please run `cargo login --registry crates-io`
or use environment variable CARGO_REGISTRY_TOKEN
",
)
.with_status(101)
.run();
}
2 changes: 1 addition & 1 deletion tests/testsuite/registry_auth.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Tests for normal registry dependencies.
//! Tests for registry authentication.

use cargo_test_support::registry::{Package, RegistryBuilder};
use cargo_test_support::{project, Execs, Project};
Expand Down