Skip to content
This repository has been archived by the owner on Jul 3, 2020. It is now read-only.

[DOC] Adding contextualized permission #281

Open
bakura10 opened this issue Jan 3, 2015 · 15 comments
Open

[DOC] Adding contextualized permission #281

bakura10 opened this issue Jan 3, 2015 · 15 comments

Comments

@bakura10
Copy link
Member

bakura10 commented Jan 3, 2015

Hi everyone :),

In my application, I have a permission called "project.read". However, a project owner can invite many other users to have access to the project. Therefore, the assertion becomes quite complicated:

    class ProjectReadAssertion implements AssertionInterface
    {
        private $teamService;

        public function __construct(TeamService $teamService)
        {
            $this->teamService = $teamService;
        }

        public function isGranted(AuthorizationService $auth, $context)
        {
            if ($context->getOwner() === $auth->getIdentity()) {
                return true;
            }

            if ($teamService->projectHasMember($context, $auth->getIdentity())) {
                return true;
            }

            return $auth->isGranted('project.read_others');
        }
    }

This is complicated because it requires an injection of another service, it requires an additional DB call... It would actually be better to simply create a new permission each time a project is created ("project.read_1"), and assign/remove it each time a team member is removed. This way, instead of asking for the generic "project.read" permission, would ask for "project.read_1". The assertion would therefore be written like:

    class ProjectReadAssertion implements AssertionInterface
    {
        public function isGranted(AuthorizationService $auth, $context)
        {
                        $assertion = 'project.read_' . $context->getId();

            if ($auth->isGranted($assertion)) {
                return true;
            }

            return $auth->isGranted('project.read_others');
        }
    }

And the ProjectService/TeamService would be responsible for adding/removing the permission based on user actions.

How anyone did tackle this problem (especially at larger scale)? This seems a common problem, so I'd like to add a doc about it.

ping @danizord @Ocramius @arekkas

@Ocramius
Copy link
Contributor

Ocramius commented Jan 3, 2015

Having one permission per entry leads you back to the problems I had with ACL and per-record access control.

It's not a good idea to do that IMO. What you are doing is creating some sort of hash and using it for a lookup: not much difference with having dynamic logic and dependencies behind the scenes, except that you now have coupling with a hash format.

@aeneasr
Copy link
Contributor

aeneasr commented Jan 3, 2015

When using tokens, UAA from cloud foundry is suggesting to do it like this, because you can wildcard scopes: https://github.com/cloudfoundry/uaa/blob/master/docs/UAA-Tokens.md

If you can assure the permissions uniqueness (no colliding scopes etc), I think it should be fine. When you come to larger scales I would try to avoid db lookups and rather use permission scoping. It's cheaper to call computePermissionName('myscope', $object, 'read'); // = myscope.doc1.read rather than having that extra network roundtrip.

@Ocramius what do you mean with per-record ac?

@bakura10
Copy link
Member Author

bakura10 commented Jan 3, 2015

But this scope thing is actually what I suggested? So having one permission per "document" (or whatever) ?

@aeneasr
Copy link
Contributor

aeneasr commented Jan 3, 2015

Yes it is :)

@bakura10
Copy link
Member Author

bakura10 commented Jan 3, 2015

Regarding you saying "avoiding db lookups", do you mean sending all the permissions when getting the token, and checking client side in Javascript? The issue is that in my example, if a user is removed from a project (for instance), then if the permission is not validated server side, you could potentially have leaked information (JS still thinks user has permission, but it's not the case)

@aeneasr
Copy link
Contributor

aeneasr commented Jan 3, 2015

Never do authorization tasks in the front-end, the user can always manipulate browser behaviour ;) You will have to check that server-side.

@bakura10
Copy link
Member Author

bakura10 commented Jan 3, 2015

I know, so why do you say "avoiding roundtrip to server" then ?

@aeneasr
Copy link
Contributor

aeneasr commented Jan 3, 2015

When calling the database, you're doing a network roundtrip (tcp to mysql server), so avoid it if you can ;)

            if ($teamService->projectHasMember($context, $auth->getIdentity())) {
                return true;
            }

@bakura10
Copy link
Member Author

@arekkas , I have though of a simpler way. It is a kind of parameterized RBAC, but much simpler. Actually, most of the time, a simple "context" (can be anything and we should not enforce it, but we can assume that most of the time, it's simply an int that is the identifier of another resource).

The context is set as another column in the permission table:

/**
 * @ORM\Entity(repositoryClass="Account\Repository\PermissionRepository")
 * @ORM\Table(name="permissions")
 */
class Permission implements PermissionInterface
{
    /**
     * @var int
     *
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @var string
     *
     * @ORM\Column(type="string", length=50, unique=true)
     */
    private $name;

    /**
     * @ORM\Column(type="integer")
     */
    private $context;
}

Here the context is an integer, but once again, as ZfcRbac does not enforce schema, you can do what you want.

Now, instead of passing a permission name, you can pass an array:

$auth->isGranted(['project.read', 2]);

And you can change your Role hasAssertion implementation:

public function hasAssertion($assertionOrContext)
{
    if (is_array($assertionOrContext)) {
        // Create a Criteria object that match name and context
    }

    // Fallback to just checking name
}

Obviously, this still requires to add/remove permissions, but at least, you keep the same permission name and avoid the need to create a specific assertion name. And I also expect the queries to be faster because of the possibility to filter on an integer rather than string.

What do you think of this approach?

@bakura10
Copy link
Member Author

Actually, the main issue is that it makes migration very hard. If I decide to add a new permisssion, how am I supposed to deal with that? Should I add potentially thousands of rows to add it for each context combination?

@aeneasr
Copy link
Contributor

aeneasr commented Jan 23, 2015

Actually, the main issue is that it makes migration very hard. If I decide to add a new permisssion, how am I supposed to deal with that? Should I add potentially thousands of rows to add it for each context combination?

Yes, that is a major problem with this approach. Very similar to plain ACL (adding a new access right across thousands of documents).

@bakura10
Copy link
Member Author

This is highly annoying, I really don't know how to tackle this issue tbh :/.

@aeneasr
Copy link
Contributor

aeneasr commented Jan 23, 2015

I don't think it's easily solvable with rbac. I didn't get a look at abac yet but could this maybe solve these kind of problems?

@bakura10
Copy link
Member Author

I've tried to read it but from what I understand, it still suffers from the same issue. At a given point, everytime you need to "contextualize" something, this means lot of permissions (or lot of roles), and therefore the same migration hassle when you need to add a permission.

@claytondaley
Copy link

What you want sounds more like "relationship-based access control". A many-to-many table establishes relationships between projects and users. You could have a bunch of different relationships in the same table like "author", "editor", "collaborator", "reader", "liked", etc. A custom guard would check that table for the requested relationship by running a query like:

SELECT true FROM relationships WHERE id = $id AND project = $project AND relationship = $relationship LIMIT 1

I assume you're doing something similar (an efficient lookup) with the "LAZY" strategy in the docs on lots of permissions

You could expand on this by using "permissions" that are really the combination of a relationship and permission. Thus it'd be easy to add owner.delete to a system that already has owner.edit and collaborator.edit without creating a bunch of new relationships/permissions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants