diff --git a/.changeset/fix-auth-legacy-credentials.md b/.changeset/fix-auth-legacy-credentials.md new file mode 100644 index 0000000..374a84f --- /dev/null +++ b/.changeset/fix-auth-legacy-credentials.md @@ -0,0 +1,10 @@ +--- +"@googleworkspace/cli": patch +--- + +fix: resolve 401 errors for legacy credentials and improve auth export command + +- Fixed `resolve_account()` rejecting legacy `credentials.enc` when no account registry exists, causing silent 401 errors on all commands +- Credential loading errors are now logged to stderr instead of silently discarded +- `gws auth export` now supports `--account EMAIL` for multi-account setups +- Documented `--unmasked` and `--account` flags in export help text diff --git a/src/auth.rs b/src/auth.rs index d895f26..5208485 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -94,9 +94,10 @@ pub async fn get_token(scopes: &[&str], account: Option<&str>) -> anyhow::Result /// Resolve which account to use: /// 1. Explicit `account` parameter takes priority. /// 2. Fall back to `accounts.json` default. -/// 3. If no registry exists but legacy `credentials.enc` exists, fail with upgrade message. +/// 3. If no registry exists but legacy `credentials.enc` exists, return None +/// so the caller falls back to the legacy credential path. /// 4. If nothing exists, return None (will fall through to standard error). -fn resolve_account(account: Option<&str>) -> anyhow::Result> { +pub fn resolve_account(account: Option<&str>) -> anyhow::Result> { let registry = crate::accounts::load_accounts()?; match (account, ®istry) { @@ -131,20 +132,8 @@ fn resolve_account(account: Option<&str>) -> anyhow::Result> { ); } } - // No account, no registry — check for legacy credentials - (None, None) => { - let legacy_path = credential_store::encrypted_credentials_path(); - if legacy_path.exists() { - anyhow::bail!( - "Legacy credentials found at {}. \ - gws now supports multiple accounts. \ - Please run 'gws auth login' to upgrade your credentials.", - legacy_path.display() - ); - } - // No registry, no legacy — fall through to standard credential loading - Ok(None) - } + // No account, no registry — fall through to legacy credential path + (None, None) => Ok(None), } } @@ -454,4 +443,28 @@ mod tests { .to_string() .contains("No credentials found")); } + + #[test] + #[serial_test::serial] + fn resolve_account_no_registry_no_legacy_returns_none() { + // When no accounts.json and no legacy credentials exist, returns Ok(None) + let result = resolve_account(None); + // On a CI/test machine with no gws config, this should return Ok(None). + // On a dev machine with an accounts.json, it may return Ok(Some(...)). + // Either way it must not error. + assert!(result.is_ok()); + } + + #[test] + #[serial_test::serial] + fn resolve_account_explicit_unknown_account_errors() { + // Requesting a specific account that doesn't exist should error + let result = resolve_account(Some("nonexistent@example.com")); + assert!(result.is_err()); + let msg = result.unwrap_err().to_string(); + assert!( + msg.contains("not found"), + "Expected 'not found' in error, got: {msg}" + ); + } } diff --git a/src/auth_commands.rs b/src/auth_commands.rs index d559d94..cfda538 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -141,7 +141,9 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { " setup Configure GCP project + OAuth client (requires gcloud)\n", " --project Use a specific GCP project\n", " status Show current authentication state\n", - " export Print decrypted credentials to stdout\n", + " export Print decrypted credentials to stdout (masked by default)\n", + " --unmasked Show full unmasked credential values\n", + " --account EMAIL Export credentials for a specific account\n", " logout Clear saved credentials and token cache\n", " --account EMAIL Logout a specific account (otherwise: all)\n", " list List all registered accounts\n", @@ -160,8 +162,14 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { "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 + let sub = &args[1..]; + let unmasked = sub.iter().any(|a| a == "--unmasked"); + let account = sub + .iter() + .position(|a| a == "--account") + .and_then(|i| sub.get(i + 1)) + .map(|s| s.as_str()); + handle_export(unmasked, account).await } "logout" => handle_logout(&args[1..]), "list" => handle_list(), @@ -423,15 +431,23 @@ 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(unmasked: bool, account: Option<&str>) -> Result<(), GwsError> { + // Resolve account: explicit flag → registry default → legacy path + 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}"); @@ -1765,4 +1781,63 @@ mod tests { "https://www.googleapis.com/auth/chat.messages" )); } + + // ── mask_secret tests ─────────────────────────────────────────────── + + #[test] + fn mask_secret_long_string_shows_prefix_and_suffix() { + let masked = mask_secret("abcdefghijklmnop"); + assert_eq!(masked, "abcd...mnop"); + } + + #[test] + fn mask_secret_short_string_fully_masked() { + let masked = mask_secret("abcd1234"); + assert_eq!(masked, "***"); + } + + #[test] + fn mask_secret_empty_string_fully_masked() { + let masked = mask_secret(""); + assert_eq!(masked, "***"); + } + + // ── export subcommand tests ───────────────────────────────────────── + + #[tokio::test] + async fn handle_export_subcommand_parses_unmasked_flag() { + // Verify export subcommand recognises --unmasked without crashing + // (will fail with "No encrypted credentials" which is expected) + let args = vec!["export".to_string(), "--unmasked".to_string()]; + let result = handle_auth_command(&args).await; + // Should error about missing credentials, not about arg parsing + assert!(result.is_err()); + match result.unwrap_err() { + GwsError::Auth(msg) => assert!( + msg.contains("No encrypted credentials") || msg.contains("resolve"), + "Unexpected error: {msg}" + ), + other => panic!("Expected Auth error, got: {other:?}"), + } + } + + #[tokio::test] + async fn handle_export_subcommand_parses_account_flag() { + // Verify export subcommand recognises --account without crashing + let args = vec![ + "export".to_string(), + "--account".to_string(), + "test@example.com".to_string(), + ]; + let result = handle_auth_command(&args).await; + assert!(result.is_err()); + // Should error about the account, not about arg parsing + match result.unwrap_err() { + GwsError::Auth(msg) => assert!( + msg.contains("not found") || msg.contains("No encrypted"), + "Unexpected error: {msg}" + ), + other => panic!("Expected Auth error, got: {other:?}"), + } + } } diff --git a/src/main.rs b/src/main.rs index 4497731..d24f459 100644 --- a/src/main.rs +++ b/src/main.rs @@ -235,10 +235,14 @@ async fn run() -> Result<(), GwsError> { // Get scopes from the method let scopes: Vec<&str> = method.scopes.iter().map(|s| s.as_str()).collect(); - // Authenticate: try OAuth, otherwise proceed unauthenticated + // Authenticate: try OAuth, otherwise report the error so users can diagnose let (token, auth_method) = match auth::get_token(&scopes, account.as_deref()).await { Ok(t) => (Some(t), executor::AuthMethod::OAuth), - Err(_) => (None, executor::AuthMethod::None), + Err(e) => { + eprintln!("Warning: failed to load credentials: {e}"); + eprintln!("Proceeding without authentication. Run `gws auth login` to authenticate."); + (None, executor::AuthMethod::None) + } }; // Execute