Browse Source

Merge branch 'v8-4328-security_revision-2' into release/2.4.beta

Olivier Massot 2 years ago
parent
commit
d89b8a4048

+ 0 - 59
src/Security/Voter/BankAccountVoter.php

@@ -1,59 +0,0 @@
-<?php
-declare(strict_types=1);
-
-namespace App\Security\Voter;
-
-use App\Entity\Access\Access;
-use App\Entity\Core\BankAccount;
-use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
-use Symfony\Component\Security\Core\Authorization\Voter\Voter;
-use Symfony\Bundle\SecurityBundle\Security;
-use Symfony\Component\Security\Core\User\UserInterface;
-
-class BankAccountVoter extends Voter
-{
-    public function __construct(private Security $security)
-    { }
-
-    protected function supports(string $attribute, $subject): bool
-    {
-        return in_array($attribute, ['BANK_ACCOUNT_READ', 'BANK_ACCOUNT_EDIT', 'BANK_ACCOUNT_DELETE'])
-            && $subject instanceof BankAccount;
-    }
-
-    /**
-     * @param string $attribute
-     * @param mixed $subject
-     * @param TokenInterface $token
-     * @return bool
-     */
-    protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token): bool
-    {
-        /** @var Access $user */
-        $user = $token->getUser();
-        // if the user is anonymous, do not grant access
-        if (!$user instanceof UserInterface) {
-            return false;
-        }
-
-        switch ($attribute) {
-            case 'BANK_ACCOUNT_READ':
-                if($subject->getOrganization()->count() === 1){
-                    return $this->security->isGranted('ROLE_ORGANIZATION_VIEW')
-                        && $subject->getOrganization()->current()->getId() === $user->getOrganization()->getId();
-                }
-                break;
-            case 'BANK_ACCOUNT_EDIT':
-            case 'BANK_ACCOUNT_DELETE':
-                if($subject->getOrganization()->count() === 1){
-                    return $this->security->isGranted('ROLE_ORGANIZATION')
-                        && $subject->getOrganization()->current()->getId() === $user->getOrganization()->getId();
-                }
-                break;
-        }
-
-        return false;
-    }
-
-
-}

+ 0 - 58
src/Security/Voter/ContactPointVoter.php

@@ -1,58 +0,0 @@
-<?php
-declare(strict_types=1);
-
-namespace App\Security\Voter;
-
-use App\Entity\Access\Access;
-use App\Entity\Core\ContactPoint;
-use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
-use Symfony\Component\Security\Core\Authorization\Voter\Voter;
-use Symfony\Bundle\SecurityBundle\Security;
-use Symfony\Component\Security\Core\User\UserInterface;
-
-class ContactPointVoter extends Voter
-{
-    public function __construct(private Security $security)
-    { }
-
-    protected function supports(string $attribute, $subject): bool
-    {
-        return in_array($attribute, ['CONTACT_POINT_READ', 'CONTACT_POINT_EDIT', 'CONTACT_POINT_DELETE'])
-            && $subject instanceof ContactPoint;
-    }
-
-    /**
-     * @param string $attribute
-     * @param mixed $subject
-     * @param TokenInterface $token
-     * @return bool
-     */
-    protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token): bool
-    {
-        /** @var Access $user */
-        $user = $token->getUser();
-        // if the user is anonymous, do not grant access
-        if (!$user instanceof UserInterface) {
-            return false;
-        }
-        switch ($attribute) {
-            case 'CONTACT_POINT_READ':
-                if($subject->getOrganization()->count() === 1){
-                    return $this->security->isGranted('ROLE_ORGANIZATION_VIEW')
-                        && $subject->getOrganization()->current()->getId() === $user->getOrganization()->getId();
-                }
-                break;
-            case 'CONTACT_POINT_EDIT':
-            case 'CONTACT_POINT_DELETE':
-                if($subject->getOrganization()->count() === 1){
-                    return $this->security->isGranted('ROLE_ORGANIZATION')
-                        && $subject->getOrganization()->current()->getId() === $user->getOrganization()->getId();
-                }
-                break;
-        }
-
-        return false;
-    }
-
-
-}

+ 1 - 1
src/Security/Voter/CotisationVoter.php

@@ -6,9 +6,9 @@ namespace App\Security\Voter;
 use App\ApiResources\Cotisation\Cotisation;
 use App\Entity\Access\Access;
 use App\Service\Network\Utils as NetworkUtils;
+use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Authorization\Voter\Voter;
-use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\Security\Core\User\UserInterface;
 
 class CotisationVoter extends Voter

+ 16 - 10
src/Security/Voter/AbstractVoter.php → src/Security/Voter/EntityVoter/AbstractEntityVoter.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace App\Security\Voter;
+namespace App\Security\Voter\EntityVoter;
 
 use App\Entity\Access\Access;
 use App\Service\Access\Utils;
@@ -10,16 +10,15 @@ use Doctrine\ORM\EntityManagerInterface;
 use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Authorization\Voter\Voter;
-use Symfony\Component\Security\Core\User\UserInterface;
 
 /**
- * Base class for custom Voters.
+ * Base class for custom Voters
  *
  * This class also defines a default behavior for entity based voters (ex: FileVoter)
  *
  * @see doc/security.md
  */
-abstract class AbstractVoter extends Voter
+abstract class AbstractEntityVoter extends Voter
 {
     protected const READ = 'READ';
     protected const EDIT = 'EDIT';
@@ -50,7 +49,7 @@ abstract class AbstractVoter extends Voter
     ];
 
     public function __construct(
-        private Security $security,
+        protected Security $security,
         protected Utils $accessUtils,
         private InternalRequestsService $internalRequestsService,
         EntityManagerInterface $em,
@@ -127,14 +126,18 @@ abstract class AbstractVoter extends Voter
      * @param object $subject
      * @return bool
      */
-    abstract protected function canView(object $subject): bool;
+    protected function canView(object $subject): bool {
+        return false;
+    }
 
     /**
      * Does the client have the right to edit this resource?
      * @param object $subject
      * @return bool
      */
-    abstract protected function canEdit(object $subject): bool;
+    protected function canEdit(object $subject): bool {
+        return false;
+    }
 
     /**
      * Does the client have the right to create this resource?
@@ -142,7 +145,9 @@ abstract class AbstractVoter extends Voter
      * @param object $subject
      * @return bool
      */
-    abstract protected function canCreate(object $subject): bool;
+    protected function canCreate(object $subject): bool {
+        return false;
+    }
 
     /**
      * Does the client have the right to delete this resource?
@@ -150,7 +155,9 @@ abstract class AbstractVoter extends Voter
      * @param object $subject
      * @return bool
      */
-    abstract protected function canDelete(object $subject): bool;
+    protected function canDelete(object $subject): bool {
+        return false;
+    }
 
     /**
      * Returns the current logged in user
@@ -183,5 +190,4 @@ abstract class AbstractVoter extends Voter
 
         return $this->internalRequestsService->isAllowed($clientIp, $internalRequestToken);
     }
-
 }

+ 73 - 0
src/Security/Voter/EntityVoter/Core/BankAccountVoter.php

@@ -0,0 +1,73 @@
+<?php
+declare(strict_types=1);
+
+namespace App\Security\Voter\EntityVoter\Core;
+
+use App\Entity\Access\Access;
+use App\Entity\Core\BankAccount;
+use App\Security\Voter\EntityVoter\AbstractEntityVoter;
+use Symfony\Bundle\SecurityBundle\Security;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
+use Symfony\Component\Security\Core\User\UserInterface;
+
+/**
+ * Contrôle l'accès à l'entité BankAccount
+ */
+class BankAccountVoter extends AbstractEntityVoter
+{
+    /**
+     * @inheritdoc
+     */
+    protected static ?string $entityClass = BankAccount::class;
+
+    /**
+     * @inheritdoc
+     */
+    protected static array $allowedOperations = [
+        self::READ, self::EDIT, self::DELETE
+    ];
+
+    /**
+     * Can the user interact with the BankAccount, with the given required role
+     *
+     * @param string $role The role needed to interract with the BankAccount
+     * @return bool
+     */
+    private function canInteractIfHasRole(object $subject, string $role): bool {
+        return $this->isUserLoggedIn()
+            && $subject->getOrganization()->count() === 1
+            && $this->security->isGranted($role)
+            && $subject->getOrganization()->current()->getId() === $this->getUser()->getOrganization()->getId();
+    }
+
+    /**
+     * @inheritdoc
+     *
+     * @param $subject BankAccount
+     * @return boolean
+     */
+    protected function canView(object $subject): bool {
+        return $this->canInteractIfHasRole($subject, 'ROLE_ORGANIZATION_VIEW');
+    }
+
+    /**
+     * @inheritdoc
+     *
+     * @param $subject BankAccount
+     * @return boolean
+     */
+    protected function canEdit(object $subject): bool {
+        return $this->canInteractIfHasRole($subject, 'ROLE_ORGANIZATION');
+    }
+
+    /**
+     * @inheritdoc
+     *
+     * @param $subject BankAccount
+     * @return boolean
+     */
+    protected function canDelete(object $subject): bool {
+        return $this->canEdit($subject);
+    }
+}

+ 72 - 0
src/Security/Voter/EntityVoter/Core/ContactPointVoter.php

@@ -0,0 +1,72 @@
+<?php
+declare(strict_types=1);
+
+namespace App\Security\Voter\EntityVoter\Core;
+
+use App\Entity\Access\Access;
+use App\Entity\Core\BankAccount;
+use App\Entity\Core\ContactPoint;
+use App\Security\Voter\EntityVoter\AbstractEntityVoter;
+use Symfony\Bundle\SecurityBundle\Security;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
+use Symfony\Component\Security\Core\User\UserInterface;
+
+class ContactPointVoter extends AbstractEntityVoter
+{
+    /**
+     * @inheritdoc
+     */
+    protected static ?string $entityClass = ContactPoint::class;
+
+    /**
+     * @inheritdoc
+     */
+    protected static array $allowedOperations = [
+        self::READ, self::EDIT, self::DELETE
+    ];
+
+    /**
+     * Can the user interact with the ContactPoint with the given required role
+     *
+     * @param string $role The role needed to interract with the ContactPoint
+     * @return bool
+     */
+    private function canInteractIfHasRole(object $subject, string $role): bool {
+        return $this->isUserLoggedIn()
+            && $subject->getOrganization()->count() === 1
+            && $this->security->isGranted($role)
+            && $subject->getOrganization()->current()->getId() === $this->getUser()->getOrganization()->getId();
+    }
+
+    /**
+     * @inheritdoc
+     *
+     * @param $subject BankAccount
+     * @return boolean
+     */
+    protected function canView(object $subject): bool {
+        return $this->canInteractIfHasRole($subject, 'ROLE_ORGANIZATION_VIEW');
+    }
+
+    /**
+     * @inheritdoc
+     *
+     * @param $subject BankAccount
+     * @return boolean
+     */
+    protected function canEdit(object $subject): bool {
+        return $this->canInteractIfHasRole($subject, 'ROLE_ORGANIZATION');
+    }
+
+    /**
+     * @inheritdoc
+     *
+     * @param $subject BankAccount
+     * @return boolean
+     */
+    protected function canDelete(object $subject): bool {
+        return $this->canEdit($subject);
+    }
+
+}

+ 4 - 5
src/Security/Voter/Core/FileVoter.php → src/Security/Voter/EntityVoter/Core/FileVoter.php

@@ -1,20 +1,19 @@
 <?php
 declare(strict_types=1);
 
-namespace App\Security\Voter\Core;
+namespace App\Security\Voter\EntityVoter\Core;
 
 use App\Entity\Access\Access;
 use App\Entity\Core\File;
-use App\Enum\Core\FileVisibilityEnum;
 use App\Enum\Core\FileTypeEnum;
-use App\Security\Voter\AbstractVoter;
+use App\Enum\Core\FileVisibilityEnum;
+use App\Security\Voter\EntityVoter\AbstractEntityVoter;
 use App\Service\Utils\DatesUtils;
-use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 
 /**
  * Contrôle l'accès à l'entité File
  */
-class FileVoter extends AbstractVoter
+class FileVoter extends AbstractEntityVoter
 {
     /**
      * @inheritdoc

+ 26 - 26
tests/Unit/Security/Voter/AbstractVoterTest.php → tests/Unit/Security/Voter/EntityVoter/AbstractEntityVoterTest.php

@@ -4,7 +4,7 @@ namespace App\Tests\Unit\Security\Voter;
 
 use App\Entity\Access\Access;
 use App\Entity\Person\Person;
-use App\Security\Voter\AbstractVoter;
+use App\Security\Voter\EntityVoter\AbstractEntityVoter;
 use App\Service\Access\Utils;
 use App\Service\Security\InternalRequestsService;
 use App\Service\Security\SwitchUser;
@@ -14,7 +14,7 @@ use PHPUnit\Framework\TestCase;
 use Symfony\Bundle\SecurityBundle\Security;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 
-class TestableAbstractVoter extends AbstractVoter {
+class TestableAbstractEntityVoter extends AbstractEntityVoter {
     public const READ = 'READ';
     public const EDIT = 'EDIT';
     public const CREATE = 'CREATE';
@@ -77,8 +77,8 @@ class AbstractVoterTest extends TestCase
     private ?string $initialInternalRequestToken = null;
 
     private function saveInitialCondition() {
-        $this->initialEntityClass = TestableAbstractVoter::getEntityClass();
-        $this->initialAllowedOperations = TestableAbstractVoter::getAllowedOperations();
+        $this->initialEntityClass = TestableAbstractEntityVoter::getEntityClass();
+        $this->initialAllowedOperations = TestableAbstractEntityVoter::getAllowedOperations();
         $this->initialSwitchHeader = $_SERVER['HTTP_X_SWITCH_USER'] ?? null;
         $this->initialRemoteAddr = $_SERVER['REMOTE_ADDR'] ?? null;
         $this->initialInternalRequestToken = $_SERVER['HTTP_INTERNAL_REQUESTS_TOKEN'] ?? null;
@@ -87,8 +87,8 @@ class AbstractVoterTest extends TestCase
 
     private function reinitialize() {
         // Reinitialize the TestableAbstractVoter static properties
-        TestableAbstractVoter::setEntityClass($this->initialEntityClass);
-        TestableAbstractVoter::setAllowedOperations($this->initialAllowedOperations);
+        TestableAbstractEntityVoter::setEntityClass($this->initialEntityClass);
+        TestableAbstractEntityVoter::setAllowedOperations($this->initialAllowedOperations);
 
         // Reinitialize the global variables
         if ($this->initialSwitchHeader !== null) {
@@ -123,8 +123,8 @@ class AbstractVoterTest extends TestCase
         $this->reinitialize();
     }
 
-    private function makeAbstractVoterMockFor(string $methodName): MockObject | TestableAbstractVoter {
-        return $this->getMockBuilder(TestableAbstractVoter::class)
+    private function makeAbstractVoterMockFor(string $methodName): MockObject | TestableAbstractEntityVoter {
+        return $this->getMockBuilder(TestableAbstractEntityVoter::class)
             ->setConstructorArgs([
                 $this->security,
                 $this->accessUtils,
@@ -137,57 +137,57 @@ class AbstractVoterTest extends TestCase
     }
 
     public function testSupports(): void {
-        TestableAbstractVoter::setEntityClass(Access::class);
+        TestableAbstractEntityVoter::setEntityClass(Access::class);
 
         $abstractVoter = $this->makeAbstractVoterMockFor('supports');
 
         $access = new Access();
 
         $this->assertTrue(
-            $abstractVoter->supports(TestableAbstractVoter::READ, $access)
+            $abstractVoter->supports(TestableAbstractEntityVoter::READ, $access)
         );
         $this->assertTrue(
-            $abstractVoter->supports(TestableAbstractVoter::EDIT, $access)
+            $abstractVoter->supports(TestableAbstractEntityVoter::EDIT, $access)
         );
         $this->assertTrue(
-            $abstractVoter->supports(TestableAbstractVoter::CREATE, $access)
+            $abstractVoter->supports(TestableAbstractEntityVoter::CREATE, $access)
         );
         $this->assertTrue(
-            $abstractVoter->supports(TestableAbstractVoter::DELETE, $access)
+            $abstractVoter->supports(TestableAbstractEntityVoter::DELETE, $access)
         );
     }
 
     public function testSupportsNotSupportedClass(): void {
-        TestableAbstractVoter::setEntityClass(Access::class);
+        TestableAbstractEntityVoter::setEntityClass(Access::class);
 
         $abstractVoter = $this->makeAbstractVoterMockFor('supports');
 
         $person = new Person();
 
         $this->assertFalse(
-            $abstractVoter->supports(TestableAbstractVoter::READ, $person)
+            $abstractVoter->supports(TestableAbstractEntityVoter::READ, $person)
         );
     }
 
     public function testSupportsNotAllowedOperation(): void {
-        TestableAbstractVoter::setEntityClass(Access::class);
-        TestableAbstractVoter::setAllowedOperations([TestableAbstractVoter::READ]);
+        TestableAbstractEntityVoter::setEntityClass(Access::class);
+        TestableAbstractEntityVoter::setAllowedOperations([TestableAbstractEntityVoter::READ]);
 
         $abstractVoter = $this->makeAbstractVoterMockFor('supports');
 
         $access = new Access();
 
         $this->assertTrue(
-            $abstractVoter->supports(TestableAbstractVoter::READ, $access)
+            $abstractVoter->supports(TestableAbstractEntityVoter::READ, $access)
         );
         $this->assertFalse(
-            $abstractVoter->supports(TestableAbstractVoter::EDIT, $access)
+            $abstractVoter->supports(TestableAbstractEntityVoter::EDIT, $access)
         );
         $this->assertFalse(
-            $abstractVoter->supports(TestableAbstractVoter::CREATE, $access)
+            $abstractVoter->supports(TestableAbstractEntityVoter::CREATE, $access)
         );
         $this->assertFalse(
-            $abstractVoter->supports(TestableAbstractVoter::DELETE, $access)
+            $abstractVoter->supports(TestableAbstractEntityVoter::DELETE, $access)
         );
     }
 
@@ -198,7 +198,7 @@ class AbstractVoterTest extends TestCase
         $this->expectExceptionMessage('Setup the self::$entityClass property, or override the supports() method');
 
         $access = new Access();
-        $abstractVoter->supports(TestableAbstractVoter::READ, $access);
+        $abstractVoter->supports(TestableAbstractEntityVoter::READ, $access);
     }
 
     public function testVoteOnAttributeRead(): void {
@@ -213,7 +213,7 @@ class AbstractVoterTest extends TestCase
         $abstractVoter->expects(self::never())->method('canDelete');
 
         $this->assertTrue(
-            $abstractVoter->voteOnAttribute(TestableAbstractVoter::READ, $subject, $token)
+            $abstractVoter->voteOnAttribute(TestableAbstractEntityVoter::READ, $subject, $token)
         );
     }
 
@@ -229,7 +229,7 @@ class AbstractVoterTest extends TestCase
         $abstractVoter->expects(self::never())->method('canDelete');
 
         $this->assertTrue(
-            $abstractVoter->voteOnAttribute(TestableAbstractVoter::EDIT, $subject, $token)
+            $abstractVoter->voteOnAttribute(TestableAbstractEntityVoter::EDIT, $subject, $token)
         );
     }
 
@@ -245,7 +245,7 @@ class AbstractVoterTest extends TestCase
         $abstractVoter->expects(self::never())->method('canDelete');
 
         $this->assertTrue(
-            $abstractVoter->voteOnAttribute(TestableAbstractVoter::CREATE, $subject, $token)
+            $abstractVoter->voteOnAttribute(TestableAbstractEntityVoter::CREATE, $subject, $token)
         );
     }
 
@@ -261,7 +261,7 @@ class AbstractVoterTest extends TestCase
         $abstractVoter->expects(self::once())->method('canDelete')->with($subject)->willReturn(true);
 
         $this->assertTrue(
-            $abstractVoter->voteOnAttribute(TestableAbstractVoter::DELETE, $subject, $token)
+            $abstractVoter->voteOnAttribute(TestableAbstractEntityVoter::DELETE, $subject, $token)
         );
     }
 

+ 1 - 3
tests/Unit/Security/Voter/Core/FileVoterTest.php → tests/Unit/Security/Voter/EntityVoter/Core/FileVoterTest.php

@@ -8,17 +8,15 @@ use App\Entity\Organization\Organization;
 use App\Entity\Person\Person;
 use App\Enum\Core\FileTypeEnum;
 use App\Enum\Core\FileVisibilityEnum;
-use App\Security\Voter\Core\FileVoter;
+use App\Security\Voter\EntityVoter\Core\FileVoter;
 use App\Service\Access\Utils;
 use App\Service\Security\InternalRequestsService;
 use App\Service\Security\SwitchUser;
 use App\Service\Utils\DatesUtils;
-use App\Tests\Unit\Security\Voter\TestableAbstractVoter;
 use Doctrine\Common\Collections\ArrayCollection;
 use Doctrine\ORM\EntityManagerInterface;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
-use Ramsey\Collection\Collection;
 use Symfony\Bundle\SecurityBundle\Security;
 
 class TestableFileVoter extends FileVoter {