Skip to content

Commit 94d04e0

Browse files
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

File tree

2 files changed

+30
-1
lines changed

2 files changed

+30
-1
lines changed

google/auth/impersonated_credentials.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,14 @@ def refresh(self, request):
640640
"Error getting ID token: {}".format(response.json())
641641
)
642642

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+
644651
self.token = id_token
645652
self.expiry = datetime.utcfromtimestamp(
646653
jwt.decode(id_token, verify=False)["exp"]

tests/test_impersonated_credentials.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,28 @@ def test_refresh_failure(self):
761761

762762
assert excinfo.match("Error getting ID token")
763763

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+
764786
def test_refresh_failure_http_error(self, mock_donor_credentials):
765787
credentials = self.make_credentials(lifetime=None)
766788

0 commit comments

Comments
 (0)