From 96de95b7c9c5376c3ce484153a7b38aa1fea7b09 Mon Sep 17 00:00:00 2001
From: Kevin Papst <kevinpapst@users.noreply.github.com>
Date: Fri, 5 Jan 2018 14:10:08 +0100
Subject: [PATCH] added form to edit user roles #4 (#18)

added voter to handle user profile access
using Security annotation to handle access authorization
---
 app/Resources/translations/messages.de.xliff  |   4 +
 app/Resources/views/user/profile.html.twig    |  44 +++--
 app/config/services.yml                       |  14 ++
 .../Controller/ProfileController.php          | 171 +++++++++---------
 src/AppBundle/Form/UserEditType.php           |  16 +-
 src/AppBundle/Form/UserRolesType.php          |  75 ++++++++
 src/AppBundle/Voter/AbstractVoter.php         |  52 ++++++
 src/AppBundle/Voter/UserVoter.php             | 112 ++++++++++++
 8 files changed, 384 insertions(+), 104 deletions(-)
 create mode 100644 src/AppBundle/Form/UserRolesType.php
 create mode 100644 src/AppBundle/Voter/AbstractVoter.php
 create mode 100644 src/AppBundle/Voter/UserVoter.php

diff --git a/app/Resources/translations/messages.de.xliff b/app/Resources/translations/messages.de.xliff
index d512eda9..b045dc55 100644
--- a/app/Resources/translations/messages.de.xliff
+++ b/app/Resources/translations/messages.de.xliff
@@ -354,6 +354,10 @@
         <source>profile.password</source>
         <target>Passwort</target>
       </trans-unit>
+      <trans-unit id="profile.roles">
+        <source>profile.roles</source>
+        <target>Rollen</target>
+      </trans-unit>
 
       <!--
         Admin: Timesheet
diff --git a/app/Resources/views/user/profile.html.twig b/app/Resources/views/user/profile.html.twig
index bc442c62..15a89c00 100644
--- a/app/Resources/views/user/profile.html.twig
+++ b/app/Resources/views/user/profile.html.twig
@@ -16,9 +16,15 @@
             <div class="nav-tabs-custom">
                 <ul class="nav nav-tabs">
                     <li {% if tab == "charts" %}class="active"{% endif %}><a href="#charts" data-toggle="tab" aria-expanded="true">{{ 'profile.tab_monthly'|trans }}</a></li>
+                    {% if form %}
                     <li {% if tab == "profile" %}class="active"{% endif %}><a href="#profile" data-toggle="tab" aria-expanded="false">{{ 'profile.settings'|trans }}</a></li>
-                    <li {% if tab == "password" %}class="active"{% endif %}><a href="#password" data-toggle="tab" aria-expanded="false">{{ 'profile.password'|trans }}</a></li>
-
+                    {% endif %}
+                    {% if form_password %}
+                        <li {% if tab == "password" %}class="active"{% endif %}><a href="#password" data-toggle="tab" aria-expanded="false">{{ 'profile.password'|trans }}</a></li>
+                    {% endif %}
+                    {% if form_roles %}
+                        <li {% if tab == "roles" %}class="active"{% endif %}><a href="#roles" data-toggle="tab" aria-expanded="false">{{ 'profile.roles'|trans }}</a></li>
+                    {% endif %}
                 </ul>
                 <div class="tab-content">
                     <div class="tab-pane {% if tab == "charts" %}active{% endif %}" id="charts">
@@ -88,18 +94,30 @@
                         {% endfor %}
 
                     </div>
-                    <div class="tab-pane {% if tab == "profile" %}active{% endif %}" id="profile">
-                        {{ form_start(form) }}
-                            {{ form_widget(form) }}
+                    {% if form %}
+                        <div class="tab-pane {% if tab == "profile" %}active{% endif %}" id="profile">
+                            {{ form_start(form) }}
+                                {{ form_widget(form) }}
+                                <input type="submit" value="{{ 'action.save'|trans }}" class="btn btn-primary" />
+                            {{ form_end(form) }}
+                        </div>
+                    {% endif %}
+                    {% if form_password %}
+                        <div class="tab-pane {% if tab == "password" %}active{% endif %}" id="password">
+                            {{ form_start(form_password) }}
+                            {{ form_widget(form_password) }}
                             <input type="submit" value="{{ 'action.save'|trans }}" class="btn btn-primary" />
-                        {{ form_end(form) }}
-                    </div>
-                    <div class="tab-pane {% if tab == "password" %}active{% endif %}" id="password">
-                        {{ form_start(form_password) }}
-                        {{ form_widget(form_password) }}
-                        <input type="submit" value="{{ 'action.save'|trans }}" class="btn btn-primary" />
-                        {{ form_end(form_password) }}
-                    </div>
+                            {{ form_end(form_password) }}
+                        </div>
+                    {% endif %}
+                    {% if form_roles %}
+                        <div class="tab-pane {% if tab == "roles" %}active{% endif %}" id="roles">
+                            {{ form_start(form_roles) }}
+                            {{ form_widget(form_roles) }}
+                            <input type="submit" value="{{ 'action.save'|trans }}" class="btn btn-primary" />
+                            {{ form_end(form_roles) }}
+                        </div>
+                    {% endif %}
                 </div>
             </div>
         </div>
diff --git a/app/config/services.yml b/app/config/services.yml
index 9367f7fc..f0ba9c2b 100644
--- a/app/config/services.yml
+++ b/app/config/services.yml
@@ -24,6 +24,20 @@ services:
         tags:
             - { name: kernel.event_listener, event: kernel.request, method: onKernelRequest }
 
+    # security voter to check user-profile access
+    app.voter.user:
+        class: AppBundle\Voter\UserVoter
+        arguments: ["@security.access.decision_manager"]
+        tags:
+            - { name: security.voter }
+
+    # form to edit user roles
+    app.admin.user_profile_roles:
+        class: AppBundle\Form\UserRolesType
+        arguments: ["%security.role_hierarchy.roles%"]
+        tags:
+            - { name: form.type }
+
     # event-listener to populate the navigation
     app.menu_builder:
         class: AppBundle\EventListener\MenuBuilder
diff --git a/src/AppBundle/Controller/ProfileController.php b/src/AppBundle/Controller/ProfileController.php
index 3c18dffc..d094f4a5 100644
--- a/src/AppBundle/Controller/ProfileController.php
+++ b/src/AppBundle/Controller/ProfileController.php
@@ -14,11 +14,9 @@ namespace AppBundle\Controller;
 use AppBundle\Entity\User;
 use AppBundle\Form\UserEditType;
 use AppBundle\Form\UserPasswordType;
-use AppBundle\Repository\UserRepository;
+use AppBundle\Form\UserRolesType;
 use Symfony\Component\Form\Form;
-use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 use TimesheetBundle\Entity\Timesheet;
-use Symfony\Bundle\FrameworkBundle\Controller\Controller;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\Method;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security;
@@ -38,67 +36,21 @@ class ProfileController extends AbstractController
     /**
      * @Route("/{username}", name="user_profile")
      * @Method("GET")
+     * @Security("is_granted('view', user)")
      */
-    public function indexAction($username)
+    public function indexAction(User $user)
     {
-        $user = $this->getUserByUsername($username);
-
         return $this->getProfileView($user);
     }
 
-    /**
-     * @param User $user
-     * @param Form|null $editForm
-     * @param Form|null $pwdForm
-     * @param string $tab
-     * @return \Symfony\Component\HttpFoundation\Response
-     * @throws \Doctrine\ORM\NonUniqueResultException
-     */
-    protected function getProfileView(User $user, Form $editForm = null, Form $pwdForm = null, $tab = 'charts')
-    {
-        /* @var $timesheetRepo TimesheetRepository */
-        $timesheetRepo = $this->getDoctrine()->getRepository(Timesheet::class);
-        $userStats = $timesheetRepo->getUserStatistics($user);
-        $monthlyStats = $timesheetRepo->getMonthlyStats($user);
-
-        $editForm = $editForm !== null ? $editForm : $this->createEditForm($user);
-        $pwdForm = $pwdForm !== null ? $pwdForm : $this->createPasswordForm($user);
-
-        return $this->render(
-            'user/profile.html.twig',
-            [
-                'tab' => $tab,
-                'user' => $user,
-                'stats' => $userStats,
-                'years' => $monthlyStats,
-                'form' => $editForm->createView(),
-                'form_password' => $pwdForm->createView(),
-            ]
-        );
-    }
-
-    protected function getRoles()
-    {
-        $roles = array();
-        foreach ($this->getParameter('security.role_hierarchy.roles') as $key => $value) {
-            $roles[] = $key;
-            foreach ($value as $value2) {
-                $roles[] = $value2;
-            }
-        }
-        $roles = array_unique($roles);
-        return $roles;
-    }
-
     /**
      * @Route("/{username}/edit", name="user_profile_edit")
      * @Method({"GET", "POST"})
+     * @Security("is_granted('edit', user)")
      */
-    public function editAction($username, Request $request)
+    public function editAction(User $user, Request $request)
     {
-        $user = $this->getUserByUsername($username);
         $editForm = $this->createEditForm($user);
-
         $editForm->handleRequest($request);
 
         if ($editForm->isSubmitted() && $editForm->isValid()) {
@@ -113,18 +65,17 @@ class ProfileController extends AbstractController
             );
         }
 
-        return $this->getProfileView($user, $editForm, null, 'profile');
+        return $this->getProfileView($user, $editForm, null, null, 'profile');
     }
 
     /**
      * @Route("/{username}/password", name="user_profile_password")
      * @Method({"GET", "POST"})
+     * @Security("is_granted('password', user)")
      */
-    public function passwordAction($username, Request $request)
+    public function passwordAction(User $user, Request $request)
     {
-        $user = $this->getUserByUsername($username);
         $pwdForm = $this->createPasswordForm($user);
-
         $pwdForm->handleRequest($request);
 
         if ($pwdForm->isSubmitted() && $pwdForm->isValid()) {
@@ -143,52 +94,88 @@ class ProfileController extends AbstractController
             );
         }
 
-        return $this->getProfileView($user, null, $pwdForm, 'password');
+        return $this->getProfileView($user, null, $pwdForm, null, 'password');
+    }
+
+    /**
+     * @Route("/{username}/roles", name="user_profile_roles")
+     * @Method({"GET", "POST"})
+     * @Security("is_granted('roles', user)")
+     */
+    public function rolesAction(User $user, Request $request)
+    {
+        $rolesForm = $this->createRolesForm($user);
+        $rolesForm->handleRequest($request);
+
+        if ($rolesForm->isSubmitted() && $rolesForm->isValid()) {
+            $entityManager = $this->getDoctrine()->getManager();
+            $entityManager->persist($user);
+            $entityManager->flush();
+
+            $this->flashSuccess('action.updated_successfully');
+
+            return $this->redirectToRoute(
+                'user_profile', ['username' => $user->getUsername()]
+            );
+        }
+
+        return $this->getProfileView($user, null, null, $rolesForm, 'roles');
     }
 
     /**
      * FIXME implement profile deletion
+     *
      * @Route("/{username}/delete", name="user_profile_delete")
      * @Method({"GET", "POST"})
+     * @Security("is_granted('delete', user)")
      */
-    public function deleteAction($username, Request $request)
+    public function deleteAction(User $user, Request $request)
     {
-        $user = $this->getUserByUsername($username);
         $deleteForm = $this->createDeleteForm($user);
 
         throw new \Exception('Delete not implemented yet');
     }
 
     /**
-     * @param $username
-     * @return User
-     * @throws NotFoundHttpException
+     * @param User $user
+     * @param Form|null $editForm
+     * @param Form|null $pwdForm
+     * @param Form|null $rolesForm
+     * @param string $tab
+     * @return \Symfony\Component\HttpFoundation\Response
+     * @throws \Doctrine\ORM\NonUniqueResultException
      */
-    protected function getUserByUsername($username)
+    protected function getProfileView(User $user, Form $editForm = null, Form $pwdForm = null, Form $rolesForm = null, $tab = 'charts')
     {
-        $user = $this->getUser();
+        /* @var $timesheetRepo TimesheetRepository */
+        $timesheetRepo = $this->getDoctrine()->getRepository(Timesheet::class);
+        $userStats = $timesheetRepo->getUserStatistics($user);
+        $monthlyStats = $timesheetRepo->getMonthlyStats($user);
 
-        // access to own profile always allowed
-        if (null === $username) {
-            $username = $user->getUsername();
+        $viewVars = [
+            'tab' => $tab,
+            'user' => $user,
+            'stats' => $userStats,
+            'years' => $monthlyStats,
+            'form' => null,
+            'form_password' => null,
+            'form_roles' => null,
+        ];
+
+        if ($this->isGranted('edit', $user)) {
+            $editForm = $editForm ?: $this->createEditForm($user);
+            $viewVars['form'] = $editForm->createView();
+        }
+        if ($this->isGranted('password', $user)) {
+            $pwdForm = $pwdForm ?: $this->createPasswordForm($user);
+            $viewVars['form_password'] = $pwdForm->createView();
+        }
+        if ($this->isGranted('roles', $user)) {
+            $rolesForm = $rolesForm ?: $this->createRolesForm($user);
+            $viewVars['form_roles'] = $rolesForm->createView();
         }
 
-        // only administrator can bypass that part if the requested user is not the current user
-        if ($username !== $user->getUsername()) {
-            $this->denyUnlessGranted('ROLE_ADMIN');
-        }
-
-        // if the user is not the current use, load the requested one
-        if ($username !== $user->getUsername()) {
-            /* @var $userRepo UserRepository */
-            $userRepo = $this->getDoctrine()->getRepository(User::class);
-            $user = $userRepo->findByUsername($username);
-            if (null === $user) {
-                throw new NotFoundHttpException('User "'.$username.'" does not exist');
-            }
-        }
-
-        return $user;
+        return $this->render('user/profile.html.twig', $viewVars);
     }
 
     /**
@@ -207,6 +194,22 @@ class ProfileController extends AbstractController
         );
     }
 
+    /**
+     * @param User $user
+     * @return \Symfony\Component\Form\FormInterface
+     */
+    private function createRolesForm(User $user)
+    {
+        return $this->createForm(
+            UserRolesType::class,
+            $user,
+            [
+                'action' => $this->generateUrl('user_profile_roles', ['username' => $user->getUsername()]),
+                'method' => 'POST',
+            ]
+        );
+    }
+
     /**
      * @param User $user
      * @return \Symfony\Component\Form\FormInterface
diff --git a/src/AppBundle/Form/UserEditType.php b/src/AppBundle/Form/UserEditType.php
index 51922b7d..f43a5592 100644
--- a/src/AppBundle/Form/UserEditType.php
+++ b/src/AppBundle/Form/UserEditType.php
@@ -15,11 +15,12 @@ use AppBundle\Entity\User;
 use AppBundle\Form\Type\LanguageType;
 use AppBundle\Form\Type\YesNoType;
 use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\Extension\Core\Type\TextType;
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 
 /**
- * Defines the form used to create and manipulate Users.
+ * Defines the form used to edit the profile of a User.
  *
  * @author Kevin Papst <kevin@kevinpapst.de>
  */
@@ -33,20 +34,22 @@ class UserEditType extends AbstractType
     {
         $builder
             // string - length 160
-            ->add('alias', null, [
+            ->add('alias', TextType::class, [
                 'label' => 'label.alias',
+                'required' => false,
             ])
             // string - length 50
-            ->add('title', null, [
-                //'attr' => ['autofocus' => true],
+            ->add('title', TextType::class, [
                 'label' => 'label.title',
+                'required' => false,
             ])
             // string - length 255
-            ->add('avatar', null, [
+            ->add('avatar', TextType::class, [
                 'label' => 'label.avatar',
+                'required' => false,
             ])
             // string - length 160
-            ->add('email', null, [
+            ->add('email', TextType::class, [
                 'label' => 'label.email',
             ])
             // string - length 5
@@ -57,7 +60,6 @@ class UserEditType extends AbstractType
             ->add('active', YesNoType::class, [
                 'label' => 'label.active',
             ])
-            // TODO roles - see ProfileController::getRoles()
         ;
     }
 
diff --git a/src/AppBundle/Form/UserRolesType.php b/src/AppBundle/Form/UserRolesType.php
new file mode 100644
index 00000000..ee076506
--- /dev/null
+++ b/src/AppBundle/Form/UserRolesType.php
@@ -0,0 +1,75 @@
+<?php
+
+/*
+ * This file is part of the Kimai package.
+ *
+ * (c) Kevin Papst <kevin@kevinpapst.de>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace AppBundle\Form;
+
+use AppBundle\Entity\User;
+use Symfony\Component\Form\AbstractType;
+use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
+use Symfony\Component\Form\FormBuilderInterface;
+use Symfony\Component\OptionsResolver\OptionsResolver;
+
+/**
+ * Defines the form used to set roles for a User.
+ *
+ * @author Kevin Papst <kevin@kevinpapst.de>
+ */
+class UserRolesType extends AbstractType
+{
+    /**
+     * @var string[]
+     */
+    protected $roles = [];
+
+    /**
+     * UserRolesType constructor.
+     * @param string[] $roles
+     */
+    public function __construct(array $roles)
+    {
+        $this->roles = $roles;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function buildForm(FormBuilderInterface $builder, array $options)
+    {
+        foreach ($this->roles as $key => $value) {
+            $roles[$key] = $key;
+            foreach ($value as $value2) {
+                $roles[$value2] = $value2;
+            }
+        }
+
+        $builder
+            // string[]
+            ->add('roles', ChoiceType::class, [
+                'label' => 'label.roles',
+                'multiple' => true,
+                'choices' => $roles,
+            ])
+        ;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function configureOptions(OptionsResolver $resolver)
+    {
+        $resolver->setDefaults([
+            'data_class' => User::class,
+            'csrf_protection' => true,
+            'csrf_field_name' => '_token',
+            'csrf_token_id' => 'edit_user_roles',
+        ]);
+    }
+}
diff --git a/src/AppBundle/Voter/AbstractVoter.php b/src/AppBundle/Voter/AbstractVoter.php
new file mode 100644
index 00000000..5e3e7a9f
--- /dev/null
+++ b/src/AppBundle/Voter/AbstractVoter.php
@@ -0,0 +1,52 @@
+<?php
+
+/*
+ * This file is part of the Kimai package.
+ *
+ * (c) Kevin Papst <kevin@kevinpapst.de>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace AppBundle\Voter;
+
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
+
+/**
+ * Abstract voter to help with checking user roles.
+ *
+ * @author Kevin Papst <kevin@kevinpapst.de>
+ */
+abstract class AbstractVoter extends Voter
+{
+    /**
+     * @var AccessDecisionManagerInterface
+     */
+    protected $decisionManager;
+
+    /**
+     * AbstractVoter constructor.
+     * @param AccessDecisionManagerInterface $decisionManager
+     */
+    public function __construct(AccessDecisionManagerInterface $decisionManager)
+    {
+        $this->decisionManager = $decisionManager;
+    }
+
+    /**
+     * @param string $role
+     * @param TokenInterface $token
+     * @return bool
+     */
+    public function hasRole($role, TokenInterface $token)
+    {
+        if ($this->decisionManager->decide($token, array($role))) {
+            return true;
+        }
+
+        return false;
+    }
+}
diff --git a/src/AppBundle/Voter/UserVoter.php b/src/AppBundle/Voter/UserVoter.php
new file mode 100644
index 00000000..562a3f48
--- /dev/null
+++ b/src/AppBundle/Voter/UserVoter.php
@@ -0,0 +1,112 @@
+<?php
+
+/*
+ * This file is part of the Kimai package.
+ *
+ * (c) Kevin Papst <kevin@kevinpapst.de>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace AppBundle\Voter;
+
+use AppBundle\Entity\User;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+
+/**
+ * A voter to check permissions on user profiles.
+ *
+ * @author Kevin Papst <kevin@kevinpapst.de>
+ */
+class UserVoter extends AbstractVoter
+{
+    const VIEW = 'view';
+    const EDIT = 'edit';
+    const DELETE = 'delete';
+    const PASSWORD = 'password';
+    const ROLES = 'roles';
+
+    /**
+     * @param string $attribute
+     * @param mixed $subject
+     * @return bool
+     */
+    protected function supports($attribute, $subject)
+    {
+        if (!in_array($attribute, array(self::VIEW, self::EDIT, self::ROLES, self::PASSWORD, self::DELETE))) {
+            return false;
+        }
+
+        if (!$subject instanceof User) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * @param string $attribute
+     * @param User $subject
+     * @param TokenInterface $token
+     * @return bool
+     */
+    protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
+    {
+        $user = $token->getUser();
+
+        if (!$user instanceof User) {
+            return false;
+        }
+
+        switch ($attribute) {
+            case self::VIEW:
+                return $this->canView($subject, $user, $token);
+            case self::EDIT:
+            case self::PASSWORD:
+                return $this->canEdit($subject, $user, $token);
+            case self::DELETE:
+            case self::ROLES:
+                return $this->canEditRoles($token);
+        }
+
+        return false;
+    }
+
+    /**
+     * @param User $profile
+     * @param User $user
+     * @return bool
+     */
+    private function canView(User $profile, User $user, TokenInterface $token)
+    {
+        if ($this->canEdit($profile, $user, $token)) {
+            return true;
+        }
+
+        return $profile->getId() == $user->getId();
+    }
+
+    /**
+     * @param User $profile
+     * @param User $user
+     * @return bool
+     */
+    private function canEdit(User $profile, User $user, TokenInterface $token)
+    {
+        if ($this->canEditRoles($token)) {
+            return true;
+        }
+
+        return $profile->getId() == $user->getId();
+    }
+
+    /**
+     * @param TokenInterface $token
+     * @return bool
+     */
+    private function canEditRoles(TokenInterface $token)
+    {
+        return $this->hasRole('ROLE_SUPER_ADMIN', $token);
+    }
+}