feat: Add trust boundary support for service accounts and impersonation. #1778

nbayati · 2025-06-03T21:50:09Z

design: go/trust-boundaries-auth-sdk-v2

…entials

google/auth/credentials.py

google/auth/impersonated_credentials.py

google/oauth2/service_account.py

tests/test_impersonated_credentials.py

lsirac

Can you handle feature gating the same way we do for pluggable auth? We should align this across all languages.

Also, please add the following test cases:

Please add a test scenario for both service_account.Credentials and impersonated_credentials.Credentials where a valid (non-noop) trust boundary is successfully fetched and cached on the first refresh. A second refresh should then mock a RefreshError from the lookup endpoint. The test should assert that no exception is raised and that the initially cached trust boundary data is preserved on the credentials object. This would validate the fallback logic in _refresh_trust_boundary.
Include explicit tests for both service_account.Credentials in tests/oauth2/test_service_account.py and impersonated_credentials.Credentials in tests/test_impersonated_credentials.py to confirm that the trust boundary lookup is skipped when the credential is instantiated with a non-default universe_domain. This ensures the check is correctly triggered by the concrete classes.

sai-sunder-s · 2025-06-24T04:24:45Z

google/auth/compute_engine/credentials.py

+
+            request = google_auth_requests.Request()
+            try:
+                info = _metadata.get_service_account_info(request, "default")


@harkamaljot you had worked on optimizing this call somewhere else in the code recently. Can you PTAL at this method and see if there are any issues in doing this call or something can be optimized.

I optimized this call when getting token for the service account, however for getting the trust boundary looks like we need service account email. Based on the documentation(http://cloud/iam/docs/reference/credentials/rest/v1/projects.serviceAccounts/getAllowedLocations) and also trying it out manually as well, I get 400 error when querying this endpoint using default.

google/auth/compute_engine/credentials.py

google/auth/credentials.py

lsirac

This is looking good! The main thing is to refactor this, if possible, so that the burden of refreshing trust boundaries is handled by the base class.

For better test coverage, we should add a few test cases for some edge scenarios.

For the new with_trust_boundary() method, it would be great to have tests for each credential type confirming that it returns a new instance and correctly copies all other attributes, modifying only the trust boundary itself.
What is the expected behavior if impersonated_credentials are created with source_credentials that don't have trust boundary support? We should probably add a test to ensure the x-allowed-locations header is not sent in that case.
For service_account.Credentials with always_use_jwt_access=True, does the trust boundary lookup still work as expected?
A test case for when _build_trust_boundary_lookup_url is called when service_account_email is None would be valuable. The code seems to handle this by raising a ValueError, and it's worth having a test to confirm this behavior.

google/auth/_helpers.py

lsirac · 2025-06-25T12:03:47Z

google/auth/external_account.py

Lets revert these changes and keep them for a future PR. We should only add this with the changes for supporting the feature

lsirac · 2025-06-25T12:08:36Z

google/auth/impersonated_credentials.py


    def _metric_header_for_usage(self):
        return metrics.CRED_TYPE_SA_IMPERSONATE

    @_helpers.copy_docstring(credentials.Credentials)
    def refresh(self, request):
        self._update_token(request)
+        self._refresh_trust_boundary(request)


One suggestion for a potential refactoring to improve the design:

Currently, each class that inherits from CredentialsWithTrustBoundary (e.g., compute_engine.Credentials, impersonated_credentials.Credentials, service_account.Credentials) is required to explicitly call self._refresh_trust_boundary(request) within its own refresh method.

To enhance encapsulation and reduce the burden on subclasses, it would be preferable if this call was handled automatically by the base class. Could this be refactored?

That's a great suggestion!
I've refactored the CredentialsWithTrustBoundary base class to handle this. The base refresh() method now handles the entire refresh process. It calls a new abstract method, _refresh_token(), which subclasses must implement, and then it calls _refresh_trust_boundary() itself.

google/oauth2/_client.py

lsirac · 2025-06-25T12:26:30Z

google/oauth2/service_account.py

How does this work for self signed JWTs?

I've updated service_account.Credentials to handle self signed JWTs. When a credential that uses a self-signed JWT also needs to perform a trust boundary lookup, the refresh() method now follows a two-step process:

IAM-Specific JWT for Lookup: It first generates a temporary self-signed JWT where the audience is specifically set to the IAM Credentials API. This token is used exclusively to authenticate the request that fetches the trust boundary information.

Final JWT for Target API: After the trust boundary is successfully retrieved, the method then proceeds to generate the final self-signed JWT with the audience required by the application's target API (e.g., Pub/Sub, Storage).

This approach ensures that the trust boundary lookup is authenticated correctly using a token the IAM service expects, while the final token used by the application is properly configured for its intended service.

google/oauth2/_client.py

…st boundary in the base class.

nbayati · 2025-06-28T01:43:27Z

What is the expected behavior if impersonated_credentials are created with source_credentials that don't have trust boundary support? We should probably add a test to ensure the x-allowed-locations header is not sent in that case.

I agree that the header shouldn't be sent in those cases. Added a unit test to cover this scenario.

For service_account.Credentials with always_use_jwt_access=True, does the trust boundary lookup still work as expected?
Added some logic to support trust boundaries for the self signed jwt flow.

Also implemented the suggested refactoring and test scenarios.
Will update the design to get clarification on some of the credential types we're not sure about, like the idtoken.

lsirac

Looks good, great job!

lsirac · 2025-07-01T13:39:08Z

google/auth/credentials.py

+        """
+        raise NotImplementedError("_refresh_token must be implemented")
+
+    def with_trust_boundary(self, trust_boundary):


Is this method necessary? When would it be used?

The with_* method is part of a design pattern used for credential objects throughout this library.

We treat credential objects as mostly immutable. You'll see similar factory methods like with_scopes(), with_quota_project(), and with_universe_domain().

lsirac · 2025-07-01T13:44:44Z

tests/test_external_account.py

@@ -920,7 +920,9 @@ def test_refresh_impersonation_without_client_auth_success(
            "Content-Type": "application/json",
            "authorization": "Bearer {}".format(token_response["access_token"]),
            "x-goog-api-client": IMPERSONATE_ACCESS_TOKEN_REQUEST_METRICS_HEADER_VALUE,
-            "x-allowed-locations": "0x0",
+            # TODO(negarb): Uncomment and update when trust boundary is supported


Would prefer to not have commented placeholders here and just add in another PR

Thanks for the feedback—I agree it's best to avoid placeholders.

The challenge here is that the commented-out value ("x-allowed-locations": "0x0") is from two years ago and is incorrect under the new design.. Since support for external accounts is out of scope for this particular PR, uncommenting this line would cause tests to fail with the wrong assertion.

I've kept it commented to preserve the test structure for the follow-up PR that will correctly implement this feature for external accounts.

Given this context, would you be okay with keeping these placeholders for this PR, or would you prefer I remove the them and create a separate tracking issue?

lsirac · 2025-07-01T14:26:15Z

tests/test_aws.py

@@ -2057,7 +2059,9 @@ def test_refresh_success_with_impersonation_ignore_default_scopes(
            "authorization": "Bearer {}".format(self.SUCCESS_RESPONSE["access_token"]),
            "x-goog-user-project": QUOTA_PROJECT_ID,
            "x-goog-api-client": IMPERSONATE_ACCESS_TOKEN_REQUEST_METRICS_HEADER_VALUE,
-            "x-allowed-locations": "0x0",
+            # TODO(negarb): Uncomment and update when trust boundary is supported


sai-sunder-s · 2025-07-07T22:45:50Z

google/auth/impersonated_credentials.py

@@ -570,6 +602,7 @@ def refresh(self, request):
            "Content-Type": "application/json",
            metrics.API_CLIENT_HEADER: metrics.token_request_id_token_impersonate(),
        }
+        headers.update(self._target_credentials._get_trust_boundary_header())


Why do we have to pass trust boundary header for id token request? _get_trust_boundary_header might not give the headers because an access token is not available in this credential.

sai-sunder-s · 2025-07-07T23:03:18Z

google/oauth2/service_account.py

+            # We temporarily set self.token for the base lookup method.
+            # The base lookup method will call self.apply() which adds the
+            # authorization header.
+            original_token = self.token


This seems to be welcoming race conditions. What happens if one thread is updating trust boundary and set self.token to iam audience and another thread tries to read self.token

sai-sunder-s · 2025-07-07T23:05:23Z

google/oauth2/service_account.py

@@ -806,13 +920,17 @@ def _refresh_with_iam_endpoint(self, request):
            additional_claims={"scope": "https://www.googleapis.com/auth/iam"},
        )
        jwt_credentials.refresh(request)
+
+        headers = self._get_trust_boundary_header()


you will get NotImplementedError here, thrown from ln 863?

Also, there is no access token in this cred to call trust boundary endpoint

nbayati added 2 commits June 2, 2025 17:01

feat: adds trust boundary lookup support for SA and impersonated cred…

dfd26f6

…entials

Add feature flag, fix ud bug, and update no-op response.

eac0a50

nbayati requested review from a team as code owners June 3, 2025 21:50

nbayati requested review from lsirac and sai-sunder-s June 4, 2025 20:27

nbayati changed the title ~~Add trust boundary support for service accounts and impersonation.~~ feat: Add trust boundary support for service accounts and impersonation. Jun 6, 2025

Add _build_trust_boundary_lookup_url to external account.

baefc0f

sai-sunder-s requested changes Jun 7, 2025

View reviewed changes

Implement additional unit tests for the trust boundary

fb08254

nbayati requested a review from sai-sunder-s June 11, 2025 19:51

lsirac reviewed Jun 23, 2025

View reviewed changes

nbayati added 2 commits June 23, 2025 17:03

implement trust boundary in compute_engine to support GCE instances.

929bd1f

Fix failing unit test, and change acceptable values for the env variable

61654c7

sai-sunder-s reviewed Jun 24, 2025

View reviewed changes

Add unit tests for gce trust boundary.

5edd899

nbayati requested a review from lsirac June 24, 2025 05:49

Use no op method instead of directly comparing values

f064e90

nbayati requested a review from sai-sunder-s June 24, 2025 18:38

nbayati added 2 commits June 24, 2025 17:23

fix a typo in calling the method noop

52b1fc5

Add x-allowed-location header to all IAM requests.

1d26a76

lsirac requested changes Jun 25, 2025

View reviewed changes

nbayati mentioned this pull request Jun 25, 2025

Refactor IAM endpoint calls to ensure consistent header application #1784

Open

nbayati added 4 commits June 27, 2025 11:49

Support self-signed jwt and refactor refresh to handle refreshing tru…

36015c6

…st boundary in the base class.

Fix failing unit tests

cd213c0

Revert changes to external account class file.

c281fee

Additional unit tests and update some old ones.

bd342ea

lsirac approved these changes Jul 1, 2025

View reviewed changes

sai-sunder-s requested changes Jul 7, 2025

View reviewed changes

feat: Add trust boundary support for service accounts and impersonation. #1778

Are you sure you want to change the base?

feat: Add trust boundary support for service accounts and impersonation. #1778

Conversation

nbayati commented Jun 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lsirac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lsirac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nbayati commented Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lsirac left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nbayati commented Jun 28, 2025 •

edited

Loading