OpenConextApps / simplesamlphp-module-stepupsfo

OpenConext Stepup Second Factor Only Authproc Filter for SimpleSAMLphp :key:
GNU Lesser General Public License v2.1
4 stars 3 forks source link

Support forwarding the AuthnContextClassRef downstream #3

Closed dnmvisser closed 2 years ago

dnmvisser commented 2 years ago

The AuthnContextClassRef that is sent to an SP, is not affected by whether the stepupsfo module is used. For example without this module the SP will receive:

<saml:AuthnContext
<saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</saml:AuthnContextClassRef>
</saml:AuthnContext>

But when the stepupsfo module is used, and the user successfully did MFA, the AuthnContextClassRef would be exactly the same. This prevents SPs to can selectively bypass their own MFA implementation from doing so. Gitlab is a example of such an SP: https://docs.gitlab.com/ee/integration/saml.html#bypass-two-factor-authentication

As a feature request, the stepupsfo module should be forwarding the AuthnContextClassRef that it received from the SFO IdP.

thijskh commented 2 years ago

The default operation of the plugin is to enfore SFO/MFA when it is invoked. So for that case this is not relevant, right? Because if it's unconditional you can just set in SSP the desired ACCR in config.

It's only relevant if you are using the skipentities setting and you want the ACCR to be different for entities that are in the list or not in the list (original ACCR for the former, SSID identifier for the latter). Do I understand this correctly?

dnmvisser commented 2 years ago

My point was that from the perspective of the SP, SimpleSAMLphp now sends an inaccurate value for AuthnContextClassRef when the stepupsfo module is used. So a downstream SP can not make correct decisions based on it.

If stepupsfo would always set AuthnContextClassRef to whatever the SFO endpoint returned, then this problem would be fixed. I don't see any (obvious) downside of this.

I looked at setting the AuthnContextClassRef with the saml:AuthnContextClassRef authproc filter, but that seems to only allow for setting static values. But even if it didn't, I don't know how to get the AuthnContextClassRef value that the SFO sent....

dnmvisser commented 2 years ago

Actually, I was able to do this with an extra authproc filter after the stepupsfo one:

$80 = [
    'class' => 'core:PHP',
    'code' => '
        $accr = \SAML2\Binding::getCurrentBinding()->receive()->getAssertions()[0]->getAuthnContextClassRef();
        $state["saml:AuthnContextClassRef"] = $accr;
    ',
];
tvdijen commented 2 years ago

Ideally this module would do this by default