Service Provider Review

[rwinch]

• If only a single IdP the Service Provider should automatically redirect to the IdP by setting the Authentication Entry Point to /saml/sp/authenticate/<idp-key>
• Saml2HttpMessageData is to general. It implies that this object would be used for Saml2 messages, but it is not used for the SAMLResponse returned by the IdP. I think we should just remove this object.
• Saml2ServiceProviderMethods has too many methods on it. Many of the methods are only used in a single place which means it can be (at least for now) be moved to a private method.
• We should try to avoid passing the HttpServletRequest to too many layers. Instead, the top level API might allow for an HttpServletRequest, but the remaining layers (if any should avoid using it). An example of this is that the HttpServletRequest is passed to Saml2AuthenticationRequestResolver which then passes it downstream to Saml2ServiceProviderMethods and then Saml2ServiceProviderResolver
• Generally, interfaces should expose interfaces and not classes (unless they are simple POJOs). For example, Saml2ServiceProviderResolver should not expose the class Saml2ServiceProviderInstance.
• Saml2Object.getImplementation does not make sense to me. There are lots of layers of abstraction only to expose the implementation. Why do we need the Saml2Object? If we do need it, why do we need Saml2Object.getImplementation?
• I'd like the domain objects to be simplified. For example, I find it challenging to believe that all of the properties on Saml2AuthenticationRequest are necessary. Many of the properties are null and thus clearly not used.

[rwinch]

Closing in favor of the new mvp/minimal branch