:embed_sign vs. IdP SingleSignOnService Binding (HTTP Method)

[johnnyshields]

Correct me if I'm wrong, but it appears the :embed_sign parameter is redundant with the IdP URL Binding. In other words:

• if the Binding is urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST, then you MUST have embed_sign: true
• else if the Binding is urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect, then you MUST have embed_sign: false

(Otherwise, your requests won't be signed, e.g. a POST with embed_sign: false will simply have no signature.)

Assuming this is the case, since embed_sign is a single value, it's not possible to mix SSO POST with SLO Redirect. Although not common in practice, it is theoretically possible that an IdP could require it that way in their metadata.

If this is the case, I wonder if there might be a better way to do this. For example, we could get rid of :embed_sign and introduce Settings.idp_sso_binding and Settings.idp_slo_binding parameters which would allow you to control which bindings you want to use for each endpoint.

[pitbulk]

'embed_sign' was an original parameter from the original ruby-saml gem.

If you review rest of OneLogin SAML toolkits, there are no 'embed_sign' there

I agree that supporting settings.idp_sso_binding and settings.idp_slo_binding would be better than the current 'embed_sign', but not sure if its implementation wont carry extra headache since we will need to keep supporting 'embed_sign' as well, until deprecate it.

`` Assuming this is the case, since embed_sign is a single value, it's not possible to mix SSO POST with SLO Redirect.```

Not really. You can modify the embed_sign parameter on the setting before initializing the logout_request/logout_response, based on idp data stored in a database setting or so, if you support multiple IdPs, or directly coded in the SSO and SLS service of the app if only supporting 1 IdP

[johnnyshields]

I will raise a PR which does the following:

• Add settings.idp_sso_binding and settings.idp_slo_binding with values :post and :redirect
• embed_sign will be deprecated and will set the above two params if they are not explicitly set (the newly added params will take priority.)

Sound good?

[pitbulk]

Sound good, but rather than :post, :redirect
Better to put the real values:

• https://github.com/onelogin/php-saml/blob/master/settings_example.php#L31
• https://github.com/onelogin/php-saml/blob/master/settings_example.php#L57

[pitbulk]

Its ok to receive :post :redirect or a realvalue...but we should store the real value

[johnnyshields]

PR raised