From 7d0f00f00d3c19cbfeb8cf1fad91df7f81c58360 Mon Sep 17 00:00:00 2001 From: dumko2001 Date: Wed, 18 Mar 2026 13:42:09 +0530 Subject: [PATCH 1/7] feat(auth): support Domain-Wide Delegation for Service Accounts (fixes #528) --- src/auth.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index b602d840..317e29d7 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -200,11 +200,20 @@ async fn get_token_inner( .map(|f| f.to_string_lossy().to_string()) .unwrap_or_else(|| "token_cache.json".to_string()); let sa_cache = token_cache_path.with_file_name(format!("sa_{tc_filename}")); - let builder = yup_oauth2::ServiceAccountAuthenticator::builder(key).with_storage( - Box::new(crate::token_storage::EncryptedTokenStorage::new(sa_cache)), - ); + + // Support Domain-Wide Delegation (impersonation) + let mut builder = yup_oauth2::ServiceAccountAuthenticator::builder(key); + if let Ok(sub) = std::env::var("GOOGLE_WORKSPACE_IMPERSONATE_USER") { + if !sub.is_empty() { + tracing::debug!(impersonate = %sub, "Using Domain-Wide Delegation"); + builder = builder.subject(sub); + } + } let auth = builder + .with_storage(Box::new(crate::token_storage::EncryptedTokenStorage::new( + sa_cache, + ))) .build() .await .context("Failed to build service account authenticator")?; From 454ad886f0bc22e3111ada0ca77e0de2a405d3f9 Mon Sep 17 00:00:00 2001 From: dumko2001 Date: Wed, 18 Mar 2026 14:54:26 +0530 Subject: [PATCH 2/7] chore: add changeset for user impersonation feature --- .changeset/feat-issue-528-impersonation.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 .changeset/feat-issue-528-impersonation.md diff --git a/.changeset/feat-issue-528-impersonation.md b/.changeset/feat-issue-528-impersonation.md new file mode 100644 index 00000000..47cedef5 --- /dev/null +++ b/.changeset/feat-issue-528-impersonation.md @@ -0,0 +1 @@ +---\n"gws": patch\n---\n\nfeat(auth): support Domain-Wide Delegation for Service Accounts From 535081a07fd6c22b6be8dcb0406e16064d6e4c15 Mon Sep 17 00:00:00 2001 From: dumko2001 Date: Wed, 18 Mar 2026 15:11:42 +0530 Subject: [PATCH 3/7] chore: correct changeset package name --- .changeset/feat-issue-528-impersonation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/feat-issue-528-impersonation.md b/.changeset/feat-issue-528-impersonation.md index 47cedef5..8133153a 100644 --- a/.changeset/feat-issue-528-impersonation.md +++ b/.changeset/feat-issue-528-impersonation.md @@ -1 +1 @@ ----\n"gws": patch\n---\n\nfeat(auth): support Domain-Wide Delegation for Service Accounts +---\n"@googleworkspace/cli": patch\n---\n\nfeat(auth): support Domain-Wide Delegation for Service Accounts From 0bca28787b2a22c88f89672e8b5716bebbdc9452 Mon Sep 17 00:00:00 2001 From: dumko2001 Date: Fri, 20 Mar 2026 00:36:18 +0530 Subject: [PATCH 4/7] fix(auth): address code review feedback for impersonation - Sanitize impersonated user email in debug log to prevent terminal escape injection - Fix critical token cache collision by using unique cache file per impersonated user (SHA-256 hash) - Add tests proving the cache collision bug and validating the fix Addresses gemini-code-assist security and critical feedback on PR #543 --- src/auth.rs | 128 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 117 insertions(+), 11 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 317e29d7..fff2121e 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -70,6 +70,32 @@ fn adc_well_known_path() -> Option { }) } +/// Computes the service account token cache path. +/// +/// When `impersonate_user` is `Some(email)`, returns a unique path using a SHA-256 hash +/// of the email to avoid token cache collision between the service account and impersonated users. +/// +/// When `impersonate_user` is `None`, returns the standard `sa_{filename}` path. +fn sa_token_cache_path(token_cache_path: &std::path::Path, impersonate_user: Option<&str>) -> std::path::PathBuf { + let tc_filename = token_cache_path + .file_name() + .map(|f| f.to_string_lossy().to_string()) + .unwrap_or_else(|| "token_cache.json".to_string()); + + if let Some(sub) = impersonate_user { + use sha2::{Sha256, Digest}; + let mut hasher = Sha256::new(); + hasher.update(sub.as_bytes()); + let hash = hasher.finalize(); + let hash_hex = format!("{:x}{:x}{:x}{:x}{:x}{:x}{:x}{:x}", + hash[0], hash[1], hash[2], hash[3], + hash[4], hash[5], hash[6], hash[7]); + token_cache_path.with_file_name(format!("sa_imp_{hash_hex}_{tc_filename}")) + } else { + token_cache_path.with_file_name(format!("sa_{tc_filename}")) + } +} + /// Types of credentials we support #[derive(Debug)] enum Credential { @@ -195,19 +221,18 @@ async fn get_token_inner( .to_string()) } Credential::ServiceAccount(key) => { - let tc_filename = token_cache_path - .file_name() - .map(|f| f.to_string_lossy().to_string()) - .unwrap_or_else(|| "token_cache.json".to_string()); - let sa_cache = token_cache_path.with_file_name(format!("sa_{tc_filename}")); - // Support Domain-Wide Delegation (impersonation) + // Read impersonation config early to determine cache file + let impersonate_user = + std::env::var("GOOGLE_WORKSPACE_IMPERSONATE_USER").ok().filter(|s| !s.is_empty()); + + // Create unique cache file for impersonation to avoid token collision + let sa_cache = sa_token_cache_path(token_cache_path, impersonate_user.as_deref()); + let mut builder = yup_oauth2::ServiceAccountAuthenticator::builder(key); - if let Ok(sub) = std::env::var("GOOGLE_WORKSPACE_IMPERSONATE_USER") { - if !sub.is_empty() { - tracing::debug!(impersonate = %sub, "Using Domain-Wide Delegation"); - builder = builder.subject(sub); - } + if let Some(sub) = impersonate_user { + tracing::debug!(impersonate = %crate::error::sanitize_for_terminal(&sub), "Using Domain-Wide Delegation"); + builder = builder.subject(sub); } let auth = builder @@ -881,4 +906,85 @@ mod tests { assert_eq!(get_quota_project(), Some("my-project-123".to_string())); } + + #[test] + fn test_sa_token_cache_path_no_impersonation() { + let cache_path = std::path::PathBuf::from("/some/path/token_cache.json"); + let result = sa_token_cache_path(&cache_path, None); + assert_eq!(result.file_name().unwrap().to_str().unwrap(), "sa_token_cache.json"); + } + + #[test] + fn test_token_cache_collision_without_fix() { + // PROOF OF BUG: The old code always used sa_{filename} regardless of impersonation. + // This caused token collision where service account tokens overwrote impersonated + // user tokens (and vice versa) when using the same scopes. + // + // Simulating OLD buggy behavior: + let cache_path = std::path::PathBuf::from("/some/path/token_cache.json"); + let old_buggy_sa_cache = cache_path.with_file_name(format!( + "sa_{}", + cache_path.file_name().unwrap().to_str().unwrap() + )); + + // OLD code: impersonation was ignored for cache filename + let old_with_impersonation = old_buggy_sa_cache.clone(); + let old_without_impersonation = old_buggy_sa_cache.clone(); + + // BUG: Both paths are IDENTICAL - tokens would collide! + assert_eq!( + old_with_impersonation, old_without_impersonation, + "BUG: Without fix, impersonated and non-impersonated use SAME cache file" + ); + + // FIX: The new sa_token_cache_path() produces different paths + let fixed_impersonated = sa_token_cache_path(&cache_path, Some("admin@example.com")); + let fixed_non_impersonated = sa_token_cache_path(&cache_path, None); + + // FIXED: Paths are different - no more collision + assert_ne!( + fixed_impersonated, fixed_non_impersonated, + "FIXED: Impersonated and non-impersonated now use DIFFERENT cache files" + ); + + // Verify the fix uses a hash-based filename for impersonation + let impersonated_name = fixed_impersonated.file_name().unwrap().to_str().unwrap(); + assert!( + impersonated_name.starts_with("sa_imp_"), + "Impersonated cache should have unique prefix, got: {}", + impersonated_name + ); + } + + #[test] + fn test_sa_token_cache_path_with_impersonation() { + let cache_path = std::path::PathBuf::from("/some/path/token_cache.json"); + let result = sa_token_cache_path(&cache_path, Some("admin@example.com")); + let filename = result.file_name().unwrap().to_str().unwrap(); + assert!(filename.starts_with("sa_imp_"), "Expected sa_imp_ prefix, got: {}", filename); + assert!(filename.contains("token_cache.json"), "Expected token_cache.json in filename, got: {}", filename); + } + + #[test] + fn test_sa_token_cache_path_impersonation_deterministic() { + let cache_path = std::path::PathBuf::from("/some/path/token_cache.json"); + let result1 = sa_token_cache_path(&cache_path, Some("admin@example.com")); + let result2 = sa_token_cache_path(&cache_path, Some("admin@example.com")); + assert_eq!(result1, result2, "Same email should produce same cache path"); + } + + #[test] + fn test_sa_token_cache_path_different_emails_different_paths() { + let cache_path = std::path::PathBuf::from("/some/path/token_cache.json"); + let result1 = sa_token_cache_path(&cache_path, Some("user1@example.com")); + let result2 = sa_token_cache_path(&cache_path, Some("user2@example.com")); + assert_ne!(result1, result2, "Different emails should produce different cache paths"); + } + + #[test] + fn test_sa_token_cache_path_preserves_parent_path() { + let cache_path = std::path::PathBuf::from("/some/path/token_cache.json"); + let result = sa_token_cache_path(&cache_path, Some("admin@example.com")); + assert_eq!(result.parent().unwrap(), std::path::Path::new("/some/path")); + } } From 8dff02314e687fad32233e667fefd1d4a3574ce3 Mon Sep 17 00:00:00 2001 From: dumko2001 Date: Fri, 20 Mar 2026 00:41:33 +0530 Subject: [PATCH 5/7] fix(auth): use full SHA-256 hash for cache file uniqueness Use all 32 bytes of SHA-256 hash for maximum collision resistance as suggested in code review feedback. --- src/auth.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index fff2121e..62868b81 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -87,9 +87,7 @@ fn sa_token_cache_path(token_cache_path: &std::path::Path, impersonate_user: Opt let mut hasher = Sha256::new(); hasher.update(sub.as_bytes()); let hash = hasher.finalize(); - let hash_hex = format!("{:x}{:x}{:x}{:x}{:x}{:x}{:x}{:x}", - hash[0], hash[1], hash[2], hash[3], - hash[4], hash[5], hash[6], hash[7]); + let hash_hex: String = hash.iter().map(|b| format!("{:02x}", b)).collect(); token_cache_path.with_file_name(format!("sa_imp_{hash_hex}_{tc_filename}")) } else { token_cache_path.with_file_name(format!("sa_{tc_filename}")) From 2c83428acf58fe4f4114f1d38d8111d602ed8a12 Mon Sep 17 00:00:00 2001 From: dumko2001 Date: Fri, 20 Mar 2026 00:46:55 +0530 Subject: [PATCH 6/7] fix(auth): cleanup sa_imp_*.json files on decryption failure When encrypted credentials become undecryptable (e.g., key migration), clean up all impersonation token cache files (sa_imp_*.json) in addition to standard token caches to prevent stale file warnings. --- src/auth.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/auth.rs b/src/auth.rs index 62868b81..35e9d867 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -337,6 +337,26 @@ async fn load_credentials_inner( } } } + // Clean up any sa_imp_*.json files (hash-based impersonation caches). + if let Some(parent) = enc_path.parent() { + if let Ok(mut entries) = tokio::fs::read_dir(parent).await { + while let Some(entry) = entries.next_entry().await.transpose() { + if let Ok(filename) = entry.map(|e| e.file_name().to_string_lossy().to_string()) { + if filename.starts_with("sa_imp_") && filename.ends_with(".json") { + let path = parent.join(&filename); + if let Err(err) = tokio::fs::remove_file(&path).await { + if err.kind() != std::io::ErrorKind::NotFound { + eprintln!( + "Warning: failed to remove stale impersonation cache '{}': {err}", + path.display() + ); + } + } + } + } + } + } + } // Fall through to remaining credential sources below. } } From a32bb8de6bfcfa3b417eead375f54a0b6247e1f4 Mon Sep 17 00:00:00 2001 From: dumko2001 Date: Fri, 20 Mar 2026 00:51:39 +0530 Subject: [PATCH 7/7] fix(auth): sanitize error messages in token cache cleanup Sanitize path and error strings in eprintln! calls during token cache cleanup to prevent terminal escape sequence injection. --- src/auth.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 35e9d867..8a4a2e08 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -331,8 +331,9 @@ async fn load_credentials_inner( if let Err(err) = tokio::fs::remove_file(&path).await { if err.kind() != std::io::ErrorKind::NotFound { eprintln!( - "Warning: failed to remove stale token cache '{}': {err}", - path.display() + "Warning: failed to remove stale token cache '{}': {}", + crate::error::sanitize_for_terminal(&path.display().to_string()), + crate::error::sanitize_for_terminal(&err.to_string()) ); } } @@ -347,8 +348,9 @@ async fn load_credentials_inner( if let Err(err) = tokio::fs::remove_file(&path).await { if err.kind() != std::io::ErrorKind::NotFound { eprintln!( - "Warning: failed to remove stale impersonation cache '{}': {err}", - path.display() + "Warning: failed to remove stale impersonation cache '{}': {}", + crate::error::sanitize_for_terminal(&path.display().to_string()), + crate::error::sanitize_for_terminal(&err.to_string()) ); } }