From aef58bd55cc6018b05c6416810d28d1901f5ac82 Mon Sep 17 00:00:00 2001 From: Unrud Date: Wed, 22 Apr 2020 19:20:07 +0200 Subject: [PATCH] Minimize accesses to rights backend --- radicale/app/__init__.py | 84 +++++++++++++++++++++++---------------- radicale/app/delete.py | 7 ++-- radicale/app/get.py | 7 ++-- radicale/app/mkcol.py | 5 +-- radicale/app/move.py | 12 +++--- radicale/app/propfind.py | 7 ++-- radicale/app/proppatch.py | 7 ++-- radicale/app/put.py | 30 +++++++------- radicale/app/report.py | 7 ++-- 9 files changed, 92 insertions(+), 74 deletions(-) diff --git a/radicale/app/__init__.py b/radicale/app/__init__.py index 8fc3e47..5b083e9 100644 --- a/radicale/app/__init__.py +++ b/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): - principal = next( - self._storage.discover(principal_path, depth="1"), - None) - if not principal: + with self._storage.acquire_lock("r", user): + principal = next(self._storage.discover( + principal_path, depth="1"), None) + if not principal: + if "W" in self._rights.authorization(user, principal_path): 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: - logger.warning("Access to principal path %r denied by " - "rights backend", principal_path) + else: + logger.warning("Access to principal path %r denied by " + "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))) diff --git a/radicale/app/delete.py b/radicale/app/delete.py index e4c8712..8d08d30 100644 --- a/radicale/app/delete.py +++ b/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): diff --git a/radicale/app/get.py b/radicale/app/get.py index c191c64..6310993 100644 --- a/radicale/app/get.py +++ b/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") diff --git a/radicale/app/mkcol.py b/radicale/app/mkcol.py index 56c3908..b4ba961 100644 --- a/radicale/app/mkcol.py +++ b/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( - self._rights.authorization(user, path), "Ww") - if not permissions: + permissions = self._rights.authorization(user, path) + if not rights.intersect(permissions, "Ww"): return httputils.NOT_ALLOWED try: xml_content = self._read_xml_content(environ) diff --git a/radicale/app/move.py b/radicale/app/move.py index 2e88ca3..0515e7b 100644 --- a/radicale/app/move.py +++ b/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 - not self._access(user, to_path, "w", item)): + if (not access.check("w", item) or + not to_access.check("w", item)): return httputils.NOT_ALLOWED if isinstance(item, storage.BaseCollection): # TODO: support moving collections diff --git a/radicale/app/propfind.py b/radicale/app/propfind.py index f348395..51472c4 100644 --- a/radicale/app/propfind.py +++ b/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) diff --git a/radicale/app/proppatch.py b/radicale/app/proppatch.py index 242bd15..53ab482 100644 --- a/radicale/app/proppatch.py +++ b/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 diff --git a/radicale/app/put.py b/radicale/app/put.py index 59b9c6c..b962fbc 100644 --- a/radicale/app/put.py +++ b/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, - parent_permissions) + vobject_items, path, content_type, + 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( - user, path): + if ("w" if tag else "W") not in access.permissions: 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, - parent_permissions, tag, write_whole_collection) + vobject_items, path, content_type, + 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( diff --git a/radicale/app/report.py b/radicale/app/report.py index f8ea151..6a2548a 100644 --- a/radicale/app/report.py +++ b/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