feat: add Application Default Credentials (ADC) support#125
feat: add Application Default Credentials (ADC) support#125jpoehnelt merged 2 commits intogoogleworkspace:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 0739dd5 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 |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
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. |
…ace#103) Extends the credential chain in get_token() to include ADC as a 4th source: 1. GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE env var 2. Encrypted credentials (~/.config/gws/credentials.enc) 3. Plaintext credentials (~/.config/gws/credentials.json) 4. ADC — GOOGLE_APPLICATION_CREDENTIALS env var, then ~/.config/gcloud/application_default_credentials.json Both authorized_user and service_account ADC formats are detected via the 'type' field and parsed accordingly. This means users can authenticate with: gcloud auth application-default login --client-id-file=client_secret.json and gws will automatically pick up those credentials. Closes googleworkspace#103 Co-Authored-By: Claude <noreply@anthropic.com>
19e4138 to
511dba8
Compare
|
/gemini review |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #125 +/- ##
==========================================
+ Coverage 54.88% 55.46% +0.57%
==========================================
Files 38 38
Lines 13085 13254 +169
==========================================
+ Hits 7182 7351 +169
Misses 5903 5903 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jpoehnelt
left a comment
There was a problem hiding this comment.
duplicated credential-parsing logic that should be extracted, missing service_account ADC test, incorrect well-known path on macOS (dirs::config_dir vs ~/.config/gcloud), silent fallthrough when GOOGLE_APPLICATION_CREDENTIALS points to missing file, incomplete changeset priority list, and a note on unsafe env var usage in tests.
There was a problem hiding this comment.
Code Review
This pull request introduces support for Application Default Credentials (ADC). While the implementation is generally robust, a significant logic flaw was identified: the application may silently fall back to ADC even when a specific account is requested via the --account flag. This can lead to actions being performed by the wrong identity and incorrect token caching. Additionally, consider improvements for documentation consistency in the changeset file, a minor performance optimization in JSON parsing, and the removal of unnecessary unsafe blocks in tests.
- Extract duplicated JSON credential parsing into parse_credential_file()
helper to reduce duplication between GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE
and ADC code paths; uses serde_json::from_value to avoid second string parse
- Fix well-known ADC path on macOS: dirs::config_dir() returns
~/Library/Application Support on macOS, not ~/.config; use
dirs::home_dir().join('.config/gcloud/...') instead
- Hard-error when GOOGLE_APPLICATION_CREDENTIALS points to a missing file
(was: silently fall through to 'No credentials found')
- Add test_load_credentials_adc_env_var_service_account covering service
account credentials loaded via GOOGLE_APPLICATION_CREDENTIALS
- Remove unnecessary unsafe blocks from env var tests (set_var/remove_var
are not unsafe functions; thread safety is already handled by serial_test)
- Update changeset to include GOOGLE_WORKSPACE_CLI_TOKEN at top of lookup
order and clarify ADC fallback behaviour
Addresses review feedback from jpoehnelt on googleworkspace#125.
Co-Authored-By: Claude <noreply@anthropic.com>
|
good catches, updated:
|
|
Re: the Gemini review concern about |
|
Re: the Gemini review concern about |
1 similar comment
|
Re: the Gemini review concern about |
Description
Closes #103
Adds Application Default Credentials (ADC) as a 4th credential source in
get_token().New credential lookup order
GOOGLE_WORKSPACE_CLI_TOKENenv var (raw token)GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILEenv var~/.config/gws/credentials.enc)~/.config/gws/credentials.json)GOOGLE_APPLICATION_CREDENTIALSenv var, then~/.config/gcloud/application_default_credentials.jsonSupported ADC flows
This unlocks two common setups:
User OAuth via gcloud ADC (recommended for personal accounts):
gcloud auth application-default login --client-id-file=client_secret.json # No need to run `gws auth login` separatelyService account via GOOGLE_APPLICATION_CREDENTIALS:
export GOOGLE_APPLICATION_CREDENTIALS=/path/to/service-account.json gws drive files listBoth
authorized_userandservice_accountADC formats are detected automatically via thetypefield.As noted in the issue, most Workspace APIs require scopes granted to a specific user OAuth client, so service account ADC coverage is limited to APIs that support domain-wide delegation. User OAuth ADC with a custom client (via
--client-id-file) works for all standard Workspace APIs.Dry Run Output:
// Not applicable — credential loading logic change, no HTTP request generatedChecklist:
AGENTS.mdguidelines (no generatedgoogle-*crates).cargo fmt --allto format the code perfectly. (no Rust toolchain on build host — CI will verify)cargo clippy -- -D warningsand resolved all warnings. (CI will verify)test_load_credentials_adc_env_var_authorized_user,test_load_credentials_adc_env_var_missing_file)🤖 Generated with Claude Code