Refactor uv-auth

crates/uv-auth/Cargo.toml

@@ -21,3 +21,4 @@ once_cell = { workspace = true }
 tempfile = { workspace = true }
 tokio = { workspace = true }
 wiremock = { workspace = true }
+insta = { version = "1.36.1" }

crates/uv-auth/src/credentials.rs

@@ -0,0 +1,197 @@
+use base64::prelude::BASE64_STANDARD;
+use base64::write::EncoderWriter;
+use netrc::Authenticator;
+use reqwest::header::HeaderValue;
+use std::io::Write;
+use url::Url;
+
+#[derive(Clone, Debug, PartialEq)]
+pub struct Credentials {
+    username: String,
+    password: Option<String>,
+}
+
+impl Credentials {
+    pub fn new(username: String, password: Option<String>) -> Self {
+        Self { username, password }
+    }
+
+    pub fn username(&self) -> &str {
+        &self.username
+    }
+
+    pub fn password(&self) -> Option<&str> {
+        self.password.as_deref()
+    }
+
+    /// Extract credentials from a URL.
+    ///
+    /// Returns `None` if `username` and `password` are not populated.
+    pub fn from_url(url: &Url) -> Option<Self> {
+        if url.username().is_empty() && url.password().is_none() {
+            return None;
+        }
+        Some(Self {
+            // Remove percent-encoding from URL credentials
+            // See <https://github.com/pypa/pip/blob/06d21db4ff1ab69665c22a88718a4ea9757ca293/src/pip/_internal/utils/misc.py#L497-L499>
+            username: urlencoding::decode(url.username())
+                .expect("An encoded username should always decode")
+                .into_owned(),
+            password: url.password().map(|password| {
+                urlencoding::decode(password)
+                    .expect("An encoded password should always decode")
+                    .into_owned()
+            }),
+        })
+    }
+}
+
+impl From<Authenticator> for Credentials {
+    fn from(auth: Authenticator) -> Self {
+        Credentials {
+            username: auth.login,
+            password: Some(auth.password),
+        }
+    }
+}
+
+impl Credentials {
+    /// Attach the credentials to the given request.
+    ///
+    /// Any existing credentials will be overridden.
+    #[must_use]
+    pub fn authenticated_request(&self, mut request: reqwest::Request) -> reqwest::Request {
+        request.headers_mut().insert(
+            reqwest::header::AUTHORIZATION,
+            basic_auth(self.username(), self.password()),
+        );
+        request
+    }
+}
+
+/// Create a `HeaderValue` for basic authentication.
+///
+/// Source: <https://github.com/seanmonstar/reqwest/blob/2c11ef000b151c2eebeed2c18a7b81042220c6b0/src/util.rs#L3>
+fn basic_auth<U, P>(username: U, password: Option<P>) -> HeaderValue
+where
+    U: std::fmt::Display,
+    P: std::fmt::Display,
+{
+    let mut buf = b"Basic ".to_vec();
+    {
+        let mut encoder = EncoderWriter::new(&mut buf, &BASE64_STANDARD);
+        let _ = write!(encoder, "{}:", username);
+        if let Some(password) = password {
+            let _ = write!(encoder, "{}", password);
+        }
+    }
+    let mut header = HeaderValue::from_bytes(&buf).expect("base64 is always valid HeaderValue");
+    header.set_sensitive(true);
+    header
+}
+
+#[cfg(test)]
+mod test {
+    use std::io::Read;
+
+    use base64::read::DecoderReader;
+    use insta::{assert_debug_snapshot, assert_snapshot};
+
+    use super::*;
+
+    fn decode_basic_auth(header: HeaderValue) -> String {
+        let mut value = header.as_bytes();
+        value = value
+            .strip_prefix(b"Basic ")
+            .expect("Basic authentication should start with 'Basic '");
+        let mut decoder = DecoderReader::new(&mut value, &BASE64_STANDARD);
+        let mut buf = "Basic: ".to_string();
+        decoder
+            .read_to_string(&mut buf)
+            .expect("Header contents should be valid base64");
+        buf
+    }
+
+    #[test]
+    fn from_url_no_credentials() {
+        let url = &Url::parse("https://example.com/simple/first/").unwrap();
+        assert_eq!(Credentials::from_url(&url), None);
+    }
+
+    #[test]
+    fn from_url_username_and_password() {
+        let url = &Url::parse("https://example.com/simple/first/").unwrap();
+        let mut auth_url = url.clone();
+        auth_url.set_username("user").unwrap();
+        auth_url.set_password(Some("password")).unwrap();
+        let credentials = Credentials::from_url(&auth_url).unwrap();
+        assert_eq!(credentials.username(), "user");
+        assert_eq!(credentials.password(), Some("password"));
+    }
+
+    #[test]
+    fn authenticated_request_from_url() {
+        let url = Url::parse("https://example.com/simple/first/").unwrap();
+        let mut auth_url = url.clone();
+        auth_url.set_username("user").unwrap();
+        auth_url.set_password(Some("password")).unwrap();
+        let credentials = Credentials::from_url(&auth_url).unwrap();
+
+        let mut request = reqwest::Request::new(reqwest::Method::GET, url);
+        request = credentials.authenticated_request(request);
+
+        let mut header = request
+            .headers()
+            .get(reqwest::header::AUTHORIZATION)
+            .expect("Authorization header should be set")
+            .clone();
+        header.set_sensitive(false);
+
+        assert_debug_snapshot!(header, @r###""Basic dXNlcjpwYXNzd29yZA==""###);
+        assert_snapshot!(decode_basic_auth(header), @"Basic: user:password");
+    }
+
+    #[test]
+    fn authenticated_request_from_url_with_percent_encoded_user() {
+        let url = Url::parse("https://example.com/simple/first/").unwrap();
+        let mut auth_url = url.clone();
+        auth_url.set_username("user@domain").unwrap();
+        auth_url.set_password(Some("password")).unwrap();
+        let credentials = Credentials::from_url(&auth_url).unwrap();
+
+        let mut request = reqwest::Request::new(reqwest::Method::GET, url);
+        request = credentials.authenticated_request(request);
+
+        let mut header = request
+            .headers()
+            .get(reqwest::header::AUTHORIZATION)
+            .expect("Authorization header should be set")
+            .clone();
+        header.set_sensitive(false);
+
+        assert_debug_snapshot!(header, @r###""Basic dXNlckBkb21haW46cGFzc3dvcmQ=""###);
+        assert_snapshot!(decode_basic_auth(header), @"Basic: user@domain:password");
+    }
+
+    #[test]
+    fn authenticated_request_from_url_with_percent_encoded_password() {
+        let url = Url::parse("https://example.com/simple/first/").unwrap();
+        let mut auth_url = url.clone();
+        auth_url.set_username("user").unwrap();
+        auth_url.set_password(Some("password==")).unwrap();
+        let credentials = Credentials::from_url(&auth_url).unwrap();
+
+        let mut request = reqwest::Request::new(reqwest::Method::GET, url);
+        request = credentials.authenticated_request(request);
+
+        let mut header = request
+            .headers()
+            .get(reqwest::header::AUTHORIZATION)
+            .expect("Authorization header should be set")
+            .clone();
+        header.set_sensitive(false);
+
+        assert_debug_snapshot!(header, @r###""Basic dXNlcjpwYXNzd29yZD09""###);
+        assert_snapshot!(decode_basic_auth(header), @"Basic: user:password==");
+    }
+}

crates/uv-auth/src/keyring.rs

@@ -4,7 +4,7 @@ use thiserror::Error;
 use tracing::debug;
 use url::Url;
 
-use crate::store::{BasicAuthData, Credential};
+use crate::credentials::Credentials;
 
 /// Keyring provider to use for authentication
 ///
@@ -37,7 +37,7 @@ pub enum Error {
 ///
 /// See `pip`'s KeyringCLIProvider
 /// <https://github.com/pypa/pip/blob/ae5fff36b0aad6e5e0037884927eaa29163c0611/src/pip/_internal/network/auth.py#L102>
-pub fn get_keyring_subprocess_auth(url: &Url) -> Result<Option<Credential>, Error> {
+pub fn get_keyring_subprocess_auth(url: &Url) -> Result<Option<Credentials>, Error> {
     let host = url.host_str();
     if host.is_none() {
         return Err(Error::NotKeyringTarget(
@@ -76,12 +76,7 @@ pub fn get_keyring_subprocess_auth(url: &Url) -> Result<Option<Credential>, Erro
     };
 
     output.map(|password| {
-        password.map(|password| {
-            Credential::Basic(BasicAuthData {
-                username: username.to_string(),
-                password: Some(password),
-            })
-        })
+        password.map(|password| Credentials::new(username.to_string(), Some(password)))
     })
 }
 

crates/uv-auth/src/lib.rs

@@ -1,139 +1,24 @@
+mod credentials;
 mod keyring;
 mod middleware;
+mod netloc;
 mod store;
 
 pub use keyring::KeyringProvider;
 pub use middleware::AuthMiddleware;
+use netloc::NetLoc;
 use once_cell::sync::Lazy;
 pub use store::AuthenticationStore;
-
 use url::Url;
 
 // TODO(zanieb): Consider passing a store explicitly throughout
 
 /// Global authentication store for a `uv` invocation
 pub static GLOBAL_AUTH_STORE: Lazy<AuthenticationStore> = Lazy::new(AuthenticationStore::default);
 
-/// Used to determine if authentication information should be retained on a new URL.
-/// Based on the specification defined in RFC 7235 and 7230.
+/// Populate the global authentication store with credentials on a URL, if there are any.
 ///
-/// <https://datatracker.ietf.org/doc/html/rfc7235#section-2.2>
-/// <https://datatracker.ietf.org/doc/html/rfc7230#section-5.5>
-//
-// The "scheme" and "authority" components must match to retain authentication
-// The "authority", is composed of the host and port.
-//
-// The scheme must always be an exact match.
-// Note some clients such as Python's `requests` library allow an upgrade
-// from `http` to `https` but this is not spec-compliant.
-// <https://github.com/pypa/pip/blob/75f54cae9271179b8cc80435f92336c97e349f9d/src/pip/_vendor/requests/sessions.py#L133-L136>
-//
-// The host must always be an exact match.
-//
-// The port is only allowed to differ if it matches the "default port" for the scheme.
-// However, `url` (and therefore `reqwest`) sets the `port` to `None` if it matches the default port
-// so we do not need any special handling here.
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
-struct NetLoc {
-    scheme: String,
-    host: Option<String>,
-    port: Option<u16>,
-}
-
-impl From<&Url> for NetLoc {
-    fn from(url: &Url) -> Self {
-        Self {
-            scheme: url.scheme().to_string(),
-            host: url.host_str().map(str::to_string),
-            port: url.port(),
-        }
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use url::{ParseError, Url};
-
-    use crate::NetLoc;
-
-    #[test]
-    fn test_should_retain_auth() -> Result<(), ParseError> {
-        // Exact match (https)
-        assert_eq!(
-            NetLoc::from(&Url::parse("https://example.com")?),
-            NetLoc::from(&Url::parse("https://example.com")?)
-        );
-
-        // Exact match (with port)
-        assert_eq!(
-            NetLoc::from(&Url::parse("https://example.com:1234")?),
-            NetLoc::from(&Url::parse("https://example.com:1234")?)
-        );
-
-        // Exact match (http)
-        assert_eq!(
-            NetLoc::from(&Url::parse("http://example.com")?),
-            NetLoc::from(&Url::parse("http://example.com")?)
-        );
-
-        // Okay, path differs
-        assert_eq!(
-            NetLoc::from(&Url::parse("http://example.com/foo")?),
-            NetLoc::from(&Url::parse("http://example.com/bar")?)
-        );
-
-        // Okay, default port differs (https)
-        assert_eq!(
-            NetLoc::from(&Url::parse("https://example.com:443")?),
-            NetLoc::from(&Url::parse("https://example.com")?)
-        );
-
-        // Okay, default port differs (http)
-        assert_eq!(
-            NetLoc::from(&Url::parse("http://example.com:80")?),
-            NetLoc::from(&Url::parse("http://example.com")?)
-        );
-
-        // Mismatched scheme
-        assert_ne!(
-            NetLoc::from(&Url::parse("https://example.com")?),
-            NetLoc::from(&Url::parse("http://example.com")?)
-        );
-
-        // Mismatched scheme, we explicitly do not allow upgrade to https
-        assert_ne!(
-            NetLoc::from(&Url::parse("http://example.com")?),
-            NetLoc::from(&Url::parse("https://example.com")?)
-        );
-
-        // Mismatched host
-        assert_ne!(
-            NetLoc::from(&Url::parse("https://foo.com")?),
-            NetLoc::from(&Url::parse("https://bar.com")?)
-        );
-
-        // Mismatched port
-        assert_ne!(
-            NetLoc::from(&Url::parse("https://example.com:1234")?),
-            NetLoc::from(&Url::parse("https://example.com:5678")?)
-        );
-
-        // Mismatched port, with one as default for scheme
-        assert_ne!(
-            NetLoc::from(&Url::parse("https://example.com:443")?),
-            NetLoc::from(&Url::parse("https://example.com:5678")?)
-        );
-        assert_ne!(
-            NetLoc::from(&Url::parse("https://example.com:1234")?),
-            NetLoc::from(&Url::parse("https://example.com:443")?)
-        );
-
-        // Mismatched port, with default for a different scheme
-        assert_ne!(
-            NetLoc::from(&Url::parse("https://example.com:80")?),
-            NetLoc::from(&Url::parse("https://example.com")?)
-        );
-
-        Ok(())
-    }
+/// Returns `true` if the store was updated.
+pub fn store_credentials_from_url(url: &Url) -> bool {
+    GLOBAL_AUTH_STORE.set_from_url(url)
 }