samlify icon indicating copy to clipboard operation
samlify copied to clipboard

Validation and relay state customization

Open dddoronnn opened this issue 7 years ago • 5 comments

This library is awesome in its simplicity and straightforwardness, nevertheless, I've encountered 2 things that were missing IMO.

Luckily, they were pretty easy to solve:

  1. The relayState parameter had to be set directly on the ServiceProvider entity, which required creating a new provider each time a login request had to be created with a different relay state. (That was already discussed in #163)
  2. All validations were mandatory. My specific concern was the issuer validation, since my app is multi-tenant, and I would like to be able to get SAML responses from multiple issuers into the same endpoint, then dispatching them to the right client based on the issuer.

dddoronnn avatar Mar 30 '19 12:03 dddoronnn

Coverage Status

Coverage decreased (-0.7%) to 86.93% when pulling 73be9f281cf740fbc6499c80c56fef21c3c0132e on dddoronnn:master into 4563872932eaac83735dba1bb169b05413fbf4a6 on tngan:master.

coveralls avatar Mar 30 '19 12:03 coveralls

Coverage Status

Coverage decreased (-0.7%) to 86.93% when pulling 73be9f281cf740fbc6499c80c56fef21c3c0132e on dddoronnn:master into 4563872932eaac83735dba1bb169b05413fbf4a6 on tngan:master.

coveralls avatar Mar 30 '19 12:03 coveralls

@dddoronnn

Thanks for your contribution.

  1. That's good, but see if you could add tests for the changes as well.
  2. Is it possible for you to create a different pair of sp and idp ? skipping the basic validation might not be a good option in terms of security.

tngan avatar Apr 01 '19 02:04 tngan

@tngan

  1. Sure, will do.

  2. It's not about creating multiple IdPs, I'm already doing so when generating the SAML request - Using the same endpoint for accepting multiple responses is the tricky part. I want to be able to publish a single app onto Okta's marketplace and then I can only provide a single ACS url that accepts responses from my different clients. After I'm validating the response, I use the IssuerID to tell which client it came from - I just do it inside my app's own code.

That is a common practice, here from Okta's documentation: https://www.okta.com/integrate/documentation/saml/#single-idp-vs-multiple-idps

For a single-instance multi-tenant application where the tenancy is not defined in the URL (such as via a subdomain) ... you must then rely on additional information in the SAML response to determine which IDP is trying to authenticate (for example, using the IssuerID)

Having that said, I agree that for most use cases the validation is important and the default should be going through all possible checks.

dddoronnn avatar Apr 01 '19 07:04 dddoronnn

For 2) if you could determine the issuer ID from the SAML response without parsing it fully, you could then construct the corresponding idp at runtime, and not have to skip the basic validation. A suggestion for how that could work is https://github.com/tngan/samlify/issues/357#issuecomment-612470777

tjunnone avatar Apr 11 '20 17:04 tjunnone