Cache CredentialConfig's not tokens.

Author: Eh2406

src/cargo/ops/registry.rs

@@ -40,7 +40,7 @@ use crate::util::auth;
 /// Registry settings loaded from config files.
 ///
 /// This is loaded based on the `--registry` flag and the config settings.
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone)]
 pub enum RegistryCredentialConfig {
     None,
     /// The authentication token.
@@ -211,12 +211,14 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
     };
 
     if !opts.dry_run {
-        registry.set_token(Some(auth::auth_token(
-            &opts.config,
-            &reg_id_no_replacement,
-            None,
-            Some(mutation),
-        )?));
+        registry.set_token(Some(
+            auth::auth_token(&opts.config, &reg_id_no_replacement)?.as_header(
+                &opts.config,
+                &reg_id_no_replacement,
+                None,
+                Some(&mutation),
+            )?,
+        ));
     }
 
     opts.config
@@ -513,11 +515,11 @@ fn registry(
                     Use the --token command-line flag to remove this warning.",
             )?;
         }
-        Some(auth::auth_token(
+        Some(auth::auth_token(config, &sid_no_replacement)?.as_header(
             config,
             &sid_no_replacement,
             None,
-            token_required,
+            token_required.as_ref(),
         )?)
     } else {
         None

src/cargo/sources/registry/download.rs

@@ -71,7 +71,12 @@ pub(super) fn download(
     }
 
     let authorization = if registry_config.auth_required {
-        Some(auth::auth_token(config, &pkg.source_id(), None, None)?)
+        Some(auth::auth_token(config, &pkg.source_id())?.as_header(
+            config,
+            &pkg.source_id(),
+            None,
+            None,
+        )?)
     } else {
         None
     };

src/cargo/sources/registry/http_remote.rs

@@ -541,8 +541,13 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
                     "authenticated registries require `-Z registry-auth`"
                 )));
             }
-            let authorization =
-                auth::auth_token(self.config, &self.source_id, self.login_url.as_ref(), None)?;
+            let credential = auth::auth_token(self.config, &self.source_id)?;
+            let authorization = credential.as_header(
+                self.config,
+                &self.source_id,
+                self.login_url.as_ref(),
+                None,
+            )?;
             headers.append(&format!("authorization: {}", authorization))?;
             trace!("including authorization for {}", full_url);
         }

src/cargo/util/auth.rs

@@ -12,6 +12,7 @@ use std::error::Error;
 use std::io::{Read, Write};
 use std::path::PathBuf;
 use std::process::{Command, Stdio};
+use std::rc::Rc;
 use time::format_description::well_known::Rfc3339;
 use time::OffsetDateTime;
 use url::Url;
@@ -269,98 +270,91 @@ my-registry = {{ index = "{}" }}
 // Store a token in the cache for future calls.
 pub fn cache_token(config: &Config, sid: &SourceId, token: &str) {
     let url = sid.canonical_url();
-    config
-        .credential_cache()
-        .insert(url.clone(), token.to_string());
+    config.credential_cache().insert(
+        url.clone(),
+        Rc::new(RegistryCredentialConfig::Token(token.to_string())),
+    );
 }
 
 /// Returns the token to use for the given registry.
-/// If a `login_url` is provided and a token is not available, the
-/// login_url will be included in the returned error.
-pub fn auth_token(
-    config: &Config,
-    sid: &SourceId,
-    login_url: Option<&Url>,
-    mutation: Option<Mutation<'_>>,
-) -> CargoResult<String> {
-    match auth_token_optional(config, sid, mutation.as_ref())? {
-        Some(token) => Ok(token),
-        None => Err(AuthorizationError {
-            sid: sid.clone(),
-            login_url: login_url.cloned(),
-            reason: AuthorizationErrorReason::TokenMissing,
-        }
-        .into()),
-    }
-}
-
-/// Returns the token to use for the given registry.
-fn auth_token_optional(
-    config: &Config,
-    sid: &SourceId,
-    mutation: Option<&'_ Mutation<'_>>,
-) -> CargoResult<Option<String>> {
+pub fn auth_token(config: &Config, sid: &SourceId) -> CargoResult<Rc<RegistryCredentialConfig>> {
     let mut cache = config.credential_cache();
     let url = sid.canonical_url();
 
-    if mutation.is_none() {
-        if let Some(token) = cache.get(url) {
-            return Ok(Some(token.clone()));
-        }
+    if let Some(token) = cache.get(url) {
+        return Ok(Rc::clone(token));
     }
 
-    let credential = registry_credential_config(config, sid)?;
-    let token = match credential {
-        RegistryCredentialConfig::None => return Ok(None),
-        RegistryCredentialConfig::Token(config_token) => config_token.to_string(),
-        RegistryCredentialConfig::Process(process) => {
-            // todo: PASETO with process
-            run_command(config, &process, sid, Action::Get)?.unwrap()
-        }
-        RegistryCredentialConfig::Key((private_key, private_key_subject)) => {
-            let secret: AsymmetricSecretKey<pasetors::version3::V3> =
-                private_key.as_str().try_into()?;
-            let public: AsymmetricPublicKey<pasetors::version3::V3> = (&secret).try_into()?;
-            let public_key_id: pasetors::paserk::Id = (&public).into();
-            let mut kip = String::new();
-            FormatAsPaserk::fmt(&public_key_id, &mut kip).unwrap();
-            let iat = OffsetDateTime::now_utc();
-
-            let message = Message {
-                iat: &iat.format(&Rfc3339)?,
-                sub: private_key_subject.as_deref(),
-                mutation: mutation.and_then(|m| m.mutation),
-                name: mutation.and_then(|m| m.name),
-                vers: mutation.and_then(|m| m.vers),
-                cksum: mutation.and_then(|m| m.cksum),
-                challenge: None, // todo: PASETO with challenges,
-                v: None,
-            };
-            let footer = Footer {
-                url: &sid.url().to_string(),
-                kip: &kip,
-            };
-
-            pasetors::version3::PublicToken::sign(
-                &secret,
-                &public,
-                serde_json::to_string(&message)
-                    .expect("cannot serialize")
-                    .as_bytes(),
-                Some(
-                    serde_json::to_string(&footer)
+    let credential = Rc::new(registry_credential_config(config, sid)?);
+
+    cache.insert(url.clone(), Rc::clone(&credential));
+    Ok(credential)
+}
+
+impl RegistryCredentialConfig {
+    /// If a `login_url` is provided and a token is not available, the
+    /// login_url will be included in the returned error.
+    pub fn as_header(
+        self: &Rc<RegistryCredentialConfig>,
+        config: &Config,
+        sid: &SourceId,
+        login_url: Option<&Url>,
+        mutation: Option<&'_ Mutation<'_>>,
+    ) -> CargoResult<String> {
+        Ok(match &**self {
+            RegistryCredentialConfig::None => {
+                return Err(AuthorizationError {
+                    sid: sid.clone(),
+                    login_url: login_url.cloned(),
+                    reason: AuthorizationErrorReason::TokenMissing,
+                }
+                .into())
+            }
+            RegistryCredentialConfig::Token(config_token) => config_token.to_string(),
+            RegistryCredentialConfig::Process(process) => {
+                // todo: PASETO with process
+                run_command(config, &process, sid, Action::Get)?.unwrap()
+            }
+            RegistryCredentialConfig::Key((private_key, private_key_subject)) => {
+                let secret: AsymmetricSecretKey<pasetors::version3::V3> =
+                    private_key.as_str().try_into()?;
+                let public: AsymmetricPublicKey<pasetors::version3::V3> = (&secret).try_into()?;
+                let public_key_id: pasetors::paserk::Id = (&public).into();
+                let mut kip = String::new();
+                FormatAsPaserk::fmt(&public_key_id, &mut kip).unwrap();
+                let iat = OffsetDateTime::now_utc();
+
+                let message = Message {
+                    iat: &iat.format(&Rfc3339)?,
+                    sub: private_key_subject.as_deref(),
+                    mutation: mutation.and_then(|m| m.mutation),
+                    name: mutation.and_then(|m| m.name),
+                    vers: mutation.and_then(|m| m.vers),
+                    cksum: mutation.and_then(|m| m.cksum),
+                    challenge: None, // todo: PASETO with challenges,
+                    v: None,
+                };
+                let footer = Footer {
+                    url: &sid.url().to_string(),
+                    kip: &kip,
+                };
+
+                pasetors::version3::PublicToken::sign(
+                    &secret,
+                    &public,
+                    serde_json::to_string(&message)
                         .expect("cannot serialize")
                         .as_bytes(),
-                ),
-                None,
-            )?
-        }
-    };
-
-    if mutation.is_none() {
-        cache.insert(url.clone(), token.clone());
+                    Some(
+                        serde_json::to_string(&footer)
+                            .expect("cannot serialize")
+                            .as_bytes(),
+                    ),
+                    None,
+                )?
+            }
+        })
     }
-    Ok(Some(token))
 }
 
 pub struct Mutation<'a> {

src/cargo/util/config/mod.rs

@@ -61,6 +61,7 @@ use std::io::prelude::*;
 use std::io::{self, SeekFrom};
 use std::mem;
 use std::path::{Path, PathBuf};
+use std::rc::Rc;
 use std::str::FromStr;
 use std::sync::Once;
 use std::time::Instant;
@@ -179,7 +180,7 @@ pub struct Config {
     updated_sources: LazyCell<RefCell<HashSet<SourceId>>>,
     /// Cache of credentials from configuration or credential providers.
     /// Maps from url to credential value.
-    credential_cache: LazyCell<RefCell<HashMap<CanonicalUrl, String>>>,
+    credential_cache: LazyCell<RefCell<HashMap<CanonicalUrl, Rc<RegistryCredentialConfig>>>>,
     /// Lock, if held, of the global package cache along with the number of
     /// acquisitions so far.
     package_cache_lock: RefCell<Option<(Option<FileLock>, usize)>>,
@@ -438,7 +439,9 @@ impl Config {
     }
 
     /// Cached credentials from credential providers or configuration.
-    pub fn credential_cache(&self) -> RefMut<'_, HashMap<CanonicalUrl, String>> {
+    pub fn credential_cache(
+        &self,
+    ) -> RefMut<'_, HashMap<CanonicalUrl, Rc<RegistryCredentialConfig>>> {
         self.credential_cache
             .borrow_with(|| RefCell::new(HashMap::new()))
             .borrow_mut()

tests/testsuite/registry_auth.rs

@@ -6,7 +6,7 @@ use cargo_test_support::{project, Execs, Project};
 fn cargo(p: &Project, s: &str) -> Execs {
     let mut e = p.cargo(s);
     e.masquerade_as_nightly_cargo()
-        .arg("-Zhttp-registry")
+        .arg("-Zsparse-registry")
         .arg("-Zregistry-auth")
         .arg("-Zasymmetric-tokens");
     e