phpsaml icon indicating copy to clipboard operation
phpsaml copied to clipboard

requestedAuthnContext = 'none' instead of requestedAuthnContext = false,

Open DonutsNL opened this issue 3 years ago • 6 comments

In my GLPI 10.0.2 environment using the latest (just downloaded) version.

When not selecting any value in the AuthNContext, the database is updated with the value 'none.'

This value is not validated properly in the getAuthn($value) function (phpsaml.class.php) expecting either a value not set or an empty value before it evaluates false. Because the value 'none' is passed it will continue parsing using the value 'none'. Eventually this value is also returned to the config resulting in an invalid phpsaml configuration.

I added an validation to the first IF statement in the getAuthn() function to correct this:

static public function getAuthn($value){ if (!isset($value) || $value == '' || $value == 'none'){ return false; }

DonutsNL avatar Jul 24 '22 17:07 DonutsNL

mysql> select requested_authn_context from glpi_plugin_phpsaml_configs; +-------------------------+ | requested_authn_context | +-------------------------+ | none | +-------------------------+ 1 row in set (0.00 sec)

DonutsNL avatar Jul 24 '22 17:07 DonutsNL

Can you submit a PR for this?

Sent from my iPhone

On Jul 24, 2022, at 11:11 AM, DonutsNL @.***> wrote:



mysql> select requested_authn_context from glpi_plugin_phpsaml_configs; +-------------------------+ | requested_authn_context | +-------------------------+ | none | +-------------------------+ 1 row in set (0.00 sec)

— Reply to this email directly, view it on GitHubhttps://github.com/derricksmith/phpsaml/issues/93#issuecomment-1193358845, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABWQAN5MPMAOACYFRWXUOE3VVV2L5ANCNFSM54P6C54A. You are receiving this because you are subscribed to this thread.Message ID: @.***>

derricksmith avatar Jul 24 '22 17:07 derricksmith

Im not a github champion im afraid :S do everything locally tbh.

DonutsNL avatar Jul 24 '22 17:07 DonutsNL

Ok, no prob. I’ll correct shortly.

Sent from my iPhone

On Jul 24, 2022, at 11:14 AM, DonutsNL @.***> wrote:



Im not a github champion im afraid :S do everything locally tbh.

— Reply to this email directly, view it on GitHubhttps://github.com/derricksmith/phpsaml/issues/93#issuecomment-1193359367, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABWQAN63ZUBOM2OTZMMKVADVVV2WTANCNFSM54P6C54A. You are receiving this because you commented.Message ID: @.***>

derricksmith avatar Jul 24 '22 17:07 derricksmith

Evaluating database behaviour on updates..

Manually set, to empty.

mysql> update glpi_plugin_phpsaml_configs set requested_authn_context = ''; Query OK, 1 row affected (0.01 sec) Rows matched: 1 Changed: 1 Warnings: 0

mysql> select requested_authn_context from glpi_plugin_phpsaml_configs; +-------------------------+ | requested_authn_context | +-------------------------+ | | +-------------------------+ 1 row in set (0.00 sec)

Select X509 from config menu.

mysql> select requested_authn_context from glpi_plugin_phpsaml_configs; +-------------------------+ | requested_authn_context | +-------------------------+ | none,X509 | +-------------------------+ 1 row in set (0.01 sec)

### Remove X509 from config menu. mysql> select requested_authn_context from glpi_plugin_phpsaml_configs; +-------------------------+ | requested_authn_context | +-------------------------+ | none | +-------------------------+ 1 row in set (0.00 sec)

Key insight, none is allways passed as value including requested authN method.

Suggested approach

// Only process if more then 'none' is in the value. if(preg_match('/^none,.+/i',$value)){ // do explode and stuff }else{ // default to not sending an header at all maybe add some debugging here as well. return false; }

DonutsNL avatar Jul 24 '22 17:07 DonutsNL

Created a pull request. Ignore the comment in the code, what I was trying to say is that the selected combination of values might not be implementable by the idp and therefore deliver unexpected results. For example selecting both X509 and Password at the same time using exact matching.

DonutsNL avatar Jul 24 '22 18:07 DonutsNL