Skip to content

Commit 6900702

Browse files
committed
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 SAML-Toolkits#353.
1 parent f7364f7 commit 6900702

File tree

5 files changed

+24
-18
lines changed

5 files changed

+24
-18
lines changed

core/src/main/java/com/onelogin/saml2/settings/Saml2Settings.java

+15-15
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
import java.security.cert.CertificateEncodingException;
66
import java.security.cert.X509Certificate;
77
import java.util.ArrayList;
8+
import java.util.HashSet;
89
import java.util.LinkedList;
910
import java.util.List;
11+
import java.util.Set;
1012

1113
import com.onelogin.saml2.model.hsm.HSM;
1214

@@ -1017,27 +1019,25 @@ public List<String> checkSPSettings() {
10171019

10181020
List<Contact> contacts = this.getContacts();
10191021
if (!contacts.isEmpty()) {
1020-
/*
1021-
List<String> validTypes = new ArrayList<String>();
1022-
validTypes.add("technical");
1023-
validTypes.add("support");
1024-
validTypes.add("administrative");
1025-
validTypes.add("billing");
1026-
validTypes.add("other");
1027-
*/
1022+
Set<String> validTypes = new HashSet<>();
1023+
validTypes.add(Constants.CONTACT_TYPE_TECHNICAL);
1024+
validTypes.add(Constants.CONTACT_TYPE_SUPPORT);
1025+
validTypes.add(Constants.CONTACT_TYPE_ADMINISTRATIVE);
1026+
validTypes.add(Constants.CONTACT_TYPE_BILLING);
1027+
validTypes.add(Constants.CONTACT_TYPE_OTHER);
10281028
for (Contact contact : contacts) {
1029-
/*
10301029
if (!validTypes.contains(contact.getContactType())) {
10311030
errorMsg = "contact_type_invalid";
10321031
errors.add(errorMsg);
10331032
LOGGER.error(errorMsg);
10341033
}
1035-
*/
1036-
1037-
if (contact.getEmailAddresses().isEmpty() || contact.getEmailAddresses().stream().allMatch(StringUtils::isEmpty) ||
1038-
(StringUtils.isEmpty(contact.getCompany()) &&
1039-
StringUtils.isEmpty(contact.getGivenName()) &&
1040-
StringUtils.isEmpty(contact.getSurName()))) {
1034+
if ((contact.getEmailAddresses().isEmpty()
1035+
|| contact.getEmailAddresses().stream().allMatch(StringUtils::isEmpty))
1036+
&& (contact.getTelephoneNumbers().isEmpty() || contact.getTelephoneNumbers()
1037+
.stream().allMatch(StringUtils::isEmpty))
1038+
&& StringUtils.isEmpty(contact.getCompany())
1039+
&& StringUtils.isEmpty(contact.getGivenName())
1040+
&& StringUtils.isEmpty(contact.getSurName())) {
10411041
errorMsg = "contact_not_enough_data";
10421042
errors.add(errorMsg);
10431043
LOGGER.error(errorMsg);

core/src/test/java/com/onelogin/saml2/test/settings/Saml2SettingsTest.java

+2
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ public void testCheckSPSettingsAllErrors() throws IOException, Error {
114114
assertThat(settingsErrors, hasItem("sp_entityId_not_found"));
115115
assertThat(settingsErrors, hasItem("sp_acs_not_found"));
116116
assertThat(settingsErrors, hasItem("sp_cert_not_found_and_required"));
117+
assertThat(settingsErrors, hasItem("contact_type_invalid"));
117118
assertThat(settingsErrors, hasItem("contact_not_enough_data"));
118119
assertThat(settingsErrors, hasItem("organization_not_enough_data"));
119120
}
@@ -151,6 +152,7 @@ public void testCheckSettingsAllErrors() throws IOException, Error {
151152
assertThat(settingsErrors, hasItem("sp_entityId_not_found"));
152153
assertThat(settingsErrors, hasItem("sp_acs_not_found"));
153154
assertThat(settingsErrors, hasItem("sp_cert_not_found_and_required"));
155+
assertThat(settingsErrors, hasItem("contact_type_invalid"));
154156
assertThat(settingsErrors, hasItem("contact_not_enough_data"));
155157
assertThat(settingsErrors, hasItem("organization_not_enough_data"));
156158
assertThat(settingsErrors, hasItem("idp_entityId_not_found"));

core/src/test/resources/config/config.allerrors.properties

+3-1
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,6 @@ onelogin.saml2.organization.name = SP Java
1919
onelogin.saml2.organization.url = http://sp.example.com
2020

2121
# Contacts
22-
onelogin.saml2.contacts.support.email_address = [email protected]
22+
onelogin.saml2.sp.contact[0].contactType=administrative
23+
onelogin.saml2.sp.contact[1].contactType=nonexistent
24+
onelogin.saml2.sp.contact[1].company=ACME

core/src/test/resources/config/config.sperrors.properties

+3-1
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,6 @@ onelogin.saml2.organization.name = SP Java
3030
onelogin.saml2.organization.displayname = SP Java Example
3131

3232
# Contacts
33-
onelogin.saml2.contacts.support.given_name = Support Guy
33+
onelogin.saml2.sp.contact[0].contactType=administrative
34+
onelogin.saml2.sp.contact[1].contactType=nonexistent
35+
onelogin.saml2.sp.contact[1].company=ACME

toolkit/src/test/java/com/onelogin/saml2/test/AuthTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ public void testConstructorInvalidSettings() throws IOException, SettingsExcepti
303303
Saml2Settings settings = new SettingsBuilder().fromFile("config/config.sperrors.properties").build();
304304

305305
expectedEx.expect(SettingsException.class);
306-
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");
306+
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");
307307
new Auth(settings, request, response);
308308
}
309309

0 commit comments

Comments
 (0)