Extract ConsentHashService from legacy consent model

Prior to this change, all consent hashing and persistence logic lived
inside the legacy Corto consent model (a God object). This commit
extracts a dedicated ConsentHashService that encapsulates both the
existing (unstable) hashing algorithm and a new stable hashing algorithm.

- Add ConsentHashService with both old and stable hashing methods
- Move DB consent queries to the existing DbalConsentRepository, applying
  naming conventions to query methods
- Extract ConsentHashServiceInterface to allow mocking in tests (the
  concrete class is final)
- Wire up service configuration

Pivotal ticket: https://www.pivotaltracker.com/story/show/176513931

Author: Koen Cornelis

config/services/compat.yml

@@ -50,7 +50,7 @@ services:
         class: EngineBlock_Corto_Model_Consent_Factory
         arguments:
             - "@engineblock.compat.corto_filter_command_factory"
-            - "@engineblock.compat.database_connection_factory"
+            - "@engineblock.service.consent.ConsentHashService"
 
     engineblock.compat.saml2_id_generator:
         public: true

config/services/services.yml

@@ -74,6 +74,10 @@ services:
             - '@OpenConext\EngineBlock\Metadata\LoaRepository'
             - '@logger'
 
+    engineblock.service.consent.ConsentHashService:
+        class: OpenConext\EngineBlock\Service\Consent\ConsentHashService
+        public: false
+
     OpenConext\EngineBlock\Service\ConsentService:
         arguments:
             - '@OpenConext\EngineBlockBundle\Authentication\Repository\DbalConsentRepository'

library/EngineBlock/Application/DiContainer.php

@@ -161,7 +161,7 @@ public function getAuthenticationLoopGuard()
     }
 
     /**
-     * @return OpenConext\EngineBlock\Service\ConsentService
+     * @return OpenConext\EngineBlock\Service\Consent\ConsentService
      */
     public function getConsentService()
     {

library/EngineBlock/Corto/Model/Consent.php

@@ -19,6 +19,7 @@
 use Doctrine\DBAL\Statement;
 use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
 use OpenConext\EngineBlock\Authentication\Value\ConsentType;
+use OpenConext\EngineBlock\Service\Consent\ConsentHashServiceInterface;
 
 class EngineBlock_Corto_Model_Consent
 {
@@ -37,15 +38,10 @@ class EngineBlock_Corto_Model_Consent
      */
     private $_response;
     /**
-     * @var array
+     * @var array All attributes as an associative array.
      */
     private $_responseAttributes;
 
-    /**
-     * @var EngineBlock_Database_ConnectionFactory
-     */
-    private $_databaseConnectionFactory;
-
     /**
      * A reflection of the eb.run_all_manipulations_prior_to_consent feature flag
      *
@@ -61,63 +57,59 @@ class EngineBlock_Corto_Model_Consent
     private $_consentEnabled;
 
     /**
-     * @param string $tableName
-     * @param bool $mustStoreValues
-     * @param EngineBlock_Saml2_ResponseAnnotationDecorator $response
-     * @param array $responseAttributes
-     * @param EngineBlock_Database_ConnectionFactory $databaseConnectionFactory
+     * @var ConsentHashServiceInterface
+     */
+    private $_hashService;
+
+    /**
      * @param bool $amPriorToConsentEnabled Is the run_all_manipulations_prior_to_consent feature enabled or not
-     * @param bool $consentEnabled Is the feature_enable_consent feature enabled or not
      */
     public function __construct(
-        $tableName,
-        $mustStoreValues,
+        string $tableName,
+        bool $mustStoreValues,
         EngineBlock_Saml2_ResponseAnnotationDecorator $response,
         array $responseAttributes,
-        EngineBlock_Database_ConnectionFactory $databaseConnectionFactory,
-        $amPriorToConsentEnabled,
-        $consentEnabled
-    )
-    {
+        bool $amPriorToConsentEnabled,
+        bool $consentEnabled,
+        ConsentHashServiceInterface $hashService
+    ) {
         $this->_tableName = $tableName;
         $this->_mustStoreValues = $mustStoreValues;
         $this->_response = $response;
         $this->_responseAttributes = $responseAttributes;
-        $this->_databaseConnectionFactory = $databaseConnectionFactory;
         $this->_amPriorToConsentEnabled = $amPriorToConsentEnabled;
+        $this->_hashService = $hashService;
         $this->_consentEnabled = $consentEnabled;
     }
 
-    public function explicitConsentWasGivenFor(ServiceProvider $serviceProvider)
+    public function explicitConsentWasGivenFor(ServiceProvider $serviceProvider): bool
     {
         return !$this->_consentEnabled ||
             $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_EXPLICIT);
     }
 
-    public function implicitConsentWasGivenFor(ServiceProvider $serviceProvider)
+    public function implicitConsentWasGivenFor(ServiceProvider $serviceProvider): bool
     {
         return !$this->_consentEnabled ||
             $this->_hasStoredConsent($serviceProvider, ConsentType::TYPE_IMPLICIT);
     }
 
-    public function giveExplicitConsentFor(ServiceProvider $serviceProvider)
+    public function giveExplicitConsentFor(ServiceProvider $serviceProvider): bool
     {
         return !$this->_consentEnabled ||
             $this->_storeConsent($serviceProvider, ConsentType::TYPE_EXPLICIT);
     }
 
-    public function giveImplicitConsentFor(ServiceProvider $serviceProvider)
+    public function giveImplicitConsentFor(ServiceProvider $serviceProvider): bool
     {
         return !$this->_consentEnabled ||
             $this->_storeConsent($serviceProvider, ConsentType::TYPE_IMPLICIT);
     }
 
-    /**
-     * @return Doctrine\DBAL\Connection
-     */
-    protected function _getConsentDatabaseConnection()
+    public function countTotalConsent(): int
     {
-        return $this->_databaseConnectionFactory->create();
+        $consentUid = $this->_getConsentUid();
+        return $this->_hashService->countTotalConsent($consentUid);
     }
 
     protected function _getConsentUid()
@@ -129,116 +121,55 @@ protected function _getConsentUid()
         return $this->_response->getNameIdValue();
     }
 
-    protected function _getAttributesHash($attributes)
+    protected function _getAttributesHash($attributes): string
     {
-        $hashBase = NULL;
-        if ($this->_mustStoreValues) {
-            ksort($attributes);
-            $hashBase = serialize($attributes);
-        } else {
-            $names = array_keys($attributes);
-            sort($names);
-            $hashBase = implode('|', $names);
-        }
-        return sha1($hashBase);
+        return $this->_hashService->getUnstableAttributesHash($attributes, $this->_mustStoreValues);
     }
 
-    private function _storeConsent(ServiceProvider $serviceProvider, $consentType)
+    protected function _getStableAttributesHash($attributes): string
     {
-        $dbh = $this->_getConsentDatabaseConnection();
-        if (!$dbh) {
-            return false;
-        }
+        return $this->_hashService->getStableAttributesHash($attributes, $this->_mustStoreValues);
+    }
 
+    private function _storeConsent(ServiceProvider $serviceProvider, $consentType): bool
+    {
         $consentUuid = $this->_getConsentUid();
-        if(! is_string($consentUuid)){
+        if (!is_string($consentUuid)) {
             return false;
         }
 
-        $query = "INSERT INTO consent (hashed_user_id, service_id, attribute, 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()";
         $parameters = array(
             sha1($consentUuid),
             $serviceProvider->entityId,
-            $this->_getAttributesHash($this->_responseAttributes),
+            $this->_getStableAttributesHash($this->_responseAttributes),
             $consentType,
         );
 
-        $statement = $dbh->prepare($query);
-        if (!$statement) {
-            throw new EngineBlock_Exception(
-                "Unable to create a prepared statement to insert consent?!",
-                EngineBlock_Exception::CODE_CRITICAL
-            );
-        }
-
-        assert($statement instanceof Statement);
-        try{
-            foreach ($parameters as $index => $parameter){
-                $statement->bindValue($index + 1, $parameter);
-            }
-
-            $statement->executeStatement();
-        }catch (\Doctrine\DBAL\Exception $e){
-            throw new EngineBlock_Corto_Module_Services_Exception(
-                sprintf('Error storing consent: "%s"', var_export($e->getMessage(), true)),
-                EngineBlock_Exception::CODE_CRITICAL
-            );
-        }
-        return true;
+        return $this->_hashService->storeConsentHash($parameters);
     }
 
-    private function _hasStoredConsent(ServiceProvider $serviceProvider, $consentType)
+    private function _hasStoredConsent(ServiceProvider $serviceProvider, $consentType): bool
     {
-        try {
-            $dbh = $this->_getConsentDatabaseConnection();
-            if (!$dbh) {
-                return false;
-            }
-
-            $attributesHash = $this->_getAttributesHash($this->_responseAttributes);
-
-            $consentUuid = $this->_getConsentUid();
-            if (!is_string($consentUuid)) {
-                return false;
-            }
-
-            $query = "
-                SELECT *
-                FROM {$this->_tableName}
-                WHERE hashed_user_id = ?
-                  AND service_id = ?
-                  AND attribute = ?
-                  AND consent_type = ?
-                  AND deleted_at IS NULL
-            ";
-            $hashedUserId = sha1($consentUuid);
-            $parameters = array(
-                $hashedUserId,
-                $serviceProvider->entityId,
-                $attributesHash,
-                $consentType,
-            );
-
-            $statement = $dbh->prepare($query);
-            assert($statement instanceof Statement);
-            foreach ($parameters as $position => $parameter) {
-                $statement->bindValue($position + 1, $parameter);
-            }
-            $rows = $statement->executeQuery();
-
-            if ($rows->rowCount() < 1) {
-                // No stored consent found
-                return false;
-            }
+        $parameters = array(
+            sha1($this->_getConsentUid()),
+            $serviceProvider->entityId,
+            $this->_getAttributesHash($this->_responseAttributes),
+            $consentType,
+        );
+
+        $hasUnstableConsentHash = $this->_hashService->retrieveConsentHash($parameters);
 
+        if ($hasUnstableConsentHash) {
             return true;
-        } catch (PDOException $e) {
-            throw new EngineBlock_Corto_ProxyServer_Exception(
-                sprintf('Consent retrieval failed! Error: "%s"', $e->getMessage()),
-                EngineBlock_Exception::CODE_ALERT
-            );
         }
+
+        $parameters[2] = array(
+            sha1($this->_getConsentUid()),
+            $serviceProvider->entityId,
+            $this->_getStableAttributesHash($this->_responseAttributes),
+            $consentType,
+        );
+
+        return $this->_hashService->retrieveConsentHash($parameters);
     }
 }

library/EngineBlock/Corto/Model/Consent/Factory.php

@@ -16,6 +16,8 @@
  * limitations under the License.
  */
 
+use OpenConext\EngineBlock\Service\Consent\ConsentHashService;
+
 /**
  * @todo write a test
  */
@@ -24,21 +26,20 @@ class EngineBlock_Corto_Model_Consent_Factory
     /** @var EngineBlock_Corto_Filter_Command_Factory */
     private $_filterCommandFactory;
 
-    /** @var EngineBlock_Database_ConnectionFactory */
-    private $_databaseConnectionFactory;
-
+    /**
+     * @var ConsentHashService
+     */
+    private $_hashService;
 
-     /**
+    /**
       * @param EngineBlock_Corto_Filter_Command_Factory $filterCommandFactory
-      * @param EngineBlock_Database_ConnectionFactory $databaseConnectionFactory
       */
     public function __construct(
         EngineBlock_Corto_Filter_Command_Factory $filterCommandFactory,
-        EngineBlock_Database_ConnectionFactory $databaseConnectionFactory
-    )
-    {
+        ConsentHashService $hashService
+    ) {
         $this->_filterCommandFactory = $filterCommandFactory;
-        $this->_databaseConnectionFactory = $databaseConnectionFactory;
+        $this->_hashService = $hashService;
     }
 
     /**
@@ -68,9 +69,9 @@ public function create(
             $proxyServer->getConfig('ConsentStoreValues', true),
             $response,
             $attributes,
-            $this->_databaseConnectionFactory,
             $amPriorToConsent,
-            $consentEnabled
+            $consentEnabled,
+            $this->_hashService
         );
     }
 }

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

@@ -19,7 +19,7 @@
 use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider;
 use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
 use OpenConext\EngineBlock\Service\AuthenticationStateHelperInterface;
-use OpenConext\EngineBlock\Service\ConsentServiceInterface;
+use OpenConext\EngineBlock\Service\Consent\ConsentServiceInterface;
 use OpenConext\EngineBlock\Service\ProcessingStateHelperInterface;
 use OpenConext\EngineBlockBundle\Service\DiscoverySelectionService;
 use Psr\Log\LogLevel;

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

@@ -37,4 +37,10 @@ public function findAllFor($userId);
     public function deleteAllFor($userId);
 
     public function deleteOneFor(string $userId, string $serviceProviderEntityId): bool;
+
+    public function hasConsentHash(array $parameters): bool;
+
+    public function storeConsentHash(array $parameters): bool;
+
+    public function countTotalConsent($consentUid): int;
 }