Permission System

[pizzapanther]

I would like to add a permission system but want to some feedback on the API before I implement.

You would have two options and I'm proposing to add both:

Option 1: Custom queryset method

This option would let you overwrite how a queryset is filtered.

class UserNode(DjangoObjectType):
  class Meta:
    model = User
    interfaces = (relay.Node,)
    only_fields = ('email', 'first_name', 'last_name')

  @classmethod
  def get_queryset (cls, queryset, args, request, info):
    return queryset.filter(owner=request.user)

Option 2: Permissions List

This option would setup a Meta API to use to define permissions

def auth_required(queryset, args, request, info):
  if request.user.is_authenticated():
    return queryset

  return queryset.none()

class UserNode(DjangoObjectType):
  class Meta:
    model = User
    interfaces = (relay.Node,)
    only_fields = ('email', 'first_name', 'last_name')
    permissions = [auth_required]

If these look like good APIs then I'll implement.

[pizzapanther]

Also note DjangoConnectionField and get_node for NodeFields would know to check for these APIs

[fmartins-zz]

@pizzapanther

I liked of your suggestion to implement a permission system. Please, could you look this other issue? graphql-python/graphene#385

At this moment I am interested and available to do this, what do you think about we plan this architecture and works together in this?

Thank you!

[nickhudkins]

@pizzapanther perhaps we could use an array of functions or strings. If listed as strings, we can check built in django permission code names and verify context.user against it. Thoughts?

[fmartins-in-loggi]

@valdergallo @romulorosa @rsalmei @nickhudkins @pizzapanther @ekampf @barakcoh

In our case, we have over a hundred objects that extends DjangoObjectType and would like a simple method to filter nodes.

Assuming that context.user has methods called has_perm and has_perms, we could use the Django perms to build an interface that seems good to us. See the example below:

class MyCustomNode(DjangoObjectType):
    my_custom_field = graphene.List(String)

    def resolve_my_custom_field(self, args, context, info):
        """Here, it is not necessary to inspect if `context.user` actually has
        permission to view this field, because the framework handles that 
        internally.
        """"
        return 1

    class Meta(object):
        model = MyCustomObjectFromDomain
        only_fields = (
            'field_a_from_model',
            'field_b_from_model',
            'my_custom_field',
        )
        permissions = {
            'django_permission_x': (
                'field_a_from_model',
                'my_custom_field',
            ),
            'django_permission_y': (
                'field_a_from_model',
                'field_b_from_model',
            ),
            # For django super users, all fields should be accessible.
        }

In this example above, the custom field my_custom_field is only available to super users or user that is in group django_permission_x.

Important: The not allowed fields for user should not be displayed in introspection query too, which would help GraphQL docs.

[pizzapanther]

@nickhudkins Like the strings and functions idea. That makes it simple and very extensible.

@fmartins-in-loggi your implementation looks a little complex. I'd rather write a small function than use so many configurations, but that is my bias and we could probably start a flame war about it. Also I think we could start with simple functions and then implement your config example as a layer on top of it.

I like to remember "code always wins". I was going to implement this and just wanted to get some feedback. But it looks like others are ready to implement. So who wants to implement?

[crucialfelix]

Another approach to this is to use the Relay convention of a top level viewer field. I am thinking of creating a top level 'admin' and 'public' field so these roles serve as an easy gateway to everything below it.

public only has certain models, and a limited number of fields on those models.

admin has many more models and all fields on all of them.

So if the request.user is not an admin, then 'admin' returns None and that is the end of that. No need to do permission checks on any nodes below. No need to write complicated runtime checks for field level view permissions. Much easier to read and write the code.

The other nice thing is that if you write some graphql to query on public and you access a private field then igraphql or your clever graphql compiler can red flag it immediately.

[BossGrand]

@crucialfelix

But wouldn't this mean you have to create two types of nodes for each model you have? One for admin and one for public?

[crucialfelix]

Yes if the nodes have very different fields, different filters etc. If you want the exact same then you should be able to just include the DjangoObjectType twice.

I'm not sure if you can do that because graphene-django has a registry that may (or may not) prevent a fields from being added twice. Maybe you can just subclass to copy it.

I will see how it goes. I would think it is much cleaner to have top level fields for roles. eg. public / user / staff. This would let you easily see exactly what is public in igraphql. The code should also be easier to read and write.

Of course if you need to do row level permission checks then that is runtime only and the schema won't help you.

[BossGrand]

@crucialfelix actually there was a recent pr adding docs showing that you can have two DjangoObjectTypes with the same model class

[BossGrand]

For mutations it would be nice if there was an permissions array on the mutation class similar to Django rest

This would be an array of functions that looked at context and args and returned true or false

Thoughts?