From 88c6632f4ab87aad70d27efb781a1ef594f88199 Mon Sep 17 00:00:00 2001 From: aeitzman Date: Tue, 9 Apr 2024 12:54:02 -0700 Subject: [PATCH 1/5] fix: makes default token url universe aware --- .../oauth2/ExternalAccountCredentials.java | 16 ++++++++++++++-- .../oauth2/ExternalAccountCredentialsTest.java | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java b/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java index ad9633da8..f2e346d89 100644 --- a/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java +++ b/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java @@ -73,7 +73,7 @@ public abstract class ExternalAccountCredentials extends GoogleCredentials { static final String EXTERNAL_ACCOUNT_FILE_TYPE = "external_account"; static final String EXECUTABLE_SOURCE_KEY = "executable"; - static final String DEFAULT_TOKEN_URL = "https://sts.googleapis.com/v1/token"; + static final String DEFAULT_TOKEN_URL = "https://sts.{UNIVERSE_DOMAIN}/v1/token"; static final String PROGRAMMATIC_METRICS_HEADER_VALUE = "programmatic"; private final String transportFactoryClassName; @@ -235,7 +235,19 @@ protected ExternalAccountCredentials(ExternalAccountCredentials.Builder builder) this.serviceAccountImpersonationUrl = builder.serviceAccountImpersonationUrl; this.clientId = builder.clientId; this.clientSecret = builder.clientSecret; - this.tokenUrl = builder.tokenUrl == null ? DEFAULT_TOKEN_URL : builder.tokenUrl; + + if (builder.tokenUrl == null){ + try { + this.tokenUrl = DEFAULT_TOKEN_URL.replace("{UNIVERSE_DOMAIN}", this.getUniverseDomain()); + } catch (IOException e) { + // Throwing an IOException would be a breaking change, so wrap it here. + throw new IllegalStateException( + "Error occurred when attempting to retrieve universe domain.", e); + } + } else { + this.tokenUrl = builder.tokenUrl; + } + this.scopes = (builder.scopes == null || builder.scopes.isEmpty()) ? Arrays.asList(CLOUD_PLATFORM_SCOPE) diff --git a/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java b/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java index 986669c9c..9cefedb8c 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java @@ -565,6 +565,24 @@ public void constructor_builder_defaultTokenUrl() { assertEquals(STS_URL, credentials.getTokenUrl()); } + @Test + public void constructor_builder_defaultTokenUrlwithUniverseDomain() { + HashMap credentialSource = new HashMap<>(); + credentialSource.put("file", "file"); + + ExternalAccountCredentials credentials = + IdentityPoolCredentials.newBuilder() + .setHttpTransportFactory(transportFactory) + .setAudience( + "//iam.googleapis.com/locations/global/workforcePools/pool/providers/provider") + .setSubjectTokenType("subjectTokenType") + .setCredentialSource(new TestCredentialSource(credentialSource)) + .setUniverseDomain("testdomain.org") + .build(); + + assertEquals("https://sts.testdomain.org/v1/token", credentials.getTokenUrl()); + } + @Test public void constructor_builder_subjectTokenTypeEnum() { HashMap credentialSource = new HashMap<>(); From d0fc1303a48a1bd30543329e334861b158425e2c Mon Sep 17 00:00:00 2001 From: aeitzman Date: Tue, 9 Apr 2024 14:32:48 -0700 Subject: [PATCH 2/5] lint and add test --- .../oauth2/ExternalAccountCredentials.java | 2 +- .../ExternalAccountCredentialsTest.java | 44 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java b/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java index f2e346d89..7162fda45 100644 --- a/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java +++ b/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java @@ -236,7 +236,7 @@ protected ExternalAccountCredentials(ExternalAccountCredentials.Builder builder) this.clientId = builder.clientId; this.clientSecret = builder.clientSecret; - if (builder.tokenUrl == null){ + if (builder.tokenUrl == null) { try { this.tokenUrl = DEFAULT_TOKEN_URL.replace("{UNIVERSE_DOMAIN}", this.getUniverseDomain()); } catch (IOException e) { diff --git a/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java b/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java index 9cefedb8c..6fe125af9 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java @@ -583,6 +583,26 @@ public void constructor_builder_defaultTokenUrlwithUniverseDomain() { assertEquals("https://sts.testdomain.org/v1/token", credentials.getTokenUrl()); } + @Test + public void constructor_builder_getUniverseDomainFails() { + HashMap credentialSource = new HashMap<>(); + credentialSource.put("file", "file"); + + try { + UniverseDomainErrorTestCredentials.newBuilder() + .setHttpTransportFactory(transportFactory) + .setAudience( + "//iam.googleapis.com/locations/global/workforcePools/pool/providers/provider") + .setSubjectTokenType("subjectTokenType") + .setUniverseDomain("testdomain.org") + .build(); + fail("Should not be able to continue without exception."); + } catch (IllegalStateException exception) { + assertEquals( + "Error occurred when attempting to retrieve universe domain.", exception.getMessage()); + } + } + @Test public void constructor_builder_subjectTokenTypeEnum() { HashMap credentialSource = new HashMap<>(); @@ -1368,4 +1388,28 @@ public String retrieveSubjectToken() { return "subjectToken"; } } + + static class UniverseDomainErrorTestCredentials extends TestExternalAccountCredentials { + protected UniverseDomainErrorTestCredentials(TestExternalAccountCredentials.Builder builder) { + super(builder); + } + + public static UniverseDomainErrorTestCredentials.Builder newBuilder() { + return new UniverseDomainErrorTestCredentials.Builder(); + } + + static class Builder extends TestExternalAccountCredentials.Builder { + Builder() {} + + @Override + public UniverseDomainErrorTestCredentials build() { + return new UniverseDomainErrorTestCredentials(this); + } + } + + @Override + public String getUniverseDomain() throws IOException { + throw new IOException("Test error"); + } + } } From 24422f59d4d9b71a0b5d03f27668b8bc415a0111 Mon Sep 17 00:00:00 2001 From: aeitzman <12433791+aeitzman@users.noreply.github.com> Date: Tue, 9 Apr 2024 16:05:19 -0700 Subject: [PATCH 3/5] Update oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java Co-authored-by: Leo <39062083+lsirac@users.noreply.github.com> --- .../com/google/auth/oauth2/ExternalAccountCredentials.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java b/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java index 7162fda45..f3cf91a8a 100644 --- a/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java +++ b/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java @@ -236,16 +236,16 @@ protected ExternalAccountCredentials(ExternalAccountCredentials.Builder builder) this.clientId = builder.clientId; this.clientSecret = builder.clientSecret; - if (builder.tokenUrl == null) { + this.tokenUrl = builder.tokenUrl; + if (this.tokenUrl == null) { try { this.tokenUrl = DEFAULT_TOKEN_URL.replace("{UNIVERSE_DOMAIN}", this.getUniverseDomain()); } catch (IOException e) { // Throwing an IOException would be a breaking change, so wrap it here. + // This should not happen for this credential type. throw new IllegalStateException( "Error occurred when attempting to retrieve universe domain.", e); } - } else { - this.tokenUrl = builder.tokenUrl; } this.scopes = From 5842c8f6c6a0272fd3e9e3ead916622f13a6fab0 Mon Sep 17 00:00:00 2001 From: aeitzman Date: Tue, 9 Apr 2024 16:13:31 -0700 Subject: [PATCH 4/5] add back else --- .../com/google/auth/oauth2/ExternalAccountCredentials.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java b/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java index f3cf91a8a..8b0eeb656 100644 --- a/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java +++ b/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java @@ -236,8 +236,7 @@ protected ExternalAccountCredentials(ExternalAccountCredentials.Builder builder) this.clientId = builder.clientId; this.clientSecret = builder.clientSecret; - this.tokenUrl = builder.tokenUrl; - if (this.tokenUrl == null) { + if (builder.tokenUrl == null) { try { this.tokenUrl = DEFAULT_TOKEN_URL.replace("{UNIVERSE_DOMAIN}", this.getUniverseDomain()); } catch (IOException e) { @@ -246,6 +245,8 @@ protected ExternalAccountCredentials(ExternalAccountCredentials.Builder builder) throw new IllegalStateException( "Error occurred when attempting to retrieve universe domain.", e); } + } else { + this.tokenUrl = builder.tokenUrl; } this.scopes = From b7cc2fedf5ed0a96c4eeac0897a9e9dc32b18298 Mon Sep 17 00:00:00 2001 From: aeitzman Date: Thu, 11 Apr 2024 09:58:34 -0700 Subject: [PATCH 5/5] move code into override --- .../oauth2/ExternalAccountCredentials.java | 20 +++++---- .../ExternalAccountCredentialsTest.java | 44 ------------------- 2 files changed, 12 insertions(+), 52 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java b/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java index 8b0eeb656..a9b3ef9eb 100644 --- a/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java +++ b/oauth2_http/java/com/google/auth/oauth2/ExternalAccountCredentials.java @@ -237,14 +237,7 @@ protected ExternalAccountCredentials(ExternalAccountCredentials.Builder builder) this.clientSecret = builder.clientSecret; if (builder.tokenUrl == null) { - try { - this.tokenUrl = DEFAULT_TOKEN_URL.replace("{UNIVERSE_DOMAIN}", this.getUniverseDomain()); - } catch (IOException e) { - // Throwing an IOException would be a breaking change, so wrap it here. - // This should not happen for this credential type. - throw new IllegalStateException( - "Error occurred when attempting to retrieve universe domain.", e); - } + this.tokenUrl = DEFAULT_TOKEN_URL.replace("{UNIVERSE_DOMAIN}", this.getUniverseDomain()); } else { this.tokenUrl = builder.tokenUrl; } @@ -334,6 +327,17 @@ public void onFailure(Throwable exception) { }); } + @Override + public String getUniverseDomain() { + try { + return super.getUniverseDomain(); + } catch (IOException e) { + // Throwing an IOException would be a breaking change, so wrap it here. + // This should not happen for this credential type. + throw new IllegalStateException(e); + } + } + @Override public Map> getRequestMetadata(URI uri) throws IOException { Map> requestMetadata = super.getRequestMetadata(uri); diff --git a/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java b/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java index 6fe125af9..9cefedb8c 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java @@ -583,26 +583,6 @@ public void constructor_builder_defaultTokenUrlwithUniverseDomain() { assertEquals("https://sts.testdomain.org/v1/token", credentials.getTokenUrl()); } - @Test - public void constructor_builder_getUniverseDomainFails() { - HashMap credentialSource = new HashMap<>(); - credentialSource.put("file", "file"); - - try { - UniverseDomainErrorTestCredentials.newBuilder() - .setHttpTransportFactory(transportFactory) - .setAudience( - "//iam.googleapis.com/locations/global/workforcePools/pool/providers/provider") - .setSubjectTokenType("subjectTokenType") - .setUniverseDomain("testdomain.org") - .build(); - fail("Should not be able to continue without exception."); - } catch (IllegalStateException exception) { - assertEquals( - "Error occurred when attempting to retrieve universe domain.", exception.getMessage()); - } - } - @Test public void constructor_builder_subjectTokenTypeEnum() { HashMap credentialSource = new HashMap<>(); @@ -1388,28 +1368,4 @@ public String retrieveSubjectToken() { return "subjectToken"; } } - - static class UniverseDomainErrorTestCredentials extends TestExternalAccountCredentials { - protected UniverseDomainErrorTestCredentials(TestExternalAccountCredentials.Builder builder) { - super(builder); - } - - public static UniverseDomainErrorTestCredentials.Builder newBuilder() { - return new UniverseDomainErrorTestCredentials.Builder(); - } - - static class Builder extends TestExternalAccountCredentials.Builder { - Builder() {} - - @Override - public UniverseDomainErrorTestCredentials build() { - return new UniverseDomainErrorTestCredentials(this); - } - } - - @Override - public String getUniverseDomain() throws IOException { - throw new IOException("Test error"); - } - } }