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.

Author: mauromol

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<String> checkSPSettings() {
 
 		List<Contact> contacts = this.getContacts();
 		if (!contacts.isEmpty()) {
-/*			
-			List<String> validTypes = new ArrayList<String>();
-			validTypes.add("technical");
-			validTypes.add("support");
-			validTypes.add("administrative");
-			validTypes.add("billing");
-			validTypes.add("other");
-*/
+			Set<String> 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);

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"));

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
+onelogin.saml2.sp.contact[0].contactType=administrative
+onelogin.saml2.sp.contact[1].contactType=nonexistent
+onelogin.saml2.sp.contact[1].company=ACME

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
+onelogin.saml2.sp.contact[0].contactType=administrative
+onelogin.saml2.sp.contact[1].contactType=nonexistent
+onelogin.saml2.sp.contact[1].company=ACME

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);
 	}