feat(auth): restore multi-account support (fixes #439)#496
feat(auth): restore multi-account support (fixes #439)#496dumko2001 wants to merge 7 commits intogoogleworkspace:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 6fed5b8 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 |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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 reintroduces and enhances multi-account capabilities within the CLI. It provides a robust mechanism for users to manage multiple Google accounts by isolating their authentication credentials, token caches, and personalized settings. This change significantly improves security and auditing for users managing various identities or AI agents, ensuring that operations are correctly attributed and isolated to the intended account. Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Code Review
The pull request implements multi-account authentication support, allowing users to isolate credentials and token caches using a global --account <name> flag. This involved updating ScopedTokenProvider and get_token to accept an optional account parameter, modifying credential and token cache path resolution to be account-specific, and plumbing the account parameter through various authentication and helper functions across the codebase. The review comments highlight several areas where the multi-account support is not fully consistent: specifically, the handle_status function in src/auth_commands.rs and the invalidate_cache function in src/timezone.rs need to be updated to correctly utilize the account parameter for path resolution and cache invalidation, respectively. Additionally, the account parameter needs to be passed to sanitize_text in src/helpers/gmail/watch.rs to ensure correct credential usage for Model Armor.
a739c11 to
60867d9
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request introduces multi-account authentication support, enabling users to specify an account alias via a global --account flag. This involved updating the ScopedTokenProvider and get_token functions to handle account-specific tokens, modifying credential and token cache paths to be account-aware, and propagating the account context through various helper functions for Google services and Model Armor sanitization. A key area for improvement identified in the review is the manual parsing of the --account flag within src/auth_commands.rs, which is complex and brittle, suggesting a refactor to leverage clap for more robust and consistent argument handling.
60867d9 to
d4b09de
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces multi-account support, a significant feature that allows users to manage separate credentials and settings for different Google accounts. The implementation correctly isolates token caches, encrypted credentials, and timezone settings by using an --account flag. The auth-related commands (status, logout, export) and the core CLI command structure have been updated accordingly. However, I've found a critical issue where plaintext credentials are not properly isolated per account, which could lead to incorrect credential usage. This issue appears in two places in the authentication logic. The rest of the changes, including test refactoring for better isolation and migration to clap for argument parsing, are well-executed.
…mezone cache - Ensure plaintext credentials use account-specific paths (src/auth.rs, src/auth_commands.rs) - Refactor path helpers into src/credential_store.rs for consistency - Update timezone::invalidate_cache to clear both account and default caches - Fix clippy warnings and format code - Address review comments on PR googleworkspace#496
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is a great enhancement, restoring multi-account support to the CLI. The implementation is thorough, with changes propagated correctly through authentication, helpers, and configuration storage. The refactoring in auth_commands.rs to use clap is a significant improvement for maintainability. However, I've identified a critical security vulnerability related to path traversal. The new --account flag's value is used directly to construct file paths, which could allow a malicious user to read or write files outside of the intended configuration directory. I've added a specific comment with a suggested fix.
…tion issues - Add value_parser to account argument to restrict allowed characters - Ensure account-specific paths are used consistently for plaintext credentials - Plumb account parameter down to Gmail watch sanitization - Fully implement account-aware timezone cache invalidation - Address all high-priority bot review comments on PR googleworkspace#496
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request does a great job of restoring multi-account support by isolating credentials and settings per account. The refactoring of command-line argument parsing to use clap is a significant improvement for maintainability. The changes are extensive and have been applied consistently across the codebase. I found one issue where the --account flag is not respected when used with setup --login, which I've detailed in a specific comment.
…flakiness - Fixed missing account isolation for token caches in `get_token` - Fixed stale token cache deletion to use account-aware file paths - Plumbed `--account` through `gws auth setup` to properly isolate post-setup `login` attempts - Isolated the temporary OAuth credentials file during `login` to prevent concurrent TOCTOU race conditions between different accounts - Added `#[serial_test::serial]` to all environment-mutating async tests in `auth.rs` to fix random parallel CI failures
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces multi-account support, a significant and well-implemented feature. The changes are consistent across the codebase, with good refactoring of authentication and configuration path logic to be account-aware. The introduction of clap for argument parsing in auth_commands.rs is a major improvement for robustness.
My review includes one high-severity suggestion for improving maintainability: refactor the new clap-based argument handling to avoid re-parsing arguments in downstream functions.
…ions - Introduced and structs to hold parsed arguments - Refactored , , and to accept these structs instead of - Removed manual command-line parsing logic in favor of -populated data structures - Updated tests and internal callers to use the new structured data flow
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces multi-account support, a significant and well-implemented feature. The code is well-structured, especially the centralization of path logic in credential_store.rs and the adoption of clap for argument parsing. I've identified a critical issue in the setup flow where credentials for the wrong account might be loaded, and a high-severity suggestion to improve correctness in the authentication code.
…p idiomatic borrows - Fixed setup wizard incorrectly loading default account credentials when pre-filling manual OAuth prompts - Optimized by removing an unnecessary allocation when reading user secrets - Verified all 180+ auth/setup unit tests pass
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces significant and well-implemented multi-account support. The use of a global --account flag to isolate credentials, token caches, and settings is a great feature for security and usability. The refactoring to centralize path logic in credential_store.rs and the adoption of clap for argument parsing in auth_commands.rs are excellent improvements to the codebase's maintainability. The test suite has also been improved with better isolation. Overall this is a solid contribution.
Description
This PR restores multi-account support to the CLI, allowing users to isolate credentials, token caches, and settings (per-account timezones) using a global
--account(alias-A) flag.This addresses the security and auditing needs of AI agents and multi-identity users as requested in #439.
Changes:
credentials.work.enc).--account <name>to the base command surface, enabling multi-account use for all service helpers.auth status,logout, andexportto respect the account flag.auth.rstests to ensure perfect isolation using temporary config directories, preventing state leakage and passing with 630+ tests.Checklist:
AGENTS.mdguidelines (no generatedgoogle-*crates).cargo fmt --allto format the code perfectly.cargo clippy -- -D warningsand resolved all warnings..changeset/multi-account-auth.md) to document my changes.