Deprecate usage of Doctrine annotations

.github/workflows/ci.yaml

@@ -13,7 +13,6 @@ jobs:
       fail-fast: false
       matrix:
         php:
-          - 7.4
           - 8.0
           - 8.1
         composer_args:
@@ -53,7 +52,7 @@ jobs:
       - uses: actions/checkout@v3
       - uses: shivammathur/setup-php@v2
         with:
-          php-version: 7.4
+          php-version: 8.0
           coverage: none
       - name: Build
         run: composer update --no-interaction --no-progress --prefer-dist --prefer-stable

README.md

@@ -23,16 +23,18 @@ 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: using phpdoc annotations 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
+
+UsagecheckReq
 -----
 
 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).

composer.json

@@ -15,12 +15,12 @@
         }
     ],
     "require": {
-        "php": ">=7.4.0 <8.2",
+        "php": ">=8.0.0 <8.2",
         "nette/utils": "^3.2@dev",
         "nette/component-model": "^3.0.2@dev",
         "nette/application": "^3.1.4@dev",
         "nette/security": "^3.0@dev",
-        "doctrine/annotations": "^1.11.0@dev"
+        "doctrine/annotations": "^1.12.0@dev"
     },
     "require-dev": {
         "nette/tester": "2.4.2",
@@ -57,7 +57,7 @@
     },
     "extra": {
         "branch-alias": {
-            "dev-master": "4.2-dev"
+            "dev-master": "4.3-dev"
         }
     }
 }

phpstan.neon.dist

@@ -8,7 +8,6 @@ includes:
     - vendor/spaze/phpstan-disallowed-calls/disallowed-insecure-calls.neon
     - vendor/spaze/phpstan-disallowed-calls/disallowed-loose-calls.neon
     - tests/PHPStan/disallowedCalls.neon
-    - tests/PHPStan/conditional.config.php
 
 parameters:
     level: max

src/Bridges/SecurityAnnotationsDI/SecurityAnnotationsExtension.php

@@ -43,19 +43,18 @@ public function loadConfiguration(): void
         $container = $this->getContainerBuilder();
 
         $readers = [];
-        if (PHP_VERSION_ID >= 8_00_00) {
-            $readers[] = $container->addDefinition($this->prefix('attributesReader'), new ServiceDefinition())
-                ->setType(AttributesReader::class)
-                ->setAutowired(AttributesReader::class);
-        }
+        $readers[] = $container->addDefinition($this->prefix('attributesReader'), new ServiceDefinition())
+            ->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);
         }
-        if ($readers === []) {
-            throw new \LogicException('You must either use PHP >= 8.0 for attributes support, or enable doctrine annotations.');
-        }
 
         $container->addDefinition($this->prefix('annotationsReader'), new ServiceDefinition())
             ->setType(AnnotationsReader::class)

src/SecurityAnnotations/Annotations/Allowed.php

@@ -17,17 +17,9 @@ final class Allowed implements NamedArgumentConstructorAnnotation
 
     use Nette\SmartObject;
 
-    /**
-     * @internal use getter instead
-     * @var string
-     */
-    public ?string $resource;
-
-    /**
-     * @internal use getter instead
-     * @var string
-     */
-    public ?string $privilege;
+    private ?string $resource;
+
+    private ?string $privilege;
 
     public function __construct(?string $resource = Nette\Security\IAuthorizator::ALL, ?string $privilege = Nette\Security\IAuthorizator::ALL)
     {

src/SecurityAnnotations/Annotations/Role.php

@@ -12,28 +12,42 @@
  * @Target({"CLASS","METHOD"})
  */
 #[Attribute(Attribute::TARGET_CLASS | Attribute::TARGET_METHOD | Attribute::IS_REPEATABLE)]
-final class Role implements NamedArgumentConstructorAnnotation
+final class Role
 {
 
     use Nette\SmartObject;
 
     /**
-     * @Required
-     * @internal use getter instead
-     * @var array<string>
+     * @var non-empty-list<string>
      */
-    public array $roles;
+    private array $roles;
 
     /**
-     * @param string|array<string> $value
+     * @param array{value: string|non-empty-list<string>}|non-empty-list<string>|string ...$roles
      */
-    public function __construct($value)
+    public function __construct(string|array ...$roles)
     {
-        $this->roles = is_string($value) ? [$value] : $value;
+        if (isset($roles[0]['value'])) { // Compatibility with Doctrine annotations
+            $roles = is_string($roles[0]['value']) ? [$roles[0]['value']] : $roles[0]['value'];
+        } elseif (func_num_args() === 1 && is_array(func_get_arg(0))) { // BC with passing roles as an array
+            trigger_error('Passing roles as a single array argument is deprecated, use variadic argument instead', E_USER_DEPRECATED);
+            $roles = func_get_arg(0);
+        }
+        if ($roles === []) {
+            throw new \InvalidArgumentException('At least one role name must be specified');
+        }
+        foreach ($roles as $role) {
+            if (! is_string($role)) {
+                $type = gettype($role);
+                throw new \InvalidArgumentException("Expected string with role name, got {$type}");
+            }
+        }
+
+        $this->roles = $roles;
     }
 
     /**
-     * @return string[]
+     * @return non-empty-list<string>
      */
     public function getRoles(): array
     {

src/SecurityAnnotations/RequirementsChecker.php

@@ -35,7 +35,7 @@ public function protectElement(\Reflector $element): void
     {
         $annotations = $this->annotationReader->getAll($element);
         foreach ($annotations as $annotation) {
-            $annotationName = get_class($annotation);
+            $annotationName = $annotation::class;
             if (isset($this->accessValidators[$annotationName])) {
                 $this->accessValidators[$annotationName]->validateAccess($annotation);
             }

src/SecurityAnnotations/SecuredComponents.php

@@ -13,7 +13,7 @@ trait SecuredComponents
      * @param mixed $element
      * @throws Nette\Application\ForbiddenRequestException;
      */
-    abstract public function checkRequirements($element): void;
+    abstract public function checkRequirements(mixed $element): void;
 
     protected function createComponent(string $name): ?IComponent
     {

src/SecurityAnnotations/SecurityAnnotations.php

@@ -19,7 +19,7 @@ public function injectRequirementsChecker(RequirementsChecker $requirementsCheck
      * @param mixed $element
      * @throws Nette\Application\ForbiddenRequestException
      */
-    public function checkRequirements($element): void
+    public function checkRequirements(mixed $element): void
     {
         parent::checkRequirements($element);
 

tests/Bridges/SecurityAnnotationsDI/Fixtures/BarValidator.php

@@ -17,9 +17,9 @@ public function getSupportedAnnotationName(): string
     }
 
     /**
-     * @param mixed $annotation parsed value of annotation
+     * @param BarValidator $annotation parsed value of annotation
      */
-    public function validateAccess($annotation): void
+    public function validateAccess(object $annotation): void
     {
     }
 

tests/Bridges/SecurityAnnotationsDI/Fixtures/Foo/FooValidator.php

@@ -17,9 +17,9 @@ public function getSupportedAnnotationName(): string
     }
 
     /**
-     * @param mixed $annotation parsed value of annotation
+     * @param FooValidator $annotation parsed value of annotation
      */
-    public function validateAccess($annotation): void
+    public function validateAccess(object $annotation): void
     {
     }
 

tests/Bridges/SecurityAnnotationsDI/Fixtures/FooValidator.php

@@ -17,9 +17,9 @@ public function getSupportedAnnotationName(): string
     }
 
     /**
-     * @param mixed $annotation parsed value of annotation
+     * @param FooValidator $annotation parsed value of annotation
      */
-    public function validateAccess($annotation): void
+    public function validateAccess(object $annotation): void
     {
     }
 

tests/Bridges/SecurityAnnotationsDI/SecurityAnnotationsExtensionTest.phpt

@@ -37,6 +37,7 @@ class SecurityAnnotationsExtensionTest extends TestCase
 
     public function testDefaultValidators(): void
     {
+        $this->configurator->addConfig(__DIR__ . '/Fixtures/config.doctrine-annotations-disabled.neon');
         $container = $this->configurator->createContainer();
 
         Assert::type(LoggedInValidator::class, $container->getService('securityAnnotations.loggedInValidator'));
@@ -55,6 +56,7 @@ class SecurityAnnotationsExtensionTest extends TestCase
 
     public function testCustomValidators(): void
     {
+        $this->configurator->addConfig(__DIR__ . '/Fixtures/config.doctrine-annotations-disabled.neon');
         $this->configurator->addConfig(__DIR__ . '/Fixtures/config.custom-validators.neon');
         $container = $this->configurator->createContainer();
 
@@ -73,6 +75,7 @@ class SecurityAnnotationsExtensionTest extends TestCase
 
     public function testInvalidValidator(): void
     {
+        $this->configurator->addConfig(__DIR__ . '/Fixtures/config.doctrine-annotations-disabled.neon');
         $this->configurator->addConfig(__DIR__ . '/Fixtures/config.invalid-validator.neon');
 
         Assert::exception(
@@ -86,6 +89,7 @@ class SecurityAnnotationsExtensionTest extends TestCase
 
     public function testNotFoundValidator(): void
     {
+        $this->configurator->addConfig(__DIR__ . '/Fixtures/config.doctrine-annotations-disabled.neon');
         $this->configurator->addConfig(__DIR__ . '/Fixtures/config.not-found-validator.neon');
 
         Assert::exception(
@@ -103,27 +107,21 @@ class SecurityAnnotationsExtensionTest extends TestCase
             new SecurityAnnotations\Annotations\Role('attribute'),
             new SecurityAnnotations\Annotations\Role('annotation'),
         ];
-        if (PHP_VERSION_ID < 8_00_00) {
-            array_shift($expected);
-        }
-        /** @var SecurityAnnotations\AnnotationReaders\AnnotationsReader $reader */
-        $reader = $this->configurator->createContainer()->getByType(SecurityAnnotations\AnnotationReaders\AnnotationsReader::class);
-        Assert::equal($expected, $reader->getAll(new \ReflectionClass(LoremIpsum::class)));
+        Assert::error(
+            function () use ($expected): void {
+                /** @var SecurityAnnotations\AnnotationReaders\AnnotationsReader $reader */
+                $reader = $this->configurator->createContainer()->getByType(SecurityAnnotations\AnnotationReaders\AnnotationsReader::class);
+                Assert::equal($expected, $reader->getAll(new \ReflectionClass(LoremIpsum::class)));
+            },
+            E_USER_DEPRECATED,
+            'Using Doctrine annotations is deprecated, migrate to native PHP8 attributes and set enableDoctrineAnnotations: false in your config',
+        );
     }
 
     public function testReaderWithDoctrineAnnotationsDisabled(): void
     {
         $this->configurator->addConfig(__DIR__ . '/Fixtures/config.doctrine-annotations-disabled.neon');
 
-        if (PHP_VERSION_ID < 8_00_00) {
-            Assert::exception(
-                fn () => $this->configurator->createContainer()->getByType(SecurityAnnotations\AnnotationReaders\AnnotationsReader::class),
-                \LogicException::class,
-                'You must either use PHP >= 8.0 for attributes support, or enable doctrine annotations.',
-            );
-            return;
-        }
-
         /** @var SecurityAnnotations\AnnotationReaders\AnnotationsReader $reader */
         $reader = $this->configurator->createContainer()->getByType(SecurityAnnotations\AnnotationReaders\AnnotationsReader::class);
         Assert::equal(