OpenSamlAuthenticationProvider should validate Response Status

[fpagliar]

Describe the bug

The new SAML implementation is missing a couple validations that were performed on the AuthnResponse in spring-security-saml. The most important one is ResponseStatus, that can be different from Success.

This is commonly used by some IdPs (like ADFS) and I'm still trying to run a validation scenario but will probably just fail because it has no assertions, which is not great handling.

Here's the list I've come up with so far after my analysis:

• AuthnResponse Status Code.
• AuthnResponse issue instant.
• InResponseTo (this one is expected, since request-response pairing is not implemented yet).
• Assertion IssueInstant vs a configurable MaxAssertionTime.
• Assertion Issuer vs the IdP EntityID.
• Assertion Subject InResponseTo vs the local endpoint.
• Assertion AuthenticationStatement contextClassRef.
• Assertion AuthenticationStatement AuthnInstant vs configurable maxAuthenticationAge.
• Assertion AuthenticationStatement sessionNotOnOrAfter.

I understand that some might be deprecated or not necessary, or not implemented yet, but wanted to track them and double check to make sure I'm not missing anything.

[jzheaux]

Thanks for researching this, @fpagliar.

but will probably just fail because it has no assertions, which is not great handling

Do you mean to say here that you've got a scenario where the response's status is not success and has no assertions, but you don't want authentication to fail? Can you elaborate on that scenario?

Assertion Issuer vs the IdP EntityID

Note that OpenSamlAuthenticationProvider does validate the Assertion Issuer against the IdP EntityID. Is this what you were looking for?

As for the others, the SAML spec is obviously very large and OpenSamlAuthenticationProvider is not going to be able to make configurable all of the corner cases. So, I'd suggest that we proceed based on community need as opposed to trying to replicate each thing that spring-security-saml did. If there are items in this list that you need, then please create tickets for those, and I'm happy to take a look.

[fpagliar]

The scenario is ADFS will send back an AuthnResponse for a failed authentication flow, with a status code expressing what failed and no assertions since the user was not able to sign in. My point was there should be some graceful handling and logging for this case.

On the other scenario, the code there is validating the issuer of the Response instead of the Assertion. In general the values should be the same, but I think it makes sense to perform the validation on the component that is signed, since it provides integrity.

[jzheaux]

Great, thanks for clearing both of those items up.

I agree that if the response's status indicates a failure, then the failure result from the authentication provider should indicate that instead of simply complaining that there are no assertions.