Merge pull request biig-io#24 from swagindustries/feature/optional-se…

…curity Make security optional in crud controllers
Nek- · Aug 17, 2020 · 39d08be · 39d08be
2 parents 6b91985 + 6fd2255
commit 39d08be
Show file tree

Hide file tree

Showing 13 changed files with 63 additions and 50 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 - Huge BC Break on namespaces. You need to rename all classes used to SwagIndustries instead of Biig
+- BC Break on CRUD classes. It's big changes time. So we made the security optional for crud controllers, this has a
+  consequence on their constructor
 
 ## [0.6.0] 2020-06-01
 ### Added

diff --git a/docs/Crud.md b/docs/Crud.md
@@ -1,6 +1,18 @@
 CRUDs with Melodiia
 ===================
 
+Requirements
+------------
+
+The CRUD of Melodiia is based on the following components of Symfony. This means you need to use install & enable them:
+- **The Serializer Component**
+
+  Melodiia uses the serializer of Symfony to render responses.
+- **The Form Component**
+
+  Melodiia uses the form component to manage input.
+
+
 Register your first CRUD
 ------------------------
 
@@ -36,7 +48,6 @@ acme_article_get:
     defaults:
         melodiia_model: App\Entity\Article
         melodiia_security_check: 'some content runnable in AuthorizationChecker class'
-
 ```
 
 #### Collection

diff --git a/src/Bridge/Symfony/Resources/config/crud.yaml b/src/Bridge/Symfony/Resources/config/crud.yaml
@@ -27,7 +27,7 @@ services:
             $dataStore: '@melodiia.doctrine.data_provider'
             $formFactory: '@form.factory'
             $dispatcher: '@event_dispatcher'
-            $checker: '@security.authorization_checker'
+            $checker: '@?security.authorization_checker'
         tags: [ controller.service_arguments ]
 
     melodiia.crud.controller.update:
@@ -36,32 +36,32 @@ services:
             $dataStore: '@melodiia.doctrine.data_provider'
             $formFactory: '@form.factory'
             $dispatcher: '@event_dispatcher'
-            $checker: '@security.authorization_checker'
             $idResolver: '@melodiia.crud.id_resolver'
+            $checker: '@?security.authorization_checker'
         tags: [ controller.service_arguments ]
 
     melodiia.crud.controller.get_all:
         class: SwagIndustries\Melodiia\Crud\Controller\GetAll
         arguments:
             $dataStore: '@melodiia.doctrine.data_provider'
-            $checker: '@security.authorization_checker'
             $collectionFactory: '@melodiia.crud.filters.filter_collection_factory'
             $pagesRequestFactory: '@SwagIndustries\Melodiia\Crud\Pagination\PaginationRequestFactoryInterface'
+            $checker: '@?security.authorization_checker'
         tags: [ controller.service_arguments ]
 
     melodiia.crud.controller.get:
         class: SwagIndustries\Melodiia\Crud\Controller\Get
         arguments:
             $dataStore: '@melodiia.doctrine.data_provider'
-            $checker: '@security.authorization_checker'
             $idResolver: '@melodiia.crud.id_resolver'
+            $checker: '@?security.authorization_checker'
         tags: [ controller.service_arguments ]
 
     melodiia.crud.controller.delete:
         class: SwagIndustries\Melodiia\Crud\Controller\Delete
         arguments:
             $dataStore: '@melodiia.doctrine.data_provider'
-            $checker: '@security.authorization_checker'
             $dispatcher: '@event_dispatcher'
             $idResolver: '@melodiia.crud.id_resolver'
+            $checker: '@?security.authorization_checker'
         tags: [ controller.service_arguments ]
diff --git a/src/Crud/Controller/Create.php b/src/Crud/Controller/Create.php
@@ -12,7 +12,6 @@
 use Symfony\Component\Form\FormFactoryInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
-use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 
 /**
  * Crud controller that create data model with the data from the request using a form.
@@ -30,10 +29,10 @@ final class Create extends BaseCrudController
     /** @var FormFactoryInterface */
     private $formFactory;
 
-    /** @var AuthorizationCheckerInterface */
+    /** @var AuthorizationCheckerInterface|null */
     private $checker;
 
-    public function __construct(DataStoreInterface $dataStore, FormFactoryInterface $formFactory, EventDispatcherInterface $dispatcher, AuthorizationCheckerInterface $checker)
+    public function __construct(DataStoreInterface $dataStore, FormFactoryInterface $formFactory, EventDispatcherInterface $dispatcher, AuthorizationCheckerInterface $checker = null)
     {
         parent::__construct($dispatcher);
         $this->dataStore = $dataStore;
@@ -46,14 +45,11 @@ public function __invoke(Request $request): ApiResponse
         // Metadata you can specify in routing definition
         $modelClass = $request->attributes->get(self::MODEL_ATTRIBUTE);
         $form = $request->attributes->get(self::FORM_ATTRIBUTE);
-        $securityCheck = $request->attributes->get(self::SECURITY_CHECK, null);
         $clearMissing = $request->attributes->getBoolean(self::FORM_CLEAR_MISSING, true);
 
         $this->assertModelClassInvalid($modelClass);
 
-        if ($securityCheck && !$this->checker->isGranted($securityCheck)) {
-            throw new AccessDeniedException(\sprintf('Access denied to data of type "%s".', $modelClass));
-        }
+        $this->assertResourceRights($request);
 
         if (empty($form) || !class_exists($form)) {
             throw new MelodiiaLogicException('If you use melodiia CRUD classes, you need to specify a model.');

diff --git a/src/Crud/Controller/CrudControllerTrait.php b/src/Crud/Controller/CrudControllerTrait.php
@@ -5,7 +5,10 @@
 namespace SwagIndustries\Melodiia\Crud\Controller;
 
 use SwagIndustries\Melodiia\Crud\CrudableModelInterface;
+use SwagIndustries\Melodiia\Crud\CrudControllerInterface;
 use SwagIndustries\Melodiia\Exception\MelodiiaLogicException;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 
 trait CrudControllerTrait
 {
@@ -20,4 +23,22 @@ public function assertModelClassInvalid(string $modelClass): void
             throw new MelodiiaLogicException('If you use melodiia CRUD classes, you need to specify a model.');
         }
     }
+
+    private function assertResourceRights(Request $request, $data = null)
+    {
+        $securityCheck = $request->attributes->get(CrudControllerInterface::SECURITY_CHECK, null);
+        $modelClass = $request->attributes->get(CrudControllerInterface::MODEL_ATTRIBUTE);
+
+        if (null === $securityCheck) {
+            return;
+        }
+
+        if (!$this->checker) {
+            throw new MelodiiaLogicException('The security component of Symfony seems to not be enabled');
+        }
+
+        if (($data && !$this->checker->isGranted($securityCheck, $data)) || !$this->checker->isGranted($securityCheck)) {
+            throw new AccessDeniedException(\sprintf('Access denied to data of type "%s".', $modelClass));
+        }
+    }
 }
diff --git a/src/Crud/Controller/Delete.php b/src/Crud/Controller/Delete.php
@@ -14,7 +14,6 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
-use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 
 final class Delete extends BaseCrudController implements CrudControllerInterface
 {
@@ -26,24 +25,23 @@ final class Delete extends BaseCrudController implements CrudControllerInterface
     /** @var DataStoreInterface */
     private $dataStore;
 
-    /** @var AuthorizationCheckerInterface */
+    /** @var AuthorizationCheckerInterface|null */
     private $checker;
 
     /** @var IdResolverInterface */
     private $idResolver;
 
-    public function __construct(DataStoreInterface $dataStore, AuthorizationCheckerInterface $checker, EventDispatcherInterface $dispatcher, IdResolverInterface $idResolver = null)
+    public function __construct(DataStoreInterface $dataStore, EventDispatcherInterface $dispatcher, IdResolverInterface $idResolver = null, AuthorizationCheckerInterface $checker = null)
     {
         parent::__construct($dispatcher);
         $this->dataStore = $dataStore;
-        $this->checker = $checker;
         $this->idResolver = $idResolver ?? new SimpleIdResolver();
+        $this->checker = $checker;
     }
 
     public function __invoke(Request $request)
     {
         $modelClass = $request->attributes->get(self::MODEL_ATTRIBUTE);
-        $securityCheck = $request->attributes->get(self::SECURITY_CHECK, null);
 
         try {
             $id = $this->idResolver->resolveId($request, $modelClass);
@@ -59,9 +57,7 @@ public function __invoke(Request $request)
             throw new NotFoundHttpException(\sprintf('resource item "%s" with id "%s" can not be found', $modelClass, $id));
         }
 
-        if ($securityCheck && !$this->checker->isGranted($securityCheck, $data)) {
-            throw new AccessDeniedException(\sprintf('You can\'t perform a delete operation of the resource item "%s" with id "%s"', $modelClass, $id));
-        }
+        $this->assertResourceRights($request, $data);
 
         $this->dispatch(new CrudEvent($data), self::EVENT_PRE_DELETE);
         $this->dataStore->remove($data);

diff --git a/src/Crud/Controller/Get.php b/src/Crud/Controller/Get.php
@@ -13,7 +13,6 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
-use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 
 final class Get implements CrudControllerInterface
 {
@@ -22,24 +21,23 @@ final class Get implements CrudControllerInterface
     /** @var DataStoreInterface */
     private $dataStore;
 
-    /** @var AuthorizationCheckerInterface */
+    /** @var AuthorizationCheckerInterface|null */
     private $checker;
 
     /** @var IdResolverInterface */
     private $idResolver;
 
-    public function __construct(DataStoreInterface $dataStore, AuthorizationCheckerInterface $checker, IdResolverInterface $idResolver = null)
+    public function __construct(DataStoreInterface $dataStore, IdResolverInterface $idResolver = null, AuthorizationCheckerInterface $checker = null)
     {
         $this->dataStore = $dataStore;
-        $this->checker = $checker;
         $this->idResolver = $idResolver ?? new SimpleIdResolver();
+        $this->checker = $checker;
     }
 
     public function __invoke(Request $request): ApiResponse
     {
         $modelClass = $request->attributes->get(self::MODEL_ATTRIBUTE);
         $groups = $request->attributes->get(self::SERIALIZATION_GROUP, []);
-        $securityCheck = $request->attributes->get(self::SECURITY_CHECK, null);
         try {
             $id = $this->idResolver->resolveId($request, $modelClass);
         } catch (IdMissingException $e) {
@@ -52,9 +50,7 @@ public function __invoke(Request $request): ApiResponse
             return new NotFound();
         }
 
-        if ($securityCheck && !$this->checker->isGranted($securityCheck, $data)) {
-            throw new AccessDeniedException(\sprintf('Access denied to data of type "%s".', get_class($data)));
-        }
+        $this->assertResourceRights($request, $data);
 
         return new OkContent($data, $groups);
     }

diff --git a/src/Crud/Controller/GetAll.php b/src/Crud/Controller/GetAll.php
@@ -10,7 +10,6 @@
 use SwagIndustries\Melodiia\Response\OkContent;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
-use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 
 class GetAll implements CrudControllerInterface
 {
@@ -30,9 +29,9 @@ class GetAll implements CrudControllerInterface
 
     public function __construct(
         DataStoreInterface $dataStore,
-        AuthorizationCheckerInterface $checker,
         FilterCollectionFactoryInterface $collectionFactory,
-        PaginationRequestFactoryInterface $pagesRequestFactory
+        PaginationRequestFactoryInterface $pagesRequestFactory,
+        AuthorizationCheckerInterface $checker = null
     ) {
         $this->dataStore = $dataStore;
         $this->checker = $checker;
@@ -44,14 +43,10 @@ public function __invoke(Request $request)
     {
         // Metadata you can specify in routing definition
         $modelClass = $request->attributes->get(self::MODEL_ATTRIBUTE);
-        $securityCheck = $request->attributes->get(self::SECURITY_CHECK, null);
         $groups = $request->attributes->get(self::SERIALIZATION_GROUP, []);
 
         $this->assertModelClassInvalid($modelClass);
-
-        if ($securityCheck && !$this->checker->isGranted($securityCheck)) {
-            throw new AccessDeniedException(\sprintf('Access denied to data of type "%s".', $modelClass));
-        }
+        $this->assertResourceRights($request);
 
         $filters = $this->filterCollectionFactory->createCollection($modelClass);
         $form = $filters->getForm();

diff --git a/src/Crud/Controller/Update.php b/src/Crud/Controller/Update.php
@@ -16,7 +16,6 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
-use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 
 /**
  * Crud controller that update data model with the data from the request using a form.
@@ -40,7 +39,7 @@ final class Update extends BaseCrudController
     /** @var IdResolverInterface */
     private $idResolver;
 
-    public function __construct(DataStoreInterface $dataStore, FormFactoryInterface $formFactory, EventDispatcherInterface $dispatcher, AuthorizationCheckerInterface $checker, IdResolverInterface $idResolver = null)
+    public function __construct(DataStoreInterface $dataStore, FormFactoryInterface $formFactory, EventDispatcherInterface $dispatcher, IdResolverInterface $idResolver = null, AuthorizationCheckerInterface $checker = null)
     {
         parent::__construct($dispatcher);
         $this->dataStore = $dataStore;
@@ -53,7 +52,6 @@ public function __invoke(Request $request): ApiResponse
     {
         $modelClass = $request->attributes->get(self::MODEL_ATTRIBUTE);
         $form = $request->attributes->get(self::FORM_ATTRIBUTE);
-        $securityCheck = $request->attributes->get(self::SECURITY_CHECK, null);
         $clearMissing = $request->attributes->has(self::FORM_CLEAR_MISSING)
             ? $request->attributes->getBoolean(self::FORM_CLEAR_MISSING)
             : null
@@ -69,9 +67,7 @@ public function __invoke(Request $request): ApiResponse
 
         $data = $this->dataStore->find($modelClass, $id);
 
-        if ($securityCheck && !$this->checker->isGranted($securityCheck, $data)) {
-            throw new AccessDeniedException(\sprintf('Access denied to data of type "%s".', $modelClass));
-        }
+        $this->assertResourceRights($request, $data);
 
         if (empty($form) || !class_exists($form)) {
             throw new MelodiiaLogicException('If you use melodiia CRUD classes, you need to specify a model.');

diff --git a/tests/Melodiia/Crud/Controller/DeleteTest.php b/tests/Melodiia/Crud/Controller/DeleteTest.php
@@ -59,9 +59,9 @@ protected function setUp()
 
         $this->controller = new Delete(
             $this->dataStore->reveal(),
-            $this->checker->reveal(),
             $this->dispatcher->reveal(),
-            $idResolver->reveal()
+            $idResolver->reveal(),
+            $this->checker->reveal()
         );
     }
 

diff --git a/tests/Melodiia/Crud/Controller/GetAllTest.php b/tests/Melodiia/Crud/Controller/GetAllTest.php
@@ -82,9 +82,9 @@ public function setUp()
 
         $this->controller = new GetAll(
             $this->dataStore->reveal(),
-            $this->authorizationChecker->reveal(),
             $this->filtersFactory->reveal(),
-            $paginationFactory->reveal()
+            $paginationFactory->reveal(),
+            $this->authorizationChecker->reveal()
         );
     }
 

diff --git a/tests/Melodiia/Crud/Controller/GetTest.php b/tests/Melodiia/Crud/Controller/GetTest.php
@@ -35,7 +35,7 @@ public function setUp()
         $idResolver = $this->prophesize(IdResolverInterface::class);
         $idResolver->resolveId(Argument::type(Request::class), Argument::type('string'))->willReturn('id');
 
-        $this->controller = new Get($this->dataStore->reveal(), $this->authorizationChecker->reveal(), $idResolver->reveal());
+        $this->controller = new Get($this->dataStore->reveal(), $idResolver->reveal(), $this->authorizationChecker->reveal());
     }
 
     public function testItIsIntanceOfMelodiiaController()

diff --git a/tests/Melodiia/Crud/Controller/UpdateTest.php b/tests/Melodiia/Crud/Controller/UpdateTest.php
@@ -82,8 +82,8 @@ public function setUp()
             $this->dataStore->reveal(),
             $this->formFactory->reveal(),
             $this->dispatcher->reveal(),
-            $this->checker->reveal(),
-            $idResolver->reveal()
+            $idResolver->reveal(),
+            $this->checker->reveal()
         );
     }