Repair the app according to QA findings

OpenConext · Sep 17, 2024 · d40c9e5 · d40c9e5
1 parent 4ad4f53
commit d40c9e5
Show file tree

Hide file tree

Showing 8 changed files with 55 additions and 14 deletions.
diff --git a/src/EventSubscriber/RequiresActiveSessionAttributeListener.php b/src/EventSubscriber/RequiresActiveSessionAttributeListener.php
@@ -41,8 +41,9 @@
     private string $sessionName;
 
     public function __construct(
-        private LoggerInterface             $logger,
+        private LoggerInterface $logger,
         private SessionCorrelationIdService $sessionCorrelationIdService,
+        /** @var array<string, string> */
         private array $sessionOptions,
     ) {
         if (!array_key_exists('name', $this->sessionOptions)) {

diff --git a/src/EventSubscriber/SessionStateListener.php b/src/EventSubscriber/SessionStateListener.php
@@ -39,6 +39,7 @@
     public function __construct(
         private LoggerInterface $logger,
         private SessionCorrelationIdService $sessionCorrelationIdService,
+        /** @var array<string, string> */
         private array $sessionOptions,
     ) {
         if (!array_key_exists('name', $this->sessionOptions)) {

diff --git a/src/Service/SessionCorrelationIdService.php b/src/Service/SessionCorrelationIdService.php
@@ -29,6 +29,7 @@
 
     public function __construct(
         private RequestStack $requestStack,
+        /** @var array<string, string> */
         private array $sessionOptions,
         private string $sessionCorrelationSalt,
     ) {

diff --git a/src/Tiqr/Legacy/TiqrService.php b/src/Tiqr/Legacy/TiqrService.php
@@ -38,7 +38,6 @@
 use Tiqr_Service;
 use Tiqr_StateStorage_StateStorageInterface;
 
-
 /**
  * Wrapper around the legacy Tiqr service.
  *

diff --git a/tests/Unit/EventSubscriber/RequiresActiveSessionAttributeListenerTest.php b/tests/Unit/EventSubscriber/RequiresActiveSessionAttributeListenerTest.php
@@ -66,7 +66,14 @@ public function testControllersWithoutTheRequiresActiveSessionAttributeAreIgnore
         $mockLogger = Mockery::mock(LoggerInterface::class);
         $mockLogger->shouldNotReceive('log');
 
-        $listener = new RequiresActiveSessionAttributeListener($mockLogger, new SessionCorrelationIdService($requestStack));
+        $listener = new RequiresActiveSessionAttributeListener(
+            $mockLogger,
+            new SessionCorrelationIdService(
+                $requestStack,
+                ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'
+            ),
+            ['name' => 'PHPSESSID'],
+        );
 
         $dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelControllerArguments']);
         $dispatcher->dispatch($event, KernelEvents::REQUEST);
@@ -103,7 +110,11 @@ public function testItDeniesAccessWhenThereIsNoActiveSession(): void
                 ['correlationId' => '', 'route' => '/route']
             );
 
-        $listener = new RequiresActiveSessionAttributeListener($mockLogger, new SessionCorrelationIdService($requestStack));
+        $listener = new RequiresActiveSessionAttributeListener(
+            $mockLogger,
+            new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
+            ['name' => 'PHPSESSID'],
+        );
 
         $dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelControllerArguments']);
         $dispatcher->dispatch($event, KernelEvents::REQUEST);
@@ -144,7 +155,11 @@ public function testItDeniesAccessWhenThereIsNoSessionCookie(): void
                 ['correlationId' => '', 'route' => '/route']
             );
 
-        $listener = new RequiresActiveSessionAttributeListener($mockLogger, new SessionCorrelationIdService($requestStack));
+        $listener = new RequiresActiveSessionAttributeListener(
+            $mockLogger,
+            new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
+            ['name' => 'PHPSESSID'],
+        );
 
         $dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelControllerArguments']);
         $dispatcher->dispatch($event, KernelEvents::REQUEST);
@@ -185,7 +200,11 @@ public function testItDeniesAccessWhenTheActiveSessionDoesNotMatchTheSessionCook
                 ['correlationId' => 'f6e7cfb6f0861f577c48f171e27542236b1184f7a599dde82aca1640d86da961', 'route' => '/route']
             );
 
-        $listener = new RequiresActiveSessionAttributeListener($mockLogger, new SessionCorrelationIdService($requestStack));
+        $listener = new RequiresActiveSessionAttributeListener(
+            $mockLogger,
+            new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
+            ['name' => 'PHPSESSID'],
+        );
 
         $dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelControllerArguments']);
         $dispatcher->dispatch($event, KernelEvents::REQUEST);
@@ -219,7 +238,11 @@ public function testItDoesNotThrowWhenTheActiveSessionMatchesTheSessionCookie():
         $mockLogger = Mockery::mock(LoggerInterface::class);
         $mockLogger->shouldNotReceive('log');
 
-        $listener = new RequiresActiveSessionAttributeListener($mockLogger, new SessionCorrelationIdService($requestStack));
+        $listener = new RequiresActiveSessionAttributeListener(
+            $mockLogger,
+            new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
+            ['name' => 'PHPSESSID'],
+        );
 
         $dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelControllerArguments']);
         $dispatcher->dispatch($event, KernelEvents::REQUEST);

diff --git a/tests/Unit/EventSubscriber/SessionStateListenerTest.php b/tests/Unit/EventSubscriber/SessionStateListenerTest.php
@@ -56,7 +56,11 @@ public function testItLogsWhenUserHasNoSessionCookie(): void
             ->once()
             ->with(LogLevel::INFO, 'User made a request without a session cookie.', ['correlationId' => '', 'route' => '/route']);
 
-        $listener = new SessionStateListener($mockLogger, new SessionCorrelationIdService($requestStack));
+        $listener = new SessionStateListener(
+            $mockLogger,
+            new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
+            ['name' => 'PHPSESSIONID'],
+        );
 
         $dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelRequest']);
         $dispatcher->dispatch($event, KernelEvents::REQUEST);
@@ -85,7 +89,11 @@ public function testItLogsWhenUserHasNoSession(): void
             ->once()
             ->with(LogLevel::INFO, 'Session not found.', ['correlationId' => 'f6e7cfb6f0861f577c48f171e27542236b1184f7a599dde82aca1640d86da961', 'route' => '/route']);
 
-        $listener = new SessionStateListener($mockLogger, new SessionCorrelationIdService($requestStack));
+        $listener = new SessionStateListener(
+            $mockLogger,
+            new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
+            ['name' => 'PHPSESSID'],
+        );
 
         $dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelRequest']);
         $dispatcher->dispatch($event, KernelEvents::REQUEST);
@@ -121,7 +129,11 @@ public function testItLogsAnErrorWhenTheSessionIdDoesNotMatchTheSessionCookie():
             ->once()
             ->with(LogLevel::ERROR, 'The session cookie does not match the session id.', ['correlationId' => 'f6e7cfb6f0861f577c48f171e27542236b1184f7a599dde82aca1640d86da961', 'route' => '/route']);
 
-        $listener = new SessionStateListener($mockLogger, new SessionCorrelationIdService($requestStack));
+        $listener = new SessionStateListener(
+            $mockLogger,
+            new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
+            ['name' => 'PHPSESSID'],
+        );
 
         $dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelRequest']);
         $dispatcher->dispatch($event, KernelEvents::REQUEST);
@@ -158,7 +170,11 @@ public function testTheUserSessionMatchesTheSessionCookie(): void
             ->once()
             ->with(LogLevel::INFO, 'User session matches the session cookie.', ['correlationId' => 'f6e7cfb6f0861f577c48f171e27542236b1184f7a599dde82aca1640d86da961', 'route' => '/route']);
 
-        $listener = new SessionStateListener($mockLogger, new SessionCorrelationIdService($requestStack));
+        $listener = new SessionStateListener(
+            $mockLogger,
+            new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
+            ['name' => 'PHPSESSID'],
+        );
 
         $dispatcher->addListener(KernelEvents::REQUEST, [$listener, 'onKernelRequest']);
         $dispatcher->dispatch($event, KernelEvents::REQUEST);

diff --git a/tests/Unit/Service/SessionCorrelationIdServiceTest.php b/tests/Unit/Service/SessionCorrelationIdServiceTest.php
@@ -30,7 +30,7 @@ public function testItHasNoCorrelationIdWhenThereIsNoSessionCookie(): void
         $requestStack = new RequestStack();
         $requestStack->push($request);
 
-        $service = new SessionCorrelationIdService($requestStack);
+        $service = new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ');
 
         $this->assertNull($service->generateCorrelationId());
     }
@@ -41,7 +41,7 @@ public function testItGeneratesACorrelationIdBasedOnTheSessionCookie(): void
         $requestStack = new RequestStack();
         $requestStack->push($request);
 
-        $service = new SessionCorrelationIdService($requestStack);
+        $service = new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ');
 
         $this->assertSame('f6e7cfb6f0861f577c48f171e27542236b1184f7a599dde82aca1640d86da961', $service->generateCorrelationId());
     }

diff --git a/tests/Unit/Session/LoggingSessionFactoryTest.php b/tests/Unit/Session/LoggingSessionFactoryTest.php
@@ -45,7 +45,7 @@ public function testItLogsWheneverASessionIsCreated(): void
             $requestStack,
             $this->createStub(SessionStorageFactoryInterface::class),
             $mockLogger,
-            new SessionCorrelationIdService($requestStack),
+            new SessionCorrelationIdService($requestStack, ['name' => 'PHPSESSID'], 'Mr6LpJYtuWRDdVR2_7VgTChFhzQ'),
         );
 
         $this->assertInstanceOf(SessionInterface::class, $sessionFactory->createSession());
-Original file line number
+Diff line change
@@ Expand Up / @@ -38,7 +38,6 @@ @@
     use Tiqr_Service;
     use Tiqr_StateStorage_StateStorageInterface;
     /**
      * Wrapper around the legacy Tiqr service.
      *
@@ Expand Down @@