From 534521716a8f2ec4cdd8ad9f2ab667caa74432ed Mon Sep 17 00:00:00 2001 From: Micah Stairs Date: Wed, 27 May 2020 17:01:21 -0400 Subject: [PATCH 1/3] Add remaining properties to SAML provider config. --- .../firebase/auth/SamlProviderConfig.java | 68 ++++++- .../google/firebase/auth/FirebaseAuthIT.java | 2 + .../auth/FirebaseUserManagerTest.java | 23 ++- .../firebase/auth/SamlProviderConfigTest.java | 173 +++++++++++++++++- .../auth/TenantAwareFirebaseAuthIT.java | 2 + src/test/resources/listSaml.json | 2 + src/test/resources/saml.json | 1 + 7 files changed, 255 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/google/firebase/auth/SamlProviderConfig.java b/src/main/java/com/google/firebase/auth/SamlProviderConfig.java index c8970cc6d..07fce85bd 100644 --- a/src/main/java/com/google/firebase/auth/SamlProviderConfig.java +++ b/src/main/java/com/google/firebase/auth/SamlProviderConfig.java @@ -27,6 +27,7 @@ import com.google.firebase.auth.ProviderConfig.AbstractCreateRequest; import com.google.firebase.auth.ProviderConfig.AbstractUpdateRequest; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -52,6 +53,13 @@ public String getSsoUrl() { return (String) idpConfig.get("ssoUrl"); } + public boolean isRequestSigningEnabled() { + if (!idpConfig.containsKey("signRequest")) { + return false; + } + return (boolean) idpConfig.get("signRequest"); + } + public List getX509Certificates() { List> idpCertificates = (List>) idpConfig.get("idpCertificates"); @@ -162,6 +170,16 @@ public CreateRequest setSsoUrl(String ssoUrl) { return this; } + /** + * Sets whether the request should be signed. + * + * @param enabled A boolean indicating whether the request should be signed. + */ + public CreateRequest setRequestSigningEnabled(boolean requestSigningEnabled) { + ensureNestedMap(properties, "idpConfig").put("signRequest", requestSigningEnabled); + return this; + } + /** * Adds a x509 certificate to the new provider. * @@ -177,7 +195,23 @@ public CreateRequest addX509Certificate(String x509Certificate) { return this; } - // TODO(micahstairs): Add 'addAllX509Certificates' method. + /** + * Adds a collection of x509 certificates to the new provider. + * + * @param x509Certificates A non-null, non-empty collection of x509 certificate strings. + * @throws IllegalArgumentException If the collection is null or empty, or if any x509 + * certificates are null or empty. + */ + public CreateRequest addAllX509Certificates(Collection x509Certificates) { + checkArgument(x509Certificates != null, + "The collection of x509 certificates must not be null."); + checkArgument(!x509Certificates.isEmpty(), + "The collection of x509 certificates must not be empty."); + for (String certificate : x509Certificates) { + addX509Certificate(certificate); + } + return this; + } /** * Sets the RP entity ID for the new provider. @@ -205,8 +239,6 @@ public CreateRequest setCallbackUrl(String callbackUrl) { return this; } - // TODO(micahstairs): Add 'setRequestSigningEnabled' method. - CreateRequest getThis() { return this; } @@ -264,6 +296,16 @@ public UpdateRequest setSsoUrl(String ssoUrl) { return this; } + /** + * Sets whether the request should be signed. + * + * @param enabled A boolean indicating whether the request should be signed. + */ + public UpdateRequest setRequestSigningEnabled(boolean requestSigningEnabled) { + ensureNestedMap(properties, "idpConfig").put("signRequest", requestSigningEnabled); + return this; + } + /** * Adds a x509 certificate to the existing provider. * @@ -279,7 +321,23 @@ public UpdateRequest addX509Certificate(String x509Certificate) { return this; } - // TODO(micahstairs): Add 'addAllX509Certificates' method. + /** + * Adds a collection of x509 certificates to the existing provider. + * + * @param x509Certificates A non-null, non-empty collection of x509 certificate strings. + * @throws IllegalArgumentException If the collection is null or empty, or if any x509 + * certificates are null or empty. + */ + public UpdateRequest addAllX509Certificates(Collection x509Certificates) { + checkArgument(x509Certificates != null, + "The collection of x509 certificates must not be null."); + checkArgument(!x509Certificates.isEmpty(), + "The collection of x509 certificates must not be empty."); + for (String certificate : x509Certificates) { + addX509Certificate(certificate); + } + return this; + } /** * Sets the RP entity ID for the existing provider. @@ -307,8 +365,6 @@ public UpdateRequest setCallbackUrl(String callbackUrl) { return this; } - // TODO(micahstairs): Add 'setRequestSigningEnabled' method. - 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 bc6431d60..7a53c0ea1 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java @@ -685,6 +685,7 @@ public void testSamlProviderConfigLifecycle() throws Exception { .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") + .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -694,6 +695,7 @@ public void testSamlProviderConfigLifecycle() throws Exception { assertTrue(config.isEnabled()); assertEquals("IDP_ENTITY_ID", config.getIdpEntityId()); assertEquals("https://example.com/login", config.getSsoUrl()); + assertFalse(config.isRequestSigningEnabled()); assertEquals(ImmutableList.of("certificate1", "certificate2"), config.getX509Certificates()); assertEquals("RP_ENTITY_ID", config.getRpEntityId()); assertEquals("https://projectId.firebaseapp.com/__/auth/handler", config.getCallbackUrl()); diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java index 9f3a24d48..86bef4e68 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -1798,8 +1798,6 @@ public void testTenantAwareDeleteOidcProviderConfig() throws Exception { public void testCreateSamlProvider() throws Exception { TestResponseInterceptor interceptor = initializeAppForUserManagement( TestUtils.loadResource("saml.json")); - // TODO(micahstairs): Add 'signRequest' to the create request once that field is added to - // SamlProviderConfig. SamlProviderConfig.CreateRequest createRequest = new SamlProviderConfig.CreateRequest() .setProviderId("saml.provider-id") @@ -1807,6 +1805,7 @@ public void testCreateSamlProvider() throws Exception { .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") + .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -1823,16 +1822,19 @@ public void testCreateSamlProvider() throws Exception { GenericJson parsed = parseRequestContent(interceptor); assertEquals("DISPLAY_NAME", parsed.get("displayName")); assertTrue((boolean) parsed.get("enabled")); + Map idpConfig = (Map) parsed.get("idpConfig"); assertNotNull(idpConfig); - assertEquals(3, idpConfig.size()); + assertEquals(4, idpConfig.size()); assertEquals("IDP_ENTITY_ID", idpConfig.get("idpEntityId")); assertEquals("https://example.com/login", idpConfig.get("ssoUrl")); + assertFalse((boolean) idpConfig.get("signRequest")); List idpCertificates = (List) idpConfig.get("idpCertificates"); assertNotNull(idpCertificates); assertEquals(2, idpCertificates.size()); assertEquals(ImmutableMap.of("x509Certificate", "certificate1"), idpCertificates.get(0)); assertEquals(ImmutableMap.of("x509Certificate", "certificate2"), idpCertificates.get(1)); + Map spConfig = (Map) parsed.get("spConfig"); assertNotNull(spConfig); assertEquals(2, spConfig.size()); @@ -1907,6 +1909,7 @@ public void testCreateSamlProviderMissingId() throws Exception { .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") + .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -1931,6 +1934,7 @@ public void testTenantAwareCreateSamlProvider() throws Exception { .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") + .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -1938,7 +1942,7 @@ public void testTenantAwareCreateSamlProvider() throws Exception { TenantAwareFirebaseAuth tenantAwareAuth = FirebaseAuth.getInstance().getTenantManager().getAuthForTenant("TENANT_ID"); - SamlProviderConfig config = tenantAwareAuth.createSamlProviderConfig(createRequest); + tenantAwareAuth.createSamlProviderConfig(createRequest); checkRequestHeaders(interceptor); checkUrl(interceptor, "POST", TENANTS_BASE_URL + "/TENANT_ID/inboundSamlConfigs"); @@ -1948,14 +1952,13 @@ public void testTenantAwareCreateSamlProvider() throws Exception { public void testUpdateSamlProvider() throws Exception { TestResponseInterceptor interceptor = initializeAppForUserManagement( TestUtils.loadResource("saml.json")); - // TODO(micahstairs): Add 'signRequest' to the create request once that field is added to - // SamlProviderConfig. SamlProviderConfig.UpdateRequest updateRequest = new SamlProviderConfig.UpdateRequest("saml.provider-id") .setDisplayName("DISPLAY_NAME") .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") + .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -1968,8 +1971,8 @@ public void testUpdateSamlProvider() throws Exception { checkUrl(interceptor, "PATCH", PROJECT_BASE_URL + "/inboundSamlConfigs/saml.provider-id"); GenericUrl url = interceptor.getResponse().getRequest().getUrl(); assertEquals( - "displayName,enabled,idpConfig.idpCertificates,idpConfig.idpEntityId,idpConfig.ssoUrl," - + "spConfig.callbackUri,spConfig.spEntityId", + "displayName,enabled,idpConfig.idpCertificates,idpConfig.idpEntityId,idpConfig.signRequest," + + "idpConfig.ssoUrl,spConfig.callbackUri,spConfig.spEntityId", url.getFirst("updateMask")); GenericJson parsed = parseRequestContent(interceptor); @@ -1978,9 +1981,10 @@ public void testUpdateSamlProvider() throws Exception { Map idpConfig = (Map) parsed.get("idpConfig"); assertNotNull(idpConfig); - assertEquals(3, idpConfig.size()); + assertEquals(4, idpConfig.size()); assertEquals("IDP_ENTITY_ID", idpConfig.get("idpEntityId")); assertEquals("https://example.com/login", idpConfig.get("ssoUrl")); + assertFalse((boolean) idpConfig.get("signRequest")); List idpCertificates = (List) idpConfig.get("idpCertificates"); assertNotNull(idpCertificates); assertEquals(2, idpCertificates.size()); @@ -2408,6 +2412,7 @@ private static void checkSamlProviderConfig(SamlProviderConfig config, String pr assertTrue(config.isEnabled()); assertEquals("IDP_ENTITY_ID", config.getIdpEntityId()); assertEquals("https://example.com/login", config.getSsoUrl()); + assertFalse(config.isRequestSigningEnabled()); assertEquals(ImmutableList.of("certificate1", "certificate2"), config.getX509Certificates()); assertEquals("RP_ENTITY_ID", config.getRpEntityId()); assertEquals("https://projectId.firebaseapp.com/__/auth/handler", config.getCallbackUrl()); diff --git a/src/test/java/com/google/firebase/auth/SamlProviderConfigTest.java b/src/test/java/com/google/firebase/auth/SamlProviderConfigTest.java index 162c3b63e..08616242f 100644 --- a/src/test/java/com/google/firebase/auth/SamlProviderConfigTest.java +++ b/src/test/java/com/google/firebase/auth/SamlProviderConfigTest.java @@ -43,6 +43,7 @@ public class SamlProviderConfigTest { + " 'idpConfig': {" + " 'idpEntityId': 'IDP_ENTITY_ID'," + " 'ssoUrl': 'https://example.com/login'," + + " 'signRequest': false," + " 'idpCertificates': [" + " { 'x509Certificate': 'certificate1' }," + " { 'x509Certificate': 'certificate2' }" @@ -63,6 +64,7 @@ public void testJsonDeserialization() throws IOException { assertTrue(config.isEnabled()); assertEquals("IDP_ENTITY_ID", config.getIdpEntityId()); assertEquals("https://example.com/login", config.getSsoUrl()); + assertFalse(config.isRequestSigningEnabled()); assertEquals(ImmutableList.of("certificate1", "certificate2"), config.getX509Certificates()); assertEquals("RP_ENTITY_ID", config.getRpEntityId()); assertEquals("https://projectId.firebaseapp.com/__/auth/handler", config.getCallbackUrl()); @@ -77,6 +79,7 @@ public void testCreateRequest() throws IOException { .setEnabled(false) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") + .setRequestSigningEnabled(true) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -90,9 +93,10 @@ public void testCreateRequest() throws IOException { Map idpConfig = (Map) properties.get("idpConfig"); assertNotNull(idpConfig); - assertEquals(3, idpConfig.size()); + assertEquals(4, idpConfig.size()); assertEquals("IDP_ENTITY_ID", idpConfig.get("idpEntityId")); assertEquals("https://example.com/login", idpConfig.get("ssoUrl")); + assertTrue((boolean) idpConfig.get("signRequest")); List idpCertificates = (List) idpConfig.get("idpCertificates"); assertNotNull(idpCertificates); assertEquals(2, idpCertificates.size()); @@ -106,6 +110,29 @@ public void testCreateRequest() throws IOException { assertEquals("https://projectId.firebaseapp.com/__/auth/handler", spConfig.get("callbackUri")); } + @Test + public void testCreateRequestX509Certificates() throws IOException { + SamlProviderConfig.CreateRequest createRequest = + new SamlProviderConfig.CreateRequest() + .addX509Certificate("certificate1") + .addAllX509Certificates(ImmutableList.of("certificate2", "certificate3")) + .addX509Certificate("certificate4"); + + Map properties = createRequest.getProperties(); + assertEquals(1, properties.size()); + Map idpConfig = (Map) properties.get("idpConfig"); + assertNotNull(idpConfig); + assertEquals(1, idpConfig.size()); + + List idpCertificates = (List) idpConfig.get("idpCertificates"); + assertNotNull(idpCertificates); + assertEquals(4, idpCertificates.size()); + assertEquals(ImmutableMap.of("x509Certificate", "certificate1"), idpCertificates.get(0)); + assertEquals(ImmutableMap.of("x509Certificate", "certificate2"), idpCertificates.get(1)); + assertEquals(ImmutableMap.of("x509Certificate", "certificate3"), idpCertificates.get(2)); + assertEquals(ImmutableMap.of("x509Certificate", "certificate4"), idpCertificates.get(3)); + } + @Test(expected = IllegalArgumentException.class) public void testCreateRequestMissingProviderId() { new SamlProviderConfig.CreateRequest().setProviderId(null); @@ -141,6 +168,16 @@ public void testCreateRequestMissingX509Certificate() { new SamlProviderConfig.CreateRequest().addX509Certificate(null); } + @Test(expected = IllegalArgumentException.class) + public void testCreateRequestNullX509CertificatesCollection() { + new SamlProviderConfig.CreateRequest().addAllX509Certificates(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testCreateRequestEmptyX509CertificatesCollection() { + new SamlProviderConfig.CreateRequest().addAllX509Certificates(ImmutableList.of()); + } + @Test(expected = IllegalArgumentException.class) public void testCreateRequestMissingRpEntityId() { new SamlProviderConfig.CreateRequest().setRpEntityId(null); @@ -155,4 +192,138 @@ public void testCreateRequestMissingCallbackUrl() { public void testCreateRequestInvalidCallbackUrl() { new SamlProviderConfig.CreateRequest().setCallbackUrl("not a valid url"); } + + @Test + public void testUpdateRequestFromSamlProviderConfig() throws IOException { + SamlProviderConfig config = jsonFactory.fromString(SAML_JSON_STRING, SamlProviderConfig.class); + + SamlProviderConfig.UpdateRequest updateRequest = config.updateRequest(); + + assertEquals("saml.provider-id", updateRequest.getProviderId()); + assertTrue(updateRequest.getProperties().isEmpty()); + } + + @Test + public void testUpdateRequest() throws IOException { + SamlProviderConfig.UpdateRequest updateRequest = + new SamlProviderConfig.UpdateRequest("saml.provider-id"); + updateRequest + .setDisplayName("DISPLAY_NAME") + .setEnabled(false) + .setIdpEntityId("IDP_ENTITY_ID") + .setSsoUrl("https://example.com/login") + .setRequestSigningEnabled(true) + .addX509Certificate("certificate1") + .addX509Certificate("certificate2") + .setRpEntityId("RP_ENTITY_ID") + .setCallbackUrl("https://projectId.firebaseapp.com/__/auth/handler"); + + Map properties = updateRequest.getProperties(); + assertEquals(4, properties.size()); + assertEquals("DISPLAY_NAME", (String) properties.get("displayName")); + assertFalse((boolean) properties.get("enabled")); + + Map idpConfig = (Map) properties.get("idpConfig"); + assertNotNull(idpConfig); + assertEquals(4, idpConfig.size()); + assertEquals("IDP_ENTITY_ID", idpConfig.get("idpEntityId")); + assertEquals("https://example.com/login", idpConfig.get("ssoUrl")); + assertTrue((boolean) idpConfig.get("signRequest")); + List idpCertificates = (List) idpConfig.get("idpCertificates"); + assertNotNull(idpCertificates); + assertEquals(2, idpCertificates.size()); + assertEquals(ImmutableMap.of("x509Certificate", "certificate1"), idpCertificates.get(0)); + assertEquals(ImmutableMap.of("x509Certificate", "certificate2"), idpCertificates.get(1)); + + Map spConfig = (Map) properties.get("spConfig"); + assertNotNull(spConfig); + assertEquals(2, spConfig.size()); + assertEquals("RP_ENTITY_ID", spConfig.get("spEntityId")); + assertEquals("https://projectId.firebaseapp.com/__/auth/handler", spConfig.get("callbackUri")); + } + + @Test + public void testUpdateRequestX509Certificates() throws IOException { + SamlProviderConfig.UpdateRequest updateRequest = + new SamlProviderConfig.UpdateRequest("saml.provider-id"); + updateRequest + .addX509Certificate("certificate1") + .addAllX509Certificates(ImmutableList.of("certificate2", "certificate3")) + .addX509Certificate("certificate4"); + + Map properties = updateRequest.getProperties(); + assertEquals(1, properties.size()); + Map idpConfig = (Map) properties.get("idpConfig"); + assertNotNull(idpConfig); + assertEquals(1, idpConfig.size()); + + List idpCertificates = (List) idpConfig.get("idpCertificates"); + assertNotNull(idpCertificates); + assertEquals(4, idpCertificates.size()); + assertEquals(ImmutableMap.of("x509Certificate", "certificate1"), idpCertificates.get(0)); + assertEquals(ImmutableMap.of("x509Certificate", "certificate2"), idpCertificates.get(1)); + assertEquals(ImmutableMap.of("x509Certificate", "certificate3"), idpCertificates.get(2)); + assertEquals(ImmutableMap.of("x509Certificate", "certificate4"), idpCertificates.get(3)); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestMissingProviderId() { + new SamlProviderConfig.UpdateRequest(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestInvalidProviderId() { + new SamlProviderConfig.UpdateRequest("oidc.invalid-saml-provider-id"); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestMissingDisplayName() { + new SamlProviderConfig.UpdateRequest("saml.provider-id").setDisplayName(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestMissingIdpEntityId() { + new SamlProviderConfig.UpdateRequest("saml.provider-id").setIdpEntityId(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestMissingSsoUrl() { + new SamlProviderConfig.UpdateRequest("saml.provider-id").setSsoUrl(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestInvalidSsoUrl() { + new SamlProviderConfig.UpdateRequest("saml.provider-id").setSsoUrl("not a valid url"); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestMissingX509Certificate() { + new SamlProviderConfig.UpdateRequest("saml.provider-id").addX509Certificate(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestNullX509CertificatesCollection() { + new SamlProviderConfig.UpdateRequest("saml.provider-id").addAllX509Certificates(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestEmptyX509CertificatesCollection() { + new SamlProviderConfig.UpdateRequest("saml.provider-id") + .addAllX509Certificates(ImmutableList.of()); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestMissingRpEntityId() { + new SamlProviderConfig.UpdateRequest("saml.provider-id").setRpEntityId(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestMissingCallbackUrl() { + new SamlProviderConfig.UpdateRequest("saml.provider-id").setCallbackUrl(null); + } + + @Test(expected = IllegalArgumentException.class) + public void testUpdateRequestInvalidCallbackUrl() { + new SamlProviderConfig.UpdateRequest("saml.provider-id").setCallbackUrl("not a valid url"); + } } diff --git a/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java index 95c8cad5c..43404d987 100644 --- a/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java @@ -346,6 +346,7 @@ public void testSamlProviderConfigLifecycle() throws Exception { .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") + .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -355,6 +356,7 @@ public void testSamlProviderConfigLifecycle() throws Exception { assertTrue(config.isEnabled()); assertEquals("IDP_ENTITY_ID", config.getIdpEntityId()); assertEquals("https://example.com/login", config.getSsoUrl()); + assertFalse(config.isRequestSigningEnabled()); assertEquals(ImmutableList.of("certificate1", "certificate2"), config.getX509Certificates()); assertEquals("RP_ENTITY_ID", config.getRpEntityId()); assertEquals("https://projectId.firebaseapp.com/__/auth/handler", config.getCallbackUrl()); diff --git a/src/test/resources/listSaml.json b/src/test/resources/listSaml.json index 64b1e1e36..a355d0261 100644 --- a/src/test/resources/listSaml.json +++ b/src/test/resources/listSaml.json @@ -6,6 +6,7 @@ "idpConfig": { "idpEntityId": "IDP_ENTITY_ID", "ssoUrl": "https://example.com/login", + "signRequest": false, "idpCertificates": [ { "x509Certificate": "certificate1" }, { "x509Certificate": "certificate2" } @@ -22,6 +23,7 @@ "idpConfig": { "idpEntityId": "IDP_ENTITY_ID", "ssoUrl": "https://example.com/login", + "signRequest": false, "idpCertificates": [ { "x509Certificate": "certificate1" }, { "x509Certificate": "certificate2" } diff --git a/src/test/resources/saml.json b/src/test/resources/saml.json index ef425b0a8..1928d4c6d 100644 --- a/src/test/resources/saml.json +++ b/src/test/resources/saml.json @@ -5,6 +5,7 @@ "idpConfig": { "idpEntityId": "IDP_ENTITY_ID", "ssoUrl": "https://example.com/login", + "signRequest": false, "idpCertificates": [ { "x509Certificate": "certificate1" }, { "x509Certificate": "certificate2" } From 58543bb0c0ebded9adbd384ddb2aad043674dc09 Mon Sep 17 00:00:00 2001 From: Micah Stairs Date: Thu, 28 May 2020 21:15:27 -0400 Subject: [PATCH 2/3] Add comment describing why 'suppressLoadErrors' needs to be set. --- checkstyle.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/checkstyle.xml b/checkstyle.xml index da46b4844..77e2dba54 100644 --- a/checkstyle.xml +++ b/checkstyle.xml @@ -229,6 +229,8 @@ + + From ac111f22a8983ca0337df7b5df9391f03bab84c1 Mon Sep 17 00:00:00 2001 From: Micah Stairs Date: Fri, 29 May 2020 22:16:08 -0400 Subject: [PATCH 3/3] Remove request signing from SAML API. --- .../firebase/auth/SamlProviderConfig.java | 27 ------------------- .../google/firebase/auth/FirebaseAuthIT.java | 2 -- .../auth/FirebaseUserManagerTest.java | 15 +++-------- .../firebase/auth/SamlProviderConfigTest.java | 10 ++----- .../auth/TenantAwareFirebaseAuthIT.java | 2 -- src/test/resources/listSaml.json | 2 -- src/test/resources/saml.json | 1 - 7 files changed, 6 insertions(+), 53 deletions(-) diff --git a/src/main/java/com/google/firebase/auth/SamlProviderConfig.java b/src/main/java/com/google/firebase/auth/SamlProviderConfig.java index 07fce85bd..e74478bea 100644 --- a/src/main/java/com/google/firebase/auth/SamlProviderConfig.java +++ b/src/main/java/com/google/firebase/auth/SamlProviderConfig.java @@ -53,13 +53,6 @@ public String getSsoUrl() { return (String) idpConfig.get("ssoUrl"); } - public boolean isRequestSigningEnabled() { - if (!idpConfig.containsKey("signRequest")) { - return false; - } - return (boolean) idpConfig.get("signRequest"); - } - public List getX509Certificates() { List> idpCertificates = (List>) idpConfig.get("idpCertificates"); @@ -170,16 +163,6 @@ public CreateRequest setSsoUrl(String ssoUrl) { return this; } - /** - * Sets whether the request should be signed. - * - * @param enabled A boolean indicating whether the request should be signed. - */ - public CreateRequest setRequestSigningEnabled(boolean requestSigningEnabled) { - ensureNestedMap(properties, "idpConfig").put("signRequest", requestSigningEnabled); - return this; - } - /** * Adds a x509 certificate to the new provider. * @@ -296,16 +279,6 @@ public UpdateRequest setSsoUrl(String ssoUrl) { return this; } - /** - * Sets whether the request should be signed. - * - * @param enabled A boolean indicating whether the request should be signed. - */ - public UpdateRequest setRequestSigningEnabled(boolean requestSigningEnabled) { - ensureNestedMap(properties, "idpConfig").put("signRequest", requestSigningEnabled); - return this; - } - /** * Adds a x509 certificate to the existing provider. * diff --git a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java index 7a53c0ea1..bc6431d60 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/FirebaseAuthIT.java @@ -685,7 +685,6 @@ public void testSamlProviderConfigLifecycle() throws Exception { .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") - .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -695,7 +694,6 @@ public void testSamlProviderConfigLifecycle() throws Exception { assertTrue(config.isEnabled()); assertEquals("IDP_ENTITY_ID", config.getIdpEntityId()); assertEquals("https://example.com/login", config.getSsoUrl()); - assertFalse(config.isRequestSigningEnabled()); assertEquals(ImmutableList.of("certificate1", "certificate2"), config.getX509Certificates()); assertEquals("RP_ENTITY_ID", config.getRpEntityId()); assertEquals("https://projectId.firebaseapp.com/__/auth/handler", config.getCallbackUrl()); diff --git a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java index 86bef4e68..8c33d5ab8 100644 --- a/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java +++ b/src/test/java/com/google/firebase/auth/FirebaseUserManagerTest.java @@ -1805,7 +1805,6 @@ public void testCreateSamlProvider() throws Exception { .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") - .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -1825,10 +1824,9 @@ public void testCreateSamlProvider() throws Exception { Map idpConfig = (Map) parsed.get("idpConfig"); assertNotNull(idpConfig); - assertEquals(4, idpConfig.size()); + assertEquals(3, idpConfig.size()); assertEquals("IDP_ENTITY_ID", idpConfig.get("idpEntityId")); assertEquals("https://example.com/login", idpConfig.get("ssoUrl")); - assertFalse((boolean) idpConfig.get("signRequest")); List idpCertificates = (List) idpConfig.get("idpCertificates"); assertNotNull(idpCertificates); assertEquals(2, idpCertificates.size()); @@ -1909,7 +1907,6 @@ public void testCreateSamlProviderMissingId() throws Exception { .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") - .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -1934,7 +1931,6 @@ public void testTenantAwareCreateSamlProvider() throws Exception { .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") - .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -1958,7 +1954,6 @@ public void testUpdateSamlProvider() throws Exception { .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") - .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -1971,8 +1966,8 @@ public void testUpdateSamlProvider() throws Exception { checkUrl(interceptor, "PATCH", PROJECT_BASE_URL + "/inboundSamlConfigs/saml.provider-id"); GenericUrl url = interceptor.getResponse().getRequest().getUrl(); assertEquals( - "displayName,enabled,idpConfig.idpCertificates,idpConfig.idpEntityId,idpConfig.signRequest," - + "idpConfig.ssoUrl,spConfig.callbackUri,spConfig.spEntityId", + "displayName,enabled,idpConfig.idpCertificates,idpConfig.idpEntityId,idpConfig.ssoUrl," + + "spConfig.callbackUri,spConfig.spEntityId", url.getFirst("updateMask")); GenericJson parsed = parseRequestContent(interceptor); @@ -1981,10 +1976,9 @@ public void testUpdateSamlProvider() throws Exception { Map idpConfig = (Map) parsed.get("idpConfig"); assertNotNull(idpConfig); - assertEquals(4, idpConfig.size()); + assertEquals(3, idpConfig.size()); assertEquals("IDP_ENTITY_ID", idpConfig.get("idpEntityId")); assertEquals("https://example.com/login", idpConfig.get("ssoUrl")); - assertFalse((boolean) idpConfig.get("signRequest")); List idpCertificates = (List) idpConfig.get("idpCertificates"); assertNotNull(idpCertificates); assertEquals(2, idpCertificates.size()); @@ -2412,7 +2406,6 @@ private static void checkSamlProviderConfig(SamlProviderConfig config, String pr assertTrue(config.isEnabled()); assertEquals("IDP_ENTITY_ID", config.getIdpEntityId()); assertEquals("https://example.com/login", config.getSsoUrl()); - assertFalse(config.isRequestSigningEnabled()); assertEquals(ImmutableList.of("certificate1", "certificate2"), config.getX509Certificates()); assertEquals("RP_ENTITY_ID", config.getRpEntityId()); assertEquals("https://projectId.firebaseapp.com/__/auth/handler", config.getCallbackUrl()); diff --git a/src/test/java/com/google/firebase/auth/SamlProviderConfigTest.java b/src/test/java/com/google/firebase/auth/SamlProviderConfigTest.java index 08616242f..135175227 100644 --- a/src/test/java/com/google/firebase/auth/SamlProviderConfigTest.java +++ b/src/test/java/com/google/firebase/auth/SamlProviderConfigTest.java @@ -43,7 +43,6 @@ public class SamlProviderConfigTest { + " 'idpConfig': {" + " 'idpEntityId': 'IDP_ENTITY_ID'," + " 'ssoUrl': 'https://example.com/login'," - + " 'signRequest': false," + " 'idpCertificates': [" + " { 'x509Certificate': 'certificate1' }," + " { 'x509Certificate': 'certificate2' }" @@ -64,7 +63,6 @@ public void testJsonDeserialization() throws IOException { assertTrue(config.isEnabled()); assertEquals("IDP_ENTITY_ID", config.getIdpEntityId()); assertEquals("https://example.com/login", config.getSsoUrl()); - assertFalse(config.isRequestSigningEnabled()); assertEquals(ImmutableList.of("certificate1", "certificate2"), config.getX509Certificates()); assertEquals("RP_ENTITY_ID", config.getRpEntityId()); assertEquals("https://projectId.firebaseapp.com/__/auth/handler", config.getCallbackUrl()); @@ -79,7 +77,6 @@ public void testCreateRequest() throws IOException { .setEnabled(false) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") - .setRequestSigningEnabled(true) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -93,10 +90,9 @@ public void testCreateRequest() throws IOException { Map idpConfig = (Map) properties.get("idpConfig"); assertNotNull(idpConfig); - assertEquals(4, idpConfig.size()); + assertEquals(3, idpConfig.size()); assertEquals("IDP_ENTITY_ID", idpConfig.get("idpEntityId")); assertEquals("https://example.com/login", idpConfig.get("ssoUrl")); - assertTrue((boolean) idpConfig.get("signRequest")); List idpCertificates = (List) idpConfig.get("idpCertificates"); assertNotNull(idpCertificates); assertEquals(2, idpCertificates.size()); @@ -212,7 +208,6 @@ public void testUpdateRequest() throws IOException { .setEnabled(false) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") - .setRequestSigningEnabled(true) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -225,10 +220,9 @@ public void testUpdateRequest() throws IOException { Map idpConfig = (Map) properties.get("idpConfig"); assertNotNull(idpConfig); - assertEquals(4, idpConfig.size()); + assertEquals(3, idpConfig.size()); assertEquals("IDP_ENTITY_ID", idpConfig.get("idpEntityId")); assertEquals("https://example.com/login", idpConfig.get("ssoUrl")); - assertTrue((boolean) idpConfig.get("signRequest")); List idpCertificates = (List) idpConfig.get("idpCertificates"); assertNotNull(idpCertificates); assertEquals(2, idpCertificates.size()); diff --git a/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java b/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java index 43404d987..95c8cad5c 100644 --- a/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java +++ b/src/test/java/com/google/firebase/auth/TenantAwareFirebaseAuthIT.java @@ -346,7 +346,6 @@ public void testSamlProviderConfigLifecycle() throws Exception { .setEnabled(true) .setIdpEntityId("IDP_ENTITY_ID") .setSsoUrl("https://example.com/login") - .setRequestSigningEnabled(false) .addX509Certificate("certificate1") .addX509Certificate("certificate2") .setRpEntityId("RP_ENTITY_ID") @@ -356,7 +355,6 @@ public void testSamlProviderConfigLifecycle() throws Exception { assertTrue(config.isEnabled()); assertEquals("IDP_ENTITY_ID", config.getIdpEntityId()); assertEquals("https://example.com/login", config.getSsoUrl()); - assertFalse(config.isRequestSigningEnabled()); assertEquals(ImmutableList.of("certificate1", "certificate2"), config.getX509Certificates()); assertEquals("RP_ENTITY_ID", config.getRpEntityId()); assertEquals("https://projectId.firebaseapp.com/__/auth/handler", config.getCallbackUrl()); diff --git a/src/test/resources/listSaml.json b/src/test/resources/listSaml.json index a355d0261..64b1e1e36 100644 --- a/src/test/resources/listSaml.json +++ b/src/test/resources/listSaml.json @@ -6,7 +6,6 @@ "idpConfig": { "idpEntityId": "IDP_ENTITY_ID", "ssoUrl": "https://example.com/login", - "signRequest": false, "idpCertificates": [ { "x509Certificate": "certificate1" }, { "x509Certificate": "certificate2" } @@ -23,7 +22,6 @@ "idpConfig": { "idpEntityId": "IDP_ENTITY_ID", "ssoUrl": "https://example.com/login", - "signRequest": false, "idpCertificates": [ { "x509Certificate": "certificate1" }, { "x509Certificate": "certificate2" } diff --git a/src/test/resources/saml.json b/src/test/resources/saml.json index 1928d4c6d..ef425b0a8 100644 --- a/src/test/resources/saml.json +++ b/src/test/resources/saml.json @@ -5,7 +5,6 @@ "idpConfig": { "idpEntityId": "IDP_ENTITY_ID", "ssoUrl": "https://example.com/login", - "signRequest": false, "idpCertificates": [ { "x509Certificate": "certificate1" }, { "x509Certificate": "certificate2" }