From 6ed2eeabae21cb020ac6c4d05bcbbca3ba5cc1aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20B=C3=B6hmer?= Date: Tue, 19 Mar 2019 18:36:05 +0100 Subject: [PATCH] Check for permissions before showing user infos or allow the user to change its own infos. --- config/permissions.yaml | 4 +- src/Controller/UserController.php | 4 +- src/Form/UserSettingsType.php | 24 ++++++-- src/Security/Voter/UserVoter.php | 91 +++++++++++++++++++++++++++++ src/Services/PermissionResolver.php | 17 +++++- 5 files changed, 130 insertions(+), 10 deletions(-) create mode 100644 src/Security/Voter/UserVoter.php diff --git a/config/permissions.yaml b/config/permissions.yaml index 03be0081..5131126b 100644 --- a/config/permissions.yaml +++ b/config/permissions.yaml @@ -153,9 +153,9 @@ perms: # Here comes a list with all Permission names (they have a perm_[name] co bit: 4 delete: bit: 8 - editUsername: + edit_username: bit: 2 - changeGroup: + change_group: bit: 6 edit_infos: bit: 10 diff --git a/src/Controller/UserController.php b/src/Controller/UserController.php index 35146885..7bacc760 100644 --- a/src/Controller/UserController.php +++ b/src/Controller/UserController.php @@ -55,10 +55,12 @@ class UserController extends AbstractController */ public function userInfo(?User $user, Packages $packages) { - //If no user id was passed, then we show info about the current user if($user == null) { $user = $this->getUser(); + } else { + //Else we must check, if the current user is allowed to access $user + $this->denyAccessUnlessGranted('read', $user); } if($this->getParameter("use_gravatar")) { diff --git a/src/Form/UserSettingsType.php b/src/Form/UserSettingsType.php index 9e16d3fb..aabcc344 100644 --- a/src/Form/UserSettingsType.php +++ b/src/Form/UserSettingsType.php @@ -14,21 +14,35 @@ use Symfony\Component\Form\Extension\Core\Type\TextType; use Symfony\Component\Form\Extension\Core\Type\TimezoneType; use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\OptionsResolver\OptionsResolver; +use Symfony\Component\Security\Core\Security; class UserSettingsType extends AbstractType { + protected $security; + + public function __construct(Security $security) + { + $this->security = $security; + } + + public function buildForm(FormBuilderInterface $builder, array $options) { $builder - ->add('name', TextType::class, ['label'=>'user.username.label']) + ->add('name', TextType::class, ['label'=>'user.username.label', + 'disabled' => !$this->security->isGranted('edit_username', $options['data'])]) ->add('first_name', TextType::class, ['required' => false, - 'label'=>'user.firstName.label']) + 'label'=>'user.firstName.label', + 'disabled' => !$this->security->isGranted('edit_infos', $options['data'])]) ->add('last_name', TextType::class, ['required' => false, - 'label'=>'user.lastName.label']) + 'label'=>'user.lastName.label', + 'disabled' => !$this->security->isGranted('edit_infos', $options['data'])]) ->add('department', TextType::class, ['required' => false, - 'label'=>'user.department.label']) + 'label'=>'user.department.label', + 'disabled' => !$this->security->isGranted('edit_infos', $options['data'])]) ->add('email', EmailType::class, ['required' => false, - 'label'=>'user.email.label']) + 'label'=>'user.email.label', + 'disabled' => !$this->security->isGranted('edit_infos', $options['data'])]) ->add('language', LocaleType::class, ['required' => false, 'attr'=>['class'=> 'selectpicker', 'data-live-search' => true] , 'placeholder' => 'user_settings.language.placeholder', 'label'=>'user.language_select']) diff --git a/src/Security/Voter/UserVoter.php b/src/Security/Voter/UserVoter.php new file mode 100644 index 00000000..d324c271 --- /dev/null +++ b/src/Security/Voter/UserVoter.php @@ -0,0 +1,91 @@ +resolver->listOperationsForPermission('users'), + $this->resolver->listOperationsForPermission('self')), + false + ); + } + + return false; + } + + /** + * Similar to voteOnAttribute, but checking for the anonymous user is already done. + * The current user (or the anonymous user) is passed by $user. + * @param $attribute + * @param $subject + * @param User $user + * @return bool + */ + protected function voteOnUser($attribute, $subject, User $user): bool + { + if($subject instanceof User) + { + //Check if the checked user is the user itself + if($subject->getID() === $user->getID() && + $this->resolver->isValidOperation('self', $attribute)) { + //Then we also need to check the self permission + $tmp = $this->resolver->inherit($user, 'self', $attribute) ?? false; + //But if the self value is not allowed then use just the user value: + if($tmp) + return $tmp; + } + //Else just check users permission: + return $this->resolver->inherit($user, 'users', $attribute) ?? false; + } + + return false; + } + + +} \ No newline at end of file diff --git a/src/Services/PermissionResolver.php b/src/Services/PermissionResolver.php index d38879f1..62292ea0 100644 --- a/src/Services/PermissionResolver.php +++ b/src/Services/PermissionResolver.php @@ -35,8 +35,6 @@ namespace App\Services; use App\Configuration\PermissionsConfiguration; use App\Entity\User; use App\Security\Interfaces\HasPermissionsInterface; -use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; -use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\Processor; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; use Symfony\Component\Yaml\Yaml; @@ -70,6 +68,8 @@ class PermissionResolver ); $this->permission_structure = $processedConfiguration; + + //dump($this->permission_structure); } @@ -163,5 +163,18 @@ class PermissionResolver return isset($this->permission_structure['perms'][$permission]); } + /** + * Checks if the permission operation combination with the given names is existing. + * + * @param string $permission The name of the permission which should be checked. + * @param string $operation The name of the operation which should be checked. + * @return bool True if the given permission operation combination is existing. + */ + public function isValidOperation(string $permission, string $operation) : bool + { + return $this->isValidPermission($permission) && + isset($this->permission_structure['perms'][$permission]['operations'][$operation]); + } + } \ No newline at end of file