diff --git a/crates/uv-auth/src/keyring.rs b/crates/uv-auth/src/keyring.rs index 4ee9e0ba5..e5969c438 100644 --- a/crates/uv-auth/src/keyring.rs +++ b/crates/uv-auth/src/keyring.rs @@ -84,6 +84,9 @@ impl KeyringProvider { return Ok(false); }; + // Ensure we strip credentials from the URL before storing + let url = url.without_credentials(); + match &self.backend { KeyringProviderBackend::Native => { self.store_native(url.as_str(), username, password).await?; @@ -116,6 +119,9 @@ impl KeyringProvider { /// Only [`KeyringProviderBackend::Native`] is supported at this time. #[instrument(skip_all, fields(url = % url.to_string(), username))] pub async fn remove(&self, url: &DisplaySafeUrl, username: &str) -> Result<(), Error> { + // Ensure we strip credentials from the URL before storing + let url = url.without_credentials(); + match &self.backend { KeyringProviderBackend::Native => { self.remove_native(url.as_str(), username).await?; diff --git a/crates/uv/src/commands/auth/login.rs b/crates/uv/src/commands/auth/login.rs index a0eff9b96..da1b2d66e 100644 --- a/crates/uv/src/commands/auth/login.rs +++ b/crates/uv/src/commands/auth/login.rs @@ -19,24 +19,6 @@ pub(crate) async fn login( preview: Preview, ) -> Result { let url = service.url(); - let display_url = username - .as_ref() - .map(|username| format!("{username}@{url}")) - .unwrap_or_else(|| url.to_string()); - - let username = if let Some(username) = username { - username - } else if token.is_some() { - String::from("__token__") - } else { - let term = Term::stderr(); - if term.is_term() { - let prompt = "username: "; - uv_console::username(prompt, &term)? - } else { - bail!("No username provided; did you mean to provide `--username` or `--token`?"); - } - }; // Be helpful about incompatible `keyring-provider` settings let Some(keyring_provider) = &keyring_provider else { @@ -55,28 +37,85 @@ pub(crate) async fn login( } }; - // FIXME: It would be preferable to accept the value of --password or --token - // from stdin, perhaps checking here for `-` as an indicator to read stdin. We - // could then warn if the password is provided as a plaintext argument. - let password = if let Some(password) = password { - password - } else if let Some(token) = token { - token + // Extract credentials from URL if present + let url_credentials = Credentials::from_url(url); + let url_username = url_credentials.as_ref().and_then(|c| c.username()); + let url_password = url_credentials.as_ref().and_then(|c| c.password()); + + let username = match (username, url_username) { + (Some(cli), Some(url)) => { + bail!( + "Cannot specify a username both via the URL and CLI; found `--username {cli}` and `{url}`" + ); + } + (Some(cli), None) => Some(cli), + (None, Some(url)) => Some(url.to_string()), + (None, None) => { + // When using `--token`, we'll use a `__token__` placeholder username + if token.is_some() { + Some("__token__".to_string()) + } else { + None + } + } + }; + + // Ensure that a username is not provided when using a token + if token.is_some() { + if let Some(username) = &username { + if username != "__token__" { + bail!("When using `--token`, a username cannot not be provided; found: {username}"); + } + } + } + + // Prompt for a username if not provided + let username = if let Some(username) = username { + username } else { let term = Term::stderr(); if term.is_term() { - let prompt = "password: "; - uv_console::password(prompt, &term)? + let prompt = "username: "; + uv_console::username(prompt, &term)? } else { - bail!("No password provided; did you mean to provide `--password` or `--token`?"); + bail!("No username provided; did you mean to provide `--username` or `--token`?"); } }; + let password = match (password, url_password, token) { + (Some(_), Some(_), _) => { + bail!("Cannot specify a password both via the URL and CLI"); + } + (Some(_), None, Some(_)) => { + bail!("Cannot specify a password via `--password` when using `--token`"); + } + (None, Some(_), Some(_)) => { + bail!("Cannot include a password in the URL when using `--token`") + } + (Some(cli), None, None) => cli, + (None, Some(url), None) => url.to_string(), + (None, None, Some(token)) => token, + (None, None, None) => { + let term = Term::stderr(); + if term.is_term() { + let prompt = "password: "; + uv_console::password(prompt, &term)? + } else { + bail!("No password provided; did you mean to provide `--password` or `--token`?"); + } + } + }; + + let display_url = if username == "__token__" { + url.without_credentials().to_string() + } else { + format!("{username}@{}", url.without_credentials()) + }; + // TODO(zanieb): Add support for other authentication schemes here, e.g., `Credentials::Bearer` let credentials = Credentials::basic(Some(username), Some(password)); provider.store(url, &credentials).await?; writeln!(printer.stderr(), "Logged in to {display_url}")?; - Ok(ExitStatus::Success) } diff --git a/crates/uv/src/commands/auth/logout.rs b/crates/uv/src/commands/auth/logout.rs index fe426f666..33eee7fa1 100644 --- a/crates/uv/src/commands/auth/logout.rs +++ b/crates/uv/src/commands/auth/logout.rs @@ -1,5 +1,6 @@ use anyhow::{Context, Result, bail}; -use std::{borrow::Cow, fmt::Write}; +use std::fmt::Write; +use uv_auth::Credentials; use uv_configuration::{KeyringProviderType, Service}; use uv_preview::Preview; @@ -16,13 +17,27 @@ pub(crate) async fn logout( preview: Preview, ) -> Result { let url = service.url(); - let display_url = username - .as_ref() - .map(|username| format!("{username}@{url}")) - .unwrap_or_else(|| url.to_string()); - let username = username - .map(Cow::Owned) - .unwrap_or(Cow::Borrowed("__token__")); + + // Extract credentials from URL if present + let url_credentials = Credentials::from_url(url); + let url_username = url_credentials.as_ref().and_then(|c| c.username()); + + let username = match (username, url_username) { + (Some(cli), Some(url)) => { + bail!( + "Cannot specify a username both via the URL and CLI; found `--username {cli}` and `{url}`" + ); + } + (Some(cli), None) => cli, + (None, Some(url)) => url.to_string(), + (None, None) => "__token__".to_string(), + }; + + let display_url = if username == "__token__" { + url.without_credentials().to_string() + } else { + format!("{username}@{}", url.without_credentials()) + }; // Unlike login, we'll default to the native provider if none is requested since it's the only // valid option and it doesn't matter if the credentials are available in subsequent commands. diff --git a/crates/uv/src/commands/auth/token.rs b/crates/uv/src/commands/auth/token.rs index e0bfb7f2d..a05464667 100644 --- a/crates/uv/src/commands/auth/token.rs +++ b/crates/uv/src/commands/auth/token.rs @@ -2,10 +2,11 @@ use std::fmt::Write; use anyhow::{Context, Result, bail}; +use uv_auth::Credentials; use uv_configuration::{KeyringProviderType, Service}; use uv_preview::Preview; -use crate::{Printer, commands::ExitStatus}; +use crate::{commands::ExitStatus, printer::Printer}; /// Show the token that will be used for a service. pub(crate) async fn token( @@ -15,6 +16,7 @@ pub(crate) async fn token( printer: Printer, preview: Preview, ) -> Result { + let url = service.url(); // Determine the keyring provider to use let Some(keyring_provider) = &keyring_provider else { bail!("Retrieving credentials requires setting a `keyring-provider`"); @@ -23,21 +25,36 @@ pub(crate) async fn token( bail!("Cannot retrieve credentials with `keyring-provider = {keyring_provider}`"); }; - let url = service.url(); - let display_url = username - .as_ref() - .map(|username| format!("{username}@{url}")) - .unwrap_or_else(|| url.to_string()); + // Extract credentials from URL if present + let url_credentials = Credentials::from_url(url); + let url_username = url_credentials.as_ref().and_then(|c| c.username()); + + let username = match (username, url_username) { + (Some(cli), Some(url)) => { + bail!( + "Cannot specify a username both via the URL and CLI; found `--username {cli}` and `{url}`" + ); + } + (Some(cli), None) => cli, + (None, Some(url)) => url.to_string(), + (None, None) => "__token__".to_string(), + }; + + let display_url = if username == "__token__" { + url.without_credentials().to_string() + } else { + format!("{username}@{}", url.without_credentials()) + }; let credentials = provider - .fetch(url, Some(username.as_deref().unwrap_or("__token__"))) + .fetch(url, Some(&username)) .await .with_context(|| format!("Failed to fetch credentials for {display_url}"))?; let Some(password) = credentials.password() else { bail!( "No {} found for {display_url}", - if username.is_some() { + if username != "__token__" { "password" } else { "token" diff --git a/crates/uv/tests/it/auth.rs b/crates/uv/tests/it/auth.rs index cbfc4c07f..9a87a1f8e 100644 --- a/crates/uv/tests/it/auth.rs +++ b/crates/uv/tests/it/auth.rs @@ -282,6 +282,41 @@ fn token_native_keyring() -> Result<()> { warning: The native keyring provider is experimental and may change without warning. Pass `--preview-features native-keyring` to disable this warning. "); + context + .auth_logout() + .arg("https://pypi-proxy.fly.dev/basic-auth/simple") + .arg("--username") + .arg("public") + .status()?; + + // Retrieve token using URL with embedded username (no --username needed) + uv_snapshot!(context.auth_token() + .arg("https://public@pypi-proxy.fly.dev/basic-auth/simple") + .arg("--keyring-provider") + .arg("native"), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to fetch credentials for public@https://pypi-proxy.fly.dev/basic-auth/simple + "); + + // Conflict between --username and URL username is rejected + uv_snapshot!(context.auth_token() + .arg("https://public@pypi-proxy.fly.dev/basic-auth/simple") + .arg("--username") + .arg("different") + .arg("--keyring-provider") + .arg("native"), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Cannot specify a username both via the URL and CLI; found `--username different` and `public` + "); + Ok(()) } @@ -299,7 +334,7 @@ fn token_subprocess_keyring() { ----- stdout ----- ----- stderr ----- - error: Failed to fetch credentials for https://****@pypi-proxy.fly.dev/basic-auth/simple + error: Failed to fetch credentials for public@https://pypi-proxy.fly.dev/basic-auth/simple " ); @@ -327,9 +362,9 @@ fn token_subprocess_keyring() { ----- stdout ----- ----- stderr ----- - Keyring request for __token__@https://public@pypi-proxy.fly.dev/basic-auth/simple - Keyring request for __token__@pypi-proxy.fly.dev - error: Failed to fetch credentials for https://****@pypi-proxy.fly.dev/basic-auth/simple + Keyring request for public@https://public@pypi-proxy.fly.dev/basic-auth/simple + Keyring request for public@pypi-proxy.fly.dev + error: Failed to fetch credentials for public@https://pypi-proxy.fly.dev/basic-auth/simple " ); @@ -341,14 +376,14 @@ fn token_subprocess_keyring() { .arg("subprocess") .env(EnvVars::KEYRING_TEST_CREDENTIALS, r#"{"pypi-proxy.fly.dev": {"public": "heron"}}"#) .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r" - success: false - exit_code: 2 + success: true + exit_code: 0 ----- stdout ----- + heron ----- stderr ----- - Keyring request for __token__@https://public@pypi-proxy.fly.dev/basic-auth/simple - Keyring request for __token__@pypi-proxy.fly.dev - error: Failed to fetch credentials for https://****@pypi-proxy.fly.dev/basic-auth/simple + Keyring request for public@https://public@pypi-proxy.fly.dev/basic-auth/simple + Keyring request for public@pypi-proxy.fly.dev " ); @@ -361,14 +396,12 @@ fn token_subprocess_keyring() { .arg("public") .env(EnvVars::KEYRING_TEST_CREDENTIALS, r#"{"pypi-proxy.fly.dev": {"public": "heron"}}"#) .env(EnvVars::PATH, venv_bin_path(&context.venv)), @r" - success: true - exit_code: 0 + success: false + exit_code: 2 ----- stdout ----- - heron ----- stderr ----- - Keyring request for public@https://public@pypi-proxy.fly.dev/basic-auth/simple - Keyring request for public@pypi-proxy.fly.dev + error: Cannot specify a username both via the URL and CLI; found `--username public` and `public` " ); } @@ -618,6 +651,62 @@ fn logout_native_keyring() -> Result<()> { Logged out of public@https://pypi-proxy.fly.dev/basic-auth/simple "); + // Login again + context + .auth_login() + .arg("https://pypi-proxy.fly.dev/basic-auth/simple") + .arg("--username") + .arg("public") + .arg("--password") + .arg("heron") + .arg("--keyring-provider") + .arg("native") + .assert() + .success(); + + // Logout with a username in the URL + uv_snapshot!(context.auth_logout() + .arg("https://public@pypi-proxy.fly.dev/basic-auth/simple") + .arg("--keyring-provider") + .arg("native"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Logged out of public@https://pypi-proxy.fly.dev/basic-auth/simple + "); + + // Conflict between --username and a URL username is rejected + uv_snapshot!(context.auth_logout() + .arg("https://public@pypi-proxy.fly.dev/basic-auth/simple") + .arg("--username") + .arg("foo") + .arg("--keyring-provider") + .arg("native"), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Cannot specify a username both via the URL and CLI; found `--username foo` and `public` + "); + + // Conflict between --token and a URL username is rejected + uv_snapshot!(context.auth_login() + .arg("https://public@pypi-proxy.fly.dev/basic-auth/simple") + .arg("--token") + .arg("foo") + .arg("--keyring-provider") + .arg("native"), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: When using `--token`, a username cannot not be provided; found: public + "); + Ok(()) } @@ -666,7 +755,7 @@ fn logout_token_native_keyring() -> Result<()> { } #[test] -fn login_url_parsing() { +fn login_native_keyring_url() { let context = TestContext::new_with_versions(&[]).with_real_home(); // A domain-only service name gets https:// prepended @@ -758,4 +847,79 @@ fn login_url_parsing() { For more information, try '--help'. "); + + // URL with embedded credentials works + uv_snapshot!(context.auth_login() + .arg("https://test:password@example.com/simple") + .arg("--keyring-provider") + .arg("native"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Logged in to test@https://example.com/simple + "); + + // URL with embedded username and separate password works + uv_snapshot!(context.auth_login() + .arg("https://test@example.com/simple") + .arg("--password") + .arg("password") + .arg("--keyring-provider") + .arg("native"), @r" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Logged in to test@https://example.com/simple + "); + + // Conflict between --username and URL username is rejected + uv_snapshot!(context.auth_login() + .arg("https://test@example.com/simple") + .arg("--username") + .arg("different") + .arg("--password") + .arg("password") + .arg("--keyring-provider") + .arg("native"), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Cannot specify a username both via the URL and CLI; found `--username different` and `test` + "); + + // Conflict between --password and URL password is rejected + uv_snapshot!(context.auth_login() + .arg("https://test:password@example.com/simple") + .arg("--password") + .arg("different") + .arg("--keyring-provider") + .arg("native"), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Cannot specify a password both via the URL and CLI + "); + + // Conflict between --token and URL credentials is rejected + uv_snapshot!(context.auth_login() + .arg("https://test:password@example.com/simple") + .arg("--token") + .arg("some-token") + .arg("--keyring-provider") + .arg("native"), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: When using `--token`, a username cannot not be provided; found: test + "); }