catalyst / moodle-auth_saml2

SAML done 100% in Moodle, fast, simple, secure
https://moodle.org/plugins/auth_saml2
70 stars 132 forks source link

Enhancement Request: Expanded ProtocolBinding Options for SP in SAML2 Multi-IdP Environment #784

Open Logiar opened 7 months ago

Logiar commented 7 months ago

Issue Summary:

The Service Provider (SP) configuration, currently lacks an explicit setting for the ProtocolBinding parameter. As a result, AuthnRequest messages default to ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", mandating that Identity Providers (IdPs) use HTTP-POST for response binding. This default behavior is incompatible with IdPs requiring different bindings, such as HTTP-Artifact, in a multi-IdP context.

Expected Behavior:

It should be possible to enable the explicit specification of ProtocolBinding for SAML responses, reflecting the full range of options supported by SimpleSAMLphp:

This feature should allow for per-IdP configuration, ensuring compatibility with varying IdP requirements in a multi-IdP setup.

Current Behavior:

Without a specific ProtocolBinding setting in the SP configuration, the SP's AuthnRequest messages default to instructing IdPs to use HTTP-POST for response binding. This limitation hinders interoperability with IdPs that require alternative binding methods.

Steps to Reproduce:

  1. Integrate with an IdP that requires a binding method other than HTTP-POST, such as HTTP-Artifact.
  2. Note that the SP's AuthnRequest defaults to HTTP-POST binding, causing compatibility issues.

Proposed Solution:

Implement a configurable option to override the default ProtocolBinding for each IdP, allowing selection from the supported binding methods (HTTP-POST, Holder-of-Key, HTTP-Artifact, HTTP-Redirect) as per the requirements of each integrated IdP.

Logiar commented 7 months ago

To provide some additional context to this and #791 in case it helps other users impacted by this. These issues+pull requests were made for the plugin to be compatible with the new saml2 proxy solution for the Norwegian IdP ID-porten. #791 might not be necessary anymore for compatibility, but it is still uncertain. This solution requires HTTP-Artifact to be used rather than HTTP-Post.

In addition to the pull requests I've made for these two issues I also had to increase the verification depth for the ssl_context in SOAPClient and included the root certificate for their certificate in one of the ca stores (openssl.capath or OS ca store). I've not made a pull request for this as it resides in simplesamlphp. The code seem to be written with the assumption that the signing certificate in the metadata is self signed and also used for ssl. While I'm not sure I agree with this assumption, doing a simple edit such as I've done in our modifications makes new assumptions that may not be general. Making heavy edits to allow for better configuration of the ssl context will by our estimation have a low chance of making it into a version of the library that will be compatible with this 3.9 branch. So we opted for the smallest possible local modification we could think of to solve this. Should this be an acceptable modification for the plugin let me know and I'll make an issue and pull request for it as well.

peter- commented 2 months ago

It should be possible to enable the explicit specification of ProtocolBinding for SAML responses, reflecting the full range of options supported by SimpleSAMLphp:

* urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST

* urn:oasis:names:tc:SAML:2.0:profiles:holder-of-key:SSO:browser

* urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact

* urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect

Note that not all of these protocol bindings may be legal for transmitting a SAML Response from the IDP to the SP, cf. SAML 2.0 Profiles, e.g. section 4.1.2, item 5:

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

Logiar commented 1 month ago

I updated the pull request to limit the options to post and artifact.