Open mark-webster-catalyst opened 6 years ago
I wish moodle just could have multiple instances of auth plugins ...
For the first version, I propose, instead of registering many IdPs, to allow multiple IdP definitions in the XML and use an external IdP discovery service. The XML must contain at least one IdP. Here is an example of a XML containing more than one IdP definition that you can find the complete list at http://caf-shib2ops.ca/CoreServices/testbed/caf_test_fed_unsigned.xml
<md:EntitiesDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:xi="http://www.w3.org/2001/XInclude"
xmlns:init="urn:oasis:names:tc:SAML:profiles:SSO:request-init"
xmlns:ukfedlabel="http://ukfederation.org.uk/2006/11/label"
xmlns:elab="http://eduserv.org.uk/labels"
xmlns:wayf="http://sdss.ac.uk/2006/06/WAYF"
xmlns:idpdisc="urn:oasis:names:tc:SAML:profiles:SSO:idp-discovery-protocol"
xmlns:mdrpi="urn:oasis:names:tc:SAML:metadata:rpi"
xmlns:mdattr="urn:oasis:names:tc:SAML:metadata:attribute"
xmlns:mdui="urn:oasis:names:tc:SAML:metadata:ui" xmlns:shibmd="urn:mace:shibboleth:metadata:1.0"
xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:ds="http://www.w3.org/2000/09/xmldsig#"
xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata" ID="prefix-20190508142001"
Name="urn:mace:caf-fcga.ca:testfed" validUntil="2019-05-08T14:20:01Z">
<md:EntityDescriptor entityID="https://idp.canarie.ca/idp/shibboleth">
......
</md:EntityDescriptor>
<md:EntityDescriptor entityID="https://id.canarie.ca/simplesaml/saml2/idp/metadata.php">
.....
</md:EntityDescriptor>
</md:EntitiesDescriptor>
Each of entityDescriptor contains IdP's metadata to allow authentication from many IdP. Here is the authentication flow
The current version today certainly used to actually support multiple IDPs by listing the URLs of the IDP's metadata, and hooking that into the list of IDPs etc. to log in with, though it wasn't obvious and it didn't support different mappings per IDP.
I have to be honest, while integration with a DS could be a useful improvement, it's actually not one we've been asked about, because all of the cases that we've done with multiple IDPs, there isn't a suitable discovery service to cover all the IDPs, and listing the multiple IDPs explicitly seems to work much better. (Especially in the one extreme case where we currently have 21 IDPs configured, with another 20 or so to come)
As for Mark's proposed patch, this was long since implemented mostly as described and is in use in production on several customers now, but it needs patching and ideally being merged. I'm in the middle of rebasing the patch against master, but I'd be interested to know whether I should submit it as smaller incremental patches or one big patch.
@peterspicer-catalyst probably smaller ones are easier to review