From 4f709ac3d8eafbb667ea3c4a39929c42f82fc490 Mon Sep 17 00:00:00 2001 From: Frank Date: Fri, 6 Mar 2026 09:07:47 +0800 Subject: [PATCH 1/3] fix(auth): resolve per-account credentials in `auth export` (#179) `auth export` only checked the legacy `credentials.enc` path, which no longer exists after multi-account login (credentials are stored as `credentials..enc`). Use `resolve_account` to find the correct per-account credential file, with support for `--account` flag. --- .changeset/fix-auth-export-multi-account.md | 5 ++ src/auth.rs | 2 +- src/auth_commands.rs | 92 ++++++++++++++++++--- src/main.rs | 2 +- 4 files changed, 86 insertions(+), 15 deletions(-) create mode 100644 .changeset/fix-auth-export-multi-account.md diff --git a/.changeset/fix-auth-export-multi-account.md b/.changeset/fix-auth-export-multi-account.md new file mode 100644 index 0000000..9ab45d3 --- /dev/null +++ b/.changeset/fix-auth-export-multi-account.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": patch +--- + +Fix `auth export` to resolve per-account credentials instead of only checking the legacy `credentials.enc` path diff --git a/src/auth.rs b/src/auth.rs index 97a1839..d583b2c 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -126,7 +126,7 @@ pub async fn get_token(scopes: &[&str], account: Option<&str>) -> anyhow::Result /// 1. Explicit `account` parameter takes priority. /// 2. Fall back to `accounts.json` default. /// 3. If no registry exists, return None to allow legacy `credentials.enc` fallthrough. -fn resolve_account(account: Option<&str>) -> anyhow::Result> { +pub(crate) fn resolve_account(account: Option<&str>) -> anyhow::Result> { let registry = crate::accounts::load_accounts()?; match (account, ®istry) { diff --git a/src/auth_commands.rs b/src/auth_commands.rs index c9b12e1..28ef1e5 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -129,7 +129,10 @@ fn token_cache_path() -> PathBuf { } /// Handle `gws auth `. -pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { +pub async fn handle_auth_command( + args: &[String], + global_account: Option<&str>, +) -> Result<(), GwsError> { const USAGE: &str = concat!( "Usage: gws auth [options]\n\n", " login Authenticate via OAuth2 (opens browser)\n", @@ -144,6 +147,8 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { " --project Use a specific GCP project\n", " status Show current authentication state\n", " export Print decrypted credentials to stdout\n", + " --account EMAIL Export a specific account's credentials\n", + " --unmasked Show secrets without masking\n", " logout Clear saved credentials and token cache\n", " --account EMAIL Logout a specific account (otherwise: all)\n", " list List all registered accounts\n", @@ -161,10 +166,7 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { "login" => handle_login(&args[1..]).await, "setup" => crate::setup::run_setup(&args[1..]).await, "status" => handle_status().await, - "export" => { - let unmasked = args.len() > 1 && args[1] == "--unmasked"; - handle_export(unmasked).await - } + "export" => handle_export(&args[1..], global_account).await, "logout" => handle_logout(&args[1..]), "list" => handle_list(), "default" => handle_default(&args[1..]), @@ -463,15 +465,40 @@ async fn fetch_userinfo_email(access_token: &str) -> Option { .map(|s| s.to_string()) } -async fn handle_export(unmasked: bool) -> Result<(), GwsError> { - let enc_path = credential_store::encrypted_credentials_path(); +async fn handle_export(args: &[String], global_account: Option<&str>) -> Result<(), GwsError> { + // Parse --unmasked and --account from args + let mut unmasked = false; + let mut local_account: Option = None; + let mut i = 0; + while i < args.len() { + if args[i] == "--unmasked" { + unmasked = true; + } else if args[i] == "--account" && i + 1 < args.len() { + local_account = Some(args[i + 1].clone()); + i += 1; + } else if let Some(value) = args[i].strip_prefix("--account=") { + local_account = Some(value.to_string()); + } + i += 1; + } + + // Resolve account: local --account > global --account > default account + let account = local_account.as_deref().or(global_account); + let resolved = + crate::auth::resolve_account(account).map_err(|e| GwsError::Auth(e.to_string()))?; + + let enc_path = match &resolved { + Some(email) => credential_store::encrypted_credentials_path_for(email), + None => credential_store::encrypted_credentials_path(), + }; + if !enc_path.exists() { return Err(GwsError::Auth( "No encrypted credentials found. Run 'gws auth login' first.".to_string(), )); } - match credential_store::load_encrypted() { + match credential_store::load_encrypted_from_path(&enc_path) { Ok(contents) => { if unmasked { println!("{contents}"); @@ -1740,7 +1767,7 @@ mod tests { #[tokio::test] async fn handle_auth_command_empty_args_prints_usage() { let args: Vec = vec![]; - let result = handle_auth_command(&args).await; + let result = handle_auth_command(&args, None).await; // Empty args now prints usage and returns Ok assert!(result.is_ok()); } @@ -1748,21 +1775,21 @@ mod tests { #[tokio::test] async fn handle_auth_command_help_flag_returns_ok() { let args = vec!["--help".to_string()]; - let result = handle_auth_command(&args).await; + let result = handle_auth_command(&args, None).await; assert!(result.is_ok()); } #[tokio::test] async fn handle_auth_command_help_short_flag_returns_ok() { let args = vec!["-h".to_string()]; - let result = handle_auth_command(&args).await; + let result = handle_auth_command(&args, None).await; assert!(result.is_ok()); } #[tokio::test] async fn handle_auth_command_invalid_subcommand() { let args = vec!["frobnicate".to_string()]; - let result = handle_auth_command(&args).await; + let result = handle_auth_command(&args, None).await; assert!(result.is_err()); match result.unwrap_err() { GwsError::Validation(msg) => assert!(msg.contains("frobnicate")), @@ -1815,7 +1842,7 @@ mod tests { async fn handle_status_succeeds_without_credentials() { // status should always succeed and report "none" let args = vec!["status".to_string()]; - let result = handle_auth_command(&args).await; + let result = handle_auth_command(&args, None).await; assert!(result.is_ok()); } @@ -2184,4 +2211,43 @@ mod tests { // Exactly 9 chars — first 4 + last 4 with "..." in between assert_eq!(mask_secret("123456789"), "1234...6789"); } + + #[tokio::test] + async fn handle_export_nonexistent_account_returns_auth_error() { + // Requesting a non-existent account should always fail + let args = vec![ + "--account".to_string(), + "nonexistent@example.com".to_string(), + ]; + let result = handle_export(&args, None).await; + assert!(result.is_err()); + match result.unwrap_err() { + GwsError::Auth(_) => {} // expected + other => panic!("Expected Auth error, got: {other:?}"), + } + } + + #[tokio::test] + async fn handle_export_global_account_nonexistent_returns_auth_error() { + // Global --account with non-existent email should fail + let args: Vec = vec![]; + let result = handle_export(&args, Some("nonexistent@example.com")).await; + assert!(result.is_err()); + match result.unwrap_err() { + GwsError::Auth(_) => {} // expected + other => panic!("Expected Auth error, got: {other:?}"), + } + } + + #[tokio::test] + async fn handle_export_dispatch_nonexistent_account() { + // Verify the dispatch path passes global_account through + let args = vec!["export".to_string()]; + let result = handle_auth_command(&args, Some("nonexistent@example.com")).await; + assert!(result.is_err()); + match result.unwrap_err() { + GwsError::Auth(_) => {} // expected + other => panic!("Expected Auth error, got: {other:?}"), + } + } } diff --git a/src/main.rs b/src/main.rs index 6940238..7c5dc66 100644 --- a/src/main.rs +++ b/src/main.rs @@ -134,7 +134,7 @@ async fn run() -> Result<(), GwsError> { // Handle the `auth` command if first_arg == "auth" { let auth_args: Vec = args.iter().skip(2).cloned().collect(); - return auth_commands::handle_auth_command(&auth_args).await; + return auth_commands::handle_auth_command(&auth_args, account.as_deref()).await; } // Handle the `mcp` command From e96a21da422d9b7ff6b063084e5aba7ec166a5ca Mon Sep 17 00:00:00 2001 From: Frank Date: Fri, 6 Mar 2026 09:15:55 +0800 Subject: [PATCH 2/3] refactor(auth): use iterator for export arg parsing with validation Reject unknown arguments and missing --account values instead of silently ignoring them, as suggested in PR review. --- src/auth_commands.rs | 54 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 28ef1e5..fc7774a 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -469,17 +469,29 @@ async fn handle_export(args: &[String], global_account: Option<&str>) -> Result< // Parse --unmasked and --account from args let mut unmasked = false; let mut local_account: Option = None; - let mut i = 0; - while i < args.len() { - if args[i] == "--unmasked" { - unmasked = true; - } else if args[i] == "--account" && i + 1 < args.len() { - local_account = Some(args[i + 1].clone()); - i += 1; - } else if let Some(value) = args[i].strip_prefix("--account=") { - local_account = Some(value.to_string()); + let mut args_iter = args.iter(); + while let Some(arg) = args_iter.next() { + match arg.as_str() { + "--unmasked" => unmasked = true, + "--account" => { + if let Some(val) = args_iter.next() { + local_account = Some(val.clone()); + } else { + return Err(GwsError::Validation( + "The --account flag requires a value.".to_string(), + )); + } + } + _ => { + if let Some(value) = arg.strip_prefix("--account=") { + local_account = Some(value.to_string()); + } else { + return Err(GwsError::Validation(format!( + "Unknown argument for export: '{arg}'" + ))); + } + } } - i += 1; } // Resolve account: local --account > global --account > default account @@ -2250,4 +2262,26 @@ mod tests { other => panic!("Expected Auth error, got: {other:?}"), } } + + #[tokio::test] + async fn handle_export_unknown_arg_returns_validation_error() { + let args = vec!["--unmask".to_string()]; + let result = handle_export(&args, None).await; + assert!(result.is_err()); + match result.unwrap_err() { + GwsError::Validation(msg) => assert!(msg.contains("--unmask")), + other => panic!("Expected Validation error, got: {other:?}"), + } + } + + #[tokio::test] + async fn handle_export_account_missing_value_returns_validation_error() { + let args = vec!["--account".to_string()]; + let result = handle_export(&args, None).await; + assert!(result.is_err()); + match result.unwrap_err() { + GwsError::Validation(msg) => assert!(msg.contains("requires a value")), + other => panic!("Expected Validation error, got: {other:?}"), + } + } } From 00a15a1f213e4f054f070c7310ef819dd0f23de8 Mon Sep 17 00:00:00 2001 From: Frank Date: Fri, 6 Mar 2026 09:26:01 +0800 Subject: [PATCH 3/3] refactor(auth): use peekable iterator to prevent flag misinterpretation Prevent `--account --unmasked` from treating `--unmasked` as the account value by peeking at the next arg and rejecting flag-like values. --- src/auth_commands.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index fc7774a..482a533 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -469,19 +469,20 @@ async fn handle_export(args: &[String], global_account: Option<&str>) -> Result< // Parse --unmasked and --account from args let mut unmasked = false; let mut local_account: Option = None; - let mut args_iter = args.iter(); + let mut args_iter = args.iter().peekable(); while let Some(arg) = args_iter.next() { match arg.as_str() { "--unmasked" => unmasked = true, - "--account" => { - if let Some(val) = args_iter.next() { - local_account = Some(val.clone()); - } else { + "--account" => match args_iter.peek() { + Some(val) if !val.starts_with('-') => { + local_account = Some(args_iter.next().unwrap().clone()); + } + _ => { return Err(GwsError::Validation( "The --account flag requires a value.".to_string(), )); } - } + }, _ => { if let Some(value) = arg.strip_prefix("--account=") { local_account = Some(value.to_string()); @@ -2284,4 +2285,16 @@ mod tests { other => panic!("Expected Validation error, got: {other:?}"), } } + + #[tokio::test] + async fn handle_export_account_flag_as_value_returns_validation_error() { + // --account followed by another flag should not treat the flag as a value + let args = vec!["--account".to_string(), "--unmasked".to_string()]; + let result = handle_export(&args, None).await; + assert!(result.is_err()); + match result.unwrap_err() { + GwsError::Validation(msg) => assert!(msg.contains("requires a value")), + other => panic!("Expected Validation error, got: {other:?}"), + } + } }