Error in Apache log

[nickjer]

I see this for an unmapped user who logged in from the Ohio Technology Consortium through CILogon:

[Fri Sep 23 09:08:41.476526 2016] [:error] [pid 37389] [client 131.187.116.27:3743] PHP Notice:  Undefined index: OIDC_CLAIM_name in /var/www/ood/register/index.php on line 88, referer: https://idp.oar.net/idp/profile/SAML2/Redirect/SSO

[ericfranz]

Interesting.

The issue is with this code in display_login_form function:

  $provider_claims = array_filter(array(
    "Name" => $_SERVER["OIDC_CLAIM_idp_name"],
    "Login ID" => $_SERVER["OIDC_CLAIM_eppn"],
    "Login User" => $_SERVER["OIDC_CLAIM_name"],
    "Login Email" => $_SERVER["OIDC_CLAIM_email"]
  ));

In the version of PHP we are using, if OIDC_CLAIM_name doesn't exist, $_SERVER["OIDC_CLAIM_name"] is null, so when I log in with an OH-TECH account, this ends up being:

  $provider_claims = array_filter(array(
    "Name" => "Ohio Technology Consortium (OH-TECH)",
    "Login ID" => "efranz@oar.net",
    "Login User" => null,
    "Login Email" => "efranz@osc.edu"
  ));

And array_filter without any extra arguments will filter out all the key value pairs where the value is null. So we end up with:

  $provider_claims = array(
    "Name" => "Ohio Technology Consortium (OH-TECH)",
    "Login ID" => "efranz@oar.net",
    "Login Email" => "efranz@osc.edu"
  );

I can still navigate to register but do not see an error displayed. So the user doesn't experience a crash, though the log file gets this PHP Notice: Undefined index:. Perhaps I should use another method to get the value that is more intentional about being okay with an undefined index.

[ericfranz]

From http://php.net/manual/en/language.types.array.php

Note: Attempting to access an array key which has not been defined is the same as accessing any other undefined variable: an E_NOTICE-level error message will be issued, and the result will be NULL.

[nickjer]

Is there an equivalent fetch method in PHP?

[ericfranz]

I haven't found it yet. @basilgohar do you know of one off the top of your head? or else I'll accomplish this some other way; perhaps:

$provider_claim_display_fields = array(
  "Name" => "OIDC_CLAIM_idp_name",
  "Login ID" => "OIDC_CLAIM_eppn",
  "Login User" => "OIDC_CLAIM_name",
  "Login Email" => "OIDC_CLAIM_email"
);

$provider_claims = array();
foreach($provider_claim_display_fields as $k => $v){
  if(array_key_exists($v, $_SERVER)){
     $provider_claims[$k] = $_SERVER[$v]
  }
}

[ericfranz]

@brianmcmichael my last comment here shows a solution. This is to avoid warnings being posted.

[brianmcmichael]

What about setting defaults with the ternary shorthand?

  $provider_claims = array_filter(array(
    "Name" => $_SERVER["OIDC_CLAIM_idp_name"] ?: "Undefined",
    "Login ID" => $_SERVER["OIDC_CLAIM_eppn"] ?: "Undefined",
    "Login User" => $_SERVER["OIDC_CLAIM_name"] ?: "Undefined",
    "Login Email" => $_SERVER["OIDC_CLAIM_email"] ?: "Undefined"
  ));

I'm not sure how to test this in production. Seems to work on commandline.

bmcmichael@webdev02:~$ php -a
Interactive shell
php >   $provider_claims = array_filter(array(
php (     "Name" => $_SERVER["OIDC_CLAIM_idp_name"] ?: "Undefined",
php (     "Login ID" => $_SERVER["OIDC_CLAIM_eppn"] ?: "Undefined",
php (     "Login User" => $_SERVER["OIDC_CLAIM_name"] ?: "Undefined",
php (     "Login Email" => $_SERVER["OIDC_CLAIM_email"] ?: "Undefined"
php (   ));
PHP Notice:  Undefined index: OIDC_CLAIM_idp_name in php shell code on line 2
PHP Notice:  Undefined index: OIDC_CLAIM_eppn in php shell code on line 3
PHP Notice:  Undefined index: OIDC_CLAIM_name in php shell code on line 4
PHP Notice:  Undefined index: OIDC_CLAIM_email in php shell code on line 5

php > print_r($provider_claims);
Array
(
    [Name] => Undefined
    [Login ID] => Undefined
    [Login User] => Undefined
    [Login Email] => Undefined
)
php > 

[brianmcmichael]

Disregard the last message, I see you want to get rid of the undefined index warnings.

What about this? Seems a little simpler:

function serverFetch($key) {
    return isset($_SERVER[$key]) ? $_SERVER[$key] : $key;
}

$provider_claims = array_filter(array(
  "Name" => serverFetch("OIDC_CLAIM_idp_name"),
  "Login ID" => serverFetch("OIDC_CLAIM_eppn"),
  "Login User" => serverFetch("OIDC_CLAIM_name"),
  "Login Email" => serverFetch("OIDC_CLAIM_email")
));

Result:

php > function serverFetch($key) {
php { return isset($_SERVER[$key]) ? $_SERVER[$key] : $key;
php { }
php > 
php > $provider_claims = array_filter(array(
php (   "Name" => serverFetch("OIDC_CLAIM_idp_name"),
php (   "Login ID" => serverFetch("OIDC_CLAIM_eppn"),
php (   "Login User" => serverFetch("OIDC_CLAIM_name"),
php (   "Login Email" => serverFetch("OIDC_CLAIM_email")
php ( ));
php > print_r($provider_claims);
Array
(
    [Name] => OIDC_CLAIM_idp_name
    [Login ID] => OIDC_CLAIM_eppn
    [Login User] => OIDC_CLAIM_name
    [Login Email] => OIDC_CLAIM_email
)
php > 

[ericfranz]

If you do that we would want this:

function serverFetch($key, $default = NULL) {
    return isset($_SERVER[$key]) ? $_SERVER[$key] : $default;
}

The reason is that the whole point is to filter out claims values that are not set. This way if OIDC_CLAIM_idp_name and OIDC_CLAIM_eppn are the only values set you would end up with:

$provider_claims = array_filter(array(
  "Name" => serverFetch("OIDC_CLAIM_idp_name"),
  "Login ID" => serverFetch("OIDC_CLAIM_eppn"),
  "Login User" => serverFetch("OIDC_CLAIM_name"),
  "Login Email" => serverFetch("OIDC_CLAIM_email")
));

# => call serverFetch

$provider_claims = array_filter(array(
  "Name" => "Google",
  "Login ID" => "efranzosc@google.com",
  "Login User" => NULL,
  "Login Email" => NULL
));

# = > call array_filter:

$provider_claims = array(
  "Name" => "Google",
  "Login ID" => "efranzosc@google.com"
);

Sorry - automated tests would have helped here.

[brianmcmichael]

Ok, so no errors, and no undefined keys.

[ericfranz]

FWIW these are similar in length:

serverFetch("OIDC_CLAIM_idp_name")
fetch($_SERVER, "OIDC_CLAIM_idp_name")

I'd vote for the second, as its pretty clear without looking at the implementation of fetch what it does, especially coming from Ruby.

[ericfranz]

To clarify, this isn't really a bug, just a nuisance. The log file claims its an [error] but its really just an annoying warning PHP Notice: Undefined index.

[brianmcmichael]

I didn't want to do that because it's specific to the $_SERVER variable here, and should not be confused with similar functionality on a $_GET or $_POST request.

It was also a safeguard because I didn't want to override any existing functionality in php.

[brianmcmichael]

Oh, I see, you are passing in the $_SERVER, so we're making a general fetch() method.

[ericfranz]

Yep. There is no fetch function in PHP

[brianmcmichael]

I was just going to scope this to the method. But it can probably be more general if we think it's going to be used elsewhere.

[brianmcmichael]

PR at https://github.com/OSC/ood_auth_registration/pull/10