Added logging error if missing the encrypt_key in parameters file.

Throw an exception if missing encrypt_key and attempting to encrypt/decrypt. Added OpenSslEncryptorTest.
mogilvie · Jun 22, 2019 · 07e1968 · 07e1968
1 parent 03dc4bc
commit 07e1968
Show file tree

Hide file tree

Showing 8 changed files with 125 additions and 79 deletions.
diff --git a/DependencyInjection/SpecShaperEncryptExtension.php b/DependencyInjection/SpecShaperEncryptExtension.php
@@ -6,21 +6,15 @@
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
 use Symfony\Component\DependencyInjection\Loader;
-use Psr\Log\LoggerAwareInterface;
 
 /**
  * This is the class that loads and manages your bundle configuration.
  *
  * @link http://symfony.com/doc/current/cookbook/bundles/extension.html
  */
-class SpecShaperEncryptExtension extends Extension implements LoggerAwareInterface
+class SpecShaperEncryptExtension extends Extension
 {
-    private $logger;
-
-    public functoin setLogger($logger){
-        $this->logger = $logger;
-    }
-
+
     /**
      * {@inheritdoc}
      */
@@ -32,15 +26,13 @@ public function load(array $configs, ContainerBuilder $container)
         $loader = new Loader\YamlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
         $loader->load('services.yml');
 
-        $encryptKey = "NotDefined";
+        $encryptKey = null;
 
         if($container->hasParameter('encrypt_key')){
-            $encryptKey = $container->getParameter('encrypt_key);
-        } else {
-            $this->logger->error('Parameters.yml file had no encrypt_key parameter');            
+            $encryptKey = $container->getParameter('encrypt_key');
         }
 
-        $container->setParameter($this->getAlias() . '.encrypt_key', $config['annotation_classes']);
+        $container->setParameter($this->getAlias() . '.encrypt_key',$encryptKey);
         $container->setParameter($this->getAlias() . '.method', $config['method']);
         $container->setParameter($this->getAlias() . '.subscriber_class', $config['subscriber_class']);
         $container->setParameter($this->getAlias() . '.annotation_classes', $config['annotation_classes']);

diff --git a/Encryptors/EncryptorFactory.php b/Encryptors/EncryptorFactory.php
@@ -8,13 +8,21 @@
 
 namespace SpecShaper\EncryptBundle\Encryptors;
 
+use Psr\Log\LoggerInterface;
 
 class EncryptorFactory
 {
     const SUPPORTED_EXTENSIONS = [
         OpenSslEncryptor::class
     ];
 
+    private $logger;
+
+    public function __construct(LoggerInterface $logger)
+    {
+        $this->logger = $logger;
+    }
+
     /**
      * @param $method
      * @param $encryptKey
@@ -23,6 +31,12 @@ class EncryptorFactory
     public function createService($encryptor, $encryptKey)
     {
 
+        // Log an error if there is no value set for the encrypt_key.
+        if($encryptKey === null){
+            $this->logger->error('The bundle specshaper\encrypt-bundle requires a parameter.yml value for "encrypt_key". 
+            Use cli command "php bin/console encrypt:genkey" to create a key.');
+        }
+
         switch($encryptor){
             default:
                 $encryptor = new OpenSslEncryptor($encryptKey);

diff --git a/Encryptors/OpenSslEncryptor.php b/Encryptors/OpenSslEncryptor.php
@@ -42,14 +42,19 @@ public function __toString()
     public function encrypt($data)
     {
         // If not data return data (null)
-        if (is_null($data) || is_object($data)) {
+        if (is_null($data)) {
             return $data;
         }
 
         if (is_object($data)) {
             throw new EncryptException('You cannot encrypt an object.',  $data);
         }
 
+        // If the value already has the suffix <ENC> then ignore.
+        if(substr($data, -5) === DoctrineEncryptSubscriberInterface::ENCRYPTED_SUFFIX) {
+            return $data;
+        }
+
         $key = $this->getSecretKey();
 
         // Create a cipher of the appropriate length for this method.
@@ -65,8 +70,8 @@ public function encrypt($data)
             $iv
         );
 
-        // Prefix the encoded text with the iv and encode it to base 64.
-        $encoded = base64_encode($iv . $ciphertext);
+        // Prefix the encoded text with the iv and encode it to base 64. Append the encoded suffix.
+        $encoded = base64_encode($iv . $ciphertext) . DoctrineEncryptSubscriberInterface::ENCRYPTED_SUFFIX;
 
         return $encoded;
     }
@@ -80,17 +85,26 @@ public function decrypt($data)
     {
 
         // If the value is an object or null then ignore
-        if($data === null || is_object($data)) {
+        if($data === null) {
             return $data;
         }
 
+        if (is_object($data)) {
+            throw new EncryptException('You cannot decrypt an object.',  $data);
+        }
+
         // If the value does not have the suffix <ENC> then ignore.
         if(substr($data, -5) !== DoctrineEncryptSubscriberInterface::ENCRYPTED_SUFFIX) {
             return $data;
         }
 
         $data = substr($data, 0,-5);
 
+        // If the data is just <ENC> the return null;
+        if(empty($data)) {
+            return $data;
+        }
+
         $key = $this->getSecretKey();
 
         $data = base64_decode($data);
@@ -99,13 +113,15 @@ public function decrypt($data)
         $iv = mb_substr($data, 0, $ivsize, '8bit');
         $ciphertext = mb_substr($data, $ivsize, null, '8bit');
 
-        return openssl_decrypt(
+        $decrypted = openssl_decrypt(
             $ciphertext,
             self::METHOD,
             $key,
             OPENSSL_RAW_DATA,
             $iv
         );
+
+        return $decrypted;
     }
 
     /**
@@ -119,6 +135,11 @@ public function decrypt($data)
      */
     private function getSecretKey(){
 
+        if($this->secretKey === null){
+            throw new EncryptException('The bundle specshaper\encrypt-bundle requires a parameter.yml value for "encrypt_key"
+            Use cli command "php bin/console encrypt:genkey" to create a key.');
+        }
+
         // Decode the key
         $key = base64_decode($this->secretKey);
 

diff --git a/README.md b/README.md
@@ -4,13 +4,13 @@ A bundle to handle encoding and decoding of parameters using OpenSSL and Doctrin
 
 Features include:
 
-- Written for Symfony verison 3.x.x
+- Written for Symfony version 3.4
 - Uses OpenSSL
 - Uses Lifecycle events
 
 **Warning**
 - This bundle has not been unit tested.
-- It has only been running on a Symfony2 v3.0.1 project, and not backward
+- It has only been running on a Symfony 3.4 project, and not backward
 compatibility tested.
 
 Features road map:
@@ -29,8 +29,8 @@ This bundle is under the MIT license. See the complete license in the bundle:
 
 ## About
 
-EncryptBundle has been written for the [SpecShaper](http://about.specshaper.com) and [Parolla](http://parolla.ie) websites
-to encode users private data. The bundle will be expanded as part of a larger EU GDPR data management bundle.
+EncryptBundle has been written for the [SpecShaper](http://about.specshaper.com) and [Parolla](https://www.parolla.ie) websites
+to encode users private data. The bundle is expanded in a larger [gdpr-bundle](https://github.com/mogilvie/GdprBundle).
 
 ## Reporting an issue or a feature request
 
@@ -115,7 +115,7 @@ can be configured to extend the bundle.
 You can disable encryption by setting the 'is_disabled' option to true. Decryption still continues if any values
 contain the \<ENC> suffix.
 
-You can extend the EnryptBundle default Subscriber and override its methods. Use the 'subscriber_class' option
+You can extend the EncryptBundle default Subscriber and override its methods. Use the 'subscriber_class' option
 to point the bundle at your custom subscriber.
 
 If you want to define your own annotation, then this can be used to trigger encryption by adding the annotation 
@@ -166,7 +166,7 @@ Add the annotation '@Encrypted' to the parameters that you want encrypted.
 ```
 Where encrypting a field you will need to set the column type as string.  
 
-Your getters and setters may also need to be type delcared.  
+Your getters and setters may also need to be type declared.  
 
 For example, boolean should either be return declared bool, or return a bool using a ternary method.  
 

diff --git a/Resources/config/services.yml b/Resources/config/services.yml
@@ -27,7 +27,10 @@ services:
             - { name: kernel.event_subscriber, connection: default }
 
     # Factory to create the encryptor/decryptor
-    SpecShaper\EncryptBundle\Encryptors\EncryptorFactory: ~
+    SpecShaper\EncryptBundle\Encryptors\EncryptorFactory:
+        arguments: ['@logger']
+        tags:
+            - { name: monolog.logger, channel: app }
 
     # The encryptor service created by the factory according to the passed method and using the encrypt_key
     SpecShaper\EncryptBundle\Encryptors\EncryptorService:

diff --git a/Subscribers/DoctrineEncryptSubscriber.php b/Subscribers/DoctrineEncryptSubscriber.php
@@ -78,7 +78,6 @@ public function __construct(Reader $annReader, EncryptorInterface $encryptor, $a
         $this->encryptor = $encryptor;
         $this->annotationArray = $annotationArray;
         $this->isDisabled = $isDisabled;
-
     }
 
 
@@ -128,6 +127,7 @@ public function getSubscribedEvents()
      * every time (Because it is going to differ from the un-encrypted value)
      *
      * @param OnFlushEventArgs $args
+     * @throws EncryptException
      */
     public function onFlush(OnFlushEventArgs $args)
     {
@@ -151,12 +151,12 @@ public function onFlush(OnFlushEventArgs $args)
         }
     }
 
-
     /**
      * Processes the entity for an onFlush event.
      *
-     * @param object $entity
+     * @param               $entity
      * @param EntityManager $em
+     * @throws EncryptException
      */
     protected function entityOnFlush($entity, EntityManager $em)
     {
@@ -217,7 +217,9 @@ public function postFlush(PostFlushEventArgs $args)
     /**
      * Listen a postLoad lifecycle event. Checking and decrypt entities
      * which have @Encrypted annotations
+     *
      * @param LifecycleEventArgs $args
+     * @throws EncryptException
      */
     public function postLoad(LifecycleEventArgs $args)
     {
@@ -247,7 +249,6 @@ public function decryptValue($value){
         $decrypted = $this->encryptor->decrypt($value);
 
         return $decrypted;
-
     }
 
     /**
@@ -268,7 +269,6 @@ public function getEncryptionableProperties($allProperties)
                     $encryptedFields[] = $refProperty;
                 }
             }
-
         }
 
         return $encryptedFields;
@@ -277,11 +277,11 @@ public function getEncryptionableProperties($allProperties)
     /**
      * Process (encrypt/decrypt) entities fields
      *
-     * @param                             $entity
-     * @param \Doctrine\ORM\EntityManager $em
-     * @param bool                        $isEncryptOperation
-     *
+     * @param               $entity
+     * @param EntityManager $em
+     * @param bool          $isEncryptOperation
      * @return bool
+     * @throws EncryptException
      */
     protected function processFields($entity, EntityManager $em, $isEncryptOperation = true)
     {
@@ -292,6 +292,7 @@ protected function processFields($entity, EntityManager $em, $isEncryptOperation
         $oid = spl_object_hash($entity);
 
         foreach ($properties as $refProperty) {
+
             $value = $refProperty->getValue($entity);
 
             // Skip any empty values.
@@ -303,17 +304,13 @@ protected function processFields($entity, EntityManager $em, $isEncryptOperation
                 throw new EncryptException('You cannot encrypt an object at ' . $refProperty->class .':'.  $refProperty->getName() , $value);
             }
 
-            // If the required opteration is to encrypt then encrypt the value.
+            // If the required operation is to encrypt then encrypt the value.
             if($isEncryptOperation) {
-                $encrypted = $this->encryptor->encrypt($value);
-                $value =  $encrypted . DoctrineEncryptSubscriberInterface::ENCRYPTED_SUFFIX ;
+                $encryptedValue = $this->encryptor->encrypt($value);
+                $refProperty->setValue($entity, $encryptedValue);
             } else {
-                $value = $this->decryptValue($value);
-            }
-
-            $refProperty->setValue($entity, $value);
-
-            if (!$isEncryptOperation) {
+                $decryptedValue = $this->decryptValue($value);
+                $refProperty->setValue($entity, $decryptedValue);
                 //we don't want the object to be dirty immediately after reading
                 $unitOfWork->setOriginalEntityProperty($oid, $refProperty->getName(), $value);
             }
@@ -378,6 +375,4 @@ protected function getEncryptedFields($entity, EntityManager $em)
 
         return $encryptedFields;
     }
-
-
 }
diff --git a/composer.json b/composer.json
@@ -18,10 +18,12 @@
         "symfony/console": "^3.2 || ^4.0",
         "symfony/framework-bundle": "^3.2 || ^4.0",
         "doctrine/orm": "^2.5",
-        "doctrine/doctrine-bundle": "^1.6"
+        "doctrine/doctrine-bundle": "^1.6",
+        "symfony/monolog-bundle": "^3.4"
     },
     "require-dev": {
         "phpunit/phpunit": "^7"
+
     },
     "autoload" : {
         "psr-4": { "SpecShaper\\EncryptBundle\\": "" }