requestedAuthnContext = 'none' instead of requestedAuthnContext = false,
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; }
mysql> select requested_authn_context from glpi_plugin_phpsaml_configs; +-------------------------+ | requested_authn_context | +-------------------------+ | none | +-------------------------+ 1 row in set (0.00 sec)
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: @.***>
Im not a github champion im afraid :S do everything locally tbh.
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: @.***>
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; }
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.