Skip to content

test(auth): Assert self-signed JWT is used by ImpersonatedCredentials#13487

Draft
westarle wants to merge 1 commit into
googleapis:mainfrom
westarle:fix-test-gap-impersonated-jwt
Draft

test(auth): Assert self-signed JWT is used by ImpersonatedCredentials#13487
westarle wants to merge 1 commit into
googleapis:mainfrom
westarle:fix-test-gap-impersonated-jwt

Conversation

@westarle

Copy link
Copy Markdown
Contributor

This commit adds an assertion to ImpersonatedCredentialsTest to verify that when a ServiceAccountJwtAccessCredentials is used as the source credential, the actual HTTP request to the impersonation endpoint correctly sends the generated self-signed JWT in the Authorization header.

This brings the Java library's test suite into alignment with the expected auth specification, validating that source credentials properly inject their authentication headers during token exchanges.

This commit adds an assertion to ImpersonatedCredentialsTest to verify that when a ServiceAccountJwtAccessCredentials is used as the source credential, the actual HTTP request to the impersonation endpoint correctly sends the generated self-signed JWT in the Authorization header.

This brings the Java library's test suite into alignment with the expected auth specification, validating that source credentials properly inject their authentication headers during token exchanges.

@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 adds assertions to the refreshAccessToken_success_SSJflow test in ImpersonatedCredentialsTest.java to verify that the authorization header contains a valid self-signed JWT with the correct issuer and audience. The reviewer suggested a more robust assertion approach by checking for the presence of the unique strings directly, rather than asserting exact JSON fragments which can be fragile due to potential formatting variations.

Comment on lines +527 to +529
String payload = new String(java.util.Base64.getUrlDecoder().decode(parts[1]), java.nio.charset.StandardCharsets.UTF_8);
assertTrue(payload.contains("\"iss\":\"" + SA_CLIENT_EMAIL + "\""), "JWT must be issued by the source service account");
assertTrue(payload.contains("\"aud\":\"" + DEFAULT_IMPERSONATION_URL + "\""), "JWT audience must be the impersonation endpoint");

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

Asserting exact JSON fragments like "iss":" or "aud":" is fragile because JSON serializers can vary in formatting (e.g., adding spaces around colons, escaping slashes / in URLs, or key ordering). Since SA_CLIENT_EMAIL and DEFAULT_IMPERSONATION_URL are highly specific and unique strings, asserting their presence in the decoded payload directly is much more robust and less prone to breaking on library or environment updates.

Suggested change
String payload = new String(java.util.Base64.getUrlDecoder().decode(parts[1]), java.nio.charset.StandardCharsets.UTF_8);
assertTrue(payload.contains("\"iss\":\"" + SA_CLIENT_EMAIL + "\""), "JWT must be issued by the source service account");
assertTrue(payload.contains("\"aud\":\"" + DEFAULT_IMPERSONATION_URL + "\""), "JWT audience must be the impersonation endpoint");
String payload = new String(java.util.Base64.getUrlDecoder().decode(parts[1]), java.nio.charset.StandardCharsets.UTF_8);
assertTrue(payload.contains(SA_CLIENT_EMAIL), "JWT must be issued by the source service account");
assertTrue(payload.contains(DEFAULT_IMPERSONATION_URL), "JWT audience must be the impersonation endpoint");

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