Skip to content

test(oauth2): sign mTLS WIF OIDC test JWTs with static test key to prevent key-rotation breakage - #14914

Draft
macastelaz wants to merge 1 commit into
googleapis:mainfrom
macastelaz:fix-wif-mtls-test-key-rotation
Draft

macastelaz wants to merge 1 commit into
googleapis:mainfrom
macastelaz:fix-wif-mtls-test-key-rotation

Conversation

@macastelaz

Copy link
Copy Markdown
Contributor

Summary

Decouples ITWorkloadIdentityFederationTest's mock external OIDC JWT signing (oidc-jwt-1) from the GOOGLE_APPLICATION_CREDENTIALS service account private key, which is rotated every 2 months by secret-rotator.

Root Cause

ITWorkloadIdentityFederationTest previously signed mock external OIDC JWTs (iss: https://storage.googleapis.com/example-oidc-issuer) using the user-managed private key in GOOGLE_APPLICATION_CREDENTIALS (it-service-account@gcloud-devel.iam.gserviceaccount.com). When secret-rotator rotated GOOGLE_APPLICATION_CREDENTIALS, the new private_key_id (kid) no longer matched the static JWKs uploaded to pool-1/providers/oidc-jwt-1, causing 400 invalid_grant failures until oidc-jwt-1 was manually updated.

Fix

  • Sign the mock external OIDC JWTs in ITWorkloadIdentityFederationTest using the static RSA keypair already checked into testresources/mtls/test_key.pem (kid: "mtls-test-key"), while continuing to use GOOGLE_APPLICATION_CREDENTIALS's permanent client_id as the JWT sub claim and client_email for service account impersonation.
  • Update scripts/workloadidentityfederation-setup.sh to include the mtls-test-key public JWK in oidc-jwt-${suffix}.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the workload identity federation integration tests to use a static test private key instead of the service account's private key. It modifies the setup script to append this static key to the JWK set retrieved from Google. The review feedback suggests improving the inline Python script in the setup script by adding a timeout and using a context manager for the network request to prevent potential hangs in CI/CD pipelines.

Comment on lines +140 to +142
import json, sys, urllib.request
url = f"https://www.googleapis.com/service_accounts/v1/jwk/{sys.argv[1]}"
jwks = json.loads(urllib.request.urlopen(url).read().decode())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using urllib.request.urlopen without a timeout can cause the script to hang indefinitely if there are network issues, which can block CI/CD pipelines. Additionally, it is recommended to use a context manager (with statement) to ensure the connection/socket is properly closed after reading.

Suggested change
import json, sys, urllib.request
url = f"https://www.googleapis.com/service_accounts/v1/jwk/{sys.argv[1]}"
jwks = json.loads(urllib.request.urlopen(url).read().decode())
import json, sys, urllib.request
url = f"https://www.googleapis.com/service_accounts/v1/jwk/{sys.argv[1]}"
with urllib.request.urlopen(url, timeout=10) as response:
jwks = json.loads(response.read().decode("utf-8"))

This branch has not been deployed

No deployments
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.

1 participant