Constraints imposed on contacts on settings validation

[mauromol]

Currently, when settings are validated, the following gets executed:

if (contact.getEmailAddress().isEmpty() || contact.getGivenName().isEmpty()) {
	errorMsg = "contact_not_enough_data";
	errors.add(errorMsg);
	LOGGER.error(errorMsg);
}

Question is: why? I mean, both the given name and e-mail address are optional in SAML 2.0 specification (section 2.3.2.2 of the metadata specification). We can say that it's good practice to specify them, however there are scenarios in which they are not defined and metadata should still validate correctly (e.g.: SPID assessment for full aggregator subjects).

What do you think?

[pitbulk]

The contactType value should be validated (not sure why its validation was commented, I can't remember) and verify its presence.

Related to the validation that you shared in the ticket, the xsd defines:
Sequence [1..1]
md:Extensions [0..1]
md:Company [0..1]
md:GivenName [0..1]
md:SurName [0..1]
md:EmailAddress [0..]
md:TelephoneNumber [0..]

So all are optional, but at least one may be defined. Since we are only support as the toolkit as Contact attributes the EmailAddress and the GivenName, at least on may be provide in order to generate a valid md:ContactPerson

I think makes sense to validate in the app that uses the java-saml toolkit that the minimun required fields where provided to the Contact object if you want to avoid this error on Metadata validation cycle.

[mauromol]

Thank you @pitbulk , this is clear now. However then note that there's a problem with the above check: indeed, if just ONE of given name or e-mail address is empty, validation will fail. Instead, from what we're saying, validation should fail if BOTH are empty.

I agree that perhaps a looser validation here won't hurt, especially because, even if the application does not do it by itself, the generated metadata will then be invalid and com.onelogin.saml2.settings.Saml2Settings.validateMetadata(String) (which we all are supposed to call, right? :-D) will notify us there's a problem (due to XML schema validation failure).

So we may return on this topic if we will decide to implement #326...