How to call safely a delete action in a Controller?

Question

I created a blog with a comment system, and I would like the author or administrator to delete his comment.

So I searched the internet, but I found only posts in reference to Symfony 2/3 and I had a hard time understanding.

So I created my own function

/**
 * @Route("/blog/commentDelete/{id}-{articleId}-{articleSlug}", name="comment_delete")
 */
public function commentDelete($id, $articleId, $articleSlug, CommentRepository $commentRepository, AuthorizationCheckerInterface $authChecker){

   $em = $this->getDoctrine()->getManager();
   $comment = $commentRepository->find($id);

    $user = $this->getUser();
    if ($user->getId() != $comment->getAuthor()->getId() && $authChecker->isGranted('ROLE_MODERATOR') == false ){
        throw exception_for("Cette page n'existe pas");
    }

   $em->remove($comment);
   $em->flush();
   $this->addFlash('comment_success', 'Commentaire supprimé avec succès');
   return $this->redirectToRoute('blog_show', array('id' => $articleId, 'slug' => $articleSlug));
}

On twig, I've this link:

<a href="{{ path('comment_delete', {'id': comment.id, 'articleId': article.id, 'articleSlug': article.slug}) }}">Supprimer</a>

I need the comment id for the action, and article id et article slug to redirect the user once the comment has been deleted.

I check that the person who delete the comment is the author or a moderator.

However, I heard that is absolutely not secure because I have to use a form, but I really don't know how to use a form in this case... Or maybe with JS to hide the link to the final user?

So I would like to know if my function is secure enough or if exists a better solution and how to implement it?

Answer 1

A way to protect your delete action, it is to do something like :

• you create a voter according to this documentation How to Use Voters to Check User Permissions as Victor Kochkarev said.

    <?php

    namespace App\Security\Voter;

    use App\Entity\User;
    use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
    use Symfony\Component\Security\Core\Authorization\Voter\Voter;
    use App\Entity\Comment;

    class CommentVoter extends Voter
    {
        const CAN_DELETE = 'CAN_DELETE';

        protected function supports($attribute, $subject)
        {

            return in_array($attribute, [self::CAN_DELETE]) && $subject instanceof Comment;
        }

        protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
        {
            $user = $token->getUser();
            // if the user is anonymous, do not grant access
            if (!$user instanceof User) {
                return false;
            }

            /** @var Comment $comment */
            $comment = $subject;

            switch ($attribute) {
                case self::CAN_DELETE:
                    return $this->canDelete($comment, $user);
            }

            throw new \LogicException('This code should not be reached!');
        }

        private function canDelete(Comment $comment, User $user)
        {
            if($user->getId() !== $comment->getAuthor()->getId() && $user->hasRole('ROLE_MODERATOR') === false) {
                return false;  
            }

            return true;
        }

    }

In your user entity, the hasRole method can be something like :

   /**
     * @param string $role
     */
    public function hasRole(string $role)
    {
        return in_array(strtoupper($role), $this->getRoles(), true);
    }

• In your template, you can do something like :

{% if is_granted('CAN_DELETE', comment) %}
    <form action="{{ path('comment_delete', {'id': comment.id, 'articleId': article.id, 'articleSlug': article.slug}) }}" method="post">
       <input type="hidden" name="_csrf_token" value="{{csrf_token('delete_comment')}}" />
       <button>supprimer</button>
    </form>
{% endif %}

• Finally in your controller, you can do something like :

    /**
     * @Route("/blog/commentDelete/{id}-{articleId}-{articleSlug}", methods={"POST"}, name="comment_delete")
     */
    public function commentDelete($id, $articleId, $articleSlug, CommentRepository $commentRepository, EntityManagerInterface $em){

       $comment = $commentRepository->find($id);
       $csrfToken = $request->request->get('_csrf_token');

       if(!$this->isCsrfTokenValid('delete_comment', $csrfToken) || !$this->isGranted('CAN_DELETE', $comment){
           throw exception_for("Cette page n'existe pas");
       }

       $em->remove($comment);
       $em->flush();
       $this->addFlash('comment_success', 'Commentaire supprimé avec succès');
       return $this->redirectToRoute('blog_show', array('id' => $articleId, 'slug' => $articleSlug));
    }

Here your delete method is protected by the csrf token and the voter. I think this an attempt of solution.

Answer 2

In order to solve this kind of problems I would recommend to use Symfony Voters https://symfony.com/doc/current/security/voters.html

Answer 3

You are heading in the right direction. Always do validation and permission checks in the backend.

Hiding a link or using a form and setting it to disabled won't prevent people using dev-tools from sending the request to your controller. I'd rather see frontend checks as a convinience for users - directly showing them that some data is invalid / they are not allowed to do something, before making a request.

I am using the SensioFrameworkExtraBundle for ROLE checks (still i don't like annotations for such checks.. hmm) - throwing a permissionDeniedException if an user hasn't got the fitting role for the controllers action. Following it can be needed to do further checks like you did with $user->getId() != $comment->getAuthor()->getId()