strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension
https://strawberry.rocks/docs/django
MIT License
414 stars 119 forks source link

Allow enforcement of permissions for related fields in mutations #618

Open SupImDos opened 2 months ago

SupImDos commented 2 months ago

Description

At a high-level our problem is:

We can't really do this kind of validation in our model-layer, as we need certain request context in order to determine if the user has access to the record they're trying to attach, such as the current user, and which related fields are actually being set. In other frameworks that we've used, we've been able to hook into the layer where primary keys are resolved to do this kind of validation, but this can't currently be done in strawberry-django.

Example

Take for example a simple project management application where users can create projects and assign tasks to those projects. Some models for this application might look like this:

class Project(models.Model):
    name = models.CharField()
    owner = models.ForeignKey(User, on_delete=models.CASCADE)

class Task(models.Model):
    name = models.CharField()
    description = models.TextField()
    project = models.ForeignKey(Project, on_delete=models.CASCADE)

We only want to allow users to create tasks for projects that they own. This means that when setting the project field on a task, the allowed values should only be the subset that they own:

@strawberry_django.type(Task)
class TaskType:
    id: strawberry.auto
    name: strawberry.auto
    description: strawberry.auto

@strawberry_django.input(Task)
class CreateTaskInput:
    name: strawberry.auto
    description: strawberry.auto
    project: strawberry.auto  # The allowed projects here should be filtered

@strawberry.type
class Mutation:
    create_task: Task = strawberry_django.mutations.create(CreateTaskInput)

Feature Request Type

Potential Solutions

At risk of muddying the waters, here are some potential approaches we see which could solve this (although we're open to other suggestions if they're more in line with the project's goals):

a) Allow the queryset for a related field to be customized

In some way or another, allow the queryset that primary keys are resolved from to be customized. This would allow for the enforcement of permissions for related fields, and the flexibility of other filtering as well.

def my_queryset_func(queryset, info, **kwargs):
    user = get_current_user(info)
    return queryset.do_some_filtering_with_the_user(user=user)

@strawberry_django.input(MyModel)
class MyInputType:
    my_related_field: strawberry.auto = strawberry_django.field(queryset=my_queryset_func)

b) Allow a hook for resolving primary keys to be customized

In some way or another, allow the whole resolution of primary keys to be customized. This would also allow for the enforcement of permissions for related fields, and the flexibility of other filtering as well.

def my_resolver_func(pk, info, **kwargs):
    obj = MyRelatedModel.objects.get(pk=pk)
    user = get_current_user(info)
    if not user.has_perm("myapp.view_myrelatedmodel", obj):
        raise PermissionDenied
    return obj

@strawberry_django.input(MyModel)
class MyInputType:
    my_related_field: strawberry.auto = strawberry_django.field(resolver=my_resolver_func)

c) Implement permissions field extensions for input fields

Continue the current pattern of using field extensions to add permissions to input fields. This is consistent with the project's current approach, but doesn't provide the added flexibility of queryset filtering.

@strawberry_django.input(MyModel)
class MyInputType:
    my_related_field: strawberry.auto = strawberry_django.field(extensions=[HasPerm("myapp.view_myrelatedmodel")])

Setting aside doing the actual work, are any of these options inline with your vision for the project and how it should work? Otherwise, do have you any other ideas or suggestions?

Upvote & Fund

Fund with Polar

SupImDos commented 2 months ago

I've discovered that this functionality appears to (maybe incidentally) be possible when you use Relay.

For reference, take the following example:

@strawberry_django.type(MyParentModel)
class MyParentType(relay.Node)
    name: strawberry.auto

    @classmethod
    def get_queryset(cls, queryset, info, kwargs):
        user = get_current_user(info)
        return queryset.filter_for_permissions(user)

@strawberry_django.input(MyChildModel)
class MyChildInput(relay.NodeInput):
    name: strawberry.auto
    parent: strawberry.auto

@strawberry.type
class Mutation:
    create_child: MyChildType = strawberry_django.mutations.create(MyChildInput)

In this scenario, the parent field on the MyChildInput is a NodeInput (i.e., a Global ID) which is ultimately resolved via the MyParentType type using its get_queryset method. If the get_queryset method is set up to filter for permissions based on the current user, then you implicitly get the functionality requested in this ticket.

See:

So, its possible this issue is resolved, but maybe we could do with some documentation to make it clearer how this works?

bellini666 commented 1 month ago

@SupImDos sorry for taking long to reply, but glad you found out how to solve this.

So, its possible this issue is resolved, but maybe we could do with some documentation to make it clearer how this works?

Yes, strawberry-django unfortunately is lacking a lot of documentation. I'm marking this with a documentation label and will try to tackle this in the future. But anyone is very welcomed to open a PR to contribute to this :)