graphql-python / graphql-python/graphene-django
Inconsistency in the way ForeignKeys resolutions are treated
- Dominant language
- Python
- Stars
- 4.4k
- Forks
- 760
- PR merge metrics
- No merged PRs in 30d
Description
In the [converters](https://github.com/graphql-python/graphene-django/blob/main/graphene_django/converter.py) 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()`](https://github.com/graphql-python/graphene-django/blob/main/graphene_django/converter.py#L253) 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()`](https://github.com/graphql-python/graphene-django/blob/main/graphene_django/converter.py#L292) 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:
```python
@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...
Contributor guide
Assessment
This issue has not been assessed yet.