spring-security icon indicating copy to clipboard operation
spring-security copied to clipboard

Provide templating or extension to generate SAML2 POST form

Open tkupka opened this issue 3 years ago • 8 comments

The Saml2WebSsoAuthenticationRequestFilter#createSamlPostRequestFormData(...) should allow to customize generated HTML form for SAML2 login request.

The Saml2WebSsoAuthenticationRequestFilter#createSamlPostRequestFormData(...) create a HTML form which is hardcoded. It should supports custom form generation by adding a templating or providing extension for custom implementation. The existing implementation uses <body onload=\"document.forms[0].submit()\"> which is preventing for adopting stricter CSP see https://csp.withgoogle.com/docs/adopting-csp.html. The onLoad event handler must be added from extrenal js file.

tkupka avatar Mar 30 '21 09:03 tkupka

@tkupka, thanks for the report.

I think Saml2WebSsoAuthenticationRequestFilter can be changed in the following way:

  • Save the Saml2AuthenticationRequest to a request attribute
  • Allow for configuring a custom forwarding URL
  • If the custom forwarding URL is set, then the dispatcher forwards to that URL; otherwise the default HTML is used.

With this arrangement, you can introduce an endpoint that retrieves the Saml2AuthenticationRequest as a request attribute and render your own UI.

Does this sound like it would address your concerns? If so, are you able to submit a PR?

jzheaux avatar Feb 10 '22 20:02 jzheaux

Hello Josh, the proposed solution is feasible but it looks slightly overcomplicated with custom forwarding URL which is responsible for rendering the UI. I looked through the code and the same issue might affect 3 places.

  • Saml2WebSsoAuthenticationRequestFilter
  • Saml2LogoutRequestFilter
  • Saml2RelyingPartyInitiatedLogoutSuccessHandler

I had in the mind that instead of hardcoding the page content you might have freemarker template for example. The default would be included in the archive and custom one can be configured in the same way like forwarding URL (for all 3 places). This would give implementor easy way to change generated page with no additional code.

On the other hand the I see bigger flexibility with custom endpoint which renders the UI. But it requires overhead with coding custom endpoint(s).

What do you think about templating solution ? I can provide PR with the fix.

tkupka avatar Jun 07 '22 12:06 tkupka

Thanks for the idea, @tkupka. I agree that using Freemarker could also achieve this; however, I don't want to introduce it as a Spring Security dependency. One reason for this is that applications often already have chosen a templating engine for other things, and it's not ideal that Spring Security might force them to bring in an additional one.

Are you okay with instead submitting a PR that supports the custom endpoint?

jzheaux avatar Jun 07 '22 13:06 jzheaux

OK I see your point it makes no sense to add new dependency just for that. Looking into opensaml-saml-impl I see that they similar approach with velocity template(s). Would it be an option?

##
## Velocity Template for SAML 2 HTTP-POST binding
##
## Velocity context may contain the following properties
## action - String - the action URL for the form
## binding - String - the SAML binding type in use
## RelayState - String - the relay state for the message
## SAMLRequest - String - the Base64 encoded SAML Request
## SAMLResponse - String - the Base64 encoded SAML Response
##
<!DOCTYPE html>
<html>
    <head>
        <meta charset="utf-8" />
        #parse ( "/templates/add-html-head-content.vm" )
    </head>
    <body onload="document.forms[0].submit()">
        <noscript>
            <p>
                <strong>Note:</strong> Since your browser does not support JavaScript,
                you must press the Continue button once to proceed.
            </p>
        </noscript>
        
        <form action="${action}" method="post">
            <div>
                #if($RelayState)<input type="hidden" name="RelayState" value="${RelayState}"/>#end
                
                #if($SAMLRequest)<input type="hidden" name="SAMLRequest" value="${SAMLRequest}"/>#end
                
                #if($SAMLResponse)<input type="hidden" name="SAMLResponse" value="${SAMLResponse}"/>#end
                
            </div>
            <noscript>
                <div>
                    <input type="submit" value="Continue"/>
                </div>
            </noscript>
        </form>
        #parse ( "/templates/add-html-body-content.vm" )
    </body>
</html>

tkupka avatar Jun 07 '22 13:06 tkupka

This may not be the best fix but I found this similar issue with a way to support hashes. The part where they add the meta tag, if that is not cumulative with spring security's existing CSP config, then that part should be omitted.

Github Similar Issue Github Commit Adding Basic Hash Support

mmoussa-mapfre avatar Jul 08 '22 11:07 mmoussa-mapfre

Interesting suggestion, @mmoussa-mapfre.

Would you be able to open a new ticket and submit a PR that specifies a hash-based Content-Security-Policy for the SAML pages?

jzheaux avatar Jul 11 '22 18:07 jzheaux

@jzheaux Sorry, I cannot.

mmoussa-mapfre avatar Jul 11 '22 18:07 mmoussa-mapfre

Hi,

i added a new ticket #11631 and will prepare a PR.

ugrave avatar Jul 27 '22 06:07 ugrave