diff --git a/Cargo.lock b/Cargo.lock index a984477db..3145fefcc 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 e23fe4610..7ff0a4cbd 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 273eb2f64..a2081ff7d 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 URL, if any. - /// - /// The [`Url`] is not checked for credentials. Existing credentials should be extracted and passed - /// separately. - /// - /// If complete credentials are provided, they will be returned as [`CheckResponse::Existing`] - /// If the credentials are partial, i.e. missing a password, the cache will be checked - /// for a corresponding entry. - pub(crate) fn check(&self, url: &Url, credentials: Option) -> CheckResponse { - let store = self.store.lock().unwrap(); - - let credentials = credentials.map(Arc::new); - let key = CredentialsCache::key( - url, - credentials - .as_ref() - .and_then(|credentials| credentials.username().map(str::to_string)), - ); - - if let Some(credentials) = credentials { - if credentials.password().is_some() { - trace!("Existing credentials include password, skipping cache"); - // No need to look-up, we have a password already - return CheckResponse::Uncached(credentials); - } - trace!("Existing credentials missing password, checking cache"); - let existing = store.get(&key); - existing - .cloned() - .map(CheckResponse::Cached) - .inspect(|_| trace!("Found cached credentials.")) - .unwrap_or_else(|| { - trace!("No credentials in cache, using existing credentials"); - CheckResponse::Uncached(credentials) - }) + /// 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 { - 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 - }) - } + 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 + }) } - /// 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()); + /// Return the cached credentials for a URL and username, if any. + /// + /// 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 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()); } - false - } - - /// Returns true if a key is in the cache. - fn contains_key(&self, key: &(NetLoc, Option)) -> bool { - let store = self.store.lock().unwrap(); - store.contains_key(key) + 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) + } + + 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); + + 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 5bceac793..c8826bdb1 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 ce564074d..6d6ea2bd4 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 f028d7dc9..ae003429b 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 netrc::Netrc; -use reqwest::{Request, Response}; -use reqwest_middleware::{Middleware, Next}; -use tracing::{debug, trace}; +use http::{Extensions, StatusCode}; +use url::Url; use crate::{ - cache::CheckResponse, credentials::Credentials, CredentialsCache, KeyringProvider, - CREDENTIALS_CACHE, + credentials::{Credentials, Username}, + realm::Realm, + CredentialsCache, KeyringProvider, CREDENTIALS_CACHE, }; +use anyhow::anyhow; +use netrc::Netrc; +use reqwest::{Request, Response}; +use reqwest_middleware::{Error, Middleware, Next}; +use tracing::{debug, trace}; -/// 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); + if let Some(credentials) = credentials { + let credentials = Arc::new(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 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 + Some(credentials) } 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 - } 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,61 @@ 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(); + 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_seed() -> Result<(), Error> { + let username = "user"; + 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(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(); @@ -307,23 +515,29 @@ mod tests { let password = ""; let server = start_test_server(username, password).await; - let client = test_client_builder() - .with(AuthMiddleware::new().with_cache(CredentialsCache::new())) - .build(); - let base_url = Url::parse(&server.uri())?; + 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 65e348117..92d73d5f8 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 8d657169f..000000000 --- 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 346d53018..06077758d 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() {