Auto merge of #12499 - arlosi:cred-args, r=Eh2406

login: allow passing additional args to provider

As part of moving asymmetric token support to a credential provider in #12334, support for passing `--key-subject` to `cargo login` was removed.

This change allows passing additional arguments to credential providers when running `cargo login`. For example:
`cargo login -- --key-subject foo`.

The asymmetric token provider (`cargo:paseto`) is updated to take advantage of this and re-enables setting `--key-subject` from `cargo login`.

r? `@Eh2406`

cc #8933

src/bin/cargo/commands/login.rs

@@ -7,16 +7,28 @@ pub fn cli() -> Command {
         .about("Log in to a registry.")
         .arg(Arg::new("token").action(ArgAction::Set))
         .arg(opt("registry", "Registry to use").value_name("REGISTRY"))
+        .arg(
+            Arg::new("args")
+                .help("Arguments for the credential provider (unstable)")
+                .num_args(0..)
+                .last(true),
+        )
         .arg_quiet()
         .after_help("Run `cargo help login` for more detailed information.\n")
 }
 
 pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
     let registry = args.registry(config)?;
+    let extra_args = args
+        .get_many::<String>("args")
+        .unwrap_or_default()
+        .map(String::as_str)
+        .collect::<Vec<_>>();
     ops::registry_login(
         config,
         args.get_one::<String>("token").map(|s| s.as_str().into()),
         registry.as_deref(),
+        &extra_args,
     )?;
     Ok(())
 }

src/cargo/ops/registry/login.rs

@@ -21,6 +21,7 @@ pub fn registry_login(
     config: &Config,
     token_from_cmdline: Option<Secret<&str>>,
     reg: Option<&str>,
+    args: &[&str],
 ) -> CargoResult<()> {
     let source_ids = get_source_id(config, None, reg)?;
 
@@ -50,6 +51,6 @@ pub fn registry_login(
         login_url: login_url.as_deref(),
     };
 
-    auth::login(config, &source_ids.original, options)?;
+    auth::login(config, &source_ids.original, options, args)?;
     Ok(())
 }

src/cargo/util/auth/mod.rs

@@ -429,6 +429,7 @@ fn credential_action(
     sid: &SourceId,
     action: Action<'_>,
     headers: Vec<String>,
+    args: &[&str],
 ) -> CargoResult<CredentialResponse> {
     let name = if sid.is_crates_io() {
         Some(CRATES_IO_REGISTRY)
@@ -442,7 +443,11 @@ fn credential_action(
     };
     let providers = credential_provider(config, sid)?;
     for provider in providers {
-        let args: Vec<&str> = provider.iter().map(String::as_str).collect();
+        let args: Vec<&str> = provider
+            .iter()
+            .map(String::as_str)
+            .chain(args.iter().map(|s| *s))
+            .collect();
         let process = args[0];
         tracing::debug!("attempting credential provider: {args:?}");
         let provider: Box<dyn Credential> = match process {
@@ -528,7 +533,7 @@ fn auth_token_optional(
         }
     }
 
-    let credential_response = credential_action(config, sid, Action::Get(operation), headers);
+    let credential_response = credential_action(config, sid, Action::Get(operation), headers, &[]);
     if let Some(e) = credential_response.as_ref().err() {
         if let Some(e) = e.downcast_ref::<cargo_credential::Error>() {
             if matches!(e, cargo_credential::Error::NotFound) {
@@ -567,7 +572,7 @@ fn auth_token_optional(
 
 /// Log out from the given registry.
 pub fn logout(config: &Config, sid: &SourceId) -> CargoResult<()> {
-    let credential_response = credential_action(config, sid, Action::Logout, vec![]);
+    let credential_response = credential_action(config, sid, Action::Logout, vec![], &[]);
     if let Some(e) = credential_response.as_ref().err() {
         if let Some(e) = e.downcast_ref::<cargo_credential::Error>() {
             if matches!(e, cargo_credential::Error::NotFound) {
@@ -590,8 +595,13 @@ pub fn logout(config: &Config, sid: &SourceId) -> CargoResult<()> {
 }
 
 /// Log in to the given registry.
-pub fn login(config: &Config, sid: &SourceId, options: LoginOptions<'_>) -> CargoResult<()> {
-    let credential_response = credential_action(config, sid, Action::Login(options), vec![])?;
+pub fn login(
+    config: &Config,
+    sid: &SourceId,
+    options: LoginOptions<'_>,
+    args: &[&str],
+) -> CargoResult<()> {
+    let credential_response = credential_action(config, sid, Action::Login(options), vec![], args)?;
     let CredentialResponse::Login = credential_response else {
         bail!("credential provider produced unexpected response for `login` request: {credential_response:?}")
     };

src/cargo/util/credential/paseto.rs

@@ -4,6 +4,7 @@ use anyhow::Context;
 use cargo_credential::{
     Action, CacheControl, Credential, CredentialResponse, Error, Operation, RegistryInfo, Secret,
 };
+use clap::Command;
 use pasetors::{
     keys::{AsymmetricKeyPair, AsymmetricPublicKey, AsymmetricSecretKey, Generate},
     paserk::FormatAsPaserk,
@@ -14,7 +15,7 @@ use url::Url;
 use crate::{
     core::SourceId,
     ops::RegistryCredentialConfig,
-    util::{auth::registry_credential_config_raw, config},
+    util::{auth::registry_credential_config_raw, command_prelude::opt, config},
     Config,
 };
 
@@ -60,7 +61,7 @@ impl<'a> Credential for PasetoCredential<'a> {
         &self,
         registry: &RegistryInfo<'_>,
         action: &Action<'_>,
-        _args: &[&str],
+        args: &[&str],
     ) -> Result<CredentialResponse, Error> {
         let index_url = Url::parse(registry.index_url).context("parsing index url")?;
         let sid = if let Some(name) = registry.name {
@@ -71,6 +72,13 @@ impl<'a> Credential for PasetoCredential<'a> {
 
         let reg_cfg = registry_credential_config_raw(self.config, &sid)?;
 
+        let matches = Command::new("cargo:paseto")
+            .no_binary_name(true)
+            .arg(opt("key-subject", "Set the key subject for this registry").value_name("SUBJECT"))
+            .try_get_matches_from(args)
+            .map_err(Box::new)?;
+        let key_subject = matches.get_one("key-subject").map(String::as_str);
+
         match action {
             Action::Get(operation) => {
                 let Some(reg_cfg) = reg_cfg else {
@@ -163,6 +171,7 @@ impl<'a> Credential for PasetoCredential<'a> {
                 })
             }
             Action::Login(options) => {
+                let old_key_subject = reg_cfg.and_then(|cfg| cfg.secret_key_subject);
                 let new_token;
                 let secret_key: Secret<String>;
                 if let Some(key) = &options.token {
@@ -180,7 +189,13 @@ impl<'a> Credential for PasetoCredential<'a> {
                 } else {
                     return Err("not a validly formatted PASERK secret key".into());
                 }
-                new_token = RegistryCredentialConfig::AsymmetricKey((secret_key, None));
+                new_token = RegistryCredentialConfig::AsymmetricKey((
+                    secret_key,
+                    match key_subject {
+                        Some(key_subject) => Some(key_subject.to_string()),
+                        None => old_key_subject,
+                    },
+                ));
                 config::save_credentials(self.config, Some(new_token), &sid)?;
                 Ok(CredentialResponse::Login)
             }

tests/testsuite/cargo_login/help/stdout.log

@@ -1,9 +1,10 @@
 Log in to a registry.
 
-Usage: cargo[EXE] login [OPTIONS] [token]
+Usage: cargo[EXE] login [OPTIONS] [token] [-- [args]...]
 
 Arguments:
-  [token]  
+  [token]    
+  [args]...  Arguments for the credential provider (unstable)
 
 Options:
       --registry <REGISTRY>  Registry to use

tests/testsuite/credential_process.rs

@@ -185,15 +185,19 @@ Caused by:
 fn login() {
     let registry = registry::RegistryBuilder::new()
         .no_configure_token()
-        .credential_provider(&[&build_provider("test-cred", r#"{"Ok": {"kind": "login"}}"#)])
+        .credential_provider(&[
+            &build_provider("test-cred", r#"{"Ok": {"kind": "login"}}"#),
+            "cfg1",
+            "--cfg2",
+        ])
         .build();
 
-    cargo_process("login -Z credential-process abcdefg")
+    cargo_process("login -Z credential-process abcdefg -- cmd3 --cmd4")
         .masquerade_as_nightly_cargo(&["credential-process"])
         .replace_crates_io(registry.index_url())
         .with_stderr(
             r#"[UPDATING] [..]
-{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io"},"kind":"login","token":"abcdefg","login-url":"[..]","args":[]}
+{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io"},"kind":"login","token":"abcdefg","login-url":"[..]","args":["cfg1","--cfg2","cmd3","--cmd4"]}
 "#,
         )
         .run();

tests/testsuite/login.rs

@@ -189,6 +189,24 @@ Caused by:
     );
 }
 
+#[cargo_test]
+fn bad_asymmetric_token_args() {
+    let registry = RegistryBuilder::new()
+        .credential_provider(&["cargo:paseto"])
+        .no_configure_token()
+        .build();
+
+    // These cases are kept brief as the implementation is covered by clap, so this is only smoke testing that we have clap configured correctly.
+    cargo_process("login -Zcredential-process -- --key-subject")
+        .masquerade_as_nightly_cargo(&["credential-process"])
+        .replace_crates_io(registry.index_url())
+        .with_stderr_contains(
+            "  error: a value is required for '--key-subject <SUBJECT>' but none was supplied",
+        )
+        .with_status(101)
+        .run();
+}
+
 #[cargo_test]
 fn login_with_no_cargo_dir() {
     // Create a config in the root directory because `login` requires the
@@ -203,6 +221,28 @@ fn login_with_no_cargo_dir() {
     assert_eq!(credentials, "[registry]\ntoken = \"foo\"\n");
 }
 
+#[cargo_test]
+fn login_with_asymmetric_token_and_subject_on_stdin() {
+    let registry = RegistryBuilder::new()
+        .credential_provider(&["cargo:paseto"])
+        .no_configure_token()
+        .build();
+    let credentials = credentials_toml();
+    cargo_process("login -v -Z credential-process -- --key-subject=foo")
+        .masquerade_as_nightly_cargo(&["credential-process"])
+        .replace_crates_io(registry.index_url())
+        .with_stderr_contains(
+            "\
+k3.public.AmDwjlyf8jAV3gm5Z7Kz9xAOcsKslt_Vwp5v-emjFzBHLCtcANzTaVEghTNEMj9PkQ",
+        )
+        .with_stdin("k3.secret.fNYVuMvBgOlljt9TDohnaYLblghqaHoQquVZwgR6X12cBFHZLFsaU3q7X3k1Zn36")
+        .run();
+    let credentials = fs::read_to_string(&credentials).unwrap();
+    assert!(credentials.starts_with("[registry]\n"));
+    assert!(credentials.contains("secret-key-subject = \"foo\"\n"));
+    assert!(credentials.contains("secret-key = \"k3.secret.fNYVuMvBgOlljt9TDohnaYLblghqaHoQquVZwgR6X12cBFHZLFsaU3q7X3k1Zn36\"\n"));
+}
+
 #[cargo_test]
 fn login_with_differently_sized_token() {
     // Verify that the configuration file gets properly truncated.