diff --git a/app/Resources/translations/exceptions.de.xliff b/app/Resources/translations/exceptions.de.xliff index 3ebc85a6..feceb6fd 100644 --- a/app/Resources/translations/exceptions.de.xliff +++ b/app/Resources/translations/exceptions.de.xliff @@ -7,10 +7,6 @@ access.denied Der Zugriff wurde verweigert - - timesheet.deny.stop - Der Benutzer "%user%" darf den Zeiteintrag "%entry%" nicht aufrufen - diff --git a/app/Resources/views/macros/widgets.html.twig b/app/Resources/views/macros/widgets.html.twig index aba01f2c..a54df83c 100644 --- a/app/Resources/views/macros/widgets.html.twig +++ b/app/Resources/views/macros/widgets.html.twig @@ -17,6 +17,11 @@ {{ macro.label(role, 'primary') }} {% endmacro %} +{% macro label_activity(role) %} + {% import _self as macro %} + {{ macro.label(role, 'primary') }} +{% endmacro %} + {% macro label_project(project) %} {% import _self as macro %} {{ macro.label(project.name, 'primary') }} diff --git a/app/config/services.yml b/app/config/services.yml index cdcdcf0b..82c92a74 100644 --- a/app/config/services.yml +++ b/app/config/services.yml @@ -58,6 +58,13 @@ services: tags: - { name: security.voter } + # security voter to check activity access + timesheet.voter.timesheet: + class: TimesheetBundle\Voter\TimesheetVoter + arguments: ["@security.access.decision_manager"] + tags: + - { name: security.voter } + # ================================================================================ # FORMS # ================================================================================ diff --git a/src/TimesheetBundle/Controller/Admin/TimesheetController.php b/src/TimesheetBundle/Controller/Admin/TimesheetController.php index 80b99689..973016dd 100644 --- a/src/TimesheetBundle/Controller/Admin/TimesheetController.php +++ b/src/TimesheetBundle/Controller/Admin/TimesheetController.php @@ -66,6 +66,7 @@ class TimesheetController extends AbstractController * * @Route("/{id}/stop", name="admin_timesheet_stop") * @Method({"GET"}) + * @Security("is_granted('stop', entry)") * * @param Timesheet $entry * @return \Symfony\Component\HttpFoundation\RedirectResponse|\Symfony\Component\HttpFoundation\Response diff --git a/src/TimesheetBundle/Controller/TimesheetController.php b/src/TimesheetBundle/Controller/TimesheetController.php index 4c0e7bd2..2200a674 100644 --- a/src/TimesheetBundle/Controller/TimesheetController.php +++ b/src/TimesheetBundle/Controller/TimesheetController.php @@ -78,6 +78,7 @@ class TimesheetController extends AbstractController * * @Route("/{id}/stop", name="timesheet_stop") * @Method({"GET"}) + * @Security("is_granted('stop', entry)") * * @param Timesheet $entry * @param Request $request @@ -85,19 +86,6 @@ class TimesheetController extends AbstractController */ public function stopAction(Timesheet $entry, Request $request) { - $user = $this->getUser(); - - // make sure only ADMIN can stop other users entries - if ($user->getId() !== $entry->getUser()->getId()) { - // TODO move me to a voter - $this->denyUnlessGranted( - 'ROLE_ADMIN', - null, - 'timesheet.access.denied', - ['%user%' => $user->getId(), '%entry%' => $entry->getId()] - ); - } - try { $this->getRepository()->stopRecording($entry); $this->flashSuccess('timesheet.stop.success'); @@ -113,6 +101,7 @@ class TimesheetController extends AbstractController * * @Route("/start/{id}", name="timesheet_start", requirements={"id" = "\d+"}) * @Method({"GET", "POST"}) + * @Security("is_granted('start', activity)") * * @param Request $request * @return \Symfony\Component\HttpFoundation\RedirectResponse|\Symfony\Component\HttpFoundation\Response @@ -136,6 +125,7 @@ class TimesheetController extends AbstractController * * @Route("/{id}/edit", name="timesheet_edit") * @Method({"GET", "POST"}) + * @Security("is_granted('edit', entry)") * * @param Timesheet $entry * @param Request $request @@ -143,21 +133,7 @@ class TimesheetController extends AbstractController */ public function editAction(Timesheet $entry, Request $request) { - $user = $this->getUser(); - - // make sure only ADMIN can edit other users entries - if ($user->getId() !== $entry->getUser()->getId()) { - // TODO move me to a voter - $this->denyUnlessGranted( - 'ROLE_ADMIN', - null, - 'timesheet.access.denied', - ['%user%' => $user->getId(), '%entry%' => $entry->getId()] - ); - } - $editForm = $this->createEditForm($entry, $request->get('page')); - $editForm->handleRequest($request); if ($editForm->isSubmitted() && $editForm->isValid()) { diff --git a/src/TimesheetBundle/Repository/TimesheetRepository.php b/src/TimesheetBundle/Repository/TimesheetRepository.php index 560b20f5..d94d2f80 100644 --- a/src/TimesheetBundle/Repository/TimesheetRepository.php +++ b/src/TimesheetBundle/Repository/TimesheetRepository.php @@ -279,9 +279,10 @@ class TimesheetRepository extends AbstractRepository { $qb = $this->getEntityManager()->createQueryBuilder(); - $qb->select('t', 'a', 'p', 'c') + $qb->select('t', 'a', 'p', 'c', 'u') ->from('TimesheetBundle:Timesheet', 't') ->join('t.activity', 'a') + ->join('t.user', 'u') ->join('a.project', 'p') ->join('p.customer', 'c') ->orderBy('t.' . $query->getOrderBy(), $query->getOrder()); diff --git a/src/TimesheetBundle/Resources/views/admin/timesheet.html.twig b/src/TimesheetBundle/Resources/views/admin/timesheet.html.twig index 71579a86..fd2c5579 100644 --- a/src/TimesheetBundle/Resources/views/admin/timesheet.html.twig +++ b/src/TimesheetBundle/Resources/views/admin/timesheet.html.twig @@ -17,6 +17,7 @@ 'label.endtime': 'hidden-xs', 'label.duration': '', 'label.rate': '', + 'label.activity': 'hidden-xs hidden-sm', 'label.username': 'hidden-xs', 'label.description': 'hidden-xs hidden-sm', 'label.actions': '', @@ -35,6 +36,7 @@ ‐ ‐ {% endif %} + {{ widgets.label_activity(entry.activity.name) }} {{ entry.user.username }} {{ entry.description }} diff --git a/src/TimesheetBundle/Resources/views/timesheet/index.html.twig b/src/TimesheetBundle/Resources/views/timesheet/index.html.twig index 95430516..2ca3c68a 100644 --- a/src/TimesheetBundle/Resources/views/timesheet/index.html.twig +++ b/src/TimesheetBundle/Resources/views/timesheet/index.html.twig @@ -35,7 +35,7 @@ {{ entry.duration|duration }} ‐ {% endif %} - {{ entry.activity.name }} + {{ widgets.label_activity(entry.activity.name) }} {{ entry.description }} {% if entry.end %} diff --git a/src/TimesheetBundle/Voter/TimesheetVoter.php b/src/TimesheetBundle/Voter/TimesheetVoter.php new file mode 100644 index 00000000..4aa33f5a --- /dev/null +++ b/src/TimesheetBundle/Voter/TimesheetVoter.php @@ -0,0 +1,173 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace TimesheetBundle\Voter; + +use AppBundle\Entity\User; +use AppBundle\Voter\AbstractVoter; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use TimesheetBundle\Entity\Activity; +use TimesheetBundle\Entity\Timesheet; + +/** + * A voter to check permissions on Timesheets. + * + * @author Kevin Papst + */ +class TimesheetVoter extends AbstractVoter +{ + const START = 'start'; + const STOP = 'stop'; + const VIEW = 'view'; + const EDIT = 'edit'; + const DELETE = 'delete'; + + /** + * @param string $attribute + * @param mixed $subject + * @return bool + */ + protected function supports($attribute, $subject) + { + if (!in_array($attribute, array(self::START, self::STOP, self::VIEW, self::EDIT, self::DELETE))) { + return false; + } + + if ($subject instanceof Activity && $attribute == self::START) { + return true; + } + + if (!$subject instanceof Timesheet) { + return false; + } + + return true; + } + + /** + * @param string $attribute + * @param Timesheet]Activity $subject + * @param TokenInterface $token + * @return bool + */ + protected function voteOnAttribute($attribute, $subject, TokenInterface $token) + { + $user = $token->getUser(); + + if (!$user instanceof User) { + return false; + } + + // Customer cannot do anything with timesheet entries + if (!$this->hasRole('ROLE_USER', $token)) { + return false; + } + + switch ($attribute) { + case self::STOP: + return $this->canStop($subject, $user, $token); + case self::START: + return $this->canStart($subject, $user, $token); + case self::VIEW: + return $this->canView($subject, $user, $token); + case self::EDIT: + return $this->canEdit($subject, $user, $token); + case self::DELETE: + return $this->canDelete($subject, $user, $token); + } + + return false; + } + + /** + * @param Timesheet $timesheet + * @param User $user + * @param TokenInterface $token + * @return bool + */ + protected function canStop(Timesheet $timesheet, User $user, TokenInterface $token) + { + // if a teamlead stops an entry for another user, check that this user is part of his team + return $this->isOwnOrTeamlead($timesheet, $user, $token); + } + + /** + * @param Activity $activity + * @param User $user + * @param TokenInterface $token + * @return bool + */ + protected function canStart(Activity $activity, User $user, TokenInterface $token) + { + // we could check the amount of active entries + // TODO limit to activities that are not hidden + + // if a teamlead starts an entry for another user, check that this user is part of his team + return true; + } + + /** + * @param Timesheet $timesheet + * @param User $user + * @param TokenInterface $token + * @return bool + */ + protected function canView(Timesheet $timesheet, User $user, TokenInterface $token) + { + return $this->isOwnOrTeamlead($timesheet, $user, $token); + } + + /** + * @param Timesheet $timesheet + * @param User $user + * @param TokenInterface $token + * @return bool + */ + protected function canEdit(Timesheet $timesheet, User $user, TokenInterface $token) + { + return $this->isOwnOrTeamlead($timesheet, $user, $token); + } + + /** + * @param TokenInterface $token + * @return bool + */ + protected function canDelete(Timesheet $timesheet, User $user, TokenInterface $token) + { + return $this->isOwnOrAdmin($timesheet, $user, $token); + } + + /** + * @param TokenInterface $token + * @return bool + */ + protected function isOwnOrTeamlead(Timesheet $timesheet, User $user, TokenInterface $token) + { + if ($timesheet->getUser()->getId() == $user->getId()) { + return true; + } + + return $this->hasRole('ROLE_TEAMLEAD', $token); + } + + /** + * @param TokenInterface $token + * @return bool + */ + protected function isOwnOrAdmin(Timesheet $timesheet, User $user, TokenInterface $token) + { + if ($timesheet->getUser()->getId() == $user->getId()) { + return true; + } + + return $this->hasRole('ROLE_ADMIN', $token); + } +}