improved permission handlin

cookbook/helper/permission_helper.py

@@ -4,7 +4,7 @@ Source: https://djangosnippets.org/snippets/1703/
 from django.contrib import messages
 from django.contrib.auth.decorators import user_passes_test
 from django.core.exceptions import ValidationError
-from django.db.models import Q
+
 from django.utils.translation import gettext as _
 from django.http import HttpResponseRedirect
 from django.urls import reverse_lazy, reverse
@@ -13,7 +13,15 @@ from rest_framework import permissions
 from cookbook.models import ShareLink
 
 
+# Helper Functions
+
 def get_allowed_groups(groups_required):
+    """
+    Builds a list of all groups equal or higher to the provided groups
+    This means checking for guest will also allow admins to access
+    :param groups_required: list or tuple of groups
+    :return: tuple of groups
+    """
     groups_allowed = tuple(groups_required)
     if 'guest' in groups_required:
         groups_allowed = groups_allowed + ('user', 'admin')
@@ -22,16 +30,71 @@ def get_allowed_groups(groups_required):
     return groups_allowed
 
 
-def group_required(*groups_required):
+def has_group_permission(user, groups):
-    """Requires user membership in at least one of the groups passed in."""
+    """
-
+    Tests if a given user is member of a certain group (or any higher group)
-    def in_groups(u):
+    Superusers always bypass permission checks. Unauthenticated users cant be member of any
-        groups_allowed = get_allowed_groups(groups_required)
+    group thus always return false.
-        if u.is_authenticated:
+    :param user: django auth user object
-            if u.is_superuser | bool(u.groups.filter(name__in=groups_allowed)):
+    :param groups: list or tuple of groups the user should be checked for
+    :return: True if user is in allowed groups, false otherwise
+    """
+    if not user.is_authenticated:
+        return False
+    groups_allowed = get_allowed_groups(groups)
+    if user.is_authenticated:
+        if user.is_superuser | bool(user.groups.filter(name__in=groups_allowed)):
             return True
     return False
 
+
+def is_object_owner(user, obj):
+    """
+    Tests if a given user is the owner of a given object
+    test performed by checking user against the objects user and create_by field (if exists)
+    superusers bypass all checks, unauthenticated users cannot own anything
+    :param user django auth user object
+    :param obj any object that should be tested
+    :return: true if user is owner of object, false otherwise
+    """
+    # TODO this could be improved/cleaned up by adding get_owner methods to all models that allow owner checks
+    if not user.is_authenticated:
+        return False
+    if user.is_superuser:
+        return True
+    if owner := getattr(obj, 'created_by', None):
+        return owner == user
+    if owner := getattr(obj, 'user', None):
+        return owner == user
+    return False
+
+
+def share_link_valid(recipe, share):
+    """
+    Verifies the validity of a share uuid
+    :param recipe: recipe object
+    :param share: share uuid
+    :return: true if a share link with the given recipe and uuid exists, false otherwise
+    """
+    try:
+        return True if ShareLink.objects.filter(recipe=recipe, uuid=share).exists() else False
+    except ValidationError:
+        return False
+
+
+# Django Views
+
+def group_required(*groups_required):
+    """
+    Decorator that tests the requesting user to be member of at least one of the provided groups
+    or higher level groups
+    :param groups_required: list of required groups
+    :return: true if member of group, false otherwise
+    """
+
+    def in_groups(u):
+        return has_group_permission(u, groups_required)
+
     return user_passes_test(in_groups, login_url='index')
 
 
@@ -43,18 +106,10 @@ class GroupRequiredMixin(object):
     groups_required = None
 
     def dispatch(self, request, *args, **kwargs):
-        if not request.user.is_authenticated:
+        if not has_group_permission(request.user, self.groups_required):
-            messages.add_message(request, messages.ERROR, _('You are not logged in and therefore cannot view this page!'))
-            return HttpResponseRedirect(reverse_lazy('login'))
-        else:
-            if not request.user.is_superuser:
-                group_allowed = get_allowed_groups(self.groups_required)
-                user_groups = []
-                for group in request.user.groups.values_list('name', flat=True):
-                    user_groups.append(group)
-                if len(set(user_groups).intersection(group_allowed)) <= 0:
             messages.add_message(request, messages.ERROR, _('You do not have the required permissions to view this page!'))
             return HttpResponseRedirect(reverse_lazy('index'))
+
         return super(GroupRequiredMixin, self).dispatch(request, *args, **kwargs)
 
 
@@ -65,38 +120,55 @@ class OwnerRequiredMixin(object):
             messages.add_message(request, messages.ERROR, _('You are not logged in and therefore cannot view this page!'))
             return HttpResponseRedirect(reverse_lazy('login'))
         else:
-            obj = self.get_object()
+            if not is_object_owner(request.user, self.get_object()):
-            if not (obj.created_by == request.user or request.user.is_superuser):
                 messages.add_message(request, messages.ERROR, _('You cannot interact with this object as its not owned by you!'))
                 return HttpResponseRedirect(reverse('index'))
 
         return super(OwnerRequiredMixin, self).dispatch(request, *args, **kwargs)
 
 
-def share_link_valid(recipe, share):
+# Django Rest Framework Permission classes
-    """
-    Verifies if a share uuid is valid for a given recipe
-    """
-    try:
-        return True if ShareLink.objects.filter(recipe=recipe, uuid=share).exists() else False
-    except ValidationError:
-        return False
 
-
+class CustomIsOwner(permissions.BasePermission):
-class DRFOwnerPermissions(permissions.BasePermission):
     """
     Custom permission class for django rest framework views
     verifies user has ownership over object
     (either user or created_by or user is request user)
     """
+    message = _('You cannot interact with this object as its not owned by you!')
 
     def has_object_permission(self, request, view, obj):
-        if not request.user.is_authenticated:
+        return is_object_owner(request.user, obj)
-            return False
+
-        #if request.user.is_superuser:
+
-        #    return True
+class CustomIsGuest(permissions.BasePermission):
-        if owner := getattr(obj, 'created_by', None):
+    """
-            return owner == request.user
+    Custom permission class for django rest framework views
-        if owner := getattr(obj, 'user', None):
+    verifies the user is member of at least the group: guest
-            return owner == request.user
+    """
-        return False
+    message = _('You do not have the required permissions to view this page!')
+
+    def has_permission(self, request, view):
+        has_group_permission(request.user, ['guest'])
+
+
+class CustomIsUser(permissions.BasePermission):
+    """
+    Custom permission class for django rest framework views
+    verifies the user is member of at least the group: user
+    """
+    message = _('You do not have the required permissions to view this page!')
+
+    def has_permission(self, request, view):
+        has_group_permission(request.user, ['guest'])
+
+
+class CustomIsAdmin(permissions.BasePermission):
+    """
+    Custom permission class for django rest framework views
+    verifies the user is member of at least the group: admin
+    """
+    message = _('You do not have the required permissions to view this page!')
+
+    def has_permission(self, request, view):
+        has_group_permission(request.user, ['guest'])

cookbook/views/api.py

@@ -15,7 +15,7 @@ from rest_framework import viewsets, permissions
 from rest_framework.exceptions import APIException
 from rest_framework.mixins import RetrieveModelMixin, UpdateModelMixin, ListModelMixin
 
-from cookbook.helper.permission_helper import group_required, DRFOwnerPermissions
+from cookbook.helper.permission_helper import group_required, CustomIsOwner, CustomIsAdmin
 from cookbook.models import Recipe, Sync, Storage, CookLog, MealPlan, MealType, ViewLog, UserPreference, RecipeBook
 from cookbook.provider.dropbox import Dropbox
 from cookbook.provider.nextcloud import Nextcloud
@@ -47,12 +47,9 @@ class UserNameViewSet(viewsets.ModelViewSet):
 
 
 class UserPreferenceViewSet(viewsets.ModelViewSet):
-    """
-    Update user preference settings
-    """
     queryset = UserPreference.objects.all()
     serializer_class = UserPreferenceSerializer
-    permission_classes = [DRFOwnerPermissions, ]
+    permission_classes = [CustomIsOwner, ]
 
     def perform_create(self, serializer):
         if UserPreference.objects.filter(user=self.request.user).exists():
@@ -60,23 +57,20 @@ class UserPreferenceViewSet(viewsets.ModelViewSet):
         serializer.save(user=self.request.user)
 
     def get_queryset(self):
-        # if self.request.user.is_superuser:
+        if self.request.user.is_superuser:
-        #   return UserPreference.objects.all()
+            return self.queryset
-        return UserPreference.objects.filter(user=self.request.user).all()
+        return self.queryset.filter(user=self.request.user)
 
 
 class RecipeBookViewSet(RetrieveModelMixin, UpdateModelMixin, ListModelMixin, viewsets.GenericViewSet):
-    """
-    Update user preference settings
-    """
     queryset = RecipeBook.objects.all()
     serializer_class = RecipeBookSerializer
-    permission_classes = [DRFOwnerPermissions, ]
+    permission_classes = [CustomIsOwner, CustomIsAdmin]
 
     def get_queryset(self):
         if self.request.user.is_superuser:
-            return RecipeBook.objects.all()
+            return self.queryset
-        return RecipeBook.objects.filter(created_by=self.request.user).all()
+        return self.queryset.filter(created_by=self.request.user)
 
 
 class MealPlanViewSet(viewsets.ModelViewSet):