fixes #322: allow SAML responses without Conditions

[ThePetrov]

The Conditions element is technically optional, so for maximum compatibility I don't think it should be required by the library. On the other hand I can't deny the security benefits brought by requiring that element to exist. To get the best of both worlds I've added a new configuration setting wantConditionsPresent and retained the default behavior of requiring the Conditions element for a response to be considered valid. If someone actually needs to validate responses without the Conditions element they can set that property to false.

[mauromol]

Please note, however, that the SAML 2.0 specification says:

• section 3.4.1.4 of the Core specification, describing the processing rules of the Authentication Request Protocol:

The resulting assertion(s) MUST contain a <saml:AudienceRestriction> element
referencing the requester as an acceptable relying party. Other audiences MAY be included as
deemed appropriate by the identity provider.

• section 4.1.4.2 of the Profiles specification, describing the Response usage in the Web SSO Profile:

The assertion(s) containing a bearer subject confirmation MUST contain an
<AudienceRestriction> including the service provider's unique identifier as an <Audience>

So, since the <AudienceRestriction> element appears in the <Conditions> block and java-saml scope is the Web SSO Profile, IMHO enforcing that a <Conditions> element is present is correct.

What I would argue, instead, is why java-saml requires just one <Conditions> element at most to be present: the Profiles specification says in fact:

Other conditions (and other <Audience> elements) MAY be included as requested by the service
provider or at the discretion of the identity provider. (Of course, all such conditions MUST be
understood by and accepted by the service provider in order for the assertion to be considered valid.)

[ThePetrov]

That's very insightful, thanks for the reply @mauromol ! You're correct, I wasn't aware that the Conditions were enforced in the profiles part of the spec.

You're correct about multiple Conditions being a possibility, besides the specification this is also allowed in the DTD. I wonder what's the maintainer's take on the subject.
EDIT: I was wrong actually, the DTD doesn't declare a maxOccurs attribute on the Conditions element (source) and the default value of maxOccurs is 1. Which means that there can be either no Conditions element in an Assertion or exactly once. Given that the specification mentions conditions in a lower case I assume it refers to conditions declared within a Conditions element rather than multiple Conditions elements, especially considering that contents of a Conditions element don't have an upper bound on the amount and that the semantics of multiple audience restrictions are discussed in section 2.5.1.4 of the core specification.

Note that multiple elements MAY be included in a single assertion, and each MUST be evaluated independently. The effect of this requirement and the preceding definition is that within a given condition, the audiences form a disjunction (an "OR") while multiple conditions form a conjunction (an "AND")

[ThePetrov]

@pitbulk Do you think this is a change worth pursuing?

[mauromol]

EDIT: I was wrong actually, the DTD doesn't declare a maxOccurs attribute on the Conditions element (source) and the default value of maxOccurs is 1. Which means that there can be either no Conditions element in an Assertion or exactly once. Given that the specification mentions conditions in a lower case I assume it refers to conditions declared within a Conditions element rather than multiple Conditions elements, especially considering that contents of a Conditions element don't have an upper bound on the amount and that the semantics of multiple audience restrictions are discussed in section 2.5.1.4 of the core specification.

You are right: it's not the <Conditions> element which might appear multiple times, but rather the existing one, if any, may contain multiple <Condition> and <AudienceRestriction> subelements.
However, the requirement imposed by the profile specification with regards to the need of an <AudienceRestriction> still stands.
So, indeed, the code in com.onelogin.saml2.authn.SamlResponse.checkOneCondition() is partially useless: the actual check that should be made is that at least one <AudienceRestriction> exists and that at least one matches the SP entity id. com.onelogin.saml2.authn.SamlResponse.validateAudiences() should be doing that but indeed fails because it accepts the lack of any <AudienceRestriction>.

I'm going to open an issue for this.