Minor cleanup

crates/uv-auth/src/cache.rs

@@ -3,8 +3,8 @@ use std::{collections::HashMap, sync::Mutex};
 
 use crate::credentials::Credentials;
 use crate::NetLoc;
-use reqwest::Request;
-use tracing::debug;
+
+use tracing::trace;
 use url::Url;
 
 type CacheKey = (NetLoc, Option<String>);
@@ -15,11 +15,11 @@ pub struct CredentialsCache {
 
 #[derive(Debug, Clone)]
 pub enum CheckResponse {
-    /// Credentials are already on the request.
-    OnRequest(Arc<Credentials>),
+    /// The given credentials should be used and are not present in the cache.
+    Uncached(Arc<Credentials>),
     /// Credentials were found in the cache.
     Cached(Arc<Credentials>),
-    // Credentials were not found in the cache or request.
+    // Credentials were not found in the cache and none were provided.
     None,
 }
 
@@ -28,7 +28,7 @@ impl CheckResponse {
     pub fn get(&self) -> Option<&Credentials> {
         match self {
             Self::Cached(credentials) => Some(credentials.as_ref()),
-            Self::OnRequest(credentials) => Some(credentials.as_ref()),
+            Self::Uncached(credentials) => Some(credentials.as_ref()),
             Self::None => None,
         }
     }
@@ -59,45 +59,52 @@ impl CredentialsCache {
         (NetLoc::from(url), username)
     }
 
-    /// Return the credentials that should be used for a request, if any.
+    /// Return the credentials that should be used for a URL, if any.
+    ///
+    /// The [`Url`] is not checked for credentials. Existing credentials should be extracted and passed
+    /// separately.
     ///
-    /// If complete credentials are already present on the request, they will be returned.
+    /// If complete credentials are provided, they will be returned as [`CheckResponse::Existing`]
     /// If the credentials are partial, i.e. missing a password, the cache will be checked
     /// for a corresponding entry.
-    pub(crate) fn check_request(&self, request: &Request) -> CheckResponse {
+    pub(crate) fn check(&self, url: &Url, credentials: Option<Credentials>) -> CheckResponse {
         let store = self.store.lock().unwrap();
 
-        let credentials = Credentials::from_request(request).map(Arc::new);
+        let credentials = credentials.map(Arc::new);
         let key = CredentialsCache::key(
-            request.url(),
+            url,
             credentials
                 .as_ref()
                 .and_then(|credentials| credentials.username().map(str::to_string)),
         );
 
         if let Some(credentials) = credentials {
             if credentials.password().is_some() {
-                debug!("Request already has password, skipping cache");
+                trace!("Existing credentials include password, skipping cache");
                 // No need to look-up, we have a password already
-                return CheckResponse::OnRequest(credentials);
+                return CheckResponse::Uncached(credentials);
             }
-            debug!("No password found on request, checking cache");
+            trace!("Existing credentials missing password, checking cache");
             let existing = store.get(&key);
             existing
                 .cloned()
                 .map(CheckResponse::Cached)
-                .unwrap_or(CheckResponse::OnRequest(credentials))
+                .inspect(|_| trace!("Found cached credentials."))
+                .unwrap_or_else(|| {
+                    trace!("No credentials in cache, using existing credentials");
+                    CheckResponse::Uncached(credentials)
+                })
         } else {
-            debug!("No credentials on request, checking cache");
-            let credentials = store.get(&key).cloned();
-            if credentials.is_some() {
-                debug!("Found cached credentials: {credentials:?}");
-            } else {
-                debug!("No credentials in cache");
-            }
-            credentials
+            trace!("No credentials on request, checking cache...");
+            store
+                .get(&key)
+                .cloned()
                 .map(CheckResponse::Cached)
-                .unwrap_or(CheckResponse::None)
+                .inspect(|_| trace!("Found cached credentials."))
+                .unwrap_or_else(|| {
+                    trace!("No credentials in cache.");
+                    CheckResponse::None
+                })
         }
     }
 

crates/uv-auth/src/middleware.rs

@@ -75,13 +75,28 @@ impl Middleware for AuthMiddleware {
         extensions: &mut Extensions,
         next: Next<'_>,
     ) -> reqwest_middleware::Result<Response> {
-        let url = request.url().clone();
-        debug!("Handling request for {url}");
+        // Check for credentials attached to (1) the request itself
+        let credentials = Credentials::from_request(&request);
+        // In the middleware, existing credentials are already moved from the URL
+        // to the headers so for display purposes we restore some information
+        let url = if tracing::enabled!(tracing::Level::DEBUG) {
+            let mut url = request.url().clone();
+            credentials
+                .as_ref()
+                .and_then(|credentials| credentials.username())
+                .and_then(|username| url.set_username(username).ok());
+            credentials
+                .as_ref()
+                .and_then(|credentials| credentials.password())
+                .and_then(|_| url.set_password(Some("****")).ok());
+            url.to_string()
+        } else {
+            request.url().to_string()
+        };
+        trace!("Handling request for {url}");
 
-        // Check for credentials attached to:
-        // (1) The request itself
-        // (2) The cache
-        let credentials = self.cache().check_request(&request);
+        // Then check for credentials in (2) the cache
+        let credentials = self.cache().check(request.url(), credentials);
 
         // Track credentials that we might want to insert into the cache
         let mut new_credentials = None;
@@ -93,15 +108,13 @@ impl Middleware for AuthMiddleware {
                 CheckResponse::Cached(credentials) => request = credentials.authenticate(request),
                 // If we get credentials from the request, we should update the cache
                 // but don't need to update the request
-                CheckResponse::OnRequest(credentials) => {
-                    new_credentials = Some(credentials.clone())
-                }
+                CheckResponse::Uncached(credentials) => new_credentials = Some(credentials.clone()),
                 CheckResponse::None => unreachable!("No credentials cannot be authenticated"),
             }
         // Otherwise, look for complete credentials in:
         // (3) The netrc file
         } else if let Some(credentials) = self.netrc.as_ref().and_then(|netrc| {
-            debug!("Checking netrc for credentials for `{}`", url.to_string());
+            trace!("Checking netrc for credentials for {url}");
             Credentials::from_netrc(
                 netrc,
                 request.url(),
@@ -110,7 +123,7 @@ impl Middleware for AuthMiddleware {
                     .and_then(|credentials| credentials.username()),
             )
         }) {
-            debug!("Adding credentials from the netrc file to {url}");
+            debug!("Found credentials in netrc file for {url}");
             request = credentials.authenticate(request);
             new_credentials = Some(Arc::new(credentials));
         // (4) The keyring
@@ -119,46 +132,42 @@ impl Middleware for AuthMiddleware {
                 .get()
                 .and_then(|credentials| credentials.username())
             {
-                debug!("Checking keyring for credentials for `{}`", url.to_string());
+                debug!("Checking keyring for credentials for {url}");
                 keyring.fetch(request.url(), username)
             } else {
-                trace!(
-                    "Skipping keyring lookup for `{}`: no username found",
-                    url.to_string()
-                );
+                trace!("Skipping keyring lookup for {url} with no username");
                 None
             }
         }) {
-            debug!("Adding credentials from the keyring to {url}");
+            debug!("Found credentials in keyring for {url}");
             request = credentials.authenticate(request);
             new_credentials = Some(Arc::new(credentials))
+        // No additional credentials were found
         } else {
             match credentials {
                 CheckResponse::Cached(credentials) => request = credentials.authenticate(request),
-                CheckResponse::OnRequest(credentials) => {
-                    new_credentials = Some(credentials.clone())
-                }
+                CheckResponse::Uncached(credentials) => new_credentials = Some(credentials.clone()),
                 CheckResponse::None => {
-                    debug!("No credentials found.")
+                    debug!("No credentials found for {url}")
                 }
             }
         }
 
-        // Perform the request
-        let result = next.run(request, extensions).await;
-
-        // Only update the cache with new credentials on a successful request
         if let Some(credentials) = new_credentials {
+            // Only update the cache with new credentials on a successful request
+            let url = request.url().clone();
+            let result = next.run(request, extensions).await;
             if result
                 .as_ref()
                 .is_ok_and(|response| response.error_for_status_ref().is_ok())
             {
-                debug!("Updating credentials for {url} to {credentials:?}");
+                trace!("Updating cached credentials for {url}");
                 self.cache().insert(&url, credentials)
             };
+            result
+        } else {
+            next.run(request, extensions).await
         }
-
-        result
     }
 }
 

crates/uv-configuration/src/authentication.rs

@@ -1,4 +1,3 @@
-
 use uv_auth::{self, KeyringProvider};
 
 /// Keyring provider type to use for credential lookup.