Skip to content

[WIP] create voters_data_permission.rst article #3138

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cookbook/security/acl.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ the ACL system comes in.
Using ACL's isn't trivial, and for simpler use cases, it may be overkill.
If your permission logic could be described by just writing some code (e.g.
to check if a Blog is owned by the current User), then consider using
:doc:`voters </cookbook/security/voters>`. A voter is passed the object
:doc:`voters </cookbook/security/dataPermissionVoters>`. A voter is passed the object
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This document doesn't exist.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is the link then for

.. index::
    single: Security; Data Permission Voters

with dashes? sorry I could not figure out.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc directive references documents (e.g. filenames). So, if you want to reference your newly created voter chapter, this should like this:

:doc:`voters </cookbook/security/voters_data_permission>`

being voted on, which you can use to make complex decisions and effectively
implement your own ACL. Enforcing authorization (e.g. the ``isGranted``
part) will look similar to what you see in this entry, but your voter
Expand Down
25 changes: 25 additions & 0 deletions cookbook/security/voter_interface.rst.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
.. code-block:: php

interface VoterInterface
{
public function supportsAttribute($attribute);
public function supportsClass($class);
public function vote(TokenInterface $token, object, array $attributes);
}

The :method:`Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::supportsAttribute`
method is used to check if the voter supports the given user attribute (i.e: a role, an acl, etc.).

The :method:`Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::supportsClass`
method is used to check if the voter supports the current user token class.

The :method:`Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::vote`
method must implement the business logic that verifies whether or not the
user is granted access. This method must return one of the following values:

* ``VoterInterface::ACCESS_GRANTED``: The user is allowed to access the
application
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these items should start at the same level as the backticks of the previous line. So:

* ``VoterInterface::ACCESS_GRANTED``: The user is allowed to access the
  application

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, same for the other list items below

* ``VoterInterface::ACCESS_ABSTAIN``: The voter cannot decide if the user
is granted or not
* ``VoterInterface::ACCESS_DENIED``: The user is not allowed to access
the application
23 changes: 1 addition & 22 deletions cookbook/security/voters.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,7 @@ A custom voter must implement
:class:`Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface`,
which requires the following three methods:

.. code-block:: php

interface VoterInterface
{
public function supportsAttribute($attribute);
public function supportsClass($class);
public function vote(TokenInterface $token, $object, array $attributes);
}

The ``supportsAttribute()`` method is used to check if the voter supports
the given user attribute (i.e: a role, an ACL, etc.).

The ``supportsClass()`` method is used to check if the voter supports the
class of the object whose access is being checked (doesn't apply to this entry).

The ``vote()`` method must implement the business logic that verifies whether
or not the user is granted access. This method must return one of the following
values:

* ``VoterInterface::ACCESS_GRANTED``: The authorization will be granted by this voter;
* ``VoterInterface::ACCESS_ABSTAIN``: The voter cannot decide if authorization should be granted;
* ``VoterInterface::ACCESS_DENIED``: The authorization will be denied by this voter.
.. include:: /cookbook/security/voter_interface.rst.inc

In this example, you'll check if the user's IP address matches against a list of
blacklisted addresses and "something" will be the application. If the user's IP is blacklisted, you'll return
Expand Down
207 changes: 207 additions & 0 deletions cookbook/security/voters_data_permission.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
.. index::
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new document needs to be referenced in /cookbook/map.rst and /cookbook/security/index.rst.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added them right under the voter article.

single: Security; Data Permission Voters

How to Use Voters to Check User Permissions
===========================================

In Symfony2 you can check the permission to access data by using the
:doc:`ACL module </cookbook/security/acl>`, which is a bit overwhelming
for many applications. A much easier solution is to work with custom voters,
which are like simple conditional statements. Voters can be
also be used to check for permission as a part or even the whole
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the first be: Voters can also be used [...]

You can then move also and be to the line above I suppose.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[...] for permission to of the application or even [...]

application: ":doc:`/cookbook/security/voters`".

.. tip::

Have a look at the chapter
:doc:`authorization </components/security/authorization>`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd move chapter behind the doc ref:

.. tip::

    Have a look at the
    :doc:`authorization </components/security/authorization>`
    chapter for a better understanding on voters.

for a better understanding on voters.

How Symfony Uses Voters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How Symfony uses Voters

-----------------------

In order to use voters, you have to understand how Symfony works with them.
In general, all registered custom voters will be called every time you ask
Symfony about permissions (ACL). You can use one of three different
approaches on how to handle the feedback from all voters: affirmative,
consensus and unanimous. For more information have a look at
":ref:`components-security-access-decision-manager`".
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:ref:`the section about access decision managers <components-security-access-decision-manager>`

This sould make it more readable.


The Voter Interface
-------------------

A custom voter must implement
:class:`Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface`,
which has this structure:

.. include:: /cookbook/security/voter_interface.rst.inc

In this example, it'll check if the user will have access to a specific
object according to your custom conditions (e.g. he must be the owner of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace he with they

the object). If the condition fails, you'll return
``VoterInterface::ACCESS_DENIED``, otherwise you'll return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you -> it?

``VoterInterface::ACCESS_GRANTED``. In case the responsibility for this decision
does not belong to this voter, it will return ``VoterInterface::ACCESS_ABSTAIN``.

Creating the Custom Voter
-------------------------

You could store your Voter to check permission for the view and edit action like the following::
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say implement instead of store.


// src/Acme/DemoBundle/Security/Authorization/Entity/PostVoter.php
namespace Acme\DemoBundle\Security\Authorization\Entity;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The voter should not live in the Entity namespace. Use the Acme\DemoBundle\Security\Authorization\Voter namespace instead.


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use Acme\DemoBundle\Entity\Post

(see below)

use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\User\UserInterface;
use Doctrine\Common\Util\ClassUtils;

class PostVoter implements VoterInterface
{
const VIEW = 'view';
const EDIT = 'edit';

public function supportsAttribute($attribute)
{
return in_array($attribute, array(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have used constants like in built-in voters.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you also want to use the constant within the controller to check for?

e.g.

if (false === $this->get('security.context')->isGranted(PostVoter::VIEW, $post)) {
    throw new AccessDeniedException('Unauthorised access!');
}

self::VIEW,
self::EDIT,
));
}

public function supportsClass($obj)
{
if ($obj instanceof 'Acme\DemoBundle\Entity\Post') return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ($obj instanceof Post) {
    return true;
}

and add a use statement for this class.


return false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add new line before return statement

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not simplify this to?

return $obj instanceof Post;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point

}

/** @var \Acme\DemoBundle\Entity\Post $post */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use multiple lines for this

public function vote(TokenInterface $token, $post, array $attributes)
{
// check if class of this object is supported by this voter
if (!$this->supportsClass($post)) {
return VoterInterface::ACCESS_ABSTAIN;
}

// check if voter is used correct, only allow one attribute for a check
if(count($attributes) !== 1 || !is_string($attributes[0])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 !== count($attributes)

throw new InvalidArgumentException(
'Only one attribute is allowed for VIEW or EDIT'
);
}

// set the attribute to check against
$attribute = $attributes[0];

// get current logged in user
$user = $token->getUser();

// check if the given attribute is covered by this voter
if (!$this->supportsAttribute($attribute)) {
return VoterInterface::ACCESS_ABSTAIN;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this implementation looks weird to me: you are abstaining even if some of the attributes are supported. None of the core voters are behaving this way (most of the time, you are checking a single attribute so it is not a common case though)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, I am going to allow one Attribute only.


// check if given user is instance of user interface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment?

if (!$user instanceof UserInterface) {
return VoterInterface::ACCESS_DENIED;
}

switch($attribute) {
case 'view':
// the data object could have for e.g. a method isPrivate()
// which checks the Boolean attribute $private
if (!$post->isPrivate()) {
return VoterInterface::ACCESS_GRANTED;
}
break;

case 'edit':
// we assume that our data object has a method getOwner() to
// get the current owner user entity for this data object
if ($user->getId() === $post->getOwner()->getId()) {
return VoterInterface::ACCESS_GRANTED;
}
break;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this line break and you did not even use the container 👎

}
}

That's it! The voter is done. The next step is to inject the voter into
the security layer.

Declaring the Voter as a Service
--------------------------------

To inject the voter into the security layer, you must declare it as a service
and tag it as a 'security.voter':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use literals instead of the single quotes


.. configuration-block::

.. code-block:: yaml

# src/Acme/AcmeBundle/Resources/config/services.yml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

src/Acme/DemoBundle/Resources/config/services.yml

services:
security.access.post_voter:
class: Acme\DemoBundle\Security\Authorization\Entity\PostVoter
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be Acme\DemoBundle\Security\Authorization\Voter\PostVoter

public: false
tags:
- { name: security.voter }

.. code-block:: xml

<?xml version="1.0" encoding="UTF-8" ?>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add <!-- src/Acme/DemoBundle/Resources/config/services.xml --> above this line

<container xmlns="http://symfony.com/schema/dic/services">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XSD location is missing. It should look like this:

<container xmlns="http://symfony.com/schema/dic/services"
    xsi:schemaLocation="http://symfony.com/schema/dic/services
        http://symfony.com/schema/dic/services/services-1.0.xsd"
>

<services>
<service id="security.access.post_document_voter"
class="Acme\DemoBundle\Security\Authorization\Document\PostVoter"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be Acme\DemoBundle\Security\Authorization\Voter\PostVoter

public="false">
<tag name="security.voter" />
</service>
</services>
</container>

.. code-block:: php

$container
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add // src/Acme/DemoBundle/Resources/config/services.php above this line

->register(
'security.access.post_document_voter',
'Acme\DemoBundle\Security\Authorization\Document\PostVoter'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be Acme\DemoBundle\Security\Authorization\Voter\PostVoter

)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong indentation:

$container
    ->register(
        'security.access.post_document_voter',
        'Acme\DemoBundle\Security\Authorization\Voter\PostVoter'
    )
    ->addTag('security.voter')
;

->addTag('security.voter')
;

How to Use the Voter in a Controller
------------------------------------

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing some text

The registered voter will then always be asked as soon as the method 'isGranted'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use literals here:

``isGranted()``

from the security context is called.

.. code-block:: php

// src/Acme/DemoBundle/Controller/PostController.php
namespace Acme\DemoBundle\Controller;

use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;

class PostController
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have to extend the Controller class from the FrameworkBundle. Otherwise, you could not use this->get(...) to retrieve a service from the container.

{

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this line

/**
* @Route("/blog/{id}")
* @ParamConverter("post", class="SensioBlogBundle:Post")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if I like these. We can just remove them and say "Then register this controller to a route". And also put $post = ...; in the controller below.

Otherwise, the use statements for both annotations are missing

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if I covered everything from this answer of you? I am not sure about the $post = ...;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You didn't, I meant this:

        public function showAction()
        {
            // get a Post instance
            $post = ...;

            // keep in mind, this will call all registered security voters
            if (false === $this->get('security.context')->isGranted('view', $post)) {
                throw new AccessDeniedException('Unauthorised access!');
            }

            return new Response('<h1>'.$post->getName().'</h1>');
        }
    }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I think you are right to simplify that example be removing obvious stuff. Changed it.

*/
public function showAction(Post $post)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use statement for the Post class is missing

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats solved but not detected by github

{
// keep in mind, this will call all registered security voters
if (false === $this->get('security.context')->isGranted('view', $post)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears that $post object should be recover before this check. (by a find($id) or with ParamConverter feature)

throw new AccessDeniedException('Unauthorised access!');
}

return new Response('<h1>'.$post->getName().'</h1>');
}
}