impl(oauth2): add method populate AllowedLocationsRequest type to Credentials#16080
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements the AllowedLocationsRequest mechanism across several OAuth2 credential types to support location-restricted requests. The changes involve defining request structures in oauth2_credentials.h and implementing the corresponding virtual methods in specific credential classes. Feedback highlights a critical bug where the Authorization method is missing from the base Credentials class, causing compilation failures in subclasses. Furthermore, the reviewer suggests replacing std::regex with more efficient alternatives and identifies a logic error in ExternalAccountCredentials where a project ID is incorrectly used as a pool ID.
1463286 to
1ea2cd1
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #16080 +/- ##
==========================================
- Coverage 92.67% 92.67% -0.01%
==========================================
Files 2348 2348
Lines 217492 217564 +72
==========================================
+ Hits 201556 201619 +63
- Misses 15936 15945 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The eventual type handling the allowed locations lookups will be implemented as a decorator on
oauth2_internal::Credentials, similar tooauth2_internal::CachedCredentials. However, the allowed locations RPC needs different data depending on the credential type (service account, workforce, workload) so in order to percolate that info up to the decorator a new method, AllowedLocationsRequest, is being added. One driving factor behind the decision to make it a decorator was its need to use the cached access token fromoauth2_internal::CachedCredentials.While we anticipate this feature being GA for the next release, the GOOGLE_CLOUD_CPP_TESTING_ENABLE_RAB macro was added in order to quickly disable the feature if that's not the case. Adding it to only the bazel path allows us to test both paths of the conditional code. A call to
AllowedLocationsRequestreturningstd::monostateresults in a no-op by the eventual RAB decorator (coming soon to a PR near you).Additionally,
Credentials::AuthenticationHeaderswas made non-virtual andApiKeyCredentialsnow overridesAuthorizationinstead ofAuthenticationHeaders.