Skip to content

Commit 8a54b24

Browse files
committed
Add only_authenticated option to the client
When sending an upload request, we use HTTP formdata requests, which can't be cloned (seanmonstar/reqwest#2416, plus a limitation that formdata bodies are always internally streaming), but also know that we need to always have credentials. The authentication middleware by default tries to clone the request and send an authenticated request first. By introducing an `only_authenticated` setting, we can skip this behaviour for publishing. Split out from #7475
1 parent 4fdf5fc commit 8a54b24

2 files changed

Lines changed: 81 additions & 39 deletions

File tree

crates/uv-auth/src/middleware.rs

Lines changed: 55 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::{
88
realm::Realm,
99
CredentialsCache, KeyringProvider, CREDENTIALS_CACHE,
1010
};
11-
use anyhow::anyhow;
11+
use anyhow::{anyhow, format_err};
1212
use netrc::Netrc;
1313
use reqwest::{Request, Response};
1414
use reqwest_middleware::{Error, Middleware, Next};
@@ -22,6 +22,9 @@ pub struct AuthMiddleware {
2222
netrc: Option<Netrc>,
2323
keyring: Option<KeyringProvider>,
2424
cache: Option<CredentialsCache>,
25+
/// We know that the endpoint needs authentication, so we don't try to send an unauthenticated
26+
/// request, avoiding cloning an uncloneable request.
27+
only_authenticated: bool,
2528
}
2629

2730
impl AuthMiddleware {
@@ -30,6 +33,7 @@ impl AuthMiddleware {
3033
netrc: Netrc::new().ok(),
3134
keyring: None,
3235
cache: None,
36+
only_authenticated: false,
3337
}
3438
}
3539

@@ -56,6 +60,14 @@ impl AuthMiddleware {
5660
self
5761
}
5862

63+
/// We know that the endpoint needs authentication, so we don't try to send an unauthenticated
64+
/// request, avoiding cloning an uncloneable request.
65+
#[must_use]
66+
pub fn with_only_authenticated(mut self, only_authenticated: bool) -> Self {
67+
self.only_authenticated = only_authenticated;
68+
self
69+
}
70+
5971
/// Get the configured authentication store.
6072
///
6173
/// If not set, the global store is used.
@@ -198,32 +210,42 @@ impl Middleware for AuthMiddleware {
198210
.as_ref()
199211
.is_some_and(|credentials| credentials.username().is_some());
200212

201-
// Otherwise, attempt an anonymous request
202-
trace!("Attempting unauthenticated request for {url}");
203-
204-
// <https://github.com/TrueLayer/reqwest-middleware/blob/abdf1844c37092d323683c2396b7eefda1418d3c/reqwest-retry/src/middleware.rs#L141-L149>
205-
// Clone the request so we can retry it on authentication failure
206-
let mut retry_request = request.try_clone().ok_or_else(|| {
207-
Error::Middleware(anyhow!(
208-
"Request object is not cloneable. Are you passing a streaming body?".to_string()
209-
))
210-
})?;
211-
212-
let response = next.clone().run(request, extensions).await?;
213-
214-
// If we don't fail with authorization related codes, return the response
215-
if !matches!(
216-
response.status(),
217-
StatusCode::FORBIDDEN | StatusCode::NOT_FOUND | StatusCode::UNAUTHORIZED
218-
) {
219-
return Ok(response);
220-
}
213+
let (mut retry_request, response) = if self.only_authenticated {
214+
// For endpoints where we require the user to provide credentials, we don't try the
215+
// unauthenticated request first.
216+
trace!("Checking for credentials for {url}");
217+
(request, None)
218+
} else {
219+
// Otherwise, attempt an anonymous request
220+
trace!("Attempting unauthenticated request for {url}");
221+
222+
// <https://github.com/TrueLayer/reqwest-middleware/blob/abdf1844c37092d323683c2396b7eefda1418d3c/reqwest-retry/src/middleware.rs#L141-L149>
223+
// Clone the request so we can retry it on authentication failure
224+
let retry_request = request.try_clone().ok_or_else(|| {
225+
Error::Middleware(anyhow!(
226+
"Request object is not cloneable. Are you passing a streaming body?"
227+
.to_string()
228+
))
229+
})?;
230+
231+
let response = next.clone().run(request, extensions).await?;
232+
233+
// If we don't fail with authorization related codes, return the response
234+
if !matches!(
235+
response.status(),
236+
StatusCode::FORBIDDEN | StatusCode::NOT_FOUND | StatusCode::UNAUTHORIZED
237+
) {
238+
return Ok(response);
239+
}
221240

222-
// Otherwise, search for credentials
223-
trace!(
224-
"Request for {url} failed with {}, checking for credentials",
225-
response.status()
226-
);
241+
// Otherwise, search for credentials
242+
trace!(
243+
"Request for {url} failed with {}, checking for credentials",
244+
response.status()
245+
);
246+
247+
(retry_request, Some(response))
248+
};
227249

228250
// Check in the cache first
229251
let credentials = self.cache().get_realm(
@@ -265,7 +287,13 @@ impl Middleware for AuthMiddleware {
265287
}
266288
}
267289

268-
Ok(response)
290+
if let Some(response) = response {
291+
Ok(response)
292+
} else {
293+
Err(Error::Middleware(format_err!(
294+
"Missing credentials for {url}"
295+
)))
296+
}
269297
}
270298
}
271299

crates/uv-client/src/base_client.rs

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ pub struct BaseClientBuilder<'a> {
3636
client: Option<Client>,
3737
markers: Option<&'a MarkerEnvironment>,
3838
platform: Option<&'a Platform>,
39+
only_authenticated: bool,
3940
}
4041

4142
impl Default for BaseClientBuilder<'_> {
@@ -55,6 +56,7 @@ impl BaseClientBuilder<'_> {
5556
client: None,
5657
markers: None,
5758
platform: None,
59+
only_authenticated: false,
5860
}
5961
}
6062
}
@@ -108,6 +110,12 @@ impl<'a> BaseClientBuilder<'a> {
108110
self
109111
}
110112

113+
#[must_use]
114+
pub fn only_authenticated(mut self, only_authenticated: bool) -> Self {
115+
self.only_authenticated = only_authenticated;
116+
self
117+
}
118+
111119
pub fn is_offline(&self) -> bool {
112120
matches!(self.connectivity, Connectivity::Offline)
113121
}
@@ -230,20 +238,26 @@ impl<'a> BaseClientBuilder<'a> {
230238
fn apply_middleware(&self, client: Client) -> ClientWithMiddleware {
231239
match self.connectivity {
232240
Connectivity::Online => {
233-
let client = reqwest_middleware::ClientBuilder::new(client);
234-
235-
// Initialize the retry strategy.
236-
let retry_policy =
237-
ExponentialBackoff::builder().build_with_max_retries(self.retries);
238-
let retry_strategy = RetryTransientMiddleware::new_with_policy_and_strategy(
239-
retry_policy,
240-
UvRetryableStrategy,
241-
);
242-
let client = client.with(retry_strategy);
241+
let mut client = reqwest_middleware::ClientBuilder::new(client);
242+
243+
// Avoid non-cloneable errors with a streaming body during publish.
244+
if self.retries > 0 {
245+
// Initialize the retry strategy.
246+
let retry_policy =
247+
ExponentialBackoff::builder().build_with_max_retries(self.retries);
248+
let retry_strategy = RetryTransientMiddleware::new_with_policy_and_strategy(
249+
retry_policy,
250+
UvRetryableStrategy,
251+
);
252+
client = client.with(retry_strategy);
253+
}
243254

244255
// Initialize the authentication middleware to set headers.
245-
let client =
246-
client.with(AuthMiddleware::new().with_keyring(self.keyring.to_provider()));
256+
client = client.with(
257+
AuthMiddleware::new()
258+
.with_keyring(self.keyring.to_provider())
259+
.with_only_authenticated(self.only_authenticated),
260+
);
247261

248262
client.build()
249263
}

0 commit comments

Comments
 (0)