fix(auth): fall back to Discovery docs when -s specifies services not in static scope lists#246
fix(auth): fall back to Discovery docs when -s specifies services not in static scope lists#246haunchen wants to merge 3 commits intogoogleworkspace:mainfrom
-s specifies services not in static scope lists#246Conversation
…ot in static scope lists When `gws auth login -s chat` (or any service not in the 7 static scope lists) is used, the static filter returns no matching scopes. Add a dynamic fallback that detects unmatched services and fetches their OAuth scopes from the Google Discovery API. This leverages the existing `fetch_discovery_document` with 24h caching. Fixes googleworkspace#236
🦋 Changeset detectedLatest commit: 7c507f9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature to dynamically fetch scopes from Discovery documents for services not in the static lists, which significantly improves the tool's flexibility. The implementation is well-structured with new helper functions. I've identified a couple of performance improvement opportunities in the new functions: one to avoid allocations in a loop, and another to parallelize network requests. Addressing these will make the new feature more robust and efficient.
…fetches Address review feedback: - Avoid per-service HashSet allocation in find_unmatched_services by collecting matched services first then computing the difference. - Use futures_util::future::join_all to fetch Discovery docs in parallel instead of sequentially.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements a dynamic fallback for resolving OAuth scopes, which is a great improvement for handling services not in the static lists. The changes are well-structured, introducing new helper functions for finding unmatched services and fetching their scopes from discovery documents. The new logic is correctly integrated into the resolve_scopes function for various scenarios (--readonly, --full, and default). The addition of unit tests for the new functionality is also appreciated. I have one suggestion to address code duplication, which will improve the long-term maintainability of the code.
…pping Share the service-name-to-scope-prefix mapping between scope_matches_service and find_unmatched_services via a single helper.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is an excellent pull request that addresses a significant limitation in scope resolution. The introduction of a dynamic fallback to Discovery documents for services not present in the static lists is a great enhancement, making the tool much more flexible and future-proof. The implementation is well-structured, with clear separation of concerns into new helper functions like find_unmatched_services, extract_scopes_from_doc, and fetch_scopes_for_unmatched_services. The asynchronous fetching of discovery documents is handled efficiently using futures::join_all. The addition of comprehensive unit tests for the new logic is also commendable. I have reviewed the changes and found no issues of high or critical severity. The code is robust, correct, and maintainable.
2 participants
Summary
Fixes #236
When
gws auth login -s chat(or any service not in the static scope lists) is used, the-sfilter returns no matching scopes becauseMINIMAL_SCOPES,FULL_SCOPES, andREADONLY_SCOPESonly contain 7 common services (drive, sheets, gmail, calendar, docs, slides, tasks).This PR adds a dynamic fallback: when the static filter produces no scopes for a requested service, gws fetches that service's Discovery document and extracts its OAuth scopes automatically. This leverages the existing
fetch_discovery_documentwith 24h caching, consistent with the project's Dynamic Discovery philosophy.Changes
find_unmatched_services()— identifies which services from-shad zero matching scopes in the static listextract_scopes_from_doc()— pure function to extract scope URLs from aRestDescription, filtering out app-only scopes (chat.bot,chat.app.*, etc.) with optional readonly restrictionfetch_scopes_for_unmatched_services()— resolves service aliases viaresolve_service(), fetches Discovery docs, and extracts scopes; failures are silently skipped (graceful degradation)augment_with_dynamic_scopes()— integration wrapper applied at the three non-interactive return points inresolve_scopes(--readonly,--full, default)Behavior
gws auth login -s chatgws auth login -s drive,chatgws auth login -s chat --readonlychat.*.readonlyscopesgws auth login(no-s)Related issues
auth logindoes not include Google Keep scope (same root cause: Keep not in static lists, now resolved by dynamic fallback)-s)Test plan
cargo build— compiles cleancargo clippy -- -D warnings— zero warningscargo test— all new tests pass (5 added), no regressionsgws auth login -s chatfetches Chat API scopesgws auth login -s drive,chatincludes both drive and chat scopesgws auth login -s chat --readonlyreturns only readonly chat scopes