Deprecate Doctrine annotations

nepada · May 14, 2022 · 6737cd0 · 6737cd0
1 parent 88264db
commit 6737cd0
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 122 deletions.
diff --git a/README.md b/README.md
@@ -23,7 +23,9 @@ extensions:
     securityAnnotations: Nepada\Bridges\SecurityAnnotationsDI\SecurityAnnotationsExtension
 ```
 
-The package relies on [doctrine/annotations](https://packagist.org/packages/doctrine/annotations) for parsing annotations. It's up to you to choose and set up the integration, the recommended package to do the job is [nettrine/annotations](https://packagist.org/packages/nettrine/annotations).
+For parsing phpdoc annotation this package relies on [doctrine/annotations](https://packagist.org/packages/doctrine/annotations) for parsing annotations. It's up to you to choose and set up the integration, the recommended package to do the job is [nettrine/annotations](https://packagist.org/packages/nettrine/annotations).
+
+**Note: annotations driver is deprecated and will be removed in next major release. Migrate all your annotations to native PHP8 attributes and set `enableDoctrineAnnotations: false` in your config.** 
 
 
 Usage
@@ -32,7 +34,7 @@ Usage
 This package builds on top of the standard access authorization of Nette components, namely `Nette\Application\UI\Component::checkRequirements()` method.
 This method is called before invoking any of component/presenter signal handlers, and before presenter `startup`, `action<>` and `render<>` methods.
 
-With this package you can specify the access rules via annotations on any of the mentioned methods, or on presenter class.
+With this package you can specify the access rules via attributes on any of the mentioned methods, or on presenter class.
 To enable this feature simple use `SecurityAnnotations` trait in any presenter or component and make sure `RequirementsChecker` service gets injected via `injectRequirementsChecker()` - with default Nette configuration this should work on presenters out of the box, but you need to take care of components, e.g. by enabling inject calls.
 
 **Example:**
@@ -42,25 +44,21 @@ use Nepada\SecurityAnnotations\Annotations\LoggedIn;
 use Nepada\SecurityAnnotations\Annotations\Role;
 
 /**
- * @LoggedIn
  * To access this presenter the user must be logged in.
  */
+ #[LoggedIn]
 class SecuredPresenter extends Nette\Application\UI\Presenter
 {
 
     use Nepada\SecurityAnnotations\SecurityAnnotations;
 
-    /**
-     * @Role({"admin", "superadmin"})
-     */
+    #[Role(["admin", "superadmin"])]
     public function actionForAdmins(): void
     {
         // Only users with role admin or superadmin are allowed here.
     }
 
-    /**
-     * @Allowed(resource="world", privilege="destroy")
-     */
+     #[Allowed(resource: "world", privilege: "destroy")]
     public function handleDestroyWorld(): void
     {
         // Only users with specific permission are allowed to call this signal.
@@ -69,12 +67,12 @@ class SecuredPresenter extends Nette\Application\UI\Presenter
 }
 ```
 
-The annotations and rules they enforce are completely customizable (see below), however the default setup comes with three predefined rules:
+The attributes and rules they enforce are completely customizable (see below), however the default setup comes with three predefined rules:
 
-- **@LoggedIn** - checks whether the user is logged in.
-- **@Role({"admin", "superadmin"})** - checks whether the user has at least one of the specified roles.
+- **LoggedIn** - checks whether the user is logged in.
+- **Role(["admin", "superadmin"])** - checks whether the user has at least one of the specified roles.
   If you use `Nette\Security\Permission` as your authorizator, then role inheritance is taken into account, i.e. users that have at least one role that inherits from at least one of the specified roles are allowed as well.
-- **@Allowed(resource="world", privilege="destroy")** - checks whether the user has at least one role that is granted the specified privilege on the specified resource.
+- **Allowed(resource: "world", privilege: "destroy")** - checks whether the user has at least one role that is granted the specified privilege on the specified resource.
 
 
 ### Securing components
@@ -89,9 +87,7 @@ class SecuredPresenter extends Nette\Application\UI\Presenter
 
     use Nepada\SecurityAnnotations\SecurityAnnotations;
 
-    /**
-     * @LoggedIn
-     */
+    #[LoggedIn]
     public function actionDefault(): void
     {
         // ...
@@ -112,7 +108,7 @@ class SecuredPresenter extends Nette\Application\UI\Presenter
 
 Securing presenter `action<>` (or `render<>`) methods is not sufficient! All it takes is a one general route in your router, e.g. a very common `Route('<presenter>/<action>')`, and anyone may successfully submit the form by sending POST request to `/secured/foo` URL.
 
-You should always check user's permissions when creating the component. To make your life easier there is `SecuredComponents` trait that calls the standard `Nette\Application\UI\Component::checkRequirements()` method before calling the component factory. Combining it with `SecurityAnnotations` it allows you to control access to components via annotations on `createComponent<>` methods.
+You should always check user's permissions when creating the component. To make your life easier there is `SecuredComponents` trait that calls the standard `Nette\Application\UI\Component::checkRequirements()` method before calling the component factory. Combining it with `SecurityAnnotations` it allows you to control access to components via attributes on `createComponent<>` methods.
 
 
 ### Customizing access validators
@@ -133,28 +129,6 @@ services:
 
 #### How do access validators work?
 
-Every access validator implements `Nepada\SecurityAnnotations\AccessValidators\AccessValidator` interface. The access validator specifies which annotation type it supports via its public API.
-
-When checking the requirements PHP attributes and all annotations parsed using `doctrine/annotations` are passed one by one to associated access validator for inspection. Based on the annotation value the validator decides either to deny access (throws `Nette\Application\BadRequestException`), or grant access (no exception is thrown).
-
-
-### PHP attributes support
-
-Security annotations may be defined also via standard PHP attributes (introduced in PHP 8.0), e.g.
-```php
-use Nepada\SecurityAnnotations\Annotations\LoggedIn;
-
-#[LoggedIn()]
-class SecuredPresenter extends Nette\Application\UI\Presenter
-{
-    // ...
-}
-```
+Every access validator implements `Nepada\SecurityAnnotations\AccessValidators\AccessValidator` interface. The access validator specifies which attribute type it supports via its public API.
 
-PHP attributes are the preferred way of specifying your security metadata. The support for legacy PHP DocBlock annotations will be removed in the future major version. 
-
-If you do not use legacy PHP DocBlock annotations, consider completely disabling doctrine annotations reader:
-```yaml
-securityAnnotations:
-    enableDoctrineAnnotations: false
-```
+When checking the requirements PHP attributes and all annotations parsed using `doctrine/annotations` are passed one by one to associated access validator for inspection. Based on the attribute value the validator decides either to deny access (throws `Nette\Application\BadRequestException`), or grant access (no exception is thrown).
diff --git a/src/Bridges/SecurityAnnotationsDI/SecurityAnnotationsExtension.php b/src/Bridges/SecurityAnnotationsDI/SecurityAnnotationsExtension.php
@@ -47,6 +47,10 @@ public function loadConfiguration(): void
             ->setType(AttributesReader::class)
             ->setAutowired(AttributesReader::class);
         if ($this->config->enableDoctrineAnnotations) {
+            trigger_error(
+                'Using Doctrine annotations is deprecated, migrate to native PHP8 attributes and set enableDoctrineAnnotations: false in your config',
+                E_USER_DEPRECATED,
+            );
             $readers[] = $container->addDefinition($this->prefix('doctrineAnnotationsReader'), new ServiceDefinition())
                 ->setType(DoctrineAnnotationsReader::class)
                 ->setAutowired(DoctrineAnnotationsReader::class);

diff --git a/tests/Bridges/SecurityAnnotationsDI/Fixtures/config.doctrine-annotations-enabled.neon b/tests/Bridges/SecurityAnnotationsDI/Fixtures/config.doctrine-annotations-enabled.neon
@@ -0,0 +1,2 @@
+securityAnnotations:
+    enableDoctrineAnnotations: true
diff --git a/tests/Bridges/SecurityAnnotationsDI/Fixtures/config.neon b/tests/Bridges/SecurityAnnotationsDI/Fixtures/config.neon
@@ -16,3 +16,6 @@ security:
 
 services:
     - Doctrine\Common\Cache\ArrayCache
+
+securityAnnotations:
+    enableDoctrineAnnotations: false
diff --git a/tests/Bridges/SecurityAnnotationsDI/SecurityAnnotationsExtensionTest.phpt b/tests/Bridges/SecurityAnnotationsDI/SecurityAnnotationsExtensionTest.phpt
@@ -99,12 +99,14 @@ class SecurityAnnotationsExtensionTest extends TestCase
 
     public function testDefaultReader(): void
     {
+        $this->configurator->addConfig(__DIR__ . '/Fixtures/config.doctrine-annotations-enabled.neon');
+
         $expected = [
             new SecurityAnnotations\Annotations\Role('attribute'),
             new SecurityAnnotations\Annotations\Role('annotation'),
         ];
         /** @var SecurityAnnotations\AnnotationReaders\AnnotationsReader $reader */
-        $reader = $this->configurator->createContainer()->getByType(SecurityAnnotations\AnnotationReaders\AnnotationsReader::class);
+        $reader = @$this->configurator->createContainer()->getByType(SecurityAnnotations\AnnotationReaders\AnnotationsReader::class);
         Assert::equal($expected, $reader->getAll(new \ReflectionClass(LoremIpsum::class)));
     }
 

diff --git a/tests/SecurityAnnotations/AccessValidators/LoggedInValidatorTest.phpt b/tests/SecurityAnnotations/AccessValidators/LoggedInValidatorTest.phpt
@@ -3,8 +3,6 @@ declare(strict_types = 1);
 
 namespace NepadaTests\SecurityAnnotations\AccessValidators;
 
-use Doctrine\Common\Annotations\AnnotationRegistry;
-use Doctrine\Common\Annotations\DocParser;
 use Mockery;
 use Mockery\MockInterface;
 use Nepada\SecurityAnnotations\AccessValidators;
@@ -24,19 +22,15 @@ require_once __DIR__ . '/../../bootstrap.php';
 class LoggedInValidatorTest extends TestCase
 {
 
-    private DocParser $docParser;
-
     protected function setUp(): void
     {
         parent::setUp();
         Environment::bypassFinals();
-        AnnotationRegistry::registerUniqueLoader('class_exists');
-        $this->docParser = new DocParser();
     }
 
     public function testAccessAllowed(): void
     {
-        $annotation = $this->parseAnnotation('@Nepada\SecurityAnnotations\Annotations\LoggedIn');
+        $annotation = new LoggedIn();
         $user = $this->mockUser(true);
         $validator = new AccessValidators\LoggedInValidator($user);
 
@@ -47,7 +41,7 @@ class LoggedInValidatorTest extends TestCase
 
     public function testAccessDenied(): void
     {
-        $annotation = $this->parseAnnotation('@Nepada\SecurityAnnotations\Annotations\LoggedIn');
+        $annotation = new LoggedIn();
         $user = $this->mockUser(false);
         $validator = new AccessValidators\LoggedInValidator($user);
 
@@ -68,14 +62,6 @@ class LoggedInValidatorTest extends TestCase
         return $user;
     }
 
-    private function parseAnnotation(string $input): LoggedIn
-    {
-        $annotations = $this->docParser->parse($input);
-        Assert::count(1, $annotations);
-        Assert::type(LoggedIn::class, $annotations[0]);
-        return $annotations[0];
-    }
-
 }
 
 

diff --git a/tests/SecurityAnnotations/AccessValidators/PermissionValidatorTest.phpt b/tests/SecurityAnnotations/AccessValidators/PermissionValidatorTest.phpt
@@ -3,8 +3,6 @@ declare(strict_types = 1);
 
 namespace NepadaTests\SecurityAnnotations\AccessValidators;
 
-use Doctrine\Common\Annotations\AnnotationRegistry;
-use Doctrine\Common\Annotations\DocParser;
 use Mockery;
 use Mockery\MockInterface;
 use Nepada\SecurityAnnotations\AccessValidators;
@@ -24,24 +22,14 @@ require_once __DIR__ . '/../../bootstrap.php';
 class PermissionValidatorTest extends TestCase
 {
 
-    private DocParser $docParser;
-
-    protected function setUp(): void
-    {
-        parent::setUp();
-        AnnotationRegistry::registerUniqueLoader('class_exists');
-        $this->docParser = new DocParser();
-    }
-
     /**
      * @dataProvider getDataForAccessAllowed
-     * @param string $input
+     * @param Allowed $annotation
      * @param string|null $resource
      * @param string|null $privilege
      */
-    public function testAccessAllowed(string $input, ?string $resource, ?string $privilege): void
+    public function testAccessAllowed(Allowed $annotation, ?string $resource, ?string $privilege): void
     {
-        $annotation = $this->parseAnnotation($input);
         $user = $this->mockUser($resource, $privilege, IAuthorizator::ALLOW);
         $validator = new AccessValidators\PermissionValidator($user);
 
@@ -57,22 +45,22 @@ class PermissionValidatorTest extends TestCase
     {
         return [
             [
-                'input' => '@Nepada\SecurityAnnotations\Annotations\Allowed()',
+                'annotation' => new Allowed(),
                 'resource' => IAuthorizator::ALL,
                 'privilege' => IAuthorizator::ALL,
             ],
             [
-                'input' => '@Nepada\SecurityAnnotations\Annotations\Allowed(resource="foo")',
+                'annotation' => new Allowed('foo'),
                 'resource' => 'foo',
                 'privilege' => IAuthorizator::ALL,
             ],
             [
-                'input' => '@Nepada\SecurityAnnotations\Annotations\Allowed(privilege="edit")',
+                'annotation' => new Allowed(null, 'edit'),
                 'resource' => IAuthorizator::ALL,
                 'privilege' => 'edit',
             ],
             [
-                'input' => '@Nepada\SecurityAnnotations\Annotations\Allowed(resource="foo", privilege="edit")',
+                'annotation' => new Allowed('foo', 'edit'),
                 'resource' => 'foo',
                 'privilege' => 'edit',
             ],
@@ -81,14 +69,13 @@ class PermissionValidatorTest extends TestCase
 
     /**
      * @dataProvider getDataForAccessDenied
-     * @param string $input
+     * @param Allowed $annotation
      * @param string|null $resource
      * @param string|null $privilege
      * @param string $message
      */
-    public function testAccessDenied(string $input, ?string $resource, ?string $privilege, string $message): void
+    public function testAccessDenied(Allowed $annotation, ?string $resource, ?string $privilege, string $message): void
     {
-        $annotation = $this->parseAnnotation($input);
         $user = $this->mockUser($resource, $privilege, IAuthorizator::DENY);
         $validator = new AccessValidators\PermissionValidator($user);
 
@@ -104,25 +91,25 @@ class PermissionValidatorTest extends TestCase
     {
         return [
             [
-                'input' => '@Nepada\SecurityAnnotations\Annotations\Allowed()',
+                'annotation' => new Allowed(),
                 'resource' => IAuthorizator::ALL,
                 'privilege' => IAuthorizator::ALL,
                 'message' => 'User is not allowed to access the resource.',
             ],
             [
-                'input' => '@Nepada\SecurityAnnotations\Annotations\Allowed(resource="foo")',
+                'annotation' => new Allowed('foo'),
                 'resource' => 'foo',
                 'privilege' => IAuthorizator::ALL,
                 'message' => "User is not allowed to access the resource 'foo'.",
             ],
             [
-                'input' => '@Nepada\SecurityAnnotations\Annotations\Allowed(privilege="edit")',
+                'annotation' => new Allowed(null, 'edit'),
                 'resource' => IAuthorizator::ALL,
                 'privilege' => 'edit',
                 'message' => 'User is not allowed to edit the resource.',
             ],
             [
-                'input' => '@Nepada\SecurityAnnotations\Annotations\Allowed(resource="foo", privilege="edit")',
+                'annotation' => new Allowed('foo', 'edit'),
                 'resource' => 'foo',
                 'privilege' => 'edit',
                 'message' => "User is not allowed to edit the resource 'foo'.",
@@ -144,14 +131,6 @@ class PermissionValidatorTest extends TestCase
         return $user;
     }
 
-    private function parseAnnotation(string $input): Allowed
-    {
-        $annotations = $this->docParser->parse($input);
-        Assert::count(1, $annotations);
-        Assert::type(Allowed::class, $annotations[0]);
-        return $annotations[0];
-    }
-
 }