From 20c7b03bd91a7336d408ac16cab07d424f7a29f0 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Sun, 26 Apr 2026 17:06:59 +0200 Subject: [PATCH] Re-usable ACL checks on teams (#5925) --- src/Security/RolePermissionManager.php | 46 +- src/Voter/ActivityVoter.php | 35 +- src/Voter/CustomerVoter.php | 16 +- src/Voter/ProjectVoter.php | 28 +- src/Voter/TimesheetVoter.php | 13 +- tests/Ldap/LdapManagerTest.php | 2 +- tests/Security/RolePermissionManagerTest.php | 543 +++++++++++++++++++ tests/Twig/ConfigurationTest.php | 10 +- tests/Twig/ContextTest.php | 18 + tests/Voter/ActivityVoterTest.php | 143 +++++ tests/Voter/CustomerVoterTest.php | 49 ++ tests/Voter/ProjectVoterTest.php | 137 +++++ tests/Voter/TimesheetVoterTest.php | 487 +++++++++++++++-- tests/phpstan.neon | 15 - 14 files changed, 1393 insertions(+), 149 deletions(-) diff --git a/src/Security/RolePermissionManager.php b/src/Security/RolePermissionManager.php index 43c35208..c15bf1ae 100644 --- a/src/Security/RolePermissionManager.php +++ b/src/Security/RolePermissionManager.php @@ -13,6 +13,7 @@ use App\Entity\Activity; use App\Entity\Customer; use App\Entity\Project; use App\Entity\Team; +use App\Entity\Timesheet; use App\Entity\User; use App\User\PermissionService; use Doctrine\Common\Collections\Collection; @@ -115,15 +116,15 @@ final class RolePermissionManager } /** - * @param Collection $teams + * @param array|Collection $teams */ - private function checkTeamAccess(Collection $teams, User $user): bool + private function checkTeamAccess(Collection|array $teams, User $user): bool { if ($user->canSeeAllData()) { return true; } - if ($teams->count() === 0) { + if (\count($teams) === 0) { return true; } @@ -136,6 +137,28 @@ final class RolePermissionManager return false; } + /** + * @param array|Collection $teams + */ + private function checkTeamLeadAccess(Collection|array $teams, User $user): bool + { + if ($user->canSeeAllData()) { + return true; + } + + if (\count($teams) === 0) { + return true; + } + + foreach ($teams as $team) { + if ($user->isTeamleadOf($team)) { + return true; + } + } + + return false; + } + public function checkTeamAccessCustomer(Customer $customer, User $user): bool { return $this->checkTeamAccess($customer->getTeams(), $user); @@ -158,4 +181,21 @@ final class RolePermissionManager return $this->checkTeamAccess($activity->getTeams(), $user); } + + public function checkTeamAccessTimesheet(Timesheet $timesheet, User $user): bool + { + if ($user->getId() !== null && $user->getId() === $timesheet->getUser()?->getId()) { + return true; + } + + if ($timesheet->getProject() !== null && !$this->checkTeamAccessProject($timesheet->getProject(), $user)) { + return false; + } + + if ($timesheet->getActivity() !== null && !$this->checkTeamAccessActivity($timesheet->getActivity(), $user)) { + return false; + } + + return $this->checkTeamLeadAccess($timesheet->getUser()?->getTeams() ?? [], $user); + } } diff --git a/src/Voter/ActivityVoter.php b/src/Voter/ActivityVoter.php index 2cfa539d..77058d65 100644 --- a/src/Voter/ActivityVoter.php +++ b/src/Voter/ActivityVoter.php @@ -10,8 +10,6 @@ namespace App\Voter; use App\Entity\Activity; -use App\Entity\Customer; -use App\Entity\Project; use App\Entity\Team; use App\Entity\User; use App\Security\RolePermissionManager; @@ -57,37 +55,6 @@ final class ActivityVoter extends Voter return $subject instanceof Activity && $this->supportsAttribute($attribute); } - private function checkTeamPermission(Activity|Project|Customer $subject, User $user): bool - { - if ($user->canSeeAllData()) { - return true; - } - - if ($subject instanceof Activity && $subject->getProject() !== null) { - if (!$this->checkTeamPermission($subject->getProject(), $user)) { - return false; - } - } - - if ($subject instanceof Project && $subject->getCustomer() !== null) { - if (!$this->checkTeamPermission($subject->getCustomer(), $user)) { - return false; - } - } - - if ($subject->getTeams()->count() === 0) { - return true; - } - - foreach ($subject->getTeams() as $team) { - if ($user->isInTeam($team)) { - return true; - } - } - - return false; - } - protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool { $user = $token->getUser(); @@ -99,7 +66,7 @@ final class ActivityVoter extends Voter // this is a virtual permission, only meant to be used by developer // it checks if access to the given activity is potentially possible if ($attribute === 'access') { - return $this->checkTeamPermission($subject, $user); + return $this->permissionManager->checkTeamAccessActivity($subject, $user); } if ($this->permissionManager->hasRolePermission($user, $attribute . '_activity')) { diff --git a/src/Voter/CustomerVoter.php b/src/Voter/CustomerVoter.php index 28b6c6d9..be9d0a7a 100644 --- a/src/Voter/CustomerVoter.php +++ b/src/Voter/CustomerVoter.php @@ -69,21 +69,7 @@ final class CustomerVoter extends Voter // this is a virtual permission, only meant to be used by developer // it checks if access to the given customer is potentially possible if ($attribute === 'access') { - if ($subject->getTeams()->count() === 0) { - return true; - } - - foreach ($subject->getTeams() as $team) { - if ($user->isInTeam($team)) { - return true; - } - } - - if ($user->canSeeAllData()) { - return true; - } - - return false; + return $this->permissionManager->checkTeamAccessCustomer($subject, $user); } if ($this->permissionManager->hasRolePermission($user, $attribute . '_customer')) { diff --git a/src/Voter/ProjectVoter.php b/src/Voter/ProjectVoter.php index f44036a3..fd4f2e28 100644 --- a/src/Voter/ProjectVoter.php +++ b/src/Voter/ProjectVoter.php @@ -9,7 +9,6 @@ namespace App\Voter; -use App\Entity\Customer; use App\Entity\Project; use App\Entity\Team; use App\Entity\User; @@ -58,31 +57,6 @@ final class ProjectVoter extends Voter return $subject instanceof Project && $this->supportsAttribute($attribute); } - private function checkTeamPermission(Project|Customer $subject, User $user): bool - { - if ($user->canSeeAllData()) { - return true; - } - - if ($subject instanceof Project && $subject->getCustomer() !== null) { - if (!$this->checkTeamPermission($subject->getCustomer(), $user)) { - return false; - } - } - - if ($subject->getTeams()->count() === 0) { - return true; - } - - foreach ($subject->getTeams() as $team) { - if ($user->isInTeam($team)) { - return true; - } - } - - return false; - } - protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool { $user = $token->getUser(); @@ -94,7 +68,7 @@ final class ProjectVoter extends Voter // this is a virtual permission, only meant to be used by developer // it checks if access to the given project is potentially possible if ($attribute === 'access') { - return $this->checkTeamPermission($subject, $user); + return $this->permissionManager->checkTeamAccessProject($subject, $user); } if ($this->permissionManager->hasRolePermission($user, $attribute . '_project')) { diff --git a/src/Voter/TimesheetVoter.php b/src/Voter/TimesheetVoter.php index 06cb8229..59f9e520 100644 --- a/src/Voter/TimesheetVoter.php +++ b/src/Voter/TimesheetVoter.php @@ -131,18 +131,15 @@ final class TimesheetVoter extends Voter return false; } - $permission .= '_'; - - // extend me for "team" support later on if ($subject->getUser()?->getId() === $user->getId()) { - $permission .= 'own'; - } else { - $permission .= 'other'; + return $this->permissionManager->hasRolePermission($user, $permission . '_own_timesheet'); } - $permission .= '_timesheet'; + if (!$this->permissionManager->checkTeamAccessTimesheet($subject, $user)) { + return false; + } - return $this->permissionManager->hasRolePermission($user, $permission); + return $this->permissionManager->hasRolePermission($user, $permission . '_other_timesheet'); } private function canStart(Timesheet $timesheet): bool diff --git a/tests/Ldap/LdapManagerTest.php b/tests/Ldap/LdapManagerTest.php index 5700e1fa..b2a8215e 100644 --- a/tests/Ldap/LdapManagerTest.php +++ b/tests/Ldap/LdapManagerTest.php @@ -583,7 +583,7 @@ class LdapManagerTest extends TestCase } #[Group('legacy')] - public function testHydrateWithDepercatedSetter(): void + public function testHydrateWithDeprecatedSetter(): void { $ldapConfig = [ 'connection' => [ diff --git a/tests/Security/RolePermissionManagerTest.php b/tests/Security/RolePermissionManagerTest.php index 94635179..8a356e80 100644 --- a/tests/Security/RolePermissionManagerTest.php +++ b/tests/Security/RolePermissionManagerTest.php @@ -13,6 +13,7 @@ use App\Entity\Activity; use App\Entity\Customer; use App\Entity\Project; use App\Entity\Team; +use App\Entity\Timesheet; use App\Entity\User; use App\Repository\RolePermissionRepository; use App\Security\RolePermissionManager; @@ -275,6 +276,548 @@ class RolePermissionManagerTest extends TestCase self::assertTrue($sut->checkTeamAccessActivity($activity, $user)); } + public function testCheckTeamAccessTimesheetGrantsAccessForOwner(): void + { + $sut = $this->createSut(); + + $owner = new User(); + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $owner)); + } + + public function testCheckTeamAccessTimesheetGrantsForOwnerEvenWithBlockingTeams(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(42); + $customer = new Customer('Acme'); + $customer->addTeam(new Team('Customer team')); + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam(new Team('Project team')); + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam(new Team('Activity team')); + + $ownerTeam = new Team('Owner team'); + $ownerTeam->addUser($owner); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + // Owner is not in any of the customer/project/activity teams and is only + // a member (not teamlead) of his own team — must still be granted access + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $owner)); + } + + public function testCheckTeamAccessTimesheetGrantsForDifferentInstancesWithSameId(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(42); + $requester = self::userWithId(42); + // Make sure the requester does not pass via canSeeAllData or team membership. + self::assertNotSame($owner, $requester); + + $customer = new Customer('Acme'); + $customer->addTeam(new Team('Customer team')); + $project = new Project(); + $project->setCustomer($customer); + $activity = new Activity(); + $activity->setProject($project); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetGrantsForCanSeeAllData(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $ownerTeam = new Team('Owner team'); + $ownerTeam->addUser($owner); + + $customer = new Customer('Acme'); + $customer->addTeam(new Team('Customer team')); + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam(new Team('Project team')); + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam(new Team('Activity team')); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + $requester = self::userWithId(2); + $requester->initCanSeeAllData(true); + + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetGrantsForCanSeeAllDataWithEmptyTimesheet(): void + { + $sut = $this->createSut(); + + $requester = self::userWithId(2); + $requester->initCanSeeAllData(true); + + $timesheet = new Timesheet(); + + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetDeniesWhenCustomerTeamRestricts(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $customer = new Customer('Acme'); + $customer->addTeam(new Team('Customer team')); + + $project = new Project(); + $project->setCustomer($customer); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + + // Even if requester would be a teamlead of the timesheet user's team, the + // customer gate must still deny. + $ownerTeam = new Team('Owner team'); + $ownerTeam->addUser($owner); + $ownerTeam->addTeamlead($requester); + + self::assertFalse($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetDeniesWhenProjectTeamRestricts(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $project = new Project(); + $project->setCustomer(new Customer('Acme')); + $project->addTeam(new Team('Project team')); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + + self::assertFalse($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetDeniesWhenCustomerOkButProjectRestricts(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $customer = new Customer('Acme'); + $customerTeam = new Team('Customer team'); + $customer->addTeam($customerTeam); + $customerTeam->addUser($requester); + + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam(new Team('Project team')); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + + self::assertFalse($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetPassesProjectGateWithoutTeams(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + // No customer team, no project team -> project gate is permissive. + $project = new Project(); + $project->setCustomer(new Customer('Acme')); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + + // Owner has no teams -> teamlead gate is permissive too. + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetDeniesWhenActivityTeamRestricts(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $project = new Project(); + $project->setCustomer(new Customer('Acme')); + + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam(new Team('Activity team')); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + self::assertFalse($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetPassesActivityGateWithoutTeams(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $project = new Project(); + $project->setCustomer(new Customer('Acme')); + + $activity = new Activity(); + $activity->setProject($project); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetActivityChainsThroughItsProjectAndCustomer(): void + { + // checkTeamAccessActivity walks activity -> project -> customer. Even when + // the timesheet's project gate already passed, the activity must re-pass + // its own project's customer gate. + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + // Timesheet's own project is unrestricted. + $timesheetProject = new Project(); + $timesheetProject->setCustomer(new Customer('Acme')); + + // Activity is wired to a different project whose customer locks out the requester. + $blockedCustomer = new Customer('Blocked'); + $blockedCustomer->addTeam(new Team('Blocked customer team')); + $activityProject = new Project(); + $activityProject->setCustomer($blockedCustomer); + + $activity = new Activity(); + $activity->setProject($activityProject); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($timesheetProject); + $timesheet->setActivity($activity); + + self::assertFalse($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetGrantsWhenTimesheetUserHasNoTeams(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + + // Owner has no team memberships -> teamlead gate returns true. + self::assertSame([], $owner->getTeams()); + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetDeniesPlainMemberOfTimesheetUserTeam(): void + { + // Headline behaviour: plain team membership is NOT enough — the requester + // must be a teamlead. + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $sharedTeam = new Team('Shared'); + $sharedTeam->addUser($owner); + $sharedTeam->addUser($requester); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + + self::assertTrue($requester->isInTeam($sharedTeam)); + self::assertFalse($requester->isTeamleadOf($sharedTeam)); + self::assertFalse($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetGrantsTeamleadOfTimesheetUserTeam(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $sharedTeam = new Team('Shared'); + $sharedTeam->addUser($owner); + $sharedTeam->addTeamlead($requester); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + + self::assertTrue($requester->isTeamleadOf($sharedTeam)); + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetGrantsTeamleadOfOneOfMultipleTeams(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $teamA = new Team('A'); + $teamB = new Team('B'); + $teamA->addUser($owner); + $teamB->addUser($owner); + $teamA->addUser($requester); + $teamB->addTeamlead($requester); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetDeniesWhenTeamleadOfNoneOfMultipleTeams(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $teamA = new Team('A'); + $teamB = new Team('B'); + $teamA->addUser($owner); + $teamB->addUser($owner); + $teamA->addUser($requester); + $teamB->addUser($requester); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + + self::assertFalse($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetDeniesWhenTeamleadOfUnrelatedTeam(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $ownerTeam = new Team('Owner team'); + $ownerTeam->addUser($owner); + + $unrelatedTeam = new Team('Unrelated'); + $unrelatedTeam->addTeamlead($requester); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + + self::assertFalse($requester->isInTeam($ownerTeam)); + self::assertFalse($requester->isTeamleadOf($ownerTeam)); + self::assertFalse($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetGrantsWhenTimesheetUserIsNullAndNoProjectActivity(): void + { + // Documents current behaviour: an orphaned timesheet (no user) bypasses + // the teamlead gate because getTeams() ?? [] is empty. + $sut = $this->createSut(); + + $requester = self::userWithId(2); + $timesheet = new Timesheet(); + + self::assertNull($timesheet->getUser()); + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetDeniesWhenTimesheetUserNullButProjectRestricts(): void + { + $sut = $this->createSut(); + + $requester = self::userWithId(2); + + $project = new Project(); + $project->setCustomer(new Customer('Acme')); + $project->addTeam(new Team('Project team')); + + $timesheet = new Timesheet(); + $timesheet->setProject($project); + + self::assertFalse($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetGrantsWithoutProjectAndActivityWhenTeamlead(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $sharedTeam = new Team('Shared'); + $sharedTeam->addUser($owner); + $sharedTeam->addTeamlead($requester); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + + self::assertNull($timesheet->getProject()); + self::assertNull($timesheet->getActivity()); + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetGrantsFullChainAsMemberAndTeamlead(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $customer = new Customer('Acme'); + $customerTeam = new Team('Customer team'); + $customer->addTeam($customerTeam); + $customerTeam->addUser($requester); + + $project = new Project(); + $project->setCustomer($customer); + $projectTeam = new Team('Project team'); + $project->addTeam($projectTeam); + $projectTeam->addUser($requester); + + $activity = new Activity(); + $activity->setProject($project); + $activityTeam = new Team('Activity team'); + $activity->addTeam($activityTeam); + $activityTeam->addUser($requester); + + $ownerTeam = new Team('Owner team'); + $ownerTeam->addUser($owner); + $ownerTeam->addTeamlead($requester); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + self::assertTrue($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetDeniesFullChainMissingActivityTeam(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $customer = new Customer('Acme'); + $customerTeam = new Team('Customer team'); + $customer->addTeam($customerTeam); + $customerTeam->addUser($requester); + + $project = new Project(); + $project->setCustomer($customer); + $projectTeam = new Team('Project team'); + $project->addTeam($projectTeam); + $projectTeam->addUser($requester); + + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam(new Team('Activity team')); // requester missing + + $ownerTeam = new Team('Owner team'); + $ownerTeam->addUser($owner); + $ownerTeam->addTeamlead($requester); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + self::assertFalse($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + public function testCheckTeamAccessTimesheetDeniesFullChainOnlyMemberNotTeamlead(): void + { + $sut = $this->createSut(); + + $owner = self::userWithId(1); + $requester = self::userWithId(2); + + $customer = new Customer('Acme'); + $customerTeam = new Team('Customer team'); + $customer->addTeam($customerTeam); + $customerTeam->addUser($requester); + + $project = new Project(); + $project->setCustomer($customer); + $projectTeam = new Team('Project team'); + $project->addTeam($projectTeam); + $projectTeam->addUser($requester); + + $activity = new Activity(); + $activity->setProject($project); + $activityTeam = new Team('Activity team'); + $activity->addTeam($activityTeam); + $activityTeam->addUser($requester); + + $ownerTeam = new Team('Owner team'); + $ownerTeam->addUser($owner); + $ownerTeam->addUser($requester); // plain member only + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + self::assertFalse($sut->checkTeamAccessTimesheet($timesheet, $requester)); + } + + private static function userWithId(int $id): User + { + $user = new User(); + $reflection = new \ReflectionClass($user); + $property = $reflection->getProperty('id'); + $property->setAccessible(true); + $property->setValue($user, $id); + + return $user; + } + private function createSut(): RolePermissionManager { $repository = $this->getMockBuilder(RolePermissionRepository::class)->onlyMethods(['getAllAsArray'])->disableOriginalConstructor()->getMock(); diff --git a/tests/Twig/ConfigurationTest.php b/tests/Twig/ConfigurationTest.php index 21f722c4..5d3079e9 100644 --- a/tests/Twig/ConfigurationTest.php +++ b/tests/Twig/ConfigurationTest.php @@ -79,7 +79,7 @@ class ConfigurationTest extends TestCase self::assertInstanceOf(TwigFunction::class, $functions[0]); self::assertSame('config', $functions[0]->getName()); - $environment = $this->createEnvironment(['template' => '{{ config(\'theme.branding.company\') }}']); + $environment = $this->createEnvironment(['template' => '{{ config("theme.branding.company") }}']); $environment->addExtension($sut); self::assertSame('Acme Inc.', $environment->render('template')); @@ -105,9 +105,9 @@ class ConfigurationTest extends TestCase public static function provideSandboxAllowedTemplates(): iterable { - yield 'avatar urls' => ['{{ config(\'themeAllowAvatarUrls\') ? \'1\' : \'0\' }}', '1']; - yield 'branding logo' => ['{{ config(\'theme.branding.logo\') }}', 'logo.png']; - yield 'branding company' => ['{{ config(\'theme.branding.company\') }}', 'Acme Inc.']; + yield 'avatar urls' => ['{{ config("themeAllowAvatarUrls") ? "1" : "0" }}', '1']; + yield 'branding logo' => ['{{ config("theme.branding.logo") }}', 'logo.png']; + yield 'branding company' => ['{{ config("theme.branding.company") }}', 'Acme Inc.']; } #[DataProvider('provideSandboxAllowedTemplates')] @@ -140,7 +140,7 @@ class ConfigurationTest extends TestCase public function testSandboxRejectsNonWhitelistedConfigAccess(): void { - $environment = $this->createEnvironment(['template' => '{{ config(\'showAbout\') ? \'1\' : \'0\' }}'], true); + $environment = $this->createEnvironment(['template' => '{{ config("showAbout") ? "1" : "0" }}'], true); $environment->addExtension($this->createExtension($this->getDefaultSettings())); $this->expectException(SecurityError::class); diff --git a/tests/Twig/ContextTest.php b/tests/Twig/ContextTest.php index b7c01d80..67151890 100644 --- a/tests/Twig/ContextTest.php +++ b/tests/Twig/ContextTest.php @@ -77,4 +77,22 @@ class ContextTest extends TestCase $sut = $this->getSut($this->getDefaultSettings(), ['X-Requested-With' => 'Kimai']); self::assertTrue($sut->isJavascriptRequest()); } + + public function testDeprecatedGetBrandingAlwaysReturnsNull(): void + { + $sut = $this->getSut($this->getDefaultSettings()); + + $previousHandler = set_error_handler(static function (int $type, string $message): true { + self::assertSame(E_USER_DEPRECATED, $type); + self::assertSame('Use config() instead of "kimai_context" to access system configurations', $message); + + return true; + }); + + try { + self::assertNull($sut->getBranding('legacyAccess')); + } finally { + restore_error_handler(); + } + } } diff --git a/tests/Voter/ActivityVoterTest.php b/tests/Voter/ActivityVoterTest.php index a278b37e..099592d1 100644 --- a/tests/Voter/ActivityVoterTest.php +++ b/tests/Voter/ActivityVoterTest.php @@ -19,6 +19,7 @@ use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use Symfony\Component\Security\Core\User\InMemoryUser; #[CoversClass(ActivityVoter::class)] class ActivityVoterTest extends AbstractVoterTestCase @@ -166,4 +167,146 @@ class ActivityVoterTest extends AbstractVoterTestCase $this->assertVote($user, $activity, 'edit', VoterInterface::ACCESS_GRANTED); } + + public function testAccessGrantedWhenAllChainsHaveNoTeams(): void + { + $project = new Project(); + $project->setCustomer(new Customer('foo')); + + $activity = new Activity(); + $activity->setProject($project); + + $this->assertVote(new User(), $activity, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessGrantedWhenActivityHasNoProject(): void + { + // checkTeamAccessActivity() skips the project chain when activity->getProject() is null. + $activity = new Activity(); + + $this->assertVote(new User(), $activity, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessGrantedForCanSeeAllDataDespiteRestrictiveChain(): void + { + $customer = new Customer('foo'); + $customer->addTeam(new Team('customerTeam')); + + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam(new Team('projectTeam')); + + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam(new Team('activityTeam')); + + $user = new User(); + $user->initCanSeeAllData(true); + + $this->assertVote($user, $activity, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessGrantedAsMemberOfFullChain(): void + { + $customerTeam = new Team('customerTeam'); + $customer = new Customer('foo'); + $customer->addTeam($customerTeam); + + $projectTeam = new Team('projectTeam'); + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam($projectTeam); + + $activityTeam = new Team('activityTeam'); + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam($activityTeam); + + $user = new User(); + $customerTeam->addUser($user); + $projectTeam->addUser($user); + $activityTeam->addUser($user); + + $this->assertVote($user, $activity, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessDeniedWhenCustomerTeamBlocks(): void + { + $customer = new Customer('foo'); + $customer->addTeam(new Team('customerTeam')); + + $project = new Project(); + $project->setCustomer($customer); + + $activity = new Activity(); + $activity->setProject($project); + + $this->assertVote(new User(), $activity, 'access', VoterInterface::ACCESS_DENIED); + } + + public function testAccessDeniedWhenProjectTeamBlocks(): void + { + $customerTeam = new Team('customerTeam'); + $customer = new Customer('foo'); + $customer->addTeam($customerTeam); + + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam(new Team('projectTeam')); + + $activity = new Activity(); + $activity->setProject($project); + + $user = new User(); + $customerTeam->addUser($user); + + $this->assertVote($user, $activity, 'access', VoterInterface::ACCESS_DENIED); + } + + public function testAccessDeniedWhenActivityTeamBlocks(): void + { + $customerTeam = new Team('customerTeam'); + $customer = new Customer('foo'); + $customer->addTeam($customerTeam); + + $projectTeam = new Team('projectTeam'); + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam($projectTeam); + + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam(new Team('activityTeam')); + + $user = new User(); + $customerTeam->addUser($user); + $projectTeam->addUser($user); + + $this->assertVote($user, $activity, 'access', VoterInterface::ACCESS_DENIED); + } + + public function testAccessDeniedWhenActivityTeamsExistAndUserOnlyInUnrelatedTeam(): void + { + $project = new Project(); + $project->setCustomer(new Customer('foo')); + + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam(new Team('activityTeam')); + + $unrelated = new Team('unrelated'); + $user = new User(); + $unrelated->addUser($user); + + $this->assertVote($user, $activity, 'access', VoterInterface::ACCESS_DENIED); + } + + public function testAccessDeniedForNonUserToken(): void + { + $activity = new Activity(); + $token = new UsernamePasswordToken(new InMemoryUser('anon', null), 'bar', []); + $sut = $this->getVoter(ActivityVoter::class); + + self::assertEquals(VoterInterface::ACCESS_DENIED, $sut->vote($token, $activity, ['access'])); + } } diff --git a/tests/Voter/CustomerVoterTest.php b/tests/Voter/CustomerVoterTest.php index 1674f0e2..6bf07c7e 100644 --- a/tests/Voter/CustomerVoterTest.php +++ b/tests/Voter/CustomerVoterTest.php @@ -16,6 +16,7 @@ use App\Voter\CustomerVoter; use PHPUnit\Framework\Attributes\CoversClass; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use Symfony\Component\Security\Core\User\InMemoryUser; #[CoversClass(CustomerVoter::class)] class CustomerVoterTest extends AbstractVoterTestCase @@ -163,4 +164,52 @@ class CustomerVoterTest extends AbstractVoterTestCase $this->assertVote($user, $customer, 'access', VoterInterface::ACCESS_DENIED); } + + public function testAccessGrantedForCanSeeAllDataDespiteRestrictiveTeams(): void + { + $customer = new Customer('foo'); + $customer->addTeam(new Team('locked')); + + $user = new User(); + $user->initCanSeeAllData(true); + + $this->assertVote($user, $customer, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessGrantedWhenUserMatchesOneOfMultipleCustomerTeams(): void + { + $sharedTeam = new Team('shared'); + $foreignTeam = new Team('foreign'); + + $customer = new Customer('foo'); + $customer->addTeam($foreignTeam); + $customer->addTeam($sharedTeam); + + $user = new User(); + $sharedTeam->addUser($user); + + $this->assertVote($user, $customer, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessDeniedWhenUserOnlyInUnrelatedTeams(): void + { + $customer = new Customer('foo'); + $customer->addTeam(new Team('customerTeam')); + + $user = new User(); + $unrelated = new Team('unrelated'); + $unrelated->addUser($user); + $unrelated->addTeamlead($user); + + $this->assertVote($user, $customer, 'access', VoterInterface::ACCESS_DENIED); + } + + public function testAccessDeniedForNonUserToken(): void + { + $customer = new Customer('foo'); + $token = new UsernamePasswordToken(new InMemoryUser('anon', null), 'bar', []); + $sut = $this->getVoter(CustomerVoter::class); + + self::assertEquals(VoterInterface::ACCESS_DENIED, $sut->vote($token, $customer, ['access'])); + } } diff --git a/tests/Voter/ProjectVoterTest.php b/tests/Voter/ProjectVoterTest.php index 10ee837c..1b0d39e4 100644 --- a/tests/Voter/ProjectVoterTest.php +++ b/tests/Voter/ProjectVoterTest.php @@ -17,6 +17,7 @@ use App\Voter\ProjectVoter; use PHPUnit\Framework\Attributes\CoversClass; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use Symfony\Component\Security\Core\User\InMemoryUser; #[CoversClass(ProjectVoter::class)] class ProjectVoterTest extends AbstractVoterTestCase @@ -137,4 +138,140 @@ class ProjectVoterTest extends AbstractVoterTestCase $this->assertVote($user, $project, 'edit', VoterInterface::ACCESS_GRANTED); } + + public function testAccessGrantedWhenProjectAndCustomerHaveNoTeams(): void + { + $project = new Project(); + $project->setCustomer(new Customer('foo')); + + $this->assertVote(new User(), $project, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessGrantedWhenProjectHasNoCustomer(): void + { + // checkTeamAccessProject() skips the customer chain when project->getCustomer() is null. + $project = new Project(); + + $this->assertVote(new User(), $project, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessGrantedForCanSeeAllDataDespiteRestrictiveTeams(): void + { + $project = new Project(); + $customer = new Customer('foo'); + $customer->addTeam(new Team('customerTeam')); + $project->setCustomer($customer); + $project->addTeam(new Team('projectTeam')); + + $user = new User(); + $user->initCanSeeAllData(true); + + $this->assertVote($user, $project, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessGrantedWhenUserMemberOfCustomerTeamAndNoProjectTeams(): void + { + $customerTeam = new Team('customerTeam'); + $customer = new Customer('foo'); + $customer->addTeam($customerTeam); + + $project = new Project(); + $project->setCustomer($customer); + + $user = new User(); + $customerTeam->addUser($user); + + $this->assertVote($user, $project, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessGrantedWhenUserMemberOfProjectTeamAndNoCustomerTeams(): void + { + $project = new Project(); + $project->setCustomer(new Customer('foo')); + $projectTeam = new Team('projectTeam'); + $project->addTeam($projectTeam); + + $user = new User(); + $projectTeam->addUser($user); + + $this->assertVote($user, $project, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessGrantedWhenUserMemberOfBothCustomerAndProjectTeams(): void + { + $customerTeam = new Team('customerTeam'); + $customer = new Customer('foo'); + $customer->addTeam($customerTeam); + + $project = new Project(); + $project->setCustomer($customer); + $projectTeam = new Team('projectTeam'); + $project->addTeam($projectTeam); + + $user = new User(); + $customerTeam->addUser($user); + $projectTeam->addUser($user); + + $this->assertVote($user, $project, 'access', VoterInterface::ACCESS_GRANTED); + } + + public function testAccessDeniedWhenCustomerTeamBlocks(): void + { + $customer = new Customer('foo'); + $customer->addTeam(new Team('customerTeam')); + + $project = new Project(); + $project->setCustomer($customer); + // project has no teams of its own — would otherwise be permissive + $this->assertVote(new User(), $project, 'access', VoterInterface::ACCESS_DENIED); + } + + public function testAccessDeniedWhenCustomerOkButProjectTeamBlocks(): void + { + $customerTeam = new Team('customerTeam'); + $customer = new Customer('foo'); + $customer->addTeam($customerTeam); + + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam(new Team('projectTeam')); + + $user = new User(); + $customerTeam->addUser($user); + + $this->assertVote($user, $project, 'access', VoterInterface::ACCESS_DENIED); + } + + public function testAccessDeniedWhenProjectTeamBlocksAndCustomerHasNoTeams(): void + { + $project = new Project(); + $project->setCustomer(new Customer('foo')); + $project->addTeam(new Team('projectTeam')); + + $this->assertVote(new User(), $project, 'access', VoterInterface::ACCESS_DENIED); + } + + public function testAccessDeniedForUserInUnrelatedTeam(): void + { + $unrelated = new Team('unrelated'); + $user = new User(); + $unrelated->addTeamlead($user); + + $customer = new Customer('foo'); + $customer->addTeam(new Team('customerTeam')); + + $project = new Project(); + $project->setCustomer($customer); + + $this->assertVote($user, $project, 'access', VoterInterface::ACCESS_DENIED); + } + + public function testAccessDeniedForNonUserToken(): void + { + $project = new Project(); + $token = new UsernamePasswordToken(new InMemoryUser('anon', null), 'bar', []); + $sut = $this->getVoter(ProjectVoter::class); + + self::assertEquals(VoterInterface::ACCESS_DENIED, $sut->vote($token, $project, ['access'])); + } } diff --git a/tests/Voter/TimesheetVoterTest.php b/tests/Voter/TimesheetVoterTest.php index c4723c1e..4ab19914 100644 --- a/tests/Voter/TimesheetVoterTest.php +++ b/tests/Voter/TimesheetVoterTest.php @@ -13,6 +13,7 @@ use App\Configuration\ConfigLoaderInterface; use App\Entity\Activity; use App\Entity\Customer; use App\Entity\Project; +use App\Entity\Team; use App\Entity\Timesheet; use App\Entity\User; use App\Tests\Mocks\SystemConfigurationFactory; @@ -31,7 +32,7 @@ class TimesheetVoterTest extends AbstractVoterTestCase return $this->getLockdownVoter(); } - protected function assertVote(User $user, $subject, $attribute, $result): void + private function assertVote(User $user, Timesheet $subject, string $attribute, int $result): void { $token = new UsernamePasswordToken($user, 'bar', $user->getRoles()); $sut = $this->getVoter(TimesheetVoter::class); @@ -39,28 +40,27 @@ class TimesheetVoterTest extends AbstractVoterTestCase self::assertEquals($result, $sut->vote($token, $subject, [$attribute])); } - public function testVote(): void + #[DataProvider('getTestData')] + public function testVote(User $user, Timesheet $subject, string $attribute, int $result): void { - foreach ($this->getTestData() as $row) { - $this->assertVote($row[0], $row[1], $row[2], $row[3]); - } + $this->assertVote($user, $subject, $attribute, $result); } - public function getTestData() + public static function getTestData() { - $user0 = $this->getTestUser(0, 'unknown'); - $user1 = $this->getTestUser(1, User::ROLE_USER); - $user2 = $this->getTestUser(2, User::ROLE_TEAMLEAD); - $user3 = $this->getTestUser(3, User::ROLE_ADMIN); - $user4 = $this->getTestUser(4, User::ROLE_SUPER_ADMIN); + $user0 = self::getTestUser(0, 'unknown'); + $user1 = self::getTestUser(1, User::ROLE_USER); + $user2 = self::getTestUser(2, User::ROLE_TEAMLEAD); + $user3 = self::getTestUser(3, User::ROLE_ADMIN); + $user4 = self::getTestUser(4, User::ROLE_SUPER_ADMIN); - $timesheet1 = $this->getTimesheet($user1); - $timesheet2 = $this->getTimesheet($user2); - $timesheet3 = $this->getTimesheet($user3); - $timesheet4 = $this->getTimesheet($user4); - $timesheet5 = $this->getTimesheet($user2); + $timesheet1 = self::getTimesheet($user1); + $timesheet2 = self::getTimesheet($user2); + $timesheet3 = self::getTimesheet($user3); + $timesheet4 = self::getTimesheet($user4); + $timesheet5 = self::getTimesheet($user2); $timesheet5->setExported(true); - $timesheet6 = $this->getTimesheet($user1); + $timesheet6 = self::getTimesheet($user1); $timesheet6->getActivity()?->setVisible(false); $result = VoterInterface::ACCESS_GRANTED; @@ -97,7 +97,7 @@ class TimesheetVoterTest extends AbstractVoterTestCase #[DataProvider('getLockDownTestData')] public function testWithLockdown(string $permission, int $expected, string $beginModifier, string $lockdownBegin, string $lockdownEnd, ?string $lockdownGrace): void { - $user = $this->getTestUser(1, User::ROLE_USER); + $user = self::getTestUser(1, User::ROLE_USER); $begin = new \DateTime('now'); $begin->modify($beginModifier); @@ -125,16 +125,16 @@ class TimesheetVoterTest extends AbstractVoterTestCase public function testSpecialCases(): void { - $user1 = $this->getTestUser(1, User::ROLE_USER); - $user2 = $this->getTestUser(2, User::ROLE_TEAMLEAD); - $user3 = $this->getTestUser(3, User::ROLE_ADMIN); - $user4 = $this->getTestUser(4, User::ROLE_SUPER_ADMIN); + $user1 = self::getTestUser(1, User::ROLE_USER); + $user2 = self::getTestUser(2, User::ROLE_TEAMLEAD); + $user3 = self::getTestUser(3, User::ROLE_ADMIN); + $user4 = self::getTestUser(4, User::ROLE_SUPER_ADMIN); // unknown attribute - $timesheet = $this->getTimesheet($user3); + $timesheet = self::getTimesheet($user3); $this->assertVote($user3, $timesheet, 'edit2', VoterInterface::ACCESS_ABSTAIN); - $timesheet = $this->getTimesheet($user2); + $timesheet = self::getTimesheet($user2); $timesheet->setExported(true); // edit exported timesheet disallowed for teamleads $this->assertVote($user2, $timesheet, 'edit', VoterInterface::ACCESS_DENIED); @@ -144,17 +144,17 @@ class TimesheetVoterTest extends AbstractVoterTestCase $this->assertVote($user4, $timesheet, 'delete', VoterInterface::ACCESS_GRANTED); // hidden activities might not be started - $timesheet = $this->getTimesheet($user1); + $timesheet = self::getTimesheet($user1); $timesheet->getActivity()?->setVisible(false); $this->assertVote($user2, $timesheet, 'start', VoterInterface::ACCESS_DENIED); // hidden projects might not be started - $timesheet = $this->getTimesheet($user1); + $timesheet = self::getTimesheet($user1); $timesheet->getProject()?->setVisible(false); $this->assertVote($user2, $timesheet, 'start', VoterInterface::ACCESS_DENIED); // hidden customers might not be started - $timesheet = $this->getTimesheet($user1); + $timesheet = self::getTimesheet($user1); $timesheet->getProject()?->getCustomer()?->setVisible(false); $this->assertVote($user2, $timesheet, 'start', VoterInterface::ACCESS_DENIED); // cannot start timesheet without activity @@ -169,38 +169,34 @@ class TimesheetVoterTest extends AbstractVoterTestCase $this->assertVote($user2, $timesheet, 'start', VoterInterface::ACCESS_DENIED); } - protected function getTimesheet($user): Timesheet + private static function getTimesheet($user): Timesheet { - $timesheet = new Timesheet(); - $timesheet->setUser($user); - $activity = new Activity(); $project = new Project(); + $customer = new Customer('foo'); $activity->setProject($project); + $project->setCustomer($customer); + + $timesheet = new Timesheet(); + $timesheet->setUser($user); $timesheet->setProject($project); $timesheet->setActivity($activity); - $customer = new Customer('foo'); - $project->setCustomer($customer); return $timesheet; } - /** - * @param int $id - * @param string $role - * @return User - */ - protected function getTestUser(int $id, string $role): User + private static function getTestUser(int $id, string $role): User { - $user = $this->createMock(User::class); + $user = self::createStub(User::class); $user->method('getId')->willReturn($id); $user->method('getRoles')->willReturn([$role]); + $user->method('getTeams')->willReturn([]); $user->method('getTimezone')->willReturn(date_default_timezone_get()); return $user; } - protected function getLockdownVoter(?string $lockdownBegin = null, ?string $lockdownEnd = null, ?string $lockdownGrace = null): TimesheetVoter + private function getLockdownVoter(?string $lockdownBegin = null, ?string $lockdownEnd = null, ?string $lockdownGrace = null): TimesheetVoter { $loader = $this->createMock(ConfigLoaderInterface::class); $config = SystemConfigurationFactory::create($loader, [ @@ -215,4 +211,413 @@ class TimesheetVoterTest extends AbstractVoterTestCase return new TimesheetVoter($this->getRolePermissionManager(), new LockdownService($config)); } + + /** + * @return array + */ + public static function teamCheckedAttributes(): array + { + // Attributes that go through the new checkTeamAccessTimesheet() gate when + // accessing another user's timesheet. 'start' is excluded because it + // additionally requires visibility on project/activity (canStart()). + return ['view', 'edit', 'delete', 'export', 'view_rate', 'edit_rate', 'edit_export', 'edit_billable', 'stop']; + } + + public function testOwnerCanAccessOwnTimesheetEvenWithRestrictiveTeams(): void + { + // Owner short-circuit: the team gate must NOT apply to the user's own timesheet. + $owner = self::getUser(1, User::ROLE_USER); + + $customer = new Customer('Acme'); + $customer->addTeam(new Team('locked customer team')); + + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam(new Team('locked project team')); + + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam(new Team('locked activity team')); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + $this->assertVote($owner, $timesheet, 'view', VoterInterface::ACCESS_GRANTED); + $this->assertVote($owner, $timesheet, 'edit', VoterInterface::ACCESS_GRANTED); + $this->assertVote($owner, $timesheet, 'delete', VoterInterface::ACCESS_GRANTED); + $this->assertVote($owner, $timesheet, 'export', VoterInterface::ACCESS_GRANTED); + } + + public function testTeamleadDeniedWhenOnlyPlainMemberOfOwnerTeam(): void + { + // Headline new behaviour: a TEAMLEAD role with view_other_timesheet must + // not access another user's timesheet by being a plain team member — + // they must be the team's teamlead. + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_TEAMLEAD); + + $sharedTeam = new Team('shared'); + $sharedTeam->addUser($owner); + $sharedTeam->addUser($requester); + + $timesheet = self::getTimesheetFor($owner); + + foreach (self::teamCheckedAttributes() as $attribute) { + $this->assertVote($requester, $timesheet, $attribute, VoterInterface::ACCESS_DENIED); + } + } + + public function testTeamleadGrantedWhenTeamleadOfOwnerTeam(): void + { + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_TEAMLEAD); + + $sharedTeam = new Team('shared'); + $sharedTeam->addUser($owner); + $sharedTeam->addTeamlead($requester); + + $timesheet = self::getTimesheetFor($owner); + + $this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_GRANTED); + $this->assertVote($requester, $timesheet, 'edit', VoterInterface::ACCESS_GRANTED); + $this->assertVote($requester, $timesheet, 'delete', VoterInterface::ACCESS_GRANTED); + $this->assertVote($requester, $timesheet, 'export', VoterInterface::ACCESS_GRANTED); + } + + public function testTeamleadDeniedWhenOnlyTeamleadOfUnrelatedTeam(): void + { + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_TEAMLEAD); + + $ownerTeam = new Team('owner team'); + $ownerTeam->addUser($owner); + + $unrelated = new Team('unrelated'); + $unrelated->addTeamlead($requester); + + $timesheet = self::getTimesheetFor($owner); + + $this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_DENIED); + $this->assertVote($requester, $timesheet, 'edit', VoterInterface::ACCESS_DENIED); + } + + public function testTeamleadGrantedWhenOwnerHasNoTeams(): void + { + // Owner has no teams -> teamlead gate is permissive. + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_TEAMLEAD); + + $timesheet = self::getTimesheetFor($owner); + + $this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_GRANTED); + $this->assertVote($requester, $timesheet, 'edit', VoterInterface::ACCESS_GRANTED); + } + + public function testTeamleadDeniedWhenCustomerTeamBlocks(): void + { + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_TEAMLEAD); + + // Even though requester is a teamlead of the owner's team, the customer + // team blocks access. + $sharedTeam = new Team('shared'); + $sharedTeam->addUser($owner); + $sharedTeam->addTeamlead($requester); + + $customer = new Customer('Acme'); + $customer->addTeam(new Team('locked customer team')); + + $project = new Project(); + $project->setCustomer($customer); + $activity = new Activity(); + $activity->setProject($project); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + $this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_DENIED); + $this->assertVote($requester, $timesheet, 'edit', VoterInterface::ACCESS_DENIED); + $this->assertVote($requester, $timesheet, 'delete', VoterInterface::ACCESS_DENIED); + } + + public function testTeamleadDeniedWhenProjectTeamBlocks(): void + { + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_TEAMLEAD); + + $sharedTeam = new Team('shared'); + $sharedTeam->addUser($owner); + $sharedTeam->addTeamlead($requester); + + $project = new Project(); + $project->setCustomer(new Customer('Acme')); + $project->addTeam(new Team('locked project team')); + + $activity = new Activity(); + $activity->setProject($project); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + $this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_DENIED); + } + + public function testTeamleadDeniedWhenActivityTeamBlocks(): void + { + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_TEAMLEAD); + + $sharedTeam = new Team('shared'); + $sharedTeam->addUser($owner); + $sharedTeam->addTeamlead($requester); + + $project = new Project(); + $project->setCustomer(new Customer('Acme')); + + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam(new Team('locked activity team')); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + $this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_DENIED); + } + + public function testTeamleadGrantedThroughFullTeamChainAsMemberAndTeamlead(): void + { + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_TEAMLEAD); + + $customerTeam = new Team('customer team'); + $customer = new Customer('Acme'); + $customer->addTeam($customerTeam); + $customerTeam->addUser($requester); + + $projectTeam = new Team('project team'); + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam($projectTeam); + $projectTeam->addUser($requester); + + $activityTeam = new Team('activity team'); + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam($activityTeam); + $activityTeam->addUser($requester); + + $ownerTeam = new Team('owner team'); + $ownerTeam->addUser($owner); + $ownerTeam->addTeamlead($requester); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + $this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_GRANTED); + $this->assertVote($requester, $timesheet, 'edit', VoterInterface::ACCESS_GRANTED); + $this->assertVote($requester, $timesheet, 'delete', VoterInterface::ACCESS_GRANTED); + $this->assertVote($requester, $timesheet, 'export', VoterInterface::ACCESS_GRANTED); + $this->assertVote($requester, $timesheet, 'view_rate', VoterInterface::ACCESS_GRANTED); + } + + public function testSuperAdminCanAccessOtherTimesheetDespiteRestrictiveTeams(): void + { + // SUPER_ADMIN gets canSeeAllData via isSuperAdmin() — short-circuits every team gate. + $owner = self::getUser(1, User::ROLE_USER); + $admin = self::getUser(99, User::ROLE_SUPER_ADMIN); + + $ownerTeam = new Team('owner team'); + $ownerTeam->addUser($owner); + + $customer = new Customer('Acme'); + $customer->addTeam(new Team('locked customer team')); + + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam(new Team('locked project team')); + + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam(new Team('locked activity team')); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + self::assertTrue($admin->canSeeAllData()); + $this->assertVote($admin, $timesheet, 'view', VoterInterface::ACCESS_GRANTED); + $this->assertVote($admin, $timesheet, 'edit', VoterInterface::ACCESS_GRANTED); + $this->assertVote($admin, $timesheet, 'delete', VoterInterface::ACCESS_GRANTED); + $this->assertVote($admin, $timesheet, 'export', VoterInterface::ACCESS_GRANTED); + } + + public function testAdminWithoutCanSeeAllDataIsBlockedByCustomerTeam(): void + { + // ROLE_ADMIN does NOT automatically have canSeeAllData() — only SUPER_ADMIN does. + // An ADMIN without the flag is subject to the team gate just like everyone else. + // Documents that the new check tightens admin access too. + $owner = self::getUser(1, User::ROLE_USER); + $admin = self::getUser(3, User::ROLE_ADMIN); + self::assertFalse($admin->canSeeAllData()); + + $customer = new Customer('Acme'); + $customer->addTeam(new Team('locked customer team')); + + $project = new Project(); + $project->setCustomer($customer); + $activity = new Activity(); + $activity->setProject($project); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + $this->assertVote($admin, $timesheet, 'view', VoterInterface::ACCESS_DENIED); + $this->assertVote($admin, $timesheet, 'edit', VoterInterface::ACCESS_DENIED); + } + + public function testAdminCanSeeAllDataBypassesAllTeamGates(): void + { + // Activating canSeeAllData on a non-super-admin must restore full access. + $owner = self::getUser(1, User::ROLE_USER); + $admin = self::getUser(3, User::ROLE_ADMIN); + $admin->initCanSeeAllData(true); + + $ownerTeam = new Team('owner team'); + $ownerTeam->addUser($owner); + + $customer = new Customer('Acme'); + $customer->addTeam(new Team('locked customer team')); + + $project = new Project(); + $project->setCustomer($customer); + $project->addTeam(new Team('locked project team')); + + $activity = new Activity(); + $activity->setProject($project); + $activity->addTeam(new Team('locked activity team')); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + $this->assertVote($admin, $timesheet, 'view', VoterInterface::ACCESS_GRANTED); + $this->assertVote($admin, $timesheet, 'edit', VoterInterface::ACCESS_GRANTED); + } + + public function testRoleUserDeniedOnOtherTimesheetEvenWhenTeamGatePasses(): void + { + // ROLE_USER has only *_own_timesheet permissions. The team gate may pass, + // but the role still has no _other permission → denied. Defence-in-depth check. + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_USER); + + // No teams anywhere — team gate is fully permissive. + $timesheet = self::getTimesheetFor($owner); + + $this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_DENIED); + $this->assertVote($requester, $timesheet, 'edit', VoterInterface::ACCESS_DENIED); + $this->assertVote($requester, $timesheet, 'export', VoterInterface::ACCESS_DENIED); + } + + public function testTeamleadGrantedWhenTeamleadOfOneOfMultipleOwnerTeams(): void + { + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_TEAMLEAD); + + $teamA = new Team('A'); + $teamB = new Team('B'); + $teamA->addUser($owner); + $teamB->addUser($owner); + + // Plain member of A, teamlead of B — granted because at least one match. + $teamA->addUser($requester); + $teamB->addTeamlead($requester); + + $timesheet = self::getTimesheetFor($owner); + + $this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_GRANTED); + } + + public function testTeamleadDeniedWhenMemberOfAllButTeamleadOfNoneOfOwnerTeams(): void + { + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_TEAMLEAD); + + $teamA = new Team('A'); + $teamB = new Team('B'); + $teamA->addUser($owner); + $teamB->addUser($owner); + $teamA->addUser($requester); + $teamB->addUser($requester); + + $timesheet = self::getTimesheetFor($owner); + + $this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_DENIED); + } + + public function testTeamleadGrantedWhenOwnerTeamHasNoTeamsAndChainsAreMemberOnly(): void + { + // Confirms that the customer/project/activity gate uses the simpler + // "is member" rule (not teamlead) — only the final owner-team gate + // requires teamlead. + $owner = self::getUser(1, User::ROLE_USER); + $requester = self::getUser(2, User::ROLE_TEAMLEAD); + + $customerTeam = new Team('customer team'); + $customer = new Customer('Acme'); + $customer->addTeam($customerTeam); + $customerTeam->addUser($requester); // plain member is enough here + + $project = new Project(); + $project->setCustomer($customer); + $activity = new Activity(); + $activity->setProject($project); + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + // Owner has no teams -> teamlead gate is permissive. + $this->assertVote($requester, $timesheet, 'view', VoterInterface::ACCESS_GRANTED); + } + + private static function getTimesheetFor(User $owner, ?Team $customerTeam = null, ?Team $projectTeam = null, ?Team $activityTeam = null): Timesheet + { + $customer = new Customer('Acme'); + if ($customerTeam !== null) { + $customer->addTeam($customerTeam); + } + $project = new Project(); + $project->setCustomer($customer); + if ($projectTeam !== null) { + $project->addTeam($projectTeam); + } + $activity = new Activity(); + $activity->setProject($project); + if ($activityTeam !== null) { + $activity->addTeam($activityTeam); + } + + $timesheet = new Timesheet(); + $timesheet->setUser($owner); + $timesheet->setProject($project); + $timesheet->setActivity($activity); + + return $timesheet; + } } diff --git a/tests/phpstan.neon b/tests/phpstan.neon index 4b79ca9e..6e604566 100644 --- a/tests/phpstan.neon +++ b/tests/phpstan.neon @@ -2371,21 +2371,6 @@ parameters: count: 1 path: Voter/RolePermissionVoterTest.php - - - message: "#^Method App\\\\Tests\\\\Voter\\\\TimesheetVoterTest\\:\\:assertVote\\(\\) has parameter \\$attribute with no type specified\\.$#" - count: 1 - path: Voter/TimesheetVoterTest.php - - - - message: "#^Method App\\\\Tests\\\\Voter\\\\TimesheetVoterTest\\:\\:assertVote\\(\\) has parameter \\$result with no type specified\\.$#" - count: 1 - path: Voter/TimesheetVoterTest.php - - - - message: "#^Method App\\\\Tests\\\\Voter\\\\TimesheetVoterTest\\:\\:assertVote\\(\\) has parameter \\$subject with no type specified\\.$#" - count: 1 - path: Voter/TimesheetVoterTest.php - - message: "#^Method App\\\\Tests\\\\Voter\\\\TimesheetVoterTest\\:\\:getLockDownTestData\\(\\) has no return type specified\\.$#" count: 1