This repository was archived by the owner on Mar 6, 2026. It is now read-only.
Commit 94d04e0
authored
fix: raise RefreshError for missing token in impersonated credentials (#1897)
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](https://jules.google.com/task/3520893520890582850)
started by @chalmerlowe*
---------
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>1 parent 0e28e6f commit 94d04e0
2 files changed
Lines changed: 30 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -640,7 +640,14 @@ def refresh(self, request): | |||
| 640 | 640 | "Error getting ID token: {}".format(response.json()) | |
| 641 | 641 | ) | |
| 642 | 642 | ||
| 643 | - id_token = response.json()["token"] | ||
| 643 | + try: | ||
| 644 | + id_token = response.json()["token"] | ||
| 645 | + except (KeyError, ValueError) as caught_exc: | ||
| 646 | + new_exc = exceptions.RefreshError( | ||
| 647 | + "No ID token in response.", response.json() | ||
| 648 | + ) | ||
| 649 | + raise new_exc from caught_exc | ||
| 650 | + | ||
| 644 | 651 | self.token = id_token | |
| 645 | 652 | self.expiry = datetime.utcfromtimestamp( | |
| 646 | 653 | jwt.decode(id_token, verify=False)["exp"] | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -761,6 +761,28 @@ def test_refresh_failure(self): | |||
| 761 | 761 | ||
| 762 | 762 | assert excinfo.match("Error getting ID token") | |
| 763 | 763 | ||
| 764 | + def test_refresh_failure_missing_token_in_200_response(self): | ||
| 765 | + credentials = self.make_credentials(lifetime=None) | ||
| 766 | + credentials.expiry = None | ||
| 767 | + credentials.token = "token" | ||
| 768 | + id_creds = impersonated_credentials.IDTokenCredentials( | ||
| 769 | + credentials, target_audience="audience" | ||
| 770 | + ) | ||
| 771 | + | ||
| 772 | + # Response has 200 OK status but is missing the "token" field | ||
| 773 | + response = mock.create_autospec(transport.Response, instance=False) | ||
| 774 | + response.status_code = http_client.OK | ||
| 775 | + response.json = mock.Mock(return_value={"not_token": "something"}) | ||
| 776 | + | ||
| 777 | + with mock.patch( | ||
| 778 | + "google.auth.transport.requests.AuthorizedSession.post", | ||
| 779 | + return_value=response, | ||
| 780 | + ): | ||
| 781 | + with pytest.raises(exceptions.RefreshError) as excinfo: | ||
| 782 | + id_creds.refresh(None) | ||
| 783 | + | ||
| 784 | + assert excinfo.match("No ID token in response") | ||
| 785 | + | ||
| 764 | 786 | def test_refresh_failure_http_error(self, mock_donor_credentials): | |
| 765 | 787 | credentials = self.make_credentials(lifetime=None) | |
| 766 | 788 | ||
0 commit comments