Allow to configure auth access from the resource class #938

Merged
merged 1 commit into from Feb 19, 2017

Projects

None yet

4 participants

@dunglas
Member
dunglas commented Feb 8, 2017 edited
Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets api-platform/api-platform#109 (partially), #234
License MIT
Doc PR todo

Edit: now with expression language support

This PR allows to setup authorization controls directly from the resource.

In the following exemple, the only logged in admins can access all operations related to the Secured resource. However, regular users owning a specific resource can also access to it.

All variables available in Symfony's access control expressions are available. The object variable value is the current resource (or collection of resources).

<?php

// src/AppBundle/Entity/Secured.php

namespace AppBundle\Entity;

use ApiPlatform\Core\Annotation\ApiResource;
use Doctrine\ORM\Mapping as ORM;

/**
 * @ApiResource(
 *     attributes={"is_granted"="has_role('ROLE_ADMIN')"},
 *     itemOperations={
 *         "get"={"method"="GET", "is_granted"="object.getOwner() == user"}
 *     }
 * )
 * @ORM\Entity
 */
class Secured
{
    /**
     * @ORM\Column(type="integer")
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    public $id;

    /**
     * @ORM\Column(type="text")
     */
    public $owner;
}
@desmax
desmax commented Feb 10, 2017

Great feature!
But what if you pass to isGranted() not only attribute, but api resource object as well?
This way we could use advantage of symfony voters.

@dunglas
Member
dunglas commented Feb 10, 2017

@desmax good idea.

@teohhanhui
Member

(Old suggestion: api-platform/core#234)

I think we should use the Security expression language:

"access_control"="is_granted('ROLE_ADMIN')"

Basically to have the same effect as http://symfony.com/doc/current/bundles/SensioFrameworkExtraBundle/annotations/security.html

@teohhanhui
Member

And this feature must be disabled when the Symfony Security component is not present. (Additionally, the Symfony Expression component too, if we go with that.)

@dunglas
Member
dunglas commented Feb 15, 2017

PR updated:

  • Now use expression language
  • Give access to the current object
  • Throw an exception if (and only if) a security rule is used but the security bundle or the expression language component isn't installed
src/EventListener/DenyAccessListener.php
+ }
+
+ if (null === $this->authorizationChecker) {
+ throw new \LogicException('The "symfony/security" library must be installed to use the "is_granted" attribute.');
@lyrixx
lyrixx Feb 15, 2017

Does it worth it to add something like "on property X on class Y" ? to ease debug ? (same for next exception)

src/EventListener/DenyAccessListener.php
+
+ foreach ($isGranted as $expression) {
+ if ($this->authorizationChecker->isGranted(new Expression($expression), $data)) {
+ return;
@lyrixx
lyrixx Feb 15, 2017

This logic is a bit weird. If at least one expression is valid, then it's OK.
What if I want an and between all my expressions? And In order to avoid dealing with this kind of decission, why not allowing only one is_granted?

(Actually, I don't know, I'm just asking)

@dunglas
dunglas Feb 15, 2017 Member

Indeed, this is a rest of the old logic (without expression language) but it's better to only allow one expression now.

I'll update the PR.

@dunglas dunglas Allow to configure auth access from the resource class
3d12f34
@dunglas dunglas merged commit b5323f1 into api-platform:master Feb 19, 2017

4 of 5 checks passed

coverage/coveralls Coverage decreased (-0.01%) to 96.756%
Details
SensioLabsInsight Code quality OK.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/styleci/pr The StyleCI analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dunglas dunglas deleted the dunglas:security branch Feb 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment