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
Changes from 1 commit
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
173 changes: 173 additions & 0 deletions cookbook/security/voters_data_permission.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
.. 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 implement your own Voter to check the permission for a object agains a user
Copy link
Contributor

Choose a reason for hiding this comment

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

to check user permissions for accessing a given object

==================================================================================

In Symfony2 you can check the permission to access data by the
:doc:`ACL module </cookbook/security/acl>` which is a bit overhelming
Copy link
Member

Choose a reason for hiding this comment

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

, which is a bit

for many applications. A much easier solution is working with custom
Copy link
Member

Choose a reason for hiding this comment

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

is to work with custom voters

voters, which are like simple conditional statements. Voters can be
also 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.

can also be used ... for permissions ...

application: :doc:`cookbook/security/voters`.
Copy link
Member

Choose a reason for hiding this comment

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

use absolute paths (start with a slash) and put it inside double quotes

Copy link
Author

Choose a reason for hiding this comment

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

so like :doc:"/cookbook/security/voters"`` ? Sorry if its dump question, but I haven't seen in the whole documentation something like :doc:"cookbook/security/voters"


.. tip::

It is good to understand the basics about what and how
Copy link
Contributor

Choose a reason for hiding this comment

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

why is it good? need to connect it

:doc:`authorization </components/security/authorization>` works.
Copy link
Member

Choose a reason for hiding this comment

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

please link to the correct section into the book/security article instead

Copy link
Author

Choose a reason for hiding this comment

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

Well the book covers not the same information like in /components/security/authorization, which is in my opinion much more relevant. While the book entry covers more the parts and bits to secure the whole application, does the components entry explain the tools that we are using here.


How symfony works with voters
Copy link
Member

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.
Copy link
Member

Choose a reason for hiding this comment

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

voters, you have to understand how Symfony works with them.

In general all registered custom voters will be called every time you ask
Copy link
Member

Choose a reason for hiding this comment

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

In general, all registered

symfony about permission (ACL). In general there are three different
Copy link
Contributor

Choose a reason for hiding this comment

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

permissions

Copy link
Member

Choose a reason for hiding this comment

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

Symfony

approaches on how to handle the feedback from all voters:
: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.

put also in double quotes


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

A custom voter must implement
:class:`Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface`,
which requires the following three methods:
Copy link
Member

Choose a reason for hiding this comment

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

which has this structure:


.. code-block:: php
Copy link
Member

Choose a reason for hiding this comment

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

use the :: shortcut

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I don't know hat you mean exactly?


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
Copy link
Member

Choose a reason for hiding this comment

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

use :method: role

Copy link
Author

Choose a reason for hiding this comment

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

sorry I dont get it

Copy link
Member

Choose a reason for hiding this comment

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

Use:

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 ``supportsClass()`` method is used to check if the voter supports the
Copy link
Member

Choose a reason for hiding this comment

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

same here

current user token class.

The ``vote()`` method must implement the business logic that verifies whether
Copy link
Member

Choose a reason for hiding this comment

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

and here

or not the user is granted access. This method must return one of the following
values:
Copy link
Member

Choose a reason for hiding this comment

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

put this in a list

Copy link
Author

Choose a reason for hiding this comment

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

You mean line 44 to 52, covering each point as a list entry?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, and this can also be added to the included file, I guess.


* ``VoterInterface::ACCESS_GRANTED``: The user is allowed to access the application
* ``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

In this example, you'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 the object). If the condition fails, 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.

missing line breaks after the first word that crosses the 72th character

``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 responsebility for this decision belong not to this voter, he will return
Copy link
Contributor

Choose a reason for hiding this comment

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

responsibility

Copy link
Contributor

Choose a reason for hiding this comment

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

belongs

Copy link
Contributor

Choose a reason for hiding this comment

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

it is not a he, it

``VoterInterface::ACCESS_ABSTAIN``.

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

You could store your Voter for the view and edit method of a post within ACME/DemoBundle/Security/Authorization/Document/PostVoter.php.
Copy link
Contributor

Choose a reason for hiding this comment

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

of a blog 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 Acme not ACME

Copy link
Member

Choose a reason for hiding this comment

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

also, use a literal (double quotes) and I don't think the filename needed, because you already add it to the example


.. code-block:: php
Copy link
Member

Choose a reason for hiding this comment

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

and then remove this

Copy link
Author

Choose a reason for hiding this comment

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

should I not declare somewhere that it is php?

Is it maybe then, wourl that work?: ... view and edit action like following:: php

Copy link
Member

Choose a reason for hiding this comment

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

no, the default language is PHP for the Symfony docs. So just [...] edit action like the following:: is the way it should be done (please note the missing 'the' too)

Copy link
Author

Choose a reason for hiding this comment

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

right :-)


// src/Acme/DemoBundle/Security/Authorization/Document/PostVoter.php
Copy link
Member

Choose a reason for hiding this comment

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

we always use the ORM in the docs, so please use the Entity namespace instead of the Document namespace

namespace Acme\DemoBundle\Security\Authorization\Document;

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\DependencyInjection\ContainerInterface;
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;

class PostVoter implements VoterInterface
{
private $container;

public function __construct(ContainerInterface $container)
Copy link
Contributor

Choose a reason for hiding this comment

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

teaching to inject the container is outrageous at this date and age

{
$this->container = $container;
}

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 blank line

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.

use 4 spaces to indent

'view',
'edit'
Copy link
Member

Choose a reason for hiding this comment

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

always add a comma after multi-line array items

));
}

public function supportsClass($class)
{
// could be "ACME\DemoBundle\Entity\Post" as well
$array = array("ACME\DemoBundle\Document\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 instanceof check


foreach ($array as $item) {
// check with stripos in case doctrine is using a proxy class for this 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 is not the right way to check for Doctrine proxies (you can have lots of false positive with your stripos detection).

  • You should be using Doctrine\Common\Util\ClassUtils::getRealClass($class) to resolve proxies (available since Common 2.2, so it is usable in all Symfony 2.1+ projects)
  • Your voter should support all child classes of Post according to the rules of OOP, not only the exact match on the class

Copy link
Author

Choose a reason for hiding this comment

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

Ah cool, I didn't know that. I will change that to be more proper.

Copy link
Contributor

Choose a reason for hiding this comment

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

i think there is even a util in doctrine common for this, but i may be wrong

Copy link
Author

Choose a reason for hiding this comment

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

yes this will be replaced by a better solution, please wait till tomorrow

if (stripos($s, $item) !== FALSE) {
Copy link
Member

Choose a reason for hiding this comment

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

use lowercase php constants: false

return true;
}
}
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

}

public function vote(TokenInterface $token, $object, array $attributes)
{
// get current logged in user
$user = $token->getUser();

// check if class of this object is supported by this voter
if ( !($this->supportsClass(get_class($object))) ) {
Copy link
Member

Choose a reason for hiding this comment

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

remove space between ()

return VoterInterface::ACCESS_ABSTAIN;
}

// check if the given attribute is covered by this voter
foreach ($attributes as $attribute) {
Copy link

Choose a reason for hiding this comment

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

Maybe I'm missing something but I don't see the point of checking the whole array if you are only using the first element after that.

Copy link
Member

Choose a reason for hiding this comment

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

I agree. Personally, I only ever allow 1 attribute to be passed - passing an array is confusing to me, because we could internally implement it using OR or AND logic. So, I like just using the first attribute, but we should throw an exception if there is more than 1 attribute and then grab the first attribute so that we don't need this foreach.

Copy link
Author

Choose a reason for hiding this comment

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

Alright, as the interface is forcing us to have a array parameter as the last one, I will do use always the first array element as described earlier: #3138 (comment)

if ( !$this->supportsAttribute($attribute) ) {
Copy link
Member

Choose a reason for hiding this comment

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

same here

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) ) {
Copy link
Member

Choose a reason for hiding this comment

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

and here

Copy link
Member

Choose a reason for hiding this comment

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

missing use statement for UserInterface

return VoterInterface::ACCESS_DENIED;
}

switch($this->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.

undefined property


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

case 'view':
if($object->isPrivate() === false) {
Copy link
Member

Choose a reason for hiding this comment

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

if (!$object->isPrivate()) {
}

(space between if and open ( and removing === false)

return VoterInterface::ACCESS_GRANTED;
}
break;

case 'edit':
if($object->getOwner()->getId() === $user->getId()) {
Copy link
Member

Choose a reason for hiding this comment

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

space after if

and switch $object->getOwner()->getId() and $user->getId()

return VoterInterface::ACCESS_GRANTED;
}
break;

default:
Copy link
Contributor

Choose a reason for hiding this comment

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

This case can be remove because we are already out with !$this->supportsAttribute($attribute) from https://github.com/symfony/symfony-docs/pull/3138/files#diff-70c7f0c16e0c51e7dc991ab9b801ff73R107

// otherwise denied access
return VoterInterface::ACCESS_DENIED;
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 rather throw an exception here - because of the supportsAttribute, we should never hit this line. If we do, something is programmed wrong.

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, I am adding this.

}

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. This can be done easily through the service container.
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the part for service container since you already removed it in the code example

Copy link
Author

Choose a reason for hiding this comment

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


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

To inject the voter into the security layer, you must declare it as a service,
Copy link
Member

Choose a reason for hiding this comment

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

remove last ,

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.

change " into double backticks


.. 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_document_voter:
class: Acme\DemoBundle\Security\Authorization\Document\PostVoter
Copy link
Contributor

Choose a reason for hiding this comment

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

also it seems you are using mongodb from the Document folder name

Copy link
Member

Choose a reason for hiding this comment

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

let's change it to ORM

public: false
arguments: [@service_container]
Copy link
Contributor

Choose a reason for hiding this comment

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

remove please

# we need to assign this service to be a security voter
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not needed, we can just say it gets tagged to be a voter

Copy link
Member

Choose a reason for hiding this comment

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

+1

tags:
- { name: 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.

missing xml and php:

<?xml version="1.0" encoding="UTF-8" ?>
<container xmlns="http://symfony.com/schema/dic/services">
    <services>
        <service id="security.access.post_document_voter"
            class="Acme\DemoBundle\Security\Authorization\Document\PostVoter"
            public="false">
            <tag name="security.voter" />
        </service>
    </services>
</container>
$container
    ->register('security.access.post_document_voter', 'Acme\DemoBundle\Security\Authorization\Document\PostVoter')
    ->addTag('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.

a trailing newline is missing