From a5dd1632bf8ecacf3f9f7f5e7f53f0fc6d5c85ec Mon Sep 17 00:00:00 2001
From: Michiel Kodde <mkodde@ibuildings.nl>
Date: Wed, 4 May 2022 11:58:58 +0200
Subject: [PATCH] Support NameId objects in consent hash generator

The NameID objects where causing issues when creating a stabilized
consent hash. The objects can not be serialized/unserialized. By
normalizing them before starting to perform other normalization tasks on
the attribute array, this issue can be prevented.
---
 .../Service/Consent/ConsentHashService.php    | 27 ++++++++++++++++---
 .../Consent/ConsentHashServiceTest.php        | 25 +++++++++++++++++
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/src/OpenConext/EngineBlock/Service/Consent/ConsentHashService.php b/src/OpenConext/EngineBlock/Service/Consent/ConsentHashService.php
index 12c88a184b..7846d2e780 100644
--- a/src/OpenConext/EngineBlock/Service/Consent/ConsentHashService.php
+++ b/src/OpenConext/EngineBlock/Service/Consent/ConsentHashService.php
@@ -19,6 +19,8 @@
 namespace OpenConext\EngineBlock\Service\Consent;
 
 use OpenConext\EngineBlock\Authentication\Repository\ConsentRepository;
+use OpenConext\UserLifecycle\Domain\ValueObject\Client\Name;
+use SAML2\XML\saml\NameID;
 use function array_filter;
 use function array_keys;
 use function array_values;
@@ -30,6 +32,7 @@
 use function serialize;
 use function sha1;
 use function sort;
+use function str_replace;
 use function strtolower;
 use function unserialize;
 
@@ -90,7 +93,8 @@ public function getUnstableAttributesHash(array $attributes, bool $mustStoreValu
 
     public function getStableAttributesHash(array $attributes, bool $mustStoreValues) : string
     {
-        $lowerCasedAttributes = $this->caseNormalizeStringArray($attributes);
+        $nameIdNormalizedAttributes = $this->nameIdNormalize($attributes);
+        $lowerCasedAttributes = $this->caseNormalizeStringArray($nameIdNormalizedAttributes);
         $hashBase = $mustStoreValues
             ? $this->createHashBaseWithValues($lowerCasedAttributes)
             : $this->createHashBaseWithoutValues($lowerCasedAttributes);
@@ -112,11 +116,13 @@ private function createHashBaseWithoutValues(array $lowerCasedAttributes): strin
 
     /**
      * Lowercases all array keys and values.
-     * Performs the lowercasing on a copy (which is returned), to avoid side-effects.
      */
     private function caseNormalizeStringArray(array $original): array
     {
-        return unserialize(strtolower(serialize($original)));
+        $serialized = serialize($original);
+        $lowerCased = strtolower($serialized);
+        $unserialized = unserialize($lowerCased);
+        return $unserialized;
     }
 
     /**
@@ -187,4 +193,19 @@ private function isBlank($value): bool
     {
         return empty($value) && !is_numeric($value);
     }
+
+    /**
+     * NameId objects can not be serialized/unserialized after being lower cased
+     * Thats why the object is converted to a simple array representation where only the
+     * relevant NameID aspects are stored.
+     */
+    private function nameIdNormalize(array $attributes): array
+    {
+        array_walk_recursive($attributes, function (&$value) {
+            if ($value instanceof NameID) {
+                $value = ['value' => $value->getValue(), 'Format' => $value->getFormat()];
+            }
+        });
+        return $attributes;
+    }
 }
diff --git a/tests/unit/OpenConext/EngineBlock/Service/Consent/ConsentHashServiceTest.php b/tests/unit/OpenConext/EngineBlock/Service/Consent/ConsentHashServiceTest.php
index ace9c0a432..51464edda0 100644
--- a/tests/unit/OpenConext/EngineBlock/Service/Consent/ConsentHashServiceTest.php
+++ b/tests/unit/OpenConext/EngineBlock/Service/Consent/ConsentHashServiceTest.php
@@ -22,6 +22,7 @@
 use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
 use OpenConext\EngineBlock\Authentication\Repository\ConsentRepository;
 use PHPUnit\Framework\TestCase;
+use SAML2\XML\saml\NameID;
 
 class ConsentHashServiceTest extends TestCase
 {
@@ -449,4 +450,28 @@ public function test_stable_attribute_hash_multiple_value_vs_single_value_sequen
         $this->assertEquals($this->chs->getStableAttributesHash($attributes, false), $this->chs->getStableAttributesHash($attributesSingleValue, false));
         $this->assertNotEquals($this->chs->getStableAttributesHash($attributes, true), $this->chs->getStableAttributesHash($attributesSingleValue, true));
     }
+
+    public function test_stable_attribute_hash_can_handle_nameid_objects()
+    {
+        $nameId = new NameID();
+        $nameId->setValue('83aa0a79363edcf872c966b0d6eaf3f5e26a6a77');
+        $nameId->setFormat('urn:oasis:names:tc:SAML:2.0:nameid-format:persistent');
+
+        $attributes = [
+            'urn:mace:dir:attribute-def:displayName' => ['John Doe'],
+            'urn:mace:dir:attribute-def:uid' => ['joe-f12'],
+            'urn:mace:dir:attribute-def:cn' => ['John Doe'],
+            'urn:mace:dir:attribute-def:sn' => ['Doe'],
+            'urn:mace:dir:attribute-def:eduPersonPrincipalName' => ['j.doe@example.com'],
+            'urn:mace:dir:attribute-def:givenName' => ['John'],
+            'urn:mace:dir:attribute-def:mail' => ['j.doe@example.com'],
+            'urn:mace:terena.org:attribute-def:schacHomeOrganization' => ['example.com'],
+            'nl:surf:test:something' => [0 => 'arbitrary-value'],
+            'urn:mace:dir:attribute-def:eduPersonTargetedID' => [$nameId],
+            'urn:oid:1.3.6.1.4.1.5923.1.1.1.10' => [$nameId],
+        ];
+
+        $hash = $this->chs->getStableAttributesHash($attributes, false);
+        $this->assertTrue(is_string($hash));
+    }
 }