Skip to content

feat(auth): support Domain-Wide Delegation for Service Accounts #543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dumko2001 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(auth): support Domain-Wide Delegation for Service Accounts #543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d0f00f
feat(auth): support Domain-Wide Delegation for Service Accounts (fixe…
dumko2001 Mar 18, 2026
454ad88
chore: add changeset for user impersonation feature
dumko2001 Mar 18, 2026
535081a
chore: correct changeset package name
dumko2001 Mar 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .changeset/feat-issue-528-impersonation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
---\n"@googleworkspace/cli": patch\n---\n\nfeat(auth): support Domain-Wide Delegation for Service Accounts
15 changes: 12 additions & 3 deletions src/auth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The sub variable, read from the GOOGLE_WORKSPACE_IMPERSONATE_USER environment variable, is logged without proper sanitization. This can lead to a terminal escape sequence injection vulnerability if the environment variable contains malicious control characters. While the repository's general rule specifically mentions 'error strings', the principle of sanitizing any untrusted input printed to the terminal should be applied to prevent this security risk.

Suggested change
tracing::debug!(impersonate = %sub, "Using Domain-Wide Delegation");
tracing::debug!(impersonate = %sub.chars().flat_map(|c| c.escape_default()).collect::<String>(), "Using Domain-Wide Delegation");
References
  1. Sanitize error strings printed to the terminal to prevent escape sequence injection.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The sub variable, which is read from the GOOGLE_WORKSPACE_IMPERSONATE_USER environment variable, is logged directly without sanitization. This is a security risk as it can lead to terminal escape sequence injection if the logs are viewed on a terminal. An attacker could use this to hide malicious activity or execute commands on the user's terminal.

Please sanitize the user-provided input before logging it. The project already has a utility for this: crate::error::sanitize_for_terminal.

Suggested change
tracing::debug!(impersonate = %sub, "Using Domain-Wide Delegation");
tracing::debug!(impersonate = %crate::error::sanitize_for_terminal(&sub), "Using Domain-Wide Delegation");
References
  1. Sanitize error strings printed to the terminal to prevent escape sequence injection.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The user-provided sub value from the GOOGLE_WORKSPACE_IMPERSONATE_USER environment variable is logged directly using Display formatting (%sub). This could allow a malicious user to inject terminal escape sequences into the logs, which can lead to various security issues if the logs are ever displayed on a terminal. To prevent this, you should use Debug formatting (?sub) which will properly escape the string. This aligns with the general rule to sanitize strings printed to the terminal.

Suggested change
tracing::debug!(impersonate = %sub, "Using Domain-Wide Delegation");
tracing::debug!(impersonate = ?sub, "Using Domain-Wide Delegation");
References
  1. Sanitize error strings printed to the terminal to prevent escape sequence injection.

builder = builder.subject(sub);
}
}
Comment on lines +206 to +211
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There is a critical token cache collision issue. The token cache for service accounts is keyed by scopes, but the cache filename is the same whether impersonation is used or not (sa_token_cache.json). This means a token for the service account itself will be overwritten by a token for an impersonated user (and vice-versa) if they use the same scopes.

This will lead to incorrect authentication, causing either permission errors or actions being performed as the wrong identity.

To fix this, you should use a different cache file when impersonation is active. I recommend modifying the sa_cache path to include a hash of the impersonated user's email. This change would be on line 202, which is outside the current diff, but is essential to fix this bug.

For example:

// (on line 202, which is outside the diff)
let impersonate_user = std::env::var("GOOGLE_WORKSPACE_IMPERSONATE_USER").ok().filter(|s| !s.is_empty());

let cache_key_suffix = if let Some(sub) = &impersonate_user {
    // Use a hash to create a unique, filename-safe suffix.
    // You'll need to add a dependency like `sha256`.
    use sha256::digest;
    let hash = digest(sub.as_bytes());
    format!("_impersonate_{}", &hash[..12])
} else {
    String::new()
};

let sa_cache = token_cache_path.with_file_name(format!("sa{}_{tc_filename}", cache_key_suffix));

// ... then later use `impersonate_user` to set the subject
if let Some(sub) = impersonate_user {
    // ...
}


let auth = builder
.with_storage(Box::new(crate::token_storage::EncryptedTokenStorage::new(
sa_cache,
)))
.build()
.await
.context("Failed to build service account authenticator")?;
Expand Down
Loading