Merge pull request #40 from symfony-cmf/issue-11/security

Added security protection of the API

Author: wouterj

Controller/ResourceController.php

@@ -18,11 +18,16 @@
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use Symfony\Component\Routing\Exception\RouteNotFoundException;
+use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
+use Symfony\Component\Security\Core\Exception\AccessDeniedException;
 use JMS\Serializer\SerializerInterface;
 use JMS\Serializer\SerializationContext;
 
 class ResourceController
 {
+    const ROLE_RESOURCE_READ = 'CMF_RESOURCE_READ';
+    const ROLE_RESOURCE_WRITE = 'CMF_RESOURCE_WRITE';
+
     /**
      * @var RepositoryRegistryInterface
      */
@@ -33,14 +38,20 @@ class ResourceController
      */
     private $serializer;
 
+    /**
+     * @var AuthorizationCheckerInterface|null
+     */
+    private $authorizationChecker;
+
     /**
      * @param SerializerInterface         $serializer
      * @param RepositoryRegistryInterface $registry
      */
-    public function __construct(SerializerInterface $serializer, RepositoryRegistryInterface $registry)
+    public function __construct(SerializerInterface $serializer, RepositoryRegistryInterface $registry, AuthorizationCheckerInterface $authorizationChecker = null)
     {
         $this->serializer = $serializer;
         $this->registry = $registry;
+        $this->authorizationChecker = $authorizationChecker;
     }
 
     /**
@@ -51,9 +62,15 @@ public function __construct(SerializerInterface $serializer, RepositoryRegistryI
      */
     public function getResourceAction($repositoryName, $path)
     {
+        $path = '/'.ltrim($path, '/');
+
         try {
             $repository = $this->registry->get($repositoryName);
-            $resource = $repository->get('/'.$path);
+
+            $fullPath = method_exists($repository, 'resolvePath') ? $repository->resolvePath($path) : $path;
+            $this->guardAccess('read', $repositoryName, $fullPath);
+
+            $resource = $repository->get($path);
 
             return $this->createResponse($resource);
         } catch (ResourceNotFoundException $e) {
@@ -86,9 +103,12 @@ public function getResourceAction($repositoryName, $path)
      */
     public function patchResourceAction($repositoryName, $path, Request $request)
     {
+        $path = '/'.ltrim($path, '/');
         $repository = $this->registry->get($repositoryName);
 
-        $path = '/'.ltrim($path, '/');
+        $fullPath = method_exists($repository, 'resolvePath') ? $repository->resolvePath($path) : $path;
+        $this->guardAccess('write', $repositoryName, $fullPath);
+
 
         $requestContent = json_decode($request->getContent(), true);
         if (!$requestContent) {
@@ -124,9 +144,11 @@ public function patchResourceAction($repositoryName, $path, Request $request)
      */
     public function deleteResourceAction($repositoryName, $path)
     {
+        $path = '/'.ltrim($path, '/');
         $repository = $this->registry->get($repositoryName);
 
-        $path = '/'.ltrim($path, '/');
+        $fullPath = method_exists($repository, 'resolvePath') ? $repository->resolvePath($path) : $path;
+        $this->guardAccess('write', $repositoryName, $fullPath);
 
         $repository->remove($path);
 
@@ -143,6 +165,18 @@ private function badRequestResponse($message)
         return $this->createResponse(['message' => $message], Response::HTTP_BAD_REQUEST);
     }
 
+    private function guardAccess($attribute, $repository, $path)
+    {
+        if (null !== $this->authorizationChecker
+            && !$this->authorizationChecker->isGranted(
+                'CMF_RESOURCE_'.strtoupper($attribute),
+                ['repository_name' => $repository, 'path' => $path]
+            )
+        ) {
+            throw new AccessDeniedException(sprintf('%s access denied for "%s".', ucfirst($attribute), $path));
+        }
+    }
+
     /**
      * @param mixed $resource
      * @param int   $httpStatusCode

Resources/config/resource-rest.xml

@@ -9,6 +9,7 @@
         <service id="cmf_resource_rest.controller.resource" class="Symfony\Cmf\Bundle\ResourceRestBundle\Controller\ResourceController">
             <argument type="service" id="serializer" />
             <argument type="service" id="cmf_resource.registry" />
+            <argument type="service" id="security.authorization_checker" on-invalid="ignore" />
         </service>
 
         <service id="cmf_resource_rest.registry.payload_alias" class="Symfony\Cmf\Bundle\ResourceRestBundle\Registry\PayloadAliasRegistry">

Tests/Features/Context/ResourceContext.php

@@ -23,7 +23,7 @@
 use Symfony\Component\Finder\Finder;
 use Webmozart\Assert\Assert;
 
-class ResourceContext implements Context, KernelAwareContext
+class ResourceContext implements Context
 {
     private $session;
     private $manager;
@@ -33,6 +33,14 @@ class ResourceContext implements Context, KernelAwareContext
      */
     private $kernel;
 
+    public function __construct()
+    {
+        require_once __DIR__.'/../../../vendor/symfony-cmf/testing/bootstrap/bootstrap.php';
+        require_once __DIR__.'/../../Resources/app/AppKernel.php';
+
+        $this->kernel = new \AppKernel('test', true);
+    }
+
     /**
      * Return the path of the configuration file used by the AppKernel.
      *
@@ -45,14 +53,6 @@ public static function getConfigurationFile()
         return __DIR__.'/../../Resources/app/cache/resource.yml';
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function setKernel(KernelInterface $kernel)
-    {
-        $this->kernel = $kernel;
-    }
-
     /**
      * @BeforeScenario
      */
@@ -64,6 +64,8 @@ public function beforeScenario(BeforeScenarioScope $scope)
 
         $this->clearDiCache();
 
+        $this->kernel->boot();
+
         $this->manager = $this->kernel->getContainer()->get('doctrine_phpcr.odm.document_manager');
         $this->session = $this->manager->getPhpcrSession();
 
@@ -79,6 +81,7 @@ public function beforeScenario(BeforeScenarioScope $scope)
     public function refreshSession()
     {
         $this->session->refresh(true);
+        $this->kernel->shutdown();
     }
 
     /**

Tests/Features/security.feature

@@ -0,0 +1,33 @@
+Feature: Security
+    In order to deny API access to private files
+    As a developer
+    I need to be able to write security voters
+
+    Background:
+        Given the test application has the following configuration:
+            """
+            cmf_resource:
+                repositories:
+                    security:
+                        type: doctrine_phpcr
+                        basepath: /tests/cmf/articles
+            """
+        And there exists an "Article" document at "/private/foo":
+            | title | Article 1          |
+            | body  | This is my article |
+
+    Scenario: Retrieve a protected resource
+        When I send a GET request to "/api/security/private/foo"
+        Then the response code should be 401
+
+    Scenario: Retrieve a protected non-existent resource
+        When I send a GET request to "/api/security/private/bar"
+        Then the response code should be 401
+
+    Scenario: Remove a protected resource
+        When I send a DELETE request to "/api/security/private/admin/something"
+        Then the response code should be 401
+
+    Scenario: Edit a resource
+        When I send a PATCH request to "/api/security/admin/file"
+        Then the response code should be 401

Tests/Resources/TestBundle/Security/ResourceVoter.php

@@ -0,0 +1,32 @@
+<?php
+
+namespace Symfony\Cmf\Bundle\ResourceRestBundle\Tests\Resources\TestBundle\Security;
+
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
+use Symfony\Cmf\Bundle\ResourceRestBundle\Controller\ResourceController;
+
+class ResourceVoter extends Voter
+{
+    protected function supports($attribute, $subject)
+    {
+        return in_array($attribute, [ResourceController::ROLE_RESOURCE_READ, ResourceController::ROLE_RESOURCE_WRITE]);
+    }
+
+    protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
+    {
+        if ('security' !== $subject['repository_name']) {
+            return true;
+        }
+        
+        if ('/tests/cmf/articles/public' !== substr($subject['path'], 0, 27)) {
+            return false;
+        }
+
+        if (ResourceController::ROLE_RESOURCE_WRITE === $attribute) {
+            return false === strpos($subject['path'], 'admin');
+        }
+
+        return true;
+    }
+}

Tests/Resources/app/config/config.php

@@ -9,9 +9,15 @@
  * file that was distributed with this source code.
  */
 
+use Symfony\Cmf\Bundle\ResourceRestBundle\Tests\Resources\TestBundle\Security\ResourceVoter;
+
 $container->setParameter('cmf_testing.bundle_fqn', 'Symfony\Cmf\Bundle\ResourceRestBundle');
 $loader->import(CMF_TEST_CONFIG_DIR.'/dist/parameters.yml');
 $loader->import(CMF_TEST_CONFIG_DIR.'/dist/framework.php');
 $loader->import(CMF_TEST_CONFIG_DIR.'/dist/monolog.yml');
 $loader->import(CMF_TEST_CONFIG_DIR.'/dist/doctrine.yml');
+$loader->import(CMF_TEST_CONFIG_DIR.'/dist/security.yml');
 $loader->import(CMF_TEST_CONFIG_DIR.'/phpcr_odm.php');
+
+$container->register('app.resource_voter', ResourceVoter::class)
+    ->addTag('security.voter');

behat.yml

@@ -7,10 +7,5 @@ default:
             paths:
                 - Tests/Features
     extensions:
-        Behat\Symfony2Extension:
-            kernel:
-                path: Tests/Resources/app/AppKernel.php
-                env: behat
-                bootstrap: vendor/symfony-cmf/testing/bootstrap/bootstrap.php
         Behat\WebApiExtension:
             base_url: http://localhost:8000/

composer.json

@@ -22,7 +22,6 @@
         "jms/serializer": "^1.2",
         "behat/behat": "^3.0.6",
         "behat/web-api-extension" : "^1.0@dev",
-        "behat/symfony2-extension": "^2.0",
         "matthiasnoback/symfony-dependency-injection-test": "~0.6",
         "matthiasnoback/symfony-config-test": "^1.3.1",
         "sonata-project/admin-bundle": "^3.1"