From e4a9206b0e1a2cf944513bd85c84a9eba3c080f3 Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Fri, 2 Apr 2021 15:31:14 -0700 Subject: [PATCH 1/5] feat: enables OIDC auth code flow --- .../firebase/auth/OidcProviderConfig.java | 117 ++++++++++++++++++ .../google/firebase/auth/FirebaseAuthIT.java | 24 +++- .../auth/FirebaseUserManagerTest.java | 43 ++++++- .../firebase/auth/OidcProviderConfigTest.java | 59 ++++++++- .../TenantAwareFirebaseAuthIT.java | 24 +++- src/test/resources/listOidc.json | 14 ++- src/test/resources/oidc.json | 7 +- 7 files changed, 270 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/google/firebase/auth/OidcProviderConfig.java b/src/main/java/com/google/firebase/auth/OidcProviderConfig.java index 26931788e..95767bbc6 100644 --- a/src/main/java/com/google/firebase/auth/OidcProviderConfig.java +++ b/src/main/java/com/google/firebase/auth/OidcProviderConfig.java @@ -18,8 +18,11 @@ import static com.google.common.base.Preconditions.checkArgument; +import com.google.api.client.json.GenericJson; import com.google.api.client.util.Key; import com.google.common.base.Strings; +import java.util.HashMap; +import java.util.Map; /** * Contains metadata associated with an OIDC Auth provider. @@ -31,17 +34,31 @@ public final class OidcProviderConfig extends ProviderConfig { @Key("clientId") private String clientId; + @Key("clientSecret") + private String clientSecret; + @Key("issuer") private String issuer; + @Key("responseType") + private GenericJson responseType; + public String getClientId() { return clientId; } + public String getClientSecret() { + return clientSecret; + } + public String getIssuer() { return issuer; } + public GenericJson getResponseType() { + return responseType; + } + /** * Returns a new {@link UpdateRequest}, which can be used to update the attributes of this * provider config. @@ -99,6 +116,19 @@ public CreateRequest setClientId(String clientId) { return this; } + /** + * Sets the client secret for the new provider. This is required for the code flow. + * + * @param clientSecret A non-null, non-empty client secret string. + * @throws IllegalArgumentException If the client secret is null or empty. + */ + public CreateRequest setClientSecret(String clientSecret) { + checkArgument(!Strings.isNullOrEmpty(clientSecret), + "Client Secret must not be null or empty."); + properties.put("clientSecret", clientSecret); + return this; + } + /** * Sets the issuer for the new provider. * @@ -113,6 +143,43 @@ public CreateRequest setIssuer(String issuer) { return this; } + /** + * Sets whether to enable the code response flow for the new provider. By default, this is not + * enabled if no response type is specified. + * + *

A client secret must be set for this response type. + * + *

Having both the code and ID token response flows is currently not supported. + * + * @param enabled A boolean signifying whether the code response type is supported. + */ + public CreateRequest setCodeResponseType(boolean enabled) { + if (properties.get("responseType") == null) { + properties.put("responseType", new HashMap()); + } + Map map = (Map) properties.get("responseType"); + map.put("code", enabled); + return this; + } + + /** + * Sets whether to enable the ID token response flow for the new provider. By default, this is + * enabled if no response type is specified. + * + *

Having both the code and ID token response flows is currently not supported. + * + * @param enabled A boolean signifying whether the ID token response type is supported. + */ + public CreateRequest setIdTokenResponseType(boolean enabled) { + if (properties.get("responseType") == null) { + properties.put("responseType", new HashMap()); + } + + Map map = (Map) properties.get("responseType"); + map.put("idToken", enabled); + return this; + } + CreateRequest getThis() { return this; } @@ -156,6 +223,19 @@ public UpdateRequest setClientId(String clientId) { return this; } + /** + * Sets the client secret for the new provider. This is required for the code flow. + * + * @param clientSecret A non-null, non-empty client secret string. + * @throws IllegalArgumentException If the client secret is null or empty. + */ + public UpdateRequest setClientSecret(String clientSecret) { + checkArgument(!Strings.isNullOrEmpty(clientSecret), + "Client Secret must not be null or empty."); + properties.put("clientSecret", clientSecret); + return this; + } + /** * Sets the issuer for the existing provider. * @@ -170,6 +250,43 @@ public UpdateRequest setIssuer(String issuer) { return this; } + /** + * Sets whether to enable the code response flow for the new provider. By default, this is not + * enabled if no response type is specified. + * + *

A client secret must be set for this response type. + * + *

Having both the code and ID token response flows is currently not supported. + * + * @param enabled A boolean signifying whether the code response type is supported. + */ + public UpdateRequest setCodeResponseType(boolean enabled) { + if (properties.get("responseType") == null) { + properties.put("responseType", new HashMap()); + } + Map map = (Map) properties.get("responseType"); + map.put("code", enabled); + return this; + } + + /** + * Sets whether to enable the ID token response flow for the new provider. By default, this is + * enabled if no response type is specified. + * + *

Having both the code and ID token response flows is currently not supported. + * + * @param enabled A boolean signifying whether the ID token response type is supported. + */ + public UpdateRequest setIdTokenResponseType(boolean enabled) { + if (properties.get("responseType") == null) { + properties.put("responseType", new HashMap()); + } + + Map map = (Map) properties.get("responseType"); + map.put("idToken", enabled); + return this; + } + UpdateRequest getThis() { return this; } diff --git a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java index 35fa21d4d..a85582c7c 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java @@ -703,12 +703,20 @@ public void testOidcProviderConfigLifecycle() throws Exception { .setDisplayName("DisplayName") .setEnabled(true) .setClientId("ClientId") - .setIssuer("https://oidc.com/issuer")); + .setClientSecret("ClientSecret") + .setIssuer("https://oidc.com/issuer") + .setCodeResponseType(true) + .setIdTokenResponseType(false)); + assertEquals(providerId, config.getProviderId()); assertEquals("DisplayName", config.getDisplayName()); assertTrue(config.isEnabled()); assertEquals("ClientId", config.getClientId()); + assertEquals("ClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); + GenericJson responseType = config.getResponseType(); + assertTrue((boolean) responseType.get("code")); + assertNull(responseType.get("idToken")); // Get provider config config = auth.getOidcProviderConfigAsync(providerId).get(); @@ -716,7 +724,11 @@ public void testOidcProviderConfigLifecycle() throws Exception { assertEquals("DisplayName", config.getDisplayName()); assertTrue(config.isEnabled()); assertEquals("ClientId", config.getClientId()); + assertEquals("ClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); + responseType = config.getResponseType(); + assertTrue((boolean) responseType.get("code")); + assertNull(responseType.get("idToken")); // Update provider config OidcProviderConfig.UpdateRequest updateRequest = @@ -724,13 +736,21 @@ public void testOidcProviderConfigLifecycle() throws Exception { .setDisplayName("NewDisplayName") .setEnabled(false) .setClientId("NewClientId") - .setIssuer("https://oidc.com/new-issuer"); + .setClientSecret("NewClientSecret") + .setIssuer("https://oidc.com/new-issuer") + .setCodeResponseType(false) + .setIdTokenResponseType(true); + config = auth.updateOidcProviderConfigAsync(updateRequest).get(); assertEquals(providerId, config.getProviderId()); assertEquals("NewDisplayName", config.getDisplayName()); assertFalse(config.isEnabled()); assertEquals("NewClientId", config.getClientId()); + assertEquals("NewClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/new-issuer", config.getIssuer()); + responseType = config.getResponseType(); + assertTrue((boolean) responseType.get("idToken")); + assertNull(responseType.get("code")); // Delete provider config temporaryProviderConfig.deleteOidcProviderConfig(providerId); diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java index 772f9ba8d..40b862a1e 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -1490,7 +1490,10 @@ public void testCreateOidcProvider() throws Exception { .setDisplayName("DISPLAY_NAME") .setEnabled(true) .setClientId("CLIENT_ID") - .setIssuer("https://oidc.com/issuer"); + .setClientSecret("CLIENT_SECRET") + .setIssuer("https://oidc.com/issuer") + .setCodeResponseType(true) + .setIdTokenResponseType(true); OidcProviderConfig config = FirebaseAuth.getInstance().createOidcProviderConfig(createRequest); @@ -1501,7 +1504,13 @@ public void testCreateOidcProvider() throws Exception { assertEquals("DISPLAY_NAME", parsed.get("displayName")); assertTrue((boolean) parsed.get("enabled")); assertEquals("CLIENT_ID", parsed.get("clientId")); + assertEquals("CLIENT_SECRET", parsed.get("clientSecret")); assertEquals("https://oidc.com/issuer", parsed.get("issuer")); + + Map responseType = (Map) parsed.get("responseType"); + assertTrue(responseType.get("code")); + assertTrue(responseType.get("idToken")); + GenericUrl url = interceptor.getResponse().getRequest().getUrl(); assertEquals("oidc.provider-id", url.getFirst("oauthIdpConfigId")); } @@ -1515,9 +1524,12 @@ public void testCreateOidcProviderAsync() throws Exception { .setDisplayName("DISPLAY_NAME") .setEnabled(true) .setClientId("CLIENT_ID") - .setIssuer("https://oidc.com/issuer"); + .setClientSecret("CLIENT_SECRET") + .setIssuer("https://oidc.com/issuer") + .setCodeResponseType(true) + .setIdTokenResponseType(true); - OidcProviderConfig config = + OidcProviderConfig config = FirebaseAuth.getInstance().createOidcProviderConfigAsync(createRequest).get(); checkOidcProviderConfig(config, "oidc.provider-id"); @@ -1527,7 +1539,13 @@ public void testCreateOidcProviderAsync() throws Exception { assertEquals("DISPLAY_NAME", parsed.get("displayName")); assertTrue((boolean) parsed.get("enabled")); assertEquals("CLIENT_ID", parsed.get("clientId")); + assertEquals("CLIENT_SECRET", parsed.get("clientSecret")); assertEquals("https://oidc.com/issuer", parsed.get("issuer")); + + Map responseType = (Map) parsed.get("responseType"); + assertTrue(responseType.get("code")); + assertTrue(responseType.get("idToken")); + GenericUrl url = interceptor.getResponse().getRequest().getUrl(); assertEquals("oidc.provider-id", url.getFirst("oauthIdpConfigId")); } @@ -1730,7 +1748,10 @@ public void testTenantAwareUpdateOidcProvider() throws Exception { .setDisplayName("DISPLAY_NAME") .setEnabled(true) .setClientId("CLIENT_ID") - .setIssuer("https://oidc.com/issuer"); + .setClientSecret("CLIENT_SECRET") + .setIssuer("https://oidc.com/issuer") + .setCodeResponseType(true) + .setIdTokenResponseType(true); OidcProviderConfig config = tenantAwareAuth.updateOidcProviderConfig(request); @@ -1739,12 +1760,18 @@ public void testTenantAwareUpdateOidcProvider() throws Exception { String expectedUrl = TENANTS_BASE_URL + "/TENANT_ID/oauthIdpConfigs/oidc.provider-id"; checkUrl(interceptor, "PATCH", expectedUrl); GenericUrl url = interceptor.getResponse().getRequest().getUrl(); - assertEquals("clientId,displayName,enabled,issuer", url.getFirst("updateMask")); + assertEquals("clientId,clientSecret,displayName,enabled,issuer,responseType.code," + + "responseType.idToken", url.getFirst("updateMask")); GenericJson parsed = parseRequestContent(interceptor); assertEquals("DISPLAY_NAME", parsed.get("displayName")); assertTrue((boolean) parsed.get("enabled")); assertEquals("CLIENT_ID", parsed.get("clientId")); + assertEquals("CLIENT_SECRET", parsed.get("clientSecret")); assertEquals("https://oidc.com/issuer", parsed.get("issuer")); + + Map responseType = (Map) parsed.get("responseType"); + assertTrue(responseType.get("code")); + assertTrue(responseType.get("idToken")); } @Test @@ -2792,7 +2819,12 @@ private static void checkOidcProviderConfig(OidcProviderConfig config, String pr assertEquals("DISPLAY_NAME", config.getDisplayName()); assertTrue(config.isEnabled()); assertEquals("CLIENT_ID", config.getClientId()); + assertEquals("CLIENT_SECRET", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); + + GenericJson responseType = config.getResponseType(); + assertTrue((boolean) responseType.get("code")); + assertFalse((boolean) responseType.get("idToken")); } private static void checkSamlProviderConfig(SamlProviderConfig config, String providerId) { @@ -2824,5 +2856,4 @@ private static void checkUrl(TestResponseInterceptor interceptor, String method, private interface UserManagerOp { void call(FirebaseAuth auth) throws Exception; } - } diff --git a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java index 1fb4ca37b..3ede08961 100644 --- a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java +++ b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java @@ -18,9 +18,11 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import com.google.api.client.googleapis.util.Utils; +import com.google.api.client.json.GenericJson; import com.google.api.client.json.JsonFactory; import java.io.IOException; import java.util.Map; @@ -36,7 +38,12 @@ public class OidcProviderConfigTest { + " 'displayName': 'DISPLAY_NAME'," + " 'enabled': true," + " 'clientId': 'CLIENT_ID'," - + " 'issuer': 'https://oidc.com/issuer'" + + " 'clientSecret':'CLIENT_SECRET'," + + " 'issuer': 'https://oidc.com/issuer'," + + " 'responseType': {" + + " 'code': true," + + " 'idToken': false" + + " }" + "}").replace("'", "\""); @Test @@ -47,7 +54,13 @@ public void testJsonDeserialization() throws IOException { assertEquals("DISPLAY_NAME", config.getDisplayName()); assertTrue(config.isEnabled()); assertEquals("CLIENT_ID", config.getClientId()); + assertEquals("CLIENT_SECRET", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); + assertNotNull(config.getResponseType()); + + GenericJson responseType = config.getResponseType(); + assertTrue((boolean)responseType.get("code")); + assertFalse((boolean)responseType.get("idToken")); } @Test @@ -58,15 +71,23 @@ public void testCreateRequest() throws IOException { .setDisplayName("DISPLAY_NAME") .setEnabled(false) .setClientId("CLIENT_ID") - .setIssuer("https://oidc.com/issuer"); + .setClientSecret("CLIENT_SECRET") + .setIssuer("https://oidc.com/issuer") + .setCodeResponseType(true) + .setIdTokenResponseType(false); assertEquals("oidc.provider-id", createRequest.getProviderId()); Map properties = createRequest.getProperties(); - assertEquals(properties.size(), 4); + assertEquals(properties.size(), 6); assertEquals("DISPLAY_NAME", (String) properties.get("displayName")); assertFalse((boolean) properties.get("enabled")); assertEquals("CLIENT_ID", (String) properties.get("clientId")); + assertEquals("CLIENT_SECRET", properties.get("clientSecret")); assertEquals("https://oidc.com/issuer", (String) properties.get("issuer")); + + Map responseType = (Map) properties.get("responseType"); + assertTrue(responseType.get("code")); + assertFalse(responseType.get("idToken")); } @Test(expected = IllegalArgumentException.class) @@ -99,6 +120,16 @@ public void testCreateRequestInvalidIssuerUrl() { new OidcProviderConfig.CreateRequest().setIssuer("not a valid url"); } + @Test(expected = IllegalArgumentException.class) + public void testCreateRequestMissingClientSecret() { + new OidcProviderConfig.CreateRequest().setClientSecret(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testCreateRequestEmptyClientSecret() { + new OidcProviderConfig.CreateRequest().setClientSecret(""); + } + @Test public void testUpdateRequestFromOidcProviderConfig() throws IOException { OidcProviderConfig config = jsonFactory.fromString(OIDC_JSON_STRING, OidcProviderConfig.class); @@ -117,15 +148,23 @@ public void testUpdateRequest() throws IOException { .setDisplayName("DISPLAY_NAME") .setEnabled(false) .setClientId("CLIENT_ID") - .setIssuer("https://oidc.com/issuer"); + .setClientSecret("CLIENT_SECRET") + .setIssuer("https://oidc.com/issuer") + .setCodeResponseType(true) + .setIdTokenResponseType(false); assertEquals("oidc.provider-id", updateRequest.getProviderId()); Map properties = updateRequest.getProperties(); - assertEquals(properties.size(), 4); + assertEquals(properties.size(), 6); assertEquals("DISPLAY_NAME", (String) properties.get("displayName")); assertFalse((boolean) properties.get("enabled")); assertEquals("CLIENT_ID", (String) properties.get("clientId")); + assertEquals("CLIENT_SECRET", (String) properties.get("clientSecret")); assertEquals("https://oidc.com/issuer", (String) properties.get("issuer")); + + Map responseType = (Map) properties.get("responseType"); + assertTrue(responseType.get("code")); + assertFalse(responseType.get("idToken")); } @Test(expected = IllegalArgumentException.class) @@ -157,4 +196,14 @@ public void testUpdateRequestMissingIssuer() { public void testUpdateRequestInvalidIssuerUrl() { new OidcProviderConfig.UpdateRequest("oidc.provider-id").setIssuer("not a valid url"); } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestMissingClientSecret() { + new OidcProviderConfig.UpdateRequest("oidc.provider-id").setClientSecret(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestEmptyClientSecret() { + new OidcProviderConfig.UpdateRequest("oidc.provider-id").setClientSecret(""); + } } diff --git a/src/test/java/com/google/firebase/auth/multitenancy/TenantAwareFirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/multitenancy/TenantAwareFirebaseAuthIT.java index 1ebac213f..bd05d78a0 100644 --- a/src/test/java/com/google/firebase/auth/multitenancy/TenantAwareFirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/multitenancy/TenantAwareFirebaseAuthIT.java @@ -279,18 +279,30 @@ public void testOidcProviderConfigLifecycle() throws Exception { .setDisplayName("DisplayName") .setEnabled(true) .setClientId("ClientId") - .setIssuer("https://oidc.com/issuer")); + .setClientSecret("ClientSecret") + .setIssuer("https://oidc.com/issuer") + .setCodeResponseType(true) + .setIdTokenResponseType(false)); + assertEquals(providerId, config.getProviderId()); assertEquals("DisplayName", config.getDisplayName()); assertEquals("ClientId", config.getClientId()); + assertEquals("ClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); + GenericJson responseType = config.getResponseType(); + assertTrue((boolean) responseType.get("code")); + assertNull(responseType.get("idToken")); // Get provider config config = tenantAwareAuth.getOidcProviderConfigAsync(providerId).get(); assertEquals(providerId, config.getProviderId()); assertEquals("DisplayName", config.getDisplayName()); assertEquals("ClientId", config.getClientId()); + assertEquals("ClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); + responseType = config.getResponseType(); + assertTrue((boolean) responseType.get("code")); + assertNull(responseType.get("idToken")); // Update provider config OidcProviderConfig.UpdateRequest updateRequest = @@ -298,13 +310,21 @@ public void testOidcProviderConfigLifecycle() throws Exception { .setDisplayName("NewDisplayName") .setEnabled(false) .setClientId("NewClientId") - .setIssuer("https://oidc.com/new-issuer"); + .setClientSecret("NewClientSecret") + .setIssuer("https://oidc.com/new-issuer") + .setCodeResponseType(false) + .setIdTokenResponseType(true); + config = tenantAwareAuth.updateOidcProviderConfigAsync(updateRequest).get(); assertEquals(providerId, config.getProviderId()); assertEquals("NewDisplayName", config.getDisplayName()); assertFalse(config.isEnabled()); assertEquals("NewClientId", config.getClientId()); + assertEquals("NewClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/new-issuer", config.getIssuer()); + responseType = config.getResponseType(); + assertTrue((boolean) responseType.get("idToken")); + assertNull(responseType.get("code")); // Delete provider config temporaryProviderConfig.deleteOidcProviderConfig(providerId); diff --git a/src/test/resources/listOidc.json b/src/test/resources/listOidc.json index 0c13ea48b..82a2f7669 100644 --- a/src/test/resources/listOidc.json +++ b/src/test/resources/listOidc.json @@ -4,12 +4,22 @@ "displayName" : "DISPLAY_NAME", "enabled" : true, "clientId" : "CLIENT_ID", - "issuer" : "https://oidc.com/issuer" + "clientSecret" : "CLIENT_SECRET", + "issuer" : "https://oidc.com/issuer", + "responseType" : { + "code": true, + "idToken": false + } }, { "name": "projects/projectId/oauthIdpConfigs/oidc.provider-id2", "displayName" : "DISPLAY_NAME", "enabled" : true, "clientId" : "CLIENT_ID", - "issuer" : "https://oidc.com/issuer" + "clientSecret" : "CLIENT_SECRET", + "issuer" : "https://oidc.com/issuer", + "responseType" : { + "code": true, + "idToken": false + } } ] } diff --git a/src/test/resources/oidc.json b/src/test/resources/oidc.json index e2f1845de..0aed0df50 100644 --- a/src/test/resources/oidc.json +++ b/src/test/resources/oidc.json @@ -3,5 +3,10 @@ "displayName" : "DISPLAY_NAME", "enabled" : true, "clientId" : "CLIENT_ID", - "issuer" : "https://oidc.com/issuer" + "clientSecret" : "CLIENT_SECRET", + "issuer" : "https://oidc.com/issuer", + "responseType" : { + "code": true, + "idToken": false + } } From 99a7a26b6d87372234b334bb9f4979f31d24055f Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Thu, 15 Apr 2021 11:25:50 -0700 Subject: [PATCH 2/5] fix: remove GenericJson from public API surface --- .../google/firebase/auth/OidcProviderConfig.java | 8 ++++++-- .../com/google/firebase/auth/FirebaseAuthIT.java | 15 ++++++--------- .../firebase/auth/FirebaseUserManagerTest.java | 6 ++---- .../firebase/auth/OidcProviderConfigTest.java | 7 ++----- .../multitenancy/TenantAwareFirebaseAuthIT.java | 15 ++++++--------- 5 files changed, 22 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/google/firebase/auth/OidcProviderConfig.java b/src/main/java/com/google/firebase/auth/OidcProviderConfig.java index 95767bbc6..757134708 100644 --- a/src/main/java/com/google/firebase/auth/OidcProviderConfig.java +++ b/src/main/java/com/google/firebase/auth/OidcProviderConfig.java @@ -55,8 +55,12 @@ public String getIssuer() { return issuer; } - public GenericJson getResponseType() { - return responseType; + public boolean isCodeResponseType() { + return (responseType.containsKey("code") && (boolean) responseType.get("code")); + } + + public boolean isIdTokenResponseType() { + return (responseType.containsKey("idToken") && (boolean) responseType.get("idToken")); } /** diff --git a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java index a85582c7c..f474f0436 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java @@ -714,9 +714,8 @@ public void testOidcProviderConfigLifecycle() throws Exception { assertEquals("ClientId", config.getClientId()); assertEquals("ClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); - GenericJson responseType = config.getResponseType(); - assertTrue((boolean) responseType.get("code")); - assertNull(responseType.get("idToken")); + assertTrue(config.isCodeResponseType()); + assertFalse(config.isIdTokenResponseType()); // Get provider config config = auth.getOidcProviderConfigAsync(providerId).get(); @@ -726,9 +725,8 @@ public void testOidcProviderConfigLifecycle() throws Exception { assertEquals("ClientId", config.getClientId()); assertEquals("ClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); - responseType = config.getResponseType(); - assertTrue((boolean) responseType.get("code")); - assertNull(responseType.get("idToken")); + assertTrue(config.isCodeResponseType()); + assertFalse(config.isIdTokenResponseType()); // Update provider config OidcProviderConfig.UpdateRequest updateRequest = @@ -748,9 +746,8 @@ public void testOidcProviderConfigLifecycle() throws Exception { assertEquals("NewClientId", config.getClientId()); assertEquals("NewClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/new-issuer", config.getIssuer()); - responseType = config.getResponseType(); - assertTrue((boolean) responseType.get("idToken")); - assertNull(responseType.get("code")); + assertTrue(config.isIdTokenResponseType()); + assertFalse(config.isCodeResponseType()); // Delete provider config temporaryProviderConfig.deleteOidcProviderConfig(providerId); diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java index 40b862a1e..fa4bcd7f3 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -2821,10 +2821,8 @@ private static void checkOidcProviderConfig(OidcProviderConfig config, String pr assertEquals("CLIENT_ID", config.getClientId()); assertEquals("CLIENT_SECRET", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); - - GenericJson responseType = config.getResponseType(); - assertTrue((boolean) responseType.get("code")); - assertFalse((boolean) responseType.get("idToken")); + assertTrue(config.isCodeResponseType()); + assertFalse(config.isIdTokenResponseType()); } private static void checkSamlProviderConfig(SamlProviderConfig config, String providerId) { diff --git a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java index 3ede08961..07bd44036 100644 --- a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java +++ b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java @@ -56,11 +56,8 @@ public void testJsonDeserialization() throws IOException { assertEquals("CLIENT_ID", config.getClientId()); assertEquals("CLIENT_SECRET", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); - assertNotNull(config.getResponseType()); - - GenericJson responseType = config.getResponseType(); - assertTrue((boolean)responseType.get("code")); - assertFalse((boolean)responseType.get("idToken")); + assertTrue(config.isCodeResponseType()); + assertFalse(config.isIdTokenResponseType()); } @Test diff --git a/src/test/java/com/google/firebase/auth/multitenancy/TenantAwareFirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/multitenancy/TenantAwareFirebaseAuthIT.java index bd05d78a0..3c4578e20 100644 --- a/src/test/java/com/google/firebase/auth/multitenancy/TenantAwareFirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/multitenancy/TenantAwareFirebaseAuthIT.java @@ -289,9 +289,8 @@ public void testOidcProviderConfigLifecycle() throws Exception { assertEquals("ClientId", config.getClientId()); assertEquals("ClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); - GenericJson responseType = config.getResponseType(); - assertTrue((boolean) responseType.get("code")); - assertNull(responseType.get("idToken")); + assertTrue(config.isCodeResponseType()); + assertFalse(config.isIdTokenResponseType()); // Get provider config config = tenantAwareAuth.getOidcProviderConfigAsync(providerId).get(); @@ -300,9 +299,8 @@ public void testOidcProviderConfigLifecycle() throws Exception { assertEquals("ClientId", config.getClientId()); assertEquals("ClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/issuer", config.getIssuer()); - responseType = config.getResponseType(); - assertTrue((boolean) responseType.get("code")); - assertNull(responseType.get("idToken")); + assertTrue(config.isCodeResponseType()); + assertFalse(config.isIdTokenResponseType()); // Update provider config OidcProviderConfig.UpdateRequest updateRequest = @@ -322,9 +320,8 @@ public void testOidcProviderConfigLifecycle() throws Exception { assertEquals("NewClientId", config.getClientId()); assertEquals("NewClientSecret", config.getClientSecret()); assertEquals("https://oidc.com/new-issuer", config.getIssuer()); - responseType = config.getResponseType(); - assertTrue((boolean) responseType.get("idToken")); - assertNull(responseType.get("code")); + assertFalse(config.isCodeResponseType()); + assertTrue(config.isIdTokenResponseType()); // Delete provider config temporaryProviderConfig.deleteOidcProviderConfig(providerId); From 2aa7f38ae48c2d36969797b5c162f96729448eaa Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Thu, 15 Apr 2021 13:23:47 -0700 Subject: [PATCH 3/5] fix: remove duplication --- .../firebase/auth/OidcProviderConfig.java | 29 +++++++------------ .../firebase/auth/OidcProviderConfigTest.java | 26 +++++++++++++++++ 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/google/firebase/auth/OidcProviderConfig.java b/src/main/java/com/google/firebase/auth/OidcProviderConfig.java index 757134708..0a4f7c0b8 100644 --- a/src/main/java/com/google/firebase/auth/OidcProviderConfig.java +++ b/src/main/java/com/google/firebase/auth/OidcProviderConfig.java @@ -79,6 +79,13 @@ static void checkOidcProviderId(String providerId) { "Invalid OIDC provider ID (must be prefixed with 'oidc.'): " + providerId); } + static Map ensureResponseType(Map properties) { + if (properties.get("responseType") == null) { + properties.put("responseType", new HashMap()); + } + return (Map) properties.get("responseType"); + } + /** * A specification class for creating a new OIDC Auth provider. * @@ -158,10 +165,7 @@ public CreateRequest setIssuer(String issuer) { * @param enabled A boolean signifying whether the code response type is supported. */ public CreateRequest setCodeResponseType(boolean enabled) { - if (properties.get("responseType") == null) { - properties.put("responseType", new HashMap()); - } - Map map = (Map) properties.get("responseType"); + Map map = ensureResponseType(properties); map.put("code", enabled); return this; } @@ -175,11 +179,7 @@ public CreateRequest setCodeResponseType(boolean enabled) { * @param enabled A boolean signifying whether the ID token response type is supported. */ public CreateRequest setIdTokenResponseType(boolean enabled) { - if (properties.get("responseType") == null) { - properties.put("responseType", new HashMap()); - } - - Map map = (Map) properties.get("responseType"); + Map map = ensureResponseType(properties); map.put("idToken", enabled); return this; } @@ -265,10 +265,7 @@ public UpdateRequest setIssuer(String issuer) { * @param enabled A boolean signifying whether the code response type is supported. */ public UpdateRequest setCodeResponseType(boolean enabled) { - if (properties.get("responseType") == null) { - properties.put("responseType", new HashMap()); - } - Map map = (Map) properties.get("responseType"); + Map map = ensureResponseType(properties); map.put("code", enabled); return this; } @@ -282,11 +279,7 @@ public UpdateRequest setCodeResponseType(boolean enabled) { * @param enabled A boolean signifying whether the ID token response type is supported. */ public UpdateRequest setIdTokenResponseType(boolean enabled) { - if (properties.get("responseType") == null) { - properties.put("responseType", new HashMap()); - } - - Map map = (Map) properties.get("responseType"); + Map map = ensureResponseType(properties); map.put("idToken", enabled); return this; } diff --git a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java index 07bd44036..28437f95f 100644 --- a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java +++ b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java @@ -25,6 +25,7 @@ import com.google.api.client.json.GenericJson; import com.google.api.client.json.JsonFactory; import java.io.IOException; +import java.util.HashMap; import java.util.Map; import org.junit.Test; @@ -60,6 +61,31 @@ public void testJsonDeserialization() throws IOException { assertFalse(config.isIdTokenResponseType()); } + @Test + public void testEnsureResponseType() { + Map properties = new HashMap<>(); + + Map responseType = OidcProviderConfig.ensureResponseType(properties); + + assertNotNull(responseType); + assertEquals(responseType, properties.get("responseType")); + } + + @Test + public void testEnsureResponseType_alreadyPresent() { + Map properties = new HashMap<>(); + Map responseType = new HashMap<>(); + responseType.put("code", true); + properties.put("responseType", responseType); + + Map returnedResponseType = OidcProviderConfig.ensureResponseType(properties); + + assertEquals(responseType, returnedResponseType); + assertTrue(returnedResponseType.get("code")); + assertEquals(returnedResponseType.size(), 1); + assertEquals(responseType, properties.get("responseType")); + } + @Test public void testCreateRequest() throws IOException { OidcProviderConfig.CreateRequest createRequest = new OidcProviderConfig.CreateRequest(); From 0e58c511f7049e61164335f08ad695f033e39511 Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Fri, 16 Apr 2021 11:15:44 -0700 Subject: [PATCH 4/5] fix: camel case for test cases --- .../java/com/google/firebase/auth/OidcProviderConfigTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java index 28437f95f..65c0aa715 100644 --- a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java +++ b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java @@ -72,7 +72,7 @@ public void testEnsureResponseType() { } @Test - public void testEnsureResponseType_alreadyPresent() { + public void testEnsureResponseTypeAlreadyPresent() { Map properties = new HashMap<>(); Map responseType = new HashMap<>(); responseType.put("code", true); From 6943b9085c0e6a53721caaa0f0aa18b4eb4b1ab8 Mon Sep 17 00:00:00 2001 From: Leonardo Siracusa Date: Fri, 16 Apr 2021 11:19:03 -0700 Subject: [PATCH 5/5] fix: remove import --- .../java/com/google/firebase/auth/OidcProviderConfigTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java index 65c0aa715..b8a5d5078 100644 --- a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java +++ b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java @@ -22,7 +22,6 @@ import static org.junit.Assert.assertTrue; import com.google.api.client.googleapis.util.Utils; -import com.google.api.client.json.GenericJson; import com.google.api.client.json.JsonFactory; import java.io.IOException; import java.util.HashMap;