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? https://github.com/graphql-python/graphene/issues/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?

[smmoosavi]

Can we use inheritance for authorization? (related to #26) example: we have ProfileNode and has tow subclass PublicProfileNode and PrivateProfileNode.

when we want resolve ProfileNode if context.user == profile.user or context.user.isSupperAdmin we return PrivateProfileNode else return PublicProfileNode.

class ProfileNode(DjangoObjectType):
    class Meta:
        only_fields = ()
        model = Profile
        interfaces = (relay.Node, )

    def resolve_type(self, context, info): # or some function like this. i don't know :)
        if check_some_perm(self, context.user):
            return PrivateProfileNode
        return PublicProfileNode

class PublicProfileNode(ProfileNode):
    class Meta:
        only_fields = ('name', 'age')

class PrivateProfileNode(ProfileNode):
    class Meta:
        only_fields = ('name', 'age', 'secret_1', 'secret_2')

[dfee]

Late to the party... and I'm not actually using django (I'm use sqlalchemy), but here are my thoughts on how to approach the problem:

• get_node works fine for relay requests against node.
• ConnectionField is probably not the best place to perform ACL validation, as it would require loading / checking every database instance and filtering the list that way
• middleware might be the right way to go, but it can only check after the instance has been selected (but before individual fields are loaded).

I'd be interested in hearing your guys approach to these points.

[dfee]

Attached is a version of ACLMiddleware I'm experimenting with.

Note that my implementation has:

• get_principals returns principals (i.e. ['group:Admin', 'user:1234'])
• permits is a function that returns True or False with arguments (resource typically an instance of the model, principals as above, permission such as 'edit').

Those implementation details aren't Django specific, but I think this problem is more general than the Django, and this is where the conversation is happening.

As for constraints from previous comments: 1) this doesn't support permissions per field (though Meta could pretty easily be extended to support that), 2) when querying node, we don't (can't?) get access to the ObjectType so we have to re-implement the get_node method on ObjectType classes.

class ACLMiddleware:
    def resolve(self, next, root, args, context, info):
        result = next(root, args, context, info)
        graphene_type = getattr(info.return_type, 'graphene_type', None)
        if not isinstance(graphene_type, SQLAlchemyObjectTypeMeta):
            return result

        permission = getattr(graphene_type._meta, 'permission', None)
        if not permission:
            return result

        authenticated_user = context.get('authenticated_user', None)
        principals = _User.get_principals(
            context['session'],
            authenticated_user,
        )
        return result if permits(result.value, principals, permission) else None

[audiolion]

I would agree that with the way Graphene is currently structured, the middleware is probably the best place to do permissions checking.

I am trying to understand your code @dfee , does SQLAlchemyObjectTypeMeta allow you to put permissions on specific nodes and check against them?

[dfee]

No, but with an PR I made a while back, you can subclass SQLAlchemyObjectTypeMeta. https://github.com/graphql-python/graphene-sqlalchemy/pull/51

[mariuszstr]

I need permission system for graphene-django, similar as in django-rest-framework. In django-rest-framework in api view class we can add: permission_classes = (SomePermissionClass, ) I think in grapheene-django permission_classes woud be allowed for: -Node for Query, for example:

class SomeObjectNode(DjangoObjectType):
    class Meta:
        model = SomeModelClass
        interfaces = (graphene.Node, )
        permission_classes = (SomePermissionClass, )

class OurQuery(graphene.AbstractType):

    some_object = graphene.Node.Field(SomeObjectNode)
    all_some_object = DjangoFilterConnectionField(SomeObjectNode)

-Fields for mutations, for example:

class DeleteSomeObject(graphene.ClientIDMutation):
    permission_classes = (SomePermissionClass, ) 
    class Input:
        id = graphene.String()

    @classmethod
    def mutate_and_get_payload(cls, input, context, info):
        SomeObject.objects.get(pk=from_global_id(input.get('id'))[1]).delete()
        return DeleteSomeObject()

class OurMutation(graphene.AbstractType):
    delete_some_object = DeleteSomeObject.Field()

Anyone is implementing this functionality now? If nobody, i can try to do it. Anyone has any advices, comments, etc for me?

[crucialfelix]

Here's a decorator for adding auth to a mutation: https://gist.github.com/crucialfelix/cb106a008a7a62bdab4a68e1b4ab7a3c

It is even easier than your example:

    @classmethod
    @is_staff
    def mutate_and_get_payload(cls, input, context, info):
        # etc.

You can do something similar with queries and individual def resolve_things with as complex auth as you need to do (row permissions, group membership) etc.

[mariuszstr]

Thanks, It could be very usefull for me :)

[matteing]

Anyone working on this?

[crucialfelix]

There is a 2.0 version in development: https://github.com/graphql-python/graphene-django/tree/2.0

I have not yet looked at it. There is a new resolver API

[strogonoff]

@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.

By chance, did you try the top-level viewer field approach to permission handling in the end? Seems conventional, do you think it works out with graphene-django?

[crucialfelix]

That is what I do. I even have a level below that.

query Listings {
  viewer {
    id
    agent {
      id
      listings(
        first: 50
        after: $after
        visibility: $visibility
        category: $category
        categoryId: $categoryId
        search: $search
        section: $section
        sr: $sr
        aptids: $aptids
      ) {
        edges {
          node {
            id
            pk
            // etc
          }
        }
      }
    }
  }
}

Agent (which is a logged in staff member) is checked only once and I know that everything inside of that are fields on AgentRole. So it's more secure with no risk of forgetting a decorator or some check in the resolve function.

A client (a logged in user) or anon has different fields with different security and visibility requirements.

I'm a bit divided on whether it was a great way to lay it out. There is a bit more overhead on the client with the extra nesting, but I guess it's always clear what is being accessed.

[strogonoff]

@crucialfelix Seems like a sane way to do it, I might actually adopt the extra nested call too since I’m dealing with staff and non-staff roles. Currently figuring out how to implement this approach with Graphene+Django being a bit of a noob, authentication docs aren’t going into much detail on how to add root calls populated based on context like current user. From reading Facebook’s early docs it seemed like the most graphql-y way to go about it though.

The decorator you posted above might be my backup plan so thanks for that (I guess I can use it for more than mutations, so I’ll just keep two endpoints and for one of them hide key methods behind @is_staff).

[rmoorman]

@crucialfelix thank you for sharing your approach! Do you also have a root level node field? How do you handle permissions there (in case you do have one)? Do you just rely on the different types you introduce for different parts of the tree and the respective get_node to implement all checks needed?

[crucialfelix]

Currently my root node only has viewer on it

All model queries are on viewer.

Viewer works well as a parent for all public queries, both anonymous and non staff users.

They could be on the root since context aka Django request is available to the resolvers.

Of course all mutations are at the top level and I have auth decorators on those.

On Wed, Mar 14, 2018, 09:06 Rico Moorman notifications@github.com wrote:

@crucialfelix https://github.com/crucialfelix thank you for sharing your approach! Do you also have a root level node field https://facebook.github.io/relay/graphql/objectidentification.htm? How do you handle permissions there (in case you do have one)? Do you just rely on the different types you introduce for different parts of the tree and the respective get_node to implement all checks needed?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/graphql-python/graphene-django/issues/79#issuecomment-372936342, or mute the thread https://github.com/notifications/unsubscribe-auth/AANWcuio4gHlHjugoEPOlh9Z0ZX7lq1jks5teM-WgaJpZM4LVFvv .

[zbyte64]

My two cents:

DjangoObjectType should define a default get_queryset regardless of whether we adopt a more opinionated permission system. It's a common Django pattern and provides a useful hook for more specialized use cases.

[patrick91]

@pizzapanther would mind having a look at my proposal here: https://github.com/graphql-python/graphene/pull/846 ?

It is a more generic solution that works with any kind of fields (either scalars and object types) and also works with vanilla graphene, and it would probably be easy to integrate this with graphene Django

[daveoconnor]

I needed this functionality and it looked like it might take a while before it would be implemented into graphene itself, so I've created some middleware and a decorator to handle it for my use case.

If anyone else is in immediate need it's available at: https://github.com/daveoconnor/graphene-field-permission

[mvanlonden]

The GraphQL organization recommends delegating authorization to the business layer. You could use django-guardian or django-rules to create object-level permissions. Alternatively, you could protect the endpoint at the view layer.

[Cito]

Another interesting approach is taken by PostGraphile, where the security is pushed down even further, to the database level. Maybe this could be provided as an alternative option.

[pizzapanther]

Row level security is nice in postgres if you can do it but it is not always featureful enough. Had that problem in a recent project so we had to push it up a level from the DB.

[Nabellaleen]

Reading a lot various discussions about the subjects, I'm more and more convinced that this way of doing things (declaring in Graphene schema the permissions) is the "old way". The "REST" way ;)

As mentioned by @mvanlonden, GraphQL organization recommends to delegate this authorization to the business layer. This method is also similar to what should be implemented, for example, when you let a user on a mobile device, select the authorization it accords to an app : to the "low level API consumer", it should be transparent.

To be explicit, I think it's a better way to have :

Database ----> ORM & Business logic ----> Graphene Schema ----> GraphQL consumer
                               \--> Here, you implement auth
                          and let only the real values cascade to
                          the Graphene layer -- for non-authorized
                          fields, you replace value by "empty" one

In practice, if you have this request :

query getUserFamilyInfo {
    user {
        family {
            siblings {
                firstname
            }
            father {
                firstname
            }
        }
    }
}

An authorized user will get :

{ 
    user: {
        family: {
            siblings: [
                { firstname: 'phillip' },
                { firstname: 'mary' }
            ],
            father: { firstname: 'william' }
        }
    }
}

And an unauthorized user will get :

{ 
    user: {
        family: {
            siblings: [ ],
            father: null
        }
    }
}

In practice, both approach are interesting, depending on the case. So I think we should implement, in graphene, an easy way to implement various authorization mechanism, to not be too much opiniated

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[jakubste]

I think that's still an open issue. I can't find a proper way to implement access restrictions to some fields for different users.

[jkimbo]

@jakubste I'm not convinced that trying to build authentication into graphene-django is a good idea. The recommended way of dealing with authentication is to build it into your resolvers (ref: https://graphql.org/learn/authorization/) so I would recommend following that approach.

[jakubste]

First of all: we're talking about authorization, not authentication.

But on the topic: I'm currently at place where I have to implement API that is accessible by admins and normal users. Both have access to the same models, but we don't want to show users every field. As an example imagine having user object, which obviously user should have access to, but we don't want to show him, for example, notes about him made by admins. With one field placing it in resolver is ok, but with many models with many fields, it's a lot of copy-paste code, just to check if_staff or whatever permission one would have to check.

I think that DRF deals with that problem by using different serializers for different users. Unfortunately, I can't see an easy way to copy such behaviour to graphene.

I really like the idea presented by @bk-equityzen in #485 and we are probably going to reimplement his proposal in some way. I noticed that it's a common problem and mitigating it by simply saying "build it into your resolvers" doesn't seem like a proper solution to a common problem.

[cglacet]

@jkimbo I think that's quite the opposite actually, unless I'm misunderstanding this:

It is tempting to place authorization logic in the GraphQL layer like so:

var postType = new GraphQLObjectType({
  name: ‘Post’,
  fields: {
    body: {
      type: GraphQLString,
      resolve: (post, args, context, { rootValue }) => {
        // return the post body only if the user is the post's author
        if (context.user && (context.user.id === post.authorId)) {
          return post.body;
        }
        return null;
      }
    }
  }
});

Then right after:

Defining authorization logic inside the resolver is fine when learning GraphQL or prototyping. However, for a production codebase, delegate authorization logic to the business logic layer. Here’s an example:

//Authorization logic lives inside postRepository
var postRepository = require('postRepository');

var postType = new GraphQLObjectType({
  name: ‘Post’,
  fields: {
    body: {
      type: GraphQLString,
      resolve: (post, args, context, { rootValue }) => {
        return postRepository.getBody(context.user, post);
      }
    }
  }
});

I found this article that may help on this question: Authentication and Authorization in GraphQL

[jakubste]

I know that GraphQL authors approach is just "give it to the business logic layer". I just can't find any neat solution that works with Django.

If we are really going this way, that means that there should be something in between DjangoObjectType and Model that knows about restrictions. Then again graphene-django does not provide any opportunity to inject this "business layer" other than writing custom resolver for each and every field in your schema.

[cglacet]

I don't know django, but I'm pretty sure you would have a way to do it. For Starlette I posted a question about that kind of problem (not having the opportunity to patch how requests are handled).

Basically you would intercept calls schema.execute to add your logic here, in starlette this is possible via GraphQLApp.execute (which internally call schema.execute).

You could then simply add a decorator to that method so authorization checks are performed before execute. Again, I'm not sure that's the best solution, but at least you can plug/unplug security checks on a single place.

edit looking at how django handles this it seems like calls to schema.execute have to be performed manually, which makes this even easier as you could patch schema.execute directly.

[jkimbo]

@jakubste sorry you're right we are talking about authorization not authentication. That was a typo.

To restrict access at an object level using the get_queryset method that @zbyte64 implemented is the best way to do that: https://github.com/graphql-python/graphene-django/pull/528/files

Restricting at a field level is harder and currently the only way to do it is by having customer resolvers. I am open to PRs that implement a field level pre-processing API. I just don't know what that looks like or how to implement it efficiently.

---

@jakubste Regarding your uses case of having a single API handle both users and admins: I would suggest splitting the API into 2 separate ones because it makes everything a lot simpler (this is what I did at a previous company). You can share types between them if you like but having separate schemas means you can have fields that only make sense to admins (like the admin notes field) and you can drastically simplify your authorization logic. The user facing API just needs to handle the case where the current user can only view their data and the staff API can bail our early if the current user isn't a staff user.

[sbernier1]

Is it possible to restrict access to specific ObjectTypes? Like if I had multiple object types that access RestrictedType and I wanted that only users with a specific permission could access RestrictedType. Right now I have to create a custom resolver on each object types, but I think it would be nice to add the permission directly on the RestrictedType so that no one can access it unless they have the needed permission, from wherever I try to access it.

If it's not possible, the next best thing in my opinion would be to have the possibility to use django permissions on the fields without having to use a custom resolver. Maybe something like this:

class MyType(DjangoObjectType):
  class Meta:
    model = MyModel
    field_permissions = {
      "field_a": "is_authenticated"
      "field_b": "myapp.view_b"
      ...
    }

[rmoorman]

@sbernier1 maybe you could try to define a field class for this that contains a custom resolver function in order to centralize the permission checking

class MyQueryField(graphene.Field):
    def __init__(self, *args, **kwargs):
        kwargs["resolver"] = kwargs.get("resolver", self.__class__.resolver)
        return super().__init__(MyType, *args, **kwargs)

    @staticmethod
    @some_permission_decorator
    def resolver(parent, info, **kwargs):
        # ... or maybe some permission checking code here
        return MyModel.objects.get(parent_pk=parent.pk)

and then inside the object maybe something like

    my_field = MyQueryField()

I hope this makes sense, works and/or is remotely what you are looking for :smile:

[sbernier1]

I ended up overriding the default resolver like this:

class MyType(DjangoObjectType):
  class Meta:
    ...
    @permission_decorators
    def default_resolver(attname, default_value, root, info, **args):
      return dict_or_attr_resolver(attname, default_value, root, info, **args)

It still uses the default dict_or_attr_resolver (unless specified otherwise on a field), it's pretty nice I think :) But thanks!

[jkimbo]

@sbernier1 I think overriding the default resolver is the way to go and I like your example. I think creating something like an AuthDjangoObjectType could work. I'm thinking an API like this:

def staff_required(user, info):
    if not info.context.user.is_staff:
        return False
    return True

class MyType(AuthDjangoObjectType):
    class Meta:
        model = User
        fields = ("first_name", "last_name", "email")

    class Auth:
        fields = {
            "email": [staff_required],
        }

What do you think?

Also I'm going to reopen this issue because we should at least have an official answer to this question.

[sbernier1]

looks great! If it was possible to access the querySet in the auth function (staff_required in this case), I think it would cover the use cases I can think of. I want to access the querySet to check if the user is allowed to access specific rows. It would also be nice if we could declare a default permission function in the auth class.

[jkimbo]

@sbernier1 why would you need to access the queryset? Conceptually the ObjectType doesn't have access to how it was resolved so I don't think it would be possible anyway. You could do it on the parent type though?

+1 on being able to declare a default permission function.

[sbernier1]

maybe I used the wrong expression, sorry. In the resolvers, be it the default resolver or specific resolvers, we have access to the data. I want this because, for example, if I have a db containing books and book drafts, and I want only the author of the book to access the draft. With resolvers I can access the data to check that info.context.user.id == book.author.id .

[jkimbo]

Oh yeah thats the first parameter to the permission function. The user parameter in staff_required is the current user:

def staff_required(user, info):
    if not info.context.user.is_staff:
        return False
    return True

[smmoosavi]

I suggest everyone in this thread see this video after 38:30 until 41:15

Authentication

who you are?

It should be done in the transport layer (HTTP). and Django context has user:

def resolve_items(parent, info):
  info.context.user # it can be anonymous or authenticated user

Authorization

do you have permission?

it should be done in business logic.

example:

def resolve_items(parent, info):
  return get_all_items(info.context.user)

what is get_all_items function? it's an Interactor function. Interactor function is pure logic, it's not know anything about graphql, rest, GRPC, shell command, or ... any other interface. interactor function only knows logics, including authorization logics. (interactor name is stolen from this video by Uncle Bob Martin.)

In my opinion, In Django, interactors are allowed to know about models and query sets.

interactors are defined in a separate file. graphql, Django view/templates, rest, ... can use them as core logic.

def get_all_items(user):
  if not user.is_authenticated:
    return Item.objects.none()
  return item.objects.filter(owner=user)

graphql layer mission is about translating graphql request data into interactor call

interactor:

• simple about converting data (shouldn't have any of this steps: HTTP request => text => json => model)
• encapsulate logic complexity
• testable

graphql:

• data conversion, type validation is already done by graphene
• resolvers and mutations function only should pass/convert their args to the interactor args
• it shouldn't contain business logic

[sbernier1]

If I do not have legacy stuff and only access data through graphql, is it really that bad to put authorization in the resolvers? if I have to put it in the business layer, the only thing it changes for me is that I have to another layer of functions. Thanks