Support configuration of protocol binding for authentication requests

[blucas]

Fixes gh-7513.

Note: This is not intended as the final solution, but as a starting point for discussion. I believe a better solution may be to add protocolBinding to Saml2AuthenticationRequest and RelyingPartyRegistration. But I'd like to hear feedback on that approach before submitting any code.

@fhanik FYI.

[fhanik]

Awesome @blucas

Can you add a test case to your pull request, you can use this one so you get familiar with our style:
fhanik@f195ac7

The naming convention for methods is

1. method name of method being tested
2. 'When'
3. condition for tests
4. 'Then'
5. expected results

[blucas]

@fhanik - I didn't add a test as I wasn't convinced this approach was the best solution to the problem. Do you have any thoughts around exposing the protocolBinding to Saml2AuthenticationRequest and RelyingPartyRegistration?

[blucas]

@fhanik - I've added the test and fixed the failure. Please consider my proposal around exposing the protocolBinding to Saml2AuthenticationRequest and RelyingPartyRegistration

[fhanik]

Hi @blucas ,

I've spent some time thinking about this. And the different options as you noted are

1. Just default to POST, no configuration option
2. Allow the OpenSamlAuthenticationRequestFactory to be configured for POST/REDIRECT
3. Expose it in the Saml2AuthenticationRequest object
4. Expose it in the RelyingPartyRegistration object
5. Make is available in Spring Boot's application.yml file

The further down the list we reach, the broader the impact it has on the overall codebase.

Then I started thinking about the use case. It is my belief that POST will probably cover 95%+ of all use cases. I can see why REDIRECT may be frowned upon because you're risking an assertion to be shared elsewhere through the browser, for example via the Referrer header.

I believe that option 1 and 2 are both sufficient. I believe going further down the list, why it may seem flexible, it's a lot of work for very little gain. Specially if we don't see this value change frequently. So I'm thinking a minimalist approach for the sake of simplicity and not creating an overarching configuration base.

What are your thoughts? Am I missing something ?

[blucas]

@fhanik I completely agree with everything you said. I do believe POST as default is the right direction and covers majority of use cases. Additionally it doesn't expose the SAML object to other services (such as access logs).

I feel like the setter approach provided in this PR is the bare minimum we can propose and will suffice for my purposes at the very least.

The reason I mention exposing it further (3 and 4 above) is it feels more like the "spring way" of doing things. Plus is provides more specific configuration (one RPR could use POST while another could use REDIRECT). But having said that, I don't think many people would make sure of that configurability/flexibility.

In summary. I believe the approach of this PR is enough. Any further configuration requests could be handled at a later date.

[fhanik]

Thank @blucas , I'll check out the CI failures tomorrow

[blucas]

@fhanik - I'm not too sure why the build is failing, I haven't touched CAS code... thanks for looking into it for me.