Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions crates/uv-auth/src/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,14 +803,18 @@ impl AuthMiddleware {
// Text credential store support.
} else if let Some(credentials) = self.text_store.get().await.and_then(|text_store| {
debug!("Checking text store for credentials for {url}");
text_store
.get_credentials(
url,
credentials
.as_ref()
.and_then(|credentials| credentials.username()),
)
.cloned()
match text_store.get_credentials(
url,
credentials
.as_ref()
.and_then(|credentials| credentials.username()),
) {
Ok(credentials) => credentials.cloned(),
Err(err) => {
debug!("Failed to get credentials from text store: {err}");
None
}
}
}) {
debug!("Found credentials in plaintext store for {url}");
Some(credentials)
Expand Down
81 changes: 59 additions & 22 deletions crates/uv-auth/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ pub enum BearerAuthError {
UnexpectedPassword,
}

#[derive(Debug, Error, PartialEq)]
pub enum LookupError {
#[error("Multiple credentials found for URL '{0}', specify which username to use")]
AmbiguousUsername(DisplaySafeUrl),
}

/// A single credential entry in a TOML credentials file.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(try_from = "TomlCredentialWire", into = "TomlCredentialWire")]
Expand Down Expand Up @@ -334,7 +340,7 @@ impl TextCredentialStore {
&self,
url: &DisplaySafeUrl,
username: Option<&str>,
) -> Option<&Credentials> {
) -> Result<Option<&Credentials>, LookupError> {
let request_realm = Realm::from(url);

// Perform an exact lookup first
Expand All @@ -345,7 +351,7 @@ impl TextCredentialStore {
url_service.clone(),
Username::from(username.map(str::to_string)),
)) {
return Some(credential);
return Ok(Some(credential));
}
}

Expand Down Expand Up @@ -376,15 +382,17 @@ impl TextCredentialStore {
let specificity = service.url().path().len();
if best.is_none_or(|(best_specificity, _, _)| specificity > best_specificity) {
best = Some((specificity, service, credential));
} else if best.is_some_and(|(best_specificity, _, _)| specificity == best_specificity) {
return Err(LookupError::AmbiguousUsername(url.clone()));
}
}

// Return the most specific match
if let Some((_, _, credential)) = best {
return Some(credential);
return Ok(Some(credential));
}

None
Ok(None)
}

/// Store credentials for a given service.
Expand Down Expand Up @@ -455,10 +463,10 @@ mod tests {
let service = Service::from_str("https://example.com").unwrap();
store.insert(service.clone(), credentials.clone());
let url = DisplaySafeUrl::parse("https://example.com/").unwrap();
assert!(store.get_credentials(&url, None).is_some());
assert!(store.get_credentials(&url, None).unwrap().is_some());

let url = DisplaySafeUrl::parse("https://example.com/path").unwrap();
let retrieved = store.get_credentials(&url, None).unwrap();
let retrieved = store.get_credentials(&url, None).unwrap().unwrap();
assert_eq!(retrieved.username(), Some("user"));
assert_eq!(retrieved.password(), Some("pass"));

Expand All @@ -468,7 +476,7 @@ mod tests {
.is_some()
);
let url = DisplaySafeUrl::parse("https://example.com/").unwrap();
assert!(store.get_credentials(&url, None).is_none());
assert!(store.get_credentials(&url, None).unwrap().is_none());
}

#[tokio::test]
Expand All @@ -494,12 +502,12 @@ password = "pass2"
let store = TextCredentialStore::from_file(temp_file.path()).unwrap();

let url = DisplaySafeUrl::parse("https://example.com/").unwrap();
assert!(store.get_credentials(&url, None).is_some());
assert!(store.get_credentials(&url, None).unwrap().is_some());
let url = DisplaySafeUrl::parse("https://test.org/").unwrap();
assert!(store.get_credentials(&url, None).is_some());
assert!(store.get_credentials(&url, None).unwrap().is_some());

let url = DisplaySafeUrl::parse("https://example.com").unwrap();
let cred = store.get_credentials(&url, None).unwrap();
let cred = store.get_credentials(&url, None).unwrap().unwrap();
assert_eq!(cred.username(), Some("testuser"));
assert_eq!(cred.password(), Some("testpass"));

Expand Down Expand Up @@ -535,7 +543,7 @@ password = "pass2"

for url_str in matching_urls {
let url = DisplaySafeUrl::parse(url_str).unwrap();
let cred = store.get_credentials(&url, None);
let cred = store.get_credentials(&url, None).unwrap();
assert!(cred.is_some(), "Failed to match URL with prefix: {url_str}");
}

Expand All @@ -548,7 +556,7 @@ password = "pass2"

for url_str in non_matching_urls {
let url = DisplaySafeUrl::parse(url_str).unwrap();
let cred = store.get_credentials(&url, None);
let cred = store.get_credentials(&url, None).unwrap();
assert!(cred.is_none(), "Should not match non-prefix URL: {url_str}");
}
}
Expand All @@ -572,7 +580,7 @@ password = "pass2"

for url_str in matching_urls {
let url = DisplaySafeUrl::parse(url_str).unwrap();
let cred = store.get_credentials(&url, None);
let cred = store.get_credentials(&url, None).unwrap();
assert!(
cred.is_some(),
"Failed to match URL in same realm: {url_str}"
Expand All @@ -588,7 +596,7 @@ password = "pass2"

for url_str in non_matching_urls {
let url = DisplaySafeUrl::parse(url_str).unwrap();
let cred = store.get_credentials(&url, None);
let cred = store.get_credentials(&url, None).unwrap();
assert!(
cred.is_none(),
"Should not match URL in different realm: {url_str}"
Expand All @@ -612,12 +620,12 @@ password = "pass2"

// Should match the most specific prefix
let url = DisplaySafeUrl::parse("https://example.com/api/v1/users").unwrap();
let cred = store.get_credentials(&url, None).unwrap();
let cred = store.get_credentials(&url, None).unwrap().unwrap();
assert_eq!(cred.username(), Some("specific"));

// Should match the general prefix for non-specific paths
let url = DisplaySafeUrl::parse("https://example.com/api/v2").unwrap();
let cred = store.get_credentials(&url, None).unwrap();
let cred = store.get_credentials(&url, None).unwrap().unwrap();
assert_eq!(cred.username(), Some("general"));
}

Expand All @@ -630,17 +638,17 @@ password = "pass2"
store.insert(service.clone(), user1_creds.clone());

// Should return credentials when username matches
let result = store.get_credentials(&url, Some("user1"));
let result = store.get_credentials(&url, Some("user1")).unwrap();
assert!(result.is_some());
assert_eq!(result.unwrap().username(), Some("user1"));
assert_eq!(result.unwrap().password(), Some("pass1"));

// Should not return credentials when username doesn't match
let result = store.get_credentials(&url, Some("user2"));
let result = store.get_credentials(&url, Some("user2")).unwrap();
assert!(result.is_none());

// Should return credentials when no username is specified
let result = store.get_credentials(&url, None);
let result = store.get_credentials(&url, None).unwrap();
assert!(result.is_some());
assert_eq!(result.unwrap().username(), Some("user1"));
}
Expand Down Expand Up @@ -668,21 +676,50 @@ password = "pass2"
let url = DisplaySafeUrl::parse("https://example.com/api/v1/users").unwrap();

// Should match specific credentials when username matches
let result = store.get_credentials(&url, Some("specific_user"));
let result = store.get_credentials(&url, Some("specific_user")).unwrap();
assert!(result.is_some());
assert_eq!(result.unwrap().username(), Some("specific_user"));

// Should match the general credentials when requesting general_user (falls back to less specific prefix)
let result = store.get_credentials(&url, Some("general_user"));
let result = store.get_credentials(&url, Some("general_user")).unwrap();
assert!(
result.is_some(),
"Should match general_user from less specific prefix"
);
assert_eq!(result.unwrap().username(), Some("general_user"));

// Should match most specific when no username specified
let result = store.get_credentials(&url, None);
let result = store.get_credentials(&url, None).unwrap();
assert!(result.is_some());
assert_eq!(result.unwrap().username(), Some("specific_user"));
}

#[test]
fn test_ambiguous_username_error() {
let mut store = TextCredentialStore::default();

// Add two credentials for the same service with different usernames
let service = Service::from_str("https://example.com/api").unwrap();
let user1_creds = Credentials::basic(Some("user1".to_string()), Some("pass1".to_string()));
let user2_creds = Credentials::basic(Some("user2".to_string()), Some("pass2".to_string()));

store.insert(service.clone(), user1_creds);
store.insert(service.clone(), user2_creds);

let url = DisplaySafeUrl::parse("https://example.com/api/v1").unwrap();

// When no username is specified, should return an error because there are multiple matches with same specificity
let result = store.get_credentials(&url, None);
assert!(result.is_err());
assert_eq!(result, Err(LookupError::AmbiguousUsername(url.clone())));

// When a specific username is provided, should return the correct credentials
let result = store.get_credentials(&url, Some("user1")).unwrap();
assert!(result.is_some());
assert_eq!(result.unwrap().username(), Some("user1"));

let result = store.get_credentials(&url, Some("user2")).unwrap();
assert!(result.is_some());
assert_eq!(result.unwrap().username(), Some("user2"));
}
}
2 changes: 1 addition & 1 deletion crates/uv/src/commands/auth/helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ async fn credentials_for_url(
let backend = AuthBackend::from_settings(preview).await?;
let credentials = match &backend {
AuthBackend::System(provider) => provider.fetch(url, username).await,
AuthBackend::TextStore(store, _lock) => store.get_credentials(url, username).cloned(),
AuthBackend::TextStore(store, _lock) => store.get_credentials(url, username)?.cloned(),
};
Ok(credentials)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/auth/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub(crate) async fn token(
.await
.ok_or_else(|| anyhow::anyhow!("Failed to fetch credentials for {display_url}"))?,
AuthBackend::TextStore(store, _lock) => store
.get_credentials(url, Some(&username))
.get_credentials(url, Some(&username))?
.cloned()
.ok_or_else(|| anyhow::anyhow!("Failed to fetch credentials for {display_url}"))?,
};
Expand Down
Loading