Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions .changeset/fix-741-auth-services-validation.md
Original file line number Diff line number Diff line change
@@ -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).
129 changes: 105 additions & 24 deletions crates/google-workspace-cli/src/auth_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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<HashSet<String>>) {
///
/// Returns `Err` if any token in `--services` is not a known service alias.
fn parse_login_args(
matches: &clap::ArgMatches,
) -> Result<(ScopeMode, Option<HashSet<String>>), GwsError> {
let scope_mode = if let Some(scopes_str) = matches.get_one::<String>("scopes") {
ScopeMode::Custom(
scopes_str
Expand All @@ -501,14 +505,54 @@ fn parse_login_args(matches: &clap::ArgMatches) -> (ScopeMode, Option<HashSet<St
ScopeMode::Default
};

let services_filter: Option<HashSet<String>> = matches.get_one::<String>("services").map(|v| {
v.split(',')
.map(|s| s.trim().to_lowercase())
.filter(|s| !s.is_empty())
.collect()
});
let services_filter: Option<HashSet<String>> =
if let Some(v) = matches.get_one::<String>("services") {
let tokens: Vec<String> = 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::<Vec<_>>()
.join(", "),
valid.join(", ")
)));
}
Comment thread
hoyt-harness marked this conversation as resolved.

Some(tokens.into_iter().collect())
} else {
None
};

(scope_mode, services_filter)
Ok((scope_mode, services_filter))
}

/// Run the `auth login` flow.
Expand All @@ -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
}
Expand Down Expand Up @@ -817,11 +861,6 @@ fn scope_matches_service(scope_url: &str, services: &HashSet<String>) -> 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| {
Expand Down Expand Up @@ -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());
}

Expand Down Expand Up @@ -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<String> = ["drive"].iter().map(|s| s.to_string()).collect();
assert!(scope_matches_service(
assert!(!scope_matches_service(
"https://www.googleapis.com/auth/cloud-platform",
&services
));
Expand Down Expand Up @@ -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}"
);
}
Expand All @@ -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}"
);
}
Expand All @@ -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}"
);
}
Expand All @@ -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![
Expand Down
8 changes: 1 addition & 7 deletions crates/google-workspace-cli/src/helpers/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,7 @@ fn process_file(path: &Path) -> Result<Option<serde_json::Value>, 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),
};

Expand Down
Loading