get_node is not called on related objects

[McPo]

Currently get_node is not called on related models. i.e.

class Customer(models.Model):
    display_name = models.CharField()
    ....

class Member(models.Model):
    customer = models.ForeignKey(Customer)
    ....

class Customer(DjangoObjectType):
    class Meta:
        model = models.Customer
        filter_fields = []
        interfaces = [graphene.Node]

    @classmethod
    def get_node(cls, info, id):
        print('NEVER CALLED')
        return None

Then querying with

member(id:"...") {
    customer {
        id
    }
}

Does not result in get_node being called. this is because get_node is only called on Relay Nodes. However relay nodes expect an ID to be passed in via GraphQL. Instead I would like the get_node to be called with the id for the related object.

If this support was added it means you can add per-object level auth.

Furthermore as get_node calls get_queryset, all you would have to do is to filter out all the objects a user isn't allowed to access i.e.

class Customer(DjangoObjectType):

    @classmethod
    def get_queryset(cls, queryset, info):
        return queryset.filter(admins__in=[info.context.user])

For example lets say Im allowed to see a list of Members, some of those Members are not under the Customer that I am an admin of. However I should still be able to interact with them, as they have some other relationship to my Customer. Maybe the Member is related to another Member under my Customer, and I can access them via that path. However I don't want to to give an admin the ability to view other Customers. This would achieve that.

[McPo]

class DjangoModelField(graphene.Field):

    @staticmethod
    def model_resolver(django_object_type, resolver, root, info, **args):
        id_field_name = '%s_id' % to_snake_case(info.field_name)
        id_field_value = getattr(root, id_field_name)
        return django_object_type.get_node(info, id_field_value)

    def get_resolver(self, parent_resolver):
        if issubclass(self.type, DjangoObjectType):
            return partial(self.model_resolver, self.type, parent_resolver)
        return super(DjangoModelField, self).get_resolver(parent_resolver)

The above accomplishes that. Note this will remove any optimisation of select/prefect_related(), as we hit the DB each time. However I don't believe there is any other performance hit if youre not using selected/prefetch_related, as Django still has to access the DB when following the related model. I still need to verify this though

We could possibly accommodate for the related model already having been loaded by checking root._state.fields_cache, however if its preloaded we wouldn't be able to use get_node to check if the person is authorised to view it without hitting the DB again anyway.

The method used, here is similar to customising the Django RelatedManager for the Model.

Ive made this an Issue instead of PR, as Im still not familiar enough with this library to know of all the gotchas, so figured its best that this is something people opt into. Although I'll be looking at a way to make this the default behaviour for myself. Maybe subclass DjangoObjectType or something

[jkimbo]

@McPo I'm not sure I follow your use case. You should be able to do all the permission handling you want by defining the get_queryset method on the ObjectType. Is there a particular case when that won't work?

[McPo]

Yes,

Ive already documented the use case above, although I appreciate it might not be particularly clear so I'll try again.

Lets assume my application has a few models, 3 of which are Admin, Company and Worker. Every Company has multiple Workers, and an Admin. The Admin is able to administrate the Company they have been assigned too. While a Worker belongs to one Company, they can be under other Companies as well (i.e. Secondment).

The Admin of one Company should be able to see all its registered Workers, and any Workers as secondments. However an Admin must not be able to see any Company it is not registered too, including the registered Company of any secondments under their control.

As get_queryset/get_node is only currently applied to Relay fields (Connection and Nodes), and more recently DjangoListFields. It is possible for an Admin to access another company (and all its workers) by following the graph through a secondment, and access a Company they would not normally have access too. This is because get_queryset/get_node is not being called when following related fields in a Django Model. Instead they are just been evaluated.

[3pns]

The purpose of GraphQL if I'm not mistaken is to have 1 Node per model and do the authorization at the node level VS Restfull API where we have to make separate scopes for different type of users such as Admin/StaffUser.

For example our project which is 80-90 tables with Admin/Staff and regular Users. A user have a PrivateTable containing private informations Admin and Staff can access every private table, a User can only access it's private table.

The problem is because we are in a Graph and it is possible to jump from one node to another. Filtering the point of entry where get_queryset may happens is not enough as it may be possible for users to access the private table through reverse relations or foreign keys on other tables where neither get_queryset and get_node get called.

In our project I ended up starting to write a total separate scope with different Nodes for a same ressource because get_queryset and get_node don't get triggered every-time a node get accessed inside the graph.

I noticed that in our case : get_queryset is triggered when accessing related records through a DjangoConnectionField FK , o2o and their reverse wont trigger get_queryset get_node does not trigger on FK, o2o and their reverse

It seems like get_queryset and get_node was designed without thinking about the fact that it is possible for a user to jump through the graph to access unauthorized information ?

[joeydebreuk]

Same issue here. The only way I found makes sure everything is secure, is by implementing middleware and checking all fields there. But this is feels more like a workaround than a solution. Also is very slow because you still get all instances from DB and in a way filter them by looping through them. I uploaded middelware code here: https://gist.github.com/joeydebreuk/dbd0a7c27a2a42f4dff48019f00c571e#file-permission_middleware-py

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