[WIP] Add support for permissions on fields

[patrick91]

This is partly inspired by Django Rest Framework's permission system.

The idea is to add another parameter to the Field class to specify a list of permissions classes that can be used to allow or not a field to be returned.

class Query(graphene.ObjectType):
    hello = graphene.Field(
        graphene.String(name=graphene.String(default_value="stranger")),
        permission_classes=[IsAdmin]
    )

    def resolve_hello(self, info, name):
        return "Hello " + name

schema = graphene.Schema(query=Query)

We also thought about a different implementation (which is not being implemented in this PR) using metaclasses, and it would look similar to this:

class NestedType(graphene.ObjectType):
    a = graphene.String()

    class Meta:
        permissions_classes = [IsAdmin]

class Query(graphene.ObjectType):
    field_1 = graphene.String()
    field_2 = graphene.List(graphene.String)

    field_3 = graphene.Field(NestedType)

class MyMutation(SerializerMutation):
    class Meta:
        serializer_class = ReporterSerializer
        permissions_classes = [IsAdmin]

class Mutation(graphene.ObjectType):
    my_mutation = MyMutation.Field()

Related issues/PRs:

graphql-python/graphene-django#301
graphql-python/graphene-django#79

Note: even if the related issues are from graphene Django we think this would be a great addition to graphene itself, and then it can be extended in Graphene Django to add support for Django Permissions (like in graphql-python/graphene-django#301)

[pizzapanther]

Looks good but I would probably move the permissions off the field. I think its more concise for the user to generate the fields dynamically and then a user can switch fields based on the permission. See the DRF example which is similar: https://www.django-rest-framework.org/api-guide/serializers/#dynamically-modifying-fields

Note DRF really doesn't do this exactly and Django Forms don't do this either because you have to base fields on the request. But it is a constant hack I always use. So if graphene could support, that would be incredible.

More info for reference: https://stackoverflow.com/questions/19128793/per-field-permission-in-django-rest-framework

Note often times with DRF you just have 2 different serializers based on the user.

[patrick91]

That’s interesting, I’ll think about it, if you have an example on how this would look in graphene post it :)

Changing the field won’t work much in graphene or graphql since the schema should not change at runtime, at least not based on the user, what do you think?

[pizzapanther]

one example that i've seen on other GraphQL implementations is that you have access to different data based on whether you are logged in or not. So I could see that going down to the field level. And ideally you want to not show fields you don't have access too.

Again not really a feature I've seen in other Python frameworks so you find ways around it to produce the same result. So I think it makes sense to bake it in.

[patrick91]

I see, I think GitHub has the concept of public and private fields, private fields are only used in the private api. Which I think it is quite nice, but I still think it is separate from permissions. Imagine if we are building an admin app where there are different permissions level, I’d still want to see the whole schema while working on it, if I change the schema based on the user that would complicate things a bit.

I’m just brainstorming right now, I might be wrong :) I’ll keep thinking on it! I’m really looking forward to see what we come up with:)

[pizzapanther]

[pizzapanther]

i mention dynamic fields because I'm thinking it might be easier to implement, but that's just my best guess.

[erback]

Shouldn't kwargs be passed as well?, I have cases where i need to check permissions based on input params

[pizzapanther]

maybe not it the same place but yes. You often times need the context (request) and all the inputs to validate. When you have inputs that depend on each other you need that. Although you shouldn't need inputs to determine field access, just to validate.

[erback]

agreed!

[patrick91]

Yup, I think we should pass all the arguments to the permission class, I was a bit lazy with the comment 😅

[zbyte64]

I think graphql has an emphasis on fields that DRF does not in that they are also connections. Relying on different serializers/ObjectType's might result in recreating large trees of serialization. Thinking of an example; moderating blog comments we might see::

class Portal(ObjectType):
    blog = Field(Blog)

class Blog(ObjectType):
    comments = List(Comment)

class Comment(ObjectType):
     user = Field(User)
     flags = List(ModeratorFlags)

But for a standard user we would need to reimplement the entirety with a different User reference as not to include sensitive information.

[patrick91]

@zbyte64 I usually have two types, PrivateUser and PublicUser to solve that issue, but with field permissions we might not need that anymore :)

[taoufik07]

Hey, Since we're talking here about permissions, I want to get your feedback about this new package https://github.com/taoufik07/django-graphene-permissions

[firaskafri]

Any updates on this?

[patrick91]

I've stopped working on Graphene until we have a clear maintenance proposal, see #884 :)

[firaskafri]

@patrick91 any timeline has been shared with you? there are great pull requests stuck in the pipline and the community is losing interest.

[patrick91]

@firasalkafri no :(

[stale]

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.

[begimai]

This feature would be nice to have now

[bolerap]

How about this?