From 6900702b216646f0fda46040e200b38c626fa942 Mon Sep 17 00:00:00 2001 From: Mauro Molinari Date: Fri, 5 Nov 2021 18:39:17 +0100 Subject: [PATCH] Revise SP contacts settings validation The contact type check now becomes useful (and so it was restored), because with the new full Contacts support the user may indeed specify invalid contact types in settings. The "not enough data" check, instead, was fixed and it is now raised only if ALL of the contact data (company, given name, surname, e-mail addresses and phone names) are empty, reflecting the actual SAML 2.0 metadata schema constraint. Fixes #353. --- .../saml2/settings/Saml2Settings.java | 30 +++++++++---------- .../test/settings/Saml2SettingsTest.java | 2 ++ .../config/config.allerrors.properties | 4 ++- .../config/config.sperrors.properties | 4 ++- .../com/onelogin/saml2/test/AuthTest.java | 2 +- 5 files changed, 24 insertions(+), 18 deletions(-) diff --git a/core/src/main/java/com/onelogin/saml2/settings/Saml2Settings.java b/core/src/main/java/com/onelogin/saml2/settings/Saml2Settings.java index 292b75e6..9ee29465 100644 --- a/core/src/main/java/com/onelogin/saml2/settings/Saml2Settings.java +++ b/core/src/main/java/com/onelogin/saml2/settings/Saml2Settings.java @@ -5,8 +5,10 @@ import java.security.cert.CertificateEncodingException; import java.security.cert.X509Certificate; import java.util.ArrayList; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.Set; import com.onelogin.saml2.model.hsm.HSM; @@ -1017,27 +1019,25 @@ public List checkSPSettings() { List contacts = this.getContacts(); if (!contacts.isEmpty()) { -/* - List validTypes = new ArrayList(); - validTypes.add("technical"); - validTypes.add("support"); - validTypes.add("administrative"); - validTypes.add("billing"); - validTypes.add("other"); -*/ + Set validTypes = new HashSet<>(); + validTypes.add(Constants.CONTACT_TYPE_TECHNICAL); + validTypes.add(Constants.CONTACT_TYPE_SUPPORT); + validTypes.add(Constants.CONTACT_TYPE_ADMINISTRATIVE); + validTypes.add(Constants.CONTACT_TYPE_BILLING); + validTypes.add(Constants.CONTACT_TYPE_OTHER); for (Contact contact : contacts) { -/* if (!validTypes.contains(contact.getContactType())) { errorMsg = "contact_type_invalid"; errors.add(errorMsg); LOGGER.error(errorMsg); } -*/ - - if (contact.getEmailAddresses().isEmpty() || contact.getEmailAddresses().stream().allMatch(StringUtils::isEmpty) || - (StringUtils.isEmpty(contact.getCompany()) && - StringUtils.isEmpty(contact.getGivenName()) && - StringUtils.isEmpty(contact.getSurName()))) { + if ((contact.getEmailAddresses().isEmpty() + || contact.getEmailAddresses().stream().allMatch(StringUtils::isEmpty)) + && (contact.getTelephoneNumbers().isEmpty() || contact.getTelephoneNumbers() + .stream().allMatch(StringUtils::isEmpty)) + && StringUtils.isEmpty(contact.getCompany()) + && StringUtils.isEmpty(contact.getGivenName()) + && StringUtils.isEmpty(contact.getSurName())) { errorMsg = "contact_not_enough_data"; errors.add(errorMsg); LOGGER.error(errorMsg); diff --git a/core/src/test/java/com/onelogin/saml2/test/settings/Saml2SettingsTest.java b/core/src/test/java/com/onelogin/saml2/test/settings/Saml2SettingsTest.java index 8a3875da..67bbf84b 100644 --- a/core/src/test/java/com/onelogin/saml2/test/settings/Saml2SettingsTest.java +++ b/core/src/test/java/com/onelogin/saml2/test/settings/Saml2SettingsTest.java @@ -114,6 +114,7 @@ public void testCheckSPSettingsAllErrors() throws IOException, Error { assertThat(settingsErrors, hasItem("sp_entityId_not_found")); assertThat(settingsErrors, hasItem("sp_acs_not_found")); assertThat(settingsErrors, hasItem("sp_cert_not_found_and_required")); + assertThat(settingsErrors, hasItem("contact_type_invalid")); assertThat(settingsErrors, hasItem("contact_not_enough_data")); assertThat(settingsErrors, hasItem("organization_not_enough_data")); } @@ -151,6 +152,7 @@ public void testCheckSettingsAllErrors() throws IOException, Error { assertThat(settingsErrors, hasItem("sp_entityId_not_found")); assertThat(settingsErrors, hasItem("sp_acs_not_found")); assertThat(settingsErrors, hasItem("sp_cert_not_found_and_required")); + assertThat(settingsErrors, hasItem("contact_type_invalid")); assertThat(settingsErrors, hasItem("contact_not_enough_data")); assertThat(settingsErrors, hasItem("organization_not_enough_data")); assertThat(settingsErrors, hasItem("idp_entityId_not_found")); diff --git a/core/src/test/resources/config/config.allerrors.properties b/core/src/test/resources/config/config.allerrors.properties index 1b41f6f0..80f5e1a4 100644 --- a/core/src/test/resources/config/config.allerrors.properties +++ b/core/src/test/resources/config/config.allerrors.properties @@ -19,4 +19,6 @@ onelogin.saml2.organization.name = SP Java onelogin.saml2.organization.url = http://sp.example.com # Contacts -onelogin.saml2.contacts.support.email_address = support@example.com \ No newline at end of file +onelogin.saml2.sp.contact[0].contactType=administrative +onelogin.saml2.sp.contact[1].contactType=nonexistent +onelogin.saml2.sp.contact[1].company=ACME \ No newline at end of file diff --git a/core/src/test/resources/config/config.sperrors.properties b/core/src/test/resources/config/config.sperrors.properties index 6a1a649f..f80b95d1 100644 --- a/core/src/test/resources/config/config.sperrors.properties +++ b/core/src/test/resources/config/config.sperrors.properties @@ -30,4 +30,6 @@ onelogin.saml2.organization.name = SP Java onelogin.saml2.organization.displayname = SP Java Example # Contacts -onelogin.saml2.contacts.support.given_name = Support Guy \ No newline at end of file +onelogin.saml2.sp.contact[0].contactType=administrative +onelogin.saml2.sp.contact[1].contactType=nonexistent +onelogin.saml2.sp.contact[1].company=ACME \ No newline at end of file diff --git a/toolkit/src/test/java/com/onelogin/saml2/test/AuthTest.java b/toolkit/src/test/java/com/onelogin/saml2/test/AuthTest.java index 6c376839..cd45125f 100644 --- a/toolkit/src/test/java/com/onelogin/saml2/test/AuthTest.java +++ b/toolkit/src/test/java/com/onelogin/saml2/test/AuthTest.java @@ -303,7 +303,7 @@ public void testConstructorInvalidSettings() throws IOException, SettingsExcepti Saml2Settings settings = new SettingsBuilder().fromFile("config/config.sperrors.properties").build(); expectedEx.expect(SettingsException.class); - expectedEx.expectMessage("Invalid settings: sp_entityId_not_found, sp_acs_not_found, sp_cert_not_found_and_required, contact_not_enough_data, organization_not_enough_data, idp_cert_or_fingerprint_not_found_and_required, idp_cert_not_found_and_required"); + expectedEx.expectMessage("Invalid settings: sp_entityId_not_found, sp_acs_not_found, sp_cert_not_found_and_required, contact_not_enough_data, contact_type_invalid, organization_not_enough_data, idp_cert_or_fingerprint_not_found_and_required, idp_cert_not_found_and_required"); new Auth(settings, request, response); }