imp: Allow and combine multiple authorizations

docs/developers/roles.md

@@ -40,15 +40,10 @@ It's called the scope of the authorization.
 The scope is only relevant to the Agent and User roles.
 Indeed, the Administrator roles give permissions outside of the organizations.
 
-## Authorizations conflicts
+## Multiple authorizations
 
-A user cannot have two authorizations concerning the same scope.
-Bileto refuses to give two authorizations concerning the same organization to a user.
-
-However, two scopes can be in conflict (e.g. an authorization with a global scope, and another one associated to an organization).
-When the scope of two authorizations conflict with each other, it's the most specific one which applies (organization > global).
-
-Also, only one administrator role can be given to a user.
+A user can have more than one authorization concerning the same scope.
+In this case, Bileto combines the permissions given by the different authorizations.
 
 ## The “Super” role
 
@@ -84,7 +79,7 @@ Only the method `getRoles()` of the `User` entity returns a "Symfony role" becau
 Documentation: [symfony.com](https://symfony.com/doc/current/security/voters.html)
 
 Bileto has a unique Voter to check the permission of a user: [`AppVoter`](/src/Security/AppVoter.php).
-It loads the applicable user authorization and related role, then it checks that the role includes the current checked permission.
+It loads the applicable user authorizations and related roles, then it checks that at least one role includes the current checked permission.
 
 #### How to check the permissions
 
@@ -126,13 +121,7 @@ In templates:
 {% endif %}
 ```
 
-You can check that a permission is given by a global authorization:
-
-```php
-$this->denyAccessUnlessGranted('orga:see', 'global');
-```
-
-Or that a permission is given at least by one authorization:
+You can check that a permission is given at least by one authorization:
 
 ```php
 $this->denyAccessUnlessGranted('orga:see', 'any');

src/Command/SeedsCommand.php

@@ -162,19 +162,19 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             // Make sure that the users exist for the grant() method.
             $this->entityManager->flush();
 
-            if (!$this->authorizationRepository->getAdminAuthorizationFor($userAlix)) {
+            if (empty($this->authorizationRepository->getAdminAuthorizationsFor($userAlix))) {
                 $this->authorizationRepository->grant($userAlix, $roleSuper);
             }
 
-            if (!$this->authorizationRepository->getOrgaAuthorizationFor($userAlix, null)) {
+            if (empty($this->authorizationRepository->getOrgaAuthorizationsFor($userAlix, $orgaFriendlyCoorp))) {
                 $this->authorizationRepository->grant($userAlix, $roleTech, null);
             }
 
-            if (!$this->authorizationRepository->getOrgaAuthorizationFor($userBenedict, null)) {
+            if (empty($this->authorizationRepository->getOrgaAuthorizationsFor($userBenedict, $orgaFriendlyCoorp))) {
                 $this->authorizationRepository->grant($userBenedict, $roleSalesman, null);
             }
 
-            if (!$this->authorizationRepository->getOrgaAuthorizationFor($userCharlie, $orgaFriendlyCoorp)) {
+            if (empty($this->authorizationRepository->getOrgaAuthorizationsFor($userCharlie, $orgaFriendlyCoorp))) {
                 $this->authorizationRepository->grant($userCharlie, $roleUser, $orgaFriendlyCoorp);
             }
 

src/Controller/Users/AuthorizationsController.php

@@ -161,34 +161,6 @@ public function create(
             ]);
         }
 
-        if ($role->getType() === 'admin' || $role->getType() === 'super') {
-            $existingRole = $authorizationRepository->getAdminAuthorizationFor($holder);
-            if ($existingRole) {
-                return $this->renderBadRequest('users/authorizations/new.html.twig', [
-                    'organizations' => $organizations,
-                    'roles' => $roles,
-                    'user' => $holder,
-                    'type' => $type,
-                    'roleUid' => $roleUid,
-                    'organizationUid' => $organizationUid,
-                    'error' => $translator->trans('authorization.user.already_admin', [], 'errors'),
-                ]);
-            }
-        } else {
-            $existingRole = $authorizationRepository->getOrgaAuthorizationFor($holder, $organization);
-            if ($existingRole && $existingRole->getOrganization() === $organization) {
-                return $this->renderBadRequest('users/authorizations/new.html.twig', [
-                    'organizations' => $organizations,
-                    'roles' => $roles,
-                    'user' => $holder,
-                    'type' => $type,
-                    'roleUid' => $roleUid,
-                    'organizationUid' => $organizationUid,
-                    'error' => $translator->trans('authorization.user.already_orga', [], 'errors'),
-                ]);
-            }
-        }
-
         $authorizationRepository->grant($holder, $role, $organization);
 
         return $this->redirectToRoute('user authorizations', [

src/Repository/AuthorizationRepository.php

@@ -60,7 +60,10 @@ public function grant(User $user, Role $role, ?Organization $organization = null
         $this->save($authorization, true);
     }
 
-    public function getAdminAuthorizationFor(User $user): ?Authorization
+    /**
+     * @return Authorization[]
+     */
+    public function getAdminAuthorizationsFor(User $user): array
     {
         $entityManager = $this->getEntityManager();
 
@@ -73,52 +76,27 @@ public function getAdminAuthorizationFor(User $user): ?Authorization
         SQL);
         $query->setParameter('user', $user);
 
-        return $query->getOneOrNullResult();
+        return $query->getResult();
     }
 
-    public function getOrgaAuthorizationFor(User $user, ?Organization $organization): ?Authorization
+    /**
+     * @return Authorization[]
+     */
+    public function getOrgaAuthorizationsFor(User $user, Organization $organization): array
     {
         $entityManager = $this->getEntityManager();
 
-        if ($organization) {
-            $query = $entityManager->createQuery(<<<SQL
-                SELECT a, r
-                FROM App\Entity\Authorization a
-                INDEX BY a.organization
-                JOIN a.role r
-                WHERE a.holder = :user
-                AND (a.organization = :organization OR a.organization IS NULL)
-                AND (r.type = 'user' OR r.type = 'agent')
-            SQL);
-            $query->setParameter('user', $user);
-            $query->setParameter('organization', $organization);
-
-            $authorizationsIndexByOrgaIds = $query->getResult();
-            if (count($authorizationsIndexByOrgaIds) === 0) {
-                // no authorization? too bad
-                return null;
-            } elseif (count($authorizationsIndexByOrgaIds) === 1) {
-                // There is only one authorization given for this organization.
-                return array_pop($authorizationsIndexByOrgaIds);
-            } else {
-                // There are several authorizations given (i.e. a global and a
-                // scoped), return only the scoped one as it's more specific.
-                unset($authorizationsIndexByOrgaIds[null]);
-                return array_pop($authorizationsIndexByOrgaIds);
-            }
-        } else {
-            $query = $entityManager->createQuery(<<<SQL
-                SELECT a, r
-                FROM App\Entity\Authorization a
-                INDEX BY a.organization
-                JOIN a.role r
-                WHERE a.holder = :user
-                AND a.organization IS NULL
-                AND (r.type = 'user' OR r.type = 'agent')
-            SQL);
-            $query->setParameter('user', $user);
-
-            return $query->getOneOrNullResult();
-        }
+        $query = $entityManager->createQuery(<<<SQL
+            SELECT a, r
+            FROM App\Entity\Authorization a
+            JOIN a.role r
+            WHERE a.holder = :user
+            AND (a.organization = :organization OR a.organization IS NULL)
+            AND (r.type = 'user' OR r.type = 'agent')
+        SQL);
+        $query->setParameter('user', $user);
+        $query->setParameter('organization', $organization);
+
+        return $query->getResult();
     }
 }

src/Security/AppVoter.php

@@ -30,7 +30,6 @@ protected function supports(string $attribute, mixed $subject): bool
             (str_starts_with($attribute, 'admin:') && $subject === null) ||
             (str_starts_with($attribute, 'orga:') && (
                 $subject instanceof Organization ||
-                $subject === 'global' ||
                 $subject === 'any'
             ))
         );
@@ -46,35 +45,21 @@ protected function voteOnAttribute(string $attribute, mixed $subject, TokenInter
         }
 
         if (str_starts_with($attribute, 'orga:') && $subject === 'any') {
-            $authorizations = $this->authorizationRepo->findBy([
-                'holder' => $user,
-            ]);
-
-            foreach ($authorizations as $authorization) {
-                if ($authorization->getRole()->hasPermission($attribute)) {
-                    return true;
-                }
-            }
-
-            return false;
-        }
-
-        if (str_starts_with($attribute, 'orga:') && $subject === 'global') {
-            $authorization = $this->authorizationRepo->getOrgaAuthorizationFor($user, null);
+            $authorizations = $this->authorizationRepo->findBy(['holder' => $user]);
         } elseif (str_starts_with($attribute, 'orga:') && $subject instanceof Organization) {
-            /** @var Organization $organization */
-            $organization = $subject;
-            $authorization = $this->authorizationRepo->getOrgaAuthorizationFor($user, $organization);
+            $authorizations = $this->authorizationRepo->getOrgaAuthorizationsFor($user, $subject);
         } elseif (str_starts_with($attribute, 'admin:')) {
-            $authorization = $this->authorizationRepo->getAdminAuthorizationFor($user);
+            $authorizations = $this->authorizationRepo->getAdminAuthorizationsFor($user);
         } else {
-            $authorization = null;
+            $authorizations = [];
         }
 
-        if (!$authorization) {
-            return false;
+        foreach ($authorizations as $authorization) {
+            if ($authorization->getRole()->hasPermission($attribute)) {
+                return true;
+            }
         }
 
-        return $authorization->getRole()->hasPermission($attribute);
+        return false;
     }
 }

tests/Controller/Users/AuthorizationsControllerTest.php

@@ -261,53 +261,6 @@ public function testPostCreateFailsIfSuperRoleAndNotCorrectAuthorization(): void
         $this->assertSame(1, AuthorizationFactory::count());
     }
 
-    public function testPostCreateFailsIfUserHasAlreadyAnAdminRole(): void
-    {
-        $client = static::createClient();
-        $user = UserFactory::createOne();
-        $client->loginUser($user->object());
-        $this->grantAdmin($user->object(), ['admin:manage:users']);
-        $role = RoleFactory::createOne([
-            'type' => 'admin',
-        ]);
-
-        $client->request('POST', "/users/{$user->getUid()}/authorizations/new", [
-            '_csrf_token' => $this->generateCsrfToken($client, 'create user authorization'),
-            'role' => $role->getUid(),
-        ]);
-
-        $this->assertSelectorTextContains(
-            '[data-test="alert-error"]',
-            'You cannot grant another admin role to this user',
-        );
-        $this->assertSame(1, AuthorizationFactory::count());
-    }
-
-    public function testPostCreateFailsIfUserHasAlreadyAnOrgaRoleOnTheGivenOrganization(): void
-    {
-        $client = static::createClient();
-        $user = UserFactory::createOne();
-        $client->loginUser($user->object());
-        $this->grantAdmin($user->object(), ['admin:manage:users']);
-        $role = RoleFactory::createOne([
-            'type' => 'agent',
-        ]);
-        $organization = OrganizationFactory::createOne();
-        $this->grantOrga($user->object(), ['orga:see'], $organization->object());
-
-        $client->request('POST', "/users/{$user->getUid()}/authorizations/new", [
-            '_csrf_token' => $this->generateCsrfToken($client, 'create user authorization'),
-            'role' => $role->getUid(),
-            'organization' => $organization->getUid(),
-        ]);
-
-        $this->assertSelectorTextContains(
-            '[data-test="alert-error"]',
-            'You cannot grant another role to this user in this organization'
-        );
-        $this->assertSame(2, AuthorizationFactory::count());
-    }
-
     public function testPostCreateFailsIfCsrfTokenIsInvalid(): void
     {
         $client = static::createClient();

tests/SearchEngine/TicketSearcherTest.php

@@ -186,30 +186,6 @@ public function testGetTicketsCanRestrictToAGivenContract(): void
         $this->assertSame($ticket1->getId(), $ticketsPagination->items[0]->getId());
     }
 
-    public function testGetTicketsTakesCareOfSpecificPermissions(): void
-    {
-        $client = static::createClient();
-        $container = static::getContainer();
-        /** @var TicketSearcher $ticketSearcher */
-        $ticketSearcher = $container->get(TicketSearcher::class);
-        $user = UserFactory::createOne();
-        $client->loginUser($user->object());
-        $organization = OrganizationFactory::createOne();
-        // orga:see:tickets:all is applied globally (i.e. to all organizations)
-        $this->grantOrga($user->object(), ['orga:see:tickets:all']);
-        // but we re-specify a specific role without this permission for this
-        // specific organization
-        $this->grantOrga($user->object(), ['orga:see'], $organization->object());
-        $ticket = TicketFactory::createOne([
-            'organization' => $organization,
-        ]);
-        $ticketSearcher->setOrganization($organization->object());
-
-        $ticketsPagination = $ticketSearcher->getTickets();
-
-        $this->assertSame(0, $ticketsPagination->count);
-    }
-
     public function testGetTicketsReturnsTicketMatchingAQuery(): void
     {
         $client = static::createClient();

translations/errors+intl-icu.en_GB.yaml

@@ -1,8 +1,6 @@
 authorization.cannot_revoke: 'You are not allowed to revoke this super-admin authorization.'
 authorization.role.invalid: 'Select a role from the list.'
 authorization.super.unauthorized: 'You are not allowed to grant super-admin authorization.'
-authorization.user.already_admin: 'You cannot grant another admin role to this user.'
-authorization.user.already_orga: 'You cannot grant another role to this user in this organization.'
 contract.end_at.greater_than_start_at: 'Enter a date greater than the start date.'
 contract.end_at.required: 'Enter an end date.'
 contract.max_hours.greater_than_zero: 'Enter a number of hours greater than zero.'

translations/errors+intl-icu.fr_FR.yaml

@@ -1,8 +1,6 @@
 authorization.cannot_revoke: 'Vous n’êtes pas autorisé à révoquer cette autorisation super-admin.'
 authorization.role.invalid: 'Sélectionnez un rôle de la liste.'
 authorization.super.unauthorized: 'Vous n’êtes pas autorisé à accorder cette autorisation super-admin.'
-authorization.user.already_admin: 'Vous ne pouvez pas accorder un autre rôle admin à cet utilisateur.'
-authorization.user.already_orga: 'Vous ne pouvez pas accorder un autre rôle à cet utilisateur dans cette organisation.'
 contract.end_at.greater_than_start_at: 'Saisissez une date plus grande que la date de début.'
 contract.end_at.required: 'Saisissez une date de fin.'
 contract.max_hours.greater_than_zero: 'Saisissez un nombre d’heures supérieur à zéro.'