feat: Adds support for user defined subject token suppliers in AWSCredentials and IdentityPoolCredentials #1336

aeitzman · 2023-11-27T17:47:41Z

see go/programmable-auth-design.
Also adds quality of life improvements for building credentials directly via the builder methods, and a subject token type enum.

…edential types

…credentials in code.

# Conflicts: # oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java

# Conflicts: # oauth2_http/javatests/com/google/auth/oauth2/AwsCredentialsTest.java

# Conflicts: # oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java # oauth2_http/javatests/com/google/auth/oauth2/AwsCredentialsTest.java # oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java

oauth2_http/java/com/google/auth/oauth2/AwsCredentials.java

oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java

oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java

Co-authored-by: Leo <39062083+lsirac@users.noreply.github.com>

lsirac · 2023-12-06T13:36:47Z

We should also update the README here + add integration tests

oauth2_http/java/com/google/auth/oauth2/AwsSecurityCredentialsProvider.java

lsirac · 2024-01-09T21:21:15Z

oauth2_http/java/com/google/auth/oauth2/AwsCredentials.java

@@ -103,13 +107,15 @@ public class AwsCredentials extends ExternalAccountCredentials {
      this.awsSecurityCredentialsProvider =
          new ProgrammaticAwsSecurityCredentialsProvider(
              builder.awsSecurityCredentialsSupplier, this.awsRegion);
+      this.builtWithSupplier = true;


Hmm, we shouldn't have to track this. Why not handle this in the builder so we can set the provider directly and not have to handle this logic here?

Figured out a better way to handle in the builder, we can just check whether the builder has a credential source, and only set the supplier if it doesn't

oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java

lsirac · 2024-01-09T21:42:10Z

oauth2_http/java/com/google/auth/oauth2/IdentityPoolSubjectTokenProvider.java

+   *
+   * @return a valid subject token.
+   * @throws IOException
+   */


Should we just call this getCredentials()?

I don't think so, its getting a subject token, not some sort of credential object.

oauth2_http/java/com/google/auth/oauth2/AwsSecurityCredentialsProvider.java

oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java

oauth2_http/java/com/google/auth/oauth2/IdentityPoolSubjectTokenProvider.java

westarle · 2024-01-09T22:10:24Z

oauth2_http/java/com/google/auth/oauth2/IdentityPoolSubjectTokenProvider.java

+   *
+   * @return the supplier used for getSubjectToken.
+   */
+  abstract Supplier<String> getSupplier();


Maybe similar to @lsirac comment: we can probably remove getSupplier() and use getToken() directly (it would call the supplier in the programmatic case);

That said, it might be better if customers provided a SubjectTokenProvider directly. The (internal) API allows us to cheat and deal with IOException in a way we don't allow via Supplier<String>; I see both problems of us eating IOException and turning them into RTEs as well as us catching RTEs and turning them into not-retryable IOException -- even though we can anticipate that there will be network, IPC or File errors present based on all of our own implementations.

So...my suggestion is to ditch "Supplier" and offer "Provider" to our customers. Now everyone has the same API String getSubjectToken() throws IOException; they can implement Retryable on their exceptions. You might need to find another way to support the metrics header value if you're worried about clients overriding it, but overall the code should be simpler.

Made this change - I wanted to keep the supplier so we could have a functional interface for users so they could use a lambda instead of creating a whole class, but I agree that its creating more problems than its worth. Refactored everything to be public. I named the objects IdentityPoolSubjectTokenSupplier and AwsSecurityCredentialSupplier instead of provider just because I call the similar function/object supplier in other PR's for other languages (still in flight, not merged) but don't feel super strongly about that either way.

westarle · 2024-01-09T22:13:54Z

oauth2_http/java/com/google/auth/oauth2/InternalAwsSecurityCredentialsProvider.java

+  @VisibleForTesting
+  boolean shouldUseMetadataServer() {
+    return (!canRetrieveRegionFromEnvironment()
+        || !canRetrieveSecurityCredentialsFromEnvironment());


This can happen in the future, but:

now that you have an indirection to choose token providers, this logic can happen when installing a token provider and therefore break this class into one for environment and one for MDS.

Unfortunately I think that would be a breaking change because the currently implementation checks the environment variables when get security credentials is called instead of in the constructor, so technically its possible for a user to create the credential and set environment variables after.

westarle · 2024-01-09T22:18:33Z

oauth2_http/java/com/google/auth/oauth2/IdentityPoolSubjectTokenProvider.java

+   */
+  abstract Supplier<String> getSupplier();
+
+  protected static String parseToken(


Can this live with FileIDPoolCredential?

Both file and URL need this, moved to file provider for now since it can't live in the interface and I don't think its worth creating a new class to hold 1 utility method.

oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java

westarle

This is looking pretty good now, can you please comment on the coverage of the change in unit and functional tests?

aeitzman · 2024-01-16T17:52:54Z

This is looking pretty good now, can you please comment on the coverage of the change in unit and functional tests?

Added unit tests for the builders using the new supplier objects, as well as tests for the new default values. Also added tests for the credentials built with the suppliers (happy path with/without service account impersonation and error paths). Did not add tests for the internal suppliers since those paths are covered by existing testing. Also added integration tests for AWS and IdentityPool subject token supplier built credentials.

lsirac

LGTM thanks Alex 💯

oauth2_http/java/com/google/auth/oauth2/IdentityPoolSubjectTokenSupplier.java

generated-files-bot · 2024-01-17T21:59:22Z

Warning: This pull request is touching the following templated files:

.github/CODEOWNERS

westarle

If @lsirac or another reviewer has covered the Java details for bugs etc. this looks good to me.

sonarcloud · 2024-01-25T17:32:03Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

aeitzman added 5 commits November 26, 2023 22:04

feat: adds programmatic auth credentials for identity pool and aws cr…

b21127a

…edential types

feat: add quality of life improvements for building external account …

e6457d9

…credentials in code.

fix: formatting

448a8ec

fix: add formatting

59eb856

Merge remote-tracking branch 'upstream/main' into fix_builders

f2ab1a2

# Conflicts: # oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java

product-auto-label bot added the size: l Pull request size is large. label Nov 27, 2023

aeitzman added 2 commits November 27, 2023 09:49

Adds @CanIgnoreReturnValue on new builder methods

0495b7f

Merge remote-tracking branch 'upstream/main' into programmatic-auth

8d12e07

# Conflicts: # oauth2_http/javatests/com/google/auth/oauth2/AwsCredentialsTest.java

aeitzman assigned TimurSadykov and lsirac Nov 27, 2023

aeitzman marked this pull request as ready for review November 28, 2023 21:29

aeitzman requested review from a team as code owners November 28, 2023 21:29

aeitzman added 4 commits November 30, 2023 16:29

Change test for impersonated credentials

a8b2f92

formatting

616fb13

adding id_token type

b2552eb

product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Dec 1, 2023

formatting

6726160

lsirac reviewed Dec 5, 2023

View reviewed changes

aeitzman and others added 3 commits December 5, 2023 09:51

Update oauth2_http/java/com/google/auth/oauth2/AwsCredentials.java

2fc4f99

Co-authored-by: Leo <39062083+lsirac@users.noreply.github.com>

PR comments

e5a9c59

Added header value constants

bfb83fa

lsirac and others added 5 commits December 6, 2023 09:55

Merge branch 'main' into programmatic-auth

6e7a975

updating java doc

164ac25

adding integration tests

a257e55

fix tests

f09adfa

fix tests, add javadoc, and format

97946b3

aeitzman added 2 commits January 9, 2024 11:31

PR comments

188b803

fix test

e69108e

lsirac requested changes Jan 9, 2024

View reviewed changes

westarle reviewed Jan 9, 2024

View reviewed changes

oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java Outdated Show resolved Hide resolved

aeitzman and others added 4 commits January 10, 2024 12:56

refactoring to expose rename provider to supplier and expose it publicly

4f3e253

Merge branch 'main' into programmatic-auth

c22a4e9

formatting

dd659c4

Merge branch 'main' into programmatic-auth

9cd8d96

westarle reviewed Jan 15, 2024

View reviewed changes

lsirac approved these changes Jan 17, 2024

View reviewed changes

oauth2_http/java/com/google/auth/oauth2/IdentityPoolSubjectTokenSupplier.java Show resolved Hide resolved

aeitzman added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 17, 2024

updating codeowners

8963d68

aeitzman requested a review from a team as a code owner January 17, 2024 21:59

westarle approved these changes Jan 18, 2024

View reviewed changes

lsirac and others added 8 commits January 18, 2024 08:44

Merge branch 'main' into programmatic-auth

f4cadd2

make subject token supplier interface public

20c2f7d

Making AwsSecurityCredentials public and change name to sessionToken

abd462b

lint

9835df7

Merge branch 'main' into programmatic-auth

d06eb36

Merge remote-tracking branch 'upstream/main' into programmatic-auth

d59fb92

fix tests

4371463

lint

5b21010

aeitzman removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 25, 2024

aeitzman merged commit 64ce8a1 into googleapis:main Jan 25, 2024
18 checks passed

release-please bot mentioned this pull request Jan 25, 2024

chore(main): release 1.23.0 #1353

Merged

aeitzman mentioned this pull request Feb 7, 2024

For AWS Credentials , can we use AWSCredentialsProviderChain . This will provide the credentials if the application running on EC2, ECS, EKS ,etc or from default env variables. #714

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Adds support for user defined subject token suppliers in AWSCredentials and IdentityPoolCredentials #1336

feat: Adds support for user defined subject token suppliers in AWSCredentials and IdentityPoolCredentials #1336

aeitzman commented Nov 27, 2023 •

edited

lsirac commented Dec 6, 2023 •

edited

lsirac Jan 9, 2024

aeitzman Jan 10, 2024

lsirac Jan 9, 2024

aeitzman Jan 9, 2024

westarle Jan 9, 2024

aeitzman Jan 10, 2024

westarle Jan 9, 2024

aeitzman Jan 10, 2024

westarle Jan 9, 2024

aeitzman Jan 10, 2024

westarle left a comment

aeitzman commented Jan 16, 2024

lsirac left a comment

generated-files-bot bot commented Jan 17, 2024

westarle left a comment

sonarcloud bot commented Jan 25, 2024

feat: Adds support for user defined subject token suppliers in AWSCredentials and IdentityPoolCredentials #1336

feat: Adds support for user defined subject token suppliers in AWSCredentials and IdentityPoolCredentials #1336

Conversation

aeitzman commented Nov 27, 2023 • edited

lsirac commented Dec 6, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westarle left a comment

Choose a reason for hiding this comment

aeitzman commented Jan 16, 2024

lsirac left a comment

Choose a reason for hiding this comment

generated-files-bot bot commented Jan 17, 2024

westarle left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Jan 25, 2024

Quality Gate passed

aeitzman commented Nov 27, 2023 •

edited

lsirac commented Dec 6, 2023 •

edited