Skip to content
Closed
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
15 changes: 15 additions & 0 deletions .changeset/service-first-scope-picker.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"@googleworkspace/cli": minor
---

Redesign the scope picker to show **services** instead of raw scope names.

**Before:** Users saw a flat list of scope shortcodes like `drive.metadata.readonly`, `gmail.compose`, `calendar.events`.

**After:** Users see services by name with descriptions:
- `Drive` — Manage files, folders, and shared drives · 8 scopes · ⛔ 7 restricted
- `Gmail` — Send, read, and manage email · 5 scopes · ⛔ 6 restricted
- `Calendar` — Manage calendars and events · 2 scopes

Templates (Recommended, Read Only, Full Access) now select/deselect services.
Scope resolution to URLs happens automatically based on the selected service and template.
254 changes: 154 additions & 100 deletions src/auth_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -698,18 +698,18 @@ fn is_recommended_scope(
}
}

/// Run the rich discovery-based scope picker with templates.
/// Run the rich discovery-based scope picker with service-first UX.
///
/// Instead of showing raw scope names, presents services by name with descriptions
/// (e.g., "Drive — Manage files, folders, and shared drives") and lets users
/// select services, then resolve to the appropriate scopes.
fn run_discovery_scope_picker(
relevant_scopes: &[crate::setup::DiscoveredScope],
services_filter: Option<&HashSet<String>>,
) -> Option<Vec<String>> {
use crate::setup::{ScopeClassification, PLATFORM_SCOPE};
use crate::setup_tui::{PickerResult, SelectItem};

let mut recommended_scopes = vec![];
let mut readonly_scopes = vec![];
let mut all_scopes = vec![];

// Pre-filter scopes by services if a filter is specified
let filtered_scopes: Vec<&crate::setup::DiscoveredScope> = relevant_scopes
.iter()
Expand All @@ -720,109 +720,151 @@ fn run_discovery_scope_picker(
})
.collect();

// Collect all short names for hierarchical dedup of Full Access template
// Group scopes by service prefix (first segment of the short name).
// e.g., "drive", "drive.metadata.readonly", "drive.file" → all under "drive"
let mut service_groups: std::collections::BTreeMap<
String,
Vec<&crate::setup::DiscoveredScope>,
> = std::collections::BTreeMap::new();

for entry in &filtered_scopes {
if is_app_only_scope(&entry.url) {
continue;
}
let service_key = entry.short.split('.').next().unwrap_or(&entry.short);
service_groups
.entry(service_key.to_string())
.or_default()
.push(entry);
}

// Build templates: collect scope short names per template category
let all_shorts: Vec<&str> = filtered_scopes
.iter()
.filter(|e| !is_app_only_scope(&e.url))
.map(|e| e.short.as_str())
.collect();

for entry in &filtered_scopes {
// Skip app-only scopes that can't be used with user OAuth
if is_app_only_scope(&entry.url) {
continue;
}
let mut recommended_services = vec![];
let mut readonly_services = vec![];
let mut all_services = vec![];

if is_recommended_scope(entry, &all_shorts, services_filter.is_some()) {
recommended_scopes.push(entry.short.to_string());
}
if entry.is_readonly {
readonly_scopes.push(entry.short.to_string());
for (svc_key, _scopes) in &service_groups {
// A service is "recommended" if any of its scopes are recommended
let has_recommended = _scopes
.iter()
.any(|s| is_recommended_scope(s, &all_shorts, services_filter.is_some()));
if has_recommended {
recommended_services.push(svc_key.clone());
}
// For "Full Access": skip if a broader scope exists (hierarchical dedup)
// e.g. "drive.metadata" is subsumed by "drive", "calendar.events" by "calendar"
if !is_subsumed_scope(&entry.short, &all_shorts) {
all_scopes.push(entry.short.to_string());

// A service has readonly scopes
let has_readonly = _scopes.iter().any(|s| s.is_readonly);
if has_readonly {
readonly_services.push(svc_key.clone());
}

all_services.push(svc_key.clone());
}

let mut items: Vec<SelectItem> = vec![
SelectItem {
label: "✨ Recommended (Core Consumer Scopes)".to_string(),
description: "Selects Drive, Gmail, Calendar, Docs, Sheets, Slides, and Tasks"
label: "✨ Recommended".to_string(),
description: "Core consumer APIs: Drive, Gmail, Calendar, Docs, Sheets, Slides, Tasks"
.to_string(),
selected: true,
is_fixed: false,
is_template: true,
template_selects: recommended_scopes,
template_selects: recommended_services.clone(),
},
SelectItem {
label: "🔒 Read Only".to_string(),
description: "Selects only readonly scopes for enabled APIs".to_string(),
description: "Read-only access to all enabled APIs".to_string(),
selected: false,
is_fixed: false,
is_template: true,
template_selects: readonly_services.clone(),
},
SelectItem {
label: "⚠️ Full Access".to_string(),
description: "Full read/write access to ALL enabled APIs".to_string(),
selected: false,
is_fixed: false,
is_template: true,
template_selects: readonly_scopes,
template_selects: all_services.clone(),
},
SelectItem {
label: "⚠️ Full Access (All Scopes)".to_string(),
description: "Selects ALL scopes, including restricted write scopes".to_string(),
label: "🔧 Custom".to_string(),
description: "Pick individual scopes manually".to_string(),
selected: false,
is_fixed: false,
is_template: true,
template_selects: all_scopes,
template_selects: vec![],
},
];
let template_count = items.len();

let mut valid_scope_indices: Vec<usize> = Vec::new();
for (idx, entry) in filtered_scopes.iter().enumerate() {
// Skip app-only scopes from the picker entirely
if is_app_only_scope(&entry.url) {
continue;
}
// Add service items with human-readable labels
let mut service_keys_in_order: Vec<String> = Vec::new();
for (svc_key, scopes) in &service_groups {
// Look up service description from registry
let svc_entry = crate::services::SERVICES
.iter()
.find(|e| e.aliases.contains(&svc_key.as_str()));
let description = svc_entry.map(|e| e.description).unwrap_or("Google API");

let (prefix, emoji) = match entry.classification {
ScopeClassification::Restricted => ("RESTRICTED ", "⛔ "),
ScopeClassification::Sensitive => ("SENSITIVE ", "⚠️ "),
ScopeClassification::NonSensitive => ("", ""),
};
// Count scope types for the description
let total = scopes.len();
let restricted_count = scopes
.iter()
.filter(|s| s.classification == ScopeClassification::Restricted)
.count();
let sensitive_count = scopes
.iter()
.filter(|s| s.classification == ScopeClassification::Sensitive)
.count();

let desc_str = if entry.description.is_empty() {
entry.url.clone()
} else {
entry.description.clone()
};
let mut badges = Vec::new();
if restricted_count > 0 {
badges.push(format!("⛔ {restricted_count} restricted"));
}
if sensitive_count > 0 {
badges.push(format!("⚠️ {sensitive_count} sensitive"));
}

let description = if prefix.is_empty() {
desc_str
} else {
format!("{}{}{}", emoji, prefix, desc_str)
};
let label = svc_entry
.and_then(|e| {
// Capitalize the first alias for display
e.aliases.first().map(|name| {
let mut chars = name.chars();
match chars.next() {
None => String::new(),
Some(c) => c.to_uppercase().collect::<String>() + chars.as_str(),
}
})
})
.unwrap_or_else(|| svc_key.clone());
Comment on lines +835 to +846
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There's a potential for a panic here if a ServiceEntry has an empty aliases slice. The code at line 830 accesses e.aliases[0] without checking if the slice is empty. While all current ServiceEntry definitions in src/services.rs have at least one alias, this is fragile and could lead to a crash if a new entry is added incorrectly in the future.

To make the code more robust, you should safely access the first element of the aliases slice.

Suggested change
let label = svc_entry
.map(|e| {
// Capitalize the first alias for display
let name = e.aliases[0];
let mut chars = name.chars();
match chars.next() {
None => String::new(),
Some(c) => c.to_uppercase().collect::<String>() + chars.as_str(),
}
})
.unwrap_or_else(|| svc_key.clone());
let label = svc_entry
.and_then(|e| {
// Capitalize the first alias for display
e.aliases.first().map(|name| {
let mut chars = name.chars();
match chars.next() {
None => String::new(),
Some(c) => c.to_uppercase().collect::<String>() + chars.as_str(),
}
})
})
.unwrap_or_else(|| svc_key.clone());


let is_recommended = if entry.is_readonly {
let superset = entry.url.strip_suffix(".readonly").unwrap_or(&entry.url);
let superset_is_recommended = filtered_scopes
.iter()
.any(|s| s.url == superset && s.classification != ScopeClassification::Restricted);
!superset_is_recommended
} else {
entry.classification != ScopeClassification::Restricted
};
let desc_parts: Vec<String> = std::iter::once(description.to_string())
.chain(std::iter::once(format!("{total} scopes")))
.chain(badges)
.collect();

let is_recommended = recommended_services.contains(svc_key);

items.push(SelectItem {
label: entry.short.to_string(),
description,
label,
description: desc_parts.join(" · "),
selected: is_recommended,
is_fixed: false,
is_template: false,
template_selects: vec![],
});
valid_scope_indices.push(idx);
service_keys_in_order.push(svc_key.clone());
}

match crate::setup_tui::run_picker(
"Select OAuth scopes",
"Select services to authorize",
"Space to toggle, Enter to confirm",
items,
true,
Expand All @@ -831,44 +873,60 @@ fn run_discovery_scope_picker(
let recommended = items.first().is_some_and(|i| i.selected);
let readonly = items.get(1).is_some_and(|i| i.selected);
let full = items.get(2).is_some_and(|i| i.selected);
let custom = items.get(3).is_some_and(|i| i.selected);

let mut selected: Vec<String> = Vec::new();
// Custom mode: fall through to individual scope picker
if custom {
return run_simple_scope_picker(services_filter);
}

if full && !recommended && !readonly {
// Full Access: include all non-app-only scopes
// (hierarchical dedup is applied in post-processing below)
for entry in &filtered_scopes {
if is_app_only_scope(&entry.url) {
continue;
}
selected.push(entry.url.to_string());
}
// Determine which services are selected
let selected_service_keys: Vec<&str> = if full && !recommended && !readonly {
all_services.iter().map(|s| s.as_str()).collect()
} else if recommended && !full && !readonly {
// Recommended: consumer scopes only (or top-level scopes if filtered).
for entry in &filtered_scopes {
if is_app_only_scope(&entry.url) {
continue;
}
if is_recommended_scope(entry, &all_shorts, services_filter.is_some()) {
selected.push(entry.url.to_string());
}
}
recommended_services.iter().map(|s| s.as_str()).collect()
} else if readonly && !full && !recommended {
for entry in &filtered_scopes {
if is_app_only_scope(&entry.url) {
continue;
}
if entry.is_readonly {
selected.push(entry.url.to_string());
}
}
readonly_services.iter().map(|s| s.as_str()).collect()
} else {
for (i, item) in items.iter().enumerate().skip(template_count) {
if item.selected {
let picker_idx = i - template_count;
if let Some(&scope_idx) = valid_scope_indices.get(picker_idx) {
if let Some(entry) = filtered_scopes.get(scope_idx) {
selected.push(entry.url.to_string());
// Individual service selection
items
.iter()
.enumerate()
.skip(template_count)
.filter(|(_, item)| item.selected)
.filter_map(|(i, _)| {
service_keys_in_order
.get(i - template_count)
.map(|s| s.as_str())
})
.collect()
};

// Map selected services to scope URLs
let mut selected: Vec<String> = Vec::new();
let is_readonly_mode = readonly && !full && !recommended;

for svc_key in &selected_service_keys {
if let Some(scopes) = service_groups.get(*svc_key) {
if is_readonly_mode {
// In Read Only mode, prefer readonly scopes
for s in scopes {
if s.is_readonly {
selected.push(s.url.clone());
}
}
} else if full && !recommended && !readonly {
// Full Access template: include top-level (non-subsumed) scopes
for s in scopes {
if !is_subsumed_scope(&s.short, &all_shorts) {
selected.push(s.url.clone());
}
}
} else {
// Recommended or custom: use recommended scope selection logic
for s in scopes {
if is_recommended_scope(s, &all_shorts, services_filter.is_some()) {
selected.push(s.url.clone());
}
}
}
Comment on lines +883 to 932
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There's a logic issue in how selected services and their corresponding scopes are determined when multiple templates are selected. The current implementation uses mutually exclusive conditions (e.g., full && !recommended && !readonly) for both determining the list of selected services and for choosing which scopes to grant. This approach fails for combinations of templates.

For example, if a user selects both 'Full Access' and 'Read Only', the logic falls through to the final else block, which applies 'Recommended' scope logic. This is likely not the intended behavior. A user would probably expect 'Read Only' to take precedence.

This can be fixed by simplifying the logic:

  1. Always determine the selected services by inspecting the UI state, since the TUI updates the service selections when a template is toggled.
  2. Establish a clear priority for which scope-selection logic to use (e.g., Read Only > Full Access > Recommended).

This ensures that template combinations behave predictably and correctly.

            // Determine which services are selected by looking at the UI state,
            // which is updated by the TUI when templates are toggled.
            let selected_service_keys: Vec<&str> = items
                .iter()
                .enumerate()
                .skip(template_count)
                .filter(|(_, item)| item.selected)
                .filter_map(|(i, _)| {
                    service_keys_in_order
                        .get(i - template_count)
                        .map(|s| s.as_str())
                })
                .collect();

            // Map selected services to scope URLs
            let mut selected: Vec<String> = Vec::new();

            // Establish priority for templates: Read Only > Full Access > Recommended/Custom
            let use_readonly = readonly;
            let use_full = full && !use_readonly;

            for svc_key in &selected_service_keys {
                if let Some(scopes) = service_groups.get(*svc_key) {
                    if use_readonly {
                        // In Read Only mode, prefer readonly scopes
                        for s in scopes {
                            if s.is_readonly {
                                selected.push(s.url.clone());
                            }
                        }
                    } else if use_full {
                        // Full Access mode: include top-level (non-subsumed) scopes
                        for s in scopes {
                            if !is_subsumed_scope(&s.short, &all_shorts) {
                                selected.push(s.url.clone());
                            }
                        }
                    } else {
                        // Recommended or individual selection: use recommended scope selection logic
                        for s in scopes {
                            if is_recommended_scope(s, &all_shorts, services_filter.is_some()) {
                                selected.push(s.url.clone());
                            }
                        }
                    }
                }
            }

Expand All @@ -880,9 +938,7 @@ fn run_discovery_scope_picker(
selected.push(PLATFORM_SCOPE.to_string());
}

// Hierarchical dedup: if we have both a broad scope (e.g. `.../auth/drive`)
// and a narrower scope (e.g. `.../auth/drive.metadata`, `.../auth/drive.readonly`),
// drop the narrower one since the broad scope subsumes it.
// Hierarchical dedup
let prefix = "https://www.googleapis.com/auth/";
let shorts: Vec<&str> = selected
.iter()
Expand All @@ -892,8 +948,6 @@ fn run_discovery_scope_picker(
let mut deduplicated: Vec<String> = Vec::new();
for scope in &selected {
if let Some(short) = scope.strip_prefix(prefix) {
// Check if any OTHER selected scope is a prefix of this one
// e.g. "drive" is a prefix of "drive.metadata" → drop "drive.metadata"
let is_subsumed = shorts.iter().any(|&other| {
other != short
&& short.starts_with(other)
Expand Down
1 change: 1 addition & 0 deletions src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ pub struct DiscoveredScope {
/// Short label, e.g. "drive"
pub short: String,
/// Human-readable description from the Discovery Document.
#[allow(dead_code)]
pub description: String,
/// Which API this scope came from, e.g. "Google Drive"
#[allow(dead_code)]
Expand Down
Loading