fix test

Keleo · Dec 4, 2024 · ffaed84 · ffaed84
1 parent 923e9d9
commit ffaed84
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 63 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,7 +4,6 @@ Compatibility: requires minimum Kimai 2.25.0
 
 - Added rate limiter to password protection form (10 failures within 1-hour will block access)
 - Remove form target (password protection) to prevent proxy issues with http vs https
-- Removed md5 password from session key
 - Use non-deprecated API to fetch timesheets
 
 ## Version 4.2.0

diff --git a/Service/ViewService.php b/Service/ViewService.php
@@ -47,7 +47,8 @@ public function hasAccess(SharedProjectTimesheet $sharedProject, ?string $givenP
         if ($hashedPassword !== null) {
             // Check session
             $shareKey = $sharedProject->getShareKey();
-            $sessionPasswordKey = \sprintf('spt-authed-%d-%s', $sharedProject->getId(), $shareKey);
+            // the password is in the session key, so changing it will revoke previous access
+            $sessionPasswordKey = \sprintf('spt-authed-%d-%s-%s', $sharedProject->getId(), $shareKey, md5($hashedPassword));
 
             if (!$this->request->getSession()->has($sessionPasswordKey)) {
                 $limiter = $this->customerPortalLimiter->create($request->getClientIp());

diff --git a/tests/Service/ViewServiceTest.php b/tests/Service/ViewServiceTest.php
@@ -16,45 +16,42 @@
 use KimaiPlugin\CustomerPortalBundle\Service\ViewService;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestStack;
+use Symfony\Component\HttpFoundation\Session\Session;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
+use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
 use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface;
 use Symfony\Component\PasswordHasher\PasswordHasherInterface;
+use Symfony\Component\RateLimiter\RateLimiterFactory;
+use Symfony\Component\RateLimiter\Storage\InMemoryStorage;
 
 class ViewServiceTest extends TestCase
 {
-    /**
-     * @var ViewService
-     */
-    private $service;
-
-    /**
-     * @var SessionInterface|MockObject
-     */
-    private $session;
-
-    /**
-     * @var PasswordHasherInterface|MockObject
-     */
-    private $encoder;
-
-    /**
-     * @var string
-     */
-    private $sessionKey;
+    private ViewService $service;
+    private SessionInterface $session;
+    private PasswordHasherInterface|MockObject $encoder;
+    private string $sessionKey;
+    private Request $request;
 
     protected function setUp(): void
     {
         $timesheetRepository = $this->createMock(TimesheetRepository::class);
         $sharedProjectTimesheetRepository = $this->createMock(SharedProjectTimesheetRepository::class);
-        $request = new RequestStack();
-        $this->session = $this->createPartialMock(SessionInterface::class, []);
+        $requestStack = new RequestStack();
+        $this->session = new Session(new MockArraySessionStorage());
 
         $factory = $this->createMock(PasswordHasherFactoryInterface::class);
         $this->encoder = $this->createMock(PasswordHasherInterface::class);
         $factory->method('getPasswordHasher')->willReturn($this->encoder);
 
-        $this->service = new ViewService($timesheetRepository, $request, $factory, $sharedProjectTimesheetRepository);
+        $this->request = new Request([], [], [], [], [], ['REMOTE_ADDR' => '1.1.1.1']);
+        $this->request->setSession($this->session);
+        $requestStack->push($this->request);
+
+        $rateLimiter = new RateLimiterFactory(['id' => 'customer_portal', 'policy' => 'sliding_window', 'limit' => 5, 'interval' => '1 hour'], new InMemoryStorage());
+
+        $this->service = new ViewService($timesheetRepository, $requestStack, $factory, $sharedProjectTimesheetRepository, $rateLimiter);
     }
 
     private function createSharedProjectTimesheet(): SharedProjectTimesheet
@@ -73,99 +70,74 @@ private function createSharedProjectTimesheet(): SharedProjectTimesheet
     public function testNoPassword(): void
     {
         $sharedProjectTimesheet = $this->createSharedProjectTimesheet();
-        $hasAccess = $this->service->hasAccess($sharedProjectTimesheet, '');
+        $hasAccess = $this->service->hasAccess($sharedProjectTimesheet, '', $this->request);
         self::assertTrue($hasAccess);
     }
 
     public function testValidPassword(): void
     {
-        $this->encoder->method('isPasswordValid')
+        $this->encoder->method('verify')
             ->willReturnCallback(function ($hashedPassword, $givenPassword) {
                 return $hashedPassword === $givenPassword;
             });
 
         $sharedProjectTimesheet = $this->createSharedProjectTimesheet();
         $sharedProjectTimesheet->setPassword('password');
 
-        $hasAccess = $this->service->hasAccess($sharedProjectTimesheet, 'password');
+        $hasAccess = $this->service->hasAccess($sharedProjectTimesheet, 'password', $this->request);
         self::assertTrue($hasAccess);
     }
 
     public function testInvalidPassword(): void
     {
-        $this->encoder->method('isPasswordValid')
+        $this->encoder->method('verify')
             ->willReturnCallback(function ($hashedPassword, $givenPassword) {
                 return $hashedPassword === $givenPassword;
             });
 
         $sharedProjectTimesheet = $this->createSharedProjectTimesheet();
         $sharedProjectTimesheet->setPassword('password');
 
-        $hasAccess = $this->service->hasAccess($sharedProjectTimesheet, 'wrong');
+        $hasAccess = $this->service->hasAccess($sharedProjectTimesheet, 'wrong', $this->request);
         self::assertFalse($hasAccess);
     }
 
     public function testPasswordRemember(): void
     {
-        // Mock session behaviour
-        $this->session->expects($this->exactly(1))
-            ->method('set')
-            ->willReturnCallback(function ($key) {
-                $this->sessionKey = $key;
-            });
-
-        $this->session->expects($this->exactly(2))
-            ->method('has')
-            ->willReturnCallback(function ($key) {
-                return $key === $this->sessionKey;
-            });
-
-        // Expect the encoder->isPasswordValid method is called only once
+        // Expect the encoder->verify method is called only once
         $this->encoder->expects($this->exactly(1))
-            ->method('isPasswordValid')
+            ->method('verify')
             ->willReturnCallback(function ($hashedPassword, $givenPassword) {
                 return $hashedPassword === $givenPassword;
             });
 
         $sharedProjectTimesheet = $this->createSharedProjectTimesheet();
         $sharedProjectTimesheet->setPassword('test');
 
-        $this->service->hasAccess($sharedProjectTimesheet, 'test');
-        $this->service->hasAccess($sharedProjectTimesheet, 'test');
+        self::assertFalse($this->service->hasAccess($sharedProjectTimesheet, null, $this->request));
+        self::assertTrue($this->service->hasAccess($sharedProjectTimesheet, 'test', $this->request));
+        self::assertTrue($this->service->hasAccess($sharedProjectTimesheet, null, $this->request));
     }
 
     public function testPasswordChange(): void
     {
-        // Mock session behaviour
-        $this->session->expects($this->exactly(1))
-            ->method('set')
-            ->willReturnCallback(function ($key) {
-                $this->sessionKey = $key;
-            });
-
-        $this->session->expects($this->exactly(2))
-            ->method('has')
-            ->willReturnCallback(function ($key) {
-                return $key === $this->sessionKey;
-            });
-
-        // Expect the encoder->isPasswordValid method is called only once
+        // Expect the encoder->verify method is called only once
         $this->encoder->expects($this->exactly(2))
-            ->method('isPasswordValid')
+            ->method('verify')
             ->willReturnCallback(function ($hashedPassword, $givenPassword) {
                 return $hashedPassword === $givenPassword;
             });
 
         $sharedProjectTimesheet = $this->createSharedProjectTimesheet();
         $sharedProjectTimesheet->setPassword('test');
 
-        $hasAccess = $this->service->hasAccess($sharedProjectTimesheet, 'test');
+        $hasAccess = $this->service->hasAccess($sharedProjectTimesheet, 'test', $this->request);
         self::assertTrue($hasAccess);
 
         $sharedProjectTimesheet = $this->createSharedProjectTimesheet();
         $sharedProjectTimesheet->setPassword('changed');
 
-        $hasAccess = $this->service->hasAccess($sharedProjectTimesheet, 'test');
+        $hasAccess = $this->service->hasAccess($sharedProjectTimesheet, 'test', $this->request);
         self::assertFalse($hasAccess);
     }
 }