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

ServiceAccountCredentials with SSJ flow will get error when invoking refreshAccessToke() #1534

Open
zhumin8 opened this issue Oct 11, 2024 · 0 comments
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@zhumin8
Copy link
Contributor

zhumin8 commented Oct 11, 2024

refreshAccessToke() posts request to OAuth2 token server (see code).
This code path is not exercised for TPC flow via getRequestMetadata(), but it is an public method and is possible to accidentally get triggered by customer's code.

One invocation within this library (that we are patching in #1528 ) is here

@zhumin8 zhumin8 added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p3 Desirable enhancement or fix. May not be included in next release. labels Oct 11, 2024
zhumin8 added a commit that referenced this issue Oct 18, 2024
…verse domain (#1528)

for context: b/340602527

Changes in this pr:
- Override `getUniverseDomain()` to grab source credentials’s universe domain (UD) by default. Always use source credentials UD, not explicit provided UD. (In current design, impersonated credentials may not have universe domain in the outer layer. relay on UD from source credential. This may change in future)
- Fix `isDefaultUniverseDomain()` in `GoogleCredentials` to account for `getUniverseDomain()` overrides in child classes.
- In refreshAccessToken(), use endpoint url pattern to account for TPC case.
  - note that I choose to bypass this refreshIfExpired step because it wrongly steps into code path meant only for OAuth2 token request (GDU flow). Filed #1534 to address this separately. But for GDU flow here, this refresh step is redundant because the SSJ will get re-generated at [initialize request](https://github.com/googleapis/google-auth-library-java/blob/a987ecd06fd25a0048cdb3da6d1df4d029d85d79/oauth2_http/java/com/google/auth/oauth2/ImpersonatedCredentials.java#L558). Also skip this step for SA GDU with SSJ flow.
- Throw IllegalStateException if UD is explicitly set (with parent class setter) and not matching source credential's UD

- Fix toBuilder() to invoke super, and fix related issue with createScoped. (see #1489, #1428); Also fix equals() to compare super first.


Not in this pr: 
- idtoken and signBlob endpoint changes are out-of-scope for this pr, will raise separate pr for it.

sa-to-sa impersonation is successfully E2E tested for TPC usage according to [go/prptst-testing-service-account-impersonation](http://goto.google.com/prptst-testing-service-account-impersonation).



---------

Co-authored-by: Blake Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

1 participant