Skip to content

Commit

Permalink
Don't suggest cargo login when using incompatible credental provide…
Browse files Browse the repository at this point in the history
…rs (#15124)

Authentication errors shouldn't unconditionally recommend running `cargo
login`, because alternative registries configured to use custom
credential providers won't use the token obtained by `cargo login`.
  • Loading branch information
weihanglo authored Jan 30, 2025
2 parents 51346ad + aa6c48f commit 776129a
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 12 deletions.
35 changes: 23 additions & 12 deletions src/cargo/util/auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,8 @@ pub struct AuthorizationError {
pub login_url: Option<Url>,
/// Specific reason indicating what failed
reason: AuthorizationErrorReason,
/// Should the _TOKEN environment variable name be included when displaying this error?
display_token_env_help: bool,
/// Should `cargo login` and the `_TOKEN` env var be included when displaying this error?
supports_cargo_token_credential_provider: bool,
}

impl AuthorizationError {
Expand All @@ -409,15 +409,16 @@ impl AuthorizationError {
// Only display the _TOKEN environment variable suggestion if the `cargo:token` credential
// provider is available for the source. Otherwise setting the environment variable will
// have no effect.
let display_token_env_help = credential_provider(gctx, &sid, false, false)?
.iter()
.any(|p| p.first().map(String::as_str) == Some("cargo:token"));
let supports_cargo_token_credential_provider =
credential_provider(gctx, &sid, false, false)?
.iter()
.any(|p| p.first().map(String::as_str) == Some("cargo:token"));
Ok(AuthorizationError {
sid,
default_registry: gctx.default_registry()?,
login_url,
reason,
display_token_env_help,
supports_cargo_token_credential_provider,
})
}
}
Expand All @@ -432,20 +433,30 @@ impl fmt::Display for AuthorizationError {
""
};
write!(f, "{}, please run `cargo login{args}`", self.reason)?;
if self.display_token_env_help {
if self.supports_cargo_token_credential_provider {
write!(f, "\nor use environment variable CARGO_REGISTRY_TOKEN")?;
}
Ok(())
} else if let Some(name) = self.sid.alt_registry_key() {
let key = ConfigKey::from_str(&format!("registries.{name}.token"));
write!(
f,
"{} for `{}`, please run `cargo login --registry {name}`",
"{} for `{}`",
self.reason,
self.sid.display_registry_name(),
self.sid.display_registry_name()
)?;
if self.display_token_env_help {
write!(f, "\nor use environment variable {}", key.as_env_key())?;
if self.supports_cargo_token_credential_provider {
let key = ConfigKey::from_str(&format!("registries.{name}.token"));
write!(
f,
", please run `cargo login --registry {name}`\n\
or use environment variable {}",
key.as_env_key()
)?;
} else {
write!(
f,
"\nYou may need to log in using this registry's credential provider"
)?;
}
Ok(())
} else if self.reason == AuthorizationErrorReason::TokenMissing {
Expand Down
25 changes: 25 additions & 0 deletions tests/testsuite/credential_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,31 @@ You may press ctrl-c [..]
.run();
}

#[cargo_test]
fn credential_provider_auth_failure() {
let _reg = registry::RegistryBuilder::new()
.http_index()
.auth_required()
.alternative()
.no_configure_token()
.credential_provider(&["cargo:token-from-stdout", "true"])
.build();

cargo_process("install libc --registry=alternative")
.with_status(101)
.with_stderr_data(str![[r#"
[UPDATING] `alternative` index
[ERROR] token rejected for `alternative`
You may need to log in using this registry's credential provider
Caused by:
failed to get successful HTTP response from [..]
body:
[..]
"#]])
.run();
}

#[cargo_test]
fn basic_unsupported() {
// Non-action commands don't support login/logout.
Expand Down

0 comments on commit 776129a

Please sign in to comment.