Merge pull request #78 from nayzo/refactor

Code refactoring + make doctrine/annotations dependency optional
nayzo · Feb 14, 2025 · b3eff65 · b3eff65
2 parents 3a8299a + 3d8e992
commit b3eff65
Show file tree

Hide file tree

Showing 17 changed files with 271 additions and 83 deletions.
diff --git a/.gitignore b/.gitignore
@@ -3,3 +3,5 @@
 /composer.lock
 /vendor
 /.idea
+/docker-compose.yml
+/Makefile
diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php
@@ -0,0 +1,33 @@
+<?php
+
+$finder = PhpCsFixer\Finder::create()
+    ->in(__DIR__)
+    ->exclude([
+        '.github/',
+        'vendor/',
+    ])
+;
+
+$config = (new PhpCsFixer\Config())
+    ->setRiskyAllowed(true)
+    ->setRules([
+        '@PSR12' => true,
+        '@Symfony' => true,
+        'array_syntax' => ['syntax' => 'short'],
+        'combine_consecutive_unsets' => true,
+        'heredoc_to_nowdoc' => true,
+        'no_extra_blank_lines' => ['tokens' => ['break', 'continue', 'extra', 'return', 'throw', 'use', 'parenthesis_brace_block', 'square_brace_block', 'curly_brace_block']],
+        'no_unreachable_default_argument_value' => true,
+        'no_useless_else' => true,
+        'no_useless_return' => true,
+        'ordered_class_elements' => true,
+        'ordered_imports' => true,
+        'php_unit_strict' => true,
+        'phpdoc_order' => true,
+        'strict_comparison' => true,
+        'strict_param' => true,
+        'concat_space' => ['spacing' => 'one'],
+    ])
+    ->setFinder($finder);
+
+return $config;
diff --git a/Annotations/AnnotationResolver.php b/Annotations/AnnotationResolver.php
@@ -11,21 +11,18 @@
 
 namespace Nzo\UrlEncryptorBundle\Annotations;
 
-use Doctrine\Common\Annotations\Reader;
+use Doctrine\Common\Annotations\AnnotationReader;
 use Nzo\UrlEncryptorBundle\Encryptor\Encryptor;
 use Symfony\Component\HttpKernel\Event\ControllerEvent;
 
 class AnnotationResolver
 {
-    private $decryptor;
+    private $encryptor;
     private $reader;
 
-    /**
-     * @param Reader|null $reader
-     */
-    public function __construct(Encryptor $decryptor, $reader)
+    public function __construct(Encryptor $encryptor, ?AnnotationReader $reader = null)
     {
-        $this->decryptor = $decryptor;
+        $this->encryptor = $encryptor;
         $this->reader = $reader;
     }
 
@@ -41,14 +38,16 @@ public function onKernelController(ControllerEvent $event)
             return;
         }
 
-        // handle php8 attribute
+        // Handle PHP8 Attributes
         if (class_exists('ReflectionAttribute')) {
-            if ($this->hasAnnotation($method) && !$this->reader instanceof Reader) {
+            if ($this->hasAnnotation($method) && !$this->reader instanceof AnnotationReader) {
                 throw new \InvalidArgumentException('NzoEncryptor:  Annotation service not loaded, PHP Attributes should be used instead.');
             }
             $annotations = $this->getAnnotation($method);
-        } else {
+        } elseif ($this->reader instanceof AnnotationReader) { // Handle Annotation only without Attributes
             $annotations = $this->reader->getMethodAnnotations($method);
+        } else {
+            throw new \InvalidArgumentException('NzoEncryptor: Doctrine Annotation package must be installed, doctrine/annotations.');
         }
 
         foreach ($annotations as $configuration) {
@@ -62,10 +61,10 @@ public function onKernelController(ControllerEvent $event)
                     $request = $event->getRequest();
                     foreach ($configuration->getParams() as $param) {
                         if ($request->attributes->has($param)) {
-                            $decrypted = $this->decryptor->encrypt($request->attributes->get($param));
+                            $decrypted = $this->encryptor->encrypt($request->attributes->get($param));
                             $request->attributes->set($param, $decrypted);
                         } elseif ($request->request->has($param)) {
-                            $decrypted = $this->decryptor->encrypt($request->request->get($param));
+                            $decrypted = $this->encryptor->encrypt($request->request->get($param));
                             $request->request->set($param, $decrypted);
                         }
                     }
@@ -75,10 +74,10 @@ public function onKernelController(ControllerEvent $event)
                     $request = $event->getRequest();
                     foreach ($configuration->getParams() as $param) {
                         if ($request->attributes->has($param)) {
-                            $decrypted = $this->decryptor->decrypt($request->attributes->get($param));
+                            $decrypted = $this->encryptor->decrypt($request->attributes->get($param));
                             $request->attributes->set($param, $decrypted);
                         } elseif ($request->request->has($param)) {
-                            $decrypted = $this->decryptor->decrypt($request->request->get($param));
+                            $decrypted = $this->encryptor->decrypt($request->request->get($param));
                             $request->request->set($param, $decrypted);
                         }
                     }
@@ -87,9 +86,6 @@ public function onKernelController(ControllerEvent $event)
         }
     }
 
-    /**
-     * @return mixed
-     */
     private function handleReflectionAttribute($configuration)
     {
         if ($configuration instanceof \ReflectionAttribute
@@ -109,7 +105,7 @@ private function handleReflectionAttribute($configuration)
      */
     private function getAnnotation($method)
     {
-        return $this->reader instanceof Reader && !empty($this->reader->getMethodAnnotations($method))
+        return $this->reader instanceof AnnotationReader && !empty($this->reader->getMethodAnnotations($method))
             ? $this->reader->getMethodAnnotations($method)
             : $method->getAttributes();
     }

diff --git a/Annotations/ParamDecryptor.php b/Annotations/ParamDecryptor.php
@@ -17,7 +17,7 @@
 #[\Attribute]
 class ParamDecryptor
 {
-    /** @var array  */
+    /** @var array */
     private $params;
 
     public function __construct(array $params = [])

diff --git a/Annotations/ParamEncryptor.php b/Annotations/ParamEncryptor.php
@@ -17,7 +17,7 @@
 #[\Attribute]
 class ParamEncryptor
 {
-    /** @var array  */
+    /** @var array */
     private $params;
 
     public function __construct(array $params = [])

diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -16,15 +16,12 @@
 
 class Configuration implements ConfigurationInterface
 {
-    /**
-     * {@inheritDoc}
-     */
     public function getConfigTreeBuilder(): TreeBuilder
     {
         $treeBuilder = new TreeBuilder('nzo_encryptor');
-        $rootNode = $treeBuilder->getRootNode();
 
-        $rootNode
+        $treeBuilder
+            ->getRootNode()
             ->children()
                 ->scalarNode('secret_key')
                     ->isRequired()

diff --git a/DependencyInjection/NzoEncryptorExtension.php b/DependencyInjection/NzoEncryptorExtension.php
@@ -13,8 +13,8 @@
 
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
-use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
 use Symfony\Component\DependencyInjection\Extension\Extension;
+use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
 
 class NzoEncryptorExtension extends Extension
 {
@@ -47,7 +47,7 @@ public function load(array $configs, ContainerBuilder $container): void
 
     private function cleanKey(?string $key = null): string
     {
-        if (empty($key)) {
+        if (null === $key || '' === $key || '0' === $key) {
             return '';
         }
 

diff --git a/README.md b/README.md
@@ -117,17 +117,21 @@ use Nzo\UrlEncryptorBundle\Annotations\ParamEncryptor;
 class MyController
 {
     /**
-    * @ParamDecryptor({"id", "foo"})   OR    #[ParamDecryptor(["id", "foo"])]
+    * @ParamDecryptor({"id", "foo"})
     */
+    // OR
+    #[ParamDecryptor(["id", "foo"])]
     public function decryptionAction($id, $foo)
     {
         // no need to use the decryption service here as the parameters are already decrypted by the annotation service.
         //...
     }
 
     /**
-    * @ParamEncryptor({"id", "foo"})   OR    #[ParamEncryptor(["id", "foo"])]
+    * @ParamEncryptor({"id", "foo"})
     */
+    // OR
+    #[ParamEncryptor(["id", "foo"])]
     public function encryptionAction($id, $foo)
     {
         // no need to use the encryption service here as the parameters are already encrypted by the annotation service.

diff --git a/Tests/Annotation/AnnotationResolverTest.php b/Tests/Annotation/AnnotationResolverTest.php
@@ -0,0 +1,83 @@
+<?php
+
+namespace Nzo\UrlEncryptorBundle\Tests\Annotation;
+
+use Doctrine\Common\Annotations\AnnotationReader;
+use Nzo\UrlEncryptorBundle\Annotations\AnnotationResolver;
+use Nzo\UrlEncryptorBundle\Encryptor\Encryptor;
+use Nzo\UrlEncryptorBundle\Tests\Annotation\Fixtures\DummyController;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpKernel\Event\ControllerEvent;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+
+class AnnotationResolverTest extends TestCase
+{
+    public function provideDecryptOnKernelController(): \Iterator
+    {
+        yield ['decryptWithAttribute', true];
+        yield ['decryptWithAnnotation', false];
+    }
+
+    /**
+     * @dataProvider provideDecryptOnKernelController
+     */
+    public function testDecryptOnKernelController(string $action, bool $needsPhp8): void
+    {
+        if ($needsPhp8 && PHP_VERSION_ID < 80000) {
+            $this->markTestSkipped('At least PHP 8 is needed for this test');
+        }
+
+        $encryptor = new Encryptor('foo', 'aes-256-ctr', true, true, true);
+        $encryptor->setSecretIv('secret');
+
+        $request = Request::create('/');
+        $request->attributes->set('id', $encryptor->encrypt('some_data'));
+
+        $controllerEvent = new ControllerEvent(
+            $this->createMock(HttpKernelInterface::class),
+            [new DummyController(), $action],
+            $request,
+            1
+        );
+
+        $sut = new AnnotationResolver($encryptor, new AnnotationReader());
+        $sut->onKernelController($controllerEvent);
+
+        $this->assertSame('some_data', $request->attributes->get('id'));
+    }
+
+    public function provideEncryptOnKernelController(): \Iterator
+    {
+        yield ['encryptWithAttribute', true];
+        yield ['encryptWithAnnotation', false];
+    }
+
+    /**
+     * @dataProvider provideEncryptOnKernelController
+     */
+    public function testEncryptOnKernelController(string $action, bool $needsPhp8): void
+    {
+        if ($needsPhp8 && PHP_VERSION_ID < 80000) {
+            $this->markTestSkipped('At least PHP 8 is needed for this test');
+        }
+
+        $encryptor = new Encryptor('foo', 'aes-256-ctr', true, true, true);
+        $encryptor->setSecretIv('secret');
+
+        $request = Request::create('/');
+        $request->attributes->set('id', 'some_data');
+
+        $controllerEvent = new ControllerEvent(
+            $this->createMock(HttpKernelInterface::class),
+            [new DummyController(), $action],
+            $request,
+            1
+        );
+
+        $sut = new AnnotationResolver($encryptor, new AnnotationReader());
+        $sut->onKernelController($controllerEvent);
+
+        $this->assertSame($encryptor->encrypt('some_data'), $request->attributes->get('id'));
+    }
+}
diff --git a/Tests/Annotation/Fixtures/DummyController.php b/Tests/Annotation/Fixtures/DummyController.php
@@ -0,0 +1,33 @@
+<?php
+
+namespace Nzo\UrlEncryptorBundle\Tests\Annotation\Fixtures;
+
+use Nzo\UrlEncryptorBundle\Annotations\ParamDecryptor;
+use Nzo\UrlEncryptorBundle\Annotations\ParamEncryptor;
+
+class DummyController
+{
+    #[ParamDecryptor(['id'])]
+    public function decryptWithAttribute()
+    {
+    }
+
+    /**
+     * @ParamDecryptor({"id"})
+     */
+    public function decryptWithAnnotation()
+    {
+    }
+
+    #[ParamEncryptor(['id'])]
+    public function encryptWithAttribute()
+    {
+    }
+
+    /**
+     * @ParamEncryptor({"id"})
+     */
+    public function encryptWithAnnotation()
+    {
+    }
+}
diff --git a/Tests/Encryptor/NzoEncryptorExtensionTest.php b/Tests/Encryptor/NzoEncryptorExtensionTest.php
@@ -16,13 +16,13 @@
 
 class NzoEncryptorExtensionTest extends TestCase
 {
-    const CIPHER_ALGORITHM = 'aes-256-ctr';
-    const PLAIN_TEXT = 'plain_text';
-    const SECRET_KEY = 'encryptionKeyText';
-    const SECRET_IV = 'encryptionIvText';
-    const BASE64_ENCODE = true;
-    const FORMAT_BASE64_OUTPUT = true;
-    const RANDOM_PSEUDO_BYTES = false;
+    public const CIPHER_ALGORITHM = 'aes-256-ctr';
+    public const PLAIN_TEXT = 'plain_text';
+    public const SECRET_KEY = 'encryptionKeyText';
+    public const SECRET_IV = 'encryptionIvText';
+    public const BASE64_ENCODE = true;
+    public const FORMAT_BASE64_OUTPUT = true;
+    public const RANDOM_PSEUDO_BYTES = false;
 
     private $encryptor;
 
@@ -44,6 +44,6 @@ public function testEncrypt()
         $encrypted = $this->encryptor->encrypt(self::PLAIN_TEXT);
         $decrypted = $this->encryptor->decrypt($encrypted);
 
-        $this->assertEquals(self::PLAIN_TEXT, $decrypted);
+        $this->assertSame(self::PLAIN_TEXT, $decrypted);
     }
 }
diff --git a/Tests/bootstrap.php b/Tests/bootstrap.php