feat: Add support for Workforce Identity Pools external credentials. by amanda-tarafa · Pull Request #2200 · googleapis/google-api-dotnet-client

amanda-tarafa · 2022-08-31T14:04:21Z

jpassing · 2022-09-01T01:59:18Z

My understanding is that Workforce Identity Federation shouldn't allow automatic service account impersonation. I haven't seen anything in the code to check for that.

But it's also possible that my understanding is wrong as the spec is pretty vague.

amanda-tarafa · 2022-09-01T08:35:18Z

My understanding is that Workforce Identity Federation shouldn't allow automatic service account impersonation. I haven't seen anything in the code to check for that.

But it's also possible that my understanding is wrong as the spec is pretty vague.

There's no check for that. I also had that question and also couldn't find anything about it. If my understanding of Java's implementation is right, they are not checking either. So, the implementation for Workforce is the same as for Workload in that respect:

If impersonation is configured on the credential JSON we do it implicitly and user code cannot use the resulting credential to impersontate further.
If impersonation is not configured the user code can use the resulting credential if they want, else, the STS returned access token will be used.

jskeet · 2022-09-01T09:34:25Z

+        /// </summary>
+        /// <remarks>
+        /// If this external account credential represents a Workforce Pool
+        /// enabled identity and this values is not specified, then an API key needs to be


values => value here and below

jskeet

Looks fine so far - one more question I'll follow up on by email.

jskeet · 2022-09-01T10:22:02Z

+            GoogleOptions = googleOptions;
+            AuthenticationHeader = authenticationHeader;
+        }
+


Nit: remove one of these blank lines?

jskeet · 2022-09-01T10:23:52Z

    /// Storage from a web application without a user's involvement.
    /// <para>
-    /// <code>ServiceAccountCredential</code> inherits from this class in order to support Service Account. More
+    /// <see cref="ServiceAccountCredential"/> inherits from this class in order to support Service Account. More


Nit: "in order to support Service Account" => "in order to support Service Accounts"? (While we're changing the comment anyway...)

jskeet · 2022-09-01T10:24:18Z

    /// https://cloud.google.com/compute/docs/authentication.
    /// </para>
+    /// <para>
+    /// <see cref="ExternalAccountCredential"/> inherits from this class to support both Workload Indentity Federation


Indentity => Identity

jskeet · 2022-09-01T10:24:38Z

+    /// and Workforce Identity Federation. You can read more about these topics in
+    /// https://cloud.google.com/iam/docs/workload-identity-federation and
+    /// https://cloud.google.com/iam/docs/workforce-identity-federation respectively.
+    /// Note that in the case of Workforce Identity Federation, the external account does not represent a service acount


acount => account

jskeet · 2022-09-01T10:24:51Z

+    /// https://cloud.google.com/iam/docs/workforce-identity-federation respectively.
+    /// Note that in the case of Workforce Identity Federation, the external account does not represent a service acount
+    /// but a user account, so, the fact that <see cref="ExternalAccountCredential"/> inherits from <see cref="ServiceCredential"/>
+    /// might be construed as missleading. In reality <see cref="ServiceCredential"/> it's not tied to a service account


missleading => misleading

jskeet · 2022-09-01T10:25:29Z

+    /// Note that in the case of Workforce Identity Federation, the external account does not represent a service acount
+    /// but a user account, so, the fact that <see cref="ExternalAccountCredential"/> inherits from <see cref="ServiceCredential"/>
+    /// might be construed as missleading. In reality <see cref="ServiceCredential"/> it's not tied to a service account
+    /// implementation wise, only name wise, for instance, a better name for this class might have been NoUserFlowCredential, and


"implementation wise, only name wise" => "in terms of implementation; only in terms of name". (And then a new sentence.)

lsirac · 2022-09-01T17:16:42Z

@jpassing @amanda-tarafa

We support Workforce Identity Federation + service account impersonation in the other auth libraries, we just don't document it / tell people to use it

amanda-tarafa · 2022-09-01T17:23:15Z

@jpassing @amanda-tarafa

We support Workforce Identity Federation + service account impersonation in the other auth libraries, we just don't document it / tell people to use it

Thanks Leo, then this implementation is correct!

amanda-tarafa

@jskeet I've addressed all your comments, and also replied internally.
Second commit contains review changes. Thanks.

amanda-tarafa · 2022-09-01T17:06:19Z

+            GoogleOptions = googleOptions;
+            AuthenticationHeader = authenticationHeader;
+        }
+


amanda-tarafa · 2022-09-01T17:07:32Z

    /// Storage from a web application without a user's involvement.
    /// <para>
-    /// <code>ServiceAccountCredential</code> inherits from this class in order to support Service Account. More
+    /// <see cref="ServiceAccountCredential"/> inherits from this class in order to support Service Account. More


amanda-tarafa · 2022-09-01T17:08:16Z

    /// https://cloud.google.com/compute/docs/authentication.
    /// </para>
+    /// <para>
+    /// <see cref="ExternalAccountCredential"/> inherits from this class to support both Workload Indentity Federation


amanda-tarafa · 2022-09-01T17:10:25Z

+    /// and Workforce Identity Federation. You can read more about these topics in
+    /// https://cloud.google.com/iam/docs/workload-identity-federation and
+    /// https://cloud.google.com/iam/docs/workforce-identity-federation respectively.
+    /// Note that in the case of Workforce Identity Federation, the external account does not represent a service acount


amanda-tarafa · 2022-09-01T17:10:32Z

+    /// https://cloud.google.com/iam/docs/workforce-identity-federation respectively.
+    /// Note that in the case of Workforce Identity Federation, the external account does not represent a service acount
+    /// but a user account, so, the fact that <see cref="ExternalAccountCredential"/> inherits from <see cref="ServiceCredential"/>
+    /// might be construed as missleading. In reality <see cref="ServiceCredential"/> it's not tied to a service account


amanda-tarafa · 2022-09-01T17:10:39Z

+    /// Note that in the case of Workforce Identity Federation, the external account does not represent a service acount
+    /// but a user account, so, the fact that <see cref="ExternalAccountCredential"/> inherits from <see cref="ServiceCredential"/>
+    /// might be construed as missleading. In reality <see cref="ServiceCredential"/> it's not tied to a service account
+    /// implementation wise, only name wise, for instance, a better name for this class might have been NoUserFlowCredential, and


lsirac · 2022-09-01T17:30:27Z

        public string ServiceAccountImpersonationUrl { get; }

+        /// <summary>
+        /// The GCP project ID or project number to be used for Workforce Pools


At the time I wrote the doc the project ID was not yet supported. Not sure if that changed, just FYI

Public docs still only mention project number, I'll change the text to only mention project number.

Features - googleapis#2228 Supports Quota Project environment variable for ADC. - googleapis#2219 Removes obsolete OOB code. BREAKING CHANGE: We have removed OOB support from the library because the feature is not supported by Google OAuth as of October 3rd 2022. See the [official announcement](https://developers.googleblog.com/2022/02/making-oauth-flows-safer.html#disallowed-oob) for more information. - googleapis#2216 Log errors when trying to reach Compute's metadata server. - googleapis#2177, googleapis#2197, googleapis#2200, googleapis#2206, googleapis#2215, googleapis#2235 Support for Workload Identity Federation and Worforce Identity Federation. - googleapis#2103 Compute credentials support explicit scopes. - googleapis#2096 Improves deserialization error handling.

Features - #2228 Supports Quota Project environment variable for ADC. - #2219 Removes obsolete OOB code. BREAKING CHANGE: We have removed OOB support from the library because the feature is not supported by Google OAuth as of October 3rd 2022. See the [official announcement](https://developers.googleblog.com/2022/02/making-oauth-flows-safer.html#disallowed-oob) for more information. - #2216 Log errors when trying to reach Compute's metadata server. - #2177, #2197, #2200, #2206, #2215, #2235 Support for Workload Identity Federation and Worforce Identity Federation. - #2103 Compute credentials support explicit scopes. - #2096 Improves deserialization error handling.

And support version: 1.58.0-beta02 => 1.58.0 Bug fixes: - googleapis#2273 Don't attempt to use an empty/null media type in batch responses - googleapis#2251 Which fixes a typo on then name of one of the AWS Credential environment variables. - googleapis#2264 Which adds a missing line break in AWS Credential canonical request. Features - googleapis#2269 Support per-request max retry configuration. - googleapis#2228 Supports Quota Project environment variable for ADC. - googleapis#2219 Removes obsolete OOB code. BREAKING CHANGE: We have removed OOB support from the library because the feature is not supported by Google OAuth as of October 3rd 2022. See the [official announcement](https://developers.googleblog.com/2022/02/making-oauth-flows-safer.html#disallowed-oob) for more information. - googleapis#2216 Log errors when trying to reach Compute's metadata server. - googleapis#2177, googleapis#2197, googleapis#2200, googleapis#2206, googleapis#2215, googleapis#2235 Support for Workload Identity Federation and Worforce Identity Federation. - googleapis#2103 Compute credentials support explicit scopes. - googleapis#2096 Improves deserialization error handling.

And support version: 1.58.0-beta02 => 1.58.0 Dependencies: - Client libraries no longer target netstandard1.0 or net40. Bug fixes: - googleapis#2273 Don't attempt to use an empty/null media type in batch responses - googleapis#2251 Which fixes a typo on then name of one of the AWS Credential environment variables. - googleapis#2264 Which adds a missing line break in AWS Credential canonical request. Features - googleapis#2269 Support per-request max retry configuration. - googleapis#2228 Supports Quota Project environment variable for ADC. - googleapis#2219 Removes obsolete OOB code. BREAKING CHANGE: We have removed OOB support from the library because the feature is not supported by Google OAuth as of October 3rd 2022. See the [official announcement](https://developers.googleblog.com/2022/02/making-oauth-flows-safer.html#disallowed-oob) for more information. - googleapis#2216 Log errors when trying to reach Compute's metadata server. - googleapis#2177, googleapis#2197, googleapis#2200, googleapis#2206, googleapis#2215, googleapis#2235 Support for Workload Identity Federation and Worforce Identity Federation. - googleapis#2103 Compute credentials support explicit scopes. - googleapis#2096 Improves deserialization error handling.

And support version: 1.58.0-beta02 => 1.58.0 Dependencies: - Client libraries no longer target netstandard1.0 or net40. Bug fixes: - #2273 Don't attempt to use an empty/null media type in batch responses - #2251 Which fixes a typo on then name of one of the AWS Credential environment variables. - #2264 Which adds a missing line break in AWS Credential canonical request. Features - #2269 Support per-request max retry configuration. - #2228 Supports Quota Project environment variable for ADC. - #2219 Removes obsolete OOB code. BREAKING CHANGE: We have removed OOB support from the library because the feature is not supported by Google OAuth as of October 3rd 2022. See the [official announcement](https://developers.googleblog.com/2022/02/making-oauth-flows-safer.html#disallowed-oob) for more information. - #2216 Log errors when trying to reach Compute's metadata server. - #2177, #2197, #2200, #2206, #2215, #2235 Support for Workload Identity Federation and Worforce Identity Federation. - #2103 Compute credentials support explicit scopes. - #2096 Improves deserialization error handling.

amanda-tarafa requested review from a team and jskeet August 31, 2022 14:04

jskeet reviewed Sep 1, 2022

View reviewed changes

jskeet approved these changes Sep 1, 2022

View reviewed changes

amanda-tarafa force-pushed the workforce-pools branch from b47669b to 522effd Compare September 1, 2022 17:25

amanda-tarafa commented Sep 1, 2022

View reviewed changes

lsirac reviewed Sep 1, 2022

View reviewed changes

feat: Add support for Workforce Identity Pools external credentials.

34ce5a2

amanda-tarafa force-pushed the workforce-pools branch from 522effd to 34ce5a2 Compare September 2, 2022 09:14

amanda-tarafa merged commit 92d5708 into googleapis:main Sep 2, 2022

amanda-tarafa deleted the workforce-pools branch September 2, 2022 09:20

amanda-tarafa mentioned this pull request Oct 18, 2022

Update support version: 1.57.0 -> 1.58.0-beta01 #2237

Merged

amanda-tarafa mentioned this pull request Nov 30, 2022

Update version: 1.57.0 => 1.58.0 #2274

Merged

Conversation

amanda-tarafa commented Aug 31, 2022

Uh oh!

jpassing commented Sep 1, 2022

Uh oh!

amanda-tarafa commented Sep 1, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jskeet 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!

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!

lsirac commented Sep 1, 2022

Uh oh!

amanda-tarafa commented Sep 1, 2022

Uh oh!

amanda-tarafa 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!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants