refactor permissions

Author: karlosss

simple_api/django_object/actions.py

@@ -3,6 +3,8 @@
 from simple_api.django_object.utils import determine_items, remove_item
 from simple_api.object.actions import Action, ToActionMixin, SetReferencesMixin
 from simple_api.object.datatypes import ObjectType, BooleanType
+from .permissions import permission_class_with_get_fn
+from ..utils import ensure_tuple
 
 
 class WithObjectMixin:
@@ -23,7 +25,7 @@ def __init__(self, parameters=None, data=None, return_value=None, exec_fn=None,
         self.data = data or {}
         self.return_value = return_value or ObjectType("self")
         self.exec_fn = exec_fn
-        self.permissions = permissions
+        self.permissions = ensure_tuple(permissions)
         self.kwargs = kwargs
 
         # these attributes exist to ensure that 1) the input from the user is stored unmodified in the attributes above
@@ -111,17 +113,15 @@ def fn(request, params, obj, **kwargs):
     # injects get_fn into the permission classes so that this information can be used to determine permissions
     def determine_permissions(self):
         if self._determined_permissions is None:
-            self._determined_permissions = self.permissions
-            if self._determined_permissions is None:
-                return
-            if not isinstance(self._determined_permissions, (list, tuple)):
-                self._determined_permissions = self._determined_permissions,
-
+            self._determined_permissions = []
             self.determine_get_fn()
-            instantiated_permissions = []
-            for pc in self._determined_permissions:
-                instantiated_permissions.append(pc(get_fn=self._determined_get_fn))
-            self._determined_permissions = instantiated_permissions
+            for pc in self.permissions:
+                # here we extract the information from a permission class and use it to build a new class with the
+                # function to get the related object embedded; this is because permissions are supposed to be classes
+                # and not their instances and since django actions are implemented as composition with respect to
+                # actions, there would be a problem with actions seeing instantiated permission classes, which would
+                # create problems later
+                self._determined_permissions.append(permission_class_with_get_fn(pc, self._determined_get_fn))
 
     def determine_parameters(self):
         if self._determined_parameters is None:

simple_api/django_object/permissions.py

@@ -1,17 +1,23 @@
 from simple_api.object.permissions import BasePermission
 
 
-class DjangoPermission(BasePermission):
-    def __init__(self, get_fn=None, **kwargs):
-        self.get_fn = get_fn
-        super().__init__(**kwargs)
+def permission_class_with_get_fn(permission_class, get_fn):
+    # takes a permission class and embeds the get_fn of the object into it; this is so that it can be passed around
+    # as a class and there is no need to instantiate it (otherwise we would run into trying to instantiate something
+    # that has already been instantiated)
+    class _DjangoPermission:
+        def has_permission(self, **kwargs):
+            obj = kwargs.pop("obj")
+            if obj is None:
+                obj = get_fn(**kwargs)
+            return permission_class().has_permission(obj=obj, **kwargs)
+
+        def error_message(self, **kwargs):
+            return permission_class().error_message(**kwargs)
+    return _DjangoPermission
 
-    def has_permission(self, exclude_classes=(), **kwargs):
-        obj = kwargs.pop("obj")
-        if obj is None and self.get_fn is not None:
-            obj = self.get_fn(**kwargs)
-        return super().has_permission(exclude_classes=(DjangoPermission,) + exclude_classes, obj=obj, **kwargs)
 
+class DjangoPermission(BasePermission):
     def permission_statement(self, request, obj, **kwargs):
         raise NotImplementedError
 

simple_api/object/permissions.py

@@ -5,17 +5,10 @@
 # instantiates permission classes (if they are not already, maybe due to get_fn injection) and builds a
 # function that raises if the permissions are not passed
 def build_permissions_fn(permissions):
-    instantiated_permissions = []
-    for cls_or_inst in permissions:
-        if isclass(cls_or_inst) or isinstance(cls_or_inst, LogicalConnector):
-            instantiated_permissions.append(cls_or_inst())
-        else:
-            instantiated_permissions.append(cls_or_inst)
-
     def fn(**kwargs):
-        for perm in instantiated_permissions:
-            if not perm.has_permission(**kwargs):
-                raise PermissionError(perm.error_message(**kwargs))
+        for perm in permissions:
+            if not perm().has_permission(**kwargs):
+                raise PermissionError(perm().error_message(**kwargs))
     return fn
 
 
@@ -26,12 +19,21 @@ def __init__(self, **kwargs):
     def permission_statement(self, **kwargs):
         raise NotImplementedError
 
-    def has_permission(self, exclude_classes=(), **kwargs):
+    def has_permission(self, **kwargs):
+        # to achieve hierarchical checking for permissions (a subclass calls the permission statement of the superclass
+        # and only if it passes, executes its own), we need to traverse over the whole linearization order of the
+        # permission class; however, classes like object, which are naturally also a part of the chain, do not
+        # contain the `permission_statement` method and therefore should be just skipped; the same is true for abstract
+        # permission classes which contain the method, but is not implemented - like this one for example: to achieve
+        # this, we try calling the method and if it turns out to not be implemented, we skip it as well
         for cls in reversed(self.__class__.__mro__):
-            if cls in (object, BasePermission, LogicalResolver) + exclude_classes:
+            if not hasattr(cls, "permission_statement"):
+                continue
+            try:
+                if not cls.permission_statement(self, **kwargs):
+                    return False
+            except NotImplementedError:
                 continue
-            if not cls.permission_statement(self, **kwargs):
-                return False
         return True
 
     def error_message(self, **kwargs):
@@ -43,12 +45,10 @@ def __init__(self, *permissions):
         self.permissions = permissions
 
     def __call__(self, **kwargs):
-        instantiated_perms = []
         for perm in self.permissions:
             assert isclass(perm) or isinstance(perm, LogicalConnector), \
                 "Permissions in logical connectors must be classes."
-            instantiated_perms.append(perm(**kwargs))
-        return LogicalResolver(instantiated_perms, self.resolve_fn)
+        return LogicalResolver(self.permissions, self.resolve_fn)
 
     def resolve_fn(self, permissions, **kwargs):
         raise NotImplementedError
@@ -69,23 +69,23 @@ def error_message(self, **kwargs):
 class Or(LogicalConnector):
     def resolve_fn(self, permissions, **kwargs):
         for perm in permissions:
-            if perm.has_permission(**kwargs):
+            if perm().has_permission(**kwargs):
                 return True
         return False
 
 
 class And(LogicalConnector):
     def resolve_fn(self, permissions, **kwargs):
         for perm in permissions:
-            if not perm.has_permission(**kwargs):
+            if not perm().has_permission(**kwargs):
                 return False
         return True
 
 
 class Not(LogicalConnector):
     def resolve_fn(self, permissions, **kwargs):
         assert len(permissions) == 1, "`Not` accepts only one permission class as parameter."
-        return not permissions[0].has_permission(**kwargs)
+        return not permissions[0]().has_permission(**kwargs)
 
 
 class AllowAll(BasePermission):