From 1bdc47bf4490a9dbfd78c27552089d8e877ff8fb Mon Sep 17 00:00:00 2001
From: Unrud <unrud@outlook.com>
Date: Tue, 21 Aug 2018 18:43:48 +0200
Subject: [PATCH] Make predefined rights plugins more restrictive and remove
 NoneAuth

Collections with tag are only allowed as direct children of a principal collections.
---
 radicale/rights.py            | 60 +++++++++++++++++++++--------------
 radicale/tests/test_base.py   | 10 ++++++
 radicale/tests/test_rights.py | 12 -------
 3 files changed, 46 insertions(+), 36 deletions(-)

diff --git a/radicale/rights.py b/radicale/rights.py
index 4d3bbbd..eb7a46b 100644
--- a/radicale/rights.py
+++ b/radicale/rights.py
@@ -52,11 +52,7 @@ INTERNAL_TYPES = ("none", "authenticated", "owner_write", "owner_only",
 def load(configuration):
     """Load the rights manager chosen in configuration."""
     rights_type = configuration.get("rights", "type")
-    if configuration.get("auth", "type") == "none":
-        rights_type = "none"
-    if rights_type == "none":
-        rights_class = NoneRights
-    elif rights_type == "authenticated":
+    if rights_type == "authenticated":
         rights_class = AuthenticatedRights
     elif rights_type == "owner_write":
         rights_class = OwnerWriteRights
@@ -97,38 +93,54 @@ class BaseRights:
         raise NotImplementedError
 
 
-class NoneRights(BaseRights):
-    def authorized(self, user, path, permissions):
-        return intersect_permissions(permissions)
-
-
 class AuthenticatedRights(BaseRights):
-    def authorized(self, user, path, permissions):
-        if not user:
-            return ""
-        return intersect_permissions(permissions)
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._verify_user = self.configuration.get("auth", "type") != "none"
 
-
-class OwnerWriteRights(BaseRights):
     def authorized(self, user, path, permissions):
-        if not user:
+        if self._verify_user and not user:
             return ""
         sane_path = storage.sanitize_path(path).strip("/")
-        if user != sane_path.split("/", maxsplit=1)[0]:
-            return intersect_permissions(permissions, "Rr")
-        return intersect_permissions(permissions)
+        if "/" not in sane_path:
+            return intersect_permissions(permissions, "RW")
+        if sane_path.count("/") == 1:
+            return intersect_permissions(permissions, "rw")
+        return ""
 
 
-class OwnerOnlyRights(BaseRights):
+class OwnerWriteRights(AuthenticatedRights):
     def authorized(self, user, path, permissions):
-        if not user:
+        if self._verify_user and not user:
             return ""
         sane_path = storage.sanitize_path(path).strip("/")
         if not sane_path:
             return intersect_permissions(permissions, "R")
-        if user != sane_path.split("/", maxsplit=1)[0]:
+        if self._verify_user:
+            owned = user == sane_path.split("/", maxsplit=1)[0]
+        else:
+            owned = True
+        if "/" not in sane_path:
+            return intersect_permissions(permissions, "RW" if owned else "R")
+        if sane_path.count("/") == 1:
+            return intersect_permissions(permissions, "rw" if owned else "r")
+        return ""
+
+
+class OwnerOnlyRights(AuthenticatedRights):
+    def authorized(self, user, path, permissions):
+        if self._verify_user and not user:
             return ""
-        return intersect_permissions(permissions)
+        sane_path = storage.sanitize_path(path).strip("/")
+        if not sane_path:
+            return intersect_permissions(permissions, "R")
+        if self._verify_user and user != sane_path.split("/", maxsplit=1)[0]:
+            return ""
+        if "/" not in sane_path:
+            return intersect_permissions(permissions, "RW")
+        if sane_path.count("/") == 1:
+            return intersect_permissions(permissions, "rw")
+        return ""
 
 
 class Rights(BaseRights):
diff --git a/radicale/tests/test_base.py b/radicale/tests/test_base.py
index cc1cfdf..5933ec0 100644
--- a/radicale/tests/test_base.py
+++ b/radicale/tests/test_base.py
@@ -1428,6 +1428,16 @@ class BaseFileSystemTest(BaseTest):
         self.configuration["storage"]["filesystem_folder"] = self.colpath
         # Disable syncing to disk for better performance
         self.configuration["internal"]["filesystem_fsync"] = "False"
+        # Allow access to anything for tests
+        rights_file_path = os.path.join(self.colpath, "rights")
+        with open(rights_file_path, "w") as f:
+            f.write("""\
+[allow all]
+user: .*
+collection: .*
+permissions: RrWw""")
+        self.configuration["rights"]["file"] = rights_file_path
+        self.configuration["rights"]["type"] = "from_file"
         self.application = Application(self.configuration)
 
     def teardown(self):
diff --git a/radicale/tests/test_rights.py b/radicale/tests/test_rights.py
index 5c3f9eb..6fc1f60 100644
--- a/radicale/tests/test_rights.py
+++ b/radicale/tests/test_rights.py
@@ -99,18 +99,6 @@ class TestBaseAuthRequests(BaseTest):
         self._test_rights("authenticated", "tmp", "/other", "r", 207)
         self._test_rights("authenticated", "tmp", "/other", "w", 207)
 
-    def test_none(self):
-        self._test_rights("none", "", "/", "r", 207)
-        self._test_rights("none", "", "/", "w", 207)
-        self._test_rights("none", "", "/tmp", "r", 207)
-        self._test_rights("none", "", "/tmp", "w", 207)
-        self._test_rights("none", "tmp", "/", "r", 207)
-        self._test_rights("none", "tmp", "/", "w", 207)
-        self._test_rights("none", "tmp", "/tmp", "r", 207)
-        self._test_rights("none", "tmp", "/tmp", "w", 207)
-        self._test_rights("none", "tmp", "/other", "r", 207)
-        self._test_rights("none", "tmp", "/other", "w", 207)
-
     def test_from_file(self):
         rights_file_path = os.path.join(self.colpath, "rights")
         with open(rights_file_path, "w") as f: