diff --git a/.scrutinizer.yml b/.scrutinizer.yml index 3ccf133f..ac246c3e 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -38,5 +38,5 @@ filter: build_failure_conditions: - 'project.metric("scrutinizer.quality", < 9.30)' - - 'project.metric("scrutinizer.test_coverage", < 0.70)' - - 'project.metric_change("scrutinizer.test_coverage", < 0.01)' \ No newline at end of file + - 'project.metric("scrutinizer.test_coverage", < 0.73)' + - 'project.metric_change("scrutinizer.test_coverage", < -0.01)' \ No newline at end of file diff --git a/README.md b/README.md index bb6bb6bc..0754f280 100644 --- a/README.md +++ b/README.md @@ -57,8 +57,9 @@ cd kimai2/ Make sure the [file permissions are correct](https://symfony.com/doc/current/setup/file_permissions.html) and create your `.env` file: ```bash -chown -R www-data var/ -chmod -R 777 var/ +chown -R :www-data . +chmod -R g+r . +chmod -R g+rw var/ cp .env.dist .env ``` diff --git a/src/Controller/InvoiceController.php b/src/Controller/InvoiceController.php index 1fe9ccbb..74580ecc 100644 --- a/src/Controller/InvoiceController.php +++ b/src/Controller/InvoiceController.php @@ -77,6 +77,7 @@ class InvoiceController extends AbstractController /** * @Route(path="/", name="invoice", methods={"GET"}) + * @Security("is_granted('view', 'invoice')") * * @param Request $request * @return \Symfony\Component\HttpFoundation\Response @@ -110,6 +111,7 @@ class InvoiceController extends AbstractController /** * @Route(path="/print", name="invoice_print", methods={"GET"}) + * @Security("is_granted('create', 'invoice')") * * @param Request $request * @return \Symfony\Component\HttpFoundation\Response @@ -209,8 +211,7 @@ class InvoiceController extends AbstractController /** * @Route(path="/template", defaults={"page": 1}, name="admin_invoice_template", methods={"GET", "POST"}) * @Route(path="/template/page/{page}", requirements={"page": "[1-9]\d*"}, name="admin_invoice_template_paginated", methods={"GET", "POST"}) - * - * TODO permission + * @Security("is_granted('view', 'invoice_template')") * * @param $page * @return \Symfony\Component\HttpFoundation\Response @@ -227,8 +228,7 @@ class InvoiceController extends AbstractController /** * @Route(path="/template/{id}/edit", name="admin_invoice_template_edit", methods={"GET", "POST"}) - * - * TODO permission + * @Security("is_granted('edit', template)") * * @param InvoiceTemplate $template * @param Request $request @@ -242,8 +242,7 @@ class InvoiceController extends AbstractController /** * @Route(path="/template/create", name="admin_invoice_template_create", methods={"GET", "POST"}) - * - * TODO permission + * @Security("is_granted('create', 'invoice_template')") * * @param Request $request * @return \Symfony\Component\HttpFoundation\Response @@ -261,9 +260,8 @@ class InvoiceController extends AbstractController /** * The route to delete an existing template. * - * TODO permission - * * @Route(path="/template/{id}/delete", name="admin_invoice_template_delete", methods={"GET", "POST"}) + * @Security("is_granted('delete', template)") * * @param InvoiceTemplate $template * @param Request $request diff --git a/src/Security/AclDecisionManager.php b/src/Security/AclDecisionManager.php new file mode 100644 index 00000000..2ee17015 --- /dev/null +++ b/src/Security/AclDecisionManager.php @@ -0,0 +1,61 @@ +decisionManager = $decisionManager; + } + + /** + * @param TokenInterface $token + * @return bool + */ + public function isFullyAuthenticated(TokenInterface $token) + { + if ($this->decisionManager->decide($token, ['IS_AUTHENTICATED_FULLY'])) { + return true; + } + + return false; + } + + /** + * @param TokenInterface $token + * @param string|array $roles + * @return bool + */ + public function hasRole(TokenInterface $token, $roles) + { + if (!is_array($roles)) { + $roles = [$roles]; + } + + if ($this->decisionManager->decide($token, $roles)) { + return true; + } + + return false; + } +} diff --git a/src/Voter/AbstractVoter.php b/src/Voter/AbstractVoter.php index edb17241..b9eda969 100644 --- a/src/Voter/AbstractVoter.php +++ b/src/Voter/AbstractVoter.php @@ -9,8 +9,8 @@ namespace App\Voter; +use App\Security\AclDecisionManager; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; -use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\Voter\Voter; /** @@ -19,15 +19,15 @@ use Symfony\Component\Security\Core\Authorization\Voter\Voter; abstract class AbstractVoter extends Voter { /** - * @var AccessDecisionManagerInterface + * @var AclDecisionManager */ protected $decisionManager; /** * AbstractVoter constructor. - * @param AccessDecisionManagerInterface $decisionManager + * @param AclDecisionManager $decisionManager */ - public function __construct(AccessDecisionManagerInterface $decisionManager) + public function __construct(AclDecisionManager $decisionManager) { $this->decisionManager = $decisionManager; } @@ -38,11 +38,7 @@ abstract class AbstractVoter extends Voter */ protected function isFullyAuthenticated(TokenInterface $token) { - if ($this->decisionManager->decide($token, ['IS_AUTHENTICATED_FULLY'])) { - return true; - } - - return false; + return $this->decisionManager->isFullyAuthenticated($token); } /** @@ -52,10 +48,6 @@ abstract class AbstractVoter extends Voter */ protected function hasRole($role, TokenInterface $token) { - if ($this->decisionManager->decide($token, [$role])) { - return true; - } - - return false; + return $this->decisionManager->hasRole($token, [$role]); } } diff --git a/src/Voter/InvoiceVoter.php b/src/Voter/InvoiceVoter.php new file mode 100644 index 00000000..e3357f18 --- /dev/null +++ b/src/Voter/InvoiceVoter.php @@ -0,0 +1,136 @@ +getUser(); + + if (!$user instanceof User) { + return false; + } + + switch ($attribute) { + case self::VIEW: + return $this->canView($user, $token); + case self::CREATE: + return $this->canCreate($user, $token); + case self::EDIT: + return $this->canEdit($user, $token); + case self::DELETE: + return $this->canDelete($token); + } + + return false; + } + + /** + * @param User $user + * @param TokenInterface $token + * @return bool + */ + protected function canView(User $user, TokenInterface $token) + { + if ($this->canEdit($user, $token)) { + return true; + } + + return false; + } + + /** + * @param User $user + * @param TokenInterface $token + * @return bool + */ + protected function canCreate(User $user, TokenInterface $token) + { + if ($this->canDelete($token)) { + return true; + } + + return false; + } + + /** + * @param User $user + * @param TokenInterface $token + * @return bool + */ + protected function canEdit(User $user, TokenInterface $token) + { + if ($this->canDelete($token)) { + return true; + } + + return false; + } + + /** + * @param TokenInterface $token + * @return bool + */ + protected function canDelete(TokenInterface $token) + { + return $this->isFullyAuthenticated($token) && $this->hasRole('ROLE_TEAMLEAD', $token); + } +} diff --git a/tests/Voter/InvoiceVoterTest.php b/tests/Voter/InvoiceVoterTest.php new file mode 100644 index 00000000..9c75fa8f --- /dev/null +++ b/tests/Voter/InvoiceVoterTest.php @@ -0,0 +1,80 @@ +getRoles()); + + $accessManager = $this->getMockBuilder(AclDecisionManager::class)->disableOriginalConstructor()->getMock(); + $accessManager->method('isFullyAuthenticated')->willReturn($isAuthenticated); + $accessManager->method('hasRole')->willReturn($hasRole); + + $sut = new InvoiceVoter($accessManager); + + $this->assertEquals($result, $sut->vote($token, $subject, [$attribute])); + } + + public function getTestData() + { + $user0 = $this->getUser(0, User::ROLE_CUSTOMER); + $user1 = $this->getUser(1, User::ROLE_USER); + $user2 = $this->getUser(1, User::ROLE_TEAMLEAD); + $user3 = $this->getUser(1, User::ROLE_ADMIN); + $user4 = $this->getUser(1, User::ROLE_SUPER_ADMIN); + + $users = [$user0, $user1, $user2, $user3, $user4]; + $attributes = [InvoiceVoter::VIEW, InvoiceVoter::CREATE, InvoiceVoter::EDIT, InvoiceVoter::DELETE]; + $subjects = ['invoice', 'invoice_template', new InvoiceTemplate()]; + + foreach ($attributes as $attribute) { + foreach ($users as $user) { + foreach ($subjects as $subject) { + yield [$user, false, false, $subject, $attribute, VoterInterface::ACCESS_DENIED]; + yield [$user, true, false, $subject, $attribute, VoterInterface::ACCESS_DENIED]; + yield [$user, false, true, $subject, $attribute, VoterInterface::ACCESS_DENIED]; + yield [$user, true, true, $subject, $attribute, VoterInterface::ACCESS_GRANTED]; + yield [$user, true, true, $subject, 'something', VoterInterface::ACCESS_ABSTAIN]; + } + yield [$user, true, true, new \stdClass(), $attribute, VoterInterface::ACCESS_ABSTAIN]; + yield [$user, false, true, new \stdClass(), $attribute, VoterInterface::ACCESS_ABSTAIN]; + yield [$user, true, false, new \stdClass(), $attribute, VoterInterface::ACCESS_ABSTAIN]; + yield [$user, false, false, new \stdClass(), $attribute, VoterInterface::ACCESS_ABSTAIN]; + yield [$user, true, true, null, $attribute, VoterInterface::ACCESS_ABSTAIN]; + yield [$user, true, true, 'foo', $attribute, VoterInterface::ACCESS_ABSTAIN]; + } + } + } + + protected function getUser($id, $role) + { + $user = $this->getMockBuilder(User::class)->getMock(); + $user->method('getId')->willReturn($id); + $user->method('getRoles')->willReturn([$role]); + + return $user; + } +} diff --git a/tests/Voter/TimesheetVoterTest.php b/tests/Voter/TimesheetVoterTest.php index 0f42822b..d47cb260 100644 --- a/tests/Voter/TimesheetVoterTest.php +++ b/tests/Voter/TimesheetVoterTest.php @@ -12,10 +12,10 @@ namespace App\Tests\Voter; use App\Entity\Customer; use App\Entity\Timesheet; use App\Entity\User; +use App\Security\AclDecisionManager; use App\Voter\TimesheetVoter; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; -use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -26,12 +26,13 @@ class TimesheetVoterTest extends TestCase /** * @dataProvider getTestData */ - public function testCustomerIsDisallowed(User $user, $allow, $subject, $attributes, $result) + public function testVote(User $user, $allow, $subject, $attributes, $result) { $token = new UsernamePasswordToken($user, 'foo', 'bar', $user->getRoles()); - $accessManager = $this->getMockBuilder(AccessDecisionManagerInterface::class)->getMock(); - $accessManager->method('decide')->willReturn($allow); + $accessManager = $this->getMockBuilder(AclDecisionManager::class)->disableOriginalConstructor()->getMock(); + $accessManager->method('isFullyAuthenticated')->willReturn($allow); + $accessManager->method('hasRole')->willReturn($allow); $sut = new TimesheetVoter($accessManager);