From f98eca8843092a2c25ab19242f1d587d64867c6f Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Mon, 22 Apr 2024 13:06:57 -0500 Subject: [PATCH] Fix authentication for URLs with a shared realm (#3130) In #2976 I made some changes that led to regressions: - We stopped tracking URLs that we had not seen credentials for in the cache - This means the cache no longer returns a value to indicate we've seen a realm before - We stopped seeding the cache with URLs - Combined with the above, this means we no longer had a list of locations that we would never attempt to fetch credentials for - We added caching of credentials found on requests - Previously the cache was only populated from the seed or credentials found in the netrc or keyring - This meant that the cache was populated for locations that we previously did not cache, i.e. GitHub artifacts(?) Unfortunately this unveiled problems with the granularity of our cache. We cache credentials per realm (roughly the hostname) but some realms have mixed authentication modes i.e. different credentials per URL or URLs that do not require credentials. Applying credentials to a URL that does not require it can lead to a failed request, as seen in #3123 where GitHub throws a 401 when receiving credentials. To resolve this, the cache is expanded to supporting caching at two levels: - URL, cached URL must be a prefix of the request URL - Realm, exact match required When we don't have URL-level credentials cached, we attempt the request without authentication first. On failure, we'll search for realm-level credentials or fetch credentials from external services. This avoids providing credentials to new URLs unless we know we need them. Closes https://github.com/astral-sh/uv/issues/3123 --- Cargo.lock | 1 + crates/uv-auth/Cargo.toml | 1 + crates/uv-auth/src/cache.rs | 330 +++++---- crates/uv-auth/src/credentials.rs | 73 +- crates/uv-auth/src/lib.rs | 6 +- crates/uv-auth/src/middleware.rs | 761 ++++++++++++++++++--- crates/uv-auth/src/{netloc.rs => realm.rs} | 80 ++- crates/uv-client/tests/netrc_auth.rs | 73 -- crates/uv/tests/pip_install.rs | 162 ++++- 9 files changed, 1171 insertions(+), 316 deletions(-) rename crates/uv-auth/src/{netloc.rs => realm.rs} (51%) delete mode 100644 crates/uv-client/tests/netrc_auth.rs diff --git a/Cargo.lock b/Cargo.lock index a984477db362..3145fefccd64 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4466,6 +4466,7 @@ dependencies = [ name = "uv-auth" version = "0.0.1" dependencies = [ + "anyhow", "async-trait", "base64 0.22.0", "http", diff --git a/crates/uv-auth/Cargo.toml b/crates/uv-auth/Cargo.toml index e23fe4610bf7..7ff0a4cbdda7 100644 --- a/crates/uv-auth/Cargo.toml +++ b/crates/uv-auth/Cargo.toml @@ -4,6 +4,7 @@ version = "0.0.1" edition = "2021" [dependencies] +anyhow = { workspace = true } async-trait = { workspace = true } base64 = { workspace = true } http = { workspace = true } diff --git a/crates/uv-auth/src/cache.rs b/crates/uv-auth/src/cache.rs index 273eb2f64c85..a2081ff7db39 100644 --- a/crates/uv-auth/src/cache.rs +++ b/crates/uv-auth/src/cache.rs @@ -1,43 +1,17 @@ use std::sync::Arc; use std::{collections::HashMap, sync::Mutex}; -use crate::credentials::Credentials; -use crate::NetLoc; +use crate::credentials::{Credentials, Username}; +use crate::Realm; use tracing::trace; use url::Url; -type CacheKey = (NetLoc, Option); - pub struct CredentialsCache { - store: Mutex>>, -} - -#[derive(Debug, Clone)] -pub enum CheckResponse { - /// The given credentials should be used and are not present in the cache. - Uncached(Arc), - /// Credentials were found in the cache. - Cached(Arc), - // Credentials were not found in the cache and none were provided. - None, -} - -impl CheckResponse { - /// Retrieve the credentials, if any. - pub fn get(&self) -> Option<&Credentials> { - match self { - Self::Cached(credentials) => Some(credentials.as_ref()), - Self::Uncached(credentials) => Some(credentials.as_ref()), - Self::None => None, - } - } - - /// Returns true if there are credentials with a password. - pub fn is_authenticated(&self) -> bool { - self.get() - .is_some_and(|credentials| credentials.password().is_some()) - } + /// A cache per realm and username + realms: Mutex>>, + /// A cache per URL, uses a trie for efficient prefix queries. + urls: Mutex, } impl Default for CredentialsCache { @@ -50,84 +24,48 @@ impl CredentialsCache { /// Create a new cache. pub fn new() -> Self { Self { - store: Mutex::new(HashMap::new()), + realms: Mutex::new(HashMap::new()), + urls: Mutex::new(UrlTrie::new()), } } - /// Create an owned cache key. - fn key(url: &Url, username: Option) -> CacheKey { - (NetLoc::from(url), username) + /// Return the credentials that should be used for a realm and username, if any. + pub(crate) fn get_realm(&self, realm: Realm, username: Username) -> Option> { + let realms = self.realms.lock().unwrap(); + let name = if let Some(username) = username.as_deref() { + format!("{username}@{realm}") + } else { + realm.to_string() + }; + let key = (realm, username); + + realms + .get(&key) + .cloned() + .map(Some) + .inspect(|_| trace!("Found cached credentials for realm {name}")) + .unwrap_or_else(|| { + trace!("No credentials in cache for realm {name}"); + None + }) } - /// Return the credentials that should be used for a URL, if any. - /// - /// The [`Url`] is not checked for credentials. Existing credentials should be extracted and passed - /// separately. + /// Return the cached credentials for a URL and username, if any. /// - /// If complete credentials are provided, they will be returned as [`CheckResponse::Existing`] - /// If the credentials are partial, i.e. missing a password, the cache will be checked - /// for a corresponding entry. - pub(crate) fn check(&self, url: &Url, credentials: Option) -> CheckResponse { - let store = self.store.lock().unwrap(); - - let credentials = credentials.map(Arc::new); - let key = CredentialsCache::key( - url, - credentials - .as_ref() - .and_then(|credentials| credentials.username().map(str::to_string)), - ); - + /// Note we do not cache per username, but if a username is passed we will confirm that the + /// cached credentials have a username equal to the provided one — otherwise `None` is returned. + /// If multiple usernames are used per URL, the realm cache should be queried instead. + pub(crate) fn get_url(&self, url: &Url, username: Username) -> Option> { + let urls = self.urls.lock().unwrap(); + let credentials = urls.get(url); if let Some(credentials) = credentials { - if credentials.password().is_some() { - trace!("Existing credentials include password, skipping cache"); - // No need to look-up, we have a password already - return CheckResponse::Uncached(credentials); - } - trace!("Existing credentials missing password, checking cache"); - let existing = store.get(&key); - existing - .cloned() - .map(CheckResponse::Cached) - .inspect(|_| trace!("Found cached credentials.")) - .unwrap_or_else(|| { - trace!("No credentials in cache, using existing credentials"); - CheckResponse::Uncached(credentials) - }) - } else { - trace!("No credentials on request, checking cache..."); - store - .get(&key) - .cloned() - .map(CheckResponse::Cached) - .inspect(|_| trace!("Found cached credentials.")) - .unwrap_or_else(|| { - trace!("No credentials in cache."); - CheckResponse::None - }) - } - } - - /// Update the cache with the given credentials if none exist. - pub(crate) fn set_default(&self, url: &Url, credentials: Arc) { - // Do not cache empty credentials - if credentials.is_empty() { - return; - } - - // Insert an entry for requests including the username - if let Some(username) = credentials.username() { - let key = CredentialsCache::key(url, Some(username.to_string())); - if !self.contains_key(&key) { - self.insert_entry(key, credentials.clone()); + if username.is_none() || username.as_deref() == credentials.username() { + trace!("Found cached credentials for URL {url}"); + return Some(credentials.clone()); } } - - // Insert an entry for requests with no username - let key = CredentialsCache::key(url, None); - if !self.contains_key(&key) { - self.insert_entry(key, credentials.clone()); - } + trace!("No credentials in URL cache for {url}"); + None } /// Update the cache with the given credentials. @@ -138,47 +76,197 @@ impl CredentialsCache { } // Insert an entry for requests including the username - if let Some(username) = credentials.username() { - self.insert_entry( - CredentialsCache::key(url, Some(username.to_string())), - credentials.clone(), - ); + let username = credentials.to_username(); + if username.is_some() { + let realm = (Realm::from(url), username.clone()); + self.insert_realm(realm, credentials.clone()); } // Insert an entry for requests with no username - self.insert_entry(CredentialsCache::key(url, None), credentials.clone()); + self.insert_realm((Realm::from(url), Username::none()), credentials.clone()); + + // Insert an entry for the URL + let mut urls = self.urls.lock().unwrap(); + urls.insert(url.clone(), credentials.clone()); } - /// Private interface to update a cache entry. - fn insert_entry(&self, key: (NetLoc, Option), credentials: Arc) -> bool { + /// Private interface to update a realm cache entry. + /// + /// Returns replaced credentials, if any. + fn insert_realm( + &self, + key: (Realm, Username), + credentials: Arc, + ) -> Option> { // Do not cache empty credentials if credentials.is_empty() { - return false; + return None; } - let mut store = self.store.lock().unwrap(); + let mut realms = self.realms.lock().unwrap(); // Always replace existing entries if we have a password if credentials.password().is_some() { - store.insert(key, credentials.clone()); - return true; + return realms.insert(key, credentials.clone()); } // If we only have a username, add a new entry or replace an existing entry if it doesn't have a password - let existing = store.get(&key); + let existing = realms.get(&key); if existing.is_none() || existing.is_some_and(|credentials| credentials.password().is_none()) { - store.insert(key, credentials.clone()); - return true; + return realms.insert(key, credentials.clone()); + } + + None + } +} + +#[derive(Debug)] +struct UrlTrie { + states: Vec, +} + +#[derive(Debug, Default)] +struct TrieState { + children: Vec<(String, usize)>, + value: Option>, +} + +impl UrlTrie { + fn new() -> UrlTrie { + let mut trie = UrlTrie { states: vec![] }; + trie.alloc(); + trie + } + + fn get(&self, url: &Url) -> Option<&Arc> { + let mut state = 0; + let realm = Realm::from(url).to_string(); + for component in [realm.as_str()] + .into_iter() + .chain(url.path_segments().unwrap().filter(|item| !item.is_empty())) + { + state = self.states[state].get(component)?; + if let Some(ref value) = self.states[state].value { + return Some(value); + } + } + self.states[state].value.as_ref() + } + + fn insert(&mut self, url: Url, value: Arc) { + let mut state = 0; + let realm = Realm::from(&url).to_string(); + for component in [realm.as_str()] + .into_iter() + .chain(url.path_segments().unwrap().filter(|item| !item.is_empty())) + { + match self.states[state].index(component) { + Ok(i) => state = self.states[state].children[i].1, + Err(i) => { + let new_state = self.alloc(); + self.states[state] + .children + .insert(i, (component.to_string(), new_state)); + state = new_state; + } + } } + self.states[state].value = Some(value); + } + + fn alloc(&mut self) -> usize { + let id = self.states.len(); + self.states.push(TrieState::default()); + id + } +} + +impl TrieState { + fn get(&self, component: &str) -> Option { + let i = self.index(component).ok()?; + Some(self.children[i].1) + } - false + fn index(&self, component: &str) -> Result { + self.children + .binary_search_by(|(label, _)| label.as_str().cmp(component)) } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_trie() { + let credentials1 = Arc::new(Credentials::new( + Some("username1".to_string()), + Some("password1".to_string()), + )); + let credentials2 = Arc::new(Credentials::new( + Some("username2".to_string()), + Some("password2".to_string()), + )); + let credentials3 = Arc::new(Credentials::new( + Some("username3".to_string()), + Some("password3".to_string()), + )); + let credentials4 = Arc::new(Credentials::new( + Some("username4".to_string()), + Some("password4".to_string()), + )); + + let mut trie = UrlTrie::new(); + trie.insert( + Url::parse("https://burntsushi.net").unwrap(), + credentials1.clone(), + ); + trie.insert( + Url::parse("https://astral.sh").unwrap(), + credentials2.clone(), + ); + trie.insert( + Url::parse("https://example.com/foo").unwrap(), + credentials3.clone(), + ); + trie.insert( + Url::parse("https://example.com/bar").unwrap(), + credentials4.clone(), + ); + + let url = Url::parse("https://burntsushi.net/regex-internals").unwrap(); + assert_eq!(trie.get(&url), Some(&credentials1)); + + let url = Url::parse("https://burntsushi.net/").unwrap(); + assert_eq!(trie.get(&url), Some(&credentials1)); + + let url = Url::parse("https://astral.sh/about").unwrap(); + assert_eq!(trie.get(&url), Some(&credentials2)); + + let url = Url::parse("https://example.com/foo").unwrap(); + assert_eq!(trie.get(&url), Some(&credentials3)); + + let url = Url::parse("https://example.com/foo/").unwrap(); + assert_eq!(trie.get(&url), Some(&credentials3)); + + let url = Url::parse("https://example.com/foo/bar").unwrap(); + assert_eq!(trie.get(&url), Some(&credentials3)); + + let url = Url::parse("https://example.com/bar").unwrap(); + assert_eq!(trie.get(&url), Some(&credentials4)); + + let url = Url::parse("https://example.com/bar/").unwrap(); + assert_eq!(trie.get(&url), Some(&credentials4)); + + let url = Url::parse("https://example.com/bar/foo").unwrap(); + assert_eq!(trie.get(&url), Some(&credentials4)); + + let url = Url::parse("https://example.com/about").unwrap(); + assert_eq!(trie.get(&url), None); - /// Returns true if a key is in the cache. - fn contains_key(&self, key: &(NetLoc, Option)) -> bool { - let store = self.store.lock().unwrap(); - store.contains_key(key) + let url = Url::parse("https://example.com/foobar").unwrap(); + assert_eq!(trie.get(&url), None); } } diff --git a/crates/uv-auth/src/credentials.rs b/crates/uv-auth/src/credentials.rs index 5bceac793019..c8826bdb1cd7 100644 --- a/crates/uv-auth/src/credentials.rs +++ b/crates/uv-auth/src/credentials.rs @@ -12,28 +12,76 @@ use url::Url; #[derive(Clone, Debug, PartialEq)] pub(crate) struct Credentials { /// The name of the user for authentication. - /// - /// Unlike `reqwest`, empty usernames should be encoded as `None` instead of an empty string. - username: Option, + username: Username, /// The password to use for authentication. password: Option, } +#[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)] +pub(crate) struct Username(Option); + +impl Username { + /// Create a new username. + /// + /// Unlike `reqwest`, empty usernames are be encoded as `None` instead of an empty string. + pub fn new(value: Option) -> Self { + // Ensure empty strings are `None` + if let Some(value) = value { + if value.is_empty() { + Self(None) + } else { + Self(Some(value)) + } + } else { + Self(value) + } + } + + pub fn none() -> Self { + Self::new(None) + } + + pub fn is_none(&self) -> bool { + self.0.is_none() + } + + pub fn is_some(&self) -> bool { + self.0.is_some() + } + + pub fn as_deref(&self) -> Option<&str> { + self.0.as_deref() + } +} + +impl From for Username { + fn from(value: String) -> Self { + Self::new(Some(value)) + } +} + +impl From> for Username { + fn from(value: Option) -> Self { + Self::new(value) + } +} + impl Credentials { pub fn new(username: Option, password: Option) -> Self { - debug_assert!( - username.is_none() - || username - .as_ref() - .is_some_and(|username| !username.is_empty()) - ); - Self { username, password } + Self { + username: Username::new(username), + password, + } } pub fn username(&self) -> Option<&str> { self.username.as_deref() } + pub fn to_username(&self) -> Username { + self.username.clone() + } + pub fn password(&self) -> Option<&str> { self.password.as_deref() } @@ -58,7 +106,7 @@ impl Credentials { }; Some(Credentials { - username: Some(entry.login.clone()), + username: Username::new(Some(entry.login.clone())), password: Some(entry.password.clone()), }) } @@ -81,7 +129,8 @@ impl Credentials { .expect("An encoded username should always decode") .into_owned(), ) - }, + } + .into(), password: url.password().map(|password| { urlencoding::decode(password) .expect("An encoded password should always decode") diff --git a/crates/uv-auth/src/lib.rs b/crates/uv-auth/src/lib.rs index ce564074d466..6d6ea2bd40d9 100644 --- a/crates/uv-auth/src/lib.rs +++ b/crates/uv-auth/src/lib.rs @@ -2,7 +2,7 @@ mod cache; mod credentials; mod keyring; mod middleware; -mod netloc; +mod realm; use std::sync::Arc; @@ -11,8 +11,9 @@ use credentials::Credentials; pub use keyring::KeyringProvider; pub use middleware::AuthMiddleware; -use netloc::NetLoc; use once_cell::sync::Lazy; +use realm::Realm; +use tracing::trace; use url::Url; // TODO(zanieb): Consider passing a cache explicitly throughout @@ -27,6 +28,7 @@ pub(crate) static CREDENTIALS_CACHE: Lazy = Lazy::new(Credenti /// Returns `true` if the store was updated. pub fn store_credentials_from_url(url: &Url) -> bool { if let Some(credentials) = Credentials::from_url(url) { + trace!("Caching credentials for {url}"); CREDENTIALS_CACHE.insert(url, Arc::new(credentials)); true } else { diff --git a/crates/uv-auth/src/middleware.rs b/crates/uv-auth/src/middleware.rs index f028d7dc9f78..ae003429be0c 100644 --- a/crates/uv-auth/src/middleware.rs +++ b/crates/uv-auth/src/middleware.rs @@ -1,20 +1,23 @@ use std::sync::Arc; -use http::Extensions; +use http::{Extensions, StatusCode}; +use url::Url; +use crate::{ + credentials::{Credentials, Username}, + realm::Realm, + CredentialsCache, KeyringProvider, CREDENTIALS_CACHE, +}; +use anyhow::anyhow; use netrc::Netrc; use reqwest::{Request, Response}; -use reqwest_middleware::{Middleware, Next}; +use reqwest_middleware::{Error, Middleware, Next}; use tracing::{debug, trace}; -use crate::{ - cache::CheckResponse, credentials::Credentials, CredentialsCache, KeyringProvider, - CREDENTIALS_CACHE, -}; - -/// A middleware that adds basic authentication to requests based on the netrc file and the keyring. +/// A middleware that adds basic authentication to requests. /// -/// Netrc support Based on: . +/// Uses a cache to propagate credentials from previously seen requests and +/// fetches credentials from a netrc file and the keyring. pub struct AuthMiddleware { netrc: Option, keyring: Option, @@ -69,14 +72,51 @@ impl Default for AuthMiddleware { #[async_trait::async_trait] impl Middleware for AuthMiddleware { + /// Handle authentication for a request. + /// + /// ## If the request has a username and password + /// + /// We already have a fully authenticated request and we don't need to perform a look-up. + /// + /// - Perform the request + /// - Add the username and password to the cache if successful + /// + /// ## If the request only has a username + /// + /// We probably need additional authentication, because a username is provided. + /// 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 netrc for a password + /// - Check the keyring for a password + /// - Perform the request + /// - Add the username and password to the cache if successful + /// + /// ## If the request has no authentication + /// + /// We may or may not need authentication. We'll check for cached credentials for the URL, + /// which is relatively specific and can save us an expensive failed request. Otherwise, + /// we'll make the request and look for less-specific credentials on failure i.e. if the + /// 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) + /// - 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 netrc for a username and password + /// - Perform the request again if found + /// - Add the username and password to the cache if successful async fn handle( &self, mut request: Request, extensions: &mut Extensions, next: Next<'_>, ) -> reqwest_middleware::Result { - // Check for credentials attached to (1) the request itself + // Check for credentials attached to the request already let credentials = Credentials::from_request(&request); + // In the middleware, existing credentials are already moved from the URL // to the headers so for display purposes we restore some information let url = if tracing::enabled!(tracing::Level::DEBUG) { @@ -100,88 +140,200 @@ impl Middleware for AuthMiddleware { }; trace!("Handling request for {url}"); - // Then check for credentials in (2) the cache - let credentials = self.cache().check(request.url(), credentials); - - // Track credentials that we might want to insert into the cache - let mut new_credentials = None; - - // If already authenticated (including a password), don't query other services - if credentials.is_authenticated() { - match credentials { - // If we get credentials from the cache, update the request - CheckResponse::Cached(credentials) => request = credentials.authenticate(request), - // If we get credentials from the request, we should update the cache - // but don't need to update the request - CheckResponse::Uncached(credentials) => new_credentials = Some(credentials.clone()), - CheckResponse::None => unreachable!("No credentials cannot be authenticated"), + if let Some(credentials) = credentials { + let credentials = Arc::new(credentials); + + // If there's a password, send the request and cache + if credentials.password().is_some() { + trace!("Request for {url} is already fully authenticated"); + return self + .complete_request(Some(credentials), request, extensions, next) + .await; } - // Otherwise, look for complete credentials in: - // (3) The netrc file - } else if let Some(credentials) = self.netrc.as_ref().and_then(|netrc| { + + 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()) + { + request = credentials.authenticate(request); + // Do not insert already-cached credentials + None + } else if let Some(credentials) = self + .fetch_credentials(Some(&credentials), request.url()) + .await + { + request = credentials.authenticate(request); + Some(Arc::new(credentials)) + } else { + // If we don't find a password, we'll still attempt the request with the existing credentials + Some(credentials) + }; + + return self + .complete_request(credentials, request, extensions, next) + .await; + } + + // We have no credentials + trace!("Request for {url} is unauthenticated, checking cache"); + + // Check the cache for a URL match + let credentials = self.cache().get_url(request.url(), Username::none()); + if let Some(credentials) = credentials.as_ref() { + request = credentials.authenticate(request); + if credentials.password().is_some() { + return self.complete_request(None, request, extensions, next).await; + } + } + let attempt_has_username = credentials + .as_ref() + .is_some_and(|credentials| credentials.username().is_some()); + + // Otherise, attempt an anonymous request + trace!("Attempting unauthenticated request for {url}"); + + // + // Clone the request so we can retry it on authentication failure + let mut retry_request = request.try_clone().ok_or_else(|| { + Error::Middleware(anyhow!( + "Request object is not clonable. Are you passing a streaming body?".to_string() + )) + })?; + + let response = next.clone().run(request, extensions).await?; + + // If we don't fail with authorization related codes, return the response + if !matches!( + response.status(), + StatusCode::FORBIDDEN | StatusCode::NOT_FOUND | StatusCode::UNAUTHORIZED + ) { + return Ok(response); + } + + // Otherwise, search for credentials + trace!( + "Request for {url} failed with {}, checking for credentials", + response.status() + ); + + // Check in the cache first + let credentials = self.cache().get_realm( + Realm::from(retry_request.url()), + credentials + .map(|credentials| credentials.to_username()) + .unwrap_or(Username::none()), + ); + if let Some(credentials) = credentials.as_ref() { + if credentials.password().is_some() { + trace!("Retrying request for {url} with credentials from cache {credentials:?}"); + retry_request = credentials.authenticate(retry_request); + return self + .complete_request(None, retry_request, extensions, next) + .await; + } + } + + // 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()) + .await + { + retry_request = credentials.authenticate(retry_request); + trace!("Retrying request for {url} with {credentials:?}"); + return self + .complete_request(Some(Arc::new(credentials)), retry_request, extensions, next) + .await; + } + + if let Some(credentials) = credentials.as_ref() { + if !attempt_has_username { + trace!("Retrying request for {url} with username from cache {credentials:?}"); + retry_request = credentials.authenticate(retry_request); + return self + .complete_request(None, retry_request, extensions, next) + .await; + } + } + + Ok(response) + } +} + +impl AuthMiddleware { + /// Run a request to completion. + /// + /// If credentials are present, insert them into the cache on success. + async fn complete_request( + &self, + credentials: Option>, + request: Request, + extensions: &mut Extensions, + next: Next<'_>, + ) -> reqwest_middleware::Result { + let Some(credentials) = credentials else { + // Nothing to insert into the cache if we don't have credentials + return next.run(request, extensions).await; + }; + + let url = request.url().clone(); + let result = next.run(request, extensions).await; + + // Update the cache with new credentials on a successful request + if result + .as_ref() + .is_ok_and(|response| response.error_for_status_ref().is_ok()) + { + trace!("Updating cached credentials for {url} to {credentials:?}"); + self.cache().insert(&url, credentials) + }; + + result + } + + /// Fetch credentials for a URL. + /// + /// Supports netrc file and keyring lookups. + async fn fetch_credentials( + &self, + credentials: Option<&Credentials>, + url: &Url, + ) -> Option { + // Netrc support based on: . + if let Some(credentials) = self.netrc.as_ref().and_then(|netrc| { trace!("Checking netrc for credentials for {url}"); Credentials::from_netrc( netrc, - request.url(), + url, credentials - .get() + .as_ref() .and_then(|credentials| credentials.username()), ) }) { debug!("Found credentials in netrc file for {url}"); - request = credentials.authenticate(request); - new_credentials = Some(Arc::new(credentials)); - // (4) The keyring + 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 host so if a keyring // implementation returns different credentials for different URLs in the // same realm we will use the wrong credentials. } else if let Some(credentials) = self.keyring.as_ref().and_then(|keyring| { if let Some(username) = credentials - .get() + .as_ref() .and_then(|credentials| credentials.username()) { - debug!("Checking keyring for credentials for {url}"); - keyring.fetch(request.url(), username) + debug!("Checking keyring for credentials for {username}@{url}"); + keyring.fetch(url, username) } else { trace!("Skipping keyring lookup for {url} with no username"); None } }) { debug!("Found credentials in keyring for {url}"); - request = credentials.authenticate(request); - new_credentials = Some(Arc::new(credentials)); - // No additional credentials were found - } else { - match credentials { - CheckResponse::Cached(credentials) => request = credentials.authenticate(request), - CheckResponse::Uncached(credentials) => new_credentials = Some(credentials.clone()), - CheckResponse::None => { - debug!("No credentials found for {url}") - } - } - } - - if let Some(credentials) = new_credentials { - let url = request.url().clone(); - - // Update the default credentials eagerly since requests are made concurrently - // and we want to avoid expensive credential lookups - self.cache().set_default(&url, credentials.clone()); - - let result = next.run(request, extensions).await; - - // Only update the cache with new credentials on a successful request - if result - .as_ref() - .is_ok_and(|response| response.error_for_status_ref().is_ok()) - { - trace!("Updating cached credentials for {url}"); - self.cache().insert(&url, credentials) - }; - result + Some(credentials) } else { - next.run(request, extensions).await + None } } } @@ -196,7 +348,7 @@ mod tests { use test_log::test; use url::Url; - use wiremock::matchers::{basic_auth, method}; + use wiremock::matchers::{basic_auth, method, path_regex}; use wiremock::{Mock, MockServer, ResponseTemplate}; use super::*; @@ -256,8 +408,9 @@ mod tests { Ok(()) } + /// Without seeding the cache, authenticated requests are not cached #[test(tokio::test)] - async fn test_credentials_in_url() -> Result<(), Error> { + async fn test_credentials_in_url_no_seed() -> Result<(), Error> { let username = "user"; let password = "password"; @@ -279,6 +432,7 @@ mod tests { 200, "Subsequent requests should not require credentials" ); + assert_eq!( client .get(format!("{}/foo", server.uri())) @@ -286,7 +440,7 @@ mod tests { .await? .status(), 200, - "Subsequent requests can be to different paths in the same realm" + "Requests can be to different paths in the same realm" ); let mut url = base_url.clone(); @@ -302,28 +456,88 @@ mod tests { } #[test(tokio::test)] - async fn test_credentials_in_url_username_only() -> Result<(), Error> { + async fn test_credentials_in_url_seed() -> Result<(), Error> { let username = "user"; - let password = ""; + let password = "password"; let server = start_test_server(username, password).await; + let base_url = Url::parse(&server.uri())?; + let cache = CredentialsCache::new(); + cache.insert( + &base_url, + Arc::new(Credentials::new( + Some(username.to_string()), + Some(password.to_string()), + )), + ); + let client = test_client_builder() - .with(AuthMiddleware::new().with_cache(CredentialsCache::new())) + .with(AuthMiddleware::new().with_cache(cache)) .build(); + let mut url = base_url.clone(); + url.set_username(username).unwrap(); + url.set_password(Some(password)).unwrap(); + assert_eq!(client.get(url).send().await?.status(), 200); + + // Works for a URL without credentials too + assert_eq!( + client.get(server.uri()).send().await?.status(), + 200, + "Requests should not require credentials" + ); + + assert_eq!( + client + .get(format!("{}/foo", server.uri())) + .send() + .await? + .status(), + 200, + "Requests can be to different paths in the same realm" + ); + + let mut url = base_url.clone(); + url.set_username(username).unwrap(); + url.set_password(Some("invalid")).unwrap(); + assert_eq!( + client.get(url).send().await?.status(), + 401, + "Credentials in the URL should take precedence and fail" + ); + + Ok(()) + } + + #[test(tokio::test)] + async fn test_credentials_in_url_username_only() -> Result<(), Error> { + let username = "user"; + let password = ""; + + let server = start_test_server(username, password).await; let base_url = Url::parse(&server.uri())?; + let cache = CredentialsCache::new(); + cache.insert( + &base_url, + Arc::new(Credentials::new(Some(username.to_string()), None)), + ); + + let client = test_client_builder() + .with(AuthMiddleware::new().with_cache(cache)) + .build(); let mut url = base_url.clone(); url.set_username(username).unwrap(); url.set_password(None).unwrap(); assert_eq!(client.get(url).send().await?.status(), 200); - // Works for a URL without credentials now + // Works for a URL without credentials too assert_eq!( client.get(server.uri()).send().await?.status(), 200, - "Subsequent requests should not require credentials" + "Requests should not require credentials" ); + assert_eq!( client .get(format!("{}/foo", server.uri())) @@ -331,7 +545,7 @@ mod tests { .await? .status(), 200, - "Subsequent requests can be to different paths in the same realm" + "Requests can be to different paths in the same realm" ); let mut url = base_url.clone(); @@ -583,4 +797,397 @@ mod tests { Ok(()) } + + #[test(tokio::test)] + async fn test_credentials_in_url_multiple_realms() -> Result<(), Error> { + let username_1 = "user1"; + let password_1 = "password1"; + let server_1 = start_test_server(username_1, password_1).await; + let base_url_1 = Url::parse(&server_1.uri())?; + + let username_2 = "user2"; + let password_2 = "password2"; + let server_2 = start_test_server(username_2, password_2).await; + let base_url_2 = Url::parse(&server_2.uri())?; + + let cache = CredentialsCache::new(); + // Seed the cache with our credentials + cache.insert( + &base_url_1, + Arc::new(Credentials::new( + Some(username_1.to_string()), + Some(password_1.to_string()), + )), + ); + cache.insert( + &base_url_2, + Arc::new(Credentials::new( + Some(username_2.to_string()), + Some(password_2.to_string()), + )), + ); + + let client = test_client_builder() + .with(AuthMiddleware::new().with_cache(cache)) + .build(); + + // Both servers should work + assert_eq!( + client.get(server_1.uri()).send().await?.status(), + 200, + "Requests should not require credentials" + ); + assert_eq!( + client.get(server_2.uri()).send().await?.status(), + 200, + "Requests should not require credentials" + ); + + assert_eq!( + client + .get(format!("{}/foo", server_1.uri())) + .send() + .await? + .status(), + 200, + "Requests can be to different paths in the same realm" + ); + assert_eq!( + client + .get(format!("{}/foo", server_2.uri())) + .send() + .await? + .status(), + 200, + "Requests can be to different paths in the same realm" + ); + + Ok(()) + } + + #[test(tokio::test)] + async fn test_credentials_from_keyring_multiple_realms() -> Result<(), Error> { + let username_1 = "user1"; + let password_1 = "password1"; + let server_1 = start_test_server(username_1, password_1).await; + let base_url_1 = Url::parse(&server_1.uri())?; + + let username_2 = "user2"; + let password_2 = "password2"; + let server_2 = start_test_server(username_2, password_2).await; + let base_url_2 = Url::parse(&server_2.uri())?; + + let client = test_client_builder() + .with( + AuthMiddleware::new() + .with_cache(CredentialsCache::new()) + .with_keyring(Some(KeyringProvider::dummy([ + ((base_url_1.host_str().unwrap(), username_1), password_1), + ((base_url_2.host_str().unwrap(), username_2), password_2), + ]))), + ) + .build(); + + // Both servers do not work without a username + assert_eq!( + client.get(server_1.uri()).send().await?.status(), + 401, + "Requests should require a username" + ); + assert_eq!( + client.get(server_2.uri()).send().await?.status(), + 401, + "Requests should require a username" + ); + + let mut url_1 = base_url_1.clone(); + url_1.set_username(username_1).unwrap(); + assert_eq!( + client.get(url_1.clone()).send().await?.status(), + 200, + "Requests with a username should succeed" + ); + assert_eq!( + client.get(server_2.uri()).send().await?.status(), + 401, + "Credentials should not be re-used for the second server" + ); + + let mut url_2 = base_url_2.clone(); + url_2.set_username(username_2).unwrap(); + assert_eq!( + client.get(url_2.clone()).send().await?.status(), + 200, + "Requests with a username should succeed" + ); + + assert_eq!( + client.get(format!("{}/foo", url_1)).send().await?.status(), + 200, + "Requests can be to different paths in the same realm" + ); + assert_eq!( + client.get(format!("{}/foo", url_2)).send().await?.status(), + 200, + "Requests can be to different paths in the same realm" + ); + + Ok(()) + } + + #[test(tokio::test)] + async fn test_credentials_in_url_mixed_authentication_in_realm() -> Result<(), Error> { + let username_1 = "user1"; + let password_1 = "password1"; + let username_2 = "user2"; + let password_2 = "password2"; + + let server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path_regex("/prefix_1.*")) + .and(basic_auth(username_1, password_1)) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + Mock::given(method("GET")) + .and(path_regex("/prefix_2.*")) + .and(basic_auth(username_2, password_2)) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + // Create a third, public prefix + // It will throw a 401 if it recieves credentials + Mock::given(method("GET")) + .and(path_regex("/prefix_3.*")) + .and(basic_auth(username_1, password_1)) + .respond_with(ResponseTemplate::new(401)) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path_regex("/prefix_3.*")) + .and(basic_auth(username_2, password_2)) + .respond_with(ResponseTemplate::new(401)) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path_regex("/prefix_3.*")) + .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 base_url_3 = base_url.join("prefix_3")?; + + let cache = CredentialsCache::new(); + + // Seed the cache with our credentials + cache.insert( + &base_url_1, + Arc::new(Credentials::new( + Some(username_1.to_string()), + Some(password_1.to_string()), + )), + ); + cache.insert( + &base_url_2, + Arc::new(Credentials::new( + Some(username_2.to_string()), + Some(password_2.to_string()), + )), + ); + + let client = test_client_builder() + .with(AuthMiddleware::new().with_cache(cache)) + .build(); + + // Both servers should work + assert_eq!( + client.get(base_url_1.clone()).send().await?.status(), + 200, + "Requests should not require credentials" + ); + assert_eq!( + client.get(base_url_2.clone()).send().await?.status(), + 200, + "Requests should not require credentials" + ); + assert_eq!( + client + .get(base_url.join("prefix_1/foo")?) + .send() + .await? + .status(), + 200, + "Requests can be to different paths in the same realm" + ); + assert_eq!( + client + .get(base_url.join("prefix_2/foo")?) + .send() + .await? + .status(), + 200, + "Requests can be to different paths in the same realm" + ); + assert_eq!( + client + .get(base_url.join("prefix_1_foo")?) + .send() + .await? + .status(), + 401, + "Requests to paths with a matching prefix but different resource segments should fail" + ); + + assert_eq!( + client.get(base_url_3.clone()).send().await?.status(), + 200, + "Requests to the 'public' prefix should not use credentials" + ); + + Ok(()) + } + + #[test(tokio::test)] + async fn test_credentials_from_keyring_mixed_authentication_in_realm() -> Result<(), Error> { + let username_1 = "user1"; + let password_1 = "password1"; + let username_2 = "user2"; + let password_2 = "password2"; + + let server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path_regex("/prefix_1.*")) + .and(basic_auth(username_1, password_1)) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + Mock::given(method("GET")) + .and(path_regex("/prefix_2.*")) + .and(basic_auth(username_2, password_2)) + .respond_with(ResponseTemplate::new(200)) + .mount(&server) + .await; + + // Create a third, public prefix + // It will throw a 401 if it recieves credentials + Mock::given(method("GET")) + .and(path_regex("/prefix_3.*")) + .and(basic_auth(username_1, password_1)) + .respond_with(ResponseTemplate::new(401)) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path_regex("/prefix_3.*")) + .and(basic_auth(username_2, password_2)) + .respond_with(ResponseTemplate::new(401)) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path_regex("/prefix_3.*")) + .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 base_url_3 = base_url.join("prefix_3")?; + + let client = test_client_builder() + .with( + AuthMiddleware::new() + .with_cache(CredentialsCache::new()) + .with_keyring(Some(KeyringProvider::dummy([ + ((base_url_1.host_str().unwrap(), username_1), password_1), + ((base_url_2.host_str().unwrap(), username_2), password_2), + ]))), + ) + .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_1).unwrap(); + assert_eq!( + client.get(url_1.clone()).send().await?.status(), + 200, + "Requests with a username should succeed" + ); + assert_eq!( + client.get(base_url_2.clone()).send().await?.status(), + 401, + "Credentials should not be re-used for the second prefix" + ); + + let mut url_2 = base_url_2.clone(); + url_2.set_username(username_2).unwrap(); + assert_eq!( + client.get(url_2.clone()).send().await?.status(), + 200, + "Requests with a username should succeed" + ); + + assert_eq!( + client + .get(base_url.join("prefix_1/foo")?) + .send() + .await? + .status(), + 200, + "Requests can be to different paths in the same prefix" + ); + assert_eq!( + client + .get(base_url.join("prefix_2/foo")?) + .send() + .await? + .status(), + 200, + "Requests can be to different paths in the same prefix" + ); + assert_eq!( + client + .get(base_url.join("prefix_1_foo")?) + .send() + .await? + .status(), + 401, + "Requests to paths with a matching prefix but different resource segments should fail" + ); + assert_eq!( + client.get(base_url_3.clone()).send().await?.status(), + 200, + "Requests to the 'public' prefix should not use credentials" + ); + + Ok(()) + } } diff --git a/crates/uv-auth/src/netloc.rs b/crates/uv-auth/src/realm.rs similarity index 51% rename from crates/uv-auth/src/netloc.rs rename to crates/uv-auth/src/realm.rs index 65e348117365..92d73d5f8509 100644 --- a/crates/uv-auth/src/netloc.rs +++ b/crates/uv-auth/src/realm.rs @@ -1,3 +1,5 @@ +use std::{fmt::Display, fmt::Formatter}; + use url::Url; /// Used to determine if authentication information should be retained on a new URL. @@ -20,13 +22,13 @@ use url::Url; // However, `url` (and therefore `reqwest`) sets the `port` to `None` if it matches the default port // so we do not need any special handling here. #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub(crate) struct NetLoc { +pub(crate) struct Realm { scheme: String, host: Option, port: Option, } -impl From<&Url> for NetLoc { +impl From<&Url> for Realm { fn from(url: &Url) -> Self { Self { scheme: url.scheme().to_string(), @@ -36,88 +38,108 @@ impl From<&Url> for NetLoc { } } +impl Display for Realm { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + if let Some(port) = self.port { + write!( + f, + "{}://{}:{port}", + self.scheme, + self.host.as_deref().unwrap_or_default() + ) + } else { + write!( + f, + "{}://{}", + self.scheme, + self.host.as_deref().unwrap_or_default() + ) + } + } +} + #[cfg(test)] mod tests { use url::{ParseError, Url}; - use crate::NetLoc; + use crate::Realm; #[test] fn test_should_retain_auth() -> Result<(), ParseError> { // Exact match (https) assert_eq!( - NetLoc::from(&Url::parse("https://example.com")?), - NetLoc::from(&Url::parse("https://example.com")?) + Realm::from(&Url::parse("https://example.com")?), + Realm::from(&Url::parse("https://example.com")?) ); // Exact match (with port) assert_eq!( - NetLoc::from(&Url::parse("https://example.com:1234")?), - NetLoc::from(&Url::parse("https://example.com:1234")?) + Realm::from(&Url::parse("https://example.com:1234")?), + Realm::from(&Url::parse("https://example.com:1234")?) ); // Exact match (http) assert_eq!( - NetLoc::from(&Url::parse("http://example.com")?), - NetLoc::from(&Url::parse("http://example.com")?) + Realm::from(&Url::parse("http://example.com")?), + Realm::from(&Url::parse("http://example.com")?) ); // Okay, path differs assert_eq!( - NetLoc::from(&Url::parse("http://example.com/foo")?), - NetLoc::from(&Url::parse("http://example.com/bar")?) + Realm::from(&Url::parse("http://example.com/foo")?), + Realm::from(&Url::parse("http://example.com/bar")?) ); // Okay, default port differs (https) assert_eq!( - NetLoc::from(&Url::parse("https://example.com:443")?), - NetLoc::from(&Url::parse("https://example.com")?) + Realm::from(&Url::parse("https://example.com:443")?), + Realm::from(&Url::parse("https://example.com")?) ); // Okay, default port differs (http) assert_eq!( - NetLoc::from(&Url::parse("http://example.com:80")?), - NetLoc::from(&Url::parse("http://example.com")?) + Realm::from(&Url::parse("http://example.com:80")?), + Realm::from(&Url::parse("http://example.com")?) ); // Mismatched scheme assert_ne!( - NetLoc::from(&Url::parse("https://example.com")?), - NetLoc::from(&Url::parse("http://example.com")?) + Realm::from(&Url::parse("https://example.com")?), + Realm::from(&Url::parse("http://example.com")?) ); // Mismatched scheme, we explicitly do not allow upgrade to https assert_ne!( - NetLoc::from(&Url::parse("http://example.com")?), - NetLoc::from(&Url::parse("https://example.com")?) + Realm::from(&Url::parse("http://example.com")?), + Realm::from(&Url::parse("https://example.com")?) ); // Mismatched host assert_ne!( - NetLoc::from(&Url::parse("https://foo.com")?), - NetLoc::from(&Url::parse("https://bar.com")?) + Realm::from(&Url::parse("https://foo.com")?), + Realm::from(&Url::parse("https://bar.com")?) ); // Mismatched port assert_ne!( - NetLoc::from(&Url::parse("https://example.com:1234")?), - NetLoc::from(&Url::parse("https://example.com:5678")?) + Realm::from(&Url::parse("https://example.com:1234")?), + Realm::from(&Url::parse("https://example.com:5678")?) ); // Mismatched port, with one as default for scheme assert_ne!( - NetLoc::from(&Url::parse("https://example.com:443")?), - NetLoc::from(&Url::parse("https://example.com:5678")?) + Realm::from(&Url::parse("https://example.com:443")?), + Realm::from(&Url::parse("https://example.com:5678")?) ); assert_ne!( - NetLoc::from(&Url::parse("https://example.com:1234")?), - NetLoc::from(&Url::parse("https://example.com:443")?) + Realm::from(&Url::parse("https://example.com:1234")?), + Realm::from(&Url::parse("https://example.com:443")?) ); // Mismatched port, with default for a different scheme assert_ne!( - NetLoc::from(&Url::parse("https://example.com:80")?), - NetLoc::from(&Url::parse("https://example.com")?) + Realm::from(&Url::parse("https://example.com:80")?), + Realm::from(&Url::parse("https://example.com")?) ); Ok(()) diff --git a/crates/uv-client/tests/netrc_auth.rs b/crates/uv-client/tests/netrc_auth.rs deleted file mode 100644 index 8d657169f99c..000000000000 --- a/crates/uv-client/tests/netrc_auth.rs +++ /dev/null @@ -1,73 +0,0 @@ -use std::env; -use std::io::Write; - -use anyhow::Result; -use futures::future; -use http::header::AUTHORIZATION; -use http_body_util::Full; -use hyper::body::Bytes; -use hyper::server::conn::http1; -use hyper::service::service_fn; -use hyper::{Request, Response}; -use hyper_util::rt::TokioIo; -use tempfile::NamedTempFile; -use tokio::net::TcpListener; - -use uv_cache::Cache; -use uv_client::RegistryClientBuilder; - -#[tokio::test] -async fn test_client_with_netrc_credentials() -> Result<()> { - // Set up the TCP listener on a random available port - let listener = TcpListener::bind("127.0.0.1:0").await?; - let addr = listener.local_addr()?; - - // Spawn the server loop in a background task - tokio::spawn(async move { - let svc = service_fn(move |req: Request| { - // Get User Agent Header and send it back in the response - let auth = req - .headers() - .get(AUTHORIZATION) - .and_then(|v| v.to_str().ok()) - .map(|s| s.to_string()) - .unwrap_or_default(); // Empty Default - future::ok::<_, hyper::Error>(Response::new(Full::new(Bytes::from(auth)))) - }); - // Start Server (not wrapped in loop {} since we want a single response server) - // If you want server to accept multiple connections, wrap it in loop {} - let (socket, _) = listener.accept().await.unwrap(); - let socket = TokioIo::new(socket); - tokio::task::spawn(async move { - http1::Builder::new() - .serve_connection(socket, svc) - .with_upgrades() - .await - .expect("Server Started"); - }); - }); - - // Create a netrc file - let mut netrc_file = NamedTempFile::new()?; - env::set_var("NETRC", netrc_file.path()); - writeln!(netrc_file, "machine 127.0.0.1 login user password 1234")?; - - // Initialize uv-client - let cache = Cache::temp()?; - let client = RegistryClientBuilder::new(cache).build(); - - // Send request to our dummy server - let res = client - .uncached_client() - .get(format!("http://{addr}")) - .send() - .await?; - - // Check the HTTP status - assert!(res.status().is_success()); - - // Verify auth header - assert_eq!(res.text().await?, "Basic dXNlcjoxMjM0"); - - Ok(()) -} diff --git a/crates/uv/tests/pip_install.rs b/crates/uv/tests/pip_install.rs index 346d53018ef1..06077758d48f 100644 --- a/crates/uv/tests/pip_install.rs +++ b/crates/uv/tests/pip_install.rs @@ -23,13 +23,26 @@ const READ_ONLY_GITHUB_TOKEN: &[&str] = &[ "NVZMaExzZmtFMHZ1ZEVNd0pPZXZkV040WUdTcmk2WXREeFB4TFlybGlwRTZONEpHV01FMnFZQWJVUm4=", ]; +// This is a fine-grained token that only has read-only access to the `uv-private-pypackage-2` repository +#[cfg(not(windows))] +const READ_ONLY_GITHUB_TOKEN_2: &[&str] = &[ + "Z2l0aHViX3BhdA==", + "MTFCR0laQTdRMHV1MEpwaFp4dFFyRwo=", + "cnNmNXJwMHk2WWpteVZvb2ZFc0c5WUs5b2NPcFY1aVpYTnNmdE05eEhaM0lGSExSSktDWTcxeVBVZXkK", +]; + /// Decode a split, base64 encoded authentication token. /// We split and encode the token to bypass revoke by GitHub's secret scanning fn decode_token(content: &[&str]) -> String { let token = content .iter() .map(|part| base64.decode(part).unwrap()) - .map(|decoded| std::str::from_utf8(decoded.as_slice()).unwrap().to_string()) + .map(|decoded| { + std::str::from_utf8(decoded.as_slice()) + .unwrap() + .trim_end() + .to_string() + }) .join("_"); token } @@ -1119,7 +1132,7 @@ fn install_git_private_https_pat() { .collect(); let package = format!( - "uv-private-pypackage @ git+https://{token}@github.com/astral-test/uv-private-pypackage" + "uv-private-pypackage@ git+https://{token}@github.com/astral-test/uv-private-pypackage" ); uv_snapshot!(filters, context.install().arg(package) @@ -1138,6 +1151,77 @@ fn install_git_private_https_pat() { context.assert_installed("uv_private_pypackage", "0.1.0"); } +/// Install a package from a private GitHub repository using a PAT +/// Include a public GitHub repository too, to ensure that the authentication is not erroneously copied over. +#[test] +#[cfg(all(not(windows), feature = "git"))] +fn install_git_private_https_pat_mixed_with_public() { + let context = TestContext::new("3.8"); + let token = decode_token(READ_ONLY_GITHUB_TOKEN); + + let filters: Vec<_> = [(token.as_str(), "***")] + .into_iter() + .chain(context.filters()) + .collect(); + + let package = format!( + "uv-private-pypackage @ git+https://{token}@github.com/astral-test/uv-private-pypackage" + ); + + uv_snapshot!(filters, context.install().arg(package).arg("uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage"), + @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Downloaded 2 packages in [TIME] + Installed 2 packages in [TIME] + + uv-private-pypackage==0.1.0 (from git+https://***@github.com/astral-test/uv-private-pypackage@d780faf0ac91257d4d5a4f0c5a0e4509608c0071) + + uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@b270df1a2fb5d012294e9aaf05e7e0bab1e6a389) + "###); + + context.assert_installed("uv_private_pypackage", "0.1.0"); +} + +/// Install packages from multiple private GitHub repositories with separate PATS +#[test] +#[cfg(all(not(windows), feature = "git"))] +fn install_git_private_https_multiple_pat() { + let context = TestContext::new("3.8"); + let token_1 = decode_token(READ_ONLY_GITHUB_TOKEN); + let token_2 = decode_token(READ_ONLY_GITHUB_TOKEN_2); + + let filters: Vec<_> = [(token_1.as_str(), "***_1"), (token_2.as_str(), "***_2")] + .into_iter() + .chain(context.filters()) + .collect(); + + let package_1 = format!( + "uv-private-pypackage @ git+https://{token_1}@github.com/astral-test/uv-private-pypackage" + ); + let package_2 = format!( + "uv-private-pypackage-2 @ git+https://{token_2}@github.com/astral-test/uv-private-pypackage-2" + ); + + uv_snapshot!(filters, context.install().arg(package_1).arg(package_2) + , @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Downloaded 2 packages in [TIME] + Installed 2 packages in [TIME] + + uv-private-pypackage==0.1.0 (from git+https://***_1@github.com/astral-test/uv-private-pypackage@d780faf0ac91257d4d5a4f0c5a0e4509608c0071) + + uv-private-pypackage-2==0.1.0 (from git+https://***_2@github.com/astral-test/uv-private-pypackage-2@45c0bec7365710f09b1f4dbca61c86dde9537e4e) + "###); + + context.assert_installed("uv_private_pypackage", "0.1.0"); +} + /// Install a package from a private GitHub repository at a specific commit using a PAT #[test] #[cfg(feature = "git")] @@ -1245,6 +1329,80 @@ fn install_git_private_https_pat_not_authorized() { "###); } +/// Install a package from a private GitHub repository using a PAT +/// Does not use `git`, instead installs a distribution artifact. +/// Include a public GitHub repository too, to ensure that the authentication is not erroneously copied over. +#[test] +#[cfg(not(windows))] +fn install_github_artifact_private_https_pat_mixed_with_public() { + let context = TestContext::new("3.8"); + let token = decode_token(READ_ONLY_GITHUB_TOKEN); + + let filters: Vec<_> = [(token.as_str(), "***")] + .into_iter() + .chain(context.filters()) + .collect(); + + let private_package = format!( + "uv-private-pypackage @ https://{token}@raw.githubusercontent.com/astral-test/uv-private-pypackage/main/dist/uv_private_pypackage-0.1.0-py3-none-any.whl" + ); + let public_package = "uv-public-pypackage @ https://raw.githubusercontent.com/astral-test/uv-public-pypackage/main/dist/uv_public_pypackage-0.1.0-py3-none-any.whl"; + + uv_snapshot!(filters, context.install().arg(private_package).arg(public_package), + @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Downloaded 2 packages in [TIME] + Installed 2 packages in [TIME] + + uv-private-pypackage==0.1.0 (from https://***@raw.githubusercontent.com/astral-test/uv-private-pypackage/main/dist/uv_private_pypackage-0.1.0-py3-none-any.whl) + + uv-public-pypackage==0.1.0 (from https://raw.githubusercontent.com/astral-test/uv-public-pypackage/main/dist/uv_public_pypackage-0.1.0-py3-none-any.whl) + "###); + + context.assert_installed("uv_private_pypackage", "0.1.0"); +} + +/// Install packages from multiple private GitHub repositories with separate PATS +/// Does not use `git`, instead installs a distribution artifact. +#[test] +#[cfg(not(windows))] +fn install_github_artifact_private_https_multiple_pat() { + let context = TestContext::new("3.8"); + let token_1 = decode_token(READ_ONLY_GITHUB_TOKEN); + let token_2 = decode_token(READ_ONLY_GITHUB_TOKEN_2); + + let filters: Vec<_> = [(token_1.as_str(), "***_1"), (token_2.as_str(), "***_2")] + .into_iter() + .chain(context.filters()) + .collect(); + + let package_1 = format!( + "uv-private-pypackage @ https://astral-test-bot:{token_1}@raw.githubusercontent.com/astral-test/uv-private-pypackage/main/dist/uv_private_pypackage-0.1.0-py3-none-any.whl" + ); + let package_2 = format!( + "uv-private-pypackage-2 @ https://astral-test-bot:{token_2}@raw.githubusercontent.com/astral-test/uv-private-pypackage-2/main/dist/uv_private_pypackage_2-0.1.0-py3-none-any.whl" + ); + + uv_snapshot!(filters, context.install().arg(package_1).arg(package_2) + , @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Downloaded 2 packages in [TIME] + Installed 2 packages in [TIME] + + uv-private-pypackage==0.1.0 (from https://astral-test-bot:***_1@raw.githubusercontent.com/astral-test/uv-private-pypackage/main/dist/uv_private_pypackage-0.1.0-py3-none-any.whl) + + uv-private-pypackage-2==0.1.0 (from https://astral-test-bot:***_2@raw.githubusercontent.com/astral-test/uv-private-pypackage-2/main/dist/uv_private_pypackage_2-0.1.0-py3-none-any.whl) + "###); + + context.assert_installed("uv_private_pypackage", "0.1.0"); +} + /// Install a package without using pre-built wheels. #[test] fn reinstall_no_binary() {