Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to avoid duplicating permission logic #28

Open
jordanmkoncz opened this issue Sep 1, 2016 · 2 comments
Open

How to avoid duplicating permission logic #28

jordanmkoncz opened this issue Sep 1, 2016 · 2 comments

Comments

@jordanmkoncz
Copy link

For a particular model, Application, I have an ApplicationViewSet that includes a list view and detail view. I have logic that determines whether or not the current user has permission to read a particular Application record, and I need to apply this logic so that the current user can only access the detail view if they have permission to view that particular record, and so that the list view only returns records for which the current user has permission to access.

I'm trying to figure out how I can apply the permission logic to both the list view and detail view without duplicating it.

Here's what I'm currently doing:

views.py

class ApplicationViewSet(viewsets.ModelViewSet):
    queryset = Application.objects.all()
    serializer_class = ApplicationSerializer
    permission_classes = (DRYPermissions,)
    filter_backends = (ApplicationFilterBackend,)

models.py

class Application(models.Model):
    department = models.ForeignKey(Department, null=True, on_delete=models.SET_NULL, related_name='applications')

    @staticmethod
    @authenticated_users
    @allow_staff_or_superuser
    def has_read_permission(request):
        return True

    @authenticated_users
    @allow_staff_or_superuser
    def has_object_read_permission(self, request):
        if request.user.is_manager:
            return self.department in request.user.manager.departments.all()

        return self.applicant.user == request.user

filters.py

class ApplicationFilterBackend(DRYPermissionFiltersBase):
    def filter_list_queryset(self, request, queryset, view):
        if request.user.is_manager:
            return queryset.filter(Q(department__in=request.user.manager.departments.all()))

        return queryset.filter(Q(applicant__user=request.user))

This works, but as you can see, my permission logic is duplicated in Application.has_object_read_permission and ApplicationFilterBackend.filter_list_queryset. Is there any way to avoid this?

@dbkaplan
Copy link
Owner

dbkaplan commented Sep 1, 2016

Hey Jordan,

Currently, there isn't a way to avoid this. I thought about this a lot when I was designing this permissions package. List permissions are fundamentally different from and of the other crud operations because list permissions are filters. Unfortunately, I wasn't able to come up with a simple way to cover both with the same code.

@jordanmkoncz
Copy link
Author

@dbkaplan Fair enough, yeah the effect of the permission logic is definitely fundamentally different for list permissions (applying filters to a query rather than returning a boolean response), it's just a shame that the exact same permission logic has to be manually duplicated and kept consistent in two different places in order to apply that logic to both the list permissions and object permissions. Unfortunately I can't think of a good way to get around it either though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants