Use `Credentials` abstraction in `uv-publish` (#12682)

## Summary

I noticed that we aren't using these here -- we have a separate username
and password situation.
This commit is contained in:
Charlie Marsh 2025-04-04 19:07:51 -04:00 committed by GitHub
parent 1ff7265e8a
commit c4fd34f063
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 131 additions and 122 deletions

1
Cargo.lock generated
View File

@ -5462,6 +5462,7 @@ dependencies = [
"tokio-util", "tokio-util",
"tracing", "tracing",
"url", "url",
"uv-auth",
"uv-cache", "uv-cache",
"uv-client", "uv-client",
"uv-configuration", "uv-configuration",

View File

@ -70,7 +70,7 @@ impl From<Option<String>> for Username {
impl Credentials { impl Credentials {
/// Create a set of HTTP Basic Authentication credentials. /// Create a set of HTTP Basic Authentication credentials.
#[allow(dead_code)] #[allow(dead_code)]
pub(crate) fn basic(username: Option<String>, password: Option<String>) -> Self { pub fn basic(username: Option<String>, password: Option<String>) -> Self {
Self::Basic { Self::Basic {
username: Username::new(username), username: Username::new(username),
password, password,
@ -79,7 +79,7 @@ impl Credentials {
/// Create a set of Bearer Authentication credentials. /// Create a set of Bearer Authentication credentials.
#[allow(dead_code)] #[allow(dead_code)]
pub(crate) fn bearer(token: Vec<u8>) -> Self { pub fn bearer(token: Vec<u8>) -> Self {
Self::Bearer { token } Self::Bearer { token }
} }
@ -245,7 +245,7 @@ impl Credentials {
/// Create an HTTP Basic Authentication header for the credentials. /// Create an HTTP Basic Authentication header for the credentials.
/// ///
/// Panics if the username or password cannot be base64 encoded. /// Panics if the username or password cannot be base64 encoded.
pub(crate) fn to_header_value(&self) -> HeaderValue { pub fn to_header_value(&self) -> HeaderValue {
match self { match self {
Self::Basic { .. } => { Self::Basic { .. } => {
// See: <https://github.com/seanmonstar/reqwest/blob/2c11ef000b151c2eebeed2c18a7b81042220c6b0/src/util.rs#L3> // See: <https://github.com/seanmonstar/reqwest/blob/2c11ef000b151c2eebeed2c18a7b81042220c6b0/src/util.rs#L3>
@ -291,7 +291,7 @@ impl Credentials {
/// ///
/// Any existing credentials will be overridden. /// Any existing credentials will be overridden.
#[must_use] #[must_use]
pub(crate) fn authenticate(&self, mut request: Request) -> Request { pub fn authenticate(&self, mut request: Request) -> Request {
request request
.headers_mut() .headers_mut()
.insert(reqwest::header::AUTHORIZATION, Self::to_header_value(self)); .insert(reqwest::header::AUTHORIZATION, Self::to_header_value(self));

View File

@ -13,6 +13,7 @@ license.workspace = true
doctest = false doctest = false
[dependencies] [dependencies]
uv-auth = { workspace = true }
uv-cache = { workspace = true } uv-cache = { workspace = true }
uv-client = { workspace = true } uv-client = { workspace = true }
uv-configuration = { workspace = true } uv-configuration = { workspace = true }

View File

@ -1,8 +1,10 @@
mod trusted_publishing; mod trusted_publishing;
use crate::trusted_publishing::TrustedPublishingError; use std::path::{Path, PathBuf};
use base64::prelude::BASE64_STANDARD; use std::sync::Arc;
use base64::Engine; use std::time::{Duration, SystemTime};
use std::{env, fmt, io};
use fs_err::tokio::File; use fs_err::tokio::File;
use futures::TryStreamExt; use futures::TryStreamExt;
use glob::{glob, GlobError, PatternError}; use glob::{glob, GlobError, PatternError};
@ -15,10 +17,6 @@ use reqwest_retry::policies::ExponentialBackoff;
use reqwest_retry::{RetryPolicy, Retryable, RetryableStrategy}; use reqwest_retry::{RetryPolicy, Retryable, RetryableStrategy};
use rustc_hash::FxHashSet; use rustc_hash::FxHashSet;
use serde::Deserialize; use serde::Deserialize;
use std::path::{Path, PathBuf};
use std::sync::Arc;
use std::time::{Duration, SystemTime};
use std::{env, fmt, io};
use thiserror::Error; use thiserror::Error;
use tokio::io::{AsyncReadExt, BufReader}; use tokio::io::{AsyncReadExt, BufReader};
use tokio::sync::Semaphore; use tokio::sync::Semaphore;
@ -26,6 +24,8 @@ use tokio_util::io::ReaderStream;
use tracing::{debug, enabled, trace, warn, Level}; use tracing::{debug, enabled, trace, warn, Level};
use trusted_publishing::TrustedPublishingToken; use trusted_publishing::TrustedPublishingToken;
use url::Url; use url::Url;
use uv_auth::Credentials;
use uv_cache::{Cache, Refresh}; use uv_cache::{Cache, Refresh};
use uv_client::{ use uv_client::{
BaseClient, MetadataFormat, OwnedArchive, RegistryClientBuilder, UvRetryableStrategy, BaseClient, MetadataFormat, OwnedArchive, RegistryClientBuilder, UvRetryableStrategy,
@ -41,6 +41,8 @@ use uv_pypi_types::{HashAlgorithm, HashDigest, Metadata23, MetadataError};
use uv_static::EnvVars; use uv_static::EnvVars;
use uv_warnings::{warn_user, warn_user_once}; use uv_warnings::{warn_user, warn_user_once};
use crate::trusted_publishing::TrustedPublishingError;
#[derive(Error, Debug)] #[derive(Error, Debug)]
pub enum PublishError { pub enum PublishError {
#[error("The publish path is not a valid glob pattern: `{0}`")] #[error("The publish path is not a valid glob pattern: `{0}`")]
@ -370,8 +372,7 @@ pub async fn upload(
filename: &DistFilename, filename: &DistFilename,
registry: &Url, registry: &Url,
client: &BaseClient, client: &BaseClient,
username: Option<&str>, credentials: &Credentials,
password: Option<&str>,
check_url_client: Option<&CheckUrlClient<'_>>, check_url_client: Option<&CheckUrlClient<'_>>,
download_concurrency: &Semaphore, download_concurrency: &Semaphore,
reporter: Arc<impl Reporter>, reporter: Arc<impl Reporter>,
@ -391,8 +392,7 @@ pub async fn upload(
filename, filename,
registry, registry,
client, client,
username, credentials,
password,
&form_metadata, &form_metadata,
reporter.clone(), reporter.clone(),
) )
@ -744,8 +744,7 @@ async fn build_request(
filename: &DistFilename, filename: &DistFilename,
registry: &Url, registry: &Url,
client: &BaseClient, client: &BaseClient,
username: Option<&str>, credentials: &Credentials,
password: Option<&str>,
form_metadata: &[(&'static str, String)], form_metadata: &[(&'static str, String)],
reporter: Arc<impl Reporter>, reporter: Arc<impl Reporter>,
) -> Result<(RequestBuilder, usize), PublishPrepareError> { ) -> Result<(RequestBuilder, usize), PublishPrepareError> {
@ -767,17 +766,15 @@ async fn build_request(
let part = Part::stream_with_length(file_reader, file_size).file_name(raw_filename.to_string()); let part = Part::stream_with_length(file_reader, file_size).file_name(raw_filename.to_string());
form = form.part("content", part); form = form.part("content", part);
let url = if let Some(username) = username { // If we have a username but no password, attach the username to the URL so the authentication
if password.is_none() { // middleware can find the matching password.
// Attach the username to the URL so the authentication middleware can find the matching let url = if let Some(username) = credentials
// password. .username()
.filter(|_| credentials.password().is_none())
{
let mut url = registry.clone(); let mut url = registry.clone();
let _ = url.set_username(username); let _ = url.set_username(username);
url url
} else {
// We set the authorization header below.
registry.clone()
}
} else { } else {
registry.clone() registry.clone()
}; };
@ -793,11 +790,20 @@ async fn build_request(
reqwest::header::ACCEPT, reqwest::header::ACCEPT,
"application/json;q=0.9, text/plain;q=0.8, text/html;q=0.7", "application/json;q=0.9, text/plain;q=0.8, text/html;q=0.7",
); );
if let (Some(username), Some(password)) = (username, password) {
debug!("Using username/password basic auth"); match credentials {
let credentials = BASE64_STANDARD.encode(format!("{username}:{password}")); Credentials::Basic { password, .. } => {
request = request.header(AUTHORIZATION, format!("Basic {credentials}")); if password.is_some() {
debug!("Using HTTP Basic authentication");
request = request.header(AUTHORIZATION, credentials.to_header_value());
} }
}
Credentials::Bearer { .. } => {
debug!("Using Bearer token authentication");
request = request.header(AUTHORIZATION, credentials.to_header_value());
}
}
Ok((request, idx)) Ok((request, idx))
} }
@ -875,6 +881,7 @@ mod tests {
use std::path::PathBuf; use std::path::PathBuf;
use std::sync::Arc; use std::sync::Arc;
use url::Url; use url::Url;
use uv_auth::Credentials;
use uv_client::BaseClientBuilder; use uv_client::BaseClientBuilder;
use uv_distribution_filename::DistFilename; use uv_distribution_filename::DistFilename;
@ -958,8 +965,7 @@ mod tests {
&filename, &filename,
&Url::parse("https://example.org/upload").unwrap(), &Url::parse("https://example.org/upload").unwrap(),
&BaseClientBuilder::new().build(), &BaseClientBuilder::new().build(),
Some("ferris"), &Credentials::basic(Some("ferris".to_string()), Some("F3RR!S".to_string())),
Some("F3RR!S"),
&form_metadata, &form_metadata,
Arc::new(DummyReporter), Arc::new(DummyReporter),
) )
@ -969,7 +975,7 @@ mod tests {
insta::with_settings!({ insta::with_settings!({
filters => [("boundary=[0-9a-f-]+", "boundary=[...]")], filters => [("boundary=[0-9a-f-]+", "boundary=[...]")],
}, { }, {
assert_debug_snapshot!(&request, @r###" assert_debug_snapshot!(&request, @r#"
RequestBuilder { RequestBuilder {
inner: RequestBuilder { inner: RequestBuilder {
method: POST, method: POST,
@ -992,12 +998,12 @@ mod tests {
"content-type": "multipart/form-data; boundary=[...]", "content-type": "multipart/form-data; boundary=[...]",
"content-length": "6803", "content-length": "6803",
"accept": "application/json;q=0.9, text/plain;q=0.8, text/html;q=0.7", "accept": "application/json;q=0.9, text/plain;q=0.8, text/html;q=0.7",
"authorization": "Basic ZmVycmlzOkYzUlIhUw==", "authorization": Sensitive,
}, },
}, },
.. ..
} }
"###); "#);
}); });
} }
@ -1109,8 +1115,7 @@ mod tests {
&filename, &filename,
&Url::parse("https://example.org/upload").unwrap(), &Url::parse("https://example.org/upload").unwrap(),
&BaseClientBuilder::new().build(), &BaseClientBuilder::new().build(),
Some("ferris"), &Credentials::basic(Some("ferris".to_string()), Some("F3RR!S".to_string())),
Some("F3RR!S"),
&form_metadata, &form_metadata,
Arc::new(DummyReporter), Arc::new(DummyReporter),
) )
@ -1120,7 +1125,7 @@ mod tests {
insta::with_settings!({ insta::with_settings!({
filters => [("boundary=[0-9a-f-]+", "boundary=[...]")], filters => [("boundary=[0-9a-f-]+", "boundary=[...]")],
}, { }, {
assert_debug_snapshot!(&request, @r###" assert_debug_snapshot!(&request, @r#"
RequestBuilder { RequestBuilder {
inner: RequestBuilder { inner: RequestBuilder {
method: POST, method: POST,
@ -1143,12 +1148,12 @@ mod tests {
"content-type": "multipart/form-data; boundary=[...]", "content-type": "multipart/form-data; boundary=[...]",
"content-length": "19330", "content-length": "19330",
"accept": "application/json;q=0.9, text/plain;q=0.8, text/html;q=0.7", "accept": "application/json;q=0.9, text/plain;q=0.8, text/html;q=0.7",
"authorization": "Basic ZmVycmlzOkYzUlIhUw==", "authorization": Sensitive,
}, },
}, },
.. ..
} }
"###); "#);
}); });
} }
} }

View File

@ -9,7 +9,7 @@ use owo_colors::OwoColorize;
use tokio::sync::Semaphore; use tokio::sync::Semaphore;
use tracing::{debug, info}; use tracing::{debug, info};
use url::Url; use url::Url;
use uv_auth::Credentials;
use uv_cache::Cache; use uv_cache::Cache;
use uv_client::{AuthIntegration, BaseClient, BaseClientBuilder, RegistryClientBuilder}; use uv_client::{AuthIntegration, BaseClient, BaseClientBuilder, RegistryClientBuilder};
use uv_configuration::{KeyringProviderType, TrustedPublishing}; use uv_configuration::{KeyringProviderType, TrustedPublishing};
@ -74,7 +74,7 @@ pub(crate) async fn publish(
// We're only checking a single URL and one at a time, so 1 permit is sufficient // We're only checking a single URL and one at a time, so 1 permit is sufficient
let download_concurrency = Arc::new(Semaphore::new(1)); let download_concurrency = Arc::new(Semaphore::new(1));
let (publish_url, username, password) = gather_credentials( let (publish_url, credentials) = gather_credentials(
publish_url, publish_url,
username, username,
password, password,
@ -137,8 +137,7 @@ pub(crate) async fn publish(
&filename, &filename,
&publish_url, &publish_url,
&upload_client, &upload_client,
username.as_deref(), &credentials,
password.as_deref(),
check_url_client.as_ref(), check_url_client.as_ref(),
&download_concurrency, &download_concurrency,
// Needs to be an `Arc` because the reqwest `Body` static lifetime requirement // Needs to be an `Arc` because the reqwest `Body` static lifetime requirement
@ -207,7 +206,7 @@ async fn gather_credentials(
check_url: Option<&IndexUrl>, check_url: Option<&IndexUrl>,
prompt: Prompt, prompt: Prompt,
printer: Printer, printer: Printer,
) -> Result<(Url, Option<String>, Option<String>)> { ) -> Result<(Url, Credentials)> {
// Support reading username and password from the URL, for symmetry with the index API. // Support reading username and password from the URL, for symmetry with the index API.
if let Some(url_password) = publish_url.password() { if let Some(url_password) = publish_url.password() {
if password.is_some_and(|password| password != url_password) { if password.is_some_and(|password| password != url_password) {
@ -318,7 +317,10 @@ async fn gather_credentials(
// We may be using the keyring for the simple index. // We may be using the keyring for the simple index.
} }
} }
Ok((publish_url, username, password))
let credentials = Credentials::basic(username, password);
Ok((publish_url, credentials))
} }
fn prompt_username_and_password() -> Result<(Option<String>, Option<String>)> { fn prompt_username_and_password() -> Result<(Option<String>, Option<String>)> {
@ -343,11 +345,11 @@ mod tests {
use insta::assert_snapshot; use insta::assert_snapshot;
use url::Url; use url::Url;
async fn credentials( async fn get_credentials(
url: Url, url: Url,
username: Option<String>, username: Option<String>,
password: Option<String>, password: Option<String>,
) -> Result<(Url, Option<String>, Option<String>)> { ) -> Result<(Url, Credentials)> {
let client = BaseClientBuilder::new().build(); let client = BaseClientBuilder::new().build();
gather_credentials( gather_credentials(
url, url,
@ -370,30 +372,30 @@ mod tests {
let example_url_username_password = let example_url_username_password =
Url::from_str("https://ferris:f3rr1s@example.com").unwrap(); Url::from_str("https://ferris:f3rr1s@example.com").unwrap();
let (publish_url, username, password) = let (publish_url, credentials) = get_credentials(example_url.clone(), None, None)
credentials(example_url.clone(), None, None).await.unwrap();
assert_eq!(publish_url, example_url);
assert_eq!(username, None);
assert_eq!(password, None);
let (publish_url, username, password) =
credentials(example_url_username.clone(), None, None)
.await .await
.unwrap(); .unwrap();
assert_eq!(publish_url, example_url); assert_eq!(publish_url, example_url);
assert_eq!(username.as_deref(), Some("ferris")); assert_eq!(credentials.username(), None);
assert_eq!(password, None); assert_eq!(credentials.password(), None);
let (publish_url, username, password) = let (publish_url, credentials) = get_credentials(example_url_username.clone(), None, None)
credentials(example_url_username_password.clone(), None, None)
.await .await
.unwrap(); .unwrap();
assert_eq!(publish_url, example_url); assert_eq!(publish_url, example_url);
assert_eq!(username.as_deref(), Some("ferris")); assert_eq!(credentials.username(), Some("ferris"));
assert_eq!(password.as_deref(), Some("f3rr1s")); assert_eq!(credentials.password(), None);
let (publish_url, credentials) =
get_credentials(example_url_username_password.clone(), None, None)
.await
.unwrap();
assert_eq!(publish_url, example_url);
assert_eq!(credentials.username(), Some("ferris"));
assert_eq!(credentials.password(), Some("f3rr1s"));
// Ok: The username is the same between CLI/env vars and URL // Ok: The username is the same between CLI/env vars and URL
let (publish_url, username, password) = credentials( let (publish_url, credentials) = get_credentials(
example_url_username_password.clone(), example_url_username_password.clone(),
Some("ferris".to_string()), Some("ferris".to_string()),
None, None,
@ -401,11 +403,11 @@ mod tests {
.await .await
.unwrap(); .unwrap();
assert_eq!(publish_url, example_url); assert_eq!(publish_url, example_url);
assert_eq!(username.as_deref(), Some("ferris")); assert_eq!(credentials.username(), Some("ferris"));
assert_eq!(password.as_deref(), Some("f3rr1s")); assert_eq!(credentials.password(), Some("f3rr1s"));
// Err: There are two different usernames between CLI/env vars and URL // Err: There are two different usernames between CLI/env vars and URL
let err = credentials( let err = get_credentials(
example_url_username_password.clone(), example_url_username_password.clone(),
Some("packaging-platypus".to_string()), Some("packaging-platypus".to_string()),
None, None,
@ -418,7 +420,7 @@ mod tests {
); );
// Ok: The username and password are the same between CLI/env vars and URL // Ok: The username and password are the same between CLI/env vars and URL
let (publish_url, username, password) = credentials( let (publish_url, credentials) = get_credentials(
example_url_username_password.clone(), example_url_username_password.clone(),
Some("ferris".to_string()), Some("ferris".to_string()),
Some("f3rr1s".to_string()), Some("f3rr1s".to_string()),
@ -426,11 +428,11 @@ mod tests {
.await .await
.unwrap(); .unwrap();
assert_eq!(publish_url, example_url); assert_eq!(publish_url, example_url);
assert_eq!(username.as_deref(), Some("ferris")); assert_eq!(credentials.username(), Some("ferris"));
assert_eq!(password.as_deref(), Some("f3rr1s")); assert_eq!(credentials.password(), Some("f3rr1s"));
// Err: There are two different passwords between CLI/env vars and URL // Err: There are two different passwords between CLI/env vars and URL
let err = credentials( let err = get_credentials(
example_url_username_password.clone(), example_url_username_password.clone(),
Some("ferris".to_string()), Some("ferris".to_string()),
Some("secret".to_string()), Some("secret".to_string()),