diff --git a/.changeset/auth-login-open-browser.md b/.changeset/auth-login-open-browser.md new file mode 100644 index 000000000..dcc4c6e33 --- /dev/null +++ b/.changeset/auth-login-open-browser.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": minor +--- + +Open OAuth login URLs in the browser with a copy-paste fallback diff --git a/crates/google-workspace-cli/src/auth.rs b/crates/google-workspace-cli/src/auth.rs index 9d8847e4b..656d07673 100644 --- a/crates/google-workspace-cli/src/auth.rs +++ b/crates/google-workspace-cli/src/auth.rs @@ -763,8 +763,11 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let enc_path = dir.path().join("credentials.enc"); - // Isolate global config dir to prevent races with other tests - std::env::set_var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR", dir.path()); + // Isolate global config dir to prevent races with other tests. The + // guard restores the variable on drop; a bare set_var leaked it into + // every later test, and config_dir_returns_gws_subdir then read a + // dropped tempdir path instead of the real default. + let _config_dir = EnvVarGuard::set("GOOGLE_WORKSPACE_CLI_CONFIG_DIR", dir.path()); // Encrypt and write let encrypted = crate::credential_store::encrypt(json.as_bytes()).unwrap(); diff --git a/crates/google-workspace-cli/src/auth_commands.rs b/crates/google-workspace-cli/src/auth_commands.rs index d7571e747..e7cbc2ac3 100644 --- a/crates/google-workspace-cli/src/auth_commands.rs +++ b/crates/google-workspace-cli/src/auth_commands.rs @@ -13,9 +13,11 @@ // limitations under the License. use std::collections::HashSet; +use std::ffi::OsString; use std::io::{BufRead, BufReader, Write}; use std::net::TcpListener; use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; use serde::Deserialize; use serde_json::json; @@ -88,6 +90,62 @@ fn build_proxy_auth_url(client_id: &str, redirect_uri: &str, scopes: &[String]) ) } +fn browser_command(browser: Option, os: &str) -> Option { + browser.or_else(|| match os { + "linux" => Some(OsString::from("xdg-open")), + "macos" => Some(OsString::from("open")), + // `explorer` avoids cmd.exe URL parsing and the EDR-sensitive rundll32 opener. + "windows" => Some(OsString::from("explorer")), + _ => None, + }) +} + +fn is_openable_url(url: &str) -> bool { + url.starts_with("https://") + && !url.chars().any(|c| { + c.is_control() + || c.is_whitespace() + || crate::output::is_dangerous_unicode(c) + || matches!(c, '"' | '\'' | '\\' | ';' | '$' | '|' | '`' | '<' | '>') + }) +} + +/// Attempt to open an OAuth URL without blocking or affecting the login flow. +/// +/// Returns whether an opener was actually spawned, so the caller only claims +/// a browser is opening when one is. The child is reaped in a background +/// thread, so a slow platform opener cannot block the callback server. +fn try_open_browser(url: &str) -> bool { + if !is_openable_url(url) { + return false; + } + + let Some(program) = browser_command(std::env::var_os("BROWSER"), std::env::consts::OS) else { + return false; + }; + + match Command::new(program) + .arg(url) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + { + Ok(mut child) => { + // Builder::spawn returns Err instead of panicking when the OS + // refuses a thread; the unreaped child then lingers only until + // this short-lived CLI exits, and the login is unaffected. + let _ = std::thread::Builder::new() + .name("gws-browser-reaper".to_string()) + .spawn(move || { + let _ = child.wait(); + }); + true + } + Err(_) => false, + } +} + fn extract_authorization_code(request_line: &str) -> Result { let path = request_line .split_whitespace() @@ -126,6 +184,9 @@ async fn login_with_proxy_support( let auth_url = build_proxy_auth_url(client_id, &redirect_uri, scopes); + if try_open_browser(&auth_url) { + println!("Opening in your browser..."); + } println!("Open this URL in your browser to authenticate:\n"); println!(" {}\n", auth_url); @@ -565,6 +626,9 @@ impl yup_oauth2::authenticator_delegate::InstalledFlowDelegate for CliFlowDelega } else { url.to_string() }; + if try_open_browser(&display_url) { + eprintln!("Opening in your browser..."); + } eprintln!("Open this URL in your browser to authenticate:\n"); eprintln!(" {display_url}\n"); Ok(String::new()) @@ -2481,6 +2545,68 @@ mod tests { assert!(result.is_empty()); } + #[test] + fn browser_command_prefers_browser_env() { + // $BROWSER is a single command, not a shell line; arguments are not split. + let browser = std::ffi::OsString::from("/opt/bin/my-browser"); + + let command = browser_command(Some(browser.clone()), "linux"); + + assert_eq!(command, Some(browser)); + } + + #[test] + fn browser_command_uses_xdg_open_on_linux() { + let command = browser_command(None, "linux"); + + assert_eq!(command, Some(std::ffi::OsString::from("xdg-open"))); + } + + #[test] + fn browser_command_uses_open_on_macos() { + let command = browser_command(None, "macos"); + + assert_eq!(command, Some(std::ffi::OsString::from("open"))); + } + + #[test] + fn browser_command_uses_explorer_on_windows() { + let command = browser_command(None, "windows"); + + assert_eq!(command, Some(std::ffi::OsString::from("explorer"))); + } + + #[test] + fn browser_command_rejects_unknown_platform() { + let command = browser_command(None, "freebsd"); + + assert_eq!(command, None); + } + + #[test] + fn openable_url_accepts_https_url() { + assert!(is_openable_url( + "https://accounts.google.com/o/oauth2/auth?scope=openid" + )); + } + + #[test] + fn openable_url_rejects_control_whitespace_and_dangerous_unicode() { + assert!(!is_openable_url("https://example.com/with space")); + assert!(!is_openable_url("https://example.com/with\nnewline")); + assert!(!is_openable_url("https://example.com/\u{202E}override")); + assert!(!is_openable_url("https://example.com/zero\u{200B}width")); + } + + #[test] + fn openable_url_rejects_quotes_backslashes_and_shell_characters() { + assert!(!is_openable_url("https://example.com/\"")); + assert!(!is_openable_url("https://example.com/'")); + assert!(!is_openable_url("https://example.com/\\")); + assert!(!is_openable_url("https://example.com/;evil")); + assert!(!is_openable_url("https://example.com/$(evil)")); + } + #[test] fn build_proxy_auth_url_encodes_scope_and_redirect_uri() { let scopes = vec![ diff --git a/crates/google-workspace-cli/src/helpers/script.rs b/crates/google-workspace-cli/src/helpers/script.rs index 11bcdebec..4b31db62d 100644 --- a/crates/google-workspace-cli/src/helpers/script.rs +++ b/crates/google-workspace-cli/src/helpers/script.rs @@ -169,13 +169,7 @@ fn process_file(path: &Path) -> Result, GwsError> { filename.trim_end_matches(".js").trim_end_matches(".gs"), ), "html" => ("HTML", filename.trim_end_matches(".html")), - "json" => { - if filename == "appsscript.json" { - ("JSON", "appsscript") - } else { - return Ok(None); - } - } + "json" if filename == "appsscript.json" => ("JSON", "appsscript"), _ => return Ok(None), };