SAML-Toolkits / wordpress-saml

OneLogin SAML plugin for Wordpress
MIT License
65 stars 75 forks source link

SAML request improperly generated when requestedAuthNContext is empty #101

Closed davidaah closed 3 years ago

davidaah commented 3 years ago

Description of issue

When selecting the "empty" value for requestedAuthNContext, a SAML claim is being generated as follows:

    <samlp:RequestedAuthnContext Comparison="exact">
    </samlp:RequestedAuthnContext>

This is resulting in the IdP being unable to process the request.

Expected behavior

When selecting the "empty" value for requestedAuthNContext, the generated SAML request should exclude the above entry entirely.

Additional info

The issue appears to occur at https://github.com/onelogin/wordpress-saml/blob/master/onelogin-saml-sso/php/settings.php#L50 where the plugin assumes that a selected empty value will result in an empty array. However, in practice the attribute onelogin_saml_advanced_requestedauthncontext appears to be getting stored as an array with a single value of empty string:

array(1) { [0]=> string(0) "" }
pitbulk commented 3 years ago

The OneLogin SAML extension uses php-saml as dependency.

If you check the requestedAuthnContext setting: https://github.com/onelogin/php-saml/blob/master/advanced_settings_example.php#L74

-  Set to false and no AuthContext will be sent in the AuthNRequest,
- Set true or don't present this parameter and you will get an AuthContext 'exact' 'urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport'
- Set an array with the possible auth context values: array ('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', 'urn:oasis:names:tc:SAML:2.0:ac:classes:X509'),

An empty entry on WordPress should then be assumed by the extension as a false value, so no AuthContext should be sent at all.

Instead

empty($requested_authncontext_values

I believe it should be used instead:

empty(array_filter($requested_authncontext_values)