diff --git a/Cargo.lock b/Cargo.lock index fee504a2deb9..590d3a366de7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3730,6 +3730,27 @@ dependencies = [ "test-case-core", ] +[[package]] +name = "test-log" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b319995299c65d522680decf80f2c108d85b861d81dfe340a10d16cee29d9e6" +dependencies = [ + "test-log-macros", + "tracing-subscriber", +] + +[[package]] +name = "test-log-macros" +version = "0.2.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c8f546451eaa38373f549093fe9fd05e7d2bade739e2ddf834b9968621d60107" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.58", +] + [[package]] name = "testing_logger" version = "0.1.1" @@ -4363,13 +4384,14 @@ version = "0.0.1" dependencies = [ "async-trait", "base64 0.22.0", - "clap", "http", + "insta", "once_cell", "reqwest", "reqwest-middleware", "rust-netrc", "tempfile", + "test-log", "thiserror", "tokio", "tracing", @@ -4491,6 +4513,8 @@ dependencies = [ "rustc-hash", "serde", "serde_json", + "uv-auth", + "uv-cache", "uv-normalize", ] diff --git a/crates/uv-auth/Cargo.toml b/crates/uv-auth/Cargo.toml index 87677f03d7f5..5dbec17edff1 100644 --- a/crates/uv-auth/Cargo.toml +++ b/crates/uv-auth/Cargo.toml @@ -6,7 +6,6 @@ edition = "2021" [dependencies] async-trait = { workspace = true } base64 = { workspace = true } -clap = { workspace = true, features = ["derive", "env"], optional = true } http = { workspace = true } once_cell = { workspace = true } reqwest = { workspace = true } @@ -21,3 +20,5 @@ urlencoding = { workspace = true } tempfile = { workspace = true } tokio = { workspace = true } wiremock = { workspace = true } +insta = { version = "1.36.1" } +test-log = { version = "0.2.15", features = ["trace"], default-features = false } diff --git a/crates/uv-auth/src/cache.rs b/crates/uv-auth/src/cache.rs new file mode 100644 index 000000000000..273eb2f64c85 --- /dev/null +++ b/crates/uv-auth/src/cache.rs @@ -0,0 +1,184 @@ +use std::sync::Arc; +use std::{collections::HashMap, sync::Mutex}; + +use crate::credentials::Credentials; +use crate::NetLoc; + +use tracing::trace; +use url::Url; + +type CacheKey = (NetLoc, Option); + +pub struct CredentialsCache { + store: Mutex>>, +} + +#[derive(Debug, Clone)] +pub enum CheckResponse { + /// The given credentials should be used and are not present in the cache. + Uncached(Arc), + /// Credentials were found in the cache. + Cached(Arc), + // Credentials were not found in the cache and none were provided. + None, +} + +impl CheckResponse { + /// Retrieve the credentials, if any. + pub fn get(&self) -> Option<&Credentials> { + match self { + Self::Cached(credentials) => Some(credentials.as_ref()), + Self::Uncached(credentials) => Some(credentials.as_ref()), + Self::None => None, + } + } + + /// Returns true if there are credentials with a password. + pub fn is_authenticated(&self) -> bool { + self.get() + .is_some_and(|credentials| credentials.password().is_some()) + } +} + +impl Default for CredentialsCache { + fn default() -> Self { + Self::new() + } +} + +impl CredentialsCache { + /// Create a new cache. + pub fn new() -> Self { + Self { + store: Mutex::new(HashMap::new()), + } + } + + /// Create an owned cache key. + fn key(url: &Url, username: Option) -> CacheKey { + (NetLoc::from(url), username) + } + + /// 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 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(&self, url: &Url, credentials: Option) -> CheckResponse { + let store = self.store.lock().unwrap(); + + let credentials = credentials.map(Arc::new); + let key = CredentialsCache::key( + url, + credentials + .as_ref() + .and_then(|credentials| credentials.username().map(str::to_string)), + ); + + if let Some(credentials) = credentials { + if credentials.password().is_some() { + trace!("Existing credentials include password, skipping cache"); + // No need to look-up, we have a password already + return CheckResponse::Uncached(credentials); + } + trace!("Existing credentials missing password, checking cache"); + let existing = store.get(&key); + existing + .cloned() + .map(CheckResponse::Cached) + .inspect(|_| trace!("Found cached credentials.")) + .unwrap_or_else(|| { + trace!("No credentials in cache, using existing credentials"); + CheckResponse::Uncached(credentials) + }) + } else { + trace!("No credentials on request, checking cache..."); + store + .get(&key) + .cloned() + .map(CheckResponse::Cached) + .inspect(|_| trace!("Found cached credentials.")) + .unwrap_or_else(|| { + trace!("No credentials in cache."); + CheckResponse::None + }) + } + } + + /// Update the cache with the given credentials if none exist. + pub(crate) fn set_default(&self, url: &Url, credentials: Arc) { + // Do not cache empty credentials + if credentials.is_empty() { + return; + } + + // Insert an entry for requests including the username + if let Some(username) = credentials.username() { + let key = CredentialsCache::key(url, Some(username.to_string())); + if !self.contains_key(&key) { + self.insert_entry(key, credentials.clone()); + } + } + + // Insert an entry for requests with no username + let key = CredentialsCache::key(url, None); + if !self.contains_key(&key) { + self.insert_entry(key, credentials.clone()); + } + } + + /// Update the cache with the given credentials. + pub(crate) fn insert(&self, url: &Url, credentials: Arc) { + // Do not cache empty credentials + if credentials.is_empty() { + return; + } + + // Insert an entry for requests including the username + if let Some(username) = credentials.username() { + self.insert_entry( + CredentialsCache::key(url, Some(username.to_string())), + credentials.clone(), + ); + } + + // Insert an entry for requests with no username + self.insert_entry(CredentialsCache::key(url, None), credentials.clone()); + } + + /// Private interface to update a cache entry. + fn insert_entry(&self, key: (NetLoc, Option), credentials: Arc) -> bool { + // Do not cache empty credentials + if credentials.is_empty() { + return false; + } + + let mut store = self.store.lock().unwrap(); + + // Always replace existing entries if we have a password + if credentials.password().is_some() { + store.insert(key, credentials.clone()); + return true; + } + + // If we only have a username, add a new entry or replace an existing entry if it doesn't have a password + let existing = store.get(&key); + if existing.is_none() + || existing.is_some_and(|credentials| credentials.password().is_none()) + { + store.insert(key, credentials.clone()); + return true; + } + + false + } + + /// Returns true if a key is in the cache. + fn contains_key(&self, key: &(NetLoc, Option)) -> bool { + let store = self.store.lock().unwrap(); + store.contains_key(key) + } +} diff --git a/crates/uv-auth/src/credentials.rs b/crates/uv-auth/src/credentials.rs new file mode 100644 index 000000000000..5bceac793019 --- /dev/null +++ b/crates/uv-auth/src/credentials.rs @@ -0,0 +1,277 @@ +use base64::prelude::BASE64_STANDARD; +use base64::read::DecoderReader; +use base64::write::EncoderWriter; + +use netrc::Netrc; +use reqwest::header::HeaderValue; +use reqwest::Request; +use std::io::Read; +use std::io::Write; +use url::Url; + +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct Credentials { + /// The name of the user for authentication. + /// + /// Unlike `reqwest`, empty usernames should be encoded as `None` instead of an empty string. + username: Option, + /// The password to use for authentication. + password: Option, +} + +impl Credentials { + pub fn new(username: Option, password: Option) -> Self { + debug_assert!( + username.is_none() + || username + .as_ref() + .is_some_and(|username| !username.is_empty()) + ); + Self { username, password } + } + + pub fn username(&self) -> Option<&str> { + self.username.as_deref() + } + + pub fn password(&self) -> Option<&str> { + self.password.as_deref() + } + + pub fn is_empty(&self) -> bool { + self.password.is_none() && self.username.is_none() + } + + /// Return [`Credentials`] for a [`Url`] from a [`Netrc`] file, if any. + /// + /// If a username is provided, it must match the login in the netrc file or [`None`] is returned. + pub fn from_netrc(netrc: &Netrc, url: &Url, username: Option<&str>) -> Option { + let host = url.host_str()?; + let entry = netrc + .hosts + .get(host) + .or_else(|| netrc.hosts.get("default"))?; + + // Ensure the username matches if provided + if username.is_some_and(|username| username != entry.login) { + return None; + }; + + Some(Credentials { + username: Some(entry.login.clone()), + password: Some(entry.password.clone()), + }) + } + + /// Parse [`Credentials`] from a URL, if any. + /// + /// Returns [`None`] if both [`Url::username`] and [`Url::password`] are not populated. + pub fn from_url(url: &Url) -> Option { + if url.username().is_empty() && url.password().is_none() { + return None; + } + Some(Self { + // Remove percent-encoding from URL credentials + // See + username: if url.username().is_empty() { + None + } else { + Some( + 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() + }), + }) + } + + /// Parse [`Credentials`] from an HTTP request, if any. + /// + /// Only HTTP Basic Authentication is supported. + pub fn from_request(request: &Request) -> Option { + // First, attempt to retrieve the credentials from the URL + Self::from_url(request.url()).or( + // Then, attempt to pull the credentials from the headers + request + .headers() + .get(reqwest::header::AUTHORIZATION) + .map(Self::from_header_value)?, + ) + } + + /// Parse [`Credentials`] from an authorization header, if any. + /// + /// Only HTTP Basic Authentication is supported. + /// [`None`] will be returned if another authoriziation scheme is detected. + /// + /// Panics if the authentication is not conformant to the HTTP Basic Authentication scheme: + /// - The contents must be base64 encoded + /// - There must be a `:` separator + pub(crate) fn from_header_value(header: &HeaderValue) -> Option { + let mut value = header.as_bytes().strip_prefix(b"Basic ")?; + let mut decoder = DecoderReader::new(&mut value, &BASE64_STANDARD); + let mut buf = String::new(); + decoder + .read_to_string(&mut buf) + .expect("HTTP Basic Authentication should be base64 encoded."); + let (username, password) = buf + .split_once(':') + .expect("HTTP Basic Authentication should include a `:` separator."); + let username = if username.is_empty() { + None + } else { + Some(username.to_string()) + }; + let password = if password.is_empty() { + None + } else { + Some(password.to_string()) + }; + Some(Self::new(username, password)) + } + + /// Create an HTTP Basic Authentication header for the credentials. + /// + /// Panics if the username or password cannot be base64 encoded. + pub(crate) fn to_header_value(&self) -> HeaderValue { + // See: + let mut buf = b"Basic ".to_vec(); + { + let mut encoder = EncoderWriter::new(&mut buf, &BASE64_STANDARD); + write!(encoder, "{}:", self.username().unwrap_or_default()) + .expect("Write to base64 encoder should succeed"); + if let Some(password) = self.password() { + write!(encoder, "{}", password).expect("Write to base64 encoder should succeed"); + } + } + let mut header = HeaderValue::from_bytes(&buf).expect("base64 is always valid HeaderValue"); + header.set_sensitive(true); + header + } + + /// Attach the credentials to the given request. + /// + /// Any existing credentials will be overridden. + #[must_use] + pub fn authenticate(&self, mut request: reqwest::Request) -> reqwest::Request { + request + .headers_mut() + .insert(reqwest::header::AUTHORIZATION, Self::to_header_value(self)); + request + } +} + +#[cfg(test)] +mod test { + use insta::assert_debug_snapshot; + + use super::*; + + #[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(), Some("user")); + assert_eq!(credentials.password(), Some("password")); + } + + #[test] + fn from_url_no_username() { + let url = &Url::parse("https://example.com/simple/first/").unwrap(); + let mut auth_url = url.clone(); + auth_url.set_password(Some("password")).unwrap(); + let credentials = Credentials::from_url(&auth_url).unwrap(); + assert_eq!(credentials.username(), None); + assert_eq!(credentials.password(), Some("password")); + } + + #[test] + fn from_url_no_password() { + let url = &Url::parse("https://example.com/simple/first/").unwrap(); + let mut auth_url = url.clone(); + auth_url.set_username("user").unwrap(); + let credentials = Credentials::from_url(&auth_url).unwrap(); + assert_eq!(credentials.username(), Some("user")); + assert_eq!(credentials.password(), None); + } + + #[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.authenticate(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_eq!(Credentials::from_header_value(&header), Some(credentials)); + } + + #[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.authenticate(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_eq!(Credentials::from_header_value(&header), Some(credentials)); + } + + #[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.authenticate(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_eq!(Credentials::from_header_value(&header), Some(credentials)); + } +} diff --git a/crates/uv-auth/src/keyring.rs b/crates/uv-auth/src/keyring.rs index d3aebe5326fb..7f3f0ad2e145 100644 --- a/crates/uv-auth/src/keyring.rs +++ b/crates/uv-auth/src/keyring.rs @@ -1,88 +1,161 @@ -use std::process::Command; +use std::{collections::HashSet, process::Command, sync::Mutex}; -use thiserror::Error; -use tracing::debug; +use tracing::{debug, instrument, warn}; use url::Url; -use crate::store::{BasicAuthData, Credential}; +use crate::credentials::Credentials; -/// Keyring provider to use for authentication +/// A backend for retrieving credentials from a keyring. /// -/// See -#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] -#[cfg_attr(feature = "clap", derive(clap::ValueEnum))] -pub enum KeyringProvider { - /// Will not use keyring for authentication - #[default] - Disabled, - /// Will use keyring CLI command for authentication - Subprocess, - // /// Not yet implemented - // Auto, - // /// Not implemented yet. Maybe use for this? - // Import, +/// See pip's implementation for reference +/// +#[derive(Debug)] +pub struct KeyringProvider { + /// Tracks host and username pairs with no credentials to avoid repeated queries. + cache: Mutex>, + backend: KeyringProviderBackend, } -#[derive(Debug, Error)] -pub enum Error { - #[error("Url is not valid Keyring target: {0}")] - NotKeyringTarget(String), - #[error(transparent)] - CliFailure(#[from] std::io::Error), - #[error(transparent)] - ParseFailed(#[from] std::string::FromUtf8Error), +#[derive(Debug)] +pub enum KeyringProviderBackend { + /// Use the `keyring` command to fetch credentials. + Subprocess, + #[cfg(test)] + Dummy(std::collections::HashMap<(String, &'static str), &'static str>), } -/// Get credentials from keyring for given url -/// -/// See `pip`'s KeyringCLIProvider -/// -pub fn get_keyring_subprocess_auth(url: &Url) -> Result, Error> { - let host = url.host_str(); - if host.is_none() { - return Err(Error::NotKeyringTarget( - "Should only use keyring for urls with host".to_string(), - )); - } - if url.password().is_some() { - return Err(Error::NotKeyringTarget( - "Url already contains password - keyring not required".to_string(), - )); - } - let username = match url.username() { - u if !u.is_empty() => u, - // this is the username keyring.get_credentials returns as username for GCP registry - _ => "oauth2accesstoken", - }; - debug!( - "Running `keyring get` for `{}` with username `{}`", - url.to_string(), - username - ); - let output = match Command::new("keyring") - .arg("get") - .arg(url.to_string()) - .arg(username) - .output() - { - Ok(output) if output.status.success() => Ok(Some( +impl KeyringProvider { + /// Create a new [`KeyringProvider::Subprocess`]. + pub fn subprocess() -> Self { + Self { + cache: Mutex::new(HashSet::new()), + backend: KeyringProviderBackend::Subprocess, + } + } + + /// Fetch credentials for the given [`Url`] from the keyring. + /// + /// Returns [`None`] if no password was found for the username or if any errors + /// are encountered in the keyring backend. + pub(crate) fn fetch(&self, url: &Url, username: &str) -> Option { + // Validate the request + debug_assert!( + url.host_str().is_some(), + "Should only use keyring for urls with host" + ); + debug_assert!( + url.password().is_none(), + "Should only use keyring for urls without a password" + ); + debug_assert!( + !username.is_empty(), + "Should only use keyring with a username" + ); + + let host = url.host_str()?; + + // Avoid expensive lookups by tracking previous attempts with no credentials. + // N.B. We cache missing credentials per host so no credentials are found for + // a host but would return credentials for some other URL in the same realm + // we may not find the credentials depending on which URL we see first. + // This behavior avoids adding ~80ms to every request when the subprocess keyring + // provider is being used, but makes assumptions about the typical keyring + // use-cases. + let mut cache = self.cache.lock().unwrap(); + + let key = (host.to_string(), username.to_string()); + if cache.contains(&key) { + debug!( + "Skipping keyring lookup for {username} at {host}, already attempted and found no credentials." + ); + return None; + } + + // Check the full URL first + // + let mut password = match self.backend { + KeyringProviderBackend::Subprocess => self.fetch_subprocess(url.as_str(), username), + #[cfg(test)] + KeyringProviderBackend::Dummy(ref store) => { + self.fetch_dummy(store, url.as_str(), username) + } + }; + // And fallback to a check for the host + if password.is_none() { + password = match self.backend { + KeyringProviderBackend::Subprocess => self.fetch_subprocess(host, username), + #[cfg(test)] + KeyringProviderBackend::Dummy(ref store) => self.fetch_dummy(store, host, username), + }; + } + + if password.is_none() { + cache.insert(key); + } + + password.map(|password| Credentials::new(Some(username.to_string()), Some(password))) + } + + #[instrument] + fn fetch_subprocess(&self, service_name: &str, username: &str) -> Option { + let output = Command::new("keyring") + .arg("get") + .arg(service_name) + .arg(username) + .output() + .inspect_err(|err| warn!("Failure running `keyring` command: {err}")) + .ok()?; + + if output.status.success() { + // On success, parse the newline terminated password String::from_utf8(output.stdout) - .map_err(Error::ParseFailed)? - .trim_end() - .to_owned(), - )), - Ok(_) => Ok(None), - Err(e) => Err(Error::CliFailure(e)), - }; - - output.map(|password| { - password.map(|password| { - Credential::Basic(BasicAuthData { - username: username.to_string(), - password: Some(password), - }) - }) - }) + .inspect_err(|err| warn!("Failed to parse response from `keyring` command: {err}")) + .ok() + .map(|password| password.trim_end().to_string()) + } else { + // On failure, no password was available + None + } + } + + #[cfg(test)] + fn fetch_dummy( + &self, + store: &std::collections::HashMap<(String, &'static str), &'static str>, + service_name: &str, + username: &str, + ) -> Option { + store + .get(&(service_name.to_string(), username)) + .map(|password| password.to_string()) + } + + /// Create a new provider with [`KeyringProviderBackend::Dummy`]. + #[cfg(test)] + pub fn dummy, T: IntoIterator>( + iter: T, + ) -> Self { + use std::collections::HashMap; + + Self { + cache: Mutex::new(HashSet::new()), + backend: KeyringProviderBackend::Dummy(HashMap::from_iter( + iter.into_iter() + .map(|((service, username), password)| ((service.into(), username), password)), + )), + } + } + + /// Create a new provider with no credentials available. + #[cfg(test)] + pub fn empty() -> Self { + use std::collections::HashMap; + + Self { + cache: Mutex::new(HashSet::new()), + backend: KeyringProviderBackend::Dummy(HashMap::new()), + } + } } #[cfg(test)] @@ -90,20 +163,138 @@ mod test { use super::*; #[test] - fn hostless_url_should_err() { + fn fetch_url_no_host() { let url = Url::parse("file:/etc/bin/").unwrap(); - let res = get_keyring_subprocess_auth(&url); - assert!(res.is_err()); - assert!(matches!(res.unwrap_err(), - Error::NotKeyringTarget(s) if s == "Should only use keyring for urls with host")); + let keyring = KeyringProvider::empty(); + // Panics due to debug assertion; returns `None` in production + let result = std::panic::catch_unwind(|| keyring.fetch(&url, "user")); + assert!(result.is_err()); + } + + #[test] + fn fetch_url_with_password() { + let url = Url::parse("https://user:password@example.com").unwrap(); + let keyring = KeyringProvider::empty(); + // Panics due to debug assertion; returns `None` in production + let result = std::panic::catch_unwind(|| keyring.fetch(&url, url.username())); + assert!(result.is_err()); + } + + #[test] + fn fetch_url_with_no_username() { + let url = Url::parse("https://example.com").unwrap(); + let keyring = KeyringProvider::empty(); + // Panics due to debug assertion; returns `None` in production + let result = std::panic::catch_unwind(|| keyring.fetch(&url, url.username())); + assert!(result.is_err()); + } + + #[test] + fn fetch_url_no_auth() { + let url = Url::parse("https://example.com").unwrap(); + let keyring = KeyringProvider::empty(); + let credentials = keyring.fetch(&url, "user"); + assert!(credentials.is_none()); + } + + #[test] + fn fetch_url() { + let url = Url::parse("https://example.com").unwrap(); + let keyring = KeyringProvider::dummy([((url.host_str().unwrap(), "user"), "password")]); + assert_eq!( + keyring.fetch(&url, "user"), + Some(Credentials::new( + Some("user".to_string()), + Some("password".to_string()) + )) + ); + assert_eq!( + keyring.fetch(&url.join("test").unwrap(), "user"), + Some(Credentials::new( + Some("user".to_string()), + Some("password".to_string()) + )) + ); + } + + #[test] + fn fetch_url_no_match() { + let url = Url::parse("https://example.com").unwrap(); + let keyring = KeyringProvider::dummy([(("other.com", "user"), "password")]); + let credentials = keyring.fetch(&url, "user"); + assert_eq!(credentials, None); + } + + #[test] + fn fetch_url_prefers_url_to_host() { + let url = Url::parse("https://example.com/").unwrap(); + let keyring = KeyringProvider::dummy([ + ((url.join("foo").unwrap().as_str(), "user"), "password"), + ((url.host_str().unwrap(), "user"), "other-password"), + ]); + assert_eq!( + keyring.fetch(&url.join("foo").unwrap(), "user"), + Some(Credentials::new( + Some("user".to_string()), + Some("password".to_string()) + )) + ); + assert_eq!( + keyring.fetch(&url, "user"), + Some(Credentials::new( + Some("user".to_string()), + Some("other-password".to_string()) + )) + ); + assert_eq!( + keyring.fetch(&url.join("bar").unwrap(), "user"), + Some(Credentials::new( + Some("user".to_string()), + Some("other-password".to_string()) + )) + ); } + /// Demonstrates "incorrect" behavior in our cache which avoids an expensive fetch of + /// credentials for _every_ request URL at the cost of inconsistent behavior when + /// credentials are not scoped to a realm. #[test] - fn passworded_url_should_err() { - let url = Url::parse("https://u:p@example.com").unwrap(); - let res = get_keyring_subprocess_auth(&url); - assert!(res.is_err()); - assert!(matches!(res.unwrap_err(), - Error::NotKeyringTarget(s) if s == "Url already contains password - keyring not required")); + fn fetch_url_caches_based_on_host() { + let url = Url::parse("https://example.com/").unwrap(); + let keyring = + KeyringProvider::dummy([((url.join("foo").unwrap().as_str(), "user"), "password")]); + + // If we attempt an unmatching URL first... + assert_eq!(keyring.fetch(&url.join("bar").unwrap(), "user"), None); + + // ... we will cache the missing credentials on subsequent attempts + assert_eq!(keyring.fetch(&url.join("foo").unwrap(), "user"), None); + } + + #[test] + fn fetch_url_username() { + let url = Url::parse("https://example.com").unwrap(); + let keyring = KeyringProvider::dummy([((url.host_str().unwrap(), "user"), "password")]); + let credentials = keyring.fetch(&url, "user"); + assert_eq!( + credentials, + Some(Credentials::new( + Some("user".to_string()), + Some("password".to_string()) + )) + ); + } + + #[test] + fn fetch_url_username_no_match() { + let url = Url::parse("https://example.com").unwrap(); + let keyring = KeyringProvider::dummy([((url.host_str().unwrap(), "foo"), "password")]); + let credentials = keyring.fetch(&url, "bar"); + assert_eq!(credentials, None); + + // Still fails if we have `foo` in the URL itself + let url = Url::parse("https://foo@example.com").unwrap(); + let credentials = keyring.fetch(&url, "bar"); + assert_eq!(credentials, None); } } diff --git a/crates/uv-auth/src/lib.rs b/crates/uv-auth/src/lib.rs index 5dcb2b678ec6..81fd0aeb85f4 100644 --- a/crates/uv-auth/src/lib.rs +++ b/crates/uv-auth/src/lib.rs @@ -1,139 +1,19 @@ +mod cache; +mod credentials; mod keyring; mod middleware; -mod store; +mod netloc; + +use cache::CredentialsCache; 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 = Lazy::new(AuthenticationStore::default); +// TODO(zanieb): Consider passing a cache explicitly throughout -/// Used to determine if authentication information should be retained on a new URL. -/// Based on the specification defined in RFC 7235 and 7230. +/// Global authentication cache for a uv invocation /// -/// -/// -// -// 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. -// -// -// 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, - port: Option, -} - -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(()) - } -} +/// This is used to share credentials across uv clients. +pub(crate) static CREDENTIALS_CACHE: Lazy = Lazy::new(CredentialsCache::default); diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index 5d5e14c4a373..f028d7dc9f78 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -1,38 +1,69 @@ +use std::sync::Arc; + use http::Extensions; -use std::path::Path; use netrc::Netrc; -use reqwest::{header::HeaderValue, Request, Response}; +use reqwest::{Request, Response}; use reqwest_middleware::{Middleware, Next}; -use tracing::{debug, warn}; +use tracing::{debug, trace}; use crate::{ - keyring::{get_keyring_subprocess_auth, KeyringProvider}, - store::Credential, - GLOBAL_AUTH_STORE, + cache::CheckResponse, credentials::Credentials, CredentialsCache, KeyringProvider, + CREDENTIALS_CACHE, }; /// A middleware that adds basic authentication to requests based on the netrc file and the keyring. /// /// Netrc support Based on: . pub struct AuthMiddleware { - nrc: Option, - keyring_provider: KeyringProvider, + netrc: Option, + keyring: Option, + cache: Option, } impl AuthMiddleware { - pub fn new(keyring_provider: KeyringProvider) -> Self { + pub fn new() -> Self { Self { - nrc: Netrc::new().ok(), - keyring_provider, + netrc: Netrc::new().ok(), + keyring: None, + cache: None, } } - pub fn from_netrc_file(file: &Path, keyring_provider: KeyringProvider) -> Self { - Self { - nrc: Netrc::from_file(file).ok(), - keyring_provider, - } + /// Configure the [`Netrc`] credential file to use. + /// + /// `None` disables authentication via netrc. + #[must_use] + pub fn with_netrc(mut self, netrc: Option) -> Self { + self.netrc = netrc; + self + } + + /// Configure the [`KeyringProvider`] to use. + #[must_use] + pub fn with_keyring(mut self, keyring: Option) -> Self { + self.keyring = keyring; + self + } + + /// Configure the [`CredentialsCache`] to use. + #[must_use] + pub fn with_cache(mut self, cache: CredentialsCache) -> Self { + self.cache = Some(cache); + self + } + + /// Get the configured authentication store. + /// + /// If not set, the global store is used. + fn cache(&self) -> &CredentialsCache { + self.cache.as_ref().unwrap_or(&CREDENTIALS_CACHE) + } +} + +impl Default for AuthMiddleware { + fn default() -> Self { + AuthMiddleware::new() } } @@ -40,145 +71,516 @@ impl AuthMiddleware { impl Middleware for AuthMiddleware { async fn handle( &self, - mut req: Request, - _extensions: &mut Extensions, + mut request: Request, + extensions: &mut Extensions, next: Next<'_>, ) -> reqwest_middleware::Result { - let url = req.url().clone(); + // 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(); + if let Some(username) = credentials + .as_ref() + .and_then(|credentials| credentials.username()) + { + let _ = url.set_username(username); + }; + if credentials + .as_ref() + .and_then(|credentials| credentials.password()) + .is_some() + { + let _ = url.set_password(Some("****")); + }; + url.to_string() + } else { + request.url().to_string() + }; + trace!("Handling request for {url}"); - // If the request already has an authorization header, we don't need to do anything. - // This gives in-URL credentials precedence over the netrc file. - if req.headers().contains_key(reqwest::header::AUTHORIZATION) { - debug!("Request already has an authorization header: {url}"); - return next.run(req, _extensions).await; - } + // Then check for credentials in (2) the cache + let credentials = self.cache().check(request.url(), credentials); - // Try auth strategies in order of precedence: - if let Some(stored_auth) = GLOBAL_AUTH_STORE.get(&url) { - // If we've already seen this URL, we can use the stored credentials - if let Some(auth) = stored_auth { - debug!("Adding authentication to already-seen URL: {url}"); - req.headers_mut().insert( - reqwest::header::AUTHORIZATION, - basic_auth(auth.username(), auth.password()), - ); + // Track credentials that we might want to insert into the cache + let mut new_credentials = None; + + // If already authenticated (including a password), don't query other services + if credentials.is_authenticated() { + match credentials { + // If we get credentials from the cache, update the request + 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::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| { + trace!("Checking netrc for credentials for {url}"); + Credentials::from_netrc( + netrc, + request.url(), + credentials + .get() + .and_then(|credentials| credentials.username()), + ) + }) { + debug!("Found credentials in netrc file for {url}"); + request = credentials.authenticate(request); + new_credentials = Some(Arc::new(credentials)); + // (4) The keyring + // N.B. The keyring provider performs lookups for the exact URL then + // falls back to the host, but we cache the result per host so if a keyring + // implementation returns different credentials for different URLs in the + // same realm we will use the wrong credentials. + } else if let Some(credentials) = self.keyring.as_ref().and_then(|keyring| { + if let Some(username) = credentials + .get() + .and_then(|credentials| credentials.username()) + { + debug!("Checking keyring for credentials for {url}"); + keyring.fetch(request.url(), username) } else { - debug!("No credentials found for already-seen URL: {url}"); + trace!("Skipping keyring lookup for {url} with no username"); + None } - } else if let Some(auth) = self.nrc.as_ref().and_then(|nrc| { - // If we find a matching entry in the netrc file, we can use it - url.host_str() - .and_then(|host| nrc.hosts.get(host).or_else(|| nrc.hosts.get("default"))) }) { - let auth = Credential::from(auth.to_owned()); - req.headers_mut().insert( - reqwest::header::AUTHORIZATION, - basic_auth(auth.username(), auth.password()), - ); - GLOBAL_AUTH_STORE.set(&url, Some(auth)); - } else if matches!(self.keyring_provider, KeyringProvider::Subprocess) { - // If we have keyring support enabled, we check there as well - match get_keyring_subprocess_auth(&url) { - Ok(Some(auth)) => { - req.headers_mut().insert( - reqwest::header::AUTHORIZATION, - basic_auth(auth.username(), auth.password()), - ); - GLOBAL_AUTH_STORE.set(&url, Some(auth)); - } - Ok(None) => { - debug!("No keyring credentials found for {url}"); - } - Err(e) => { - warn!("Failed to get keyring credentials for {url}: {e}"); + 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::Uncached(credentials) => new_credentials = Some(credentials.clone()), + CheckResponse::None => { + debug!("No credentials found for {url}") } } } - // If we still don't have any credentials, we save the URL so we don't have to check netrc or keyring again - if !req.headers().contains_key(reqwest::header::AUTHORIZATION) { - debug!("No credentials found for: {url}"); - GLOBAL_AUTH_STORE.set(&url, None); - } + if let Some(credentials) = new_credentials { + let url = request.url().clone(); - next.run(req, _extensions).await - } -} + // Update the default credentials eagerly since requests are made concurrently + // and we want to avoid expensive credential lookups + self.cache().set_default(&url, credentials.clone()); -/// Create a `HeaderValue` for basic authentication. -/// -/// Source: -fn basic_auth(username: U, password: Option

) -> HeaderValue -where - U: std::fmt::Display, - P: std::fmt::Display, -{ - use base64::prelude::BASE64_STANDARD; - use base64::write::EncoderWriter; - use std::io::Write; + let result = next.run(request, extensions).await; - 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); + // Only update the cache with new credentials on a successful request + if result + .as_ref() + .is_ok_and(|response| response.error_for_status_ref().is_ok()) + { + trace!("Updating cached credentials for {url}"); + self.cache().insert(&url, credentials) + }; + result + } else { + next.run(request, extensions).await } } - let mut header = HeaderValue::from_bytes(&buf).expect("base64 is always valid HeaderValue"); - header.set_sensitive(true); - header } #[cfg(test)] mod tests { + use std::io::Write; use reqwest::Client; - use reqwest_middleware::ClientBuilder; use tempfile::NamedTempFile; - use wiremock::matchers::{basic_auth, method, path}; + use test_log::test; + + use url::Url; + use wiremock::matchers::{basic_auth, method}; use wiremock::{Mock, MockServer, ResponseTemplate}; use super::*; - const NETRC: &str = r#"default login myuser password mypassword"#; + type Error = Box; - #[tokio::test] - async fn test_init() -> Result<(), Box> { + async fn start_test_server(username: &'static str, password: &'static str) -> MockServer { let server = MockServer::start().await; Mock::given(method("GET")) - .and(path("/hello")) - .and(basic_auth("myuser", "mypassword")) + .and(basic_auth(username, password)) .respond_with(ResponseTemplate::new(200)) .mount(&server) .await; - let status = ClientBuilder::new(Client::builder().build()?) - .build() - .get(format!("{}/hello", &server.uri())) - .send() - .await? - .status(); + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(401)) + .mount(&server) + .await; + + server + } + + fn test_client_builder() -> reqwest_middleware::ClientBuilder { + reqwest_middleware::ClientBuilder::new( + Client::builder() + .build() + .expect("Reqwest client should build"), + ) + } + + #[test(tokio::test)] + async fn test_no_credentials() -> Result<(), Error> { + let server = start_test_server("user", "password").await; + let client = test_client_builder() + .with(AuthMiddleware::new().with_cache(CredentialsCache::new())) + .build(); + + assert_eq!( + client + .get(format!("{}/foo", server.uri())) + .send() + .await? + .status(), + 401 + ); + + assert_eq!( + client + .get(format!("{}/bar", server.uri())) + .send() + .await? + .status(), + 401 + ); + + Ok(()) + } + + #[test(tokio::test)] + async fn test_credentials_in_url() -> Result<(), Error> { + let username = "user"; + let password = "password"; + + let server = start_test_server(username, password).await; + let client = test_client_builder() + .with(AuthMiddleware::new().with_cache(CredentialsCache::new())) + .build(); + + let base_url = Url::parse(&server.uri())?; + + let mut url = base_url.clone(); + url.set_username(username).unwrap(); + url.set_password(Some(password)).unwrap(); + assert_eq!(client.get(url).send().await?.status(), 200); + + // Works for a URL without credentials now + assert_eq!( + client.get(server.uri()).send().await?.status(), + 200, + "Subsequent requests should not require credentials" + ); + assert_eq!( + client + .get(format!("{}/foo", server.uri())) + .send() + .await? + .status(), + 200, + "Subsequent requests can be to different paths in the same realm" + ); + + let mut url = base_url.clone(); + url.set_username(username).unwrap(); + url.set_password(Some("invalid")).unwrap(); + assert_eq!( + client.get(url).send().await?.status(), + 401, + "Credentials in the URL should take precedence and fail" + ); + + Ok(()) + } + + #[test(tokio::test)] + async fn test_credentials_in_url_username_only() -> Result<(), Error> { + let username = "user"; + let password = ""; + + let server = start_test_server(username, password).await; + let client = test_client_builder() + .with(AuthMiddleware::new().with_cache(CredentialsCache::new())) + .build(); + + let base_url = Url::parse(&server.uri())?; - assert_eq!(status, 404); + let mut url = base_url.clone(); + url.set_username(username).unwrap(); + url.set_password(None).unwrap(); + assert_eq!(client.get(url).send().await?.status(), 200); + + // Works for a URL without credentials now + assert_eq!( + client.get(server.uri()).send().await?.status(), + 200, + "Subsequent requests should not require credentials" + ); + assert_eq!( + client + .get(format!("{}/foo", server.uri())) + .send() + .await? + .status(), + 200, + "Subsequent requests can be to different paths in the same realm" + ); + + let mut url = base_url.clone(); + url.set_username(username).unwrap(); + url.set_password(Some("invalid")).unwrap(); + assert_eq!( + client.get(url).send().await?.status(), + 401, + "Credentials in the URL should take precedence and fail" + ); + + assert_eq!( + client.get(server.uri()).send().await?.status(), + 200, + "Subsequent requests should not use the invalid credentials" + ); + + Ok(()) + } + + #[test(tokio::test)] + async fn test_netrc_file_default_host() -> Result<(), Error> { + let username = "user"; + let password = "password"; + + let mut netrc_file = NamedTempFile::new()?; + writeln!(netrc_file, "default login {username} password {password}")?; + + let server = start_test_server(username, password).await; + let client = test_client_builder() + .with( + AuthMiddleware::new() + .with_cache(CredentialsCache::new()) + .with_netrc(Netrc::from_file(netrc_file.path()).ok()), + ) + .build(); + + assert_eq!( + client.get(server.uri()).send().await?.status(), + 200, + "Credentials should be pulled from the netrc file" + ); + + let mut url = Url::parse(&server.uri())?; + url.set_username(username).unwrap(); + url.set_password(Some("invalid")).unwrap(); + assert_eq!( + client.get(url).send().await?.status(), + 401, + "Credentials in the URL should take precedence and fail" + ); + + assert_eq!( + client.get(server.uri()).send().await?.status(), + 200, + "Subsequent requests should not use the invalid credentials" + ); + + Ok(()) + } + + #[test(tokio::test)] + async fn test_netrc_file_matching_host() -> Result<(), Error> { + let username = "user"; + let password = "password"; + let server = start_test_server(username, password).await; + let base_url = Url::parse(&server.uri())?; + + let mut netrc_file = NamedTempFile::new()?; + writeln!( + netrc_file, + r#"machine {} login {username} password {password}"#, + base_url.host_str().unwrap() + )?; + + let client = test_client_builder() + .with( + AuthMiddleware::new() + .with_cache(CredentialsCache::new()) + .with_netrc(Some( + Netrc::from_file(netrc_file.path()).expect("Test has valid netrc file"), + )), + ) + .build(); + + assert_eq!( + client.get(server.uri()).send().await?.status(), + 200, + "Credentials should be pulled from the netrc file" + ); + + let mut url = base_url.clone(); + url.set_username(username).unwrap(); + url.set_password(Some("invalid")).unwrap(); + assert_eq!( + client.get(url).send().await?.status(), + 401, + "Credentials in the URL should take precedence and fail" + ); + + assert_eq!( + client.get(server.uri()).send().await?.status(), + 200, + "Subsequent requests should not use the invalid credentials" + ); + + Ok(()) + } + + #[test(tokio::test)] + async fn test_netrc_file_mismatched_host() -> Result<(), Error> { + let username = "user"; + let password = "password"; + let server = start_test_server(username, password).await; let mut netrc_file = NamedTempFile::new()?; - writeln!(netrc_file, "{}", NETRC)?; - - let status = ClientBuilder::new(Client::builder().build()?) - .with(AuthMiddleware::from_netrc_file( - netrc_file.path(), - KeyringProvider::Disabled, - )) - .build() - .get(format!("{}/hello", &server.uri())) - .send() - .await? - .status(); - - assert_eq!(status, 200); + writeln!( + netrc_file, + r#"machine example.com login {username} password {password}"#, + )?; + + let client = test_client_builder() + .with( + AuthMiddleware::new() + .with_cache(CredentialsCache::new()) + .with_netrc(Some( + Netrc::from_file(netrc_file.path()).expect("Test has valid netrc file"), + )), + ) + .build(); + + assert_eq!( + client.get(server.uri()).send().await?.status(), + 401, + "Credentials should not be pulled from the netrc file due to host mistmatch" + ); + + let mut url = Url::parse(&server.uri())?; + url.set_username(username).unwrap(); + url.set_password(Some(password)).unwrap(); + assert_eq!( + client.get(url).send().await?.status(), + 200, + "Credentials in the URL should still work" + ); + + Ok(()) + } + + #[test(tokio::test)] + async fn test_netrc_file_mismatched_username() -> Result<(), Error> { + let username = "user"; + let password = "password"; + let server = start_test_server(username, password).await; + let base_url = Url::parse(&server.uri())?; + + let mut netrc_file = NamedTempFile::new()?; + writeln!( + netrc_file, + r#"machine {} login {username} password {password}"#, + base_url.host_str().unwrap() + )?; + + let client = test_client_builder() + .with( + AuthMiddleware::new() + .with_cache(CredentialsCache::new()) + .with_netrc(Some( + Netrc::from_file(netrc_file.path()).expect("Test has valid netrc file"), + )), + ) + .build(); + + let mut url = base_url.clone(); + url.set_username("other-user").unwrap(); + assert_eq!( + client.get(url).send().await?.status(), + 401, + "The netrc password should not be used due to a username mismatch" + ); + + let mut url = base_url.clone(); + url.set_username("user").unwrap(); + assert_eq!( + client.get(url).send().await?.status(), + 200, + "The netrc password should be used for a matching user" + ); + + Ok(()) + } + + #[test(tokio::test)] + async fn test_keyring() -> Result<(), Error> { + let username = "user"; + let password = "password"; + let server = start_test_server(username, password).await; + let base_url = Url::parse(&server.uri())?; + + let client = test_client_builder() + .with( + AuthMiddleware::new() + .with_cache(CredentialsCache::new()) + .with_keyring(Some(KeyringProvider::dummy([( + (base_url.host_str().unwrap(), username), + password, + )]))), + ) + .build(); + + assert_eq!( + client.get(server.uri()).send().await?.status(), + 401, + "Credentials are not pulled from the keyring without a username" + ); + + let mut url = base_url.clone(); + url.set_username(username).unwrap(); + assert_eq!( + client.get(url).send().await?.status(), + 200, + "Credentials for the username should be pulled from the keyring" + ); + + let mut url = base_url.clone(); + url.set_username(username).unwrap(); + url.set_password(Some("invalid")).unwrap(); + assert_eq!( + client.get(url).send().await?.status(), + 401, + "Password in the URL should take precedence and fail" + ); + + let mut url = base_url.clone(); + url.set_username(username).unwrap(); + assert_eq!( + client.get(url.clone()).send().await?.status(), + 200, + "Subsequent requests should not use the invalid password" + ); + + let mut url = base_url.clone(); + url.set_username("other_user").unwrap(); + assert_eq!( + client.get(url).send().await?.status(), + 401, + "Credentials are not pulled from the keyring when given another username" + ); + Ok(()) } } diff --git a/crates/uv-auth/src/netloc.rs b/crates/uv-auth/src/netloc.rs new file mode 100644 index 000000000000..65e348117365 --- /dev/null +++ b/crates/uv-auth/src/netloc.rs @@ -0,0 +1,125 @@ +use url::Url; + +/// Used to determine if authentication information should be retained on a new URL. +/// Based on the specification defined in RFC 7235 and 7230. +/// +/// +/// +// +// 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. +// +// +// 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)] +pub(crate) struct NetLoc { + scheme: String, + host: Option, + port: Option, +} + +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(()) + } +} diff --git a/crates/uv-auth/src/store.rs b/crates/uv-auth/src/store.rs deleted file mode 100644 index 9ac85234e1e7..000000000000 --- a/crates/uv-auth/src/store.rs +++ /dev/null @@ -1,170 +0,0 @@ -use std::{collections::HashMap, sync::Mutex}; - -use netrc::Authenticator; -use tracing::warn; -use url::Url; - -use crate::NetLoc; - -#[derive(Clone, Debug, PartialEq)] -pub enum Credential { - Basic(BasicAuthData), - UrlEncoded(UrlAuthData), -} - -impl Credential { - pub fn username(&self) -> &str { - match self { - Credential::Basic(auth) => &auth.username, - Credential::UrlEncoded(auth) => &auth.username, - } - } - pub fn password(&self) -> Option<&str> { - match self { - Credential::Basic(auth) => auth.password.as_deref(), - Credential::UrlEncoded(auth) => auth.password.as_deref(), - } - } -} - -impl From for Credential { - fn from(auth: Authenticator) -> Self { - Credential::Basic(BasicAuthData { - username: auth.login, - password: Some(auth.password), - }) - } -} - -// Used for URL encoded auth in User info -// -#[derive(Clone, Debug, PartialEq)] -pub struct UrlAuthData { - pub username: String, - pub password: Option, -} - -impl UrlAuthData { - pub fn apply_to_url(&self, mut url: Url) -> Url { - url.set_username(&self.username) - .unwrap_or_else(|()| warn!("Failed to set username")); - url.set_password(self.password.as_deref()) - .unwrap_or_else(|()| warn!("Failed to set password")); - url - } -} - -// HttpBasicAuth - Used for netrc and keyring auth -// -#[derive(Clone, Debug, PartialEq)] -pub struct BasicAuthData { - pub username: String, - pub password: Option, -} - -pub struct AuthenticationStore { - credentials: Mutex>>, -} - -impl Default for AuthenticationStore { - fn default() -> Self { - Self::new() - } -} - -impl AuthenticationStore { - pub fn new() -> Self { - Self { - credentials: Mutex::new(HashMap::new()), - } - } - - pub fn get(&self, url: &Url) -> Option> { - let netloc = NetLoc::from(url); - let credentials = self.credentials.lock().unwrap(); - credentials.get(&netloc).cloned() - } - - pub fn set(&self, url: &Url, auth: Option) { - let netloc = NetLoc::from(url); - let mut credentials = self.credentials.lock().unwrap(); - credentials.insert(netloc, auth); - } - - /// Store in-URL credentials for future use. - pub fn save_from_url(&self, url: &Url) { - let netloc = NetLoc::from(url); - let mut credentials = self.credentials.lock().unwrap(); - if url.username().is_empty() { - // No credentials to save - return; - } - let auth = UrlAuthData { - // Using the encoded username can break authentication when `@` is converted to `%40` - // so we decode it for storage; RFC7617 does not explicitly say that authentication should - // not be percent-encoded, but the omission of percent-encoding from all encoding discussion - // indicates that it probably should not be done. - username: urlencoding::decode(url.username()) - .expect("An encoded username should always decode") - .into_owned(), - password: url.password().map(str::to_string), - }; - credentials.insert(netloc, Some(Credential::UrlEncoded(auth))); - } -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn store_set_and_get() { - let store = AuthenticationStore::new(); - let url = Url::parse("https://example1.com/simple/").unwrap(); - let not_set_res = store.get(&url); - assert!(not_set_res.is_none()); - - let found_first_url = Url::parse("https://example2.com/simple/first/").unwrap(); - let not_found_first_url = Url::parse("https://example3.com/simple/first/").unwrap(); - - store.set( - &found_first_url, - Some(Credential::Basic(BasicAuthData { - username: "u".to_string(), - password: Some("p".to_string()), - })), - ); - store.set(¬_found_first_url, None); - - let found_second_url = Url::parse("https://example2.com/simple/second/").unwrap(); - let not_found_second_url = Url::parse("https://example3.com/simple/second/").unwrap(); - - let found_res = store.get(&found_second_url); - assert!(found_res.is_some()); - let found_res = found_res.unwrap(); - assert!(matches!(found_res, Some(Credential::Basic(_)))); - - let not_found_res = store.get(¬_found_second_url); - assert!(not_found_res.is_some()); - let not_found_res = not_found_res.unwrap(); - assert!(not_found_res.is_none()); - } - - #[test] - fn store_save_from_url() { - let store = AuthenticationStore::new(); - let url = Url::parse("https://u:p@example.com/simple/").unwrap(); - - store.save_from_url(&url); - - let found_res = store.get(&url); - assert!(found_res.is_some()); - let found_res = found_res.unwrap(); - assert!(matches!(found_res, Some(Credential::UrlEncoded(_)))); - - let url = Url::parse("https://example2.com/simple/").unwrap(); - store.save_from_url(&url); - let found_res = store.get(&url); - assert!(found_res.is_none()); - } -} diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs index fffcf08488b9..37850aafe3bf 100644 --- a/crates/uv-client/src/base_client.rs +++ b/crates/uv-client/src/base_client.rs @@ -9,7 +9,8 @@ use std::fmt::Debug; use std::ops::Deref; use std::path::Path; use tracing::debug; -use uv_auth::{AuthMiddleware, KeyringProvider}; +use uv_auth::AuthMiddleware; +use uv_configuration::KeyringProviderType; use uv_fs::Simplified; use uv_version::version; use uv_warnings::warn_user_once; @@ -21,7 +22,7 @@ use crate::Connectivity; /// A builder for an [`BaseClient`]. #[derive(Debug, Clone)] pub struct BaseClientBuilder<'a> { - keyring_provider: KeyringProvider, + keyring: KeyringProviderType, native_tls: bool, retries: u32, connectivity: Connectivity, @@ -39,7 +40,7 @@ impl Default for BaseClientBuilder<'_> { impl BaseClientBuilder<'_> { pub fn new() -> Self { Self { - keyring_provider: KeyringProvider::default(), + keyring: KeyringProviderType::default(), native_tls: false, connectivity: Connectivity::Online, retries: 3, @@ -52,8 +53,8 @@ impl BaseClientBuilder<'_> { impl<'a> BaseClientBuilder<'a> { #[must_use] - pub fn keyring_provider(mut self, keyring_provider: KeyringProvider) -> Self { - self.keyring_provider = keyring_provider; + pub fn keyring(mut self, keyring_type: KeyringProviderType) -> Self { + self.keyring = keyring_type; self } @@ -169,7 +170,8 @@ impl<'a> BaseClientBuilder<'a> { let client = client.with(retry_strategy); // Initialize the authentication middleware to set headers. - let client = client.with(AuthMiddleware::new(self.keyring_provider)); + let client = + client.with(AuthMiddleware::new().with_keyring(self.keyring.to_provider())); client.build() } diff --git a/crates/uv-client/src/registry_client.rs b/crates/uv-client/src/registry_client.rs index 5873058e07ec..c28782e57562 100644 --- a/crates/uv-client/src/registry_client.rs +++ b/crates/uv-client/src/registry_client.rs @@ -20,9 +20,9 @@ use pep440_rs::Version; use pep508_rs::MarkerEnvironment; use platform_tags::Platform; use pypi_types::{Metadata23, SimpleJson}; -use uv_auth::KeyringProvider; use uv_cache::{Cache, CacheBucket, WheelCache}; use uv_configuration::IndexStrategy; +use uv_configuration::KeyringProviderType; use uv_normalize::PackageName; use crate::base_client::{BaseClient, BaseClientBuilder}; @@ -37,7 +37,7 @@ use crate::{CachedClient, CachedClientError, Error, ErrorKind}; pub struct RegistryClientBuilder<'a> { index_urls: IndexUrls, index_strategy: IndexStrategy, - keyring_provider: KeyringProvider, + keyring: KeyringProviderType, native_tls: bool, retries: u32, connectivity: Connectivity, @@ -52,7 +52,7 @@ impl RegistryClientBuilder<'_> { Self { index_urls: IndexUrls::default(), index_strategy: IndexStrategy::default(), - keyring_provider: KeyringProvider::default(), + keyring: KeyringProviderType::default(), native_tls: false, cache, connectivity: Connectivity::Online, @@ -78,8 +78,8 @@ impl<'a> RegistryClientBuilder<'a> { } #[must_use] - pub fn keyring_provider(mut self, keyring_provider: KeyringProvider) -> Self { - self.keyring_provider = keyring_provider; + pub fn keyring(mut self, keyring_type: KeyringProviderType) -> Self { + self.keyring = keyring_type; self } @@ -145,7 +145,7 @@ impl<'a> RegistryClientBuilder<'a> { .retries(self.retries) .connectivity(self.connectivity) .native_tls(self.native_tls) - .keyring_provider(self.keyring_provider) + .keyring(self.keyring) .build(); let timeout = client.timeout(); diff --git a/crates/uv-configuration/Cargo.toml b/crates/uv-configuration/Cargo.toml index 8a176161ee97..bc4029869ba9 100644 --- a/crates/uv-configuration/Cargo.toml +++ b/crates/uv-configuration/Cargo.toml @@ -14,6 +14,8 @@ workspace = true [dependencies] pep508_rs = { workspace = true } +uv-cache = { workspace = true } +uv-auth = { workspace = true } uv-normalize = { workspace = true } anyhow = { workspace = true } diff --git a/crates/uv-configuration/src/authentication.rs b/crates/uv-configuration/src/authentication.rs new file mode 100644 index 000000000000..b4dc82320b43 --- /dev/null +++ b/crates/uv-configuration/src/authentication.rs @@ -0,0 +1,26 @@ +use uv_auth::{self, KeyringProvider}; + +/// Keyring provider type to use for credential lookup. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] +#[cfg_attr(feature = "clap", derive(clap::ValueEnum))] +pub enum KeyringProviderType { + /// Do not use keyring for credential lookup. + #[default] + Disabled, + /// Use the `keyring` command for credential lookup. + Subprocess, + // /// Not yet implemented + // Auto, + // /// Not implemented yet. Maybe use for this? + // Import, +} +// See for details. + +impl KeyringProviderType { + pub fn to_provider(&self) -> Option { + match self { + Self::Disabled => None, + Self::Subprocess => Some(KeyringProvider::subprocess()), + } + } +} diff --git a/crates/uv-configuration/src/lib.rs b/crates/uv-configuration/src/lib.rs index 665568f2bcbe..dce1dd1fde11 100644 --- a/crates/uv-configuration/src/lib.rs +++ b/crates/uv-configuration/src/lib.rs @@ -1,3 +1,4 @@ +pub use authentication::*; pub use build_options::*; pub use config_settings::*; pub use constraints::*; @@ -5,6 +6,7 @@ pub use name_specifiers::*; pub use overrides::*; pub use package_options::*; +mod authentication; mod build_options; mod config_settings; mod constraints; diff --git a/crates/uv/Cargo.toml b/crates/uv/Cargo.toml index b34b9757b1ed..df4120ad7c00 100644 --- a/crates/uv/Cargo.toml +++ b/crates/uv/Cargo.toml @@ -20,7 +20,7 @@ pep508_rs = { workspace = true } platform-tags = { workspace = true } pypi-types = { workspace = true } requirements-txt = { workspace = true, features = ["http"] } -uv-auth = { workspace = true, features = ["clap"] } +uv-auth = { workspace = true } uv-cache = { workspace = true, features = ["clap"] } uv-client = { workspace = true } uv-dispatch = { workspace = true } diff --git a/crates/uv/src/cli.rs b/crates/uv/src/cli.rs index 7e6045eddcb3..06d90826ad93 100644 --- a/crates/uv/src/cli.rs +++ b/crates/uv/src/cli.rs @@ -6,10 +6,10 @@ use chrono::{DateTime, Days, NaiveDate, NaiveTime, Utc}; use clap::{Args, Parser, Subcommand}; use distribution_types::{FlatIndexLocation, IndexUrl}; -use uv_auth::KeyringProvider; use uv_cache::CacheArgs; -use uv_configuration::IndexStrategy; -use uv_configuration::{ConfigSettingEntry, PackageNameSpecifier}; +use uv_configuration::{ + ConfigSettingEntry, IndexStrategy, KeyringProviderType, PackageNameSpecifier, +}; use uv_normalize::{ExtraName, PackageName}; use uv_resolver::{AnnotationStyle, PreReleaseMode, ResolutionMode}; use uv_toolchain::PythonVersion; @@ -365,7 +365,7 @@ pub(crate) struct PipCompileArgs { /// Due to not having Python imports, only `--keyring-provider subprocess` argument is currently /// implemented `uv` will try to use `keyring` via CLI when this flag is used. #[clap(long, default_value_t, value_enum, env = "UV_KEYRING_PROVIDER")] - pub(crate) keyring_provider: KeyringProvider, + pub(crate) keyring_provider: KeyringProviderType, /// Locations to search for candidate distributions, beyond those found in the indexes. /// @@ -575,7 +575,7 @@ pub(crate) struct PipSyncArgs { /// Function's similar to `pip`'s `--keyring-provider subprocess` argument, /// `uv` will try to use `keyring` via CLI when this flag is used. #[clap(long, default_value_t, value_enum, env = "UV_KEYRING_PROVIDER")] - pub(crate) keyring_provider: KeyringProvider, + pub(crate) keyring_provider: KeyringProviderType, /// The Python interpreter into which packages should be installed. /// @@ -848,7 +848,7 @@ pub(crate) struct PipInstallArgs { /// Due to not having Python imports, only `--keyring-provider subprocess` argument is currently /// implemented `uv` will try to use `keyring` via CLI when this flag is used. #[clap(long, default_value_t, value_enum, env = "UV_KEYRING_PROVIDER")] - pub(crate) keyring_provider: KeyringProvider, + pub(crate) keyring_provider: KeyringProviderType, /// The Python interpreter into which packages should be installed. /// @@ -997,7 +997,7 @@ pub(crate) struct PipUninstallArgs { /// Due to not having Python imports, only `--keyring-provider subprocess` argument is currently /// implemented `uv` will try to use `keyring` via CLI when this flag is used. #[clap(long, default_value_t, value_enum, env = "UV_KEYRING_PROVIDER")] - pub(crate) keyring_provider: KeyringProvider, + pub(crate) keyring_provider: KeyringProviderType, /// Use the system Python to uninstall packages. /// @@ -1281,7 +1281,7 @@ pub(crate) struct VenvArgs { /// Due to not having Python imports, only `--keyring-provider subprocess` argument is currently /// implemented `uv` will try to use `keyring` via CLI when this flag is used. #[clap(long, default_value_t, value_enum, env = "UV_KEYRING_PROVIDER")] - pub(crate) keyring_provider: KeyringProvider, + pub(crate) keyring_provider: KeyringProviderType, /// Run offline, i.e., without accessing the network. #[arg(global = true, long)] diff --git a/crates/uv/src/commands/pip_compile.rs b/crates/uv/src/commands/pip_compile.rs index 72020f97229d..9ddbb459f42b 100644 --- a/crates/uv/src/commands/pip_compile.rs +++ b/crates/uv/src/commands/pip_compile.rs @@ -17,9 +17,9 @@ use tracing::debug; use distribution_types::{IndexLocations, LocalEditable, LocalEditables, Verbatim}; use platform_tags::Tags; use requirements_txt::EditableRequirement; -use uv_auth::{KeyringProvider, GLOBAL_AUTH_STORE}; use uv_cache::Cache; use uv_client::{BaseClientBuilder, Connectivity, FlatIndexClient, RegistryClientBuilder}; +use uv_configuration::KeyringProviderType; use uv_configuration::{ ConfigSettings, Constraints, IndexStrategy, NoBinary, NoBuild, Overrides, SetupPyStrategy, Upgrade, @@ -69,7 +69,7 @@ pub(crate) async fn pip_compile( include_index_annotation: bool, index_locations: IndexLocations, index_strategy: IndexStrategy, - keyring_provider: KeyringProvider, + keyring_provider: KeyringProviderType, setup_py: SetupPyStrategy, config_settings: ConfigSettings, connectivity: Connectivity, @@ -96,7 +96,7 @@ pub(crate) async fn pip_compile( let client_builder = BaseClientBuilder::new() .connectivity(connectivity) .native_tls(native_tls) - .keyring_provider(keyring_provider); + .keyring(keyring_provider); // Read all requirements from the provided sources. let RequirementsSpecification { @@ -210,18 +210,13 @@ pub(crate) async fn pip_compile( let index_locations = index_locations.combine(index_url, extra_index_urls, find_links, no_index); - // Add all authenticated sources to the store. - for url in index_locations.urls() { - GLOBAL_AUTH_STORE.save_from_url(url); - } - // Initialize the registry client. let client = RegistryClientBuilder::new(cache.clone()) .native_tls(native_tls) .connectivity(connectivity) .index_urls(index_locations.index_urls()) .index_strategy(index_strategy) - .keyring_provider(keyring_provider) + .keyring(keyring_provider) .markers(&markers) .platform(interpreter.platform()) .build(); diff --git a/crates/uv/src/commands/pip_install.rs b/crates/uv/src/commands/pip_install.rs index 63ff8af1ec70..0e8790e614b3 100644 --- a/crates/uv/src/commands/pip_install.rs +++ b/crates/uv/src/commands/pip_install.rs @@ -19,11 +19,11 @@ use pep508_rs::{MarkerEnvironment, Requirement}; use platform_tags::Tags; use pypi_types::{Metadata23, Yanked}; use requirements_txt::EditableRequirement; -use uv_auth::{KeyringProvider, GLOBAL_AUTH_STORE}; use uv_cache::Cache; use uv_client::{ BaseClientBuilder, Connectivity, FlatIndexClient, RegistryClient, RegistryClientBuilder, }; +use uv_configuration::KeyringProviderType; use uv_configuration::{ ConfigSettings, Constraints, IndexStrategy, NoBinary, NoBuild, Overrides, Reinstall, SetupPyStrategy, Upgrade, @@ -63,7 +63,7 @@ pub(crate) async fn pip_install( upgrade: Upgrade, index_locations: IndexLocations, index_strategy: IndexStrategy, - keyring_provider: KeyringProvider, + keyring_provider: KeyringProviderType, reinstall: Reinstall, link_mode: LinkMode, compile: bool, @@ -89,7 +89,7 @@ pub(crate) async fn pip_install( let client_builder = BaseClientBuilder::new() .connectivity(connectivity) .native_tls(native_tls) - .keyring_provider(keyring_provider); + .keyring(keyring_provider); // Read all requirements from the provided sources. let RequirementsSpecification { @@ -203,18 +203,13 @@ pub(crate) async fn pip_install( let index_locations = index_locations.combine(index_url, extra_index_urls, find_links, no_index); - // Add all authenticated sources to the store. - for url in index_locations.urls() { - GLOBAL_AUTH_STORE.save_from_url(url); - } - // Initialize the registry client. let client = RegistryClientBuilder::new(cache.clone()) .native_tls(native_tls) .connectivity(connectivity) .index_urls(index_locations.index_urls()) .index_strategy(index_strategy) - .keyring_provider(keyring_provider) + .keyring(keyring_provider) .markers(markers) .platform(interpreter.platform()) .build(); diff --git a/crates/uv/src/commands/pip_sync.rs b/crates/uv/src/commands/pip_sync.rs index 218ad29c3090..caf3420acfa8 100644 --- a/crates/uv/src/commands/pip_sync.rs +++ b/crates/uv/src/commands/pip_sync.rs @@ -14,11 +14,11 @@ use install_wheel_rs::linker::LinkMode; use platform_tags::Tags; use pypi_types::Yanked; use requirements_txt::EditableRequirement; -use uv_auth::{KeyringProvider, GLOBAL_AUTH_STORE}; use uv_cache::{ArchiveTarget, ArchiveTimestamp, Cache}; use uv_client::{ BaseClientBuilder, Connectivity, FlatIndexClient, RegistryClient, RegistryClientBuilder, }; +use uv_configuration::KeyringProviderType; use uv_configuration::{ ConfigSettings, IndexStrategy, NoBinary, NoBuild, Reinstall, SetupPyStrategy, }; @@ -48,7 +48,7 @@ pub(crate) async fn pip_sync( require_hashes: bool, index_locations: IndexLocations, index_strategy: IndexStrategy, - keyring_provider: KeyringProvider, + keyring_provider: KeyringProviderType, setup_py: SetupPyStrategy, connectivity: Connectivity, config_settings: &ConfigSettings, @@ -68,7 +68,7 @@ pub(crate) async fn pip_sync( let client_builder = BaseClientBuilder::new() .connectivity(connectivity) .native_tls(native_tls) - .keyring_provider(keyring_provider); + .keyring(keyring_provider); // Read all requirements from the provided sources. let RequirementsSpecification { @@ -150,18 +150,13 @@ pub(crate) async fn pip_sync( let index_locations = index_locations.combine(index_url, extra_index_urls, find_links, no_index); - // Add all authenticated sources to the store. - for url in index_locations.urls() { - GLOBAL_AUTH_STORE.save_from_url(url); - } - // Initialize the registry client. let client = RegistryClientBuilder::new(cache.clone()) .native_tls(native_tls) .connectivity(connectivity) .index_urls(index_locations.index_urls()) .index_strategy(index_strategy) - .keyring_provider(keyring_provider) + .keyring(keyring_provider) .markers(venv.interpreter().markers()) .platform(venv.interpreter().platform()) .build(); diff --git a/crates/uv/src/commands/pip_uninstall.rs b/crates/uv/src/commands/pip_uninstall.rs index 45e19da59549..b2ca710f3a95 100644 --- a/crates/uv/src/commands/pip_uninstall.rs +++ b/crates/uv/src/commands/pip_uninstall.rs @@ -7,9 +7,9 @@ use tracing::debug; use distribution_types::{InstalledMetadata, Name}; use pep508_rs::{Requirement, RequirementsTxtRequirement, UnnamedRequirement}; -use uv_auth::KeyringProvider; use uv_cache::Cache; use uv_client::{BaseClientBuilder, Connectivity}; +use uv_configuration::KeyringProviderType; use uv_fs::Simplified; use uv_interpreter::PythonEnvironment; @@ -27,14 +27,14 @@ pub(crate) async fn pip_uninstall( cache: Cache, connectivity: Connectivity, native_tls: bool, - keyring_provider: KeyringProvider, + keyring_provider: KeyringProviderType, printer: Printer, ) -> Result { let start = std::time::Instant::now(); let client_builder = BaseClientBuilder::new() .connectivity(connectivity) .native_tls(native_tls) - .keyring_provider(keyring_provider); + .keyring(keyring_provider); // Read all requirements from the provided sources. let spec = RequirementsSpecification::from_simple_sources(sources, &client_builder).await?; diff --git a/crates/uv/src/commands/venv.rs b/crates/uv/src/commands/venv.rs index 6f44fecbed03..eddc0985a1c4 100644 --- a/crates/uv/src/commands/venv.rs +++ b/crates/uv/src/commands/venv.rs @@ -13,9 +13,9 @@ use thiserror::Error; use distribution_types::{DistributionMetadata, IndexLocations, Name, ResolvedDist}; use pep508_rs::Requirement; -use uv_auth::{KeyringProvider, GLOBAL_AUTH_STORE}; use uv_cache::Cache; use uv_client::{Connectivity, FlatIndexClient, RegistryClientBuilder}; +use uv_configuration::KeyringProviderType; use uv_configuration::{ConfigSettings, IndexStrategy, NoBinary, NoBuild, SetupPyStrategy}; use uv_dispatch::BuildDispatch; use uv_fs::Simplified; @@ -34,7 +34,7 @@ pub(crate) async fn venv( python_request: Option<&str>, index_locations: &IndexLocations, index_strategy: IndexStrategy, - keyring_provider: KeyringProvider, + keyring_provider: KeyringProviderType, prompt: uv_virtualenv::Prompt, system_site_packages: bool, connectivity: Connectivity, @@ -95,7 +95,7 @@ async fn venv_impl( python_request: Option<&str>, index_locations: &IndexLocations, index_strategy: IndexStrategy, - keyring_provider: KeyringProvider, + keyring_provider: KeyringProviderType, prompt: uv_virtualenv::Prompt, system_site_packages: bool, connectivity: Connectivity, @@ -143,17 +143,12 @@ async fn venv_impl( // Extract the interpreter. let interpreter = venv.interpreter(); - // Add all authenticated sources to the store. - for url in index_locations.urls() { - GLOBAL_AUTH_STORE.save_from_url(url); - } - // Instantiate a client. let client = RegistryClientBuilder::new(cache.clone()) .native_tls(native_tls) .index_urls(index_locations.index_urls()) .index_strategy(index_strategy) - .keyring_provider(keyring_provider) + .keyring(keyring_provider) .connectivity(connectivity) .markers(interpreter.markers()) .platform(interpreter.platform()) diff --git a/crates/uv/src/main.rs b/crates/uv/src/main.rs index f34fa774cdaa..b628455f7964 100644 --- a/crates/uv/src/main.rs +++ b/crates/uv/src/main.rs @@ -13,8 +13,7 @@ use tracing::instrument; use distribution_types::IndexLocations; use uv_cache::{Cache, Refresh}; use uv_client::Connectivity; -use uv_configuration::NoBinary; -use uv_configuration::{ConfigSettings, NoBuild, Reinstall, SetupPyStrategy, Upgrade}; +use uv_configuration::{ConfigSettings, NoBinary, NoBuild, Reinstall, SetupPyStrategy, Upgrade}; use uv_requirements::{ExtrasSpecification, RequirementsSource}; use uv_resolver::{DependencyMode, PreReleaseMode};