From 6962561467aaf28cdbea4e2fbda3064b10f3cc1e Mon Sep 17 00:00:00 2001 From: Leo <39062083+lsirac@users.noreply.github.com> Date: Tue, 11 May 2021 15:22:51 -0700 Subject: [PATCH] feat(auth): enables OIDC auth code flow (#532) * feat: enables OIDC auth code flow * fix: remove GenericJson from public API surface * fix: remove duplication * fix: camel case for test cases * fix: remove import --- .../firebase/auth/OidcProviderConfig.java | 114 ++++++++++++++++++ .../google/firebase/auth/FirebaseAuthIT.java | 21 +++- .../auth/FirebaseUserManagerTest.java | 39 +++++- .../firebase/auth/OidcProviderConfigTest.java | 81 ++++++++++++- .../TenantAwareFirebaseAuthIT.java | 21 +++- src/test/resources/listOidc.json | 14 ++- src/test/resources/oidc.json | 7 +- 7 files changed, 280 insertions(+), 17 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..0a4f7c0b8 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,35 @@ 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 boolean isCodeResponseType() { + return (responseType.containsKey("code") && (boolean) responseType.get("code")); + } + + public boolean isIdTokenResponseType() { + return (responseType.containsKey("idToken") && (boolean) responseType.get("idToken")); + } + /** * Returns a new {@link UpdateRequest}, which can be used to update the attributes of this * provider config. @@ -58,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. * @@ -99,6 +127,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 +154,36 @@ 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) { + Map map = ensureResponseType(properties); + 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) { + Map map = ensureResponseType(properties); + map.put("idToken", enabled); + return this; + } + CreateRequest getThis() { return this; } @@ -156,6 +227,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 +254,36 @@ 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) { + Map map = ensureResponseType(properties); + 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) { + Map map = ensureResponseType(properties); + 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 f8e454af6..954850100 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java @@ -792,12 +792,19 @@ 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()); + assertTrue(config.isCodeResponseType()); + assertFalse(config.isIdTokenResponseType()); // Get provider config config = auth.getOidcProviderConfigAsync(providerId).get(); @@ -805,7 +812,10 @@ 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()); + assertTrue(config.isCodeResponseType()); + assertFalse(config.isIdTokenResponseType()); // Update provider config OidcProviderConfig.UpdateRequest updateRequest = @@ -813,13 +823,20 @@ 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()); + 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 dd95eccb2..83b98068d 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -1636,7 +1636,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); @@ -1647,7 +1650,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")); } @@ -1661,7 +1670,10 @@ 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 = FirebaseAuth.getInstance().createOidcProviderConfigAsync(createRequest).get(); @@ -1673,7 +1685,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")); } @@ -1876,7 +1894,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); @@ -1885,12 +1906,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 @@ -2969,7 +2996,10 @@ 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()); + assertTrue(config.isCodeResponseType()); + assertFalse(config.isIdTokenResponseType()); } private static void checkSamlProviderConfig(SamlProviderConfig config, String providerId) { @@ -3001,6 +3031,5 @@ 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 8242d5966..0fd6299ee 100644 --- a/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java +++ b/src/test/java/com/google/firebase/auth/OidcProviderConfigTest.java @@ -18,12 +18,14 @@ 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.json.JsonFactory; import com.google.firebase.internal.ApiClientUtils; import java.io.IOException; +import java.util.HashMap; import java.util.Map; import org.junit.Test; @@ -37,7 +39,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 @@ -48,7 +55,35 @@ 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()); + assertTrue(config.isCodeResponseType()); + 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 testEnsureResponseTypeAlreadyPresent() { + 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 @@ -59,15 +94,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) @@ -100,6 +143,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); @@ -118,15 +171,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) @@ -158,4 +219,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 93b7dab94..ba6d28e0b 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,28 @@ 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()); + assertTrue(config.isCodeResponseType()); + assertFalse(config.isIdTokenResponseType()); // 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()); + assertTrue(config.isCodeResponseType()); + assertFalse(config.isIdTokenResponseType()); // Update provider config OidcProviderConfig.UpdateRequest updateRequest = @@ -298,13 +308,20 @@ 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()); + assertFalse(config.isCodeResponseType()); + assertTrue(config.isIdTokenResponseType()); // 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 + } }