Skip to content

Commit

Permalink
Add in-URL credentials to store prior to creating requests (#2446)
Browse files Browse the repository at this point in the history
## 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 #2444.

## Test Plan

`cargo run pip install` against an authenticated AWS registry.
  • Loading branch information
charliermarsh authored Mar 14, 2024
1 parent d29645c commit f1aec3e
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 22 deletions.
2 changes: 2 additions & 0 deletions crates/distribution-types/src/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions crates/distribution-types/src/index_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<Item = &'a Url> + '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.
Expand Down
15 changes: 0 additions & 15 deletions crates/pypi-types/src/base_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Url, url::ParseError> {
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
Expand Down
9 changes: 6 additions & 3 deletions crates/uv-auth/src/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,19 @@ impl Middleware for AuthMiddleware {
next: Next<'_>,
) -> reqwest_middleware::Result<Response> {
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;
}

// Try auth strategies in order of precedence:
if let Some(stored_auth) = GLOBAL_AUTH_STORE.get(&url) {
// If we've already seen this URL, we can use the stored credentials
if let Some(auth) = stored_auth {
debug!("Adding authentication to already-seen URL: {url}");
match auth {
Credential::Basic(_) => {
req.headers_mut().insert(
Expand All @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down
1 change: 1 addition & 0 deletions crates/uv-auth/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 6 additions & 1 deletion crates/uv/src/commands/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion crates/uv/src/commands/pip_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion crates/uv/src/commands/pip_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion crates/uv/src/commands/venv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit f1aec3e

Please sign in to comment.