Messaging access control (#109)

* fixup: Include permission check when building navigation; Improve logic * chore: Coding standards * chore: bump common version --------- Co-authored-by: Saul Wilcox <[email protected]>
dvsa · Mar 28, 2024 · 7c0981e · 7c0981e
1 parent 9d0ed4e
commit 7c0981e
Show file tree

Hide file tree

Showing 6 changed files with 54 additions and 88 deletions.
diff --git a/composer.lock b/composer.lock
diff --git a/module/Olcs/src/Controller/CorrespondenceController.php b/module/Olcs/src/Controller/CorrespondenceController.php
@@ -15,7 +15,7 @@
 /**
  * Class CorrespondenceController
  *
- * List an operators correspondence.
+ * List an operator's correspondence.
  *
  * @package Olcs\Controller
  *

diff --git a/module/Olcs/src/Controller/Listener/Navigation.php b/module/Olcs/src/Controller/Listener/Navigation.php
@@ -3,7 +3,9 @@
 namespace Olcs\Controller\Listener;
 
 use Common\FeatureToggle;
+use Common\Rbac\Service\Permission;
 use Common\Rbac\User as RbacUser;
+use Common\RefData;
 use Common\Service\Cqrs\Query\QuerySender;
 use Dvsa\Olcs\Transfer\Query\Messaging\Messages\UnreadCountByOrganisationAndUser;
 use Laminas\EventManager\EventManagerInterface;
@@ -13,6 +15,7 @@
 use Laminas\Http\PhpEnvironment\Request as HttpRequest;
 use Laminas\Mvc\MvcEvent;
 use Laminas\Navigation\Navigation as LaminasNavigation;
+use LmcRbacMvc\Service\AuthorizationService;
 
 /**
  * Class Navigation
@@ -32,10 +35,7 @@ class Navigation implements ListenerAggregateInterface
      */
     protected $querySender;
 
-    /**
-     * @var RbacUser $identity
-     */
-    protected $identity;
+    protected AuthorizationService $authService;
 
     /**
      * @var array
@@ -58,11 +58,11 @@ class Navigation implements ListenerAggregateInterface
      *
      * @return void
      */
-    public function __construct(LaminasNavigation $navigation, QuerySender $querySender, RbacUser $identity)
+    public function __construct(LaminasNavigation $navigation, QuerySender $querySender, AuthorizationService $authService)
     {
         $this->navigation = $navigation;
         $this->querySender = $querySender;
-        $this->identity = $identity;
+        $this->authService = $authService;
     }
 
     /**
@@ -86,7 +86,7 @@ public function onDispatch(MvcEvent $e)
         $this->togglePermitsMenus($shouldShowPermitsTab);
 
         $shouldShowMessagesTab = $this->shouldShowMessagesTab();
-        $this->toggleMessagesTab($shouldShowMessagesTab);
+        $this->prepareMessagesTab($shouldShowMessagesTab);
     }
 
     /**
@@ -127,11 +127,11 @@ private function shouldShowPermitsTab(MvcEvent $e)
      */
     private function isEligibleForPermits(): bool
     {
-        if ($this->identity->isAnonymous()) {
+        if ($this->authService->getIdentity()->isAnonymous()) {
             return false;
         }
 
-        $response = $this->identity->getUserData();
+        $response = $this->authService->getIdentity()->getUserData();
         return $response['eligibleForPermits'] ?? false;
     }
 
@@ -159,25 +159,31 @@ private function referedFromGovUkPermits(MvcEvent $e): bool
     }
 
     /**
-     * Toggle Messaging menus
+     * Toggle Messaging menus and add count if needed
      *
      * @param bool $shouldShowMessagesTab whether to show messages tab
      *
      * @return void
      */
-    private function toggleMessagesTab(bool $shouldShowMessagesTab): void
+    private function prepareMessagesTab(bool $shouldShowMessagesTab): void
     {
         $this->navigation->findBy('id', 'dashboard-messaging')
             ->setVisible($shouldShowMessagesTab);
 
-        $this->addUnreadMessagingCount();
+        if ($shouldShowMessagesTab) {
+            $this->addUnreadMessagingCount();
+        }
     }
 
     private function shouldShowMessagesTab(): bool
     {
+        if (!$this->authService->isGranted(RefData::PERMISSION_CAN_LIST_CONVERSATIONS)) {
+            return false;
+        };
+
         $messagingToggleEnabled = $this->querySender->featuresEnabled([FeatureToggle::MESSAGING]);
 
-        $userData = $this->identity->getUserData();
+        $userData = $this->authService->getIdentity()->getUserData();
 
         $hasOrganisationSubmittedLicenceApplication = $userData['hasOrganisationSubmittedLicenceApplication'];
 
@@ -207,13 +213,13 @@ public function setGovUkReferers(array $govUkReferers): void
 
     public function getUnreadMessageCount(): int
     {
-        $userOrganisationId = $this->identity->getUserData()['organisationUsers'][0]['organisation']['id'];
+        $userOrganisationId = $this->authService->getIdentity()->getUserData()['organisationUsers'][0]['organisation']['id'];
 
         $unreadByOrganisation = $this->querySender->send(
             UnreadCountByOrganisationAndUser::create(
                 [
                     'organisation' => $userOrganisationId,
-                    'user' => $this->identity->getId(),
+                    'user' => $this->authService->getIdentity()->getId(),
                 ]
             )
         );

diff --git a/module/Olcs/src/Controller/Listener/NavigationFactory.php b/module/Olcs/src/Controller/Listener/NavigationFactory.php
@@ -21,7 +21,7 @@ public function __invoke(ContainerInterface $container, $requestedName, array $o
         return new Navigation(
             $container->get('navigation'),
             $container->get('QuerySender'),
-            $container->get(AuthorizationService::class)->getIdentity()
+            $container->get(AuthorizationService::class)
         );
     }
 }
diff --git a/test/Olcs/src/Controller/Listener/NavigationFactoryTest.php b/test/Olcs/src/Controller/Listener/NavigationFactoryTest.php
@@ -15,11 +15,9 @@ class NavigationFactoryTest extends m\Adapter\Phpunit\MockeryTestCase
 {
     public function testInvoke(): void
     {
-        $identity = m::mock(RbacUser::class);
         $navigation = m::mock(LaminasNavigation::class);
         $querySender = m::mock(QuerySender::class);
         $authService = m::mock(AuthorizationService::class);
-        $authService->shouldReceive('getIdentity')->once()->withNoArgs()->andReturn($identity);
 
         $mockSl = m::mock(ContainerInterface::class);
         $mockSl->shouldReceive('get')

diff --git a/test/Olcs/src/Controller/Listener/NavigationTest.php b/test/Olcs/src/Controller/Listener/NavigationTest.php
@@ -5,8 +5,10 @@
 use Common\Rbac\User as RbacUser;
 use Common\Service\Cqrs\Query\QuerySender;
 use Laminas\Navigation\Page\AbstractPage;
+use LmcRbacMvc\Service\AuthorizationService;
 use Olcs\Controller\Listener\Navigation as NavigationListener;
 use Mockery as m;
+use Common\RefData;
 use Laminas\Http\Header\Referer as HttpReferer;
 use Laminas\Http\PhpEnvironment\Request as HttpRequest;
 use Laminas\Navigation\Navigation;
@@ -37,8 +39,9 @@ public function setUp(): void
         $this->mockNavigation = m::mock(Navigation::class);
         $this->mockQuerySender = m::mock(QuerySender::class);
         $this->mockResponse = m::mock(CqrsResponse::class);
-        $this->mockIdentity = m::mock(RbacUser::class);
-        $this->sut = new NavigationListener($this->mockNavigation, $this->mockQuerySender, $this->mockIdentity);
+        $this->mockAuthService = m::mock(AuthorizationService::class);
+
+        $this->sut = new NavigationListener($this->mockNavigation, $this->mockQuerySender, $this->mockAuthService);
 
         $this->dashboardPermitsKey = 'dashboard-permits';
         $this->dashboardPermitsPage = new Uri();
@@ -64,10 +67,10 @@ public function testAttach(): void
 
     public function testOnDispatchWithNoReferalAnonymousUser(): void
     {
-        $this->mockIdentity->shouldReceive('isAnonymous')->once()->withNoArgs()->andReturn(true);
+        $this->mockAuthService->shouldReceive('getIdentity->isAnonymous')->once()->withNoArgs()->andReturn(true);
 
-        $this->mockIdentity->expects('getUserData')
-            ->twice()
+        $this->mockAuthService->expects('getIdentity->getUserData')
+            ->once()
             ->andReturn([
                 'hasOrganisationSubmittedLicenceApplication' => false,
                 'organisationUsers' => [
@@ -90,33 +93,15 @@ public function testOnDispatchWithNoReferalAnonymousUser(): void
 
         $this->mockNavigation
             ->shouldReceive('findBy')
-            ->twice()
-            ->with('id', $this->dashboardMessagingKey)
-            ->andReturn($this->dashboardMessagingPage);
-
-        $this->mockNavigation
-            ->shouldReceive('findBy')
-            ->with('id', $this->dashboardMenuKey)
             ->once()
-            ->andReturn($this->mockDashboardMenu);
-
-        $this->mockDashboardMenu
-            ->shouldReceive('findBy')
             ->with('id', $this->dashboardMessagingKey)
-            ->once()
             ->andReturn($this->dashboardMessagingPage);
 
-        $this->mockIdentity->shouldReceive('getId')
+        $this->mockAuthService->shouldReceive('isGranted')
+            ->with(RefData::PERMISSION_CAN_LIST_CONVERSATIONS)
             ->once()
             ->andReturn(1);
 
-        $this->mockQuerySender->shouldReceive('send')
-            ->once()
-            ->andReturn($this->mockResponse);
-
-        $this->mockResponse->shouldReceive('getResult')
-            ->once();
-
         $request = m::mock(HttpRequest::class);
         $request->shouldReceive('getHeader')->once()->with('referer')->andReturn(false);
 
@@ -130,22 +115,17 @@ public function testOnDispatchWithNoReferalAnonymousUser(): void
             false,
             $this->mockNavigation->findBy('id', $this->dashboardPermitsKey)->getVisible()
         );
-
-        $this->assertEquals(
-            false,
-            $this->mockNavigation->findBy('id', $this->dashboardMessagingKey)->getVisible()
-        );
     }
 
     /**
      * @dataProvider dpDispatchNoReferer
      */
     public function testOnDispatchWithNoReferal($eligibleForPermits): void
     {
-        $this->mockIdentity->shouldReceive('isAnonymous')->once()->withNoArgs()->andReturn(false);
+        $this->mockAuthService->shouldReceive('getIdentity->isAnonymous')->once()->withNoArgs()->andReturn(false);
 
-        $this->mockIdentity->expects('getUserData')
-            ->times(3)
+        $this->mockAuthService->expects('getIdentity->getUserData')
+            ->times(2)
             ->andReturn([
                 'eligibleForPermits' => $eligibleForPermits,
                 'hasOrganisationSubmittedLicenceApplication' => false,
@@ -202,8 +182,8 @@ public function testOnDispatchWithGovUkReferalMatch(): void
         $request = m::mock(HttpRequest::class);
         $request->shouldReceive('getHeader')->once()->with('referer')->andReturn($referer);
 
-        $this->mockIdentity->expects('getUserData')
-            ->twice()
+        $this->mockAuthService->expects('getIdentity->getUserData')
+            ->once()
             ->andReturn([
                 'hasOrganisationSubmittedLicenceApplication' => false,
                 'organisationUsers' => [
@@ -236,11 +216,11 @@ public function testOnDispatchWithGovUkReferalMatch(): void
      */
     public function testOnDispatchWithNoGovUkReferal($eligibleForPermits): void
     {
-        $this->mockIdentity->shouldReceive('isAnonymous')
+        $this->mockAuthService->shouldReceive('getIdentity->isAnonymous')
             ->andReturn(false);
 
-        $this->mockIdentity->expects('getUserData')
-            ->times(3)
+        $this->mockAuthService->expects('getIdentity->getUserData')
+            ->times(2)
             ->andReturn([
                 'eligibleForPermits' => $eligibleForPermits,
                 'hasOrganisationSubmittedLicenceApplication' => false,
@@ -291,6 +271,11 @@ public function dpDispatchWithoutMatchedReferer(): array
 
     public function navigationExpectations(): void
     {
+        $this->mockAuthService->shouldReceive('isGranted')
+            ->with(RefData::PERMISSION_CAN_LIST_CONVERSATIONS)
+            ->once()
+            ->andReturn(1);
+
         $this->mockNavigation
             ->shouldReceive('findBy')
             ->twice()
@@ -302,28 +287,5 @@ public function navigationExpectations(): void
             ->once()
             ->with('id', $this->dashboardMessagingKey)
             ->andReturn($this->dashboardMessagingPage);
-
-        $this->mockNavigation
-            ->shouldReceive('findBy')
-            ->with('id', $this->dashboardMenuKey)
-            ->once()
-            ->andReturn($this->mockDashboardMenu);
-
-        $this->mockDashboardMenu
-            ->shouldReceive('findBy')
-            ->with('id', $this->dashboardMessagingKey)
-            ->once()
-            ->andReturn($this->dashboardMessagingPage);
-
-        $this->mockIdentity->shouldReceive('getId')
-            ->once()
-            ->andReturn(1);
-
-        $this->mockQuerySender->shouldReceive('send')
-            ->once()
-            ->andReturn($this->mockResponse);
-
-        $this->mockResponse->shouldReceive('getResult')
-            ->once();
     }
 }