Minimize accesses to rights backend

radicale/app/__init__.py

@@ -264,12 +264,11 @@ class Application(
         # Create principal collection
         if user:
             principal_path = "/%s/" % user
-            if "W" in self._rights.authorization(user, principal_path):
+            with self._storage.acquire_lock("r", user):
-                with self._storage.acquire_lock("r", user):
+                principal = next(self._storage.discover(
-                    principal = next(
+                    principal_path, depth="1"), None)
-                        self._storage.discover(principal_path, depth="1"),
+            if not principal:
-                        None)
+                if "W" in self._rights.authorization(user, principal_path):
-                if not principal:
                     with self._storage.acquire_lock("w", user):
                         try:
                             self._storage.create_collection(principal_path)
@@ -277,9 +276,9 @@ class Application(
                             logger.warning("Failed to create principal "
                                            "collection %r: %s", user, e)
                             user = ""
-            else:
+                else:
-                logger.warning("Access to principal path %r denied by "
+                    logger.warning("Access to principal path %r denied by "
-                               "rights backend", principal_path)
+                                   "rights backend", principal_path)
 
         if self.configuration.get("server", "_internal_server"):
             # Verify content length
@@ -313,32 +312,6 @@ class Application(
 
         return response(status, headers, answer)
 
-    def _access(self, user, path, permission, item=None):
-        if permission not in "rw":
-            raise ValueError("Invalid permission argument: %r" % permission)
-        if not item:
-            permissions = permission + permission.upper()
-            parent_permissions = permission
-        elif isinstance(item, storage.BaseCollection):
-            if item.get_meta("tag"):
-                permissions = permission
-            else:
-                permissions = permission.upper()
-            parent_permissions = ""
-        else:
-            permissions = ""
-            parent_permissions = permission
-        if permissions and rights.intersect(
-                self._rights.authorization(user, path), permissions):
-            return True
-        if parent_permissions:
-            parent_path = pathutils.unstrip_path(
-                posixpath.dirname(pathutils.strip_path(path)), True)
-            if rights.intersect(self._rights.authorization(user, parent_path),
-                                parent_permissions):
-                return True
-        return False
-
     def _read_raw_content(self, environ):
         content_length = int(environ.get("CONTENT_LENGTH") or 0)
         if not content_length:
@@ -382,3 +355,44 @@ class Application(
         headers = {"Content-Type": "text/xml; charset=%s" % self._encoding}
         content = self._write_xml_content(xmlutils.webdav_error(human_tag))
         return status, headers, content
+
+
+class Access:
+    """Helper class to check access rights of an item"""
+
+    def __init__(self, rights, user, path):
+        self._rights = rights
+        self.user = user
+        self.path = path
+        self.parent_path = pathutils.unstrip_path(
+            posixpath.dirname(pathutils.strip_path(path)), True)
+        self.permissions = self._rights.authorization(self.user, self.path)
+        self._parent_permissions = None
+
+    @property
+    def parent_permissions(self):
+        if self.path == self.parent_path:
+            return self.permissions
+        if self._parent_permissions is None:
+            self._parent_permissions = self._rights.authorization(
+                self.user, self.parent_path)
+        return self._parent_permissions
+
+    def check(self, permission, item=None):
+        if permission not in "rw":
+            raise ValueError("Invalid permission argument: %r" % permission)
+        if not item:
+            permissions = permission + permission.upper()
+            parent_permissions = permission
+        elif isinstance(item, storage.BaseCollection):
+            if item.get_meta("tag"):
+                permissions = permission
+            else:
+                permissions = permission.upper()
+            parent_permissions = ""
+        else:
+            permissions = ""
+            parent_permissions = permission
+        return bool(rights.intersect(self.permissions, permissions) or (
+            self.path != self.parent_path and
+            rights.intersect(self.parent_permissions, parent_permissions)))

radicale/app/delete.py

@@ -20,7 +20,7 @@
 from http import client
 from xml.etree import ElementTree as ET
 
-from radicale import httputils, storage, xmlutils
+from radicale import app, httputils, storage, xmlutils
 
 
 def xml_delete(base_prefix, path, collection, href=None):
@@ -49,13 +49,14 @@ def xml_delete(base_prefix, path, collection, href=None):
 class ApplicationDeleteMixin:
     def do_DELETE(self, environ, base_prefix, path, user):
         """Manage DELETE request."""
-        if not self._access(user, path, "w"):
+        access = app.Access(self._rights, user, path)
+        if not access.check("w"):
             return httputils.NOT_ALLOWED
         with self._storage.acquire_lock("w", user):
             item = next(self._storage.discover(path), None)
             if not item:
                 return httputils.NOT_FOUND
-            if not self._access(user, path, "w", item):
+            if not access.check("w", item):
                 return httputils.NOT_ALLOWED
             if_match = environ.get("HTTP_IF_MATCH", "*")
             if if_match not in ("*", item.etag):

radicale/app/get.py

@@ -21,7 +21,7 @@ import posixpath
 from http import client
 from urllib.parse import quote
 
-from radicale import httputils, pathutils, storage, xmlutils
+from radicale import app, httputils, pathutils, storage, xmlutils
 from radicale.log import logger
 
 
@@ -70,13 +70,14 @@ class ApplicationGetMixin:
         # Dispatch .web URL to web module
         if path == "/.web" or path.startswith("/.web/"):
             return self._web.get(environ, base_prefix, path, user)
-        if not self._access(user, path, "r"):
+        access = app.Access(self._rights, user, path)
+        if not access.check("r"):
             return httputils.NOT_ALLOWED
         with self._storage.acquire_lock("r", user):
             item = next(self._storage.discover(path), None)
             if not item:
                 return httputils.NOT_FOUND
-            if not self._access(user, path, "r", item):
+            if not access.check("r", item):
                 return httputils.NOT_ALLOWED
             if isinstance(item, storage.BaseCollection):
                 tag = item.get_meta("tag")

radicale/app/mkcol.py

@@ -30,9 +30,8 @@ from radicale.log import logger
 class ApplicationMkcolMixin:
     def do_MKCOL(self, environ, base_prefix, path, user):
         """Manage MKCOL request."""
-        permissions = rights.intersect(
+        permissions = self._rights.authorization(user, path)
-            self._rights.authorization(user, path), "Ww")
+        if not rights.intersect(permissions, "Ww"):
-        if not permissions:
             return httputils.NOT_ALLOWED
         try:
             xml_content = self._read_xml_content(environ)

radicale/app/move.py

@@ -21,7 +21,7 @@ import posixpath
 from http import client
 from urllib.parse import urlparse
 
-from radicale import httputils, pathutils, storage
+from radicale import app, httputils, pathutils, storage
 from radicale.log import logger
 
 
@@ -34,7 +34,8 @@ class ApplicationMoveMixin:
             logger.info("Unsupported destination address: %r", raw_dest)
             # Remote destination server, not supported
             return httputils.REMOTE_DESTINATION
-        if not self._access(user, path, "w"):
+        access = app.Access(self._rights, user, path)
+        if not access.check("w"):
             return httputils.NOT_ALLOWED
         to_path = pathutils.sanitize_path(to_url.path)
         if not (to_path + "/").startswith(base_prefix + "/"):
@@ -42,15 +43,16 @@ class ApplicationMoveMixin:
                            "start with base prefix", to_path, path)
             return httputils.NOT_ALLOWED
         to_path = to_path[len(base_prefix):]
-        if not self._access(user, to_path, "w"):
+        to_access = app.Access(self._rights, user, to_path)
+        if not to_access.check("w"):
             return httputils.NOT_ALLOWED
 
         with self._storage.acquire_lock("w", user):
             item = next(self._storage.discover(path), None)
             if not item:
                 return httputils.NOT_FOUND
-            if (not self._access(user, path, "w", item) or
+            if (not access.check("w", item) or
-                    not self._access(user, to_path, "w", item)):
+                    not to_access.check("w", item)):
                 return httputils.NOT_ALLOWED
             if isinstance(item, storage.BaseCollection):
                 # TODO: support moving collections

radicale/app/propfind.py

@@ -24,7 +24,7 @@ import socket
 from http import client
 from xml.etree import ElementTree as ET
 
-from radicale import httputils, pathutils, rights, storage, xmlutils
+from radicale import app, httputils, pathutils, rights, storage, xmlutils
 from radicale.log import logger
 
 
@@ -343,7 +343,8 @@ class ApplicationPropfindMixin:
 
     def do_PROPFIND(self, environ, base_prefix, path, user):
         """Manage PROPFIND request."""
-        if not self._access(user, path, "r"):
+        access = app.Access(self._rights, user, path)
+        if not access.check("r"):
             return httputils.NOT_ALLOWED
         try:
             xml_content = self._read_xml_content(environ)
@@ -361,7 +362,7 @@ class ApplicationPropfindMixin:
             item = next(items, None)
             if not item:
                 return httputils.NOT_FOUND
-            if not self._access(user, path, "r", item):
+            if not access.check("r", item):
                 return httputils.NOT_ALLOWED
             # put item back
             items = itertools.chain([item], items)

radicale/app/proppatch.py

@@ -21,7 +21,7 @@ import socket
 from http import client
 from xml.etree import ElementTree as ET
 
-from radicale import httputils
+from radicale import app, httputils
 from radicale import item as radicale_item
 from radicale import storage, xmlutils
 from radicale.log import logger
@@ -87,7 +87,8 @@ def xml_proppatch(base_prefix, path, xml_request, collection):
 class ApplicationProppatchMixin:
     def do_PROPPATCH(self, environ, base_prefix, path, user):
         """Manage PROPPATCH request."""
-        if not self._access(user, path, "w"):
+        access = app.Access(self._rights, user, path)
+        if not access.check("w"):
             return httputils.NOT_ALLOWED
         try:
             xml_content = self._read_xml_content(environ)
@@ -102,7 +103,7 @@ class ApplicationProppatchMixin:
             item = next(self._storage.discover(path), None)
             if not item:
                 return httputils.NOT_FOUND
-            if not self._access(user, path, "w", item):
+            if not access.check("w", item):
                 return httputils.NOT_ALLOWED
             if not isinstance(item, storage.BaseCollection):
                 return httputils.FORBIDDEN

radicale/app/put.py

@@ -25,7 +25,7 @@ from http import client
 
 import vobject
 
-from radicale import httputils
+from radicale import app, httputils
 from radicale import item as radicale_item
 from radicale import pathutils, rights, storage, xmlutils
 from radicale.log import logger
@@ -114,7 +114,8 @@ def prepare(vobject_items, path, content_type, permissions, parent_permissions,
 class ApplicationPutMixin:
     def do_PUT(self, environ, base_prefix, path, user):
         """Manage PUT request."""
-        if not self._access(user, path, "w"):
+        access = app.Access(self._rights, user, path)
+        if not access.check("w"):
             return httputils.NOT_ALLOWED
         try:
             content = self._read_content(environ)
@@ -126,12 +127,6 @@ class ApplicationPutMixin:
             return httputils.REQUEST_TIMEOUT
         # Prepare before locking
         content_type = environ.get("CONTENT_TYPE", "").split(";")[0]
-        parent_path = pathutils.unstrip_path(
-            posixpath.dirname(pathutils.strip_path(path)), True)
-        permissions = rights.intersect(
-            self._rights.authorization(user, path), "Ww")
-        parent_permissions = rights.intersect(
-            self._rights.authorization(user, parent_path), "w")
         try:
             vobject_items = tuple(vobject.readComponents(content or ""))
         except Exception as e:
@@ -140,12 +135,14 @@ class ApplicationPutMixin:
             return httputils.BAD_REQUEST
         (prepared_items, prepared_tag, prepared_write_whole_collection,
          prepared_props, prepared_exc_info) = prepare(
-             vobject_items, path, content_type, permissions,
+             vobject_items, path, content_type,
-             parent_permissions)
+             bool(rights.intersect(access.permissions, "Ww")),
+             bool(rights.intersect(access.parent_permissions, "w")))
 
         with self._storage.acquire_lock("w", user):
             item = next(self._storage.discover(path), None)
-            parent_item = next(self._storage.discover(parent_path), None)
+            parent_item = next(
+                self._storage.discover(access.parent_path), None)
             if not parent_item:
                 return httputils.CONFLICT
 
@@ -159,10 +156,9 @@ class ApplicationPutMixin:
                 tag = parent_item.get_meta("tag")
 
             if write_whole_collection:
-                if ("w" if tag else "W") not in self._rights.authorization(
+                if ("w" if tag else "W") not in access.permissions:
-                        user, path):
                     return httputils.NOT_ALLOWED
-            elif "w" not in self._rights.authorization(user, parent_path):
+            elif "w" not in access.parent_permissions:
                 return httputils.NOT_ALLOWED
 
             etag = environ.get("HTTP_IF_MATCH", "")
@@ -182,8 +178,10 @@ class ApplicationPutMixin:
                     prepared_write_whole_collection != write_whole_collection):
                 (prepared_items, prepared_tag, prepared_write_whole_collection,
                  prepared_props, prepared_exc_info) = prepare(
-                     vobject_items, path, content_type, permissions,
+                     vobject_items, path, content_type,
-                     parent_permissions, tag, write_whole_collection)
+                     bool(rights.intersect(access.permissions, "Ww")),
+                     bool(rights.intersect(access.parent_permissions, "w")),
+                     tag, write_whole_collection)
             props = prepared_props
             if prepared_exc_info:
                 logger.warning(

radicale/app/report.py

@@ -24,7 +24,7 @@ from http import client
 from urllib.parse import unquote, urlparse
 from xml.etree import ElementTree as ET
 
-from radicale import httputils, pathutils, storage, xmlutils
+from radicale import app, httputils, pathutils, storage, xmlutils
 from radicale.item import filter as radicale_filter
 from radicale.log import logger
 
@@ -257,7 +257,8 @@ def xml_item_response(base_prefix, href, found_props=(), not_found_props=(),
 class ApplicationReportMixin:
     def do_REPORT(self, environ, base_prefix, path, user):
         """Manage REPORT request."""
-        if not self._access(user, path, "r"):
+        access = app.Access(self._rights, user, path)
+        if not access.check("r"):
             return httputils.NOT_ALLOWED
         try:
             xml_content = self._read_xml_content(environ)
@@ -273,7 +274,7 @@ class ApplicationReportMixin:
             item = next(self._storage.discover(path), None)
             if not item:
                 return httputils.NOT_FOUND
-            if not self._access(user, path, "r", item):
+            if not access.check("r", item):
                 return httputils.NOT_ALLOWED
             if isinstance(item, storage.BaseCollection):
                 collection = item