From abcb2fbe2ac75e9f3f760b719604f6f0c4184154 Mon Sep 17 00:00:00 2001 From: Varun Nuthalapati Date: Sun, 31 May 2026 20:15:05 -0700 Subject: [PATCH 1/8] fix(executor): skip x-goog-user-project header for OAuth auth method --- .changeset/fix-oauth-quota-project-header.md | 5 ++ crates/google-workspace-cli/src/executor.rs | 48 ++++++++++++++++++-- 2 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 .changeset/fix-oauth-quota-project-header.md diff --git a/.changeset/fix-oauth-quota-project-header.md b/.changeset/fix-oauth-quota-project-header.md new file mode 100644 index 000000000..604dd5aee --- /dev/null +++ b/.changeset/fix-oauth-quota-project-header.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": patch +--- + +Skip x-goog-user-project header for OAuth auth to fix 403 errors for non-project-member users diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs index 46f31ac4b..80fbc6945 100644 --- a/crates/google-workspace-cli/src/executor.rs +++ b/crates/google-workspace-cli/src/executor.rs @@ -187,9 +187,12 @@ async fn build_http_request( } } - // Set quota project from ADC for billing/quota attribution - if let Some(quota_project) = crate::auth::get_quota_project() { - request = request.header("x-goog-user-project", quota_project); + // Only send quota project for ADC/service-account auth; OAuth users are not + // necessarily IAM members of the project, so the header causes 403 errors. + if *auth_method != AuthMethod::OAuth { + if let Some(quota_project) = crate::auth::get_quota_project() { + request = request.header("x-goog-user-project", quota_project); + } } let mut all_query_params = input.query_params.clone(); @@ -2399,3 +2402,42 @@ async fn test_get_does_not_set_content_length_zero() { "GET with no body should not have Content-Length header" ); } + +#[tokio::test] +async fn test_oauth_auth_does_not_set_quota_project_header() { + // Arrange: even if get_quota_project() would return a value, OAuth requests + // must NOT send x-goog-user-project because OAuth users are not necessarily + // IAM members of the project and the header would trigger 403 errors. + let client = reqwest::Client::new(); + let method = RestMethod { + http_method: "GET".to_string(), + path: "files".to_string(), + ..Default::default() + }; + let input = ExecutionInput { + full_url: "https://example.com/files".to_string(), + body: None, + params: Map::new(), + query_params: Vec::new(), + is_upload: false, + }; + + let request = build_http_request( + &client, + &method, + &input, + Some("fake-token"), + &AuthMethod::OAuth, + None, + 0, + &None, + ) + .await + .unwrap(); + + let built = request.build().unwrap(); + assert!( + built.headers().get("x-goog-user-project").is_none(), + "OAuth requests must not include x-goog-user-project header" + ); +} From 52cff70ef9b72bb7fef4d0f371e10a0cbbe119c4 Mon Sep 17 00:00:00 2001 From: Varun Nuthalapati Date: Sun, 31 May 2026 21:22:14 -0700 Subject: [PATCH 2/8] fix(executor): distinguish service-account auth to correctly gate quota header AuthMethod::OAuth covers both user OAuth and service-account credentials, so the previous check (*auth_method != AuthMethod::OAuth) would incorrectly suppress the x-goog-user-project header for service accounts (which do need it) while also setting it on unauthenticated (None) requests. Add AuthMethod::ServiceAccount and auth::CredentialKind so the call site in main.rs can tag the request with the right variant. The quota header is now only sent for ServiceAccount auth; user OAuth requests remain header-free to avoid 403 errors for users who are not IAM members of the project. --- crates/google-workspace-cli/src/auth.rs | 33 +++++++++++++++++++++ crates/google-workspace-cli/src/executor.rs | 12 ++++---- crates/google-workspace-cli/src/main.rs | 10 +++++-- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/crates/google-workspace-cli/src/auth.rs b/crates/google-workspace-cli/src/auth.rs index 9d8847e4b..2fc1127de 100644 --- a/crates/google-workspace-cli/src/auth.rs +++ b/crates/google-workspace-cli/src/auth.rs @@ -126,6 +126,15 @@ fn adc_well_known_path() -> Option { }) } +/// What kind of credential provided the token. +#[derive(Debug, Clone, PartialEq)] +pub enum CredentialKind { + /// Browser-based OAuth 2.0 user credential (from `gws auth login`) + UserOAuth, + /// Service-account key credential + ServiceAccount, +} + /// Types of credentials we support #[derive(Debug)] enum Credential { @@ -229,6 +238,30 @@ pub async fn get_token(scopes: &[&str]) -> anyhow::Result { get_token_inner(scopes, creds, &token_cache).await } +/// Like [`get_token`] but also returns the [`CredentialKind`] so callers can +/// decide whether to include the `x-goog-user-project` quota header. +pub async fn get_token_with_kind(scopes: &[&str]) -> anyhow::Result<(String, CredentialKind)> { + if let Ok(token) = std::env::var("GOOGLE_WORKSPACE_CLI_TOKEN") { + if !token.is_empty() { + return Ok((token, CredentialKind::UserOAuth)); + } + } + + let creds_file = std::env::var("GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE").ok(); + let config_dir = crate::auth_commands::config_dir(); + let enc_path = credential_store::encrypted_credentials_path(); + let default_path = config_dir.join("credentials.json"); + let token_cache = config_dir.join("token_cache.json"); + + let creds = load_credentials_inner(creds_file.as_deref(), &enc_path, &default_path).await?; + let kind = match &creds { + Credential::ServiceAccount(_) => CredentialKind::ServiceAccount, + Credential::AuthorizedUser(_) => CredentialKind::UserOAuth, + }; + let token = get_token_inner(scopes, creds, &token_cache).await?; + Ok((token, kind)) +} + /// Check if HTTP proxy environment variables are set pub(crate) fn has_proxy_env() -> bool { PROXY_ENV_VARS diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs index 80fbc6945..17c68769b 100644 --- a/crates/google-workspace-cli/src/executor.rs +++ b/crates/google-workspace-cli/src/executor.rs @@ -34,8 +34,10 @@ use crate::output::sanitize_for_terminal; /// Tracks what authentication method was used for the request. #[derive(Debug, Clone, PartialEq)] pub enum AuthMethod { - /// OAuth2 bearer token from credentials file + /// OAuth2 bearer token from a user credential (`gws auth login`) OAuth, + /// Bearer token from a service-account key + ServiceAccount, /// No authentication was provided None, } @@ -182,14 +184,14 @@ async fn build_http_request( }; if let Some(token) = token { - if *auth_method == AuthMethod::OAuth { + if matches!(*auth_method, AuthMethod::OAuth | AuthMethod::ServiceAccount) { request = request.bearer_auth(token); } } - // Only send quota project for ADC/service-account auth; OAuth users are not - // necessarily IAM members of the project, so the header causes 403 errors. - if *auth_method != AuthMethod::OAuth { + // Only send quota project for service-account auth; OAuth users may not be + // IAM members of the project, so the header would trigger 403 errors. + if *auth_method == AuthMethod::ServiceAccount { if let Some(quota_project) = crate::auth::get_quota_project() { request = request.header("x-goog-user-project", quota_project); } diff --git a/crates/google-workspace-cli/src/main.rs b/crates/google-workspace-cli/src/main.rs index 41dcc1e1f..620aefb2d 100644 --- a/crates/google-workspace-cli/src/main.rs +++ b/crates/google-workspace-cli/src/main.rs @@ -262,8 +262,14 @@ async fn run() -> Result<(), GwsError> { let scopes: Vec<&str> = select_scope(&method.scopes).into_iter().collect(); // Authenticate: try OAuth, fail with error if credentials exist but are broken - let (token, auth_method) = match auth::get_token(&scopes).await { - Ok(t) => (Some(t), executor::AuthMethod::OAuth), + let (token, auth_method) = match auth::get_token_with_kind(&scopes).await { + Ok((t, kind)) => { + let method = match kind { + auth::CredentialKind::ServiceAccount => executor::AuthMethod::ServiceAccount, + auth::CredentialKind::UserOAuth => executor::AuthMethod::OAuth, + }; + (Some(t), method) + } Err(e) => { // If credentials were found but failed (e.g. decryption error, invalid token), // propagate the error instead of silently falling back to unauthenticated. From be15ebc58d74f6b4f0bd7efdb1eedbd5a6f7730e Mon Sep 17 00:00:00 2001 From: Varun Nuthalapati Date: Mon, 1 Jun 2026 09:23:32 -0700 Subject: [PATCH 3/8] refactor(auth): move AuthMethod to auth.rs and re-export in executor.rs Replace the separate CredentialKind enum with the existing AuthMethod enum (moved from executor.rs to auth.rs so authentication types live with authentication code). get_token_with_kind now returns AuthMethod directly, eliminating the mapping boilerplate in main.rs. Resolves Gemini r2 comment on PR #827. --- crates/google-workspace-cli/src/auth.rs | 24 +++++++++++---------- crates/google-workspace-cli/src/executor.rs | 11 +--------- crates/google-workspace-cli/src/main.rs | 8 +------ 3 files changed, 15 insertions(+), 28 deletions(-) diff --git a/crates/google-workspace-cli/src/auth.rs b/crates/google-workspace-cli/src/auth.rs index 2fc1127de..66f8bdf46 100644 --- a/crates/google-workspace-cli/src/auth.rs +++ b/crates/google-workspace-cli/src/auth.rs @@ -126,13 +126,15 @@ fn adc_well_known_path() -> Option { }) } -/// What kind of credential provided the token. -#[derive(Debug, Clone, PartialEq)] -pub enum CredentialKind { - /// Browser-based OAuth 2.0 user credential (from `gws auth login`) - UserOAuth, - /// Service-account key credential +/// Tracks what authentication method was used for the request. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AuthMethod { + /// OAuth2 bearer token from a user credential (`gws auth login`) + OAuth, + /// Bearer token from a service-account key ServiceAccount, + /// No authentication was provided + None, } /// Types of credentials we support @@ -238,12 +240,12 @@ pub async fn get_token(scopes: &[&str]) -> anyhow::Result { get_token_inner(scopes, creds, &token_cache).await } -/// Like [`get_token`] but also returns the [`CredentialKind`] so callers can +/// Like [`get_token`] but also returns the [`AuthMethod`] so callers can /// decide whether to include the `x-goog-user-project` quota header. -pub async fn get_token_with_kind(scopes: &[&str]) -> anyhow::Result<(String, CredentialKind)> { +pub async fn get_token_with_kind(scopes: &[&str]) -> anyhow::Result<(String, AuthMethod)> { if let Ok(token) = std::env::var("GOOGLE_WORKSPACE_CLI_TOKEN") { if !token.is_empty() { - return Ok((token, CredentialKind::UserOAuth)); + return Ok((token, AuthMethod::OAuth)); } } @@ -255,8 +257,8 @@ pub async fn get_token_with_kind(scopes: &[&str]) -> anyhow::Result<(String, Cre let creds = load_credentials_inner(creds_file.as_deref(), &enc_path, &default_path).await?; let kind = match &creds { - Credential::ServiceAccount(_) => CredentialKind::ServiceAccount, - Credential::AuthorizedUser(_) => CredentialKind::UserOAuth, + Credential::ServiceAccount(_) => AuthMethod::ServiceAccount, + Credential::AuthorizedUser(_) => AuthMethod::OAuth, }; let token = get_token_inner(scopes, creds, &token_cache).await?; Ok((token, kind)) diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs index 17c68769b..feac04d67 100644 --- a/crates/google-workspace-cli/src/executor.rs +++ b/crates/google-workspace-cli/src/executor.rs @@ -31,16 +31,7 @@ use crate::discovery::{RestDescription, RestMethod}; use crate::error::GwsError; use crate::output::sanitize_for_terminal; -/// Tracks what authentication method was used for the request. -#[derive(Debug, Clone, PartialEq)] -pub enum AuthMethod { - /// OAuth2 bearer token from a user credential (`gws auth login`) - OAuth, - /// Bearer token from a service-account key - ServiceAccount, - /// No authentication was provided - None, -} +pub use crate::auth::AuthMethod; /// Source for media upload content. /// diff --git a/crates/google-workspace-cli/src/main.rs b/crates/google-workspace-cli/src/main.rs index 620aefb2d..5e9fb10c1 100644 --- a/crates/google-workspace-cli/src/main.rs +++ b/crates/google-workspace-cli/src/main.rs @@ -263,13 +263,7 @@ async fn run() -> Result<(), GwsError> { // Authenticate: try OAuth, fail with error if credentials exist but are broken let (token, auth_method) = match auth::get_token_with_kind(&scopes).await { - Ok((t, kind)) => { - let method = match kind { - auth::CredentialKind::ServiceAccount => executor::AuthMethod::ServiceAccount, - auth::CredentialKind::UserOAuth => executor::AuthMethod::OAuth, - }; - (Some(t), method) - } + Ok((t, method)) => (Some(t), method), Err(e) => { // If credentials were found but failed (e.g. decryption error, invalid token), // propagate the error instead of silently falling back to unauthenticated. From edc245f6a25ad1afed4b3f857544dc9c54cb2653 Mon Sep 17 00:00:00 2001 From: Varun Nuthalapati Date: Tue, 2 Jun 2026 08:31:45 -0700 Subject: [PATCH 4/8] fix(executor): honour GOOGLE_WORKSPACE_PROJECT_ID for OAuth when explicitly set The previous fix completely omitted x-goog-user-project for OAuth, which broke the documented GOOGLE_WORKSPACE_PROJECT_ID env var behaviour. Users who explicitly set that variable expect the header to be forwarded. New behaviour: - ServiceAccount: always send the header (env var, config, or ADC) - OAuth: only send when GOOGLE_WORKSPACE_PROJECT_ID is explicitly set - None: never send Also serialise the two env-var-mutating tests with a static Mutex to avoid races when the test binary runs threads in parallel. --- crates/google-workspace-cli/src/executor.rs | 77 ++++++++++++++++++--- 1 file changed, 66 insertions(+), 11 deletions(-) diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs index feac04d67..da50766b6 100644 --- a/crates/google-workspace-cli/src/executor.rs +++ b/crates/google-workspace-cli/src/executor.rs @@ -180,12 +180,19 @@ async fn build_http_request( } } - // Only send quota project for service-account auth; OAuth users may not be - // IAM members of the project, so the header would trigger 403 errors. - if *auth_method == AuthMethod::ServiceAccount { - if let Some(quota_project) = crate::auth::get_quota_project() { - request = request.header("x-goog-user-project", quota_project); - } + // For service-account auth, always forward the quota project (env var, config, or ADC). + // For OAuth, only send when GOOGLE_WORKSPACE_PROJECT_ID is explicitly set — the user + // has opted in, so we honour it even though OAuth users may not be IAM members of every + // project. Omit the header entirely when neither condition is met to avoid 403 errors. + let quota_project = match auth_method { + AuthMethod::ServiceAccount => crate::auth::get_quota_project(), + AuthMethod::OAuth => std::env::var("GOOGLE_WORKSPACE_PROJECT_ID") + .ok() + .filter(|s| !s.is_empty()), + AuthMethod::None => None, + }; + if let Some(quota_project) = quota_project { + request = request.header("x-goog-user-project", quota_project); } let mut all_query_params = input.query_params.clone(); @@ -2396,11 +2403,16 @@ async fn test_get_does_not_set_content_length_zero() { ); } +/// Mutex to serialise tests that mutate GOOGLE_WORKSPACE_PROJECT_ID so they +/// don't race with each other when the test binary runs its threads in parallel. +static QUOTA_PROJECT_ENV_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(()); + #[tokio::test] -async fn test_oauth_auth_does_not_set_quota_project_header() { - // Arrange: even if get_quota_project() would return a value, OAuth requests - // must NOT send x-goog-user-project because OAuth users are not necessarily - // IAM members of the project and the header would trigger 403 errors. +async fn test_oauth_auth_does_not_set_quota_project_header_by_default() { + let _guard = QUOTA_PROJECT_ENV_MUTEX.lock().unwrap(); + // Without GOOGLE_WORKSPACE_PROJECT_ID set, OAuth requests must omit x-goog-user-project + // because OAuth users are not necessarily IAM members of the project. + std::env::remove_var("GOOGLE_WORKSPACE_PROJECT_ID"); let client = reqwest::Client::new(); let method = RestMethod { http_method: "GET".to_string(), @@ -2431,6 +2443,49 @@ async fn test_oauth_auth_does_not_set_quota_project_header() { let built = request.build().unwrap(); assert!( built.headers().get("x-goog-user-project").is_none(), - "OAuth requests must not include x-goog-user-project header" + "OAuth requests must not include x-goog-user-project header when env var is not set" + ); +} + +#[tokio::test] +async fn test_oauth_auth_sends_quota_project_when_env_var_explicitly_set() { + let _guard = QUOTA_PROJECT_ENV_MUTEX.lock().unwrap(); + // When GOOGLE_WORKSPACE_PROJECT_ID is explicitly set, OAuth requests should + // honour it and send x-goog-user-project (the user opted in). + std::env::set_var("GOOGLE_WORKSPACE_PROJECT_ID", "my-explicit-project"); + let client = reqwest::Client::new(); + let method = RestMethod { + http_method: "GET".to_string(), + path: "files".to_string(), + ..Default::default() + }; + let input = ExecutionInput { + full_url: "https://example.com/files".to_string(), + body: None, + params: Map::new(), + query_params: Vec::new(), + is_upload: false, + }; + + let request = build_http_request( + &client, + &method, + &input, + Some("fake-token"), + &AuthMethod::OAuth, + None, + 0, + &None, + ) + .await + .unwrap(); + + std::env::remove_var("GOOGLE_WORKSPACE_PROJECT_ID"); + + let built = request.build().unwrap(); + assert_eq!( + built.headers().get("x-goog-user-project").and_then(|v| v.to_str().ok()), + Some("my-explicit-project"), + "OAuth requests must include x-goog-user-project when GOOGLE_WORKSPACE_PROJECT_ID is set" ); } From 943f8adcc9b5d3f1635c017db236a093162799a7 Mon Sep 17 00:00:00 2001 From: mateusmaaia <29810529+mateusmaaia@users.noreply.github.com> Date: Fri, 3 Jul 2026 15:56:50 -0300 Subject: [PATCH 5/8] fix(executor): gate test-only mutex behind cfg(test); cargo fmt - QUOTA_PROJECT_ENV_MUTEX is only referenced from #[tokio::test] functions, which aren't compiled outside test builds, so plain `cargo clippy --workspace` (CI's lint job) flagged it as dead code. - cargo fmt line-wrap on the new test's assert_eq! chain. --- crates/google-workspace-cli/src/executor.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs index da50766b6..39ff97c70 100644 --- a/crates/google-workspace-cli/src/executor.rs +++ b/crates/google-workspace-cli/src/executor.rs @@ -2405,6 +2405,7 @@ async fn test_get_does_not_set_content_length_zero() { /// Mutex to serialise tests that mutate GOOGLE_WORKSPACE_PROJECT_ID so they /// don't race with each other when the test binary runs its threads in parallel. +#[cfg(test)] static QUOTA_PROJECT_ENV_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(()); #[tokio::test] @@ -2484,7 +2485,10 @@ async fn test_oauth_auth_sends_quota_project_when_env_var_explicitly_set() { let built = request.build().unwrap(); assert_eq!( - built.headers().get("x-goog-user-project").and_then(|v| v.to_str().ok()), + built + .headers() + .get("x-goog-user-project") + .and_then(|v| v.to_str().ok()), Some("my-explicit-project"), "OAuth requests must include x-goog-user-project when GOOGLE_WORKSPACE_PROJECT_ID is set" ); From 42e556cd48f21a5a4c061d75ac1b331313e0bb3d Mon Sep 17 00:00:00 2001 From: mateusmaaia Date: Tue, 7 Jul 2026 02:50:57 -0300 Subject: [PATCH 6/8] fix(executor): use #[serial_test::serial] for env-var tests per review feedback Replaces the local QUOTA_PROJECT_ENV_MUTEX with the #[serial_test::serial] attribute already used elsewhere in this crate for serialising tests that mutate GOOGLE_WORKSPACE_PROJECT_ID, per gemini-code-assist's review comment on PR #827/#863. The two test fns are gated with #[cfg(test)] so plain `cargo clippy --workspace` (which doesn't set cfg(test)) doesn't try to link the serial_test dev-dependency. --- crates/google-workspace-cli/src/executor.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs index 39ff97c70..eda8e6e48 100644 --- a/crates/google-workspace-cli/src/executor.rs +++ b/crates/google-workspace-cli/src/executor.rs @@ -2403,14 +2403,10 @@ async fn test_get_does_not_set_content_length_zero() { ); } -/// Mutex to serialise tests that mutate GOOGLE_WORKSPACE_PROJECT_ID so they -/// don't race with each other when the test binary runs its threads in parallel. #[cfg(test)] -static QUOTA_PROJECT_ENV_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(()); - #[tokio::test] +#[serial_test::serial] async fn test_oauth_auth_does_not_set_quota_project_header_by_default() { - let _guard = QUOTA_PROJECT_ENV_MUTEX.lock().unwrap(); // Without GOOGLE_WORKSPACE_PROJECT_ID set, OAuth requests must omit x-goog-user-project // because OAuth users are not necessarily IAM members of the project. std::env::remove_var("GOOGLE_WORKSPACE_PROJECT_ID"); @@ -2448,9 +2444,10 @@ async fn test_oauth_auth_does_not_set_quota_project_header_by_default() { ); } +#[cfg(test)] #[tokio::test] +#[serial_test::serial] async fn test_oauth_auth_sends_quota_project_when_env_var_explicitly_set() { - let _guard = QUOTA_PROJECT_ENV_MUTEX.lock().unwrap(); // When GOOGLE_WORKSPACE_PROJECT_ID is explicitly set, OAuth requests should // honour it and send x-goog-user-project (the user opted in). std::env::set_var("GOOGLE_WORKSPACE_PROJECT_ID", "my-explicit-project"); From f00494eaf45a50898fc6c70a925ed783bd817cc7 Mon Sep 17 00:00:00 2001 From: mateusmaaia Date: Tue, 7 Jul 2026 03:17:30 -0300 Subject: [PATCH 7/8] refactor(auth): dedupe get_token via get_token_with_kind; harden env-var test cleanup - get_token now delegates to get_token_with_kind and discards the AuthMethod, removing the near-duplicate credential-loading logic between the two functions (gemini-code-assist review on #870). - The two executor tests that mutate GOOGLE_WORKSPACE_PROJECT_ID now use an RAII EnvVarGuard (mirroring auth.rs's test helper) so the env var is restored even if an assertion panics mid-test. --- crates/google-workspace-cli/src/auth.rs | 16 +------- crates/google-workspace-cli/src/executor.rs | 45 +++++++++++++++++++-- 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/crates/google-workspace-cli/src/auth.rs b/crates/google-workspace-cli/src/auth.rs index 66f8bdf46..cf96ab9d5 100644 --- a/crates/google-workspace-cli/src/auth.rs +++ b/crates/google-workspace-cli/src/auth.rs @@ -223,21 +223,7 @@ impl AccessTokenProvider for FakeTokenProvider { /// - Well-known ADC path: `~/.config/gcloud/application_default_credentials.json` /// (populated by `gcloud auth application-default login`) pub async fn get_token(scopes: &[&str]) -> anyhow::Result { - // 0. Direct token from env var (highest priority, bypasses all credential loading) - if let Ok(token) = std::env::var("GOOGLE_WORKSPACE_CLI_TOKEN") { - if !token.is_empty() { - return Ok(token); - } - } - - let creds_file = std::env::var("GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE").ok(); - let config_dir = crate::auth_commands::config_dir(); - let enc_path = credential_store::encrypted_credentials_path(); - let default_path = config_dir.join("credentials.json"); - let token_cache = config_dir.join("token_cache.json"); - - let creds = load_credentials_inner(creds_file.as_deref(), &enc_path, &default_path).await?; - get_token_inner(scopes, creds, &token_cache).await + get_token_with_kind(scopes).await.map(|(token, _)| token) } /// Like [`get_token`] but also returns the [`AuthMethod`] so callers can diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs index eda8e6e48..f58538fc0 100644 --- a/crates/google-workspace-cli/src/executor.rs +++ b/crates/google-workspace-cli/src/executor.rs @@ -2403,13 +2403,52 @@ async fn test_get_does_not_set_content_length_zero() { ); } +/// RAII guard that saves the current value of an environment variable and +/// restores it when dropped, so tests that mutate it stay panic-safe. +#[cfg(test)] +struct EnvVarGuard { + name: String, + original: Option, +} + +#[cfg(test)] +impl EnvVarGuard { + fn set(name: &str, value: impl AsRef) -> Self { + let original = std::env::var_os(name); + std::env::set_var(name, value); + Self { + name: name.to_string(), + original, + } + } + + fn remove(name: &str) -> Self { + let original = std::env::var_os(name); + std::env::remove_var(name); + Self { + name: name.to_string(), + original, + } + } +} + +#[cfg(test)] +impl Drop for EnvVarGuard { + fn drop(&mut self) { + match &self.original { + Some(v) => std::env::set_var(&self.name, v), + None => std::env::remove_var(&self.name), + } + } +} + #[cfg(test)] #[tokio::test] #[serial_test::serial] async fn test_oauth_auth_does_not_set_quota_project_header_by_default() { // Without GOOGLE_WORKSPACE_PROJECT_ID set, OAuth requests must omit x-goog-user-project // because OAuth users are not necessarily IAM members of the project. - std::env::remove_var("GOOGLE_WORKSPACE_PROJECT_ID"); + let _env_guard = EnvVarGuard::remove("GOOGLE_WORKSPACE_PROJECT_ID"); let client = reqwest::Client::new(); let method = RestMethod { http_method: "GET".to_string(), @@ -2450,7 +2489,7 @@ async fn test_oauth_auth_does_not_set_quota_project_header_by_default() { async fn test_oauth_auth_sends_quota_project_when_env_var_explicitly_set() { // When GOOGLE_WORKSPACE_PROJECT_ID is explicitly set, OAuth requests should // honour it and send x-goog-user-project (the user opted in). - std::env::set_var("GOOGLE_WORKSPACE_PROJECT_ID", "my-explicit-project"); + let _env_guard = EnvVarGuard::set("GOOGLE_WORKSPACE_PROJECT_ID", "my-explicit-project"); let client = reqwest::Client::new(); let method = RestMethod { http_method: "GET".to_string(), @@ -2478,8 +2517,6 @@ async fn test_oauth_auth_sends_quota_project_when_env_var_explicitly_set() { .await .unwrap(); - std::env::remove_var("GOOGLE_WORKSPACE_PROJECT_ID"); - let built = request.build().unwrap(); assert_eq!( built From a8b854d4655d373227faabe09995b074c9364dae Mon Sep 17 00:00:00 2001 From: mateusmaaia Date: Tue, 7 Jul 2026 03:33:41 -0300 Subject: [PATCH 8/8] fix(auth): don't forward OAuth client's project_id as quota project for service accounts get_quota_project() previously fell back to project_id from the OAuth client configuration (client_secret.json) for service-account requests. That project isn't necessarily one the service account is an IAM member of, so forwarding it as x-goog-user-project reintroduces the same class of 403 insufficientPermissions error this PR fixes for OAuth. Replaces get_quota_project() with get_quota_project_for_method(AuthMethod), which only consults GOOGLE_WORKSPACE_PROJECT_ID or ADC's quota_project_id for service accounts, and never the OAuth client config. executor.rs now calls this single helper instead of duplicating the OAuth/ServiceAccount branching inline (gemini-code-assist review on #870). --- crates/google-workspace-cli/src/auth.rs | 100 +++++++++++++------- crates/google-workspace-cli/src/executor.rs | 18 ++-- 2 files changed, 70 insertions(+), 48 deletions(-) diff --git a/crates/google-workspace-cli/src/auth.rs b/crates/google-workspace-cli/src/auth.rs index cf96ab9d5..a902462b2 100644 --- a/crates/google-workspace-cli/src/auth.rs +++ b/crates/google-workspace-cli/src/auth.rs @@ -80,37 +80,40 @@ async fn refresh_token_with_reqwest( Ok(token_response.access_token) } -/// Returns the project ID to be used for quota and billing (sets the `x-goog-user-project` header). +/// Returns the project ID to be used for quota and billing (sets the `x-goog-user-project` +/// header), based on the authentication method in use. /// /// Priority: -/// 1. `GOOGLE_WORKSPACE_PROJECT_ID` environment variable. -/// 2. `project_id` from the OAuth client configuration (`client_secret.json`). -/// 3. `quota_project_id` from Application Default Credentials (ADC). -pub fn get_quota_project() -> Option { - // 1. Explicit environment variable (highest priority) +/// 1. `GOOGLE_WORKSPACE_PROJECT_ID` environment variable (explicit opt-in, any method). +/// 2. For [`AuthMethod::ServiceAccount`] only: `quota_project_id` from Application Default +/// Credentials (ADC). The OAuth client configuration (`client_secret.json`) is deliberately +/// NOT consulted here — the service account may not be an IAM member of that project. +/// 3. For [`AuthMethod::OAuth`] and [`AuthMethod::None`]: no further fallback. Omit the header +/// to avoid 403 errors for users who are not IAM members of the OAuth client's project. +pub fn get_quota_project_for_method(method: AuthMethod) -> Option { + // 1. Explicit environment variable (highest priority, any auth method) if let Ok(project_id) = std::env::var("GOOGLE_WORKSPACE_PROJECT_ID") { if !project_id.is_empty() { return Some(project_id); } } - // 2. Project ID from the OAuth client configuration (set via `gws auth setup`) - if let Ok(config) = crate::oauth_config::load_client_config() { - if !config.project_id.is_empty() { - return Some(config.project_id); + match method { + AuthMethod::ServiceAccount => { + // 2. Fallback to Application Default Credentials (ADC). Service accounts are + // members of their own project by construction, so this is safe to forward. + let path = std::env::var("GOOGLE_APPLICATION_CREDENTIALS") + .ok() + .map(PathBuf::from) + .or_else(adc_well_known_path)?; + let content = std::fs::read_to_string(path).ok()?; + let json: serde_json::Value = serde_json::from_str(&content).ok()?; + json.get("quota_project_id") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) } + AuthMethod::OAuth | AuthMethod::None => None, } - - // 3. Fallback to Application Default Credentials (ADC) - let path = std::env::var("GOOGLE_APPLICATION_CREDENTIALS") - .ok() - .map(PathBuf::from) - .or_else(adc_well_known_path)?; - let content = std::fs::read_to_string(path).ok()?; - let json: serde_json::Value = serde_json::from_str(&content).ok()?; - json.get("quota_project_id") - .and_then(|v| v.as_str()) - .map(|s| s.to_string()) } /// Returns the well-known Application Default Credentials path: @@ -946,18 +949,31 @@ mod tests { #[test] #[serial_test::serial] - fn test_get_quota_project_priority_env_var() { + fn test_get_quota_project_for_method_env_var_wins_for_service_account() { let _env_guard = EnvVarGuard::set("GOOGLE_WORKSPACE_PROJECT_ID", "priority-env"); let _adc_guard = EnvVarGuard::remove("GOOGLE_APPLICATION_CREDENTIALS"); - let _config_guard = EnvVarGuard::remove("GOOGLE_WORKSPACE_CLI_CONFIG_DIR"); let _home_guard = EnvVarGuard::set("HOME", "/missing/home"); - assert_eq!(get_quota_project(), Some("priority-env".to_string())); + assert_eq!( + get_quota_project_for_method(AuthMethod::ServiceAccount), + Some("priority-env".to_string()) + ); + } + + #[test] + #[serial_test::serial] + fn test_get_quota_project_for_method_env_var_wins_for_oauth() { + let _env_guard = EnvVarGuard::set("GOOGLE_WORKSPACE_PROJECT_ID", "priority-env"); + + assert_eq!( + get_quota_project_for_method(AuthMethod::OAuth), + Some("priority-env".to_string()) + ); } #[test] #[serial_test::serial] - fn test_get_quota_project_priority_config() { + fn test_get_quota_project_for_method_service_account_ignores_oauth_client_config() { let tmp = tempfile::tempdir().unwrap(); let _config_guard = EnvVarGuard::set( "GOOGLE_WORKSPACE_CLI_CONFIG_DIR", @@ -967,15 +983,19 @@ mod tests { let _adc_guard = EnvVarGuard::remove("GOOGLE_APPLICATION_CREDENTIALS"); let _home_guard = EnvVarGuard::set("HOME", "/missing/home"); - // Save a client config with a project ID + // Save a client config with a project ID — this must NOT be used as the service + // account's quota project, since the service account may not be an IAM member of it. crate::oauth_config::save_client_config("id", "secret", "config-project").unwrap(); - assert_eq!(get_quota_project(), Some("config-project".to_string())); + assert_eq!( + get_quota_project_for_method(AuthMethod::ServiceAccount), + None + ); } #[test] #[serial_test::serial] - fn test_get_quota_project_priority_adc_fallback() { + fn test_get_quota_project_for_method_oauth_ignores_adc_and_config_without_env_var() { let tmp = tempfile::tempdir().unwrap(); let adc_dir = tmp.path().join(".config").join("gcloud"); std::fs::create_dir_all(&adc_dir).unwrap(); @@ -987,30 +1007,38 @@ mod tests { let _home_guard = EnvVarGuard::set("HOME", tmp.path()); let _env_guard = EnvVarGuard::remove("GOOGLE_WORKSPACE_PROJECT_ID"); - let _config_guard = EnvVarGuard::remove("GOOGLE_WORKSPACE_CLI_CONFIG_DIR"); let _adc_guard = EnvVarGuard::remove("GOOGLE_APPLICATION_CREDENTIALS"); - assert_eq!(get_quota_project(), Some("adc-project".to_string())); + assert_eq!(get_quota_project_for_method(AuthMethod::OAuth), None); } #[test] #[serial_test::serial] - fn test_get_quota_project_reads_adc() { + fn test_get_quota_project_for_method_none_returns_none() { + let _env_guard = EnvVarGuard::remove("GOOGLE_WORKSPACE_PROJECT_ID"); + + assert_eq!(get_quota_project_for_method(AuthMethod::None), None); + } + + #[test] + #[serial_test::serial] + fn test_get_quota_project_for_method_service_account_reads_adc() { let tmp = tempfile::tempdir().unwrap(); let adc_dir = tmp.path().join(".config").join("gcloud"); std::fs::create_dir_all(&adc_dir).unwrap(); std::fs::write( adc_dir.join("application_default_credentials.json"), - r#"{"quota_project_id": "my-project-123"}"#, + r#"{"quota_project_id": "adc-project"}"#, ) .unwrap(); let _home_guard = EnvVarGuard::set("HOME", tmp.path()); - let _adc_guard = EnvVarGuard::remove("GOOGLE_APPLICATION_CREDENTIALS"); - // Isolate from local environment let _env_guard = EnvVarGuard::remove("GOOGLE_WORKSPACE_PROJECT_ID"); - let _config_guard = EnvVarGuard::remove("GOOGLE_WORKSPACE_CLI_CONFIG_DIR"); + let _adc_guard = EnvVarGuard::remove("GOOGLE_APPLICATION_CREDENTIALS"); - assert_eq!(get_quota_project(), Some("my-project-123".to_string())); + assert_eq!( + get_quota_project_for_method(AuthMethod::ServiceAccount), + Some("adc-project".to_string()) + ); } } diff --git a/crates/google-workspace-cli/src/executor.rs b/crates/google-workspace-cli/src/executor.rs index f58538fc0..edbc5be9d 100644 --- a/crates/google-workspace-cli/src/executor.rs +++ b/crates/google-workspace-cli/src/executor.rs @@ -180,18 +180,12 @@ async fn build_http_request( } } - // For service-account auth, always forward the quota project (env var, config, or ADC). - // For OAuth, only send when GOOGLE_WORKSPACE_PROJECT_ID is explicitly set — the user - // has opted in, so we honour it even though OAuth users may not be IAM members of every - // project. Omit the header entirely when neither condition is met to avoid 403 errors. - let quota_project = match auth_method { - AuthMethod::ServiceAccount => crate::auth::get_quota_project(), - AuthMethod::OAuth => std::env::var("GOOGLE_WORKSPACE_PROJECT_ID") - .ok() - .filter(|s| !s.is_empty()), - AuthMethod::None => None, - }; - if let Some(quota_project) = quota_project { + // Set the quota project based on the authentication method to avoid 403 errors. For + // service-account auth, this forwards the quota project from the environment or ADC — + // but never from the OAuth client configuration (client_secret.json), since the service + // account may not be an IAM member of that project. For OAuth, we only send it when + // GOOGLE_WORKSPACE_PROJECT_ID is explicitly set (the user has opted in). + if let Some(quota_project) = crate::auth::get_quota_project_for_method(*auth_method) { request = request.header("x-goog-user-project", quota_project); }