impl(common): token exchange for external accounts #10328

coryan · 2022-11-29T23:16:52Z

To support external accounts we need to convert the "subject token" returned by the external account source (e.g. an Azure metadata service or a GitHub Actions OIDC source) into "access tokens" returned by GCP's "Security Token Service". In this change we introduce the code to perform such exchanges, including a small integration test on GitHub actions.

Part of the work for #5915

This change is

google-cloud-cpp-bot · 2022-11-29T23:18:59Z

Google Cloud Build Logs
For commit: c252da70a3f89a3722f091984b29a2974e58520f

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

codecov · 2022-11-29T23:39:23Z

Codecov Report

Base: 93.85% // Head: 93.86% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (96bfdce) compared to base (a5ad82f).
Patch coverage: 96.75% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #10328    +/-   ##
========================================
  Coverage   93.85%   93.86%            
========================================
  Files        1597     1600     +3     
  Lines      145363   145763   +400     
========================================
+ Hits       136437   136823   +386     
- Misses       8926     8940    +14

Impacted Files	Coverage Δ
...ud/internal/oauth2_external_account_credentials.cc	`94.73% <94.73%> (ø)`
...ternal/oauth2_external_account_credentials_test.cc	`97.07% <97.07%> (ø)`
...oud/internal/oauth2_external_account_credentials.h	`100.00% <100.00%> (ø)`
google/cloud/internal/async_connection_ready.cc	`89.36% <0.00%> (-4.26%)`	⬇️
...e/cloud/pubsublite/internal/alarm_registry_impl.cc	`97.05% <0.00%> (-2.95%)`	⬇️
google/cloud/pubsub/subscriber_connection_test.cc	`98.78% <0.00%> (-1.22%)`	⬇️
...integration_tests/schema_admin_integration_test.cc	`98.88% <0.00%> (-1.12%)`	⬇️
...le/cloud/internal/default_completion_queue_impl.cc	`96.59% <0.00%> (-0.57%)`	⬇️
google/cloud/completion_queue_test.cc	`97.32% <0.00%> (+0.19%)`	⬆️
...cloud/pubsub/internal/subscription_session_test.cc	`98.33% <0.00%> (+0.33%)`	⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

google-cloud-cpp-bot · 2022-11-30T18:20:42Z

Google Cloud Build Logs
For commit: 002e5879568b8a35eb8dcf99788db0f04b1550f7

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

coryan · 2022-11-30T18:25:31Z

Rebased to resolve conflicts, PTAL.

dbolduc · 2022-11-30T23:19:23Z

google/cloud/internal/oauth2_external_account_credentials.cc

+  auto request = rest_internal::RestRequest(info_.token_url);
+
+  auto client = client_factory_(options_);
+  auto status = client->Post(request, form_data);


optional: I call this thing a sor... but I thought you didn't like that...

The conventional approach would be to just call this response. That might complicate and/or simplify the code below where we introduce response to hold the (moved) OK response.

google/cloud/internal/oauth2_external_account_credentials.cc

google/cloud/internal/external_account_integration_test.cc

dbolduc · 2022-12-01T18:16:57Z

google/cloud/internal/oauth2_external_account_credentials.cc

+  auto client = client_factory_(options_);
+  auto status = client->Post(request, form_data);
+  if (!status) return std::move(status).status();
+  auto response = *std::move(status);
+  if (MapHttpCodeToStatus(response->StatusCode()) != StatusCode::kOk) {
+    return AsStatus(std::move(*response));
+  }
+  auto payload = rest_internal::ReadAll(std::move(*response).ExtractPayload());
+  if (!payload) return std::move(payload.status());


FYI: I am not the best reviewer of this chunk of code. Maybe @scotthart can give it the once-over?

Ack. There is a lot of similar code elsewhere

google/cloud/internal/oauth2_external_account_credentials_test.cc

To support external accounts we need to convert the "subject token" returned by the external account source (e.g. an Azure metadata service or a GitHub Actions OIDC source) into "access tokens" returned by GCP's "Security Token Service". In this change we introduce the code to perform such exchanges, including a small integration test on GitHub actions.

google-cloud-cpp-bot · 2022-12-01T20:35:08Z

Google Cloud Build Logs
For commit: 96bfdce3854cdd221bcdcb58e3085bcb003399ff

Public Access Build Logs
Cloud Build Console (requires auth)
Kokoro Logs (requires auth)

ℹ️ NOTE: Kokoro logs are linked from "Details" below.

dbolduc

FYI: I applied the do not merge label, although I know you know better than to merge while the release is in progress

coryan marked this pull request as ready for review November 30, 2022 04:47

coryan requested a review from a team as a code owner November 30, 2022 04:47

coryan force-pushed the feat-oauth2-workload-identity-federation-pr9 branch from c252da7 to 002e587 Compare November 30, 2022 18:11

dbolduc reviewed Dec 1, 2022

View reviewed changes

coryan added 2 commits December 1, 2022 19:20

Address review comments

96bfdce

coryan force-pushed the feat-oauth2-workload-identity-federation-pr9 branch from 002e587 to 96bfdce Compare December 1, 2022 20:33

dbolduc added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 1, 2022

dbolduc approved these changes Dec 1, 2022

View reviewed changes

coryan removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Dec 2, 2022

coryan merged commit ff0202c into googleapis:main Dec 2, 2022

coryan deleted the feat-oauth2-workload-identity-federation-pr9 branch December 2, 2022 00:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

impl(common): token exchange for external accounts #10328

impl(common): token exchange for external accounts #10328

coryan commented Nov 29, 2022 •

edited

Loading

google-cloud-cpp-bot commented Nov 29, 2022

codecov bot commented Nov 29, 2022 •

edited

Loading

google-cloud-cpp-bot commented Nov 30, 2022

coryan commented Nov 30, 2022

dbolduc Nov 30, 2022

devbww Dec 1, 2022

coryan Dec 1, 2022 •

edited

Loading

dbolduc Dec 1, 2022

coryan Dec 1, 2022

google-cloud-cpp-bot commented Dec 1, 2022

dbolduc left a comment

impl(common): token exchange for external accounts #10328

impl(common): token exchange for external accounts #10328

Conversation

coryan commented Nov 29, 2022 • edited Loading

google-cloud-cpp-bot commented Nov 29, 2022

codecov bot commented Nov 29, 2022 • edited Loading

Codecov Report

google-cloud-cpp-bot commented Nov 30, 2022

coryan commented Nov 30, 2022

dbolduc Nov 30, 2022

Choose a reason for hiding this comment

devbww Dec 1, 2022

Choose a reason for hiding this comment

coryan Dec 1, 2022 • edited Loading

Choose a reason for hiding this comment

dbolduc Dec 1, 2022

Choose a reason for hiding this comment

coryan Dec 1, 2022

Choose a reason for hiding this comment

google-cloud-cpp-bot commented Dec 1, 2022

dbolduc left a comment

Choose a reason for hiding this comment

coryan commented Nov 29, 2022 •

edited

Loading

codecov bot commented Nov 29, 2022 •

edited

Loading

coryan Dec 1, 2022 •

edited

Loading