Integrate stable attribute hash requirements

The requirements are stated in the story:

https://www.pivotaltracker.com/story/show/176513931 and specifically
these points:

A challenge is that we do not want to invalidate all given consents with the
current algorithm. So we implement it as follows:

*  We do not touch the existing consent hashing method at all
*  We create a new hashing method that is more stable.
*  We cover this new method with an abundance of unit tests to verify the
   stability given all sorts of inputs.
*  We change the consent query from (pseudocode):
     SELECT *
     FROM consent
     WHERE user = me
     AND consenthash = hashfromoldmethod
     OR consenthash = hashfromnewmethod
*  Newly given consent will be stored with the new hash.
*  When old consent matched, still generate new consent hash (without showing
   consent screen

library/EngineBlock/Corto/Model/Consent.php

@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 
+use OpenConext\EngineBlock\Authentication\Value\ConsentVersion;
 use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
 use OpenConext\EngineBlock\Authentication\Value\ConsentType;
 use OpenConext\EngineBlock\Service\Consent\ConsentHashServiceInterface;
@@ -83,14 +84,27 @@ public function __construct(
 
     public function explicitConsentWasGivenFor(ServiceProvider $serviceProvider): bool
     {
-        return !$this->_consentEnabled ||
-            $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_EXPLICIT);
+        $consent = $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_EXPLICIT);
+        return !$this->_consentEnabled || $consent->given();
+    }
+
+    /**
+     * Although the user has given consent previously we want to upgrade the deprecated unstable consent
+     * to the new stable consent type.
+     * https://www.pivotaltracker.com/story/show/176513931
+     */
+    public function upgradeAttributeHashFor(ServiceProvider $serviceProvider, string $consentType): void
+    {
+        $consentVersion = $this->_hasStoredConsent($serviceProvider, $consentType);
+        if ($consentVersion->isUnstable()) {
+            $this->_updateConsent($serviceProvider, $consentType);
+        }
     }
 
     public function implicitConsentWasGivenFor(ServiceProvider $serviceProvider): bool
     {
-        return !$this->_consentEnabled ||
-            $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_IMPLICIT);
+        $consent = $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_IMPLICIT);
+        return !$this->_consentEnabled || $consent->given();
     }
 
     public function giveExplicitConsentFor(ServiceProvider $serviceProvider): bool
@@ -142,28 +156,47 @@ private function _storeConsent(ServiceProvider $serviceProvider, $consentType):
         return $this->_hashService->storeConsentHash($parameters);
     }
 
-    private function _hasStoredConsent(ServiceProvider $serviceProvider, $consentType): bool
+    private function _updateConsent(ServiceProvider $serviceProvider, $consentType): bool
     {
         $parameters = array(
+            $this->_getStableAttributesHash($this->_responseAttributes),
+            $this->_getAttributesHash($this->_responseAttributes),
             sha1($this->_getConsentUid()),
             $serviceProvider->entityId,
-            $this->_getAttributesHash($this->_responseAttributes),
             $consentType,
         );
 
-        $hasUnstableConsentHash = $this->_hashService->retrieveConsentHash($parameters);
+        return $this->_hashService->updateConsentHash($parameters);
+    }
 
-        if ($hasUnstableConsentHash) {
-            return true;
+    private function _hasStoredConsent(ServiceProvider $serviceProvider, $consentType): ConsentVersion
+    {
+        $consentUuid = sha1($this->_getConsentUid());
+        $parameters = [
+            $consentUuid,
+            $serviceProvider->entityId,
+            $this->_getStableAttributesHash($this->_responseAttributes),
+            $consentType,
+        ];
+        $hasStableConsentHash = $this->_hashService->retrieveStableConsentHash($parameters);
+
+        if ($hasStableConsentHash) {
+            return ConsentVersion::stable();
         }
 
-        $parameters[2] = array(
-            sha1($this->_getConsentUid()),
+        $parameters = [
+            $consentUuid,
             $serviceProvider->entityId,
-            $this->_getStableAttributesHash($this->_responseAttributes),
+            $this->_getAttributesHash($this->_responseAttributes),
             $consentType,
-        );
+        ];
+
+        $hasUnstableConsentHash = $this->_hashService->retrieveConsentHash($parameters);
+
+        if ($hasUnstableConsentHash) {
+            return ConsentVersion::unstable();
+        }
 
-        return $this->_hashService->retrieveConsentHash($parameters);
+        return ConsentVersion::notGiven();
     }
 }

library/EngineBlock/Corto/Module/Service/ProcessConsent.php

@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 
+use OpenConext\EngineBlock\Authentication\Value\ConsentType;
 use OpenConext\EngineBlock\Service\AuthenticationStateHelperInterface;
 use OpenConext\EngineBlock\Service\ProcessingStateHelperInterface;
 use SAML2\Constants;
@@ -102,6 +103,7 @@ public function serve($serviceName, Request $httpRequest)
         if (!$consentRepository->explicitConsentWasGivenFor($serviceProvider)) {
             $consentRepository->giveExplicitConsentFor($destinationMetadata);
         }
+        $consentRepository->upgradeAttributeHashFor($destinationMetadata, ConsentType::TYPE_EXPLICIT);
 
         $response->setConsent(Constants::CONSENT_OBTAINED);
         $response->setDestination($response->getReturn());

library/EngineBlock/Corto/Module/Service/ProvideConsent.php

@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 
+use OpenConext\EngineBlock\Authentication\Value\ConsentType;
 use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider;
 use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
 use OpenConext\EngineBlock\Service\AuthenticationStateHelperInterface;
@@ -136,6 +137,7 @@ public function serve($serviceName, Request $httpRequest)
             if (!$consentRepository->implicitConsentWasGivenFor($serviceProviderMetadata)) {
                 $consentRepository->giveImplicitConsentFor($serviceProviderMetadata);
             }
+            $consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_IMPLICIT);
 
             $response->setConsent(Constants::CONSENT_INAPPLICABLE);
             $response->setDestination($response->getReturn());
@@ -153,6 +155,8 @@ public function serve($serviceName, Request $httpRequest)
 
         $priorConsent = $consentRepository->explicitConsentWasGivenFor($serviceProviderMetadata);
         if ($priorConsent) {
+            $consentRepository->upgradeAttributeHashFor($serviceProviderMetadata, ConsentType::TYPE_EXPLICIT);
+
             $response->setConsent(Constants::CONSENT_PRIOR);
 
             $response->setDestination($response->getReturn());

src/OpenConext/EngineBlock/Authentication/Repository/ConsentRepository.php

@@ -38,9 +38,33 @@ public function deleteAllFor($userId);
 
     public function deleteOneFor(string $userId, string $serviceProviderEntityId): bool;
 
+    /**
+     * Test if the consent row is set with the legacy (unstable) consent hash
+     * This is the consent hash that was originally created by EB. It can change
+     * based on factors that should not result in a hash change per se. Think of the
+     * change of the attribute ordering, case change or the existence of empty
+     * attribute values.
+     */
     public function hasConsentHash(array $parameters): bool;
 
+    /**
+     * Tests the presence of the stable consent hash
+     *
+     * The stable consent hash is used by default, it is not affected by attribute order, case change
+     * or other irrelevant factors that could result in a changed hash calculation.
+     */
+    public function hasStableConsentHash(array $parameters): bool;
+
+    /**
+     * By default stores the stable consent hash. The legacy consent hash is left.
+     */
     public function storeConsentHash(array $parameters): bool;
 
+    /**
+     * When a deprecated unstable consent hash is encoutered, we upgrade it to the new format using this
+     * update consent hash method.
+     */
+    public function updateConsentHash(array $parameters): bool;
+
     public function countTotalConsent($consentUid): int;
 }

src/OpenConext/EngineBlock/Service/Consent/ConsentHashService.php

@@ -50,11 +50,21 @@ public function retrieveConsentHash(array $parameters): bool
         return $this->consentRepository->hasConsentHash($parameters);
     }
 
+    public function retrieveStableConsentHash(array $parameters): bool
+    {
+        return $this->consentRepository->hasStableConsentHash($parameters);
+    }
+
     public function storeConsentHash(array $parameters): bool
     {
         return $this->consentRepository->storeConsentHash($parameters);
     }
 
+    public function updateConsentHash(array $parameters): bool
+    {
+        return $this->consentRepository->updateConsentHash($parameters);
+    }
+
     public function countTotalConsent($consentUid): int
     {
         return $this->consentRepository->countTotalConsent($consentUid);

src/OpenConext/EngineBlock/Service/Consent/ConsentHashServiceInterface.php

@@ -20,10 +20,20 @@
 
 interface ConsentHashServiceInterface
 {
+    /**
+     * Retrieve the old-style (deprecated) unstable consent hash
+     */
     public function retrieveConsentHash(array $parameters): bool;
 
+    /**
+     * Retrieve the stable consent hash
+     */
+    public function retrieveStableConsentHash(array $parameters): bool;
+
     public function storeConsentHash(array $parameters): bool;
 
+    public function updateConsentHash(array $parameters): bool;
+
     public function countTotalConsent($consentUid): int;
 
     public function getUnstableAttributesHash(array $attributes, bool $mustStoreValues): string;

src/OpenConext/EngineBlockBundle/Authentication/Repository/DbalConsentRepository.php

@@ -200,15 +200,46 @@ public function hasConsentHash(array $parameters): bool
             throw new RuntimeException(sprintf('Consent retrieval failed! Error: "%s"', $e->getMessage()));
         }
     }
+    /**
+     * @throws RuntimeException
+     */
+    public function hasStableConsentHash(array $parameters): bool
+    {
+        try {
+            $query = " SELECT
+                            *
+                        FROM
+                            consent
+                        WHERE
+                            hashed_user_id = ?
+                        AND
+                            service_id = ?
+                        AND
+                            attribute_stable = ?
+                        AND
+                            consent_type = ?
+                        AND
+                            deleted_at IS NULL
+            ";
+
+            $statement = $this->connection->prepare($query);
+            $statement->execute($parameters);
+            $rows = $statement->fetchAll();
+
+            return count($rows) >= 1;
+        } catch (PDOException $e) {
+            throw new RuntimeException(sprintf('Consent retrieval on stable consent hash failed! Error: "%s"', $e->getMessage()));
+        }
+    }
 
     /**
      * @throws RuntimeException
      */
     public function storeConsentHash(array $parameters): bool
     {
-        $query = "INSERT INTO consent (hashed_user_id, service_id, attribute, consent_type, consent_date, deleted_at)
+        $query = "INSERT INTO consent (hashed_user_id, service_id, attribute_stable, consent_type, consent_date, deleted_at)
                   VALUES (?, ?, ?, ?, NOW(), '0000-00-00 00:00:00')
-                  ON DUPLICATE KEY UPDATE attribute=VALUES(attribute), consent_type=VALUES(consent_type), consent_date=NOW()";
+                  ON DUPLICATE KEY UPDATE attribute_stable=VALUES(attribute_stable), consent_type=VALUES(consent_type), consent_date=NOW()";
         $statement = $this->connection->prepare($query);
         if (!$statement) {
             throw new RuntimeException("Unable to create a prepared statement to insert consent?!");
@@ -223,6 +254,41 @@ public function storeConsentHash(array $parameters): bool
         return true;
     }
 
+    /**
+     * @throws RuntimeException
+     */
+    public function updateConsentHash(array $parameters): bool
+    {
+        $query = "
+                UPDATE
+                    consent
+                SET
+                    attribute_stable = ?
+                WHERE
+                    attribute = ?
+                AND
+                    hashed_user_id = ?
+                AND
+                    service_id = ?
+                AND
+                    consent_type = ?
+                AND
+                    deleted_at IS NULL
+        ";
+        $statement = $this->connection->prepare($query);
+        if (!$statement) {
+            throw new RuntimeException("Unable to create a prepared statement to update consent?!");
+        }
+
+        if (!$statement->execute($parameters)) {
+            throw new RuntimeException(
+                sprintf('Error storing updated consent: "%s"', var_export($statement->errorInfo(), true))
+            );
+        }
+
+        return true;
+    }
+
     /**
      * @throws RuntimeException
      */

tests/functional/OpenConext/EngineBlockBundle/Controller/Api/ConsentControllerTest.php

@@ -507,23 +507,31 @@ private function disableConsentApiFeatureFor(Client $client)
         $container->set('engineblock.features', $featureToggles);
     }
 
-    private function addConsentFixture($userId, $serviceId, $attributeHash, $consentType, $consentDate, $deletedAt)
-    {
+    private function addConsentFixture(
+        $userId,
+        $serviceId,
+        $attributeHash,
+        $consentType,
+        $consentDate,
+        $deletedAt
+    ) {
         $queryBuilder = $this->getContainer()->get('doctrine')->getConnection()->createQueryBuilder();
         $queryBuilder
             ->insert('consent')
             ->values([
                 'hashed_user_id' => ':user_id',
                 'service_id'     => ':service_id',
                 'attribute'      => ':attribute',
+                'attribute_stable' => ':attribute_stable',
                 'consent_type'   => ':consent_type',
                 'consent_date'   => ':consent_date',
                 'deleted_at'   => ':deleted_at',
             ])
             ->setParameters([
                 ':user_id'      => sha1($userId),
                 ':service_id'   => $serviceId,
-                ':attribute'    => $attributeHash,
+                ':attribute'    => '',
+                ':attribute_stable' => $attributeHash,
                 ':consent_type' => $consentType,
                 ':consent_date' => $consentDate,
                 ':deleted_at' => $deletedAt,

tests/functional/OpenConext/EngineBlockBundle/Controller/Api/DeprovisionControllerTest.php

@@ -397,6 +397,7 @@ private function addConsentFixture($userId, $serviceId, $attributeHash, $consent
                 'hashed_user_id' => ':user_id',
                 'service_id'     => ':service_id',
                 'attribute'      => ':attribute',
+                'attribute_stable' => ':attribute',
                 'consent_type'   => ':consent_type',
                 'consent_date'   => ':consent_date',
                 'deleted_at'   => '"0000-00-00 00:00:00"',