Merge pull request #7365 from yguedidi/use-isgranted-in-ignoreorigininstancerulecontroller

Use IsGranted in IgnoreOriginInstanceRuleController

app/config/security.yml

@@ -76,5 +76,4 @@ security:
         - { path: ^/settings, roles: ROLE_SUPER_ADMIN }
         - { path: ^/annotations, roles: ROLE_USER }
         - { path: ^/2fa, role: IS_AUTHENTICATED_2FA_IN_PROGRESS }
-        - { path: ^/ignore-origin-instance-rules, roles: ROLE_SUPER_ADMIN }
         - { path: ^/, roles: ROLE_USER }

src/Controller/IgnoreOriginInstanceRuleController.php

@@ -3,6 +3,7 @@
 namespace Wallabag\Controller;
 
 use Doctrine\ORM\EntityManagerInterface;
+use Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted;
 use Symfony\Component\Form\Form;
 use Symfony\Component\Form\FormInterface;
 use Symfony\Component\HttpFoundation\RedirectResponse;
@@ -34,6 +35,7 @@ class IgnoreOriginInstanceRuleController extends AbstractController
      * Lists all IgnoreOriginInstanceRule entities.
      *
      * @Route("/", name="ignore_origin_instance_rules_index", methods={"GET"})
+     * @IsGranted("LIST_IGNORE_ORIGIN_INSTANCE_RULES")
      */
     public function indexAction(IgnoreOriginInstanceRuleRepository $repository)
     {
@@ -48,6 +50,7 @@ class IgnoreOriginInstanceRuleController extends AbstractController
      * Creates a new ignore origin instance rule entity.
      *
      * @Route("/new", name="ignore_origin_instance_rules_new", methods={"GET", "POST"})
+     * @IsGranted("CREATE_IGNORE_ORIGIN_INSTANCE_RULES")
      *
      * @return Response
      */
@@ -80,6 +83,7 @@ class IgnoreOriginInstanceRuleController extends AbstractController
      * Displays a form to edit an existing ignore origin instance rule entity.
      *
      * @Route("/{id}/edit", name="ignore_origin_instance_rules_edit", methods={"GET", "POST"})
+     * @IsGranted("EDIT", subject="ignoreOriginInstanceRule")
      *
      * @return Response
      */
@@ -112,6 +116,7 @@ class IgnoreOriginInstanceRuleController extends AbstractController
      * Deletes a site credential entity.
      *
      * @Route("/{id}", name="ignore_origin_instance_rules_delete", methods={"DELETE"})
+     * @IsGranted("DELETE", subject="ignoreOriginInstanceRule")
      *
      * @return RedirectResponse
      */

src/Security/Voter/AdminVoter.php

@@ -11,6 +11,8 @@ class AdminVoter extends Voter
 {
     public const LIST_USERS = 'LIST_USERS';
     public const CREATE_USERS = 'CREATE_USERS';
+    public const LIST_IGNORE_ORIGIN_INSTANCE_RULES = 'LIST_IGNORE_ORIGIN_INSTANCE_RULES';
+    public const CREATE_IGNORE_ORIGIN_INSTANCE_RULES = 'CREATE_IGNORE_ORIGIN_INSTANCE_RULES';
 
     private Security $security;
 
@@ -21,7 +23,7 @@ class AdminVoter extends Voter
 
     protected function supports(string $attribute, $subject): bool
     {
-        if (!\in_array($attribute, [self::LIST_USERS, self::CREATE_USERS], true)) {
+        if (!\in_array($attribute, [self::LIST_USERS, self::CREATE_USERS, self::LIST_IGNORE_ORIGIN_INSTANCE_RULES, self::CREATE_IGNORE_ORIGIN_INSTANCE_RULES], true)) {
             return false;
         }
 
@@ -39,6 +41,8 @@ class AdminVoter extends Voter
         switch ($attribute) {
             case self::LIST_USERS:
             case self::CREATE_USERS:
+            case self::LIST_IGNORE_ORIGIN_INSTANCE_RULES:
+            case self::CREATE_IGNORE_ORIGIN_INSTANCE_RULES:
                 return $this->security->isGranted('ROLE_SUPER_ADMIN');
         }
 

src/Security/Voter/IgnoreOriginInstanceRuleVoter.php

@@ -0,0 +1,45 @@
+<?php
+
+namespace Wallabag\Security\Voter;
+
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
+use Symfony\Component\Security\Core\Security;
+use Wallabag\Entity\IgnoreOriginInstanceRule;
+
+class IgnoreOriginInstanceRuleVoter extends Voter
+{
+    public const EDIT = 'EDIT';
+    public const DELETE = 'DELETE';
+
+    private Security $security;
+
+    public function __construct(Security $security)
+    {
+        $this->security = $security;
+    }
+
+    protected function supports(string $attribute, $subject): bool
+    {
+        if (!$subject instanceof IgnoreOriginInstanceRule) {
+            return false;
+        }
+
+        if (!\in_array($attribute, [self::EDIT, self::DELETE], true)) {
+            return false;
+        }
+
+        return true;
+    }
+
+    protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token): bool
+    {
+        switch ($attribute) {
+            case self::EDIT:
+            case self::DELETE:
+                return $this->security->isGranted('ROLE_SUPER_ADMIN');
+        }
+
+        return false;
+    }
+}

templates/IgnoreOriginInstanceRule/edit.html.twig

@@ -28,11 +28,13 @@
                                 {{ form_widget(edit_form.save, {'attr': {'class': 'btn waves-effect waves-light'}}) }}
                                 {{ form_widget(edit_form._token) }}
                             </form>
-                            <p>
+                            {% if is_granted('DELETE', rule) %}
-                                {{ form_start(delete_form) }}
+                                <p>
-                                    <button onclick="return confirm('{{ 'ignore_origin_instance_rule.form.delete_confirm'|trans|escape('js') }}')" type="submit" class="btn waves-effect waves-light red">{{ 'ignore_origin_instance_rule.form.delete'|trans }}</button>
+                                    {{ form_start(delete_form) }}
-                                {{ form_end(delete_form) }}
+                                        <button onclick="return confirm('{{ 'ignore_origin_instance_rule.form.delete_confirm'|trans|escape('js') }}')" type="submit" class="btn waves-effect waves-light red">{{ 'ignore_origin_instance_rule.form.delete'|trans }}</button>
-                            </p>
+                                    {{ form_end(delete_form) }}
+                                </p>
+                            {% endif %}
                             <p><a class="waves-effect waves-light btn blue-grey" href="{{ path('ignore_origin_instance_rules_index') }}">{{ 'ignore_origin_instance_rule.form.back_to_list'|trans }}</a></p>
                         </div>
                     </div>

templates/IgnoreOriginInstanceRule/index.html.twig

@@ -23,16 +23,20 @@
                                 <tr>
                                     <td>{{ rule.rule }}</td>
                                     <td>
-                                        <a href="{{ path('ignore_origin_instance_rules_edit', {'id': rule.id}) }}">{{ 'ignore_origin_instance_rule.list.edit_action'|trans }}</a>
+                                        {% if is_granted('EDIT', rule) %}
+                                            <a href="{{ path('ignore_origin_instance_rules_edit', {'id': rule.id}) }}">{{ 'ignore_origin_instance_rule.list.edit_action'|trans }}</a>
+                                        {% endif %}
                                     </td>
                                 </tr>
                             {% endfor %}
                             </tbody>
                         </table>
-                        <br />
+                        {% if is_granted('CREATE_IGNORE_ORIGIN_INSTANCE_RULES') %}
-                        <p>
+                            <br />
-                            <a href="{{ path('ignore_origin_instance_rules_new') }}" class="waves-effect waves-light btn">{{ 'ignore_origin_instance_rule.list.create_new_one'|trans }}</a>
+                            <p>
-                        </p>
+                                <a href="{{ path('ignore_origin_instance_rules_new') }}" class="waves-effect waves-light btn">{{ 'ignore_origin_instance_rule.list.create_new_one'|trans }}</a>
+                            </p>
+                        {% endif %}
                     </div>
                 </div>
             </div>

templates/layout.html.twig

@@ -124,12 +124,19 @@
                         <li><a href="{{ path('site_credentials_index') }}"><i class="material-icons">vpn_key</i> {{ 'menu.left.site_credentials'|trans }}</a></li>
                     {% endif %}
                     <li class="divider"></li>
-                    {% if is_granted('LIST_USERS') %}
+                    {% set can_list_user = is_granted('LIST_USERS') %}
+                    {% set is_super_admin = is_granted('ROLE_SUPER_ADMIN') %}
+                    {% set can_list_ignore_origin_instance_rules = is_granted('LIST_IGNORE_ORIGIN_INSTANCE_RULES') %}
+                    {% if can_list_user %}
                         <li><a href="{{ path('user_index') }}"><i class="material-icons">people</i>{{ 'menu.left.users_management'|trans }}</a></li>
                     {% endif %}
-                    {% if is_granted('ROLE_SUPER_ADMIN') %}
+                    {% if is_super_admin %}
                         <li><a href="{{ path('craue_config_settings_modify') }}"><i class="material-icons">settings</i> {{ 'menu.left.internal_settings'|trans }}</a></li>
+                    {% endif %}
+                    {% if can_list_ignore_origin_instance_rules %}
                         <li><a href="{{ path('ignore_origin_instance_rules_index') }}"><i class="material-icons">build</i> {{ 'menu.left.ignore_origin_instance_rules'|trans }}</a></li>
+                    {% endif %}
+                    {% if can_list_user or is_super_admin or can_list_ignore_origin_instance_rules %}
                         <li class="divider"></li>
                     {% endif %}
                     <li>

tests/Security/Voter/AdminVoterTest.php

@@ -64,4 +64,32 @@ class AdminVoterTest extends TestCase
 
         $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->adminVoter->vote($this->token, null, [AdminVoter::CREATE_USERS]));
     }
+
+    public function testVoteReturnsDeniedForNonSuperAdminListIgnoreOriginInstanceRules(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_SUPER_ADMIN')->willReturn(false);
+
+        $this->assertSame(VoterInterface::ACCESS_DENIED, $this->adminVoter->vote($this->token, null, [AdminVoter::LIST_IGNORE_ORIGIN_INSTANCE_RULES]));
+    }
+
+    public function testVoteReturnsGrantedForSuperAdminListIgnoreOriginInstanceRules(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_SUPER_ADMIN')->willReturn(true);
+
+        $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->adminVoter->vote($this->token, null, [AdminVoter::LIST_IGNORE_ORIGIN_INSTANCE_RULES]));
+    }
+
+    public function testVoteReturnsDeniedForNonSuperAdminCreateIgnoreOriginInstanceRules(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_SUPER_ADMIN')->willReturn(false);
+
+        $this->assertSame(VoterInterface::ACCESS_DENIED, $this->adminVoter->vote($this->token, null, [AdminVoter::CREATE_IGNORE_ORIGIN_INSTANCE_RULES]));
+    }
+
+    public function testVoteReturnsGrantedForSuperAdminCreateIgnoreOriginInstanceRules(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_SUPER_ADMIN')->willReturn(true);
+
+        $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->adminVoter->vote($this->token, null, [AdminVoter::CREATE_IGNORE_ORIGIN_INSTANCE_RULES]));
+    }
 }

tests/Security/Voter/IgnoreOriginInstanceRuleVoterTest.php

@@ -0,0 +1,66 @@
+<?php
+
+namespace Security\Voter;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
+use Symfony\Component\Security\Core\Security;
+use Wallabag\Entity\IgnoreOriginInstanceRule;
+use Wallabag\Entity\User;
+use Wallabag\Security\Voter\IgnoreOriginInstanceRuleVoter;
+
+class IgnoreOriginInstanceRuleVoterTest extends TestCase
+{
+    private $security;
+    private $token;
+    private $ignoreOriginInstanceRuleVoter;
+
+    protected function setUp(): void
+    {
+        $this->security = $this->createMock(Security::class);
+
+        $this->token = $this->createMock(TokenInterface::class);
+        $this->token->method('getUser')->willReturn(new User());
+
+        $this->ignoreOriginInstanceRuleVoter = new IgnoreOriginInstanceRuleVoter($this->security);
+    }
+
+    public function testVoteReturnsAbstainForInvalidSubject(): void
+    {
+        $this->assertSame(VoterInterface::ACCESS_ABSTAIN, $this->ignoreOriginInstanceRuleVoter->vote($this->token, new \stdClass(), [IgnoreOriginInstanceRuleVoter::EDIT]));
+    }
+
+    public function testVoteReturnsAbstainForInvalidAttribute(): void
+    {
+        $this->assertSame(VoterInterface::ACCESS_ABSTAIN, $this->ignoreOriginInstanceRuleVoter->vote($this->token, new IgnoreOriginInstanceRule(), ['INVALID']));
+    }
+
+    public function testVoteReturnsDeniedForNonSuperAdminEdit(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_SUPER_ADMIN')->willReturn(false);
+
+        $this->assertSame(VoterInterface::ACCESS_DENIED, $this->ignoreOriginInstanceRuleVoter->vote($this->token, new IgnoreOriginInstanceRule(), [IgnoreOriginInstanceRuleVoter::EDIT]));
+    }
+
+    public function testVoteReturnsGrantedForSuperAdminEdit(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_SUPER_ADMIN')->willReturn(true);
+
+        $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->ignoreOriginInstanceRuleVoter->vote($this->token, new IgnoreOriginInstanceRule(), [IgnoreOriginInstanceRuleVoter::EDIT]));
+    }
+
+    public function testVoteReturnsDeniedForNonSuperAdminDelete(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_SUPER_ADMIN')->willReturn(false);
+
+        $this->assertSame(VoterInterface::ACCESS_DENIED, $this->ignoreOriginInstanceRuleVoter->vote($this->token, new IgnoreOriginInstanceRule(), [IgnoreOriginInstanceRuleVoter::DELETE]));
+    }
+
+    public function testVoteReturnsGrantedForSuperAdminDelete(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_SUPER_ADMIN')->willReturn(true);
+
+        $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->ignoreOriginInstanceRuleVoter->vote($this->token, new IgnoreOriginInstanceRule(), [IgnoreOriginInstanceRuleVoter::DELETE]));
+    }
+}