Skip to content

impl(oauth2): add method populate AllowedLocationsRequest type to Credentials#16080

Merged
scotthart merged 3 commits intogoogleapis:mainfrom
scotthart:rab_allowed_request_type
Apr 16, 2026
Merged

impl(oauth2): add method populate AllowedLocationsRequest type to Credentials#16080
scotthart merged 3 commits intogoogleapis:mainfrom
scotthart:rab_allowed_request_type

Conversation

@scotthart
Copy link
Copy Markdown
Member

@scotthart scotthart commented Apr 15, 2026

The eventual type handling the allowed locations lookups will be implemented as a decorator on oauth2_internal::Credentials, similar to oauth2_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 from oauth2_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 AllowedLocationsRequest returning std::monostate results in a no-op by the eventual RAB decorator (coming soon to a PR near you).

Additionally, Credentials::AuthenticationHeaders was made non-virtual and ApiKeyCredentials now overrides Authorization instead of AuthenticationHeaders.

@scotthart scotthart requested a review from a team as a code owner April 15, 2026 15:55
Copy link
Copy Markdown

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

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

Comment thread google/cloud/internal/oauth2_credentials.h
Comment thread google/cloud/internal/oauth2_external_account_credentials.cc Outdated
Comment thread google/cloud/internal/oauth2_external_account_credentials.cc Outdated
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 15, 2026

Codecov Report

❌ Patch coverage is 90.19608% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.67%. Comparing base (7063180) to head (8b63db5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...oogle/cloud/internal/oauth2_logging_credentials.cc 0.00% 3 Missing ⚠️
google/cloud/internal/oauth2_cached_credentials.cc 0.00% 2 Missing ⚠️
google/cloud/internal/oauth2_credentials.h 0.00% 2 Missing ⚠️
...ternal/oauth2_external_account_credentials_test.cc 94.59% 2 Missing ⚠️
...ud/internal/oauth2_external_account_credentials.cc 97.05% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread google/cloud/internal/oauth2_external_account_credentials.cc
@scotthart scotthart merged commit f46998c into googleapis:main Apr 16, 2026
53 of 59 checks passed
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.

2 participants