feat(auth): add gws auth use-adc command #99
feat(auth): add gws auth use-adc command #99kylegallatin wants to merge 2 commits intogoogleworkspace:mainfrom
Conversation
Adds support for importing Application Default Credentials from gcloud, eliminating the need for custom OAuth clients in team setups. Features: - New `gws auth use-adc` command to import ADC credentials - Includes quota_project_id support for proper API quota tracking - Adds x-goog-user-project header to API requests - Simplifies team onboarding to 2 commands - No network validation - credentials validated on first API use Technical changes: - New handle_use_adc() function in src/auth_commands.rs (simple import, no network calls) - Added get_quota_project_id() helper in src/executor.rs - Request builder includes quota project header when available - Supports both ~/.config/gcloud/ and ~/Library/Application Support/gcloud/ paths This makes team authentication simpler - everyone uses Google's built-in OAuth client instead of creating/sharing custom clients. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 611728d 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 to the authentication workflow by integrating Google's Application Default Credentials (ADC). It provides a new command that allows users to import their 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
|
|
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. |
![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 gws auth use-adc command, simplifying authentication by importing gcloud's Application Default Credentials. The implementation is solid, and the documentation updates are clear. I've identified one high-severity bug where the new logic for retrieving the quota project ID doesn't respect credentials provided via an environment variable. I've also included a few medium-severity suggestions to improve code robustness and maintainability in the new handle_use_adc function.
Note: Security Review did not run due to the size of the PR.
| fn get_quota_project_id() -> Result<String, String> { | ||
| // Check encrypted credentials first | ||
| if let Ok(contents) = crate::credential_store::load_encrypted() { | ||
| if let Ok(creds) = serde_json::from_str::<serde_json::Value>(&contents) { | ||
| if let Some(quota_project) = creds.get("quota_project_id").and_then(|v| v.as_str()) { | ||
| return Ok(quota_project.to_string()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Check plaintext credentials | ||
| let config_dir = dirs::config_dir() | ||
| .unwrap_or_else(|| std::path::PathBuf::from(".")) | ||
| .join("gws"); | ||
| let plain_path = config_dir.join("credentials.json"); | ||
|
|
||
| if plain_path.exists() { | ||
| if let Ok(contents) = std::fs::read_to_string(&plain_path) { | ||
| if let Ok(creds) = serde_json::from_str::<serde_json::Value>(&contents) { | ||
| if let Some(quota_project) = creds.get("quota_project_id").and_then(|v| v.as_str()) | ||
| { | ||
| return Ok(quota_project.to_string()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Err("No quota_project_id found".to_string()) | ||
| } |
There was a problem hiding this comment.
This function reimplements logic for finding and parsing credentials, but it's incomplete. It doesn't check for the GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE environment variable, which is a supported way to provide credentials. This will cause get_quota_project_id to fail and the x-goog-user-project header to be omitted when credentials are set via this environment variable.
To fix this and avoid code duplication, the logic for loading credentials should be centralized and reused here. Consider moving plain_credentials_path() from auth_commands.rs to a shared module (e.g., a new config.rs) and using it here.
| let adc_path = if adc_path_config.exists() { | ||
| adc_path_config | ||
| } else if adc_path_app_support.exists() { | ||
| adc_path_app_support | ||
| } else { | ||
| adc_path_config // default for error message | ||
| }; |
There was a problem hiding this comment.
This logic for determining adc_path works, but has a small issue. If neither path exists, the subsequent error message on line 936 will only show adc_path_config, which can be misleading on platforms like Windows where the correct path is adc_path_app_support. Consider modifying this to provide a more informative error message that lists all paths that were checked.
| let client_id = adc | ||
| .get("client_id") | ||
| .and_then(|v| v.as_str()) | ||
| .ok_or_else(|| GwsError::Validation("ADC credentials missing 'client_id'".to_string()))?; | ||
|
|
||
| let client_secret = adc | ||
| .get("client_secret") | ||
| .and_then(|v| v.as_str()) | ||
| .ok_or_else(|| { | ||
| GwsError::Validation("ADC credentials missing 'client_secret'".to_string()) | ||
| })?; | ||
|
|
||
| let refresh_token = adc | ||
| .get("refresh_token") | ||
| .and_then(|v| v.as_str()) | ||
| .ok_or_else(|| { | ||
| GwsError::Validation("ADC credentials missing 'refresh_token'".to_string()) | ||
| })?; |
There was a problem hiding this comment.
The validation logic for client_id, client_secret, and refresh_token is repeated. This could be refactored into a helper function to reduce code duplication and improve readability. For example:
fn get_string_field<'a>(json: &'a serde_json::Value, field: &str) -> Result<&'a str, GwsError> {
json.get(field)
.and_then(|v| v.as_str())
.ok_or_else(|| GwsError::Validation(format!("ADC credentials missing '{field}'")))
}
// ...
let client_id = get_string_field(&adc, "client_id")?;
let client_secret = get_string_field(&adc, "client_secret")?;
let refresh_token = get_string_field(&adc, "refresh_token")?;
jpoehnelt
left a comment
jpoehnelt
left a comment
There was a problem hiding this comment.
This eliminates the need to create and share custom OAuth clients.
This isn't true. Most Workspace APIs will not be able to piggyback on the gcloud OAuth. However, ADC can be setup with a custom OAuth client and should support this some way or another. Much lower priority in this case though. Should check on how to accept the ADC credentials file.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@jpoehnelt yeah apologies that phrasing is a bit too ambitious. Really the goal here is just to alleviate the amount of Google Cloud setup required to get started. Given |
jpoehnelt
added
area: auth
cla: no
2 participants
Description
Adds a new
gws auth use-adccommand that simplifies team authentication by importing Application Default Credentials fromgcloud. This eliminates the need to createand share custom OAuth clients.
Usage