Refactor `AuthenticationStore` to inline credentials (#2427)

This commit is contained in:
Zanie Blue 2024-03-13 17:48:02 -05:00 committed by GitHub
parent 9159731792
commit 22a52391be
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 84 additions and 57 deletions

2
Cargo.lock generated
View File

@ -4277,7 +4277,7 @@ dependencies = [
"async-trait", "async-trait",
"base64 0.21.7", "base64 0.21.7",
"clap", "clap",
"lazy_static", "once_cell",
"reqwest", "reqwest",
"reqwest-middleware", "reqwest-middleware",
"rust-netrc", "rust-netrc",

View File

@ -60,7 +60,6 @@ indicatif = { version = "0.17.7" }
indoc = { version = "2.0.4" } indoc = { version = "2.0.4" }
itertools = { version = "0.12.1" } itertools = { version = "0.12.1" }
junction = { version = "1.0.0" } junction = { version = "1.0.0" }
lazy_static = { version = "1.4.0" }
mailparse = { version = "0.14.0" } mailparse = { version = "0.14.0" }
miette = { version = "6.0.0" } miette = { version = "6.0.0" }
nanoid = { version = "0.4.0" } nanoid = { version = "0.4.0" }

View File

@ -7,7 +7,7 @@ use thiserror::Error;
use pep440_rs::{VersionSpecifiers, VersionSpecifiersParseError}; use pep440_rs::{VersionSpecifiers, VersionSpecifiersParseError};
use pypi_types::{DistInfoMetadata, Hashes, Yanked}; use pypi_types::{DistInfoMetadata, Hashes, Yanked};
use url::Url; use url::Url;
use uv_auth::AuthenticationStore; use uv_auth::GLOBAL_AUTH_STORE;
/// Error converting [`pypi_types::File`] to [`distribution_type::File`]. /// Error converting [`pypi_types::File`] to [`distribution_type::File`].
#[derive(Debug, Error)] #[derive(Debug, Error)]
@ -55,7 +55,7 @@ impl File {
url: if file.url.contains("://") { url: if file.url.contains("://") {
let url = Url::parse(&file.url) let url = Url::parse(&file.url)
.map_err(|err| FileConversionError::Url(file.url.clone(), err))?; .map_err(|err| FileConversionError::Url(file.url.clone(), err))?;
let url = AuthenticationStore::with_url_encoded_auth(url); let url = GLOBAL_AUTH_STORE.with_url_encoded_auth(url);
FileLocation::AbsoluteUrl(url.to_string()) FileLocation::AbsoluteUrl(url.to_string())
} else { } else {
FileLocation::RelativeUrl(base.to_string(), file.url) FileLocation::RelativeUrl(base.to_string(), file.url)

View File

@ -7,7 +7,6 @@ edition = "2021"
async-trait = { workspace = true } async-trait = { workspace = true }
base64 = { workspace = true } base64 = { workspace = true }
clap = { workspace = true, features = ["derive", "env"], optional = true } clap = { workspace = true, features = ["derive", "env"], optional = true }
lazy_static = { workspace = true }
reqwest = { workspace = true } reqwest = { workspace = true }
reqwest-middleware = { workspace = true } reqwest-middleware = { workspace = true }
rust-netrc = { workspace = true } rust-netrc = { workspace = true }
@ -15,6 +14,7 @@ task-local-extensions = { workspace = true }
thiserror = { workspace = true } thiserror = { workspace = true }
tracing = { workspace = true } tracing = { workspace = true }
url = { workspace = true } url = { workspace = true }
once_cell = { workspace = true }
[dev-dependencies] [dev-dependencies]
tempfile = { workspace = true } tempfile = { workspace = true }

View File

@ -4,10 +4,16 @@ mod store;
pub use keyring::KeyringProvider; pub use keyring::KeyringProvider;
pub use middleware::AuthMiddleware; pub use middleware::AuthMiddleware;
use once_cell::sync::Lazy;
pub use store::AuthenticationStore; pub use store::AuthenticationStore;
use url::Url; use url::Url;
// TODO(zanieb): Consider passing a store explicitly throughout
/// Global authentication store for a `uv` invocation
pub static GLOBAL_AUTH_STORE: Lazy<AuthenticationStore> = Lazy::new(AuthenticationStore::default);
/// Used to determine if authentication information should be retained on a new URL. /// Used to determine if authentication information should be retained on a new URL.
/// Based on the specification defined in RFC 7235 and 7230. /// Based on the specification defined in RFC 7235 and 7230.
/// ///

View File

@ -7,7 +7,8 @@ use tracing::{debug, warn};
use crate::{ use crate::{
keyring::{get_keyring_subprocess_auth, KeyringProvider}, keyring::{get_keyring_subprocess_auth, KeyringProvider},
store::{AuthenticationStore, Credential}, store::Credential,
GLOBAL_AUTH_STORE,
}; };
/// A middleware that adds basic authentication to requests based on the netrc file and the keyring. /// A middleware that adds basic authentication to requests based on the netrc file and the keyring.
@ -47,13 +48,13 @@ impl Middleware for AuthMiddleware {
// This gives in-URL credentials precedence over the netrc file. // This gives in-URL credentials precedence over the netrc file.
if req.headers().contains_key(reqwest::header::AUTHORIZATION) { if req.headers().contains_key(reqwest::header::AUTHORIZATION) {
if !url.username().is_empty() { if !url.username().is_empty() {
AuthenticationStore::save_from_url(&url); GLOBAL_AUTH_STORE.save_from_url(&url);
} }
return next.run(req, _extensions).await; return next.run(req, _extensions).await;
} }
// Try auth strategies in order of precedence: // Try auth strategies in order of precedence:
if let Some(stored_auth) = AuthenticationStore::get(&url) { if let Some(stored_auth) = GLOBAL_AUTH_STORE.get(&url) {
// If we've already seen this URL, we can use the stored credentials // If we've already seen this URL, we can use the stored credentials
if let Some(auth) = stored_auth { if let Some(auth) = stored_auth {
match auth { match auth {
@ -77,7 +78,7 @@ impl Middleware for AuthMiddleware {
reqwest::header::AUTHORIZATION, reqwest::header::AUTHORIZATION,
basic_auth(auth.username(), auth.password()), basic_auth(auth.username(), auth.password()),
); );
AuthenticationStore::set(&url, Some(auth)); GLOBAL_AUTH_STORE.set(&url, Some(auth));
} else if matches!(self.keyring_provider, KeyringProvider::Subprocess) { } else if matches!(self.keyring_provider, KeyringProvider::Subprocess) {
// If we have keyring support enabled, we check there as well // If we have keyring support enabled, we check there as well
match get_keyring_subprocess_auth(&url) { match get_keyring_subprocess_auth(&url) {
@ -86,7 +87,7 @@ impl Middleware for AuthMiddleware {
reqwest::header::AUTHORIZATION, reqwest::header::AUTHORIZATION,
basic_auth(auth.username(), auth.password()), basic_auth(auth.username(), auth.password()),
); );
AuthenticationStore::set(&url, Some(auth)); GLOBAL_AUTH_STORE.set(&url, Some(auth));
} }
Ok(None) => { Ok(None) => {
debug!("No keyring credentials found for {url}"); debug!("No keyring credentials found for {url}");
@ -99,7 +100,7 @@ impl Middleware for AuthMiddleware {
// If we still don't have any credentials, we save the URL so we don't have to check netrc or keyring again // If we still don't have any credentials, we save the URL so we don't have to check netrc or keyring again
if !req.headers().contains_key(reqwest::header::AUTHORIZATION) { if !req.headers().contains_key(reqwest::header::AUTHORIZATION) {
AuthenticationStore::set(&url, None); GLOBAL_AUTH_STORE.set(&url, None);
} }
next.run(req, _extensions).await next.run(req, _extensions).await

View File

@ -1,4 +1,3 @@
use lazy_static::lazy_static;
use std::{collections::HashMap, sync::Mutex}; use std::{collections::HashMap, sync::Mutex};
use netrc::Authenticator; use netrc::Authenticator;
@ -7,11 +6,6 @@ use url::Url;
use crate::NetLoc; use crate::NetLoc;
lazy_static! {
// Store credentials for NetLoc
static ref PASSWORDS: Mutex<HashMap<NetLoc, Option<Credential>>> = Mutex::new(HashMap::new());
}
#[derive(Clone, Debug, PartialEq)] #[derive(Clone, Debug, PartialEq)]
pub enum Credential { pub enum Credential {
Basic(BasicAuthData), Basic(BasicAuthData),
@ -68,35 +62,49 @@ pub struct BasicAuthData {
pub password: Option<String>, pub password: Option<String>,
} }
pub struct AuthenticationStore; pub struct AuthenticationStore {
credentials: Mutex<HashMap<NetLoc, Option<Credential>>>,
}
impl Default for AuthenticationStore {
fn default() -> Self {
Self::new()
}
}
impl AuthenticationStore { impl AuthenticationStore {
pub fn get(url: &Url) -> Option<Option<Credential>> { pub fn new() -> Self {
let netloc = NetLoc::from(url); Self {
let passwords = PASSWORDS.lock().unwrap(); credentials: Mutex::new(HashMap::new()),
passwords.get(&netloc).cloned() }
} }
pub fn set(url: &Url, auth: Option<Credential>) { pub fn get(&self, url: &Url) -> Option<Option<Credential>> {
let netloc = NetLoc::from(url); let netloc = NetLoc::from(url);
let mut passwords = PASSWORDS.lock().unwrap(); let credentials = self.credentials.lock().unwrap();
passwords.insert(netloc, auth); credentials.get(&netloc).cloned()
}
pub fn set(&self, url: &Url, auth: Option<Credential>) {
let netloc = NetLoc::from(url);
let mut credentials = self.credentials.lock().unwrap();
credentials.insert(netloc, auth);
} }
/// Copy authentication from one URL to another URL if applicable. /// Copy authentication from one URL to another URL if applicable.
pub fn with_url_encoded_auth(url: Url) -> Url { pub fn with_url_encoded_auth(&self, url: Url) -> Url {
let netloc = NetLoc::from(&url); let netloc = NetLoc::from(&url);
let passwords = PASSWORDS.lock().unwrap(); let credentials = self.credentials.lock().unwrap();
if let Some(Some(Credential::UrlEncoded(url_auth))) = passwords.get(&netloc) { if let Some(Some(Credential::UrlEncoded(url_auth))) = credentials.get(&netloc) {
url_auth.apply_to_url(url) url_auth.apply_to_url(url)
} else { } else {
url url
} }
} }
pub fn save_from_url(url: &Url) { pub fn save_from_url(&self, url: &Url) {
let netloc = NetLoc::from(url); let netloc = NetLoc::from(url);
let mut passwords = PASSWORDS.lock().unwrap(); let mut credentials = self.credentials.lock().unwrap();
if url.username().is_empty() { if url.username().is_empty() {
// No credentials to save // No credentials to save
return; return;
@ -105,7 +113,7 @@ impl AuthenticationStore {
username: url.username().to_string(), username: url.username().to_string(),
password: url.password().map(str::to_string), password: url.password().map(str::to_string),
}; };
passwords.insert(netloc, Some(Credential::UrlEncoded(auth))); credentials.insert(netloc, Some(Credential::UrlEncoded(auth)));
} }
} }
@ -113,63 +121,76 @@ impl AuthenticationStore {
mod test { mod test {
use super::*; use super::*;
// NOTE: Because tests run in parallel, it is imperative to use different URLs for each
#[test] #[test]
fn set_get_work() { fn store_set_and_get() {
let url = Url::parse("https://test1example1.com/simple/").unwrap(); let store = AuthenticationStore::new();
let not_set_res = AuthenticationStore::get(&url); let url = Url::parse("https://example1.com/simple/").unwrap();
let not_set_res = store.get(&url);
assert!(not_set_res.is_none()); assert!(not_set_res.is_none());
let found_first_url = Url::parse("https://test1example2.com/simple/first/").unwrap(); let found_first_url = Url::parse("https://example2.com/simple/first/").unwrap();
let not_found_first_url = Url::parse("https://test1example3.com/simple/first/").unwrap(); let not_found_first_url = Url::parse("https://example3.com/simple/first/").unwrap();
AuthenticationStore::set( store.set(
&found_first_url, &found_first_url,
Some(Credential::Basic(BasicAuthData { Some(Credential::Basic(BasicAuthData {
username: "u".to_string(), username: "u".to_string(),
password: Some("p".to_string()), password: Some("p".to_string()),
})), })),
); );
AuthenticationStore::set(&not_found_first_url, None); store.set(&not_found_first_url, None);
let found_second_url = Url::parse("https://test1example2.com/simple/second/").unwrap(); let found_second_url = Url::parse("https://example2.com/simple/second/").unwrap();
let not_found_second_url = Url::parse("https://test1example3.com/simple/second/").unwrap(); let not_found_second_url = Url::parse("https://example3.com/simple/second/").unwrap();
let found_res = AuthenticationStore::get(&found_second_url); let found_res = store.get(&found_second_url);
assert!(found_res.is_some()); assert!(found_res.is_some());
let found_res = found_res.unwrap(); let found_res = found_res.unwrap();
assert!(matches!(found_res, Some(Credential::Basic(_)))); assert!(matches!(found_res, Some(Credential::Basic(_))));
let not_found_res = AuthenticationStore::get(&not_found_second_url); let not_found_res = store.get(&not_found_second_url);
assert!(not_found_res.is_some()); assert!(not_found_res.is_some());
let not_found_res = not_found_res.unwrap(); let not_found_res = not_found_res.unwrap();
assert!(not_found_res.is_none()); assert!(not_found_res.is_none());
} }
#[test] #[test]
fn with_url_encoded_auth_works() { fn store_with_url_encoded_auth() {
let url = Url::parse("https://test2example.com/simple/").unwrap(); let store = AuthenticationStore::new();
let url = Url::parse("https://example.com/simple/").unwrap();
let auth = Credential::UrlEncoded(UrlAuthData { let auth = Credential::UrlEncoded(UrlAuthData {
username: "u".to_string(), username: "u".to_string(),
password: Some("p".to_string()), password: Some("p".to_string()),
}); });
AuthenticationStore::set(&url, Some(auth.clone())); // Before adding to the store there's no change
let url = store.with_url_encoded_auth(url);
assert_eq!(url.username(), "");
assert_eq!(url.password(), None);
let url = AuthenticationStore::with_url_encoded_auth(url); store.set(&url, Some(auth.clone()));
// After adding to the store, the url is updated
let url = store.with_url_encoded_auth(url);
assert_eq!(url.username(), "u"); assert_eq!(url.username(), "u");
assert_eq!(url.password(), Some("p")); assert_eq!(url.password(), Some("p"));
} }
#[test] #[test]
fn save_from_url_works() { fn store_save_from_url() {
let url = Url::parse("https://u:p@test3example.com/simple/").unwrap(); let store = AuthenticationStore::new();
let url = Url::parse("https://u:p@example.com/simple/").unwrap();
AuthenticationStore::save_from_url(&url); store.save_from_url(&url);
let found_res = AuthenticationStore::get(&url); let found_res = store.get(&url);
assert!(found_res.is_some()); assert!(found_res.is_some());
let found_res = found_res.unwrap(); let found_res = found_res.unwrap();
assert!(matches!(found_res, Some(Credential::UrlEncoded(_)))); assert!(matches!(found_res, Some(Credential::UrlEncoded(_))));
let url = Url::parse("https://example2.com/simple/").unwrap();
store.save_from_url(&url);
let found_res = store.get(&url);
assert!(found_res.is_none());
} }
} }

View File

@ -17,7 +17,7 @@ use pep440_rs::Version;
use pep508_rs::VerbatimUrl; use pep508_rs::VerbatimUrl;
use platform_tags::Tags; use platform_tags::Tags;
use pypi_types::Hashes; use pypi_types::Hashes;
use uv_auth::AuthenticationStore; use uv_auth::GLOBAL_AUTH_STORE;
use uv_cache::{Cache, CacheBucket}; use uv_cache::{Cache, CacheBucket};
use uv_normalize::PackageName; use uv_normalize::PackageName;
@ -157,13 +157,13 @@ impl<'a> FlatIndexClient<'a> {
async { async {
// Use the response URL, rather than the request URL, as the base for relative URLs. // Use the response URL, rather than the request URL, as the base for relative URLs.
// This ensures that we handle redirects and other URL transformations correctly. // This ensures that we handle redirects and other URL transformations correctly.
let url = AuthenticationStore::with_url_encoded_auth(response.url().clone()); let url = GLOBAL_AUTH_STORE.with_url_encoded_auth(response.url().clone());
let text = response.text().await.map_err(ErrorKind::from)?; let text = response.text().await.map_err(ErrorKind::from)?;
let SimpleHtml { base, files } = SimpleHtml::parse(&text, &url) let SimpleHtml { base, files } = SimpleHtml::parse(&text, &url)
.map_err(|err| Error::from_html_err(err, url.clone()))?; .map_err(|err| Error::from_html_err(err, url.clone()))?;
let base = AuthenticationStore::with_url_encoded_auth(base.into_url()); let base = GLOBAL_AUTH_STORE.with_url_encoded_auth(base.into_url());
let files: Vec<File> = files let files: Vec<File> = files
.into_iter() .into_iter()
.filter_map(|file| { .filter_map(|file| {

View File

@ -21,7 +21,7 @@ use distribution_types::{BuiltDist, File, FileLocation, IndexUrl, IndexUrls, Nam
use install_wheel_rs::metadata::{find_archive_dist_info, is_metadata_entry}; use install_wheel_rs::metadata::{find_archive_dist_info, is_metadata_entry};
use pep440_rs::Version; use pep440_rs::Version;
use pypi_types::{Metadata23, SimpleJson}; use pypi_types::{Metadata23, SimpleJson};
use uv_auth::{AuthMiddleware, AuthenticationStore, KeyringProvider}; use uv_auth::{AuthMiddleware, KeyringProvider, GLOBAL_AUTH_STORE};
use uv_cache::{Cache, CacheBucket, WheelCache}; use uv_cache::{Cache, CacheBucket, WheelCache};
use uv_fs::Simplified; use uv_fs::Simplified;
use uv_normalize::PackageName; use uv_normalize::PackageName;
@ -317,7 +317,7 @@ impl RegistryClient {
async { async {
// Use the response URL, rather than the request URL, as the base for relative URLs. // Use the response URL, rather than the request URL, as the base for relative URLs.
// This ensures that we handle redirects and other URL transformations correctly. // This ensures that we handle redirects and other URL transformations correctly.
let url = AuthenticationStore::with_url_encoded_auth(response.url().clone()); let url = GLOBAL_AUTH_STORE.with_url_encoded_auth(response.url().clone());
let content_type = response let content_type = response
.headers() .headers()
@ -346,7 +346,7 @@ impl RegistryClient {
let text = response.text().await.map_err(ErrorKind::from)?; let text = response.text().await.map_err(ErrorKind::from)?;
let SimpleHtml { base, files } = SimpleHtml::parse(&text, &url) let SimpleHtml { base, files } = SimpleHtml::parse(&text, &url)
.map_err(|err| Error::from_html_err(err, url.clone()))?; .map_err(|err| Error::from_html_err(err, url.clone()))?;
let base = AuthenticationStore::with_url_encoded_auth(base.into_url()); let base = GLOBAL_AUTH_STORE.with_url_encoded_auth(base.into_url());
SimpleMetadata::from_files(files, package_name, &base) SimpleMetadata::from_files(files, package_name, &base)
} }