From f1aec3e779d5d5c7c78c5bc58b7293e2e543cbd1 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 13 Mar 2024 20:46:33 -0700 Subject: [PATCH] Add in-URL credentials to store prior to creating requests (#2446) ## Summary The authentication middleware extracts in-URL credentials from URLs that pass through it; however, by the time a request reaches the store, the credentials will have already been removed, and relocated to the header. So we were never propagating in-URL credentials. This PR adds an explicit pass wherein we pass in-URL credentials to the store prior to doing any work. Closes https://github.com/astral-sh/uv/issues/2444. ## Test Plan `cargo run pip install` against an authenticated AWS registry. --- crates/distribution-types/src/file.rs | 2 ++ crates/distribution-types/src/index_url.rs | 20 ++++++++++++++++++++ crates/pypi-types/src/base_url.rs | 15 --------------- crates/uv-auth/src/middleware.rs | 9 ++++++--- crates/uv-auth/src/store.rs | 1 + crates/uv/src/commands/pip_compile.rs | 7 ++++++- crates/uv/src/commands/pip_install.rs | 7 ++++++- crates/uv/src/commands/pip_sync.rs | 7 ++++++- crates/uv/src/commands/venv.rs | 7 ++++++- 9 files changed, 53 insertions(+), 22 deletions(-) diff --git a/crates/distribution-types/src/file.rs b/crates/distribution-types/src/file.rs index 5c7268b66..71c1ebdc5 100644 --- a/crates/distribution-types/src/file.rs +++ b/crates/distribution-types/src/file.rs @@ -53,11 +53,13 @@ impl File { size: file.size, upload_time_utc_ms: file.upload_time.map(|dt| dt.timestamp_millis()), url: if file.url.contains("://") { + // Copy over any credentials from the global store. let url = Url::parse(&file.url) .map_err(|err| FileConversionError::Url(file.url.clone(), err))?; let url = GLOBAL_AUTH_STORE.with_url_encoded_auth(url); FileLocation::AbsoluteUrl(url.to_string()) } else { + // It's assumed that the base URL already contains any necessary credentials. FileLocation::RelativeUrl(base.to_string(), file.url) }, yanked: file.yanked, diff --git a/crates/distribution-types/src/index_url.rs b/crates/distribution-types/src/index_url.rs index 4d6d006d9..86b338ecf 100644 --- a/crates/distribution-types/src/index_url.rs +++ b/crates/distribution-types/src/index_url.rs @@ -26,6 +26,16 @@ pub enum IndexUrl { Url(VerbatimUrl), } +impl IndexUrl { + /// Return the raw URL for the index. + pub(crate) fn url(&self) -> &Url { + match self { + Self::Pypi(url) => url.raw(), + Self::Url(url) => url.raw(), + } + } +} + impl Display for IndexUrl { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { @@ -268,6 +278,16 @@ impl<'a> IndexLocations { no_index: self.no_index, } } + + /// Return an iterator over all [`Url`] entries. + pub fn urls(&'a self) -> impl Iterator + 'a { + self.indexes() + .map(IndexUrl::url) + .chain(self.flat_index.iter().filter_map(|index| match index { + FlatIndexLocation::Path(_) => None, + FlatIndexLocation::Url(url) => Some(url), + })) + } } /// The index URLs to use for fetching packages. diff --git a/crates/pypi-types/src/base_url.rs b/crates/pypi-types/src/base_url.rs index 0afef4ebf..d4d991cdb 100644 --- a/crates/pypi-types/src/base_url.rs +++ b/crates/pypi-types/src/base_url.rs @@ -57,21 +57,6 @@ pub struct BaseUrl( ); impl BaseUrl { - /// Parse the given URL. If it's relative, join it to the current [`BaseUrl`]. Allows for - /// parsing URLs that may be absolute or relative, with a known base URL. - pub fn join_relative(&self, url: &str) -> Result { - match Url::parse(url) { - Ok(url) => Ok(url), - Err(err) => { - if err == url::ParseError::RelativeUrlWithoutBase { - self.0.join(url) - } else { - Err(err) - } - } - } - } - /// Return the underlying [`Url`]. pub fn as_url(&self) -> &Url { &self.0 diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index c1c03d1e3..b4743f5f7 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -44,12 +44,11 @@ impl Middleware for AuthMiddleware { next: Next<'_>, ) -> reqwest_middleware::Result { let url = req.url().clone(); + // If the request already has an authorization header, we don't need to do anything. // This gives in-URL credentials precedence over the netrc file. if req.headers().contains_key(reqwest::header::AUTHORIZATION) { - if !url.username().is_empty() { - GLOBAL_AUTH_STORE.save_from_url(&url); - } + debug!("Request already has an authorization header: {url}"); return next.run(req, _extensions).await; } @@ -57,6 +56,7 @@ impl Middleware for AuthMiddleware { if let Some(stored_auth) = GLOBAL_AUTH_STORE.get(&url) { // If we've already seen this URL, we can use the stored credentials if let Some(auth) = stored_auth { + debug!("Adding authentication to already-seen URL: {url}"); match auth { Credential::Basic(_) => { req.headers_mut().insert( @@ -67,6 +67,8 @@ impl Middleware for AuthMiddleware { // Url must already have auth if before middleware runs - see `AuthenticationStore::with_url_encoded_auth` Credential::UrlEncoded(_) => (), } + } else { + debug!("No credentials found for already-seen URL: {url}"); } } else if let Some(auth) = self.nrc.as_ref().and_then(|nrc| { // If we find a matching entry in the netrc file, we can use it @@ -100,6 +102,7 @@ impl Middleware for AuthMiddleware { // If we still don't have any credentials, we save the URL so we don't have to check netrc or keyring again if !req.headers().contains_key(reqwest::header::AUTHORIZATION) { + debug!("No credentials found for: {url}"); GLOBAL_AUTH_STORE.set(&url, None); } diff --git a/crates/uv-auth/src/store.rs b/crates/uv-auth/src/store.rs index 8da202888..efe6494fb 100644 --- a/crates/uv-auth/src/store.rs +++ b/crates/uv-auth/src/store.rs @@ -102,6 +102,7 @@ impl AuthenticationStore { } } + /// Store in-URL credentials for future use. pub fn save_from_url(&self, url: &Url) { let netloc = NetLoc::from(url); let mut credentials = self.credentials.lock().unwrap(); diff --git a/crates/uv/src/commands/pip_compile.rs b/crates/uv/src/commands/pip_compile.rs index ae63f21f8..d30e2b2bc 100644 --- a/crates/uv/src/commands/pip_compile.rs +++ b/crates/uv/src/commands/pip_compile.rs @@ -18,7 +18,7 @@ use tracing::debug; use distribution_types::{IndexLocations, LocalEditable, Verbatim}; use platform_tags::Tags; use requirements_txt::EditableRequirement; -use uv_auth::KeyringProvider; +use uv_auth::{KeyringProvider, GLOBAL_AUTH_STORE}; use uv_cache::Cache; use uv_client::{Connectivity, FlatIndex, FlatIndexClient, RegistryClientBuilder}; use uv_dispatch::BuildDispatch; @@ -187,6 +187,11 @@ pub(crate) async fn pip_compile( let index_locations = index_locations.combine(index_url, extra_index_urls, find_links, no_index); + // Add all authenticated sources to the store. + for url in index_locations.urls() { + GLOBAL_AUTH_STORE.save_from_url(url); + } + // Initialize the registry client. let client = RegistryClientBuilder::new(cache.clone()) .native_tls(native_tls) diff --git a/crates/uv/src/commands/pip_install.rs b/crates/uv/src/commands/pip_install.rs index 44e932fda..0e7ed8600 100644 --- a/crates/uv/src/commands/pip_install.rs +++ b/crates/uv/src/commands/pip_install.rs @@ -20,7 +20,7 @@ use pep508_rs::{MarkerEnvironment, Requirement}; use platform_tags::Tags; use pypi_types::Yanked; use requirements_txt::EditableRequirement; -use uv_auth::KeyringProvider; +use uv_auth::{KeyringProvider, GLOBAL_AUTH_STORE}; use uv_cache::Cache; use uv_client::{Connectivity, FlatIndex, FlatIndexClient, RegistryClient, RegistryClientBuilder}; use uv_dispatch::BuildDispatch; @@ -181,6 +181,11 @@ pub(crate) async fn pip_install( let index_locations = index_locations.combine(index_url, extra_index_urls, find_links, no_index); + // Add all authenticated sources to the store. + for url in index_locations.urls() { + GLOBAL_AUTH_STORE.save_from_url(url); + } + // Initialize the registry client. let client = RegistryClientBuilder::new(cache.clone()) .native_tls(native_tls) diff --git a/crates/uv/src/commands/pip_sync.rs b/crates/uv/src/commands/pip_sync.rs index f4d610b10..6a4b5d2be 100644 --- a/crates/uv/src/commands/pip_sync.rs +++ b/crates/uv/src/commands/pip_sync.rs @@ -10,7 +10,7 @@ use install_wheel_rs::linker::LinkMode; use platform_tags::Tags; use pypi_types::Yanked; use requirements_txt::EditableRequirement; -use uv_auth::KeyringProvider; +use uv_auth::{KeyringProvider, GLOBAL_AUTH_STORE}; use uv_cache::{ArchiveTarget, ArchiveTimestamp, Cache}; use uv_client::{Connectivity, FlatIndex, FlatIndexClient, RegistryClient, RegistryClientBuilder}; use uv_dispatch::BuildDispatch; @@ -115,6 +115,11 @@ pub(crate) async fn pip_sync( let index_locations = index_locations.combine(index_url, extra_index_urls, find_links, no_index); + // Add all authenticated sources to the store. + for url in index_locations.urls() { + GLOBAL_AUTH_STORE.save_from_url(url); + } + // Initialize the registry client. let client = RegistryClientBuilder::new(cache.clone()) .native_tls(native_tls) diff --git a/crates/uv/src/commands/venv.rs b/crates/uv/src/commands/venv.rs index 9b72d6d8f..caa3e63d8 100644 --- a/crates/uv/src/commands/venv.rs +++ b/crates/uv/src/commands/venv.rs @@ -13,7 +13,7 @@ use thiserror::Error; use distribution_types::{DistributionMetadata, IndexLocations, Name}; use pep508_rs::Requirement; -use uv_auth::KeyringProvider; +use uv_auth::{KeyringProvider, GLOBAL_AUTH_STORE}; use uv_cache::Cache; use uv_client::{Connectivity, FlatIndex, FlatIndexClient, RegistryClientBuilder}; use uv_dispatch::BuildDispatch; @@ -140,6 +140,11 @@ async fn venv_impl( // Extract the interpreter. let interpreter = venv.interpreter(); + // Add all authenticated sources to the store. + for url in index_locations.urls() { + GLOBAL_AUTH_STORE.save_from_url(url); + } + // Instantiate a client. let client = RegistryClientBuilder::new(cache.clone()) .native_tls(native_tls)