diff --git a/.changeset/fix-scope-selection.md b/.changeset/fix-scope-selection.md new file mode 100644 index 0000000..68fa5d7 --- /dev/null +++ b/.changeset/fix-scope-selection.md @@ -0,0 +1,5 @@ +--- +"@googleworkspace/cli": patch +--- + +Fix scope selection to use first (broadest) scope instead of all method scopes, preventing gmail.metadata restrictions from blocking query parameters diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 09d8e5e..4260e5d 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -273,6 +273,12 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { ) .await; + // Remove restrictive scopes when broader alternatives are present. + // gmail.metadata blocks query parameters like `q`, and is redundant + // when broader scopes (gmail.modify, gmail.readonly, mail.google.com) + // are already included. + let scopes = filter_redundant_restrictive_scopes(scopes); + let secret = yup_oauth2::ApplicationSecret { client_id: client_id.clone(), client_secret: client_secret.clone(), @@ -613,6 +619,39 @@ fn scope_matches_service(scope_url: &str, services: &HashSet) -> bool { }) } +/// Remove restrictive scopes that are redundant when broader alternatives +/// are present. For example, `gmail.metadata` restricts query parameters +/// and is unnecessary when `gmail.modify`, `gmail.readonly`, or the full +/// `https://mail.google.com/` scope is already included. +/// +/// This prevents Google from enforcing the restrictive scope's limitations +/// on the access token even though broader access was granted. +fn filter_redundant_restrictive_scopes(scopes: Vec) -> Vec { + // Scopes that restrict API behavior when present in a token, even alongside + // broader scopes. Each entry maps a restrictive scope to the broader scopes + // that make it redundant. The restrictive scope is removed only if at least + // one of its broader alternatives is already in the list. + const RESTRICTIVE_SCOPES: &[(&str, &[&str])] = &[( + "https://www.googleapis.com/auth/gmail.metadata", + &[ + "https://mail.google.com/", + "https://www.googleapis.com/auth/gmail.modify", + "https://www.googleapis.com/auth/gmail.readonly", + ], + )]; + + let scope_set: std::collections::HashSet = scopes.iter().cloned().collect(); + + scopes + .into_iter() + .filter(|scope| { + !RESTRICTIVE_SCOPES.iter().any(|(restrictive, broader)| { + scope.as_str() == *restrictive && broader.iter().any(|b| scope_set.contains(*b)) + }) + }) + .collect() +} + /// Filter a list of scope URLs to only those matching the given services. /// If no filter is provided, returns all scopes unchanged. fn filter_scopes_by_services( @@ -2095,6 +2134,38 @@ mod tests { assert_eq!(result, scopes); } + #[test] + fn filter_restrictive_removes_metadata_when_broader_present() { + let scopes = vec![ + "https://www.googleapis.com/auth/gmail.modify".to_string(), + "https://www.googleapis.com/auth/gmail.metadata".to_string(), + "https://www.googleapis.com/auth/drive".to_string(), + ]; + let result = filter_redundant_restrictive_scopes(scopes); + assert!(!result.iter().any(|s| s.contains("gmail.metadata"))); + assert_eq!(result.len(), 2); + } + + #[test] + fn filter_restrictive_removes_metadata_when_full_gmail_present() { + let scopes = vec![ + "https://mail.google.com/".to_string(), + "https://www.googleapis.com/auth/gmail.metadata".to_string(), + ]; + let result = filter_redundant_restrictive_scopes(scopes); + assert_eq!(result, vec!["https://mail.google.com/"]); + } + + #[test] + fn filter_restrictive_keeps_metadata_when_only_scope() { + let scopes = vec![ + "https://www.googleapis.com/auth/gmail.metadata".to_string(), + "https://www.googleapis.com/auth/drive".to_string(), + ]; + let result = filter_redundant_restrictive_scopes(scopes.clone()); + assert_eq!(result, scopes); + } + #[test] fn mask_secret_long_string() { let masked = mask_secret("GOCSPX-abcdefghijklmnopqrstuvwxyz"); diff --git a/src/main.rs b/src/main.rs index 914b485..d0fa709 100644 --- a/src/main.rs +++ b/src/main.rs @@ -232,8 +232,10 @@ async fn run() -> Result<(), GwsError> { // Build pagination config from flags let pagination = parse_pagination_config(matched_args); - // Get scopes from the method - let scopes: Vec<&str> = method.scopes.iter().map(|s| s.as_str()).collect(); + // Select the best scope for the method. Discovery Documents list scopes as + // alternatives (any one grants access). We pick the first (broadest) scope + // to avoid restrictive scopes like gmail.metadata that block query parameters. + 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, account.as_deref()).await { @@ -273,6 +275,17 @@ async fn run() -> Result<(), GwsError> { .map(|_| ()) } +/// Select the best scope from a method's scope list. +/// +/// Discovery Documents list method scopes as alternatives — any single scope +/// grants access. The first scope is typically the broadest. Using all scopes +/// causes issues when restrictive scopes (e.g., `gmail.metadata`) are included, +/// as the API enforces that scope's restrictions even when broader scopes are +/// also present. +pub(crate) fn select_scope(scopes: &[String]) -> Option<&str> { + scopes.first().map(|s| s.as_str()) +} + fn parse_pagination_config(matches: &clap::ArgMatches) -> executor::PaginationConfig { executor::PaginationConfig { page_all: matches.get_flag("page-all"), @@ -783,4 +796,30 @@ mod tests { assert!(!filtered.iter().any(|a| a.contains("account"))); assert_eq!(filtered, vec!["gws", "files", "list"]); } + + #[test] + fn test_select_scope_picks_first() { + let scopes = vec![ + "https://mail.google.com/".to_string(), + "https://www.googleapis.com/auth/gmail.metadata".to_string(), + "https://www.googleapis.com/auth/gmail.modify".to_string(), + "https://www.googleapis.com/auth/gmail.readonly".to_string(), + ]; + assert_eq!(select_scope(&scopes), Some("https://mail.google.com/")); + } + + #[test] + fn test_select_scope_single() { + let scopes = vec!["https://www.googleapis.com/auth/drive".to_string()]; + assert_eq!( + select_scope(&scopes), + Some("https://www.googleapis.com/auth/drive") + ); + } + + #[test] + fn test_select_scope_empty() { + let scopes: Vec = vec![]; + assert_eq!(select_scope(&scopes), None); + } } diff --git a/src/mcp_server.rs b/src/mcp_server.rs index 331e70d..485d9b4 100644 --- a/src/mcp_server.rs +++ b/src/mcp_server.rs @@ -786,7 +786,7 @@ async fn execute_mcp_method( page_delay_ms: 100, }; - let scopes: Vec<&str> = method.scopes.iter().map(|s| s.as_str()).collect(); + let scopes: Vec<&str> = crate::select_scope(&method.scopes).into_iter().collect(); let (token, auth_method) = match crate::auth::get_token(&scopes, None).await { Ok(t) => (Some(t), crate::executor::AuthMethod::OAuth), Err(e) => { diff --git a/src/token_storage.rs b/src/token_storage.rs index c54f5f7..d6c5c47 100644 --- a/src/token_storage.rs +++ b/src/token_storage.rs @@ -125,19 +125,10 @@ impl TokenStorage for EncryptedTokenStorage { } if let Some(map) = map_lock.as_ref() { - // First look for exact match let key = Self::cache_key(scopes); if let Some(token) = map.get(&key) { return Some(token.clone()); } - - // Fallback: check if we have a superset of the scopes (simplistic check compared to yup-oauth2 internal, but functional for this CLI) - for (cached_key, token) in map.iter() { - let cached_scopes: Vec<&str> = cached_key.split(' ').collect(); - if scopes.iter().all(|s| cached_scopes.contains(s)) { - return Some(token.clone()); - } - } } None