Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong timezones in compute_engine.IDTokenCredentials expiry #1323

Closed
juzna opened this issue Jun 7, 2023 · 5 comments · Fixed by #1327 or #1328
Closed

Wrong timezones in compute_engine.IDTokenCredentials expiry #1323

juzna opened this issue Jun 7, 2023 · 5 comments · Fixed by #1327 or #1328

Comments

@juzna
Copy link
Contributor

juzna commented Jun 7, 2023

The expiry of compute_engine.IDTokenCredentials is in the local timezone, but it's then compared to utc. This means that an expired token may be used. Expiry of all other credential types are correctly in UTC.

Environment details

  • OS: Linux
  • Python version: 3.11
  • pip version: 23.1.2
  • google-auth version: 2.19.1
  • Tested on GKE 1.24.12-gke.500 with Workload Identity, but AFAICT it would also fail anywhere on GCP

Steps to reproduce

Run on a GCE VM (or a GKE pod).

Configure Python to use some timezone far from UTC, eg export TZ=America/New_York.

import google.auth.compute_engine.credentials
import google.auth.transport.requests

r = google.auth.transport.requests.Request()
creds = google.auth.compute_engine.credentials.IDTokenCredentials(r, target_audience="foo", use_metadata_identity_endpoint=True)
creds.refresh(r)

print(f"expiry: {creds.expiry}")
print(f"expired: {creds.expired}")

Here, expired incorrectly reports false, because it compares the local expiry with utcnow.

Another failure mode is in timezones with a positive offset (eg Europe/Prague), where the token will be treated as not-expired even after it actually expired.

All other credential types use utc for everything, so they don't have the problem. Even the compute engine OAuth2 credentials in the same file (ie just Credentials, not IDTokenCredentials).

Should be a very simple fix, to use UTC datetime everywhere.

@clundin25
Copy link
Contributor

Quick scan: Issue seems similar to #1264. Will take a deeper look later today.

@juzna
Copy link
Contributor Author

juzna commented Jun 8, 2023

Btw the fix is really simple, just changing fromtimestamp to utcfromtimestamp in

return id_token, datetime.datetime.fromtimestamp(payload["exp"])
. We have deployed this as a hotfix to our app and works fine.

juzna added a commit to juzna/google-auth-library-python that referenced this issue Jun 8, 2023
juzna added a commit to juzna/google-auth-library-python that referenced this issue Jun 8, 2023
@clundin25
Copy link
Contributor

Looks like impersonated_credentials.py has the same bug. I will cut a new issue to track it.

➜ rg "\.fromtimestamp" -g '!*test*'
google/auth/compute_engine/credentials.py
392:        return id_token, datetime.datetime.fromtimestamp(payload["exp"])

google/auth/impersonated_credentials.py
457:        self.expiry = datetime.fromtimestamp(jwt.decode(id_token, verify=False)["exp"])

@juzna
Copy link
Contributor Author

juzna commented Jun 9, 2023

Hey, thx for the quick feedback. And sorry, I was too busy to properly finish the PR including tests. I can double-check the impesonated credentials too. I was about to use those in our project too :)

@clundin25
Copy link
Contributor

@juzna, No worries! Let's leave the impersonated credentials for a separate PR. We can own that update

juzna added a commit to juzna/google-auth-library-python that referenced this issue Jun 9, 2023
gcf-merge-on-green bot pushed a commit that referenced this issue Jun 13, 2023
🤖 I have created a release *beep* *boop*
---


## [2.20.0](https://togithub.com/googleapis/google-auth-library-python/compare/v2.19.1...v2.20.0) (2023-06-12)


### Features

* Add public API load_credentials_from_dict ([#1326](https://togithub.com/googleapis/google-auth-library-python/issues/1326)) ([5467ad7](https://togithub.com/googleapis/google-auth-library-python/commit/5467ad75334ee0b5e23522679171cda5fd4edb8a))


### Bug Fixes

* Expiry in compute_engine.IDTokenCredentials ([#1327](https://togithub.com/googleapis/google-auth-library-python/issues/1327)) ([56a6159](https://togithub.com/googleapis/google-auth-library-python/commit/56a6159444467717f5a5e3c04aa678bd0a5881da)), closes [#1323](https://togithub.com/googleapis/google-auth-library-python/issues/1323)
* Expiry in impersonated_credentials.IDTokenCredentials ([#1330](https://togithub.com/googleapis/google-auth-library-python/issues/1330)) ([d1b887c](https://togithub.com/googleapis/google-auth-library-python/commit/d1b887c4bebbe4ad0df6d8f7eb6a6d50355a135d))
* Invalid `dev` version identifiers in `setup.py` ([#1322](https://togithub.com/googleapis/google-auth-library-python/issues/1322)) ([a9b8f12](https://togithub.com/googleapis/google-auth-library-python/commit/a9b8f12db0c3ff4f84939646ba0777d21e68f572)), closes [#1321](https://togithub.com/googleapis/google-auth-library-python/issues/1321)

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
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 a pull request may close this issue.

2 participants