Skip to content

Commit

Permalink
Include non-standard ports in keyring host queries
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Jun 5, 2024
1 parent 191f955 commit a2d9a4d
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 7 deletions.
12 changes: 9 additions & 3 deletions crates/uv-auth/src/keyring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,18 @@ impl KeyringProvider {
};
// And fallback to a check for the host
if password.is_none() {
let host = url.host_str()?;
let host = if let Some(port) = url.port() {
format!("{}:{}", url.host_str()?, port)
} else {
url.host_str()?.to_string()
};
trace!("Checking keyring for host {host}");
password = match self.backend {
KeyringProviderBackend::Subprocess => self.fetch_subprocess(host, username).await,
KeyringProviderBackend::Subprocess => self.fetch_subprocess(&host, username).await,
#[cfg(test)]
KeyringProviderBackend::Dummy(ref store) => self.fetch_dummy(store, host, username),
KeyringProviderBackend::Dummy(ref store) => {
self.fetch_dummy(store, &host, username)
}
};
}

Expand Down
79 changes: 75 additions & 4 deletions crates/uv-auth/src/middleware.rs
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,14 @@ mod tests {
AuthMiddleware::new()
.with_cache(CredentialsCache::new())
.with_keyring(Some(KeyringProvider::dummy([(
(base_url.host_str().unwrap(), username),
(
format!(
"{}:{}",
base_url.host_str().unwrap(),
base_url.port().unwrap()
),
username,
),
password,
)]))),
)
Expand Down Expand Up @@ -832,6 +839,40 @@ mod tests {
Ok(())
}

/// We include ports in keyring requests, e.g., `localhost:8000` should be distinct from `localhost`,
/// unless the server is running on a default port, e.g., `localhost:80` is equivalent to `localhost`.
/// We don't unit test the latter case because it's possible to collide with a server a developer is
/// actually running.
#[test(tokio::test)]
async fn test_keyring_includes_non_standard_port() -> Result<(), Error> {
let username = "user";
let password = "password";
let server = start_test_server(username, password).await;
let base_url = Url::parse(&server.uri())?;

let client = test_client_builder()
.with(
AuthMiddleware::new()
.with_cache(CredentialsCache::new())
.with_keyring(Some(KeyringProvider::dummy([(
// Omit the port from the keyring entry
(base_url.host_str().unwrap(), username),
password,
)]))),
)
.build();

let mut url = base_url.clone();
url.set_username(username).unwrap();
assert_eq!(
client.get(url).send().await?.status(),
401,
"We should fail because the port is not present in the keyring entry"
);

Ok(())
}

#[test(tokio::test)]
async fn test_credentials_in_keyring_seed() -> Result<(), Error> {
let username = "user";
Expand All @@ -849,7 +890,17 @@ mod tests {
);
let client = test_client_builder()
.with(AuthMiddleware::new().with_cache(cache).with_keyring(Some(
KeyringProvider::dummy([((base_url.host_str().unwrap(), username), password)]),
KeyringProvider::dummy([(
(
format!(
"{}:{}",
base_url.host_str().unwrap(),
base_url.port().unwrap()
),
username,
),
password,
)]),
)))
.build();

Expand Down Expand Up @@ -954,8 +1005,28 @@ mod tests {
AuthMiddleware::new()
.with_cache(CredentialsCache::new())
.with_keyring(Some(KeyringProvider::dummy([
((base_url_1.host_str().unwrap(), username_1), password_1),
((base_url_2.host_str().unwrap(), username_2), password_2),
(
(
format!(
"{}:{}",
base_url_1.host_str().unwrap(),
base_url_1.port().unwrap()
),
username_1,
),
password_1,
),
(
(
format!(
"{}:{}",
base_url_2.host_str().unwrap(),
base_url_2.port().unwrap()
),
username_2,
),
password_2,
),
]))),
)
.build();
Expand Down

0 comments on commit a2d9a4d

Please sign in to comment.