mirror of https://github.com/astral-sh/uv
Allow storage of multiple usernames per service in the plaintext store (#15628)
We weren't keying our hash map with the username, which meant that only one user could be used per service.
This commit is contained in:
parent
709e0ba238
commit
45946b80b0
|
|
@ -3,7 +3,7 @@ use std::sync::{Arc, LazyLock};
|
||||||
use tracing::trace;
|
use tracing::trace;
|
||||||
|
|
||||||
use cache::CredentialsCache;
|
use cache::CredentialsCache;
|
||||||
pub use credentials::Credentials;
|
pub use credentials::{Credentials, Username};
|
||||||
pub use index::{AuthPolicy, Index, Indexes};
|
pub use index::{AuthPolicy, Index, Indexes};
|
||||||
pub use keyring::KeyringProvider;
|
pub use keyring::KeyringProvider;
|
||||||
pub use middleware::AuthMiddleware;
|
pub use middleware::AuthMiddleware;
|
||||||
|
|
|
||||||
|
|
@ -210,7 +210,7 @@ struct TomlCredentials {
|
||||||
/// A credential store with a plain text storage backend.
|
/// A credential store with a plain text storage backend.
|
||||||
#[derive(Debug, Default)]
|
#[derive(Debug, Default)]
|
||||||
pub struct TextCredentialStore {
|
pub struct TextCredentialStore {
|
||||||
credentials: FxHashMap<Service, Credentials>,
|
credentials: FxHashMap<(Service, Username), Credentials>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl TextCredentialStore {
|
impl TextCredentialStore {
|
||||||
|
|
@ -246,10 +246,19 @@ impl TextCredentialStore {
|
||||||
let content = fs::read_to_string(path)?;
|
let content = fs::read_to_string(path)?;
|
||||||
let credentials: TomlCredentials = toml::from_str(&content)?;
|
let credentials: TomlCredentials = toml::from_str(&content)?;
|
||||||
|
|
||||||
let credentials: FxHashMap<Service, Credentials> = credentials
|
let credentials: FxHashMap<(Service, Username), Credentials> = credentials
|
||||||
.credentials
|
.credentials
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|credential| (credential.service.clone(), credential.credentials))
|
.map(|credential| {
|
||||||
|
let username = match &credential.credentials {
|
||||||
|
Credentials::Basic { username, .. } => username.clone(),
|
||||||
|
Credentials::Bearer { .. } => Username::none(),
|
||||||
|
};
|
||||||
|
(
|
||||||
|
(credential.service.clone(), username),
|
||||||
|
credential.credentials,
|
||||||
|
)
|
||||||
|
})
|
||||||
.collect();
|
.collect();
|
||||||
|
|
||||||
Ok(Self { credentials })
|
Ok(Self { credentials })
|
||||||
|
|
@ -278,7 +287,7 @@ impl TextCredentialStore {
|
||||||
let credentials = self
|
let credentials = self
|
||||||
.credentials
|
.credentials
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|(service, credentials)| TomlCredential {
|
.map(|((service, _username), credentials)| TomlCredential {
|
||||||
service,
|
service,
|
||||||
credentials,
|
credentials,
|
||||||
})
|
})
|
||||||
|
|
@ -307,18 +316,18 @@ impl TextCredentialStore {
|
||||||
// TODO(zanieb): Consider adding `DisplaySafeUrlRef` so we can avoid this clone
|
// TODO(zanieb): Consider adding `DisplaySafeUrlRef` so we can avoid this clone
|
||||||
// TODO(zanieb): We could also return early here if we can't normalize to a `Service`
|
// TODO(zanieb): We could also return early here if we can't normalize to a `Service`
|
||||||
if let Ok(url_service) = Service::try_from(DisplaySafeUrl::from(url.clone())) {
|
if let Ok(url_service) = Service::try_from(DisplaySafeUrl::from(url.clone())) {
|
||||||
if let Some(credential) = self.credentials.get(&url_service) {
|
if let Some(credential) = self.credentials.get(&(
|
||||||
// If a username is provided, it must match
|
url_service.clone(),
|
||||||
if username.is_none() || username == credential.username() {
|
Username::from(username.map(str::to_string)),
|
||||||
|
)) {
|
||||||
return Some(credential);
|
return Some(credential);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
// If that fails, iterate through to find a prefix match
|
// If that fails, iterate through to find a prefix match
|
||||||
let mut best: Option<(usize, &Service, &Credentials)> = None;
|
let mut best: Option<(usize, &Service, &Credentials)> = None;
|
||||||
|
|
||||||
for (service, credential) in &self.credentials {
|
for ((service, stored_username), credential) in &self.credentials {
|
||||||
let service_realm = Realm::from(service.url().deref());
|
let service_realm = Realm::from(service.url().deref());
|
||||||
|
|
||||||
// Only consider services in the same realm
|
// Only consider services in the same realm
|
||||||
|
|
@ -332,9 +341,11 @@ impl TextCredentialStore {
|
||||||
}
|
}
|
||||||
|
|
||||||
// If a username is provided, it must match
|
// If a username is provided, it must match
|
||||||
if username.is_some() && username != credential.username() {
|
if let Some(request_username) = username {
|
||||||
|
if Some(request_username) != stored_username.as_deref() {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Update our best matching credential based on prefix length
|
// Update our best matching credential based on prefix length
|
||||||
let specificity = service.url().path().len();
|
let specificity = service.url().path().len();
|
||||||
|
|
@ -353,12 +364,17 @@ impl TextCredentialStore {
|
||||||
|
|
||||||
/// Store credentials for a given service.
|
/// Store credentials for a given service.
|
||||||
pub fn insert(&mut self, service: Service, credentials: Credentials) -> Option<Credentials> {
|
pub fn insert(&mut self, service: Service, credentials: Credentials) -> Option<Credentials> {
|
||||||
self.credentials.insert(service, credentials)
|
let username = match &credentials {
|
||||||
|
Credentials::Basic { username, .. } => username.clone(),
|
||||||
|
Credentials::Bearer { .. } => Username::none(),
|
||||||
|
};
|
||||||
|
self.credentials.insert((service, username), credentials)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Remove credentials for a given service.
|
/// Remove credentials for a given service.
|
||||||
pub fn remove(&mut self, service: &Service) -> Option<Credentials> {
|
pub fn remove(&mut self, service: &Service, username: Username) -> Option<Credentials> {
|
||||||
self.credentials.remove(service)
|
// Remove the specific credential for this service and username
|
||||||
|
self.credentials.remove(&(service.clone(), username))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -419,7 +435,11 @@ mod tests {
|
||||||
assert_eq!(retrieved.username(), Some("user"));
|
assert_eq!(retrieved.username(), Some("user"));
|
||||||
assert_eq!(retrieved.password(), Some("pass"));
|
assert_eq!(retrieved.password(), Some("pass"));
|
||||||
|
|
||||||
assert!(store.remove(&service).is_some());
|
assert!(
|
||||||
|
store
|
||||||
|
.remove(&service, Username::from(Some("user".to_string())))
|
||||||
|
.is_some()
|
||||||
|
);
|
||||||
let url = Url::parse("https://example.com/").unwrap();
|
let url = Url::parse("https://example.com/").unwrap();
|
||||||
assert!(store.get_credentials(&url, None).is_none());
|
assert!(store.get_credentials(&url, None).is_none());
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -3,9 +3,8 @@ use std::fmt::Write;
|
||||||
use anyhow::{Context, Result, bail};
|
use anyhow::{Context, Result, bail};
|
||||||
use owo_colors::OwoColorize;
|
use owo_colors::OwoColorize;
|
||||||
|
|
||||||
use uv_auth::Service;
|
|
||||||
use uv_auth::store::AuthBackend;
|
use uv_auth::store::AuthBackend;
|
||||||
use uv_auth::{Credentials, TextCredentialStore};
|
use uv_auth::{Credentials, Service, TextCredentialStore, Username};
|
||||||
use uv_distribution_types::IndexUrl;
|
use uv_distribution_types::IndexUrl;
|
||||||
use uv_pep508::VerbatimUrl;
|
use uv_pep508::VerbatimUrl;
|
||||||
use uv_preview::Preview;
|
use uv_preview::Preview;
|
||||||
|
|
@ -60,7 +59,10 @@ pub(crate) async fn logout(
|
||||||
.with_context(|| format!("Unable to remove credentials for {display_url}"))?;
|
.with_context(|| format!("Unable to remove credentials for {display_url}"))?;
|
||||||
}
|
}
|
||||||
AuthBackend::TextStore(mut store, _lock) => {
|
AuthBackend::TextStore(mut store, _lock) => {
|
||||||
if store.remove(&service).is_none() {
|
if store
|
||||||
|
.remove(&service, Username::from(Some(username.clone())))
|
||||||
|
.is_none()
|
||||||
|
{
|
||||||
bail!("No matching entry found for {display_url}");
|
bail!("No matching entry found for {display_url}");
|
||||||
}
|
}
|
||||||
store
|
store
|
||||||
|
|
|
||||||
|
|
@ -1208,3 +1208,82 @@ fn token_text_store_username() {
|
||||||
"
|
"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn logout_text_store_multiple_usernames() {
|
||||||
|
let context = TestContext::new_with_versions(&[]);
|
||||||
|
|
||||||
|
// Login with two different usernames for the same service
|
||||||
|
context
|
||||||
|
.auth_login()
|
||||||
|
.arg("https://example.com/simple")
|
||||||
|
.arg("--username")
|
||||||
|
.arg("user1")
|
||||||
|
.arg("--password")
|
||||||
|
.arg("pass1")
|
||||||
|
.assert()
|
||||||
|
.success();
|
||||||
|
|
||||||
|
context
|
||||||
|
.auth_login()
|
||||||
|
.arg("https://example.com/simple")
|
||||||
|
.arg("--username")
|
||||||
|
.arg("user2")
|
||||||
|
.arg("--password")
|
||||||
|
.arg("pass2")
|
||||||
|
.assert()
|
||||||
|
.success();
|
||||||
|
|
||||||
|
// Logout one specific username
|
||||||
|
uv_snapshot!(context.auth_logout()
|
||||||
|
.arg("https://example.com/simple")
|
||||||
|
.arg("--username")
|
||||||
|
.arg("user1"), @r"
|
||||||
|
success: true
|
||||||
|
exit_code: 0
|
||||||
|
----- stdout -----
|
||||||
|
|
||||||
|
----- stderr -----
|
||||||
|
Removed credentials for user1@https://example.com/
|
||||||
|
"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Verify the first user is gone but second remains
|
||||||
|
uv_snapshot!(context.auth_token()
|
||||||
|
.arg("https://example.com/simple")
|
||||||
|
.arg("--username")
|
||||||
|
.arg("user1"), @r"
|
||||||
|
success: false
|
||||||
|
exit_code: 2
|
||||||
|
----- stdout -----
|
||||||
|
|
||||||
|
----- stderr -----
|
||||||
|
error: Failed to fetch credentials for user1@https://example.com/simple
|
||||||
|
"
|
||||||
|
);
|
||||||
|
|
||||||
|
uv_snapshot!(context.auth_token()
|
||||||
|
.arg("https://example.com/simple")
|
||||||
|
.arg("--username")
|
||||||
|
.arg("user2"), @r"
|
||||||
|
success: true
|
||||||
|
exit_code: 0
|
||||||
|
----- stdout -----
|
||||||
|
pass2
|
||||||
|
|
||||||
|
----- stderr -----
|
||||||
|
"
|
||||||
|
);
|
||||||
|
|
||||||
|
// Try to logout without specifying username (defaults to `__token__`)
|
||||||
|
uv_snapshot!(context.auth_logout()
|
||||||
|
.arg("https://example.com/simple"), @r"
|
||||||
|
success: false
|
||||||
|
exit_code: 2
|
||||||
|
----- stdout -----
|
||||||
|
|
||||||
|
----- stderr -----
|
||||||
|
error: No matching entry found for https://example.com/
|
||||||
|
"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue