Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Include non-standard ports in keyring host queries #4061

Merged
merged 1 commit into from
Jun 7, 2024
Merged

Include non-standard ports in keyring host queries #4061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 9 additions & 3 deletions crates/uv-auth/src/keyring.rs
View file
Open in desktop
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
103 changes: 97 additions & 6 deletions crates/uv-auth/src/middleware.rs
View file
Open in desktop
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 Expand Up @@ -1188,8 +1259,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
Loading