Merge pull request #7376 from yguedidi/use-isgranted-in-sitecredentialcontroller

Use IsGranted in SiteCredentialController

src/Controller/SiteCredentialController.php

@@ -4,6 +4,7 @@ namespace Wallabag\Controller;
 
 use Craue\ConfigBundle\Util\Config;
 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;
@@ -41,6 +42,7 @@ class SiteCredentialController extends AbstractController
      * Lists all User entities.
      *
      * @Route("/", name="site_credentials_index", methods={"GET"})
+     * @IsGranted("LIST_SITE_CREDENTIALS")
      */
     public function indexAction(SiteCredentialRepository $repository)
     {
@@ -57,6 +59,7 @@ class SiteCredentialController extends AbstractController
      * Creates a new site credential entity.
      *
      * @Route("/new", name="site_credentials_new", methods={"GET", "POST"})
+     * @IsGranted("CREATE_SITE_CREDENTIALS")
      *
      * @return Response
      */
@@ -94,6 +97,7 @@ class SiteCredentialController extends AbstractController
      * Displays a form to edit an existing site credential entity.
      *
      * @Route("/{id}/edit", name="site_credentials_edit", methods={"GET", "POST"})
+     * @IsGranted("EDIT", subject="siteCredential")
      *
      * @return Response
      */
@@ -101,8 +105,6 @@ class SiteCredentialController extends AbstractController
     {
         $this->isSiteCredentialsEnabled();
 
-        $this->checkUserAction($siteCredential);
-
         $deleteForm = $this->createDeleteForm($siteCredential);
         $editForm = $this->createForm(SiteCredentialType::class, $siteCredential);
         $editForm->handleRequest($request);
@@ -133,6 +135,7 @@ class SiteCredentialController extends AbstractController
      * Deletes a site credential entity.
      *
      * @Route("/{id}", name="site_credentials_delete", methods={"DELETE"})
+     * @IsGranted("DELETE", subject="siteCredential")
      *
      * @return RedirectResponse
      */
@@ -140,8 +143,6 @@ class SiteCredentialController extends AbstractController
     {
         $this->isSiteCredentialsEnabled();
 
-        $this->checkUserAction($siteCredential);
-
         $form = $this->createDeleteForm($siteCredential);
         $form->handleRequest($request);
 
@@ -183,16 +184,4 @@ class SiteCredentialController extends AbstractController
             ->getForm()
         ;
     }
-
-    /**
-     * Check if the logged user can manage the given site credential.
-     *
-     * @param SiteCredential $siteCredential The site credential entity
-     */
-    private function checkUserAction(SiteCredential $siteCredential)
-    {
-        if (null === $this->getUser() || $this->getUser()->getId() !== $siteCredential->getUser()->getId()) {
-            throw $this->createAccessDeniedException('You can not access this site credential.');
-        }
-    }
 }

src/Security/Voter/MainVoter.php

@@ -11,6 +11,8 @@ class MainVoter extends Voter
     public const LIST_ENTRIES = 'LIST_ENTRIES';
     public const CREATE_ENTRIES = 'CREATE_ENTRIES';
     public const EDIT_ENTRIES = 'EDIT_ENTRIES';
+    public const LIST_SITE_CREDENTIALS = 'LIST_SITE_CREDENTIALS';
+    public const CREATE_SITE_CREDENTIALS = 'CREATE_SITE_CREDENTIALS';
 
     private Security $security;
 
@@ -25,7 +27,7 @@ class MainVoter extends Voter
             return false;
         }
 
-        if (!\in_array($attribute, [self::LIST_ENTRIES, self::CREATE_ENTRIES, self::EDIT_ENTRIES], true)) {
+        if (!\in_array($attribute, [self::LIST_ENTRIES, self::CREATE_ENTRIES, self::EDIT_ENTRIES, self::LIST_SITE_CREDENTIALS, self::CREATE_SITE_CREDENTIALS], true)) {
             return false;
         }
 
@@ -38,6 +40,8 @@ class MainVoter extends Voter
             case self::LIST_ENTRIES:
             case self::CREATE_ENTRIES:
             case self::EDIT_ENTRIES:
+            case self::LIST_SITE_CREDENTIALS:
+            case self::CREATE_SITE_CREDENTIALS:
                 return $this->security->isGranted('ROLE_USER');
         }
 

src/Security/Voter/SiteCredentialVoter.php

@@ -0,0 +1,46 @@
+<?php
+
+namespace Wallabag\Security\Voter;
+
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
+use Wallabag\Entity\SiteCredential;
+use Wallabag\Entity\User;
+
+class SiteCredentialVoter extends Voter
+{
+    public const EDIT = 'EDIT';
+    public const DELETE = 'DELETE';
+
+    protected function supports(string $attribute, $subject): bool
+    {
+        if (!$subject instanceof SiteCredential) {
+            return false;
+        }
+
+        if (!\in_array($attribute, [self::EDIT, self::DELETE], true)) {
+            return false;
+        }
+
+        return true;
+    }
+
+    protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token): bool
+    {
+        \assert($subject instanceof SiteCredential);
+
+        $user = $token->getUser();
+
+        if (!$user instanceof User) {
+            return false;
+        }
+
+        switch ($attribute) {
+            case self::EDIT:
+            case self::DELETE:
+                return $user === $subject->getUser();
+        }
+
+        return false;
+    }
+}

templates/SiteCredential/edit.html.twig

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

templates/SiteCredential/index.html.twig

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

templates/layout.html.twig

@@ -126,7 +126,7 @@
                     <li><a href="{{ path('config') }}"><i class="material-icons">settings</i> {{ 'menu.left.config'|trans }}</a></li>
                     <li><a href="{{ path('developer') }}"><i class="material-icons">smartphone</i> {{ 'menu.left.developer'|trans }}</a></li>
                     <li><a href="{{ path('import') }}"><i class="material-icons">import_export</i> {{ 'menu.left.import'|trans }}</a></li>
-                    {% if craue_setting('restricted_access') %}
+                    {% if craue_setting('restricted_access') and is_granted('LIST_SITE_CREDENTIALS') %}
                         <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>

tests/Security/Voter/MainVoterTest.php

@@ -83,4 +83,32 @@ class MainVoterTest extends TestCase
 
         $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->mainVoter->vote($this->token, null, [MainVoter::EDIT_ENTRIES]));
     }
+
+    public function testVoteReturnsDeniedForNonUserListSiteCredentials(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_USER')->willReturn(false);
+
+        $this->assertSame(VoterInterface::ACCESS_DENIED, $this->mainVoter->vote($this->token, null, [MainVoter::LIST_SITE_CREDENTIALS]));
+    }
+
+    public function testVoteReturnsGrantedForUserListSiteCredentials(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_USER')->willReturn(true);
+
+        $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->mainVoter->vote($this->token, null, [MainVoter::LIST_SITE_CREDENTIALS]));
+    }
+
+    public function testVoteReturnsDeniedForNonUserCreateSiteCredentials(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_USER')->willReturn(false);
+
+        $this->assertSame(VoterInterface::ACCESS_DENIED, $this->mainVoter->vote($this->token, null, [MainVoter::CREATE_SITE_CREDENTIALS]));
+    }
+
+    public function testVoteReturnsGrantedForUserCreateSiteCredentials(): void
+    {
+        $this->security->method('isGranted')->with('ROLE_USER')->willReturn(true);
+
+        $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->mainVoter->vote($this->token, null, [MainVoter::CREATE_SITE_CREDENTIALS]));
+    }
 }

tests/Security/Voter/SiteCredentialVoterTest.php

@@ -0,0 +1,57 @@
+<?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 Wallabag\Entity\SiteCredential;
+use Wallabag\Entity\User;
+use Wallabag\Security\Voter\SiteCredentialVoter;
+
+class SiteCredentialVoterTest extends TestCase
+{
+    private $user;
+    private $token;
+    private $siteCredentialVoter;
+
+    protected function setUp(): void
+    {
+        $this->user = new User();
+
+        $this->token = $this->createMock(TokenInterface::class);
+        $this->token->method('getUser')->willReturn($this->user);
+
+        $this->siteCredentialVoter = new SiteCredentialVoter();
+    }
+
+    public function testVoteReturnsAbstainForInvalidSubject(): void
+    {
+        $this->assertSame(VoterInterface::ACCESS_ABSTAIN, $this->siteCredentialVoter->vote($this->token, new \stdClass(), [SiteCredentialVoter::EDIT]));
+    }
+
+    public function testVoteReturnsAbstainForInvalidAttribute(): void
+    {
+        $this->assertSame(VoterInterface::ACCESS_ABSTAIN, $this->siteCredentialVoter->vote($this->token, new SiteCredential(new User()), ['INVALID']));
+    }
+
+    public function testVoteReturnsDeniedForNonSiteCredentialUserEdit(): void
+    {
+        $this->assertSame(VoterInterface::ACCESS_DENIED, $this->siteCredentialVoter->vote($this->token, new SiteCredential(new User()), [SiteCredentialVoter::EDIT]));
+    }
+
+    public function testVoteReturnsGrantedForSiteCredentialUserEdit(): void
+    {
+        $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->siteCredentialVoter->vote($this->token, new SiteCredential($this->user), [SiteCredentialVoter::EDIT]));
+    }
+
+    public function testVoteReturnsDeniedForNonSiteCredentialUserDelete(): void
+    {
+        $this->assertSame(VoterInterface::ACCESS_DENIED, $this->siteCredentialVoter->vote($this->token, new SiteCredential(new User()), [SiteCredentialVoter::DELETE]));
+    }
+
+    public function testVoteReturnsGrantedForSiteCredentialUserDelete(): void
+    {
+        $this->assertSame(VoterInterface::ACCESS_GRANTED, $this->siteCredentialVoter->vote($this->token, new SiteCredential($this->user), [SiteCredentialVoter::DELETE]));
+    }
+}