improve permission checks for timesheets with activated lockdown (#2271)

tests/Voter/AbstractVoterTest.php

@@ -11,29 +11,18 @@ namespace App\Tests\Voter;
 
 use App\Entity\User;
 use App\Repository\RolePermissionRepository;
-use App\Security\AclDecisionManager;
 use App\Security\RolePermissionManager;
-use App\Voter\AbstractVoter;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
 
 abstract class AbstractVoterTest extends TestCase
 {
-    /**
-     * @param string $voterClass
-     * @param User $user
-     * @return AbstractVoter
-     * @throws \ReflectionException
-     */
-    protected function getVoter(string $voterClass, User $user)
+    protected function getVoter(string $voterClass): Voter
     {
-        $isAuthenticated = empty($user->getRoles());
-        $accessManager = $this->getMockBuilder(AclDecisionManager::class)->disableOriginalConstructor()->getMock();
-        $accessManager->method('isFullyAuthenticated')->willReturn($isAuthenticated);
-
         $class = new \ReflectionClass($voterClass);
-        /** @var AbstractVoter $voter */
-        $voter = $class->newInstance($accessManager, $this->getRolePermissionManager());
-        self::assertInstanceOf(AbstractVoter::class, $voter);
+        /** @var Voter $voter */
+        $voter = $class->newInstance($this->getRolePermissionManager());
+        self::assertInstanceOf(Voter::class, $voter);
 
         return $voter;
     }

tests/Voter/ActivityVoterTest.php

@@ -34,7 +34,7 @@ class ActivityVoterTest extends AbstractVoterTest
     protected function assertVote(User $user, $subject, $attribute, $result)
     {
         $token = new UsernamePasswordToken($user, 'foo', 'bar', $user->getRoles());
-        $sut = $this->getVoter(ActivityVoter::class, $user);
+        $sut = $this->getVoter(ActivityVoter::class);
 
         $this->assertEquals($result, $sut->vote($token, $subject, [$attribute]));
     }

tests/Voter/CustomerVoterTest.php

@@ -24,7 +24,7 @@ class CustomerVoterTest extends AbstractVoterTest
     protected function assertVote(User $user, $subject, $attribute, $result)
     {
         $token = new UsernamePasswordToken($user, 'foo', 'bar', $user->getRoles());
-        $sut = $this->getVoter(CustomerVoter::class, $user);
+        $sut = $this->getVoter(CustomerVoter::class);
 
         $actual = $sut->vote($token, $subject, [$attribute]);
         $this->assertEquals($result, $actual, sprintf('Failed voting "%s" for User with roles %s.', $attribute, implode(', ', $user->getRoles())));

tests/Voter/DeprecatedAbstractVoterTest.php

@@ -0,0 +1,80 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Tests\Voter;
+
+use App\Entity\User;
+use App\Security\AclDecisionManager;
+use App\Voter\AbstractVoter;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
+
+/**
+ * @covers \App\Voter\AbstractVoter
+ * @group legacy
+ */
+class DeprecatedAbstractVoterTest extends AbstractVoterTest
+{
+    protected function getVoter(string $voterClass): Voter
+    {
+        $accessManager = $this->getMockBuilder(AclDecisionManager::class)->disableOriginalConstructor()->getMock();
+        $accessManager->method('isFullyAuthenticated')->willReturn(true);
+
+        $class = new \ReflectionClass($voterClass);
+        /** @var AbstractVoter $voter */
+        $voter = $class->newInstance($accessManager, $this->getRolePermissionManager());
+        self::assertInstanceOf(AbstractVoter::class, $voter);
+
+        return $voter;
+    }
+
+    protected function assertVote(User $user, $subject, $attribute, $result)
+    {
+        $token = new UsernamePasswordToken($user, 'foo', 'bar', $user->getRoles());
+        $sut = $this->getVoter(DeprecatedVoter::class);
+
+        $actual = $sut->vote($token, $subject, [$attribute]);
+        $this->assertEquals($result, $actual, sprintf('Failed voting "%s" for User with roles %s.', $attribute, implode(', ', $user->getRoles())));
+    }
+
+    public function testMuuu()
+    {
+        $userStandard = $this->getUser(1, User::ROLE_USER);
+        $this->assertVote($userStandard, null, 'view_own_timesheet', true);
+    }
+}
+
+class DeprecatedVoter extends AbstractVoter
+{
+    protected function supports($attribute, $subject)
+    {
+        return true;
+    }
+
+    protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
+    {
+        if (!$this->isRegisteredPermission($attribute)) {
+            return false;
+        }
+
+        if (!$this->hasPermission('ROLE_USER', $attribute)) {
+            return false;
+        }
+
+        /** @var User $user */
+        $user = $token->getUser();
+
+        if (!$this->hasRolePermission($user, $attribute)) {
+            return false;
+        }
+
+        return $this->isFullyAuthenticated($token);
+    }
+}

tests/Voter/ProjectVoterTest.php

@@ -25,7 +25,7 @@ class ProjectVoterTest extends AbstractVoterTest
     protected function assertVote(User $user, $subject, $attribute, $result)
     {
         $token = new UsernamePasswordToken($user, 'foo', 'bar', $user->getRoles());
-        $sut = $this->getVoter(ProjectVoter::class, $user);
+        $sut = $this->getVoter(ProjectVoter::class);
 
         if ($subject instanceof Project && null === $subject->getCustomer()) {
             $subject->setCustomer(new Customer());

tests/Voter/RolePermissionVoterTest.php

@@ -26,7 +26,7 @@ class RolePermissionVoterTest extends AbstractVoterTest
     public function testVote(User $user, $subject, $attribute, $result)
     {
         $token = new UsernamePasswordToken($user, 'foo', 'bar', $user->getRoles());
-        $sut = $this->getVoter(RolePermissionVoter::class, $user);
+        $sut = $this->getVoter(RolePermissionVoter::class);
 
         $actual = $sut->vote($token, $subject, [$attribute]);
         $this->assertEquals($result, $actual, sprintf('Failed voting "%s" for User with roles %s.', $attribute, implode(', ', $user->getRoles())));

tests/Voter/TeamVoterTest.php

@@ -26,7 +26,7 @@ class TeamVoterTest extends AbstractVoterTest
     public function testVote(User $user, $subject, $attribute, $result)
     {
         $token = new UsernamePasswordToken($user, 'foo', 'bar', $user->getRoles());
-        $sut = $this->getVoter(TeamVoter::class, $user);
+        $sut = $this->getVoter(TeamVoter::class);
 
         $this->assertEquals($result, $sut->vote($token, $subject, [$attribute]));
     }

tests/Voter/TimesheetVoterTest.php

@@ -9,13 +9,17 @@
 
 namespace App\Tests\Voter;
 
+use App\Configuration\ConfigLoaderInterface;
+use App\Configuration\SystemConfiguration;
 use App\Entity\Activity;
 use App\Entity\Customer;
 use App\Entity\Project;
 use App\Entity\Timesheet;
 use App\Entity\User;
+use App\Timesheet\LockdownService;
 use App\Voter\TimesheetVoter;
 use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
 use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
 
 /**
@@ -23,10 +27,15 @@ use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
  */
 class TimesheetVoterTest extends AbstractVoterTest
 {
+    protected function getVoter(string $voterClass): Voter
+    {
+        return $this->getLockdownVoter();
+    }
+
     protected function assertVote(User $user, $subject, $attribute, $result)
     {
         $token = new UsernamePasswordToken($user, 'foo', 'bar', $user->getRoles());
-        $sut = $this->getVoter(TimesheetVoter::class, $user);
+        $sut = $this->getVoter(TimesheetVoter::class);
 
         $this->assertEquals($result, $sut->vote($token, $subject, [$attribute]));
     }
@@ -87,6 +96,36 @@ class TimesheetVoterTest extends AbstractVoterTest
         }
     }
 
+    /**
+     * @dataProvider getLockDownTestData
+     */
+    public function testWithLockdown(string $permission, int $expected, string $beginModifier, string $lockdownBegin, string $lockdownEnd, ?string $lockdownGrace)
+    {
+        $user = $this->getUser(1, User::ROLE_USER);
+
+        $begin = new \DateTime('now');
+        $begin->modify($beginModifier);
+
+        $timesheet = new Timesheet();
+        $timesheet->setBegin($begin);
+        $timesheet->setUser($user);
+
+        $token = new UsernamePasswordToken($user, 'foo', 'bar', $user->getRoles());
+        $sut = $this->getLockdownVoter($lockdownBegin, $lockdownEnd, $lockdownGrace);
+
+        self::assertEquals($expected, $sut->vote($token, $timesheet, [$permission]));
+    }
+
+    public function getLockDownTestData()
+    {
+        yield ['view', VoterInterface::ACCESS_GRANTED, '+1 days', 'first day of this month', 'last day of this month', '+10 days'];
+        yield ['duplicate', VoterInterface::ACCESS_GRANTED, '+1 days', 'first day of this month', 'last day of this month', '+10 days'];
+        yield ['delete', VoterInterface::ACCESS_GRANTED, '+1 days', 'first day of this month', 'last day of this month', '+10 days'];
+        yield ['edit', VoterInterface::ACCESS_DENIED, '-50 days', 'first day of last month', 'last day of last month', '+1 days'];
+        yield ['duplicate', VoterInterface::ACCESS_DENIED, '-50 days', 'first day of last month', 'last day of last month', '+1 days'];
+        yield ['delete', VoterInterface::ACCESS_DENIED, '-50 days', 'first day of last month', 'last day of last month', '+1 days'];
+    }
+
     public function testSpecialCases()
     {
         $user1 = $this->getUser(1, User::ROLE_USER);
@@ -154,10 +193,30 @@ class TimesheetVoterTest extends AbstractVoterTest
      */
     protected function getUser($id, $role)
     {
-        $user = $this->getMockBuilder(User::class)->getMock();
+        $user = $this->createMock(User::class);
         $user->method('getId')->willReturn($id);
         $user->method('getRoles')->willReturn([$role]);
+        $user->method('getTimezone')->willReturn(date_default_timezone_get());
 
         return $user;
     }
+
+    protected function getLockdownVoter(?string $lockdownBegin = null, ?string $lockdownEnd = null, ?string $lockdownGrace = null): Voter
+    {
+        $loader = $this->createMock(ConfigLoaderInterface::class);
+        $config = new SystemConfiguration($loader, [
+            'timesheet' => [
+                'rules' => [
+                    'lockdown_period_start' => $lockdownBegin,
+                    'lockdown_period_end' => $lockdownEnd,
+                    'lockdown_grace_period' => $lockdownGrace,
+                ],
+            ]
+        ]);
+
+        $voter = new TimesheetVoter($this->getRolePermissionManager(), new LockdownService($config));
+        self::assertInstanceOf(Voter::class, $voter);
+
+        return $voter;
+    }
 }

tests/Voter/UserVoterTest.php

@@ -26,7 +26,7 @@ class UserVoterTest extends AbstractVoterTest
     public function testVote(User $user, $subject, $attribute, $result)
     {
         $token = new UsernamePasswordToken($user, 'foo', 'bar', $user->getRoles());
-        $sut = $this->getVoter(UserVoter::class, $user);
+        $sut = $this->getVoter(UserVoter::class);
 
         $this->assertEquals($result, $sut->vote($token, $subject, [$attribute]));
     }