Auto merge of #12644 - arlosi:cred-no-env, r=weihanglo

fix: don't print _TOKEN suggestion when not applicable

### What does this PR try to resolve?
When a registry token cannot be found, or a token is invalid, cargo displays an error recommending either `cargo login` or the appropriate environment variable.

For example:
```
error: no token found for `alternative`, please run `cargo login --registry alternative`
or use environment variable CARGO_REGISTRIES_ALTERNATIVE_TOKEN
```

With `-Z credential-process`, if `cargo:token` is not in `registry.global-credential-providers` or `registries.<NAME>.credential-provider` the suggested environment variable will not work.

Fixes #12642 by not including the `_TOKEN` environment variable if `cargo:token` is not enabled as a credential provider.

### How should we test and review this PR?

Two tests `not_found` and `all_not_found` cover this case by having a registry-specific and global credential providers respectively that return `not-found`.

Other tests that emit this error are not affected, since they still have the `cargo:token` provider available.

Author: bors

src/cargo/sources/registry/http_remote.rs

@@ -588,12 +588,13 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
                     }
                     .into());
                     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,
-                        }));
+                        let auth_error = auth::AuthorizationError::new(
+                            self.config,
+                            self.source_id,
+                            self.login_url.clone(),
+                            auth::AuthorizationErrorReason::TokenRejected,
+                        )?;
+                        return Poll::Ready(err.context(auth_error));
                     } else {
                         return Poll::Ready(err);
                     }

src/cargo/util/auth/mod.rs

@@ -74,7 +74,19 @@ impl RegistryConfigExtended {
 }
 
 /// Get the list of credential providers for a registry source.
-fn credential_provider(config: &Config, sid: &SourceId) -> CargoResult<Vec<Vec<String>>> {
+fn credential_provider(
+    config: &Config,
+    sid: &SourceId,
+    show_warnings: bool,
+) -> CargoResult<Vec<Vec<String>>> {
+    let warn = |message: String| {
+        if show_warnings {
+            config.shell().warn(message)
+        } else {
+            Ok(())
+        }
+    };
+
     let cfg = registry_credential_config_raw(config, sid)?;
     let default_providers = || {
         if config.cli_unstable().asymmetric_token {
@@ -111,7 +123,7 @@ fn credential_provider(config: &Config, sid: &SourceId) -> CargoResult<Vec<Vec<S
             let provider = resolve_credential_alias(config, provider);
             if let Some(token) = token {
                 if provider[0] != "cargo:token" {
-                    config.shell().warn(format!(
+                    warn(format!(
                         "{sid} has a token configured in {} that will be ignored \
                         because this registry is configured to use credential-provider `{}`",
                         token.definition, provider[0],
@@ -120,7 +132,7 @@ fn credential_provider(config: &Config, sid: &SourceId) -> CargoResult<Vec<Vec<S
             }
             if let Some(secret_key) = secret_key {
                 if provider[0] != "cargo:paseto" {
-                    config.shell().warn(format!(
+                    warn(format!(
                         "{sid} has a secret-key configured in {} that will be ignored \
                         because this registry is configured to use credential-provider `{}`",
                         secret_key.definition, provider[0],
@@ -145,14 +157,14 @@ fn credential_provider(config: &Config, sid: &SourceId) -> CargoResult<Vec<Vec<S
             match (token_pos, paseto_pos) {
                 (Some(token_pos), Some(paseto_pos)) => {
                     if token_pos < paseto_pos {
-                        config.shell().warn(format!(
+                        warn(format!(
                             "{sid} has a `secret_key` configured in {} that will be ignored \
                         because a `token` is also configured, and the `cargo:token` provider is \
                         configured with higher precedence",
                             secret_key.definition
                         ))?;
                     } else {
-                        config.shell().warn(format!("{sid} has a `token` configured in {} that will be ignored \
+                        warn(format!("{sid} has a `token` configured in {} that will be ignored \
                         because a `secret_key` is also configured, and the `cargo:paseto` provider is \
                         configured with higher precedence", token.definition))?;
                     }
@@ -172,7 +184,7 @@ fn credential_provider(config: &Config, sid: &SourceId) -> CargoResult<Vec<Vec<S
                 .iter()
                 .any(|p| p.first().map(String::as_str) == Some("cargo:token"))
             {
-                config.shell().warn(format!(
+                warn(format!(
                     "{sid} has a token configured in {} that will be ignored \
                     because the `cargo:token` credential provider is not listed in \
                     `registry.global-credential-providers`",
@@ -191,7 +203,7 @@ fn credential_provider(config: &Config, sid: &SourceId) -> CargoResult<Vec<Vec<S
                 .iter()
                 .any(|p| p.first().map(String::as_str) == Some("cargo:paseto"))
             {
-                config.shell().warn(format!(
+                warn(format!(
                     "{sid} has a secret-key configured in {} that will be ignored \
                     because the `cargo:paseto` credential provider is not listed in \
                     `registry.global-credential-providers`",
@@ -360,14 +372,40 @@ impl fmt::Display for AuthorizationErrorReason {
 #[derive(Debug)]
 pub struct AuthorizationError {
     /// Url that was attempted
-    pub sid: SourceId,
+    sid: SourceId,
     /// The `registry.default` config value.
-    pub default_registry: Option<String>,
+    default_registry: Option<String>,
     /// Url where the user could log in.
     pub login_url: Option<Url>,
     /// Specific reason indicating what failed
-    pub reason: AuthorizationErrorReason,
+    reason: AuthorizationErrorReason,
+    /// Should the _TOKEN environment variable name be included when displaying this error?
+    display_token_env_help: bool,
 }
+
+impl AuthorizationError {
+    pub fn new(
+        config: &Config,
+        sid: SourceId,
+        login_url: Option<Url>,
+        reason: AuthorizationErrorReason,
+    ) -> CargoResult<Self> {
+        // 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(config, &sid, false)?
+            .iter()
+            .any(|p| p.first().map(String::as_str) == Some("cargo:token"));
+        Ok(AuthorizationError {
+            sid,
+            default_registry: config.default_registry()?,
+            login_url,
+            reason,
+            display_token_env_help,
+        })
+    }
+}
+
 impl Error for AuthorizationError {}
 impl fmt::Display for AuthorizationError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -377,20 +415,23 @@ impl fmt::Display for AuthorizationError {
             } else {
                 ""
             };
-            write!(
-                f,
-                "{}, please run `cargo login{args}`\nor use environment variable CARGO_REGISTRY_TOKEN",
-                self.reason
-            )
+            write!(f, "{}, please run `cargo login{args}`", self.reason)?;
+            if self.display_token_env_help {
+                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}`\nor use environment variable {}",
+                "{} for `{}`, please run `cargo login --registry {name}`",
                 self.reason,
                 self.sid.display_registry_name(),
-                key.as_env_key(),
-            )
+            )?;
+            if self.display_token_env_help {
+                write!(f, "\nor use environment variable {}", key.as_env_key())?;
+            }
+            Ok(())
         } else if self.reason == AuthorizationErrorReason::TokenMissing {
             write!(
                 f,
@@ -416,7 +457,7 @@ my-registry = {{ index = "{}" }}
     }
 }
 
-// Store a token in the cache for future calls.
+/// Store a token in the cache for future calls.
 pub fn cache_token_from_commandline(config: &Config, sid: &SourceId, token: Secret<&str>) {
     let url = sid.canonical_url();
     config.credential_cache().insert(
@@ -446,7 +487,7 @@ fn credential_action(
         name,
         headers,
     };
-    let providers = credential_provider(config, sid)?;
+    let providers = credential_provider(config, sid, true)?;
     let mut any_not_found = false;
     for provider in providers {
         let args: Vec<&str> = provider
@@ -511,12 +552,12 @@ pub fn auth_token(
 ) -> CargoResult<String> {
     match auth_token_optional(config, sid, operation, headers)? {
         Some(token) => Ok(token.expose()),
-        None => Err(AuthorizationError {
-            sid: sid.clone(),
-            default_registry: config.default_registry()?,
-            login_url: login_url.cloned(),
-            reason: AuthorizationErrorReason::TokenMissing,
-        }
+        None => Err(AuthorizationError::new(
+            config,
+            *sid,
+            login_url.cloned(),
+            AuthorizationErrorReason::TokenMissing,
+        )?
         .into()),
     }
 }

tests/testsuite/credential_process.rs

@@ -320,6 +320,36 @@ fn build_provider(name: &str, response: &str) -> String {
     toml_bin(&cred_proj, name)
 }
 
+#[cargo_test]
+fn not_found() {
+    let registry = registry::RegistryBuilder::new()
+        .no_configure_token()
+        .http_index()
+        .auth_required()
+        .credential_provider(&[&build_provider(
+            "not_found",
+            r#"{"Err": {"kind": "not-found"}}"#,
+        )])
+        .build();
+
+    // should not suggest a _TOKEN environment variable since the cargo:token provider isn't available.
+    cargo_process("install -v foo -Zcredential-process -Zregistry-auth")
+        .masquerade_as_nightly_cargo(&["credential-process", "registry-auth"])
+        .replace_crates_io(registry.index_url())
+        .with_status(101)
+        .with_stderr(
+            r#"[UPDATING] [..]
+[CREDENTIAL] [..]not_found[..] get crates-io
+{"v":1[..]
+[ERROR] failed to query replaced source registry `crates-io`
+
+Caused by:
+  no token found, please run `cargo login`
+"#,
+        )
+        .run();
+}
+
 #[cargo_test]
 fn all_not_found() {
     let server = registry::RegistryBuilder::new()
@@ -342,6 +372,7 @@ fn all_not_found() {
     )
     .unwrap();
 
+    // should not suggest a _TOKEN environment variable since the cargo:token provider isn't available.
     cargo_process("install -v foo -Zcredential-process -Zregistry-auth")
         .masquerade_as_nightly_cargo(&["credential-process", "registry-auth"])
         .replace_crates_io(server.index_url())
@@ -354,7 +385,6 @@ fn all_not_found() {
 
 Caused by:
   no token found, please run `cargo login`
-  or use environment variable CARGO_REGISTRY_TOKEN
 "#,
         )
         .run();