feat(auth): support Domain-Wide Delegation for Service Accounts#543
feat(auth): support Domain-Wide Delegation for Service Accounts#543dumko2001 wants to merge 7 commits intogoogleworkspace:mainfrom
Conversation
🦋 Changeset detectedLatest commit: a32bb8d The changes in this PR will be included in the next version bump. 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 introduces a significant enhancement for enterprise environments by enabling service accounts to impersonate specific users within a Google Workspace domain. This capability is essential for automating tasks that require user-specific permissions, streamlining operations without the need for individual user credentials. Highlights
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. 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 adds support for Domain-Wide Delegation for Service Accounts by reading the GOOGLE_WORKSPACE_IMPERSONATE_USER environment variable. The implementation is mostly correct, but I've identified a security concern regarding the logging of the impersonated user's email without sanitization. This could lead to terminal escape sequence injection. Additionally, the pull request lacks tests for this new functionality, which is mentioned as a requirement in the PR's checklist.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for Domain-Wide Delegation, which is a great feature for enterprise automation. However, I've found two significant issues with the current implementation. First, there is a critical bug where tokens for the service account and impersonated users will collide in the cache, leading to incorrect authentication states. Second, there is a high-severity security issue where user-provided data from an environment variable is logged without sanitization, creating a risk of terminal escape sequence injection. Both issues need to be addressed before merging.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for Domain-Wide Delegation for Service Accounts by allowing user impersonation via the GOOGLE_WORKSPACE_IMPERSONATE_USER environment variable. The implementation correctly uses the yup-oauth2 builder pattern to add the subject for impersonation. My review includes one high-severity security recommendation to prevent terminal escape sequence injection by sanitizing the user-provided value in a debug log message.
- Sanitize impersonated user email in debug log to prevent terminal escape injection - Fix critical token cache collision by using unique cache file per impersonated user (SHA-256 hash) - Add tests proving the cache collision bug and validating the fix Addresses gemini-code-assist security and critical feedback on PR googleworkspace#543
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request successfully introduces support for Domain-Wide Delegation for Service Accounts, enabling user impersonation. This feature is implemented by generating unique token cache paths for impersonated users, effectively preventing token collisions. The changes include a new sa_token_cache_path function and its integration into the authentication flow, along with comprehensive test cases that validate the new logic and demonstrate the resolution of a previous token collision bug.
Use all 32 bytes of SHA-256 hash for maximum collision resistance as suggested in code review feedback.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for Domain-Wide Delegation for Service Accounts by allowing user impersonation via the GOOGLE_WORKSPACE_IMPERSONATE_USER environment variable. The implementation correctly creates unique token cache files for impersonated users to prevent collisions, using a hash of the user's email. The changes are well-tested.
However, I've identified one high-severity issue: the existing logic for cleaning up stale, encrypted token caches is not updated to handle the new cache file naming scheme. This will lead to leftover files and persistent warnings for users in certain scenarios. I've left a comment with details on how to address this.
When encrypted credentials become undecryptable (e.g., key migration), clean up all impersonation token cache files (sa_imp_*.json) in addition to standard token caches to prevent stale file warnings.
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request introduces support for Domain-Wide Delegation for Service Accounts, which is a valuable feature for enterprise automation. The changes include a new function to compute unique token cache paths for impersonated users, modifications to the token retrieval logic to incorporate this, and a cleanup mechanism for stale impersonation cache files. New tests have been added to cover the token cache path logic, demonstrating the fix for token collision issues. Overall, the changes are well-structured and address the stated objective.
Sanitize path and error strings in eprintln! calls during token cache cleanup to prevent terminal escape sequence injection.
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request successfully implements Domain-Wide Delegation (impersonation) for Service Accounts, addressing the critical issue of token cache collisions by introducing unique, hash-based cache paths for impersonated users. The changes are well-structured, integrating the new cache path logic into the authentication flow and including appropriate cleanup for stale cache files. Comprehensive unit tests have been added to validate the new functionality and demonstrate the fix for the token collision bug. No high or critical issues were identified during the review.
2 participants
Description
Enable user impersonation (Domain-Wide Delegation) for Service Account authentication via the
GOOGLE_WORKSPACE_IMPERSONATE_USERenvironment variable. This allows administrators to perform actions on behalf of other users in the Workspace domain, which is a standard requirement for enterprise automation.Fixes #528
Checklist:
AGENTS.mdguidelines (no generatedgoogle-*crates).cargo fmt --allto format the code perfectly.cargo clippy -- -D warningsand resolved all warnings.pnpx changeset) to document my changes.