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/auth.rs b/crates/google-workspace-cli/src/auth.rs index 9d8847e4b..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: @@ -126,6 +129,17 @@ fn adc_well_known_path() -> Option { }) } +/// 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 #[derive(Debug)] enum Credential { @@ -212,10 +226,15 @@ 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) + get_token_with_kind(scopes).await.map(|(token, _)| token) +} + +/// 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, AuthMethod)> { if let Ok(token) = std::env::var("GOOGLE_WORKSPACE_CLI_TOKEN") { if !token.is_empty() { - return Ok(token); + return Ok((token, AuthMethod::OAuth)); } } @@ -226,7 +245,12 @@ pub async fn get_token(scopes: &[&str]) -> anyhow::Result { 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 + let kind = match &creds { + Credential::ServiceAccount(_) => AuthMethod::ServiceAccount, + Credential::AuthorizedUser(_) => AuthMethod::OAuth, + }; + let token = get_token_inner(scopes, creds, &token_cache).await?; + Ok((token, kind)) } /// Check if HTTP proxy environment variables are set @@ -925,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", @@ -946,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(); @@ -966,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_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_reads_adc() { + 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 46f31ac4b..edbc5be9d 100644 --- a/crates/google-workspace-cli/src/executor.rs +++ b/crates/google-workspace-cli/src/executor.rs @@ -31,14 +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 credentials file - OAuth, - /// No authentication was provided - None, -} +pub use crate::auth::AuthMethod; /// Source for media upload content. /// @@ -182,13 +175,17 @@ 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); } } - // Set quota project from ADC for billing/quota attribution - if let Some(quota_project) = crate::auth::get_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); } @@ -2399,3 +2396,128 @@ async fn test_get_does_not_set_content_length_zero() { "GET with no body should not have Content-Length header" ); } + +/// 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. + let _env_guard = EnvVarGuard::remove("GOOGLE_WORKSPACE_PROJECT_ID"); + 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 when env var is not set" + ); +} + +#[cfg(test)] +#[tokio::test] +#[serial_test::serial] +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). + 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(), + 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_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" + ); +} diff --git a/crates/google-workspace-cli/src/main.rs b/crates/google-workspace-cli/src/main.rs index 41dcc1e1f..5e9fb10c1 100644 --- a/crates/google-workspace-cli/src/main.rs +++ b/crates/google-workspace-cli/src/main.rs @@ -262,8 +262,8 @@ 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, 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.