Skip to content

Conversation

@google-labs-jules
Copy link
Contributor

@google-labs-jules google-labs-jules bot commented Dec 19, 2025

Previously, IDTokenCredentials.refresh would raise a KeyError if the response from the IAM server was 200 OK but did not contain the expected "token" field. This change wraps the token extraction in a try...except block to catch KeyError (and ValueError for malformed JSON) and raises a google.auth.exceptions.RefreshError instead, which is the expected behavior for credential refresh failures.

I've added a new test tests/test_impersonated_credentials.py to verify:

  1. A 200 OK response with a missing token now raises RefreshError.
  2. Non-200 responses (e.g., 403) still raise RefreshError as before.

This PR replaces the PR #1167 from 3 years ago that addressed this issue, but was less robust/not fully correct.

PR created automatically by Jules for task 3520893520890582850 started by @chalmerlowe

Instead of crashing with a KeyError when the ID token is missing from the
response (even if the status code is 200), raise a proper RefreshError.

Fixes #1167
@google-labs-jules
Copy link
Contributor Author

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!


For security, I will only act on instructions from the user who triggered this task.

New to Jules? Learn more at jules.google/docs.

Instead of crashing with a KeyError when the ID token is missing from the
response (even if the status code is 200), raise a proper RefreshError.

Fixes #1167
@chalmerlowe chalmerlowe self-assigned this Dec 19, 2025
@chalmerlowe chalmerlowe added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 19, 2025
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 19, 2025
@chalmerlowe chalmerlowe marked this pull request as ready for review December 19, 2025 20:51
@chalmerlowe chalmerlowe requested review from a team as code owners December 19, 2025 20:51
Copy link
Contributor

@chalmerlowe chalmerlowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@daniel-sanche daniel-sanche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chalmerlowe chalmerlowe merged commit 94d04e0 into main Dec 20, 2025
14 of 16 checks passed
@chalmerlowe chalmerlowe deleted the patch-1-3520893520890582850 branch December 20, 2025 08:36
daniel-sanche added a commit that referenced this pull request Jan 5, 2026
PR created by the Librarian CLI to initialize a release. Merging this PR
will auto trigger a release.

Librarian Version: v1.0.1
Language Image:
us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/python-librarian-generator@sha256:b8058df4c45e9a6e07f6b4d65b458d0d059241dd34c814f151c8bf6b89211209
<details><summary>google-auth: 2.46.0</summary>

##
[2.46.0](v2.45.0...v2.46.0)
(2026-01-05)

### Features

* Recognize workload certificate config in
has_default_client_cert_source for mTLS for Agentic Identities (#1907)
([0b9107d](0b9107d5))

### Bug Fixes

* Fix test coverage for mtls_helper (#1886)
([02e7163](02e71631))

* use .read() instead of .content.read() in aiohttp transport (#1899)
([12f4470](12f4470f))

* add types to default and verify_token and Request __init__ based on
comments in the source code. (#1588)
([59a5f58](59a5f588))

* fix the document of secure_authorized_session (#1536)
([5d00147](5d001470))

* raise RefreshError for missing token in impersonated credentials
(#1897)
([94d04e0](94d04e09))

* remove setup.cfg configuration for creating universal wheels (#1693)
([c767531](c767531c))

### Documentation

* update urllib3 docstrings for v2 compatibility (#1903)
([3f1aeea](3f1aeea2))

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants