Auto merge of #10471 - Eh2406:credential_process, r=weihanglo

Use types to make clere (credential process || token)

`RegistryConfig` represents what credentials we have read from disk. It was a
```
   token: Option<String>,
   credential_process: Option<(PathBuf, Vec<String>)>,
```
with runtime checks that they were not both `Some`.
This changes it to be an Enum `None|Token|Process`.
There is somewhat more code, but I think this is clearer.

This also changed some `Option<String>` arguments to `Option<&str>`.

Author: bors

src/cargo/ops/registry.rs

@@ -36,13 +36,41 @@ mod auth;
 ///
 /// This is loaded based on the `--registry` flag and the config settings.
 #[derive(Debug)]
-pub struct RegistryConfig {
-    /// The index URL. If `None`, use crates.io.
-    pub index: Option<String>,
+pub enum RegistryConfig {
+    None,
     /// The authentication token.
-    pub token: Option<String>,
+    Token(String),
     /// Process used for fetching a token.
-    pub credential_process: Option<(PathBuf, Vec<String>)>,
+    Process((PathBuf, Vec<String>)),
+}
+
+impl RegistryConfig {
+    /// Returns `true` if the credential is [`None`].
+    ///
+    /// [`None`]: Credential::None
+    pub fn is_none(&self) -> bool {
+        matches!(self, Self::None)
+    }
+    /// Returns `true` if the credential is [`Token`].
+    ///
+    /// [`Token`]: Credential::Token
+    pub fn is_token(&self) -> bool {
+        matches!(self, Self::Token(..))
+    }
+    pub fn as_token(&self) -> Option<&str> {
+        if let Self::Token(v) = self {
+            Some(&*v)
+        } else {
+            None
+        }
+    }
+    pub fn as_process(&self) -> Option<&(PathBuf, Vec<String>)> {
+        if let Self::Process(v) = self {
+            Some(v)
+        } else {
+            None
+        }
+    }
 }
 
 pub struct PublishOpts<'cfg> {
@@ -98,8 +126,8 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
     let (mut registry, _reg_cfg, reg_id) = registry(
         opts.config,
         opts.token.clone(),
-        opts.index.clone(),
-        publish_registry,
+        opts.index.as_deref(),
+        publish_registry.as_deref(),
         true,
         !opts.dry_run,
     )?;
@@ -336,21 +364,18 @@ pub fn registry_configuration(
 ) -> CargoResult<RegistryConfig> {
     let err_both = |token_key: &str, proc_key: &str| {
         Err(format_err!(
-            "both `{TOKEN_KEY}` and `{PROC_KEY}` \
+            "both `{token_key}` and `{proc_key}` \
              were specified in the config\n\
              Only one of these values may be set, remove one or the other to proceed.",
-            TOKEN_KEY = token_key,
-            PROC_KEY = proc_key,
         ))
     };
     // `registry.default` is handled in command-line parsing.
-    let (index, token, process) = match registry {
+    let (token, process) = match registry {
         Some(registry) => {
-            let index = Some(config.get_registry_index(registry)?.to_string());
-            let token_key = format!("registries.{}.token", registry);
+            let token_key = format!("registries.{registry}.token");
             let token = config.get_string(&token_key)?.map(|p| p.val);
             let process = if config.cli_unstable().credential_process {
-                let mut proc_key = format!("registries.{}.credential-process", registry);
+                let mut proc_key = format!("registries.{registry}.credential-process");
                 let mut process = config.get::<Option<config::PathAndArgs>>(&proc_key)?;
                 if process.is_none() && token.is_none() {
                     // This explicitly ignores the global credential-process if
@@ -364,7 +389,7 @@ pub fn registry_configuration(
             } else {
                 None
             };
-            (index, token, process)
+            (token, process)
         }
         None => {
             // Use crates.io default.
@@ -380,17 +405,18 @@ pub fn registry_configuration(
             } else {
                 None
             };
-            (None, token, process)
+            (token, process)
         }
     };
 
     let credential_process =
         process.map(|process| (process.path.resolve_program(config), process.args));
 
-    Ok(RegistryConfig {
-        index,
-        token,
-        credential_process,
+    Ok(match (token, credential_process) {
+        (None, None) => RegistryConfig::None,
+        (None, Some(process)) => RegistryConfig::Process(process),
+        (Some(x), None) => RegistryConfig::Token(x),
+        (Some(_), Some(_)) => unreachable!("Only one of these values may be set."),
     })
 }
 
@@ -408,8 +434,8 @@ pub fn registry_configuration(
 fn registry(
     config: &Config,
     token: Option<String>,
-    index: Option<String>,
-    registry: Option<String>,
+    index: Option<&str>,
+    registry: Option<&str>,
     force_update: bool,
     validate_token: bool,
 ) -> CargoResult<(Registry, RegistryConfig, SourceId)> {
@@ -418,9 +444,12 @@ fn registry(
         bail!("both `--index` and `--registry` should not be set at the same time");
     }
     // Parse all configuration options
-    let reg_cfg = registry_configuration(config, registry.as_deref())?;
-    let opt_index = reg_cfg.index.as_deref().or_else(|| index.as_deref());
-    let sid = get_source_id(config, opt_index, registry.as_deref())?;
+    let reg_cfg = registry_configuration(config, registry)?;
+    let opt_index = registry
+        .map(|r| config.get_registry_index(r))
+        .transpose()?
+        .map(|u| u.to_string());
+    let sid = get_source_id(config, opt_index.as_deref().or(index), registry)?;
     if !sid.is_remote_registry() {
         bail!(
             "{} does not support API commands.\n\
@@ -459,7 +488,7 @@ fn registry(
             // people. It will affect those using source replacement, but
             // hopefully that's a relatively small set of users.
             if token.is_none()
-                && reg_cfg.token.is_some()
+                && reg_cfg.is_token()
                 && registry.is_none()
                 && !sid.is_default_registry()
                 && !crates_io::is_url_crates_io(&api_host)
@@ -471,16 +500,10 @@ fn registry(
                         see <https://github.com/rust-lang/cargo/issues/xxx>.\n\
                         Use the --token command-line flag to remove this warning.",
                 )?;
-                reg_cfg.token.clone()
+                reg_cfg.as_token().map(|t| t.to_owned())
             } else {
-                let token = auth::auth_token(
-                    config,
-                    token.as_deref(),
-                    reg_cfg.token.as_deref(),
-                    reg_cfg.credential_process.as_ref(),
-                    registry.as_deref(),
-                    &api_host,
-                )?;
+                let token =
+                    auth::auth_token(config, token.as_deref(), &reg_cfg, registry, &api_host)?;
                 Some(token)
             }
         }
@@ -692,7 +715,8 @@ pub fn registry_login(
     token: Option<String>,
     reg: Option<String>,
 ) -> CargoResult<()> {
-    let (registry, reg_cfg, _) = registry(config, token.clone(), None, reg.clone(), false, false)?;
+    let (registry, reg_cfg, _) =
+        registry(config, token.clone(), None, reg.as_deref(), false, false)?;
 
     let token = match token {
         Some(token) => token,
@@ -714,7 +738,7 @@ pub fn registry_login(
         }
     };
 
-    if let Some(old_token) = &reg_cfg.token {
+    if let RegistryConfig::Token(old_token) = &reg_cfg {
         if old_token == &token {
             config.shell().status("Login", "already logged in")?;
             return Ok(());
@@ -724,7 +748,7 @@ pub fn registry_login(
     auth::login(
         config,
         token,
-        reg_cfg.credential_process.as_ref(),
+        reg_cfg.as_process(),
         reg.as_deref(),
         registry.host(),
     )?;
@@ -740,9 +764,9 @@ pub fn registry_login(
 }
 
 pub fn registry_logout(config: &Config, reg: Option<String>) -> CargoResult<()> {
-    let (registry, reg_cfg, _) = registry(config, None, None, reg.clone(), false, false)?;
+    let (registry, reg_cfg, _) = registry(config, None, None, reg.as_deref(), false, false)?;
     let reg_name = reg.as_deref().unwrap_or(CRATES_IO_DOMAIN);
-    if reg_cfg.credential_process.is_none() && reg_cfg.token.is_none() {
+    if reg_cfg.is_none() {
         config.shell().status(
             "Logout",
             format!("not currently logged in to `{}`", reg_name),
@@ -751,7 +775,7 @@ pub fn registry_logout(config: &Config, reg: Option<String>) -> CargoResult<()>
     }
     auth::logout(
         config,
-        reg_cfg.credential_process.as_ref(),
+        reg_cfg.as_process(),
         reg.as_deref(),
         registry.host(),
     )?;
@@ -788,8 +812,8 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> {
     let (mut registry, _, _) = registry(
         config,
         opts.token.clone(),
-        opts.index.clone(),
-        opts.registry.clone(),
+        opts.index.as_deref(),
+        opts.registry.as_deref(),
         true,
         true,
     )?;
@@ -864,7 +888,8 @@ pub fn yank(
         None => bail!("a version must be specified to yank"),
     };
 
-    let (mut registry, _, _) = registry(config, token, index, reg, true, true)?;
+    let (mut registry, _, _) =
+        registry(config, token, index.as_deref(), reg.as_deref(), true, true)?;
 
     if undo {
         config
@@ -923,7 +948,8 @@ pub fn search(
         prefix
     }
 
-    let (mut registry, _, source_id) = registry(config, None, index, reg, false, false)?;
+    let (mut registry, _, source_id) =
+        registry(config, None, index.as_deref(), reg.as_deref(), false, false)?;
     let (crates, total_crates) = registry.search(query, limit).with_context(|| {
         format!(
             "failed to retrieve search results from the registry at {}",

src/cargo/ops/registry/auth.rs

@@ -8,6 +8,8 @@ use std::io::{Read, Write};
 use std::path::PathBuf;
 use std::process::{Command, Stdio};
 
+use super::RegistryConfig;
+
 enum Action {
     Get,
     Store(String),
@@ -18,18 +20,17 @@ enum Action {
 pub(super) fn auth_token(
     config: &Config,
     cli_token: Option<&str>,
-    config_token: Option<&str>,
-    credential_process: Option<&(PathBuf, Vec<String>)>,
+    credential: &RegistryConfig,
     registry_name: Option<&str>,
     api_url: &str,
 ) -> CargoResult<String> {
-    let token = match (cli_token, config_token, credential_process) {
-        (None, None, None) => {
+    let token = match (cli_token, credential) {
+        (None, RegistryConfig::None) => {
             bail!("no upload token found, please run `cargo login` or pass `--token`");
         }
-        (Some(cli_token), _, _) => cli_token.to_string(),
-        (None, Some(config_token), _) => config_token.to_string(),
-        (None, None, Some(process)) => {
+        (Some(cli_token), _) => cli_token.to_string(),
+        (None, RegistryConfig::Token(config_token)) => config_token.to_string(),
+        (None, RegistryConfig::Process(process)) => {
             let registry_name = registry_name.unwrap_or(CRATES_IO_REGISTRY);
             run_command(config, process, registry_name, api_url, Action::Get)?.unwrap()
         }