Inconsistency in the way ForeignKeys resolutions are treated

[tcleonard]

In the converters the way the reverse relation of a foreign key is dealt with differently from the forward relationship.

Indeed you can see in the ManyToOneRel conversion convert_field_to_list_or_connection() we convert the field to either a DjangoConnectionField or a DjangoListField (depending on if it uses relay or not). Both of those make sure that the resolution of the field is going through those custom classes resolvers. This is important as those implement some hooks to resolve the queryset in a custom way (or to give a custom queryset manager).

On the other hand, the ForeignKey conversion convert_field_to_djangomodel() implements a simple graphene.Field hence bypassing any potential custom queryset filtering.

This is notably a problem as those resolver hooks are typically used to implement authorization... and this behavior means that we enforce it only in one direction, resulting in permissions leaks.

I have a quick fix for this problem but I think it degrades performances by doing an additional query to the database:

@convert_django_field.register(models.OneToOneField)
@convert_django_field.register(models.ForeignKey)
def convert_field_to_djangomodel(field, registry=None):
    model = field.related_model

    def dynamic_type():
        _type = registry.get_type_for_model(model)
        if not _type:
            return

        class CustomField(Field):
            def get_resolver(self, parent_resolver):
                """
                Implements a custom resolver which goes through the `get_node` method to insure that
                it goes through the `get_queryset` method of the DjangoObjectType.
                """
                resolver = super().get_resolver(parent_resolver)
                return lambda root, info, **args: _type.get_node(info, resolver(root, info, **args).pk)

        return CustomField(_type, description=field.help_text, required=not field.null)

    return Dynamic(dynamic_type)

I am going to submit as a PR with some unit tests but I would love to get some feedback as it seems pretty hacky...

[tcleonard]

Submitted 2 PRs:

• one for v2: #1113
• one for main: #1112 (note that there was a broken test on main that I have fixed in this PR as well)

[Rainshaw]

hello, I am facing the same issue, how is this work going?

[tcleonard]

This is blocking on me, I need to review the alternative PR to see if it's a better solution to the problem. Haven't had time to take a look just yet. For the record I have been using my PR for quite some time and other than the performance overhead it does the job.