remove extra values from shib attributes (#308)

* remove extra values from shib attributes

typo

* add comment

* throw SSOException, require attributes not empty

* add test

* fix test, move from functional to unit

Author: simonLeary42

resources/lib/UnitySSO.php

@@ -26,15 +26,44 @@ private static function eppnToOrg($eppn)
         return strtolower($org);
     }
 
+    // shibboleth service provider writes attributes into "server variables"
+    // shibboleth service provider does not garuntee attributes are set, even REMOTE_USER
+    // https://shibboleth.atlassian.net/wiki/spaces/SP3/pages/2065335257/AttributeAccess
+    // I have observed attributes to be set to empty strings while shibd complains of bad config
+    private static function getAttributeRaw($attributeName, $fallbackAttributeName = null)
+    {
+        if (isset($_SERVER[$attributeName]) && $_SERVER[$attributeName] != "") {
+            return $_SERVER[$attributeName];
+        }
+        if (is_null($fallbackAttributeName)) {
+            throw new SSOException("\$_SERVER[\"$attributeName\"] is unset or empty!");
+        }
+        if (isset($_SERVER[$fallbackAttributeName]) && $_SERVER[$fallbackAttributeName] != "") {
+            return $_SERVER[$fallbackAttributeName];
+        }
+        throw new SSOException(
+            "\$_SERVER[\"$attributeName\"] and \$_SERVER[\"$fallbackAttributeName\"]"
+            . " are both unset or empty!"
+        );
+    }
+
+    private static function getAttribute($attributeName, $fallbackAttributeName = null)
+    {
+        $attribute_raw = self::getAttributeRaw($attributeName, $fallbackAttributeName);
+        // attributes may have multiple values, by default they are split by ';'
+        // see SPConfig setting attributeValueDelimiter
+        return explode(";", $attribute_raw)[0];
+    }
+
     public static function getSSO()
     {
         return array(
-            "user" => self::eppnToUID($_SERVER["REMOTE_USER"]),
-            "org" => self::eppnToOrg($_SERVER["REMOTE_USER"]),
-            "firstname" => $_SERVER["givenName"],
-            "lastname" => $_SERVER["sn"],
-            "name" => $_SERVER["givenName"] . " " . $_SERVER["sn"],
-            "mail" => isset($_SERVER["mail"]) ? $_SERVER["mail"] : $_SERVER["eppn"]
+            "user" => self::eppnToUID(self::getAttribute("REMOTE_USER")),
+            "org" => self::eppnToOrg(self::getAttribute("REMOTE_USER")),
+            "firstname" => self::getAttribute("givenName"),
+            "lastname" => self::getAttribute("sn"),
+            "name" => self::getAttribute("givenName") . " " . self::getAttribute("sn"),
+            "mail" => self::getAttribute("mail", "eppn")
         );
     }
 }

test/phpunit-bootstrap.php

@@ -196,6 +196,23 @@ function getNonExistentUserAndExpectedUIDGIDWithCustomMapping()
     return [["[email protected]", "foo", "bar", "[email protected]"], 555];
 }
 
+function getMultipleValueAttributesAndExpectedSSO()
+{
+    return [
+        [
+            "REMOTE_USER" => "[email protected]",
+            "givenName" => "foo;foo",
+            "sn" => "bar;bar",
+            "mail" => "[email protected];[email protected]",
+        ],
+        [
+            "firstname" => "foo",
+            "lastname" => "bar",
+            "mail" => "[email protected]",
+        ]
+    ];
+}
+
 function getAdminUser()
 {
     return ["[email protected]", "foo", "bar", "[email protected]"];

test/unit/UnitySSOTest.php

@@ -0,0 +1,25 @@
+<?php
+
+namespace UnityWebPortal\lib;
+
+use PHPUnit\Framework\TestCase;
+
+class UnitySSOTest extends TestCase
+{
+    public function testMultipleAttributeValues()
+    {
+        $PREVIOUS_SERVER = $_SERVER;
+        $two_vars = getMultipleValueAttributesAndExpectedSSO();
+        $attributes = $two_vars[0];
+        $expectedSSO = $two_vars[1];
+        try {
+            $_SERVER = array_merge($_SERVER, $attributes);
+            $sso = UnitySSO::getSSO();
+            $this->assertEquals($expectedSSO["firstname"], $sso["firstname"]);
+            $this->assertEquals($expectedSSO["lastname"], $sso["lastname"]);
+            $this->assertEquals($expectedSSO["mail"], $sso["mail"]);
+        } finally {
+            $_PREVIOUS_SERVER = $_SERVER;
+        }
+    }
+}