Skip to content
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

Access control based on wildcards #59

Closed
filipefigcorreia opened this issue Dec 27, 2011 · 13 comments
Closed

Access control based on wildcards #59

filipefigcorreia opened this issue Dec 27, 2011 · 13 comments
Milestone

Comments

@filipefigcorreia
Copy link

This is an enhancement suggestion.

Guardian supports object based permissions, which allows a finer-grained control compared to what django does out-of-the-box. What I'd like to suggest is to support wildcards when choosing which objects to apply the permissions.

For example, instead of saying:
"User 1 has read access to object a-42 of the model MyClassA"
"User 1 has read access to object a-43 of the model MyClassA"
...

We could instead say:
"User 1 has read access to objects of the model MyClassA that match the condition first_name = 'Jhon'"
...

I.e., let "User 1" read all objects with a "first_name" attribute with the value "Jhon".


Edit: Removed reference to Attribute-based access control, which is not what I meant at all.

@mhaligowski
Copy link

Is it supposed to be dynamic? I.e.

  1. "Create User1, object a-42 of the model MyClassA"
  2. "Assign read permission to objects of the model MyClassA that match the condition first_name = 'John'"
    Now, as a-42 matches the condition first_name = 'John', User1 has read permission to a-42
  3. "Create a-43 object of the model MyClassA, that matches first_name = 'John'"

Should the wildcard-based control be dynamic, the User1 would already have the read access. Should it be static, the read permission would have to be assigned once again.

In other words, the dynamic wildcard-based access control is resolved when calling "has_perm", and will probably require much more work to be done. The static access control means resolving the permission at the moment of calling assign function.

@lukaszb
Copy link
Contributor

lukaszb commented Apr 19, 2012

Sounds ok to me. Plus, it's relatively easy to implement (seems like allowing obj to be sequence of particular model instance should be enough). Any takers for this one? I expect this to be properly tested and documented.

@filipefigcorreia
Copy link
Author

@Halish Hi. I think I meant what you are calling the dynamic approach. That is: assign a permission to a wildcard, and all objects (current and future) which match that wildcard will be bound to that permission. It's not about batch-assigning permissions to a set of objects, but rather to check permissions based on a "rule". These rules could be easily changed, without changing lots of permission-related records on the background; because the wildcard is only checked/matched when the permission is checked.

@lukaszb I probably won't be able to work on this for some more months... But I'm still very much interested and may be able to help someone interested in taking it forward (if not more, by providing and discussing ideas, and testing).

@mhaligowski
Copy link

@lukaszb I'm not really sure what you mean by sequence of particular model instance. If you mean to call the assign as:

assign('perm', user, [obj1, obj2, obj3])
assign('perm', user, Model.objects.filter(owner__id == user.pk))

then we would get static access, which in will be a shortcut for an iteration:

for o in [obj1, obj2, obj3]:
    assign('perm', user, o)

What I would do is create new classes ('d have to think it over): UserWildcardPermission and GroupWildcardPermission. Instead of content_object field, they will utilize the model object, which would define for which model the permission is, and a set of filters (just like QuerySet).

The wildcards permissions will be resolved at the moment of calling has_perm.

Do we agree at the design?

@lukaszb
Copy link
Contributor

lukaszb commented May 16, 2012

Okey, seems I now got it right.

I don't see easy way to do this - what rules are there going to be? Simple attributes equality/similarity checks? What else? I just don't see this task to be defined well enough.

I need use cases. Now it seems to me that someone who really want such behavior can easily add some custom code over the guardian itself to achieve it. Remember that this is reusable app, it should help to solve general problems.

Few questions:

  1. Do we need to define those rules via some kind of widget/form? Or is code-side enough? How about django-authority solution (http://packages.python.org/django-authority/create_custom_permission.html) [which I really like, btw]?
  2. Maybe roles could be an answer to some of the requested behavior? Roles thtat could be attached to user/group. Which could be subclassed and extended somehow. But this is another issue (Add roles #23).

Generally, I'm not into this one too much. Not before someone would give me good use cases and implementation example (can be abstract, should include API changes/additions and data model changes).

PS. Guys, please make examples easy to read. What the fuck is a-42? User joe, class Post, object obj1/post1 and all is fine, really.

@filipefigcorreia
Copy link
Author

I don't see easy way to do this - what rules are there going to be? Simple attributes equality/similarity checks? What else? I just don't see this task to be defined well enough.

I need use cases. Now it seems to me that someone who really want such behavior can easily add some custom code over the guardian itself to achieve it. Remember that this is reusable app, it should help to solve general problems.

True. I would argue that this feature is not that specific though. I see immediate application on a couple of projects that I'm involved with, and they're not that special. I want to write a better example, using code to illustrate a possible API for this. Hopefully I'll have some extra time next week to think about it.

Few questions:
Do we need to define those rules via some kind of widget/form? Or is code-side enough?

I think we can split this in sub-goals. While a widget/form would be great, it probably involves a lot of work, and a code-side approach would already be very good. To offer the user the option to activate "rules" from a list that we have defined on the code-side would already be a good improvement.

Like @Halish mentioned, the easiest way to implement may be to use regular django QuerySets to define the "rules". The widget/form may appear later, as a (very) simplified QuerySet editor, that would allow to express equality/similarity checks. But I definitely see that as a bonus... :)

How about django-authority solution (http://packages.python.org/django-authority/create_custom_permission.html) [which I really like, btw]?

Development in django-authority seems to have slowed down. There are some forks, but they also haven't been receiving a lot of attention. So, I've been avoiding it... even though, like you point out, it seems to address this issue. It seems to use a code-side approach only too. There may be other important differences to django-authority, but the lack of maintenance is enough for me to wish guardian itself to support wildcard/custom permissions too.

Maybe roles could be an answer to some of the requested behavior? Roles thtat could be attached to user/group. Which could be subclassed and extended somehow. But this is another issue (#23).

I see a connection, but I think that what we're discussing is a very different thing from what people usually call roles.

I see roles as very similar to groups, with some differences: when people talk about groups, each user usually belongs to a single group and there's sometimes several levels (i.e., groups inside groups); When roles are used instead, users can belong to several roles simultaneously, but the roles are usually flat -- there's no hierarchy of roles.

So, roles and groups are ways to handle sets of users when assigning permissions. They are handy when the current active users change frequently, so managing the permissions for them individually becomes a nuisance. We'd rather just assign them to a role (with all the permissions that come with it).

The wildcards we are talking about, on the other hand, are a way to handle sets of objects when assigning permissions. They are useful when objects change frequently, and the permissions should be different depending on how they change. In this case, it's great if we can just define the rule that decides wich objects are accessible, and don't worry with permission assignment when the object changes.

Now, I may be seeing the connection you were making (pls confirm if this is it)... What if instead of a role we would have just a "rule" that would "select" the set of users that should have permissions in a given scenario. For example, we may want to let premium users have access to "special" objects, that the free users can't have access to. We'd just have to create a rule that uses a "is_paying_user" field that we would have extended our User model with. If we also want to deny access to users that in spite of being "premium" have overdue payments, the rule could have that constraint too. This use case would also be very useful! Even though it wasn't what I was suggesting :)

Generally, I'm not into this one too much. Not before someone would give me good use cases and implementation example (can be abstract, should include API changes/additions and data model changes).

I will try to provide some examples illustrated by code next week.

PS. Guys, please make examples easy to read. What the fuck is a-42? User joe, class Post, object obj1/post1 and all is fine, really.

Sorry, will try to make the next ones simpler :)

Thanks for listening.

@filipefigcorreia
Copy link
Author

Hi. Here's the example I came up with. Please tell me what you think. In this (simplistic) example, the wildcard permissions allow users to modify only the blog posts that they authored, and that are still in draft.

The model:

from django.db import models
from django.contrib.auth.models import User

class Post(models.Model):
    title = models.CharField(max_length=60)
    body = models.TextField()
    author = models.ForeignKey(User)
    is_published = models.BooleanField(default=False)

The views:

from models import *
from django.http import HttpResponse
from guardian.models import *

def create_data(request):
    jack = User.objects.create_user(
        username='jack', email='jack@example.com', password='topsecretagentjack')
    jack.save()
    jane = User.objects.create_user(
        username='jane', email='jane@example.com', password='topsecretagentjane')
    jane.save()
    post1 = Post.objects.create(
        title='A first post', body='Contents...', author=jack)
    post1.save()
    post2 = Post.objects.create(
        title='A second post', body='Contents...', author=jane)
    post2.save()
    post3 = Post.objects.create(
        title='A third post', body='Contents...', author=jack, is_published=True)
    post3.save()

    # allow jack to change only the posts that he authored and that are still in draft
    UserWildcardPermission.objects.assign('change_post',
        user=jack,
        mdl=Post,
        objs=Post.objects.filter(author=jack).filter(is_published=False))

    return HttpResponse("Data created!")

def try_access(request):
    jack = User.objects.get(username='jack')
    post1 = Post.objects.get(id=1)
    post2 = Post.objects.get(id=2)
    post3 = Post.objects.get(id=3)
    assert(jack.has_perm('change_post', post1) == True)
    assert(jack.has_perm('change_post', post2) == False) 
    assert(jack.has_perm('change_post', post3) == False)

    return HttpResponse("Done")

@mhaligowski
Copy link

@filipefigcorreia It's really great, but I believe there's one more feature to be added.

# tests.py
from model import Post
from django.utils import unittest
from guardian.models import *

class WildcardPermissionsTestCase(unittest.TestCase):
    def setUp(self):
        # the 1st user
        self.jack = User.objects.create_user(
            username='jack', 
            email='jack@example.com', 
            password='topsecretagentjack')
        self.jack.save()

        #the 2nd user
        self.jane = User.objects.create_user(
          username='jane', 
          email='jane@example.com', 
          password='topsecretagentjane')
        self.jane.save()

        # some objects
        self.post1 = Post.objects.create(
          title='A first post', 
          body='Contents...', 
          author=self.jack)
        self.post1.save()

        self.post2 = Post.objects.create(
          title='A second post', 
          body='Contents...', 
          author=self.jane)
        self.post2.save()

        self.post3 = Post.objects.create(
          title='A third post', 
          body='Contents...', 
          author=self.jack, 
          is_published=True)
        self.post3.save()

    def test_try_access(self):
        """
        This test is for _past_ objects. Yo would achieve the same by making this:

        objs = list(Post.objects.filter(author=jack, is_published=False)) # force evaluation
        for o in objs: assign('change_post', self.jack, o)
        """
        # allow jack to change only the posts that he authored and that are still in draft
        UserWildcardPermission.objects.assign('change_post',
           user=self.jack,
           mdl=Post,  # do we need this here?
           objs=Post.objects.filter(author=self.jack, is_published=False)

        self.assertTrue(self.jack.has_perm('change_post', self.post1))
        self.assertFalse(self.jack.has_perm('change_post', self.post2)) 
        self.assertFalse(self.jack.has_perm('change_post', self.post3))

    def test_future_try_access(self):
        """
        Here it gets more complicated. You say: I want _all_ my objects, past, and future, 
        to be subject to the given permission. Here's the test for that.

        Up to now, I've been using signals in Django to do that. 
        """
        UserWildcardPermission.objects.assign('change_post',
           user=self.jack,
           mdl=Post,
           objs=Post.objects.filter(author=self.jack, is_published = False)

        # now, create new post
        post4 = Post.objects.create(
          title='New post',
          body='Contents...'
          author=self.jack)

        self.assertTrue(self.jack.has_perm('change_post', self.post4))

I hope that made the case clearer. The real challenge here is storing the QuerySet in the database, which would probably require writing some kind of QuerySetModel adapter.

Also, it would be worth designing more general wildcard permissions, for granting permissions to owners.

@filipefigcorreia
Copy link
Author

@Halish That's a good point. That last test is a bit more thorough.

From what I understand, to persist a QuerySet we just need to store the value of the queryset.query attribute. And to recreate the queryset object later, we just need to create a generic one of the right type (e.g., queryset = Something.objects.all()) and then replace the value of queryset.query with whatever we've stored earlier. I haven't tested this yet, but it'd probably be the first thing I'd try.

@mhaligowski
Copy link

@filipefigcorreia Quick look here: https://github.com/django/django/blob/master/django/db/models/query.py#L33 reveals, that the QuerySet can even be created with an instance of a Query. The Query itself is not a Model, and I don't think that storing a pickled object would be proper here, so it will still require some kind of adapter, as I mentioned earllier.

@lukaszb The case I need it for is giving the 'edit' access only to the owner. As I mentioned earlier, I'm doing it on signals. It would be way easier though, if I could define it probably with fixtures.

@filipefigcorreia
Copy link
Author

@Halish I was thinking we could store the raw sql command only (not the pickled object), as it'll probably avoid a lot of joins with the extra QuerySetModel table. But that's assuming that the only thing we needed to store for QuerySets is their raw sql, which may be naive... If there are more attributes of the QuerySet objects that we need to serialize, than I'd say we're better of creating a proper Model for storing QuerySets.

@mhaligowski
Copy link

@filipefigcorreia I don't think that holding the raw SQL in the database is a good idea. In case of changing the database engine, there's a risk that the new database engine will not be able to process the SQL stored in the database. Also, when preparing the SQL for checking the permissions, we'll probably use only some parts of the queries, and not the whole raw SQL.

I'll try to find some time to experiment a little in my own branch, and to design it well.

@lukaszb
Copy link
Contributor

lukaszb commented Feb 22, 2013

Okey, not much discussion here. And best use case is some specific access for the owner of an object. This should be handled at the view explicitly in my view. Closing for now.

@lukaszb lukaszb closed this as completed Feb 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants