feat: permission fixes, improve permission tests, facilitate check fo…

…r last op admin user VOL-4718 VOL-5796 (#458)

app/api/module/Api/src/Domain/CommandHandler/User/DeleteUserSelfserve.php

@@ -10,14 +10,12 @@
 use Dvsa\Olcs\Api\Domain\CommandHandler\AbstractCommandHandler;
 use Dvsa\Olcs\Api\Domain\CommandHandler\TransactionedInterface;
 use Dvsa\Olcs\Api\Domain\Exception\BadRequestException;
+use Dvsa\Olcs\Api\Entity\Organisation\Organisation;
 use Dvsa\Olcs\Auth\Exception\DeleteUserException;
 use Dvsa\Olcs\Transfer\Command\CommandInterface;
 use Dvsa\Olcs\Transfer\Result\Auth\DeleteUserResult;
 use Laminas\Authentication\Adapter\ValidatableAdapterInterface;
 
-/**
- * Delete User Selfserve
- */
 final class DeleteUserSelfserve extends AbstractCommandHandler implements
     TransactionedInterface,
     AuthAwareInterface,
@@ -26,6 +24,8 @@ final class DeleteUserSelfserve extends AbstractCommandHandler implements
     use AuthAwareTrait;
     use CacheAwareTrait;
 
+    public const ADMIN_ROLE_ERROR = 'error-always-one-operator-admin';
+
     protected $repoServiceName = 'User';
 
     protected $extraRepos = ['OrganisationUser'];
@@ -52,7 +52,13 @@ public function handleCommand(CommandInterface $command)
         /** @var \Dvsa\Olcs\Api\Entity\User\User $user */
         $user = $this->getRepo()->fetchUsingId($command);
 
-        $this->getRepo('OrganisationUser')->deleteByUserId($user->getId());
+        if ($user->isLastOperatorAdmin()) {
+            throw new BadRequestException(self::ADMIN_ROLE_ERROR);
+        }
+
+        $userId = $user->getId();
+
+        $this->getRepo('OrganisationUser')->deleteByUserId($userId);
         $this->getRepo()->delete($user);
 
         $cognitoDeleteResult = $this->adapter->deleteUser($user->getLoginId());
@@ -62,8 +68,13 @@ public function handleCommand(CommandInterface $command)
             throw new DeleteUserException('Could not delete user');
         }
 
-        $userId = $user->getId();
-        $this->clearUserCaches([$userId]);
+        $organisation = $user->getRelatedOrganisation();
+
+        if ($organisation instanceof Organisation) {
+            $this->clearOrganisationCaches($organisation);
+        } else {
+            $this->clearUserCaches([$userId]);
+        }
 
         $result = new Result();
         $result->addId('user', $userId);

app/api/module/Api/src/Domain/CommandHandler/User/UpdateUserSelfserve.php

@@ -1,9 +1,5 @@
 <?php
 
-/**
- * Update User Selfserve
- */
-
 namespace Dvsa\Olcs\Api\Domain\CommandHandler\User;
 
 use Dvsa\Olcs\Api\Domain\CacheAwareInterface;
@@ -13,20 +9,17 @@
 use Dvsa\Olcs\Api\Domain\CommandHandler\TransactionedInterface;
 use Dvsa\Olcs\Api\Domain\ConfigAwareInterface;
 use Dvsa\Olcs\Api\Domain\ConfigAwareTrait;
+use Dvsa\Olcs\Api\Domain\Exception\BadRequestException;
 use Dvsa\Olcs\Api\Entity\ContactDetails\ContactDetails;
+use Dvsa\Olcs\Api\Entity\Organisation\Organisation;
 use Dvsa\Olcs\Api\Entity\User\User;
 use Dvsa\Olcs\Api\Service\EventHistory\Creator as EventHistoryCreator;
 use Dvsa\Olcs\Api\Entity\EventHistory\EventHistoryType as EventHistoryTypeEntity;
-use Dvsa\Olcs\Auth\Adapter\CognitoAdapter;
 use Dvsa\Olcs\Auth\Service\PasswordService;
 use Dvsa\Olcs\Transfer\Command\CommandInterface;
 use Doctrine\ORM\Query;
 use Laminas\Authentication\Adapter\ValidatableAdapterInterface;
-use Laminas\ServiceManager\ServiceLocatorInterface;
 
-/**
- * Update User Selfserve
- */
 final class UpdateUserSelfserve extends AbstractUserCommandHandler implements
     TransactionedInterface,
     CacheAwareInterface,
@@ -35,6 +28,8 @@ final class UpdateUserSelfserve extends AbstractUserCommandHandler implements
     use CacheAwareTrait;
     use ConfigAwareTrait;
 
+    public const ADMIN_ROLE_ERROR = 'error-always-one-operator-admin';
+
     protected $repoServiceName = 'User';
 
     protected $extraRepos = ['ContactDetails'];
@@ -65,6 +60,10 @@ public function handleCommand(CommandInterface $command)
 
         $data = $command->getArrayCopy();
 
+        if ($data['permission'] !== User::PERMISSION_ADMIN && $user->isLastOperatorAdmin()) {
+            throw new BadRequestException(self::ADMIN_ROLE_ERROR);
+        }
+
         // populate roles based on the user type and permission
         $data['roles'] = User::getRolesByUserType($user->getUserType(), $data['permission']);
 
@@ -108,7 +107,14 @@ public function handleCommand(CommandInterface $command)
         $result->addId('user', $userId);
         $result->addMessage('User updated successfully');
 
-        $this->clearUserCaches([$userId]);
+        $organisation = $user->getRelatedOrganisation();
+        $userId = $user->getId();
+
+        if ($organisation instanceof Organisation) {
+            $this->clearOrganisationCaches($organisation);
+        } else {
+            $this->clearUserCaches([$userId]);
+        }
 
         return $result;
     }

app/api/module/Api/src/Domain/QueryHandler/MyAccount/MyAccount.php

@@ -54,6 +54,7 @@ public function handleQuery(QueryInterface $query)
         $isEligibleForPrompt = false;
         $hasActivePsv = false;
         $hasSubmittedLicenceApplication = false;
+        $canDeleteOperatorAdmin = false;
         $numVehicles = 0;
 
         $dataAccess = [];
@@ -79,6 +80,7 @@ public function handleQuery(QueryInterface $query)
             ];
         } elseif ($userId !== User::USER_TYPE_ANON) {
             $isEligibleForPermits = $user->isEligibleForPermits();
+            $canDeleteOperatorAdmin = $user->organisationCanDeleteOperatorAdmin();
 
             if ($isEligibleForPermits) {
                 //for now we need to leave this, as selfserve users don't have access to system param query
@@ -118,6 +120,7 @@ public function handleQuery(QueryInterface $query)
                 'eligibleForPrompt' => $isEligibleForPrompt,
                 'dataAccess' => $dataAccess,
                 'isInternal' => $isInternal,
+                'canDeleteOperatorAdmin' => $canDeleteOperatorAdmin,
             ]
         );
 

app/api/module/Api/src/Entity/Organisation/Organisation.php

@@ -529,6 +529,28 @@ public function getAdministratorUsers()
         return $this->organisationUsers->matching($criteria);
     }
 
+    /**
+     * can only delete operator admin if more than one exists
+     */
+    public function canDeleteOperatorAdmin(): bool
+    {
+        $numOperatorAdmins = 0;
+        $administratorUsers = $this->getAdministratorUsers();
+
+        /** @var OrganisationUserEntity $orgUser */
+        foreach ($administratorUsers as $orgUser) {
+            if ($orgUser->getUser()->isOperatorAdministrator()) {
+                $numOperatorAdmins++;
+            }
+
+            if ($numOperatorAdmins > 1) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /**
      * Returns the latest Trading Name that hasnt been deleted
      *

app/api/module/Api/src/Entity/User/User.php

@@ -65,7 +65,6 @@ class User extends AbstractUser implements OrganisationProviderInterface
             RoleEntity::ROLE_OPERATOR_TM,
         ],
         self::USER_TYPE_TRANSPORT_MANAGER => [
-            RoleEntity::ROLE_OPERATOR_TC,
             RoleEntity::ROLE_OPERATOR_ADMIN,
             RoleEntity::ROLE_OPERATOR_USER,
             RoleEntity::ROLE_OPERATOR_TM,
@@ -104,7 +103,6 @@ class User extends AbstractUser implements OrganisationProviderInterface
             self::PERMISSION_TM => [RoleEntity::ROLE_OPERATOR_TM],
         ],
         self::USER_TYPE_TRANSPORT_MANAGER => [
-            self::PERMISSION_TC => [RoleEntity::ROLE_OPERATOR_TC],
             self::PERMISSION_ADMIN => [RoleEntity::ROLE_OPERATOR_ADMIN],
             self::PERMISSION_USER => [RoleEntity::ROLE_OPERATOR_USER],
             self::PERMISSION_TM => [RoleEntity::ROLE_OPERATOR_TM],
@@ -496,18 +494,32 @@ private function populateOrganisationUsers(array $orgs = null)
     }
 
     /**
-     * Checks whther the user is an administrator
-     *
-     * @return bool
+     * Checks whether the user is selfserve administrator
      */
-    private function isAdministrator()
+    public function isAdministrator(): bool
     {
         // is admin if has roles for admin permission
+        return $this->isOperatorAdministrator() || $this->isConsultantAdministrator();
+    }
+
+    /**
+     * Checks whether the user is aperator administrator
+     */
+    public function isOperatorAdministrator(): bool
+    {
         return $this->hasRoles(
             self::getRolesByUserType($this->getUserType(), self::PERMISSION_ADMIN)
-        ) || $this->hasRoles(
+        );
+    }
+
+    /**
+     * Checks whether the user is consultant administrator
+     */
+    public function isConsultantAdministrator(): bool
+    {
+        return $this->hasRoles(
             self::getRolesByUserType($this->getUserType(), self::PERMISSION_TC)
-            );
+        );
     }
 
     /**
@@ -777,4 +789,30 @@ public function hasAgreedTermsAndConditions(): bool
     {
         return $this->termsAgreed;
     }
+
+    public function isLastOperatorAdmin(): bool
+    {
+        if (!$this->isOperatorAdministrator()) {
+            return false;
+        }
+
+        $org = $this->getRelatedOrganisation();
+
+        if ($org instanceof Organisation) {
+            return !$this->getRelatedOrganisation()->canDeleteOperatorAdmin();
+        }
+
+        return false;
+    }
+
+    public function organisationCanDeleteOperatorAdmin(): bool
+    {
+        $org = $this->getRelatedOrganisation();
+
+        if ($org instanceof Organisation) {
+            return $this->getRelatedOrganisation()->canDeleteOperatorAdmin();
+        }
+
+        return false;
+    }
 }

app/api/test/module/Api/src/Domain/CommandHandler/User/DeleteUserSelfserveTest.php

@@ -7,6 +7,7 @@
 use Dvsa\Olcs\Api\Domain\CommandHandler\User\DeleteUserSelfserve;
 use Dvsa\Olcs\Api\Domain\Exception\BadRequestException;
 use Dvsa\Olcs\Api\Domain\Repository;
+use Dvsa\Olcs\Api\Entity\Organisation\Organisation;
 use Dvsa\Olcs\Api\Entity\User\User as UserEntity;
 use Dvsa\Olcs\Auth\Adapter\CognitoAdapter;
 use Dvsa\Olcs\Auth\Exception\DeleteUserException;
@@ -17,9 +18,6 @@
 use LmcRbacMvc\Service\AuthorizationService;
 use Mockery as m;
 
-/**
- * Class Delete User Selfserve Test
- */
 class DeleteUserSelfserveTest extends AbstractCommandHandlerTestCase
 {
     public const USER_ID = 8888;
@@ -55,17 +53,19 @@ public function setUp(): void
         parent::setUp();
     }
 
-    public function testHandleCommand(): void
+    public function testHandleCommandNoOrganisation(): void
     {
         $data = [
             'id' => self::USER_ID,
         ];
 
         $command = Cmd::create($data);
 
-        $userEntity = new UserEntity(self::USER_ID, UserEntity::USER_TYPE_OPERATOR);
-        $userEntity->setId(self::USER_ID);
-        $userEntity->setLoginId(self::LOGIN_ID);
+        $userEntity = m::mock(UserEntity::class);
+        $userEntity->expects('getId')->withNoArgs()->andReturn(self::USER_ID);
+        $userEntity->expects('getLoginId')->withNoArgs()->andReturn(self::LOGIN_ID);
+        $userEntity->expects('isLastOperatorAdmin')->withNoArgs()->andReturnFalse();
+        $userEntity->expects('getRelatedOrganisation')->withNoArgs()->andReturnNull();
 
         $cognitoResult = m::mock(DeleteUserResult::class);
         $cognitoResult->expects('isValid')->withNoArgs()->andReturnTrue();
@@ -93,6 +93,67 @@ public function testHandleCommand(): void
         $this->assertEquals($expected, $result->toArray());
     }
 
+    public function testHandleCommandWithOrganisation(): void
+    {
+        $data = [
+            'id' => self::USER_ID,
+        ];
+
+        $command = Cmd::create($data);
+
+        $organisation = m::mock(Organisation::class);
+
+        $userEntity = m::mock(UserEntity::class);
+        $userEntity->expects('getId')->withNoArgs()->andReturn(self::USER_ID);
+        $userEntity->expects('getLoginId')->withNoArgs()->andReturn(self::LOGIN_ID);
+        $userEntity->expects('isLastOperatorAdmin')->withNoArgs()->andReturnFalse();
+        $userEntity->expects('getRelatedOrganisation')->withNoArgs()->andReturn($organisation);
+
+        $cognitoResult = m::mock(DeleteUserResult::class);
+        $cognitoResult->expects('isValid')->withNoArgs()->andReturnTrue();
+        $this->adapter->expects('deleteUser')->with(self::LOGIN_ID)->andReturn($cognitoResult);
+
+        $this->repoMap['User']
+            ->shouldReceive('fetchUsingId')->once()->andReturn($userEntity)
+            ->shouldReceive('delete')->once();
+
+        $this->repoMap['OrganisationUser']
+            ->shouldReceive('deleteByUserId')->with(self::USER_ID)->once();
+
+        $this->expectedOrganisationCacheClear($organisation);
+        $result = $this->sut->handleCommand($command);
+
+        $expected = [
+            'id' => [
+                'user' => self::USER_ID,
+            ],
+            'messages' => [
+                'User deleted successfully'
+            ]
+        ];
+
+        $this->assertEquals($expected, $result->toArray());
+    }
+
+    public function testHandleCommandLastOperatorAdmin(): void
+    {
+        $this->expectException(BadRequestException::class);
+        $this->expectExceptionMessage(DeleteUserSelfserve::ADMIN_ROLE_ERROR);
+
+        $data = [
+            'id' => self::USER_ID,
+        ];
+
+        $command = Cmd::create($data);
+
+        $userEntity = m::mock(UserEntity::class);
+        $userEntity->expects('isLastOperatorAdmin')->withNoArgs()->andReturnTrue();
+
+        $this->repoMap['User']->expects('fetchUsingId')->with($command)->andReturn($userEntity);
+
+        $this->sut->handleCommand($command);
+    }
+
     public function testHandleCommandCognitoError(): void
     {
         $this->expectException(DeleteUserException::class);
@@ -106,6 +167,7 @@ public function testHandleCommandCognitoError(): void
         $userEntity = m::mock(UserEntity::class);
         $userEntity->expects('getId')->withNoArgs()->andReturn(self::USER_ID);
         $userEntity->expects('getLoginId')->withNoArgs()->andReturn(self::LOGIN_ID);
+        $userEntity->expects('isLastOperatorAdmin')->withNoArgs()->andReturnFalse();
 
         $cognitoResult = m::mock(DeleteUserResult::class);
         $cognitoResult->expects('isValid')->withNoArgs()->andReturnFalse();