diff --git a/.changeset/fix-741-auth-services-validation.md b/.changeset/fix-741-auth-services-validation.md new file mode 100644 index 00000000..8e361720 --- /dev/null +++ b/.changeset/fix-741-auth-services-validation.md @@ -0,0 +1,7 @@ +--- +"@googleworkspace/cli": patch +--- + +`auth login -s` / `--services`: reject unknown service names at parse time with a clear error listing valid names. Previously, unrecognised tokens were silently dropped, causing a token to cover fewer services than the user intended. + +**Behavior change:** `cloud-platform` is no longer injected automatically when a `--services` filter is active. It was previously added to every filtered login regardless of the services requested. It still appears when using `--full` (no filter) or the interactive discovery picker (no filter). diff --git a/crates/google-workspace-cli/src/auth_commands.rs b/crates/google-workspace-cli/src/auth_commands.rs index d7571e74..61f97b67 100644 --- a/crates/google-workspace-cli/src/auth_commands.rs +++ b/crates/google-workspace-cli/src/auth_commands.rs @@ -448,7 +448,7 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { match matches.subcommand() { Some(("login", sub_m)) => { - let (scope_mode, services_filter) = parse_login_args(sub_m); + let (scope_mode, services_filter) = parse_login_args(sub_m)?; handle_login_inner(scope_mode, services_filter).await } @@ -483,7 +483,11 @@ fn login_command() -> clap::Command { } /// Extract `ScopeMode` and optional services filter from parsed login args. -fn parse_login_args(matches: &clap::ArgMatches) -> (ScopeMode, Option>) { +/// +/// Returns `Err` if any token in `--services` is not a known service alias. +fn parse_login_args( + matches: &clap::ArgMatches, +) -> Result<(ScopeMode, Option>), GwsError> { let scope_mode = if let Some(scopes_str) = matches.get_one::("scopes") { ScopeMode::Custom( scopes_str @@ -501,14 +505,54 @@ fn parse_login_args(matches: &clap::ArgMatches) -> (ScopeMode, Option> = matches.get_one::("services").map(|v| { - v.split(',') - .map(|s| s.trim().to_lowercase()) - .filter(|s| !s.is_empty()) - .collect() - }); + let services_filter: Option> = + if let Some(v) = matches.get_one::("services") { + let tokens: Vec = v + .split(',') + .map(|s| s.trim().to_lowercase()) + .filter(|s| !s.is_empty()) + .collect(); + + // Platform scopes recognised by the tool but not registered as + // Workspace API services (they lack an API discovery entry). + const PLATFORM_SCOPES: &[&str] = &["cloud-platform", "pubsub"]; + + let unknown: Vec<&str> = tokens + .iter() + .filter(|name| { + let n = name.as_str(); + !PLATFORM_SCOPES.contains(&n) + && !crate::services::SERVICES + .iter() + .any(|e| e.aliases.contains(&n)) + }) + .map(|s| s.as_str()) + .collect(); + + if !unknown.is_empty() { + let mut valid: Vec<&str> = crate::services::SERVICES + .iter() + .flat_map(|e| e.aliases.iter().copied()) + .collect(); + valid.extend(PLATFORM_SCOPES); + valid.sort_unstable(); + return Err(GwsError::Validation(format!( + "Unknown service(s): {}. Valid services: {}.", + unknown + .iter() + .map(|s| s.escape_debug().to_string()) + .collect::>() + .join(", "), + valid.join(", ") + ))); + } + + Some(tokens.into_iter().collect()) + } else { + None + }; - (scope_mode, services_filter) + Ok((scope_mode, services_filter)) } /// Run the `auth login` flow. @@ -532,7 +576,7 @@ pub async fn run_login(args: &[String]) -> Result<(), GwsError> { Err(e) => return Err(GwsError::Validation(e.to_string())), }; - let (scope_mode, services_filter) = parse_login_args(&matches); + let (scope_mode, services_filter) = parse_login_args(&matches)?; handle_login_inner(scope_mode, services_filter).await } @@ -817,11 +861,6 @@ fn scope_matches_service(scope_url: &str, services: &HashSet) -> bool { .strip_prefix("https://www.googleapis.com/auth/") .unwrap_or(scope_url); - // cloud-platform is a cross-service scope, always include - if short == "cloud-platform" { - return true; - } - let prefix = short.split('.').next().unwrap_or(short); services.iter().any(|svc| { @@ -1102,8 +1141,9 @@ fn run_discovery_scope_picker( } } - // Always include cloud-platform scope - if !selected.contains(&PLATFORM_SCOPE.to_string()) { + // Include cloud-platform when no services filter is active. When the + // user passes -s, they get exactly the services they asked for. + if services_filter.is_none() && !selected.contains(&PLATFORM_SCOPE.to_string()) { selected.push(PLATFORM_SCOPE.to_string()); } @@ -2184,9 +2224,9 @@ mod tests { } #[test] - fn scope_matches_service_cloud_platform_always_matches() { + fn scope_matches_service_cloud_platform_does_not_match_unrelated_service() { let services: HashSet = ["drive"].iter().map(|s| s.to_string()).collect(); - assert!(scope_matches_service( + assert!(!scope_matches_service( "https://www.googleapis.com/auth/cloud-platform", &services )); @@ -2248,9 +2288,7 @@ mod tests { .strip_prefix("https://www.googleapis.com/auth/") .unwrap_or(scope); assert!( - short.starts_with("drive") - || short.starts_with("gmail") - || short == "cloud-platform", + short.starts_with("drive") || short.starts_with("gmail"), "Unexpected scope with service filter: {scope}" ); } @@ -2274,7 +2312,7 @@ mod tests { .strip_prefix("https://www.googleapis.com/auth/") .unwrap_or(scope); assert!( - short.starts_with("drive") || short == "cloud-platform", + short.starts_with("drive"), "Unexpected scope with service + readonly filter: {scope}" ); } @@ -2289,7 +2327,7 @@ mod tests { .strip_prefix("https://www.googleapis.com/auth/") .unwrap_or(scope); assert!( - short.starts_with("gmail") || short == "cloud-platform", + short.starts_with("gmail"), "Unexpected scope with service + full filter: {scope}" ); } @@ -2307,6 +2345,49 @@ mod tests { assert_eq!(scopes[0], "https://www.googleapis.com/auth/calendar"); } + #[test] + fn parse_login_args_rejects_unknown_service() { + let cmd = login_command(); + let matches = cmd + .try_get_matches_from(["login", "--services", "drive,typoservice"]) + .unwrap(); + let result = parse_login_args(&matches); + assert!(result.is_err()); + let msg = result.err().expect("expected error").to_string(); + assert!(msg.contains("typoservice"), "error should name the unknown token: {msg}"); + assert!(msg.contains("drive"), "error should list valid services: {msg}"); + } + + #[test] + fn parse_login_args_accepts_known_services() { + let cmd = login_command(); + let matches = cmd + .try_get_matches_from(["login", "--services", "Drive,Gmail"]) + .unwrap(); + let (_, filter) = parse_login_args(&matches).unwrap(); + let services = filter.unwrap(); + assert!(services.contains("drive"), "should lowercase: {services:?}"); + assert!(services.contains("gmail"), "should lowercase: {services:?}"); + } + + #[test] + fn parse_login_args_accepts_platform_scopes() { + let cmd = login_command(); + let matches = cmd + .try_get_matches_from(["login", "--services", "cloud-platform,pubsub"]) + .unwrap(); + let (_, filter) = parse_login_args(&matches).unwrap(); + let services = filter.unwrap(); + assert!( + services.contains("cloud-platform"), + "cloud-platform should be accepted: {services:?}" + ); + assert!( + services.contains("pubsub"), + "pubsub should be accepted: {services:?}" + ); + } + #[test] fn filter_scopes_by_services_none_returns_all() { let scopes = vec![ diff --git a/crates/google-workspace-cli/src/helpers/script.rs b/crates/google-workspace-cli/src/helpers/script.rs index 11bcdebe..4b31db62 100644 --- a/crates/google-workspace-cli/src/helpers/script.rs +++ b/crates/google-workspace-cli/src/helpers/script.rs @@ -169,13 +169,7 @@ fn process_file(path: &Path) -> Result, GwsError> { filename.trim_end_matches(".js").trim_end_matches(".gs"), ), "html" => ("HTML", filename.trim_end_matches(".html")), - "json" => { - if filename == "appsscript.json" { - ("JSON", "appsscript") - } else { - return Ok(None); - } - } + "json" if filename == "appsscript.json" => ("JSON", "appsscript"), _ => return Ok(None), };