Describe the bug When setting RelyingPartyRegistrations assertionConsumerServiceBinding to Saml2MessageBinding.REDIRECT we do not consume the "SigAlg" and "Signature" query parameters in the response to validate the SAMLResponse leading to the following error message.
Either the response or one of the assertions is unsigned. Please either sign the response or all of the assertions.
We do set the parameters on the outgoing redirect requests to the IDP as resolved in https://github.com/spring-projects/spring-security/issues/7711 so this is basically the flip side of that issue.
To Reproduce Configure a RelyingPartyRegistrationRepository with assertionConsumerServiceBinding set Saml2MessageBinding.REDIRECT and your IDP set to sign responses but not assertions. In Keycloak I just have the "Sign Documents" option checked but not the "Sign Assertions". If you change the binding to POST everything should work normally with just the document but not assertions signed but on REDIRECT it will fail because the Query Parameter isn't considered.
Expected behavior Both REDIRECT and POST SAML Response Bindings should work with just the response signed.
Sample
A link to a GitHub repository with a minimal, reproducible sample.
Reports that include a sample will take priority over reports that do not. At times, we may require a sample, so it is good to try and include a sample up front.
Comment From: jzheaux
Hi, @shawnweeks, thanks for the report.
Setting the ACS binding to REDIRECT is not supported, given that the SAML Specification (line 420) states that it must not be used (emphasis mine):
- Identity Provider issues
to Service Provider In step 5, the identity provider issues a message to be delivered by the user agent to the service provider. Either the HTTP POST, or HTTP Artifact binding can be used to transfer the message to the service provider through the user agent. The message may indicate an error, or will include (at least) an authentication assertion. The HTTP Redirect binding MUST NOT be used, as the response will typically exceed the URL length permitted by most user agents.
As this question has come up before, I think it would be good to add this detail to the JavaDoc. @shawnweeks, are you able to add a link to this detail in the spec to the RelyingPartyRegistration.Builder#assertionConsumerServiceBInding method?
Comment From: spring-projects-issues
If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.
Comment From: shawnweeks
It would be a while before I have time to submit a PR for this. It should be noted though Keycloak and presumably other SAML IDPs do actually support this binding and despite the spec saying not to do it.
Comment From: jzheaux
Thanks for the feedback, @shawnweeks. For now, I think we should focus on the spec. If applications need to depart from the spec, they can construct a filter.
Comment From: AkashB23
@jzheaux I got that as per spec HTTP-REDIRECT is not recommended for SAMLResponse, then why do we need the https://github.com/spring-projects/spring-security/blob/8f104b60fc0e7c3f7434da83f14016f9b1fbffd7/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/web/Saml2AuthenticationTokenConverter.java#L108
where if we get the request with GET, we try to inflate it, why do we need , any specific reason it is present. (as per my understanding deflated responses will only be part of REDIRECT call