diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4cc1d7c17..8b8938125 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -10,11 +10,6 @@ parameters: count: 1 path: src/Controller/AnnotationController.php - - - message: "#^Call to an undefined method Wallabag\\\\Entity\\\\RuleInterface\\:\\:getConfig\\(\\)\\.$#" - count: 1 - path: src/Controller/ConfigController.php - - message: "#^Method FOS\\\\UserBundle\\\\Model\\\\UserManagerInterface\\:\\:updateUser\\(\\) invoked with 2 parameters, 1 required\\.$#" count: 6 diff --git a/src/Controller/ConfigController.php b/src/Controller/ConfigController.php index 71b095447..390e8cef3 100644 --- a/src/Controller/ConfigController.php +++ b/src/Controller/ConfigController.php @@ -10,6 +10,7 @@ use JMS\Serializer\SerializationContext; use JMS\Serializer\SerializerBuilder; use PragmaRX\Recovery\Recovery as BackupCodes; use Scheb\TwoFactorBundle\Security\TwoFactor\Provider\Google\GoogleAuthenticatorInterface; +use Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -22,7 +23,6 @@ use Symfony\Component\Validator\Constraints\Locale as LocaleConstraint; use Symfony\Component\Validator\Validator\ValidatorInterface; use Wallabag\Entity\Config as ConfigEntity; use Wallabag\Entity\IgnoreOriginUserRule; -use Wallabag\Entity\RuleInterface; use Wallabag\Entity\TaggingRule; use Wallabag\Event\ConfigUpdatedEvent; use Wallabag\Form\Type\ChangePasswordType; @@ -75,6 +75,7 @@ class ConfigController extends AbstractController /** * @Route("/config", name="config", methods={"GET", "POST"}) + * @IsGranted("EDIT_CONFIG") */ public function indexAction(Request $request, Config $craueConfig, TaggingRuleRepository $taggingRuleRepository, IgnoreOriginUserRuleRepository $ignoreOriginUserRuleRepository, UserRepository $userRepository) { @@ -265,6 +266,7 @@ class ConfigController extends AbstractController * Disable 2FA using email. * * @Route("/config/otp/email/disable", name="disable_otp_email", methods={"POST"}) + * @IsGranted("EDIT_CONFIG") */ public function disableOtpEmailAction(Request $request) { @@ -289,6 +291,7 @@ class ConfigController extends AbstractController * Enable 2FA using email. * * @Route("/config/otp/email", name="config_otp_email", methods={"POST"}) + * @IsGranted("EDIT_CONFIG") */ public function otpEmailAction(Request $request) { @@ -316,6 +319,7 @@ class ConfigController extends AbstractController * Disable 2FA using OTP app. * * @Route("/config/otp/app/disable", name="disable_otp_app", methods={"POST"}) + * @IsGranted("EDIT_CONFIG") */ public function disableOtpAppAction(Request $request) { @@ -342,6 +346,7 @@ class ConfigController extends AbstractController * Enable 2FA using OTP app, user will need to confirm the generated code from the app. * * @Route("/config/otp/app", name="config_otp_app", methods={"POST"}) + * @IsGranted("EDIT_CONFIG") */ public function otpAppAction(Request $request, GoogleAuthenticatorInterface $googleAuthenticator) { @@ -383,6 +388,7 @@ class ConfigController extends AbstractController * Cancelling 2FA using OTP app. * * @Route("/config/otp/app/cancel", name="config_otp_app_cancel") + * @IsGranted("EDIT_CONFIG") * * XXX: commented until we rewrite 2fa with a real two-steps activation */ @@ -401,6 +407,7 @@ class ConfigController extends AbstractController * Validate OTP code. * * @Route("/config/otp/app/check", name="config_otp_app_check", methods={"POST"}) + * @IsGranted("EDIT_CONFIG") */ public function otpAppCheckAction(Request $request, GoogleAuthenticatorInterface $googleAuthenticator) { @@ -437,6 +444,7 @@ class ConfigController extends AbstractController /** * @Route("/generate-token", name="generate_token", methods={"GET"}) + * @IsGranted("EDIT_CONFIG") * * @return RedirectResponse|JsonResponse */ @@ -462,6 +470,7 @@ class ConfigController extends AbstractController /** * @Route("/revoke-token", name="revoke_token", methods={"GET"}) + * @IsGranted("EDIT_CONFIG") * * @return RedirectResponse|JsonResponse */ @@ -488,15 +497,14 @@ class ConfigController extends AbstractController /** * Deletes a tagging rule and redirect to the config homepage. * - * @Route("/tagging-rule/delete/{id}", name="delete_tagging_rule", methods={"GET"}, requirements={"id" = "\d+"}) + * @Route("/tagging-rule/delete/{taggingRule}", name="delete_tagging_rule", methods={"GET"}, requirements={"taggingRule" = "\d+"}) + * @IsGranted("DELETE", subject="taggingRule") * * @return RedirectResponse */ - public function deleteTaggingRuleAction(TaggingRule $rule) + public function deleteTaggingRuleAction(TaggingRule $taggingRule) { - $this->validateRuleAction($rule); - - $this->entityManager->remove($rule); + $this->entityManager->remove($taggingRule); $this->entityManager->flush(); $this->addFlash( @@ -510,29 +518,27 @@ class ConfigController extends AbstractController /** * Edit a tagging rule. * - * @Route("/tagging-rule/edit/{id}", name="edit_tagging_rule", methods={"GET"}, requirements={"id" = "\d+"}) + * @Route("/tagging-rule/edit/{taggingRule}", name="edit_tagging_rule", methods={"GET"}, requirements={"taggingRule" = "\d+"}) + * @IsGranted("EDIT", subject="taggingRule") * * @return RedirectResponse */ - public function editTaggingRuleAction(TaggingRule $rule) + public function editTaggingRuleAction(TaggingRule $taggingRule) { - $this->validateRuleAction($rule); - - return $this->redirect($this->generateUrl('config') . '?tagging-rule=' . $rule->getId() . '#set5'); + return $this->redirect($this->generateUrl('config') . '?tagging-rule=' . $taggingRule->getId() . '#set5'); } /** * Deletes an ignore origin rule and redirect to the config homepage. * - * @Route("/ignore-origin-user-rule/delete/{id}", name="delete_ignore_origin_rule", methods={"GET"}, requirements={"id" = "\d+"}) + * @Route("/ignore-origin-user-rule/delete/{ignoreOriginUserRule}", name="delete_ignore_origin_rule", methods={"GET"}, requirements={"ignoreOriginUserRule" = "\d+"}) + * @IsGranted("DELETE", subject="ignoreOriginUserRule") * * @return RedirectResponse */ - public function deleteIgnoreOriginRuleAction(IgnoreOriginUserRule $rule) + public function deleteIgnoreOriginRuleAction(IgnoreOriginUserRule $ignoreOriginUserRule) { - $this->validateRuleAction($rule); - - $this->entityManager->remove($rule); + $this->entityManager->remove($ignoreOriginUserRule); $this->entityManager->flush(); $this->addFlash( @@ -546,21 +552,21 @@ class ConfigController extends AbstractController /** * Edit an ignore origin rule. * - * @Route("/ignore-origin-user-rule/edit/{id}", name="edit_ignore_origin_rule", methods={"GET"}, requirements={"id" = "\d+"}) + * @Route("/ignore-origin-user-rule/edit/{ignoreOriginUserRule}", name="edit_ignore_origin_rule", methods={"GET"}, requirements={"ignoreOriginUserRule" = "\d+"}) + * @IsGranted("EDIT", subject="ignoreOriginUserRule") * * @return RedirectResponse */ - public function editIgnoreOriginRuleAction(IgnoreOriginUserRule $rule) + public function editIgnoreOriginRuleAction(IgnoreOriginUserRule $ignoreOriginUserRule) { - $this->validateRuleAction($rule); - - return $this->redirect($this->generateUrl('config') . '?ignore-origin-user-rule=' . $rule->getId() . '#set6'); + return $this->redirect($this->generateUrl('config') . '?ignore-origin-user-rule=' . $ignoreOriginUserRule->getId() . '#set6'); } /** * Remove all annotations OR tags OR entries for the current user. * * @Route("/reset/{type}", name="config_reset", methods={"POST"}, requirements={"id" = "annotations|tags|entries|tagging_rules"}) + * @IsGranted("EDIT_CONFIG") * * @return RedirectResponse */ @@ -616,6 +622,7 @@ class ConfigController extends AbstractController * Delete account for current user. * * @Route("/account/delete", name="delete_account", methods={"POST"}) + * @IsGranted("EDIT_CONFIG") * * @throws AccessDeniedHttpException * @@ -648,6 +655,7 @@ class ConfigController extends AbstractController * Switch view mode for current user. * * @Route("/config/view-mode", name="switch_view_mode", methods={"GET"}) + * @IsGranted("EDIT_CONFIG") * * @return RedirectResponse */ @@ -670,6 +678,7 @@ class ConfigController extends AbstractController * @param string $language * * @Route("/locale/{language}", name="changeLocale", methods={"GET"}) + * @IsGranted("PUBLIC_ACCESS") * * @return RedirectResponse */ @@ -688,6 +697,7 @@ class ConfigController extends AbstractController * Export tagging rules for the logged in user. * * @Route("/tagging-rule/export", name="export_tagging_rule", methods={"GET"}) + * @IsGranted("EDIT_CONFIG") * * @return Response */ @@ -768,16 +778,6 @@ class ConfigController extends AbstractController $this->entityManager->flush(); } - /** - * Validate that a rule can be edited/deleted by the current user. - */ - private function validateRuleAction(RuleInterface $rule) - { - if ($this->getUser()->getId() !== $rule->getConfig()->getUser()->getId()) { - throw $this->createAccessDeniedException('You can not access this rule.'); - } - } - /** * Retrieve config for the current user. * If no config were found, create a new one. diff --git a/src/Security/Voter/IgnoreOriginUserRuleVoter.php b/src/Security/Voter/IgnoreOriginUserRuleVoter.php new file mode 100644 index 000000000..f7186f835 --- /dev/null +++ b/src/Security/Voter/IgnoreOriginUserRuleVoter.php @@ -0,0 +1,46 @@ +getUser(); + + if (!$user instanceof User) { + return false; + } + + switch ($attribute) { + case self::EDIT: + case self::DELETE: + return $subject->getConfig()->getUser() === $user; + } + + return false; + } +} diff --git a/src/Security/Voter/MainVoter.php b/src/Security/Voter/MainVoter.php index b8deccfe7..6d1f0afeb 100644 --- a/src/Security/Voter/MainVoter.php +++ b/src/Security/Voter/MainVoter.php @@ -15,6 +15,7 @@ class MainVoter extends Voter public const IMPORT_ENTRIES = 'IMPORT_ENTRIES'; public const LIST_SITE_CREDENTIALS = 'LIST_SITE_CREDENTIALS'; public const CREATE_SITE_CREDENTIALS = 'CREATE_SITE_CREDENTIALS'; + public const EDIT_CONFIG = 'EDIT_CONFIG'; private Security $security; @@ -29,7 +30,7 @@ class MainVoter extends Voter return false; } - if (!\in_array($attribute, [self::LIST_ENTRIES, self::CREATE_ENTRIES, self::EDIT_ENTRIES, self::EXPORT_ENTRIES, self::IMPORT_ENTRIES, self::LIST_SITE_CREDENTIALS, self::CREATE_SITE_CREDENTIALS], true)) { + if (!\in_array($attribute, [self::LIST_ENTRIES, self::CREATE_ENTRIES, self::EDIT_ENTRIES, self::EXPORT_ENTRIES, self::IMPORT_ENTRIES, self::LIST_SITE_CREDENTIALS, self::CREATE_SITE_CREDENTIALS, self::EDIT_CONFIG], true)) { return false; } @@ -46,6 +47,7 @@ class MainVoter extends Voter case self::IMPORT_ENTRIES: case self::LIST_SITE_CREDENTIALS: case self::CREATE_SITE_CREDENTIALS: + case self::EDIT_CONFIG: return $this->security->isGranted('ROLE_USER'); } diff --git a/src/Security/Voter/TaggingRuleVoter.php b/src/Security/Voter/TaggingRuleVoter.php new file mode 100644 index 000000000..01113957d --- /dev/null +++ b/src/Security/Voter/TaggingRuleVoter.php @@ -0,0 +1,46 @@ +getUser(); + + if (!$user instanceof User) { + return false; + } + + switch ($attribute) { + case self::EDIT: + case self::DELETE: + return $subject->getConfig()->getUser() === $user; + } + + return false; + } +} diff --git a/templates/Config/index.html.twig b/templates/Config/index.html.twig index 81700b21c..055906a52 100644 --- a/templates/Config/index.html.twig +++ b/templates/Config/index.html.twig @@ -379,10 +379,10 @@ {{ 'config.form_rules.then_tag_as_label'|trans }} « {{ tagging_rule.tags|join(', ') }} » - + mode_edit - + delete @@ -559,10 +559,10 @@
  • {{ 'config.form_rules.if_label'|trans }} « {{ ignore_origin_rule.rule }} » - + mode_edit - + delete
    • diff --git a/templates/Static/quickstart.html.twig b/templates/Static/quickstart.html.twig index 4291047a7..cc86d3829 100644 --- a/templates/Static/quickstart.html.twig +++ b/templates/Static/quickstart.html.twig @@ -12,21 +12,23 @@

    {{ 'quickstart.intro.title'|trans }}