From 3c806fb1b1b63f8ec4c6bc452c253272c8dc9501 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Thu, 3 Apr 2025 15:55:43 +0200 Subject: [PATCH 01/13] Fetch keyring credentials from index URL in auth middleware Some registries (like Azure Artifact) can require you to authenticate separately for every package URL if you do not authenticate for the /simple endpoint. These changes make the auth middleware aware of index URL endpoints and attempts to fetch keyring credentials for such an index URL when making a request to any URL it's a prefix of. Currently, if we find a prefix index URL we attempt to fetch keyring credentials for it. Otherwise we attempt to fetch for the full URL. Addresses part of #4056 Closes #4583 --- .../uv-auth/src/{policy.rs => auth_index.rs} | 47 ++++++++----- crates/uv-auth/src/lib.rs | 4 +- crates/uv-auth/src/middleware.rs | 68 ++++++++++++------- crates/uv-client/src/base_client.rs | 35 ++++++---- crates/uv-client/src/registry_client.rs | 21 +++--- crates/uv-distribution-types/src/index_url.rs | 26 ++++--- crates/uv/src/commands/build_frontend.rs | 4 +- crates/uv/src/commands/pip/compile.rs | 4 +- crates/uv/src/commands/pip/install.rs | 4 +- crates/uv/src/commands/pip/list.rs | 4 +- crates/uv/src/commands/pip/sync.rs | 4 +- crates/uv/src/commands/pip/tree.rs | 4 +- crates/uv/src/commands/project/add.rs | 4 +- crates/uv/src/commands/project/lock.rs | 4 +- crates/uv/src/commands/project/mod.rs | 13 ++-- crates/uv/src/commands/project/sync.rs | 4 +- crates/uv/src/commands/project/tree.rs | 3 +- crates/uv/src/commands/publish.rs | 7 +- crates/uv/src/commands/venv.rs | 4 +- crates/uv/tests/it/lock.rs | 12 ++-- crates/uv/tests/it/pip_install.rs | 18 ++--- 21 files changed, 150 insertions(+), 144 deletions(-) rename crates/uv-auth/src/{policy.rs => auth_index.rs} (59%) diff --git a/crates/uv-auth/src/policy.rs b/crates/uv-auth/src/auth_index.rs similarity index 59% rename from crates/uv-auth/src/policy.rs rename to crates/uv-auth/src/auth_index.rs index 2b6410585a96b..7f2e23542a565 100644 --- a/crates/uv-auth/src/policy.rs +++ b/crates/uv-auth/src/auth_index.rs @@ -1,6 +1,6 @@ use std::fmt::{self, Display, Formatter}; -use rustc_hash::FxHashMap; +use rustc_hash::FxHashSet; use url::Url; /// When to use authentication. @@ -47,27 +47,40 @@ impl Display for AuthPolicy { } } } + +#[derive(Debug, Clone, Hash, Eq, PartialEq)] +pub struct AuthIndex { + pub index_url: Url, + pub policy_url: Url, + pub auth_policy: AuthPolicy, +} + #[derive(Debug, Default, Clone, Eq, PartialEq)] -pub struct UrlAuthPolicies(FxHashMap); +pub struct AuthIndexes(FxHashSet); -impl UrlAuthPolicies { +impl AuthIndexes { pub fn new() -> Self { - Self(FxHashMap::default()) + Self(FxHashSet::default()) } - /// Create a new [`UrlAuthPolicies`] from a list of URL and [`AuthPolicy`] - /// tuples. - pub fn from_tuples(tuples: impl IntoIterator) -> Self { - let mut auth_policies = Self::new(); - for (url, auth_policy) in tuples { - auth_policies.add_policy(url, auth_policy); + /// Create a new [`AuthIndexUrls`] from an iterator of [`AuthIndexUrl`]s. + pub fn from_auth_indexes(urls: impl IntoIterator) -> Self { + let mut auth_index_urls = Self::new(); + for url in urls { + auth_index_urls.0.insert(url); } - auth_policies + auth_index_urls } - /// An [`AuthPolicy`] for a URL. - pub fn add_policy(&mut self, url: Url, auth_policy: AuthPolicy) { - self.0.insert(url, auth_policy); + /// Get the index URL prefix for a URL if one exists. + pub fn auth_index_url_for(&self, url: &Url) -> Option<&Url> { + // TODO(john): There are probably not many URLs to iterate through, + // but we could use a trie instead of a HashSet here for more + // efficient search. + self.0 + .iter() + .find(|auth_index| url.as_str().starts_with(auth_index.index_url.as_str())) + .map(|auth_index| &auth_index.index_url) } /// Get the [`AuthPolicy`] for a URL. @@ -75,9 +88,9 @@ impl UrlAuthPolicies { // TODO(john): There are probably not many URLs to iterate through, // but we could use a trie instead of a HashMap here for more // efficient search. - for (auth_url, auth_policy) in &self.0 { - if url.as_str().starts_with(auth_url.as_str()) { - return *auth_policy; + for auth_index in &self.0 { + if url.as_str().starts_with(auth_index.policy_url.as_str()) { + return auth_index.auth_policy; } } AuthPolicy::Auto diff --git a/crates/uv-auth/src/lib.rs b/crates/uv-auth/src/lib.rs index 1c090e73f6db2..52b5418b34eec 100644 --- a/crates/uv-auth/src/lib.rs +++ b/crates/uv-auth/src/lib.rs @@ -7,15 +7,15 @@ use cache::CredentialsCache; pub use credentials::Credentials; pub use keyring::KeyringProvider; pub use middleware::AuthMiddleware; -pub use policy::{AuthPolicy, UrlAuthPolicies}; use realm::Realm; +pub use auth_index::{AuthIndex, AuthIndexes, AuthPolicy}; mod cache; mod credentials; mod keyring; mod middleware; -mod policy; mod realm; +mod auth_index; // TODO(zanieb): Consider passing a cache explicitly throughout diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index 31f116a635636..3297d4648595b 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -5,8 +5,8 @@ use url::Url; use crate::{ credentials::{Credentials, Username}, - policy::{AuthPolicy, UrlAuthPolicies}, realm::Realm, + auth_index::{AuthIndexes, AuthPolicy}, CredentialsCache, KeyringProvider, CREDENTIALS_CACHE, }; use anyhow::{anyhow, format_err}; @@ -58,7 +58,7 @@ pub struct AuthMiddleware { keyring: Option, cache: Option, /// Auth policies for specific URLs. - url_auth_policies: UrlAuthPolicies, + auth_indexes: AuthIndexes, /// Set all endpoints as needing authentication. We never try to send an /// unauthenticated request, avoiding cloning an uncloneable request. only_authenticated: bool, @@ -70,7 +70,7 @@ impl AuthMiddleware { netrc: NetrcMode::default(), keyring: None, cache: None, - url_auth_policies: UrlAuthPolicies::new(), + auth_indexes: AuthIndexes::new(), only_authenticated: false, } } @@ -104,8 +104,8 @@ impl AuthMiddleware { /// Configure the [`AuthPolicy`]s to use for URLs. #[must_use] - pub fn with_url_auth_policies(mut self, auth_policies: UrlAuthPolicies) -> Self { - self.url_auth_policies = auth_policies; + pub fn with_auth_indexes(mut self, auth_indexes: AuthIndexes) -> Self { + self.auth_indexes = auth_indexes; self } @@ -181,7 +181,7 @@ impl Middleware for AuthMiddleware { // In the middleware, existing credentials are already moved from the URL // to the headers so for display purposes we restore some information let url = tracing_url(&request, request_credentials.as_ref()); - let auth_policy = self.url_auth_policies.policy_for(request.url()); + let auth_policy = self.auth_indexes.policy_for(request.url()); trace!("Handling request for {url} with authentication policy {auth_policy}"); let credentials: Option> = if matches!(auth_policy, AuthPolicy::Never) { @@ -477,13 +477,25 @@ impl AuthMiddleware { // URLs; instead, we fetch if there's a username or if the user has requested to // always authenticate. if let Some(username) = credentials.and_then(|credentials| credentials.username()) { - debug!("Checking keyring for credentials for {username}@{url}"); - keyring.fetch(url, Some(username)).await + if let Some(index_url) = self.auth_indexes.auth_index_url_for(url) { + debug!("Checking keyring for credentials for index URL {username}@{index_url}"); + keyring.fetch(index_url, Some(username)).await + } else { + debug!("Checking keyring for credentials for full URL {username}@{url}"); + keyring.fetch(url, Some(username)).await + } } else if matches!(auth_policy, AuthPolicy::Always) { - debug!( - "Checking keyring for credentials for {url} without username due to `authenticate = always`" - ); - keyring.fetch(url, None).await + if let Some(index_url) = self.auth_indexes.auth_index_url_for(url) { + debug!( + "Checking keyring for credentials for index URL {index_url} without username due to `authenticate = always`" + ); + keyring.fetch(index_url, None).await + } else { + debug!( + "Checking keyring for credentials for full URL {url} without username due to `authenticate = always`" + ); + keyring.fetch(url, None).await + } } else { debug!("Skipping keyring fetch for {url} without username; use `authenticate = always` to force"); None @@ -539,6 +551,8 @@ mod tests { use wiremock::matchers::{basic_auth, method, path_regex}; use wiremock::{Mock, MockServer, ResponseTemplate}; + use crate::AuthIndex; + use super::*; type Error = Box; @@ -998,7 +1012,7 @@ mod tests { let server = start_test_server(username, password).await; let base_url = Url::parse(&server.uri())?; - let auth_policies = auth_policies_for(&base_url, AuthPolicy::Always); + let auth_indexes = auth_indexes_for(&base_url, AuthPolicy::Always); let client = test_client_builder() .with( AuthMiddleware::new() @@ -1012,7 +1026,7 @@ mod tests { username, password, )]))) - .with_url_auth_policies(auth_policies), + .with_auth_indexes(auth_indexes), ) .build(); @@ -1655,13 +1669,15 @@ mod tests { Ok(()) } - fn auth_policies_for(url: &Url, policy: AuthPolicy) -> UrlAuthPolicies { + fn auth_indexes_for(url: &Url, policy: AuthPolicy) -> AuthIndexes { let mut url = url.clone(); - let mut policies = UrlAuthPolicies::new(); url.set_password(None).ok(); url.set_username("").ok(); - policies.add_policy(url, policy); - policies + AuthIndexes::from_auth_indexes(vec![AuthIndex { + index_url: url.clone(), + policy_url: url, + auth_policy: policy, + }]) } /// With the "always" auth policy, requests should succeed on @@ -1675,12 +1691,12 @@ mod tests { let base_url = Url::parse(&server.uri())?; - let auth_policies = auth_policies_for(&base_url, AuthPolicy::Always); + let auth_indexes = auth_indexes_for(&base_url, AuthPolicy::Always); let client = test_client_builder() .with( AuthMiddleware::new() .with_cache(CredentialsCache::new()) - .with_url_auth_policies(auth_policies), + .with_auth_indexes(auth_indexes), ) .build(); @@ -1742,12 +1758,12 @@ mod tests { let base_url = Url::parse(&server.uri())?; - let auth_policies = auth_policies_for(&base_url, AuthPolicy::Always); + let auth_indexes = auth_indexes_for(&base_url, AuthPolicy::Always); let client = test_client_builder() .with( AuthMiddleware::new() .with_cache(CredentialsCache::new()) - .with_url_auth_policies(auth_policies), + .with_auth_indexes(auth_indexes), ) .build(); @@ -1782,12 +1798,12 @@ mod tests { .mount(&server) .await; - let auth_policies = auth_policies_for(&base_url, AuthPolicy::Never); + let auth_indexes = auth_indexes_for(&base_url, AuthPolicy::Never); let client = test_client_builder() .with( AuthMiddleware::new() .with_cache(CredentialsCache::new()) - .with_url_auth_policies(auth_policies), + .with_auth_indexes(auth_indexes), ) .build(); @@ -1827,12 +1843,12 @@ mod tests { let base_url = Url::parse(&server.uri())?; - let auth_policies = auth_policies_for(&base_url, AuthPolicy::Never); + let auth_indexes = auth_indexes_for(&base_url, AuthPolicy::Never); let client = test_client_builder() .with( AuthMiddleware::new() .with_cache(CredentialsCache::new()) - .with_url_auth_policies(auth_policies), + .with_auth_indexes(auth_indexes), ) .build(); diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs index 071779fbe2217..6329d408410c9 100644 --- a/crates/uv-client/src/base_client.rs +++ b/crates/uv-client/src/base_client.rs @@ -16,7 +16,7 @@ use reqwest_retry::{ use tracing::{debug, trace}; use url::Url; -use uv_auth::{AuthMiddleware, UrlAuthPolicies}; +use uv_auth::{AuthIndexes, AuthMiddleware, UrlAuthPolicies}; use uv_configuration::{KeyringProviderType, TrustedHost}; use uv_fs::Simplified; use uv_pep508::MarkerEnvironment; @@ -56,7 +56,7 @@ pub struct BaseClientBuilder<'a> { markers: Option<&'a MarkerEnvironment>, platform: Option<&'a Platform>, auth_integration: AuthIntegration, - url_auth_policies: Option, + auth_indexes: Option, default_timeout: Duration, extra_middleware: Option, proxies: Vec, @@ -91,7 +91,7 @@ impl BaseClientBuilder<'_> { markers: None, platform: None, auth_integration: AuthIntegration::default(), - url_auth_policies: None, + auth_indexes: None, default_timeout: Duration::from_secs(30), extra_middleware: None, proxies: vec![], @@ -149,8 +149,8 @@ impl<'a> BaseClientBuilder<'a> { } #[must_use] - pub fn url_auth_policies(mut self, auth_policies: UrlAuthPolicies) -> Self { - self.url_auth_policies = Some(auth_policies); + pub fn auth_indexes(mut self, auth_indexes: AuthIndexes) -> Self { + self.auth_indexes = Some(auth_indexes); self } @@ -342,20 +342,25 @@ impl<'a> BaseClientBuilder<'a> { // Initialize the authentication middleware to set headers. match self.auth_integration { AuthIntegration::Default => { - let mut auth_middleware = + let auth_middleware = AuthMiddleware::new().with_keyring(self.keyring.to_provider()); - if let Some(url_auth_policies) = &self.url_auth_policies { - auth_middleware = - auth_middleware.with_url_auth_policies(url_auth_policies.clone()); + if let Some(auth_indexes) = &self.auth_indexes { + client = client + .with(auth_middleware.with_auth_indexes(auth_indexes.clone())); + } else { + client = client.with(auth_middleware); } - client = client.with(auth_middleware); } AuthIntegration::OnlyAuthenticated => { - client = client.with( - AuthMiddleware::new() - .with_keyring(self.keyring.to_provider()) - .with_only_authenticated(true), - ); + let auth_middleware = AuthMiddleware::new() + .with_keyring(self.keyring.to_provider()) + .with_only_authenticated(true); + if let Some(auth_indexes) = &self.auth_indexes { + client = client + .with(auth_middleware.with_auth_indexes(auth_indexes.clone())); + } else { + client = client.with(auth_middleware); + } } AuthIntegration::NoAuthMiddleware => { // The downstream code uses custom auth logic. diff --git a/crates/uv-client/src/registry_client.rs b/crates/uv-client/src/registry_client.rs index 9450352a1eaaf..e6364fa3f9d26 100644 --- a/crates/uv-client/src/registry_client.rs +++ b/crates/uv-client/src/registry_client.rs @@ -16,14 +16,14 @@ use tokio::sync::{Mutex, Semaphore}; use tracing::{info_span, instrument, trace, warn, Instrument}; use url::Url; -use uv_auth::UrlAuthPolicies; +use uv_auth::AuthIndexes; use uv_cache::{Cache, CacheBucket, CacheEntry, WheelCache}; use uv_configuration::KeyringProviderType; use uv_configuration::{IndexStrategy, TrustedHost}; use uv_distribution_filename::{DistFilename, SourceDistFilename, WheelFilename}; use uv_distribution_types::{ - BuiltDist, File, FileLocation, IndexCapabilities, IndexFormat, IndexMetadataRef, IndexUrl, - IndexUrls, Name, + BuiltDist, File, FileLocation, IndexCapabilities, IndexFormat, IndexLocations, + IndexMetadataRef, IndexUrl, IndexUrls, Name, }; use uv_metadata::{read_metadata_async_seek, read_metadata_async_stream}; use uv_normalize::PackageName; @@ -69,8 +69,11 @@ impl RegistryClientBuilder<'_> { impl<'a> RegistryClientBuilder<'a> { #[must_use] - pub fn index_urls(mut self, index_urls: IndexUrls) -> Self { - self.index_urls = index_urls; + pub fn index_locations(mut self, index_locations: &IndexLocations) -> Self { + self.index_urls = index_locations.index_urls(); + self.base_client_builder = self + .base_client_builder + .auth_indexes(AuthIndexes::from(index_locations)); self } @@ -118,14 +121,6 @@ impl<'a> RegistryClientBuilder<'a> { self } - #[must_use] - pub fn url_auth_policies(mut self, url_auth_policies: UrlAuthPolicies) -> Self { - self.base_client_builder = self - .base_client_builder - .url_auth_policies(url_auth_policies); - self - } - #[must_use] pub fn cache(mut self, cache: Cache) -> Self { self.cache = cache; diff --git a/crates/uv-distribution-types/src/index_url.rs b/crates/uv-distribution-types/src/index_url.rs index ef3c324fbbd92..3472b7893f9e2 100644 --- a/crates/uv-distribution-types/src/index_url.rs +++ b/crates/uv-distribution-types/src/index_url.rs @@ -10,7 +10,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; use thiserror::Error; use url::{ParseError, Url}; -use uv_auth::UrlAuthPolicies; +use uv_auth::{AuthIndex, AuthIndexes}; use uv_pep508::{split_scheme, Scheme, VerbatimUrl, VerbatimUrlError}; use crate::{Index, Verbatim}; @@ -411,16 +411,20 @@ impl<'a> IndexLocations { } } -impl From<&IndexLocations> for UrlAuthPolicies { - fn from(index_locations: &IndexLocations) -> UrlAuthPolicies { - UrlAuthPolicies::from_tuples(index_locations.allowed_indexes().into_iter().map(|index| { - let mut url = index - .url() - .root() - .unwrap_or_else(|| index.url().url().clone()); - url.set_username("").ok(); - url.set_password(None).ok(); - (url, index.authenticate) +impl From<&IndexLocations> for AuthIndexes { + fn from(index_locations: &IndexLocations) -> AuthIndexes { + AuthIndexes::from_auth_indexes(index_locations.allowed_indexes().into_iter().map(|index| { + let mut index_url = index.url().url().clone(); + index_url.set_username("").ok(); + index_url.set_password(None).ok(); + let mut policy_url = index.url().root().unwrap_or_else(|| index_url.clone()); + policy_url.set_username("").ok(); + policy_url.set_password(None).ok(); + AuthIndex { + index_url, + policy_url, + auth_policy: index.authenticate, + } })) } } diff --git a/crates/uv/src/commands/build_frontend.rs b/crates/uv/src/commands/build_frontend.rs index 89b3dd2e11f23..d0ea55c3a5940 100644 --- a/crates/uv/src/commands/build_frontend.rs +++ b/crates/uv/src/commands/build_frontend.rs @@ -10,7 +10,6 @@ use anyhow::{Context, Result}; use owo_colors::OwoColorize; use thiserror::Error; use tracing::instrument; -use uv_auth::UrlAuthPolicies; use crate::commands::pip::operations; use crate::commands::project::{find_requires_python, ProjectError}; @@ -528,8 +527,7 @@ async fn build_package( .native_tls(network_settings.native_tls) .connectivity(network_settings.connectivity) .allow_insecure_host(network_settings.allow_insecure_host.clone()) - .url_auth_policies(UrlAuthPolicies::from(index_locations)) - .index_urls(index_locations.index_urls()) + .index_locations(index_locations) .index_strategy(index_strategy) .keyring(keyring_provider) .markers(interpreter.markers()) diff --git a/crates/uv/src/commands/pip/compile.rs b/crates/uv/src/commands/pip/compile.rs index 7858087b86904..3d5cb94d37a8d 100644 --- a/crates/uv/src/commands/pip/compile.rs +++ b/crates/uv/src/commands/pip/compile.rs @@ -10,7 +10,6 @@ use owo_colors::OwoColorize; use rustc_hash::FxHashSet; use tracing::debug; -use uv_auth::UrlAuthPolicies; use uv_cache::Cache; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; use uv_configuration::{ @@ -380,9 +379,8 @@ pub(crate) async fn pip_compile( // Initialize the registry client. let client = RegistryClientBuilder::try_from(client_builder)? .cache(cache.clone()) - .index_urls(index_locations.index_urls()) + .index_locations(&index_locations) .index_strategy(index_strategy) - .url_auth_policies(UrlAuthPolicies::from(&index_locations)) .torch_backend(torch_backend) .markers(interpreter.markers()) .platform(interpreter.platform()) diff --git a/crates/uv/src/commands/pip/install.rs b/crates/uv/src/commands/pip/install.rs index d1790c6977973..60b1bcafac4a9 100644 --- a/crates/uv/src/commands/pip/install.rs +++ b/crates/uv/src/commands/pip/install.rs @@ -7,7 +7,6 @@ use itertools::Itertools; use owo_colors::OwoColorize; use tracing::{debug, enabled, Level}; -use uv_auth::UrlAuthPolicies; use uv_cache::Cache; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; use uv_configuration::{ @@ -355,9 +354,8 @@ pub(crate) async fn pip_install( // Initialize the registry client. let client = RegistryClientBuilder::try_from(client_builder)? .cache(cache.clone()) - .index_urls(index_locations.index_urls()) + .index_locations(&index_locations) .index_strategy(index_strategy) - .url_auth_policies(UrlAuthPolicies::from(&index_locations)) .torch_backend(torch_backend) .markers(interpreter.markers()) .platform(interpreter.platform()) diff --git a/crates/uv/src/commands/pip/list.rs b/crates/uv/src/commands/pip/list.rs index 3d458e1d78ec6..a26954c6f5905 100644 --- a/crates/uv/src/commands/pip/list.rs +++ b/crates/uv/src/commands/pip/list.rs @@ -11,7 +11,6 @@ use serde::Serialize; use tokio::sync::Semaphore; use unicode_width::UnicodeWidthStr; -use uv_auth::UrlAuthPolicies; use uv_cache::{Cache, Refresh}; use uv_cache_info::Timestamp; use uv_cli::ListFormat; @@ -89,9 +88,8 @@ pub(crate) async fn pip_list( .native_tls(network_settings.native_tls) .connectivity(network_settings.connectivity) .allow_insecure_host(network_settings.allow_insecure_host.clone()) - .index_urls(index_locations.index_urls()) + .index_locations(&index_locations) .index_strategy(index_strategy) - .url_auth_policies(UrlAuthPolicies::from(&index_locations)) .keyring(keyring_provider) .markers(environment.interpreter().markers()) .platform(environment.interpreter().platform()) diff --git a/crates/uv/src/commands/pip/sync.rs b/crates/uv/src/commands/pip/sync.rs index d6cead51cb278..e9850acb970bc 100644 --- a/crates/uv/src/commands/pip/sync.rs +++ b/crates/uv/src/commands/pip/sync.rs @@ -6,7 +6,6 @@ use anyhow::Result; use owo_colors::OwoColorize; use tracing::debug; -use uv_auth::UrlAuthPolicies; use uv_cache::Cache; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; use uv_configuration::{ @@ -283,9 +282,8 @@ pub(crate) async fn pip_sync( // Initialize the registry client. let client = RegistryClientBuilder::try_from(client_builder)? .cache(cache.clone()) - .index_urls(index_locations.index_urls()) + .index_locations(&index_locations) .index_strategy(index_strategy) - .url_auth_policies(UrlAuthPolicies::from(&index_locations)) .torch_backend(torch_backend) .markers(interpreter.markers()) .platform(interpreter.platform()) diff --git a/crates/uv/src/commands/pip/tree.rs b/crates/uv/src/commands/pip/tree.rs index 1776ea4b95d0c..fc1252e399f6a 100644 --- a/crates/uv/src/commands/pip/tree.rs +++ b/crates/uv/src/commands/pip/tree.rs @@ -10,7 +10,6 @@ use petgraph::Direction; use rustc_hash::{FxHashMap, FxHashSet}; use tokio::sync::Semaphore; -use uv_auth::UrlAuthPolicies; use uv_cache::{Cache, Refresh}; use uv_cache_info::Timestamp; use uv_client::RegistryClientBuilder; @@ -90,9 +89,8 @@ pub(crate) async fn pip_tree( .native_tls(network_settings.native_tls) .connectivity(network_settings.connectivity) .allow_insecure_host(network_settings.allow_insecure_host.clone()) - .index_urls(index_locations.index_urls()) + .index_locations(&index_locations) .index_strategy(index_strategy) - .url_auth_policies(UrlAuthPolicies::from(&index_locations)) .keyring(keyring_provider) .markers(environment.interpreter().markers()) .platform(environment.interpreter().platform()) diff --git a/crates/uv/src/commands/project/add.rs b/crates/uv/src/commands/project/add.rs index 8b96364e53975..162068cc8b129 100644 --- a/crates/uv/src/commands/project/add.rs +++ b/crates/uv/src/commands/project/add.rs @@ -13,7 +13,6 @@ use rustc_hash::{FxBuildHasher, FxHashMap}; use tracing::debug; use url::Url; -use uv_auth::UrlAuthPolicies; use uv_cache::Cache; use uv_cache_key::RepositoryUrl; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; @@ -323,9 +322,8 @@ pub(crate) async fn add( // Initialize the registry client. let client = RegistryClientBuilder::try_from(client_builder)? - .index_urls(settings.resolver.index_locations.index_urls()) + .index_locations(&settings.resolver.index_locations) .index_strategy(settings.resolver.index_strategy) - .url_auth_policies(UrlAuthPolicies::from(&settings.resolver.index_locations)) .markers(target.interpreter().markers()) .platform(target.interpreter().platform()) .build(); diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 3675ab204d1f2..47f419d7b6c32 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -9,7 +9,6 @@ use owo_colors::OwoColorize; use rustc_hash::{FxBuildHasher, FxHashMap}; use tracing::debug; -use uv_auth::UrlAuthPolicies; use uv_cache::Cache; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; use uv_configuration::{ @@ -593,8 +592,7 @@ async fn do_lock( .native_tls(network_settings.native_tls) .connectivity(network_settings.connectivity) .allow_insecure_host(network_settings.allow_insecure_host.clone()) - .url_auth_policies(UrlAuthPolicies::from(index_locations)) - .index_urls(index_locations.index_urls()) + .index_locations(index_locations) .index_strategy(*index_strategy) .keyring(*keyring_provider) .markers(interpreter.markers()) diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index ed562f36fb157..d697fc8417878 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -8,7 +8,6 @@ use itertools::Itertools; use owo_colors::OwoColorize; use tracing::{debug, warn}; -use uv_auth::UrlAuthPolicies; use uv_cache::{Cache, CacheBucket}; use uv_cache_key::cache_digest; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; @@ -1546,8 +1545,7 @@ pub(crate) async fn resolve_names( .native_tls(network_settings.native_tls) .connectivity(network_settings.connectivity) .allow_insecure_host(network_settings.allow_insecure_host.clone()) - .url_auth_policies(UrlAuthPolicies::from(index_locations)) - .index_urls(index_locations.index_urls()) + .index_locations(index_locations) .index_strategy(*index_strategy) .keyring(*keyring_provider) .markers(interpreter.markers()) @@ -1699,8 +1697,7 @@ pub(crate) async fn resolve_environment( .native_tls(network_settings.native_tls) .connectivity(network_settings.connectivity) .allow_insecure_host(network_settings.allow_insecure_host.clone()) - .url_auth_policies(UrlAuthPolicies::from(index_locations)) - .index_urls(index_locations.index_urls()) + .index_locations(index_locations) .index_strategy(*index_strategy) .keyring(*keyring_provider) .markers(interpreter.markers()) @@ -1872,8 +1869,7 @@ pub(crate) async fn sync_environment( .native_tls(network_settings.native_tls) .connectivity(network_settings.connectivity) .allow_insecure_host(network_settings.allow_insecure_host.clone()) - .url_auth_policies(UrlAuthPolicies::from(index_locations)) - .index_urls(index_locations.index_urls()) + .index_locations(index_locations) .index_strategy(index_strategy) .keyring(keyring_provider) .markers(interpreter.markers()) @@ -2085,8 +2081,7 @@ pub(crate) async fn update_environment( .native_tls(network_settings.native_tls) .connectivity(network_settings.connectivity) .allow_insecure_host(network_settings.allow_insecure_host.clone()) - .url_auth_policies(UrlAuthPolicies::from(index_locations)) - .index_urls(index_locations.index_urls()) + .index_locations(index_locations) .index_strategy(*index_strategy) .keyring(*keyring_provider) .markers(interpreter.markers()) diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index db98dfd06dac6..9c16dadf49517 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -7,7 +7,6 @@ use anyhow::{Context, Result}; use itertools::Itertools; use owo_colors::OwoColorize; -use uv_auth::UrlAuthPolicies; use uv_cache::Cache; use uv_client::{FlatIndexClient, RegistryClientBuilder}; use uv_configuration::{ @@ -627,8 +626,7 @@ pub(super) async fn do_sync( .native_tls(network_settings.native_tls) .connectivity(network_settings.connectivity) .allow_insecure_host(network_settings.allow_insecure_host.clone()) - .url_auth_policies(UrlAuthPolicies::from(index_locations)) - .index_urls(index_locations.index_urls()) + .index_locations(index_locations) .index_strategy(index_strategy) .keyring(keyring_provider) .markers(venv.interpreter().markers()) diff --git a/crates/uv/src/commands/project/tree.rs b/crates/uv/src/commands/project/tree.rs index f88b4a38129cb..86ad6d61bd331 100644 --- a/crates/uv/src/commands/project/tree.rs +++ b/crates/uv/src/commands/project/tree.rs @@ -4,7 +4,6 @@ use anstream::print; use anyhow::{Error, Result}; use futures::StreamExt; use tokio::sync::Semaphore; -use uv_auth::UrlAuthPolicies; use uv_cache::{Cache, Refresh}; use uv_cache_info::Timestamp; use uv_client::RegistryClientBuilder; @@ -212,7 +211,7 @@ pub(crate) async fn tree( .native_tls(network_settings.native_tls) .connectivity(network_settings.connectivity) .allow_insecure_host(network_settings.allow_insecure_host.clone()) - .url_auth_policies(UrlAuthPolicies::from(index_locations)) + .index_locations(index_locations) .keyring(*keyring_provider) .build(); let download_concurrency = Semaphore::new(concurrency.downloads); diff --git a/crates/uv/src/commands/publish.rs b/crates/uv/src/commands/publish.rs index 16568373f29aa..83e964c5c551a 100644 --- a/crates/uv/src/commands/publish.rs +++ b/crates/uv/src/commands/publish.rs @@ -89,17 +89,16 @@ pub(crate) async fn publish( // Initialize the registry client. let check_url_client = if let Some(index_url) = &check_url { - let index_urls = IndexLocations::new( + let index_locations = IndexLocations::new( vec![Index::from_index_url(index_url.clone())], Vec::new(), false, - ) - .index_urls(); + ); let registry_client_builder = RegistryClientBuilder::new(cache.clone()) .native_tls(network_settings.native_tls) .connectivity(network_settings.connectivity) .allow_insecure_host(network_settings.allow_insecure_host.clone()) - .index_urls(index_urls) + .index_locations(&index_locations) .keyring(keyring_provider); Some(CheckUrlClient { index_url: index_url.clone(), diff --git a/crates/uv/src/commands/venv.rs b/crates/uv/src/commands/venv.rs index 2580d99e1095f..8d2087c57340b 100644 --- a/crates/uv/src/commands/venv.rs +++ b/crates/uv/src/commands/venv.rs @@ -10,7 +10,6 @@ use miette::{Diagnostic, IntoDiagnostic}; use owo_colors::OwoColorize; use thiserror::Error; -use uv_auth::UrlAuthPolicies; use uv_cache::Cache; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; use uv_configuration::{ @@ -296,9 +295,8 @@ async fn venv_impl( let client = RegistryClientBuilder::try_from(client_builder) .into_diagnostic()? .cache(cache.clone()) - .index_urls(index_locations.index_urls()) + .index_locations(index_locations) .index_strategy(index_strategy) - .url_auth_policies(UrlAuthPolicies::from(index_locations)) .keyring(keyring_provider) .allow_insecure_host(network_settings.allow_insecure_host.clone()) .markers(interpreter.markers()) diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index d6fcb69d6ce6e..c0b3f37444dd8 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -18802,16 +18802,16 @@ fn lock_keyring_credentials() -> Result<()> { uv_snapshot!(context.filters(), context.lock() .env(EnvVars::index_username("PROXY"), "public") .env(EnvVars::KEYRING_TEST_CREDENTIALS, r#"{"pypi-proxy.fly.dev": {"public": "heron"}}"#) - .env(EnvVars::PATH, venv_bin_path(&keyring_context.venv)), @r###" + .env(EnvVars::PATH, venv_bin_path(&keyring_context.venv)), @r" success: true exit_code: 0 ----- stdout ----- ----- stderr ----- - Request for public@https://pypi-proxy.fly.dev/basic-auth/simple/iniconfig/ + Request for public@https://pypi-proxy.fly.dev/basic-auth/simple Request for public@pypi-proxy.fly.dev Resolved 2 packages in [TIME] - "###); + "); let lock = fs_err::read_to_string(context.temp_dir.join("uv.lock")).unwrap(); @@ -18913,7 +18913,7 @@ fn lock_keyring_explicit_always() -> Result<()> { ----- stdout ----- ----- stderr ----- - Request for https://pypi-proxy.fly.dev/basic-auth/simple/iniconfig/ + Request for https://pypi-proxy.fly.dev/basic-auth/simple Request for pypi-proxy.fly.dev × No solution found when resolving dependencies: ╰─▶ Because iniconfig was not found in the package registry and your project depends on iniconfig, we can conclude that your project's requirements are unsatisfiable. @@ -18930,7 +18930,7 @@ fn lock_keyring_explicit_always() -> Result<()> { ----- stdout ----- ----- stderr ----- - Request for https://pypi-proxy.fly.dev/basic-auth/simple/iniconfig/ + Request for https://pypi-proxy.fly.dev/basic-auth/simple Request for pypi-proxy.fly.dev Resolved 2 packages in [TIME] "); @@ -18996,7 +18996,7 @@ fn lock_keyring_credentials_always_authenticate_fetches_username() -> Result<()> ----- stdout ----- ----- stderr ----- - Request for https://pypi-proxy.fly.dev/basic-auth/simple/iniconfig/ + Request for https://pypi-proxy.fly.dev/basic-auth/simple Request for pypi-proxy.fly.dev Resolved 2 packages in [TIME] "); diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index ac45d38d3588e..6793a62ca8da6 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -5335,13 +5335,13 @@ fn install_package_basic_auth_from_keyring() { .arg("subprocess") .arg("--strict") .env(EnvVars::KEYRING_TEST_CREDENTIALS, r#"{"pypi-proxy.fly.dev": {"public": "heron"}}"#) - .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r###" + .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r" success: true exit_code: 0 ----- stdout ----- ----- stderr ----- - Request for public@https://pypi-proxy.fly.dev/basic-auth/simple/anyio/ + Request for public@https://pypi-proxy.fly.dev/basic-auth/simple Request for public@pypi-proxy.fly.dev Resolved 3 packages in [TIME] Prepared 3 packages in [TIME] @@ -5349,7 +5349,7 @@ fn install_package_basic_auth_from_keyring() { + anyio==4.3.0 + idna==3.6 + sniffio==1.3.1 - "### + " ); context.assert_command("import anyio").success(); @@ -5382,19 +5382,19 @@ fn install_package_basic_auth_from_keyring_wrong_password() { .arg("subprocess") .arg("--strict") .env(EnvVars::KEYRING_TEST_CREDENTIALS, r#"{"pypi-proxy.fly.dev": {"public": "foobar"}}"#) - .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r###" + .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r" success: false exit_code: 1 ----- stdout ----- ----- stderr ----- - Request for public@https://pypi-proxy.fly.dev/basic-auth/simple/anyio/ + Request for public@https://pypi-proxy.fly.dev/basic-auth/simple Request for public@pypi-proxy.fly.dev × No solution found when resolving dependencies: ╰─▶ Because anyio was not found in the package registry and you require anyio, we can conclude that your requirements are unsatisfiable. hint: An index URL (https://pypi-proxy.fly.dev/basic-auth/simple) could not be queried due to a lack of valid authentication credentials (401 Unauthorized). - "### + " ); } @@ -5425,19 +5425,19 @@ fn install_package_basic_auth_from_keyring_wrong_username() { .arg("subprocess") .arg("--strict") .env(EnvVars::KEYRING_TEST_CREDENTIALS, r#"{"pypi-proxy.fly.dev": {"other": "heron"}}"#) - .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r###" + .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r" success: false exit_code: 1 ----- stdout ----- ----- stderr ----- - Request for public@https://pypi-proxy.fly.dev/basic-auth/simple/anyio/ + Request for public@https://pypi-proxy.fly.dev/basic-auth/simple Request for public@pypi-proxy.fly.dev × No solution found when resolving dependencies: ╰─▶ Because anyio was not found in the package registry and you require anyio, we can conclude that your requirements are unsatisfiable. hint: An index URL (https://pypi-proxy.fly.dev/basic-auth/simple) could not be queried due to a lack of valid authentication credentials (401 Unauthorized). - "### + " ); } From 23d77a2faea267810a5fc7a5f596cb7abca2847d Mon Sep 17 00:00:00 2001 From: John Mumm Date: Thu, 3 Apr 2025 16:09:05 +0200 Subject: [PATCH 02/13] .. --- crates/uv-auth/src/auth_index.rs | 3 +++ crates/uv-auth/src/lib.rs | 4 ++-- crates/uv-auth/src/middleware.rs | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/uv-auth/src/auth_index.rs b/crates/uv-auth/src/auth_index.rs index 7f2e23542a565..6805a95de6018 100644 --- a/crates/uv-auth/src/auth_index.rs +++ b/crates/uv-auth/src/auth_index.rs @@ -50,7 +50,10 @@ impl Display for AuthPolicy { #[derive(Debug, Clone, Hash, Eq, PartialEq)] pub struct AuthIndex { + /// The PEP 503 simple endpoint for the index pub index_url: Url, + /// The root endpoint where the auth policy is applied. + /// For PEP 503 endpoints, this excludes `/simple`. pub policy_url: Url, pub auth_policy: AuthPolicy, } diff --git a/crates/uv-auth/src/lib.rs b/crates/uv-auth/src/lib.rs index 52b5418b34eec..4959cfa834e95 100644 --- a/crates/uv-auth/src/lib.rs +++ b/crates/uv-auth/src/lib.rs @@ -3,19 +3,19 @@ use std::sync::{Arc, LazyLock}; use tracing::trace; use url::Url; +pub use auth_index::{AuthIndex, AuthIndexes, AuthPolicy}; use cache::CredentialsCache; pub use credentials::Credentials; pub use keyring::KeyringProvider; pub use middleware::AuthMiddleware; use realm::Realm; -pub use auth_index::{AuthIndex, AuthIndexes, AuthPolicy}; +mod auth_index; mod cache; mod credentials; mod keyring; mod middleware; mod realm; -mod auth_index; // TODO(zanieb): Consider passing a cache explicitly throughout diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index 3297d4648595b..1cb39f1a6c6da 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -4,9 +4,9 @@ use http::{Extensions, StatusCode}; use url::Url; use crate::{ + auth_index::{AuthIndexes, AuthPolicy}, credentials::{Credentials, Username}, realm::Realm, - auth_index::{AuthIndexes, AuthPolicy}, CredentialsCache, KeyringProvider, CREDENTIALS_CACHE, }; use anyhow::{anyhow, format_err}; From 7cc3f18aecec6332c78c0d0f3b8b5ebdecd4ac31 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Fri, 4 Apr 2025 10:37:15 +0200 Subject: [PATCH 03/13] .. --- .../uv-auth/src/{auth_index.rs => index.rs} | 29 ++++++++------- crates/uv-auth/src/lib.rs | 4 +-- crates/uv-auth/src/middleware.rs | 36 ++++++++++++------- crates/uv-client/src/base_client.rs | 32 ++++++++--------- crates/uv-client/src/registry_client.rs | 4 +-- crates/uv-distribution-types/src/index_url.rs | 19 +++++----- crates/uv/tests/it/pip_install.rs | 2 ++ 7 files changed, 69 insertions(+), 57 deletions(-) rename crates/uv-auth/src/{auth_index.rs => index.rs} (77%) diff --git a/crates/uv-auth/src/auth_index.rs b/crates/uv-auth/src/index.rs similarity index 77% rename from crates/uv-auth/src/auth_index.rs rename to crates/uv-auth/src/index.rs index 6805a95de6018..2038cddb4572a 100644 --- a/crates/uv-auth/src/auth_index.rs +++ b/crates/uv-auth/src/index.rs @@ -49,9 +49,8 @@ impl Display for AuthPolicy { } #[derive(Debug, Clone, Hash, Eq, PartialEq)] -pub struct AuthIndex { - /// The PEP 503 simple endpoint for the index - pub index_url: Url, +pub struct Index { + pub url: Url, /// The root endpoint where the auth policy is applied. /// For PEP 503 endpoints, this excludes `/simple`. pub policy_url: Url, @@ -59,31 +58,31 @@ pub struct AuthIndex { } #[derive(Debug, Default, Clone, Eq, PartialEq)] -pub struct AuthIndexes(FxHashSet); +pub struct Indexes(FxHashSet); -impl AuthIndexes { +impl Indexes { pub fn new() -> Self { Self(FxHashSet::default()) } /// Create a new [`AuthIndexUrls`] from an iterator of [`AuthIndexUrl`]s. - pub fn from_auth_indexes(urls: impl IntoIterator) -> Self { - let mut auth_index_urls = Self::new(); + pub fn from_indexes(urls: impl IntoIterator) -> Self { + let mut index_urls = Self::new(); for url in urls { - auth_index_urls.0.insert(url); + index_urls.0.insert(url); } - auth_index_urls + index_urls } /// Get the index URL prefix for a URL if one exists. - pub fn auth_index_url_for(&self, url: &Url) -> Option<&Url> { + pub fn index_url_for(&self, url: &Url) -> Option<&Url> { // TODO(john): There are probably not many URLs to iterate through, // but we could use a trie instead of a HashSet here for more // efficient search. self.0 .iter() - .find(|auth_index| url.as_str().starts_with(auth_index.index_url.as_str())) - .map(|auth_index| &auth_index.index_url) + .find(|index| url.as_str().starts_with(index.url.as_str())) + .map(|index| &index.url) } /// Get the [`AuthPolicy`] for a URL. @@ -91,9 +90,9 @@ impl AuthIndexes { // TODO(john): There are probably not many URLs to iterate through, // but we could use a trie instead of a HashMap here for more // efficient search. - for auth_index in &self.0 { - if url.as_str().starts_with(auth_index.policy_url.as_str()) { - return auth_index.auth_policy; + for index in &self.0 { + if url.as_str().starts_with(index.policy_url.as_str()) { + return index.auth_policy; } } AuthPolicy::Auto diff --git a/crates/uv-auth/src/lib.rs b/crates/uv-auth/src/lib.rs index 4959cfa834e95..6aa96a2454e7b 100644 --- a/crates/uv-auth/src/lib.rs +++ b/crates/uv-auth/src/lib.rs @@ -3,16 +3,16 @@ use std::sync::{Arc, LazyLock}; use tracing::trace; use url::Url; -pub use auth_index::{AuthIndex, AuthIndexes, AuthPolicy}; use cache::CredentialsCache; pub use credentials::Credentials; +pub use index::{AuthPolicy, Index, Indexes}; pub use keyring::KeyringProvider; pub use middleware::AuthMiddleware; use realm::Realm; -mod auth_index; mod cache; mod credentials; +mod index; mod keyring; mod middleware; mod realm; diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index 1cb39f1a6c6da..8918a8d56619f 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -4,8 +4,8 @@ use http::{Extensions, StatusCode}; use url::Url; use crate::{ - auth_index::{AuthIndexes, AuthPolicy}, credentials::{Credentials, Username}, + index::{AuthPolicy, Indexes}, realm::Realm, CredentialsCache, KeyringProvider, CREDENTIALS_CACHE, }; @@ -58,7 +58,7 @@ pub struct AuthMiddleware { keyring: Option, cache: Option, /// Auth policies for specific URLs. - auth_indexes: AuthIndexes, + auth_indexes: Indexes, /// Set all endpoints as needing authentication. We never try to send an /// unauthenticated request, avoiding cloning an uncloneable request. only_authenticated: bool, @@ -70,7 +70,7 @@ impl AuthMiddleware { netrc: NetrcMode::default(), keyring: None, cache: None, - auth_indexes: AuthIndexes::new(), + auth_indexes: Indexes::new(), only_authenticated: false, } } @@ -104,7 +104,7 @@ impl AuthMiddleware { /// Configure the [`AuthPolicy`]s to use for URLs. #[must_use] - pub fn with_auth_indexes(mut self, auth_indexes: AuthIndexes) -> Self { + pub fn with_auth_indexes(mut self, auth_indexes: Indexes) -> Self { self.auth_indexes = auth_indexes; self } @@ -477,19 +477,31 @@ impl AuthMiddleware { // URLs; instead, we fetch if there's a username or if the user has requested to // always authenticate. if let Some(username) = credentials.and_then(|credentials| credentials.username()) { - if let Some(index_url) = self.auth_indexes.auth_index_url_for(url) { - debug!("Checking keyring for credentials for index URL {username}@{index_url}"); + let index_credentials = if let Some(index_url) = self.auth_indexes.index_url_for(url) { + debug!("Checking keyring for credentials for index URL {}@{}", username, index_url); keyring.fetch(index_url, Some(username)).await } else { - debug!("Checking keyring for credentials for full URL {username}@{url}"); + None + }; + + if index_credentials.is_some() { + index_credentials + } else { + debug!("Checking keyring for credentials for full URL {}@{}", username, *url); keyring.fetch(url, Some(username)).await } } else if matches!(auth_policy, AuthPolicy::Always) { - if let Some(index_url) = self.auth_indexes.auth_index_url_for(url) { + let index_credentials = if let Some(index_url) = self.auth_indexes.index_url_for(url) { debug!( "Checking keyring for credentials for index URL {index_url} without username due to `authenticate = always`" ); keyring.fetch(index_url, None).await + } else { + None + }; + + if index_credentials.is_some() { + index_credentials } else { debug!( "Checking keyring for credentials for full URL {url} without username due to `authenticate = always`" @@ -551,7 +563,7 @@ mod tests { use wiremock::matchers::{basic_auth, method, path_regex}; use wiremock::{Mock, MockServer, ResponseTemplate}; - use crate::AuthIndex; + use crate::Index; use super::*; @@ -1669,12 +1681,12 @@ mod tests { Ok(()) } - fn auth_indexes_for(url: &Url, policy: AuthPolicy) -> AuthIndexes { + fn auth_indexes_for(url: &Url, policy: AuthPolicy) -> Indexes { let mut url = url.clone(); url.set_password(None).ok(); url.set_username("").ok(); - AuthIndexes::from_auth_indexes(vec![AuthIndex { - index_url: url.clone(), + Indexes::from_indexes(vec![Index { + url: url.clone(), policy_url: url, auth_policy: policy, }]) diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs index 6329d408410c9..67dce8a76400d 100644 --- a/crates/uv-client/src/base_client.rs +++ b/crates/uv-client/src/base_client.rs @@ -16,7 +16,7 @@ use reqwest_retry::{ use tracing::{debug, trace}; use url::Url; -use uv_auth::{AuthIndexes, AuthMiddleware, UrlAuthPolicies}; +use uv_auth::{AuthMiddleware, Indexes, UrlAuthPolicies}; use uv_configuration::{KeyringProviderType, TrustedHost}; use uv_fs::Simplified; use uv_pep508::MarkerEnvironment; @@ -56,7 +56,7 @@ pub struct BaseClientBuilder<'a> { markers: Option<&'a MarkerEnvironment>, platform: Option<&'a Platform>, auth_integration: AuthIntegration, - auth_indexes: Option, + auth_indexes: Option, default_timeout: Duration, extra_middleware: Option, proxies: Vec, @@ -149,7 +149,7 @@ impl<'a> BaseClientBuilder<'a> { } #[must_use] - pub fn auth_indexes(mut self, auth_indexes: AuthIndexes) -> Self { + pub fn auth_indexes(mut self, auth_indexes: Indexes) -> Self { self.auth_indexes = Some(auth_indexes); self } @@ -342,25 +342,25 @@ impl<'a> BaseClientBuilder<'a> { // Initialize the authentication middleware to set headers. match self.auth_integration { AuthIntegration::Default => { - let auth_middleware = - AuthMiddleware::new().with_keyring(self.keyring.to_provider()); - if let Some(auth_indexes) = &self.auth_indexes { - client = client - .with(auth_middleware.with_auth_indexes(auth_indexes.clone())); + let auth_middleware = if let Some(auth_indexes) = &self.auth_indexes { + AuthMiddleware::new().with_auth_indexes(auth_indexes.clone()) } else { - client = client.with(auth_middleware); + AuthMiddleware::new() } + .with_keyring(self.keyring.to_provider()); + + client = client.with(auth_middleware); } AuthIntegration::OnlyAuthenticated => { - let auth_middleware = AuthMiddleware::new() - .with_keyring(self.keyring.to_provider()) - .with_only_authenticated(true); - if let Some(auth_indexes) = &self.auth_indexes { - client = client - .with(auth_middleware.with_auth_indexes(auth_indexes.clone())); + let auth_middleware = if let Some(auth_indexes) = &self.auth_indexes { + AuthMiddleware::new().with_auth_indexes(auth_indexes.clone()) } else { - client = client.with(auth_middleware); + AuthMiddleware::new() } + .with_keyring(self.keyring.to_provider()) + .with_only_authenticated(true); + + client = client.with(auth_middleware); } AuthIntegration::NoAuthMiddleware => { // The downstream code uses custom auth logic. diff --git a/crates/uv-client/src/registry_client.rs b/crates/uv-client/src/registry_client.rs index e6364fa3f9d26..2614f85de70af 100644 --- a/crates/uv-client/src/registry_client.rs +++ b/crates/uv-client/src/registry_client.rs @@ -16,7 +16,7 @@ use tokio::sync::{Mutex, Semaphore}; use tracing::{info_span, instrument, trace, warn, Instrument}; use url::Url; -use uv_auth::AuthIndexes; +use uv_auth::Indexes; use uv_cache::{Cache, CacheBucket, CacheEntry, WheelCache}; use uv_configuration::KeyringProviderType; use uv_configuration::{IndexStrategy, TrustedHost}; @@ -73,7 +73,7 @@ impl<'a> RegistryClientBuilder<'a> { self.index_urls = index_locations.index_urls(); self.base_client_builder = self .base_client_builder - .auth_indexes(AuthIndexes::from(index_locations)); + .auth_indexes(Indexes::from(index_locations)); self } diff --git a/crates/uv-distribution-types/src/index_url.rs b/crates/uv-distribution-types/src/index_url.rs index 3472b7893f9e2..943d00b7edaf1 100644 --- a/crates/uv-distribution-types/src/index_url.rs +++ b/crates/uv-distribution-types/src/index_url.rs @@ -10,7 +10,6 @@ use rustc_hash::{FxHashMap, FxHashSet}; use thiserror::Error; use url::{ParseError, Url}; -use uv_auth::{AuthIndex, AuthIndexes}; use uv_pep508::{split_scheme, Scheme, VerbatimUrl, VerbatimUrlError}; use crate::{Index, Verbatim}; @@ -411,17 +410,17 @@ impl<'a> IndexLocations { } } -impl From<&IndexLocations> for AuthIndexes { - fn from(index_locations: &IndexLocations) -> AuthIndexes { - AuthIndexes::from_auth_indexes(index_locations.allowed_indexes().into_iter().map(|index| { - let mut index_url = index.url().url().clone(); - index_url.set_username("").ok(); - index_url.set_password(None).ok(); - let mut policy_url = index.url().root().unwrap_or_else(|| index_url.clone()); +impl From<&IndexLocations> for uv_auth::Indexes { + fn from(index_locations: &IndexLocations) -> uv_auth::Indexes { + uv_auth::Indexes::from_indexes(index_locations.allowed_indexes().into_iter().map(|index| { + let mut url = index.url().url().clone(); + url.set_username("").ok(); + url.set_password(None).ok(); + let mut policy_url = index.url().root().unwrap_or_else(|| url.clone()); policy_url.set_username("").ok(); policy_url.set_password(None).ok(); - AuthIndex { - index_url, + uv_auth::Index { + url, policy_url, auth_policy: index.authenticate, } diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 6793a62ca8da6..18d88aaf87528 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -5432,6 +5432,8 @@ fn install_package_basic_auth_from_keyring_wrong_username() { ----- stderr ----- Request for public@https://pypi-proxy.fly.dev/basic-auth/simple + Request for public@pypi-proxy.fly.dev + Request for public@https://pypi-proxy.fly.dev/basic-auth/simple/anyio/ Request for public@pypi-proxy.fly.dev × No solution found when resolving dependencies: ╰─▶ Because anyio was not found in the package registry and you require anyio, we can conclude that your requirements are unsatisfiable. From b2d8f75b7fd80a03c7b159748fe8364079d18fa4 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Fri, 4 Apr 2025 20:56:58 +0200 Subject: [PATCH 04/13] .. --- crates/uv-client/src/base_client.rs | 29 ++++++++++--------------- crates/uv-client/src/registry_client.rs | 2 +- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs index 67dce8a76400d..1fddb43dda29e 100644 --- a/crates/uv-client/src/base_client.rs +++ b/crates/uv-client/src/base_client.rs @@ -56,7 +56,7 @@ pub struct BaseClientBuilder<'a> { markers: Option<&'a MarkerEnvironment>, platform: Option<&'a Platform>, auth_integration: AuthIntegration, - auth_indexes: Option, + indexes: Indexes, default_timeout: Duration, extra_middleware: Option, proxies: Vec, @@ -91,7 +91,7 @@ impl BaseClientBuilder<'_> { markers: None, platform: None, auth_integration: AuthIntegration::default(), - auth_indexes: None, + indexes: Indexes::new(), default_timeout: Duration::from_secs(30), extra_middleware: None, proxies: vec![], @@ -149,8 +149,8 @@ impl<'a> BaseClientBuilder<'a> { } #[must_use] - pub fn auth_indexes(mut self, auth_indexes: Indexes) -> Self { - self.auth_indexes = Some(auth_indexes); + pub fn indexes(mut self, indexes: Indexes) -> Self { + self.indexes = indexes; self } @@ -342,23 +342,16 @@ impl<'a> BaseClientBuilder<'a> { // Initialize the authentication middleware to set headers. match self.auth_integration { AuthIntegration::Default => { - let auth_middleware = if let Some(auth_indexes) = &self.auth_indexes { - AuthMiddleware::new().with_auth_indexes(auth_indexes.clone()) - } else { - AuthMiddleware::new() - } - .with_keyring(self.keyring.to_provider()); - + let auth_middleware = AuthMiddleware::new() + .with_auth_indexes(self.indexes.clone()) + .with_keyring(self.keyring.to_provider()); client = client.with(auth_middleware); } AuthIntegration::OnlyAuthenticated => { - let auth_middleware = if let Some(auth_indexes) = &self.auth_indexes { - AuthMiddleware::new().with_auth_indexes(auth_indexes.clone()) - } else { - AuthMiddleware::new() - } - .with_keyring(self.keyring.to_provider()) - .with_only_authenticated(true); + let auth_middleware = AuthMiddleware::new() + .with_auth_indexes(self.indexes.clone()) + .with_keyring(self.keyring.to_provider()) + .with_only_authenticated(true); client = client.with(auth_middleware); } diff --git a/crates/uv-client/src/registry_client.rs b/crates/uv-client/src/registry_client.rs index 2614f85de70af..d08bdb7e10e09 100644 --- a/crates/uv-client/src/registry_client.rs +++ b/crates/uv-client/src/registry_client.rs @@ -73,7 +73,7 @@ impl<'a> RegistryClientBuilder<'a> { self.index_urls = index_locations.index_urls(); self.base_client_builder = self .base_client_builder - .auth_indexes(Indexes::from(index_locations)); + .indexes(Indexes::from(index_locations)); self } From c01549814d3302b7c6e33cd0a37b04aaa2486678 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Fri, 4 Apr 2025 21:06:20 +0200 Subject: [PATCH 05/13] .. --- crates/uv-auth/src/index.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/uv-auth/src/index.rs b/crates/uv-auth/src/index.rs index 2038cddb4572a..3d03eb9a8028f 100644 --- a/crates/uv-auth/src/index.rs +++ b/crates/uv-auth/src/index.rs @@ -48,6 +48,9 @@ impl Display for AuthPolicy { } } +// TODO(john): We are not using `uv_distribution_types::Index` directly +// here because it would cause circular crate dependencies. However, this +// could potentially make sense for a future refactor. #[derive(Debug, Clone, Hash, Eq, PartialEq)] pub struct Index { pub url: Url, From 2800f685246703a7d350566d53b5c8c9231a06eb Mon Sep 17 00:00:00 2001 From: John Mumm Date: Fri, 4 Apr 2025 21:08:28 +0200 Subject: [PATCH 06/13] .. --- crates/uv-client/src/base_client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs index 1fddb43dda29e..3bbd2a65b7e3f 100644 --- a/crates/uv-client/src/base_client.rs +++ b/crates/uv-client/src/base_client.rs @@ -16,7 +16,7 @@ use reqwest_retry::{ use tracing::{debug, trace}; use url::Url; -use uv_auth::{AuthMiddleware, Indexes, UrlAuthPolicies}; +use uv_auth::{AuthMiddleware, Indexes}; use uv_configuration::{KeyringProviderType, TrustedHost}; use uv_fs::Simplified; use uv_pep508::MarkerEnvironment; From b87b8526ca3604df8d186f6c1a75a9a81f0109d3 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Fri, 4 Apr 2025 21:13:13 +0200 Subject: [PATCH 07/13] .. --- crates/uv-auth/src/middleware.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index 8918a8d56619f..ad5aa58c8d219 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -480,34 +480,22 @@ impl AuthMiddleware { let index_credentials = if let Some(index_url) = self.auth_indexes.index_url_for(url) { debug!("Checking keyring for credentials for index URL {}@{}", username, index_url); keyring.fetch(index_url, Some(username)).await - } else { - None - }; - - if index_credentials.is_some() { - index_credentials } else { debug!("Checking keyring for credentials for full URL {}@{}", username, *url); keyring.fetch(url, Some(username)).await - } + }; } else if matches!(auth_policy, AuthPolicy::Always) { let index_credentials = if let Some(index_url) = self.auth_indexes.index_url_for(url) { debug!( "Checking keyring for credentials for index URL {index_url} without username due to `authenticate = always`" ); keyring.fetch(index_url, None).await - } else { - None - }; - - if index_credentials.is_some() { - index_credentials } else { debug!( "Checking keyring for credentials for full URL {url} without username due to `authenticate = always`" ); keyring.fetch(url, None).await - } + }; } else { debug!("Skipping keyring fetch for {url} without username; use `authenticate = always` to force"); None From c85d03b1f1c0ed2ea67ec7925602ea794e158894 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Fri, 4 Apr 2025 21:16:12 +0200 Subject: [PATCH 08/13] .. --- crates/uv-auth/src/middleware.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index ad5aa58c8d219..3571d32bfefa9 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -477,15 +477,15 @@ impl AuthMiddleware { // URLs; instead, we fetch if there's a username or if the user has requested to // always authenticate. if let Some(username) = credentials.and_then(|credentials| credentials.username()) { - let index_credentials = if let Some(index_url) = self.auth_indexes.index_url_for(url) { + if let Some(index_url) = self.auth_indexes.index_url_for(url) { debug!("Checking keyring for credentials for index URL {}@{}", username, index_url); keyring.fetch(index_url, Some(username)).await } else { debug!("Checking keyring for credentials for full URL {}@{}", username, *url); keyring.fetch(url, Some(username)).await - }; + } } else if matches!(auth_policy, AuthPolicy::Always) { - let index_credentials = if let Some(index_url) = self.auth_indexes.index_url_for(url) { + if let Some(index_url) = self.auth_indexes.index_url_for(url) { debug!( "Checking keyring for credentials for index URL {index_url} without username due to `authenticate = always`" ); @@ -495,7 +495,7 @@ impl AuthMiddleware { "Checking keyring for credentials for full URL {url} without username due to `authenticate = always`" ); keyring.fetch(url, None).await - }; + } } else { debug!("Skipping keyring fetch for {url} without username; use `authenticate = always` to force"); None From 0d79ca837c6500030e37749fc9b2071e933b9062 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Fri, 4 Apr 2025 21:17:48 +0200 Subject: [PATCH 09/13] .. --- crates/uv/tests/it/pip_install.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 18d88aaf87528..6793a62ca8da6 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -5432,8 +5432,6 @@ fn install_package_basic_auth_from_keyring_wrong_username() { ----- stderr ----- Request for public@https://pypi-proxy.fly.dev/basic-auth/simple - Request for public@pypi-proxy.fly.dev - Request for public@https://pypi-proxy.fly.dev/basic-auth/simple/anyio/ Request for public@pypi-proxy.fly.dev × No solution found when resolving dependencies: ╰─▶ Because anyio was not found in the package registry and you require anyio, we can conclude that your requirements are unsatisfiable. From 2042f1d6409513332be85831b78a9c8e011214c4 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Mon, 7 Apr 2025 10:29:16 +0200 Subject: [PATCH 10/13] .. --- crates/uv-auth/src/middleware.rs | 36 ++++++++++++++--------------- crates/uv-client/src/base_client.rs | 4 ++-- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index 3571d32bfefa9..3c5837d95e8d7 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -58,7 +58,7 @@ pub struct AuthMiddleware { keyring: Option, cache: Option, /// Auth policies for specific URLs. - auth_indexes: Indexes, + indexes: Indexes, /// Set all endpoints as needing authentication. We never try to send an /// unauthenticated request, avoiding cloning an uncloneable request. only_authenticated: bool, @@ -70,7 +70,7 @@ impl AuthMiddleware { netrc: NetrcMode::default(), keyring: None, cache: None, - auth_indexes: Indexes::new(), + indexes: Indexes::new(), only_authenticated: false, } } @@ -104,8 +104,8 @@ impl AuthMiddleware { /// Configure the [`AuthPolicy`]s to use for URLs. #[must_use] - pub fn with_auth_indexes(mut self, auth_indexes: Indexes) -> Self { - self.auth_indexes = auth_indexes; + pub fn with_indexes(mut self, indexes: Indexes) -> Self { + self.indexes = indexes; self } @@ -181,7 +181,7 @@ impl Middleware for AuthMiddleware { // In the middleware, existing credentials are already moved from the URL // to the headers so for display purposes we restore some information let url = tracing_url(&request, request_credentials.as_ref()); - let auth_policy = self.auth_indexes.policy_for(request.url()); + let auth_policy = self.indexes.policy_for(request.url()); trace!("Handling request for {url} with authentication policy {auth_policy}"); let credentials: Option> = if matches!(auth_policy, AuthPolicy::Never) { @@ -477,7 +477,7 @@ impl AuthMiddleware { // URLs; instead, we fetch if there's a username or if the user has requested to // always authenticate. if let Some(username) = credentials.and_then(|credentials| credentials.username()) { - if let Some(index_url) = self.auth_indexes.index_url_for(url) { + if let Some(index_url) = self.indexes.index_url_for(url) { debug!("Checking keyring for credentials for index URL {}@{}", username, index_url); keyring.fetch(index_url, Some(username)).await } else { @@ -485,7 +485,7 @@ impl AuthMiddleware { keyring.fetch(url, Some(username)).await } } else if matches!(auth_policy, AuthPolicy::Always) { - if let Some(index_url) = self.auth_indexes.index_url_for(url) { + if let Some(index_url) = self.indexes.index_url_for(url) { debug!( "Checking keyring for credentials for index URL {index_url} without username due to `authenticate = always`" ); @@ -1012,7 +1012,7 @@ mod tests { let server = start_test_server(username, password).await; let base_url = Url::parse(&server.uri())?; - let auth_indexes = auth_indexes_for(&base_url, AuthPolicy::Always); + let indexes = indexes_for(&base_url, AuthPolicy::Always); let client = test_client_builder() .with( AuthMiddleware::new() @@ -1026,7 +1026,7 @@ mod tests { username, password, )]))) - .with_auth_indexes(auth_indexes), + .with_indexes(indexes), ) .build(); @@ -1669,7 +1669,7 @@ mod tests { Ok(()) } - fn auth_indexes_for(url: &Url, policy: AuthPolicy) -> Indexes { + fn indexes_for(url: &Url, policy: AuthPolicy) -> Indexes { let mut url = url.clone(); url.set_password(None).ok(); url.set_username("").ok(); @@ -1691,12 +1691,12 @@ mod tests { let base_url = Url::parse(&server.uri())?; - let auth_indexes = auth_indexes_for(&base_url, AuthPolicy::Always); + let indexes = indexes_for(&base_url, AuthPolicy::Always); let client = test_client_builder() .with( AuthMiddleware::new() .with_cache(CredentialsCache::new()) - .with_auth_indexes(auth_indexes), + .with_indexes(indexes), ) .build(); @@ -1758,12 +1758,12 @@ mod tests { let base_url = Url::parse(&server.uri())?; - let auth_indexes = auth_indexes_for(&base_url, AuthPolicy::Always); + let indexes = indexes_for(&base_url, AuthPolicy::Always); let client = test_client_builder() .with( AuthMiddleware::new() .with_cache(CredentialsCache::new()) - .with_auth_indexes(auth_indexes), + .with_indexes(indexes), ) .build(); @@ -1798,12 +1798,12 @@ mod tests { .mount(&server) .await; - let auth_indexes = auth_indexes_for(&base_url, AuthPolicy::Never); + let indexes = indexes_for(&base_url, AuthPolicy::Never); let client = test_client_builder() .with( AuthMiddleware::new() .with_cache(CredentialsCache::new()) - .with_auth_indexes(auth_indexes), + .with_indexes(indexes), ) .build(); @@ -1843,12 +1843,12 @@ mod tests { let base_url = Url::parse(&server.uri())?; - let auth_indexes = auth_indexes_for(&base_url, AuthPolicy::Never); + let indexes = indexes_for(&base_url, AuthPolicy::Never); let client = test_client_builder() .with( AuthMiddleware::new() .with_cache(CredentialsCache::new()) - .with_auth_indexes(auth_indexes), + .with_indexes(indexes), ) .build(); diff --git a/crates/uv-client/src/base_client.rs b/crates/uv-client/src/base_client.rs index 3bbd2a65b7e3f..bf36285475018 100644 --- a/crates/uv-client/src/base_client.rs +++ b/crates/uv-client/src/base_client.rs @@ -343,13 +343,13 @@ impl<'a> BaseClientBuilder<'a> { match self.auth_integration { AuthIntegration::Default => { let auth_middleware = AuthMiddleware::new() - .with_auth_indexes(self.indexes.clone()) + .with_indexes(self.indexes.clone()) .with_keyring(self.keyring.to_provider()); client = client.with(auth_middleware); } AuthIntegration::OnlyAuthenticated => { let auth_middleware = AuthMiddleware::new() - .with_auth_indexes(self.indexes.clone()) + .with_indexes(self.indexes.clone()) .with_keyring(self.keyring.to_provider()) .with_only_authenticated(true); From 599e0cf92fc57223fe8477278bdf4df46367df6c Mon Sep 17 00:00:00 2001 From: John Mumm Date: Wed, 9 Apr 2025 14:20:08 +0200 Subject: [PATCH 11/13] .. --- crates/uv-auth/src/index.rs | 11 +++++------ crates/uv-auth/src/middleware.rs | 3 +-- crates/uv-distribution-types/src/index_url.rs | 11 +++++------ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/crates/uv-auth/src/index.rs b/crates/uv-auth/src/index.rs index 3d03eb9a8028f..63bbe2e4f137f 100644 --- a/crates/uv-auth/src/index.rs +++ b/crates/uv-auth/src/index.rs @@ -53,10 +53,9 @@ impl Display for AuthPolicy { // could potentially make sense for a future refactor. #[derive(Debug, Clone, Hash, Eq, PartialEq)] pub struct Index { - pub url: Url, - /// The root endpoint where the auth policy is applied. + /// The root endpoint where authentication is applied. /// For PEP 503 endpoints, this excludes `/simple`. - pub policy_url: Url, + pub root_url: Url, pub auth_policy: AuthPolicy, } @@ -84,8 +83,8 @@ impl Indexes { // efficient search. self.0 .iter() - .find(|index| url.as_str().starts_with(index.url.as_str())) - .map(|index| &index.url) + .find(|index| url.as_str().starts_with(index.root_url.as_str())) + .map(|index| &index.root_url) } /// Get the [`AuthPolicy`] for a URL. @@ -94,7 +93,7 @@ impl Indexes { // but we could use a trie instead of a HashMap here for more // efficient search. for index in &self.0 { - if url.as_str().starts_with(index.policy_url.as_str()) { + if url.as_str().starts_with(index.root_url.as_str()) { return index.auth_policy; } } diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index 3c5837d95e8d7..265c2bc612170 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -1674,8 +1674,7 @@ mod tests { url.set_password(None).ok(); url.set_username("").ok(); Indexes::from_indexes(vec![Index { - url: url.clone(), - policy_url: url, + root_url: url.clone(), auth_policy: policy, }]) } diff --git a/crates/uv-distribution-types/src/index_url.rs b/crates/uv-distribution-types/src/index_url.rs index 943d00b7edaf1..606be1c1cd55b 100644 --- a/crates/uv-distribution-types/src/index_url.rs +++ b/crates/uv-distribution-types/src/index_url.rs @@ -413,15 +413,14 @@ impl<'a> IndexLocations { impl From<&IndexLocations> for uv_auth::Indexes { fn from(index_locations: &IndexLocations) -> uv_auth::Indexes { uv_auth::Indexes::from_indexes(index_locations.allowed_indexes().into_iter().map(|index| { - let mut url = index.url().url().clone(); + let mut url = index + .url() + .root() + .unwrap_or_else(|| index.url().url().clone()); url.set_username("").ok(); url.set_password(None).ok(); - let mut policy_url = index.url().root().unwrap_or_else(|| url.clone()); - policy_url.set_username("").ok(); - policy_url.set_password(None).ok(); uv_auth::Index { - url, - policy_url, + root_url: url, auth_policy: index.authenticate, } })) From 3fca1959ed9ff23dc13a23eeb44569b02be0d691 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Wed, 9 Apr 2025 15:12:02 +0200 Subject: [PATCH 12/13] .. --- crates/uv/tests/it/lock.rs | 16 ++++++++-------- crates/uv/tests/it/pip_install.rs | 12 ++++++------ crates/uv/tests/it/publish.rs | 18 +++++++++--------- .../keyrings/test_keyring.py | 4 ++-- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index c0b3f37444dd8..f80f516564489 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -18808,8 +18808,8 @@ fn lock_keyring_credentials() -> Result<()> { ----- stdout ----- ----- stderr ----- - Request for public@https://pypi-proxy.fly.dev/basic-auth/simple - Request for public@pypi-proxy.fly.dev + Keyring request for public@https://pypi-proxy.fly.dev/basic-auth + Keyring request for public@pypi-proxy.fly.dev Resolved 2 packages in [TIME] "); @@ -18913,8 +18913,8 @@ fn lock_keyring_explicit_always() -> Result<()> { ----- stdout ----- ----- stderr ----- - Request for https://pypi-proxy.fly.dev/basic-auth/simple - Request for pypi-proxy.fly.dev + Keyring request for https://pypi-proxy.fly.dev/basic-auth + Keyring request for pypi-proxy.fly.dev × No solution found when resolving dependencies: ╰─▶ Because iniconfig was not found in the package registry and your project depends on iniconfig, we can conclude that your project's requirements are unsatisfiable. @@ -18930,8 +18930,8 @@ fn lock_keyring_explicit_always() -> Result<()> { ----- stdout ----- ----- stderr ----- - Request for https://pypi-proxy.fly.dev/basic-auth/simple - Request for pypi-proxy.fly.dev + Keyring request for https://pypi-proxy.fly.dev/basic-auth + Keyring request for pypi-proxy.fly.dev Resolved 2 packages in [TIME] "); @@ -18996,8 +18996,8 @@ fn lock_keyring_credentials_always_authenticate_fetches_username() -> Result<()> ----- stdout ----- ----- stderr ----- - Request for https://pypi-proxy.fly.dev/basic-auth/simple - Request for pypi-proxy.fly.dev + Keyring request for https://pypi-proxy.fly.dev/basic-auth + Keyring request for pypi-proxy.fly.dev Resolved 2 packages in [TIME] "); diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 6793a62ca8da6..aa11a613dbb7f 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -5341,8 +5341,8 @@ fn install_package_basic_auth_from_keyring() { ----- stdout ----- ----- stderr ----- - Request for public@https://pypi-proxy.fly.dev/basic-auth/simple - Request for public@pypi-proxy.fly.dev + Keyring request for public@https://pypi-proxy.fly.dev/basic-auth + Keyring request for public@pypi-proxy.fly.dev Resolved 3 packages in [TIME] Prepared 3 packages in [TIME] Installed 3 packages in [TIME] @@ -5388,8 +5388,8 @@ fn install_package_basic_auth_from_keyring_wrong_password() { ----- stdout ----- ----- stderr ----- - Request for public@https://pypi-proxy.fly.dev/basic-auth/simple - Request for public@pypi-proxy.fly.dev + Keyring request for public@https://pypi-proxy.fly.dev/basic-auth + Keyring request for public@pypi-proxy.fly.dev × No solution found when resolving dependencies: ╰─▶ Because anyio was not found in the package registry and you require anyio, we can conclude that your requirements are unsatisfiable. @@ -5431,8 +5431,8 @@ fn install_package_basic_auth_from_keyring_wrong_username() { ----- stdout ----- ----- stderr ----- - Request for public@https://pypi-proxy.fly.dev/basic-auth/simple - Request for public@pypi-proxy.fly.dev + Keyring request for public@https://pypi-proxy.fly.dev/basic-auth + Keyring request for public@pypi-proxy.fly.dev × No solution found when resolving dependencies: ╰─▶ Because anyio was not found in the package registry and you require anyio, we can conclude that your requirements are unsatisfiable. diff --git a/crates/uv/tests/it/publish.rs b/crates/uv/tests/it/publish.rs index ae33eea84d075..a45ccb8015e47 100644 --- a/crates/uv/tests/it/publish.rs +++ b/crates/uv/tests/it/publish.rs @@ -276,22 +276,22 @@ fn check_keyring_behaviours() { .arg("--publish-url") .arg("https://test.pypi.org/legacy/?ok") .arg("../../scripts/links/ok-1.0.0-py3-none-any.whl") - .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r###" + .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r" success: false exit_code: 2 ----- stdout ----- ----- stderr ----- Publishing 1 file to https://test.pypi.org/legacy/?ok - Request for dummy@https://test.pypi.org/legacy/?ok - Request for dummy@test.pypi.org + Keyring request for dummy@https://test.pypi.org/legacy/?ok + Keyring request for dummy@test.pypi.org warning: Keyring has no password for URL `https://test.pypi.org/legacy/?ok` and username `dummy` Uploading ok-1.0.0-py3-none-any.whl ([SIZE]) - Request for dummy@https://test.pypi.org/legacy/?ok - Request for dummy@test.pypi.org + Keyring request for dummy@https://test.pypi.org/legacy/?ok + Keyring request for dummy@test.pypi.org error: Failed to publish `../../scripts/links/ok-1.0.0-py3-none-any.whl` to https://test.pypi.org/legacy/?ok Caused by: Upload failed with status code 403 Forbidden. Server says: 403 Username/Password authentication is no longer supported. Migrate to API Tokens or Trusted Publishers instead. See https://test.pypi.org/help/#apitoken and https://test.pypi.org/help/#trusted-publishers - "### + " ); // Ok: There is a keyring entry for the user dummy. @@ -305,18 +305,18 @@ fn check_keyring_behaviours() { .arg("https://test.pypi.org/legacy/?ok") .arg("../../scripts/links/ok-1.0.0-py3-none-any.whl") .env(EnvVars::KEYRING_TEST_CREDENTIALS, r#"{"https://test.pypi.org/legacy/?ok": {"dummy": "dummy"}}"#) - .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r###" + .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r" success: false exit_code: 2 ----- stdout ----- ----- stderr ----- Publishing 1 file to https://test.pypi.org/legacy/?ok - Request for dummy@https://test.pypi.org/legacy/?ok + Keyring request for dummy@https://test.pypi.org/legacy/?ok Uploading ok-1.0.0-py3-none-any.whl ([SIZE]) error: Failed to publish `../../scripts/links/ok-1.0.0-py3-none-any.whl` to https://test.pypi.org/legacy/?ok Caused by: Upload failed with status code 403 Forbidden. Server says: 403 Username/Password authentication is no longer supported. Migrate to API Tokens or Trusted Publishers instead. See https://test.pypi.org/help/#apitoken and https://test.pypi.org/help/#trusted-publishers - "### + " ); } diff --git a/scripts/packages/keyring_test_plugin/keyrings/test_keyring.py b/scripts/packages/keyring_test_plugin/keyrings/test_keyring.py index f069506025b1c..59f7b0d6d54f7 100644 --- a/scripts/packages/keyring_test_plugin/keyrings/test_keyring.py +++ b/scripts/packages/keyring_test_plugin/keyrings/test_keyring.py @@ -9,7 +9,7 @@ class KeyringTest(backend.KeyringBackend): priority = 9 def get_password(self, service, username): - print(f"Request for {username}@{service}", file=sys.stderr) + print(f"Keyring request for {username}@{service}", file=sys.stderr) entries = json.loads(os.environ.get("KEYRING_TEST_CREDENTIALS", "{}")) return entries.get(service, {}).get(username) @@ -20,7 +20,7 @@ def delete_password(self, service, username): raise NotImplementedError() def get_credential(self, service, username): - print(f"Request for {service}", file=sys.stderr) + print(f"Keyring request for {service}", file=sys.stderr) entries = json.loads(os.environ.get("KEYRING_TEST_CREDENTIALS", "{}")) service_entries = entries.get(service, {}) if not service_entries: From d7b6799da7bd8ae6bc9c774099e57bbab11b82b5 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Thu, 10 Apr 2025 11:52:39 +0200 Subject: [PATCH 13/13] Use index-level instead of realm-level credential caching for known indexes (#12717) The current uv behavior is to cache credentials either at the request URL or realm level. But in general, the expected behavior for indexes is to apply credentials at the index level (as implemented in #12651). This means that we also need to cache credentials at this level. Note that when uv does not detect an index URL for a request URL, it will continue to apply the old behavior. Depends on #12651. --- crates/uv-auth/src/cache.rs | 4 +- crates/uv-auth/src/index.rs | 18 +- crates/uv-auth/src/middleware.rs | 316 +++++++++++++++--- crates/uv-distribution-types/src/index_url.rs | 11 +- crates/uv/tests/it/edit.rs | 122 ++++++- crates/uv/tests/it/lock.rs | 8 +- crates/uv/tests/it/pip_install.rs | 6 +- docs/configuration/authentication.md | 6 +- 8 files changed, 430 insertions(+), 61 deletions(-) diff --git a/crates/uv-auth/src/cache.rs b/crates/uv-auth/src/cache.rs index 0203efe8bd6d8..e8894727b7b6e 100644 --- a/crates/uv-auth/src/cache.rs +++ b/crates/uv-auth/src/cache.rs @@ -17,8 +17,8 @@ type FxOnceMap = OnceMap>; pub struct CredentialsCache { /// A cache per realm and username realms: RwLock>>, - /// A cache tracking the result of fetches from external services - pub(crate) fetches: FxOnceMap<(Realm, Username), Option>>, + /// A cache tracking the result of realm or index URL fetches from external services + pub(crate) fetches: FxOnceMap<(String, Username), Option>>, /// A cache per URL, uses a trie for efficient prefix queries. urls: RwLock, } diff --git a/crates/uv-auth/src/index.rs b/crates/uv-auth/src/index.rs index 63bbe2e4f137f..b420c5ec33762 100644 --- a/crates/uv-auth/src/index.rs +++ b/crates/uv-auth/src/index.rs @@ -53,6 +53,7 @@ impl Display for AuthPolicy { // could potentially make sense for a future refactor. #[derive(Debug, Clone, Hash, Eq, PartialEq)] pub struct Index { + pub url: Url, /// The root endpoint where authentication is applied. /// For PEP 503 endpoints, this excludes `/simple`. pub root_url: Url, @@ -83,8 +84,8 @@ impl Indexes { // efficient search. self.0 .iter() - .find(|index| url.as_str().starts_with(index.root_url.as_str())) - .map(|index| &index.root_url) + .find(|index| is_url_prefix(&index.root_url, url)) + .map(|index| &index.url) } /// Get the [`AuthPolicy`] for a URL. @@ -93,10 +94,21 @@ impl Indexes { // but we could use a trie instead of a HashMap here for more // efficient search. for index in &self.0 { - if url.as_str().starts_with(index.root_url.as_str()) { + if is_url_prefix(&index.root_url, url) { return index.auth_policy; } } AuthPolicy::Auto } } + +fn is_url_prefix(base: &Url, url: &Url) -> bool { + if base.scheme() != url.scheme() + || base.host_str() != url.host_str() + || base.port_or_known_default() != url.port_or_known_default() + { + return false; + } + + url.path().starts_with(base.path()) +} diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index 265c2bc612170..64d37315ef689 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -148,7 +148,7 @@ impl Middleware for AuthMiddleware { /// We'll avoid making a request we expect to fail and look for a password. /// The discovered credentials must have the requested username to be used. /// - /// - Check the cache (realm key) for a password + /// - Check the cache (index URL or realm key) for a password /// - Check the netrc for a password /// - Check the keyring for a password /// - Perform the request @@ -162,10 +162,10 @@ impl Middleware for AuthMiddleware { /// server tells us authorization is needed. This pattern avoids attaching credentials to /// requests that do not need them, which can cause some servers to deny the request. /// - /// - Check the cache (url key) + /// - Check the cache (URL key) /// - Perform the request /// - On 401, 403, or 404 check for authentication if there was a cache miss - /// - Check the cache (realm key) for the username and password + /// - Check the cache (index URL or realm key) for the username and password /// - Check the netrc for a username and password /// - Perform the request again if found /// - Add the username and password to the cache if successful @@ -181,6 +181,7 @@ impl Middleware for AuthMiddleware { // In the middleware, existing credentials are already moved from the URL // to the headers so for display purposes we restore some information let url = tracing_url(&request, request_credentials.as_ref()); + let maybe_index_url = self.indexes.index_url_for(request.url()); let auth_policy = self.indexes.policy_for(request.url()); trace!("Handling request for {url} with authentication policy {auth_policy}"); @@ -195,6 +196,7 @@ impl Middleware for AuthMiddleware { extensions, next, &url, + maybe_index_url, auth_policy, ) .await; @@ -272,17 +274,20 @@ impl Middleware for AuthMiddleware { (request, None) }; - // Check if there are credentials in the realm-level cache - let credentials = self - .cache() - .get_realm( - Realm::from(retry_request.url()), - credentials - .as_ref() - .map(|credentials| credentials.to_username()) - .unwrap_or(Username::none()), - ) - .or(credentials); + let username = credentials + .as_ref() + .map(|credentials| credentials.to_username()) + .unwrap_or(Username::none()); + let credentials = if let Some(index_url) = maybe_index_url { + self.cache().get_url(index_url, &username) + } else { + // Since there is no known index for this URL, check if there are credentials in + // the realm-level cache. + self.cache() + .get_realm(Realm::from(retry_request.url()), username) + } + .or(credentials); + if let Some(credentials) = credentials.as_ref() { if credentials.password().is_some() { trace!("Retrying request for {url} with credentials from cache {credentials:?}"); @@ -296,7 +301,12 @@ impl Middleware for AuthMiddleware { // Then, fetch from external services. // Here, we use the username from the cache if present. if let Some(credentials) = self - .fetch_credentials(credentials.as_deref(), retry_request.url(), auth_policy) + .fetch_credentials( + credentials.as_deref(), + retry_request.url(), + maybe_index_url, + auth_policy, + ) .await { retry_request = credentials.authenticate(retry_request); @@ -374,6 +384,7 @@ impl AuthMiddleware { extensions: &mut Extensions, next: Next<'_>, url: &str, + maybe_index_url: Option<&Url>, auth_policy: AuthPolicy, ) -> reqwest_middleware::Result { let credentials = Arc::new(credentials); @@ -387,15 +398,27 @@ impl AuthMiddleware { } trace!("Request for {url} is missing a password, looking for credentials"); - // There's just a username, try to find a password - let credentials = if let Some(credentials) = self - .cache() - .get_realm(Realm::from(request.url()), credentials.to_username()) - { + + // There's just a username, try to find a password. + // If we have an index URL, check the cache for that URL. Otherwise, + // check for the realm. + let maybe_cached_credentials = if let Some(index_url) = maybe_index_url { + self.cache() + .get_url(index_url, credentials.as_username().as_ref()) + } else { + self.cache() + .get_realm(Realm::from(request.url()), credentials.to_username()) + }; + if let Some(credentials) = maybe_cached_credentials { request = credentials.authenticate(request); // Do not insert already-cached credentials - None - } else if let Some(credentials) = self + let credentials = None; + return self + .complete_request(credentials, request, extensions, next, auth_policy) + .await; + } + + let credentials = if let Some(credentials) = self .cache() .get_url(request.url(), credentials.as_username().as_ref()) { @@ -403,11 +426,21 @@ impl AuthMiddleware { // Do not insert already-cached credentials None } else if let Some(credentials) = self - .fetch_credentials(Some(&credentials), request.url(), auth_policy) + .fetch_credentials( + Some(&credentials), + request.url(), + maybe_index_url, + auth_policy, + ) .await { request = credentials.authenticate(request); Some(credentials) + } else if maybe_index_url.is_some() { + // If this is a known index, we fall back to checking for the realm. + self.cache() + .get_realm(Realm::from(request.url()), credentials.to_username()) + .or(Some(credentials)) } else { // If we don't find a password, we'll still attempt the request with the existing credentials Some(credentials) @@ -425,18 +458,20 @@ impl AuthMiddleware { &self, credentials: Option<&Credentials>, url: &Url, + maybe_index_url: Option<&Url>, auth_policy: AuthPolicy, ) -> Option> { - // Fetches can be expensive, so we will only run them _once_ per realm and username combination - // All other requests for the same realm will wait until the first one completes - let key = ( - Realm::from(url), - Username::from( - credentials - .map(|credentials| credentials.username().unwrap_or_default().to_string()), - ), + let username = Username::from( + credentials.map(|credentials| credentials.username().unwrap_or_default().to_string()), ); + // Fetches can be expensive, so we will only run them _once_ per realm or index URL and username combination + // All other requests for the same realm or index URL will wait until the first one completes + let key = if let Some(index_url) = maybe_index_url { + (index_url.to_string(), username) + } else { + (Realm::from(url).to_string(), username) + }; if !self.cache().fetches.register(key.clone()) { let credentials = self .cache() @@ -446,9 +481,12 @@ impl AuthMiddleware { .expect("The key must exist after register is called"); if credentials.is_some() { - trace!("Using credentials from previous fetch for {url}"); + trace!("Using credentials from previous fetch for {}", key.0); } else { - trace!("Skipping fetch of credentials for {url}, previous attempt failed"); + trace!( + "Skipping fetch of credentials for {}, previous attempt failed", + key.0 + ); } return credentials; @@ -468,16 +506,17 @@ impl AuthMiddleware { debug!("Found credentials in netrc file for {url}"); Some(credentials) - // N.B. The keyring provider performs lookups for the exact URL then falls back to the host, - // but we cache the result per realm so if a keyring implementation returns different - // credentials for different URLs in the same realm we will use the wrong credentials. + // N.B. The keyring provider performs lookups for the exact URL then falls back to the host. + // But, in the absence of an index URL, we cache the result per realm. So in that case, + // 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) = match self.keyring { Some(ref keyring) => { // The subprocess keyring provider is _slow_ so we do not perform fetches for all // URLs; instead, we fetch if there's a username or if the user has requested to // always authenticate. if let Some(username) = credentials.and_then(|credentials| credentials.username()) { - if let Some(index_url) = self.indexes.index_url_for(url) { + if let Some(index_url) = maybe_index_url { debug!("Checking keyring for credentials for index URL {}@{}", username, index_url); keyring.fetch(index_url, Some(username)).await } else { @@ -485,7 +524,7 @@ impl AuthMiddleware { keyring.fetch(url, Some(username)).await } } else if matches!(auth_policy, AuthPolicy::Always) { - if let Some(index_url) = self.indexes.index_url_for(url) { + if let Some(index_url) = maybe_index_url { debug!( "Checking keyring for credentials for index URL {index_url} without username due to `authenticate = always`" ); @@ -511,7 +550,7 @@ impl AuthMiddleware { .map(Arc::new); // Register the fetch for this key - self.cache().fetches.done(key.clone(), credentials.clone()); + self.cache().fetches.done(key, credentials.clone()); credentials } @@ -1669,11 +1708,210 @@ mod tests { Ok(()) } + /// Demonstrates that when an index URL is provided, we avoid "incorrect" behavior + /// where multiple URLs with the same username and realm share the same realm-level + /// credentials cache entry. + #[test(tokio::test)] + async fn test_credentials_from_keyring_mixed_authentication_different_indexes_same_realm( + ) -> Result<(), Error> { + let username = "user"; + let password_1 = "password1"; + let password_2 = "password2"; + + let server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path_regex("/prefix_1.*")) + .and(basic_auth(username, password_1)) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + Mock::given(method("GET")) + .and(path_regex("/prefix_2.*")) + .and(basic_auth(username, password_2)) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(401)) + .mount(&server) + .await; + + let base_url = Url::parse(&server.uri())?; + let base_url_1 = base_url.join("prefix_1")?; + let base_url_2 = base_url.join("prefix_2")?; + let indexes = Indexes::from_indexes(vec![ + Index { + url: base_url_1.clone(), + root_url: base_url_1.clone(), + auth_policy: AuthPolicy::Auto, + }, + Index { + url: base_url_2.clone(), + root_url: base_url_2.clone(), + auth_policy: AuthPolicy::Auto, + }, + ]); + + let client = test_client_builder() + .with( + AuthMiddleware::new() + .with_cache(CredentialsCache::new()) + .with_keyring(Some(KeyringProvider::dummy([ + (base_url_1.clone(), username, password_1), + (base_url_2.clone(), username, password_2), + ]))) + .with_indexes(indexes), + ) + .build(); + + // Both servers do not work without a username + assert_eq!( + client.get(base_url_1.clone()).send().await?.status(), + 401, + "Requests should require a username" + ); + assert_eq!( + client.get(base_url_2.clone()).send().await?.status(), + 401, + "Requests should require a username" + ); + + let mut url_1 = base_url_1.clone(); + url_1.set_username(username).unwrap(); + assert_eq!( + client.get(url_1.clone()).send().await?.status(), + 200, + "The first request with a username will succeed" + ); + assert_eq!( + client.get(base_url_2.clone()).send().await?.status(), + 401, + "Credentials should not be re-used for the second prefix" + ); + assert_eq!( + client + .get(base_url.join("prefix_1/foo")?) + .send() + .await? + .status(), + 200, + "Subsequent requests can be to different paths in the same prefix" + ); + + let mut url_2 = base_url_2.clone(); + url_2.set_username(username).unwrap(); + assert_eq!( + client.get(url_2.clone()).send().await?.status(), + 200, + "A request with the same username and realm for a URL will use index-specific password" + ); + assert_eq!( + client + .get(base_url.join("prefix_2/foo")?) + .send() + .await? + .status(), + 200, + "Requests to other paths with that prefix will also succeed" + ); + + Ok(()) + } + + /// Demonstrates that when an index' credentials are cached for its realm, we + /// find those credentials if they're not present in the keyring. + #[test(tokio::test)] + async fn test_credentials_from_keyring_shared_authentication_different_indexes_same_realm( + ) -> Result<(), Error> { + let username = "user"; + let password = "password"; + + let server = MockServer::start().await; + + Mock::given(method("GET")) + .and(basic_auth(username, password)) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + Mock::given(method("GET")) + .and(path_regex("/prefix_1.*")) + .and(basic_auth(username, password)) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(401)) + .mount(&server) + .await; + + let base_url = Url::parse(&server.uri())?; + let index_url = base_url.join("prefix_1")?; + let indexes = Indexes::from_indexes(vec![Index { + url: index_url.clone(), + root_url: index_url.clone(), + auth_policy: AuthPolicy::Auto, + }]); + + let client = test_client_builder() + .with( + AuthMiddleware::new() + .with_cache(CredentialsCache::new()) + .with_keyring(Some(KeyringProvider::dummy([( + base_url.clone(), + username, + password, + )]))) + .with_indexes(indexes), + ) + .build(); + + // Index server does not work without a username + assert_eq!( + client.get(index_url.clone()).send().await?.status(), + 401, + "Requests should require a username" + ); + + // Send a request that will cache realm credentials. + let mut realm_url = base_url.clone(); + realm_url.set_username(username).unwrap(); + assert_eq!( + client.get(realm_url.clone()).send().await?.status(), + 200, + "The first realm request with a username will succeed" + ); + + let mut url = index_url.clone(); + url.set_username(username).unwrap(); + assert_eq!( + client.get(url.clone()).send().await?.status(), + 200, + "A request with the same username and realm for a URL will use the realm if there is no index-specific password" + ); + assert_eq!( + client + .get(base_url.join("prefix_1/foo")?) + .send() + .await? + .status(), + 200, + "Requests to other paths with that prefix will also succeed" + ); + + Ok(()) + } + fn indexes_for(url: &Url, policy: AuthPolicy) -> Indexes { let mut url = url.clone(); url.set_password(None).ok(); url.set_username("").ok(); Indexes::from_indexes(vec![Index { + url: url.clone(), root_url: url.clone(), auth_policy: policy, }]) diff --git a/crates/uv-distribution-types/src/index_url.rs b/crates/uv-distribution-types/src/index_url.rs index 606be1c1cd55b..fdb4ea1e5a6b4 100644 --- a/crates/uv-distribution-types/src/index_url.rs +++ b/crates/uv-distribution-types/src/index_url.rs @@ -413,14 +413,15 @@ impl<'a> IndexLocations { impl From<&IndexLocations> for uv_auth::Indexes { fn from(index_locations: &IndexLocations) -> uv_auth::Indexes { uv_auth::Indexes::from_indexes(index_locations.allowed_indexes().into_iter().map(|index| { - let mut url = index - .url() - .root() - .unwrap_or_else(|| index.url().url().clone()); + let mut url = index.url().url().clone(); url.set_username("").ok(); url.set_password(None).ok(); + let mut root_url = index.url().root().unwrap_or_else(|| url.clone()); + root_url.set_username("").ok(); + root_url.set_password(None).ok(); uv_auth::Index { - root_url: url, + url, + root_url, auth_policy: index.authenticate, } })) diff --git a/crates/uv/tests/it/edit.rs b/crates/uv/tests/it/edit.rs index 4f8b6cccd89c0..adf8fa650ab39 100644 --- a/crates/uv/tests/it/edit.rs +++ b/crates/uv/tests/it/edit.rs @@ -3,13 +3,13 @@ #[cfg(feature = "git")] mod conditional_imports { pub(crate) use crate::common::{decode_token, READ_ONLY_GITHUB_TOKEN}; - pub(crate) use assert_cmd::assert::OutputAssertExt; } #[cfg(feature = "git")] use conditional_imports::*; use anyhow::Result; +use assert_cmd::assert::OutputAssertExt; use assert_fs::prelude::*; use indoc::{formatdoc, indoc}; use insta::assert_snapshot; @@ -18,7 +18,7 @@ use uv_fs::Simplified; use uv_static::EnvVars; -use crate::common::{packse_index_url, uv_snapshot, TestContext}; +use crate::common::{packse_index_url, uv_snapshot, venv_bin_path, TestContext}; /// Add a PyPI requirement. #[test] @@ -10206,6 +10206,124 @@ fn add_unsupported_git_scheme() { "###); } +#[test] +fn add_index_url_in_keyring() -> Result<()> { + let keyring_context = TestContext::new("3.12"); + + // Install our keyring plugin + keyring_context + .pip_install() + .arg( + keyring_context + .workspace_root + .join("scripts") + .join("packages") + .join("keyring_test_plugin"), + ) + .assert() + .success(); + + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! { r#" + [project] + name = "foo" + version = "1.0.0" + requires-python = ">=3.11, <4" + dependencies = [] + + [tool.uv] + keyring-provider = "subprocess" + + [[tool.uv.index]] + name = "proxy" + url = "https://pypi-proxy.fly.dev/basic-auth/simple" + default = true + "# + })?; + + uv_snapshot!(context.add().arg("anyio") + .env(EnvVars::index_username("PROXY"), "public") + .env(EnvVars::KEYRING_TEST_CREDENTIALS, r#"{"https://pypi-proxy.fly.dev/basic-auth/simple": {"public": "heron"}}"#) + .env(EnvVars::PATH, venv_bin_path(&keyring_context.venv)), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Keyring request for public@https://pypi-proxy.fly.dev/basic-auth/simple + Resolved 4 packages in [TIME] + Prepared 3 packages in [TIME] + Installed 3 packages in [TIME] + + anyio==4.3.0 + + idna==3.6 + + sniffio==1.3.1 + " + ); + + context.assert_command("import anyio").success(); + Ok(()) +} + +#[test] +fn add_full_url_in_keyring() -> Result<()> { + let keyring_context = TestContext::new("3.12"); + + // Install our keyring plugin + keyring_context + .pip_install() + .arg( + keyring_context + .workspace_root + .join("scripts") + .join("packages") + .join("keyring_test_plugin"), + ) + .assert() + .success(); + + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! { r#" + [project] + name = "foo" + version = "1.0.0" + requires-python = ">=3.11, <4" + dependencies = [] + + [tool.uv] + keyring-provider = "subprocess" + + [[tool.uv.index]] + name = "proxy" + url = "https://pypi-proxy.fly.dev/basic-auth/simple" + default = true + "# + })?; + + uv_snapshot!(context.add().arg("anyio") + .env(EnvVars::index_username("PROXY"), "public") + .env(EnvVars::KEYRING_TEST_CREDENTIALS, r#"{"https://pypi-proxy.fly.dev/basic-auth/simple/anyio": {"public": "heron"}}"#) + .env(EnvVars::PATH, venv_bin_path(&keyring_context.venv)), @r" + success: false + exit_code: 1 + ----- stdout ----- + + ----- stderr ----- + Keyring request for public@https://pypi-proxy.fly.dev/basic-auth/simple + Keyring request for public@pypi-proxy.fly.dev + × No solution found when resolving dependencies: + ╰─▶ Because anyio was not found in the package registry and your project depends on anyio, we can conclude that your project's requirements are unsatisfiable. + + hint: An index URL (https://pypi-proxy.fly.dev/basic-auth/simple) could not be queried due to a lack of valid authentication credentials (401 Unauthorized). + help: If you want to add the package regardless of the failed resolution, provide the `--frozen` flag to skip locking and syncing. + " + ); + Ok(()) +} + /// In authentication "always", the normal authentication flow should still work. #[test] fn add_auth_policy_always_with_credentials() -> Result<()> { diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index f80f516564489..cd93cfb806d9a 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -18808,7 +18808,7 @@ fn lock_keyring_credentials() -> Result<()> { ----- stdout ----- ----- stderr ----- - Keyring request for public@https://pypi-proxy.fly.dev/basic-auth + Keyring request for public@https://pypi-proxy.fly.dev/basic-auth/simple Keyring request for public@pypi-proxy.fly.dev Resolved 2 packages in [TIME] "); @@ -18913,7 +18913,7 @@ fn lock_keyring_explicit_always() -> Result<()> { ----- stdout ----- ----- stderr ----- - Keyring request for https://pypi-proxy.fly.dev/basic-auth + Keyring request for https://pypi-proxy.fly.dev/basic-auth/simple Keyring request for pypi-proxy.fly.dev × No solution found when resolving dependencies: ╰─▶ Because iniconfig was not found in the package registry and your project depends on iniconfig, we can conclude that your project's requirements are unsatisfiable. @@ -18930,7 +18930,7 @@ fn lock_keyring_explicit_always() -> Result<()> { ----- stdout ----- ----- stderr ----- - Keyring request for https://pypi-proxy.fly.dev/basic-auth + Keyring request for https://pypi-proxy.fly.dev/basic-auth/simple Keyring request for pypi-proxy.fly.dev Resolved 2 packages in [TIME] "); @@ -18996,7 +18996,7 @@ fn lock_keyring_credentials_always_authenticate_fetches_username() -> Result<()> ----- stdout ----- ----- stderr ----- - Keyring request for https://pypi-proxy.fly.dev/basic-auth + Keyring request for https://pypi-proxy.fly.dev/basic-auth/simple Keyring request for pypi-proxy.fly.dev Resolved 2 packages in [TIME] "); diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index aa11a613dbb7f..faffc2071a988 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -5341,7 +5341,7 @@ fn install_package_basic_auth_from_keyring() { ----- stdout ----- ----- stderr ----- - Keyring request for public@https://pypi-proxy.fly.dev/basic-auth + Keyring request for public@https://pypi-proxy.fly.dev/basic-auth/simple Keyring request for public@pypi-proxy.fly.dev Resolved 3 packages in [TIME] Prepared 3 packages in [TIME] @@ -5388,7 +5388,7 @@ fn install_package_basic_auth_from_keyring_wrong_password() { ----- stdout ----- ----- stderr ----- - Keyring request for public@https://pypi-proxy.fly.dev/basic-auth + Keyring request for public@https://pypi-proxy.fly.dev/basic-auth/simple Keyring request for public@pypi-proxy.fly.dev × No solution found when resolving dependencies: ╰─▶ Because anyio was not found in the package registry and you require anyio, we can conclude that your requirements are unsatisfiable. @@ -5431,7 +5431,7 @@ fn install_package_basic_auth_from_keyring_wrong_username() { ----- stdout ----- ----- stderr ----- - Keyring request for public@https://pypi-proxy.fly.dev/basic-auth + Keyring request for public@https://pypi-proxy.fly.dev/basic-auth/simple Keyring request for public@pypi-proxy.fly.dev × No solution found when resolving dependencies: ╰─▶ Because anyio was not found in the package registry and you require anyio, we can conclude that your requirements are unsatisfiable. diff --git a/docs/configuration/authentication.md b/docs/configuration/authentication.md index 2425fc2623519..a05a637bac6f2 100644 --- a/docs/configuration/authentication.md +++ b/docs/configuration/authentication.md @@ -38,9 +38,9 @@ Authentication can come from the following sources, in order of precedence: - A [`.netrc`](https://everything.curl.dev/usingcurl/netrc) configuration file - A [keyring](https://github.com/jaraco/keyring) provider (requires opt-in) -If authentication is found for a single net location (scheme, host, and port), it will be cached for -the duration of the command and used for other queries to that net location. Authentication is not -cached across invocations of uv. +If authentication is found for a single index URL or net location (scheme, host, and port), it will +be cached for the duration of the command and used for other queries to that index or net location. +Authentication is not cached across invocations of uv. `.netrc` authentication is enabled by default, and will respect the `NETRC` environment variable if defined, falling back to `~/.netrc` if not.