Merge pull request #611 from Unrud/rights

Implement rights types directly and test them

radicale/rights.py

@@ -41,51 +41,29 @@ import os.path
 import re
 from configparser import ConfigParser
 from importlib import import_module
-from io import StringIO
 
 from . import storage
 
 
 def load(configuration, logger):
     """Load the rights manager chosen in configuration."""
-    auth_type = configuration.get("auth", "type")
     rights_type = configuration.get("rights", "type")
-    if auth_type == "None" or rights_type == "None":
+    if configuration.get("auth", "type") == "None":
-        return lambda user, collection, permission: True
+        rights_type = "None"
-    elif rights_type in DEFINED_RIGHTS or rights_type == "from_file":
+    logger.info("Rights type is %r", rights_type)
-        return Rights(configuration, logger).authorized
+    if rights_type == "None":
+        rights_class = NoneRights
+    elif rights_type == "authenticated":
+        rights_class = AuthenticatedRights
+    elif rights_type == "owner_write":
+        rights_class = OwnerWriteRights
+    elif rights_type == "owner_only":
+        rights_class = OwnerOnlyRights
+    elif rights_type == "from_file":
+        rights_class = Rights
     else:
-        module = import_module(rights_type)
+        rights_class = import_module(rights_type).Rights
-        return module.Rights(configuration, logger).authorized
+    return rights_class(configuration, logger).authorized
-
-
-DEFINED_RIGHTS = {
-    "authenticated": """
-[rw]
-user:.+
-collection:.*
-permission:rw
-    """,
-    "owner_write": """
-[w]
-user:.+
-collection:%(login)s(/.*)?
-permission:rw
-[r]
-user:.+
-collection:.*
-permission:r
-    """,
-    "owner_only": """
-[rw]
-user:.+
-collection:%(login)s(/.*)?
-permission:rw
-[r]
-user:.+
-collection:
-permission:r
-    """}
 
 
 class BaseRights:
@@ -102,32 +80,48 @@ class BaseRights:
         raise NotImplementedError
 
 
+class NoneRights(BaseRights):
+    def authorized(self, user, path, permission):
+        return True
+
+
+class AuthenticatedRights(BaseRights):
+    def authorized(self, user, path, permission):
+        return bool(user)
+
+
+class OwnerWriteRights(BaseRights):
+    def authorized(self, user, path, permission):
+        sane_path = storage.sanitize_path(path).strip("/")
+        return bool(user) and (permission == "r" or
+                               user == sane_path.split("/", maxsplit=1)[0])
+
+
+class OwnerOnlyRights(BaseRights):
+    def authorized(self, user, path, permission):
+        sane_path = storage.sanitize_path(path).strip("/")
+        return bool(user) and (
+            permission == "r" and not sane_path.strip("/") or
+            user == sane_path.split("/", maxsplit=1)[0])
+
+
 class Rights(BaseRights):
     def __init__(self, configuration, logger):
         super().__init__(configuration, logger)
         self.filename = os.path.expanduser(configuration.get("rights", "file"))
-        self.rights_type = configuration.get("rights", "type").lower()
 
     def authorized(self, user, path, permission):
         user = user or ""
-        if user and not storage.is_safe_path_component(user):
-            # Prevent usernames like "user/calendar.ics"
-            raise ValueError("Refused unsafe username: %s", user)
         sane_path = storage.sanitize_path(path).strip("/")
         # Prevent "regex injection"
         user_escaped = re.escape(user)
         sane_path_escaped = re.escape(sane_path)
         regex = ConfigParser(
             {"login": user_escaped, "path": sane_path_escaped})
-        if self.rights_type in DEFINED_RIGHTS:
+        if not regex.read(self.filename):
-            self.logger.debug("Rights type '%s'", self.rights_type)
+            raise RuntimeError("Failed to read rights from file %r",
-            regex.readfp(StringIO(DEFINED_RIGHTS[self.rights_type]))
+                               self.filename)
-        else:
+            return False
-            self.logger.debug("Reading rights from file '%s'", self.filename)
-            if not regex.read(self.filename):
-                self.logger.error(
-                    "File '%s' not found for rights", self.filename)
-                return False
 
         for section in regex.sections():
             re_user = regex.get(section, "user")

radicale/tests/custom/rights.py

@@ -0,0 +1,27 @@
+# This file is part of Radicale Server - Calendar Server
+# Copyright (C) 2017 Unrud <unrud@openaliasbox.org>
+#
+# This library is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Radicale.  If not, see <http://www.gnu.org/licenses/>.
+
+"""
+Custom rights management.
+
+"""
+
+from radicale import rights
+
+
+class Rights(rights.BaseRights):
+    def authorized(self, user, path, permission):
+        return path.strip("/") in ("tmp", "other")

radicale/tests/test_rights.py

@@ -0,0 +1,139 @@
+# This file is part of Radicale Server - Calendar Server
+# Copyright (C) 2017 Unrud <unrud@openaliasbox.org>
+#
+# This library is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Radicale.  If not, see <http://www.gnu.org/licenses/>.
+
+"""
+Radicale tests with simple requests and rights.
+"""
+
+import base64
+import os
+import shutil
+import tempfile
+
+from radicale import Application, config
+
+from .test_base import BaseTest
+
+
+class TestBaseAuthRequests(BaseTest):
+    """Tests basic requests with rights."""
+
+    def setup(self):
+        self.configuration = config.load()
+        self.colpath = tempfile.mkdtemp()
+        self.configuration.set("storage", "filesystem_folder", self.colpath)
+        # Disable syncing to disk for better performance
+        self.configuration.set("storage", "filesystem_fsync", "False")
+        # Required on Windows, doesn't matter on Unix
+        self.configuration.set("storage", "close_lock_file", "True")
+
+    def teardown(self):
+        shutil.rmtree(self.colpath)
+
+    def _test_rights(self, rights_type, user, path, mode, expected_status):
+        assert mode in ("r", "w")
+        assert user in ("", "tmp")
+        htpasswd_file_path = os.path.join(self.colpath, ".htpasswd")
+        with open(htpasswd_file_path, "w") as f:
+            f.write("tmp:bepo\nother:bepo")
+        self.configuration.set("rights", "type", rights_type)
+        self.configuration.set("auth", "type", "htpasswd")
+        self.configuration.set("auth", "htpasswd_filename", htpasswd_file_path)
+        self.configuration.set("auth", "htpasswd_encryption", "plain")
+        self.application = Application(self.configuration, self.logger)
+        for u in ("tmp", "other"):
+            status, _, _ = self.request(
+                "PROPFIND", "/%s" % u, HTTP_AUTHORIZATION="Basic %s" %
+                base64.b64encode(("%s:bepo" % u).encode()).decode())
+            assert status == 207
+        status, _, _ = self.request(
+            "PROPFIND" if mode == "r" else "PROPPATCH", path,
+            HTTP_AUTHORIZATION="Basic %s" % base64.b64encode(
+                ("tmp:bepo").encode()).decode() if user else "")
+        assert status == expected_status
+
+    def test_owner_only(self):
+        self._test_rights("owner_only", "", "/", "r", 401)
+        self._test_rights("owner_only", "", "/", "w", 401)
+        self._test_rights("owner_only", "", "/tmp", "r", 401)
+        self._test_rights("owner_only", "", "/tmp", "w", 401)
+        self._test_rights("owner_only", "tmp", "/", "r", 207)
+        self._test_rights("owner_only", "tmp", "/", "w", 403)
+        self._test_rights("owner_only", "tmp", "/tmp", "r", 207)
+        self._test_rights("owner_only", "tmp", "/tmp", "w", 207)
+        self._test_rights("owner_only", "tmp", "/other", "r", 403)
+        self._test_rights("owner_only", "tmp", "/other", "w", 403)
+
+    def test_owner_write(self):
+        self._test_rights("owner_write", "", "/", "r", 401)
+        self._test_rights("owner_write", "", "/", "w", 401)
+        self._test_rights("owner_write", "", "/tmp", "r", 401)
+        self._test_rights("owner_write", "", "/tmp", "w", 401)
+        self._test_rights("owner_write", "tmp", "/", "r", 207)
+        self._test_rights("owner_write", "tmp", "/", "w", 403)
+        self._test_rights("owner_write", "tmp", "/tmp", "r", 207)
+        self._test_rights("owner_write", "tmp", "/tmp", "w", 207)
+        self._test_rights("owner_write", "tmp", "/other", "r", 207)
+        self._test_rights("owner_write", "tmp", "/other", "w", 403)
+
+    def test_authenticated(self):
+        self._test_rights("authenticated", "", "/", "r", 401)
+        self._test_rights("authenticated", "", "/", "w", 401)
+        self._test_rights("authenticated", "", "/tmp", "r", 401)
+        self._test_rights("authenticated", "", "/tmp", "w", 401)
+        self._test_rights("authenticated", "tmp", "/", "r", 207)
+        self._test_rights("authenticated", "tmp", "/", "w", 207)
+        self._test_rights("authenticated", "tmp", "/tmp", "r", 207)
+        self._test_rights("authenticated", "tmp", "/tmp", "w", 207)
+        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:
+            f.write("""\
+[owner]
+user: .+
+collection: %(login)s(/.*)?
+permission: rw
+[custom]
+user: .*
+collection: custom(/.*)?
+permission: r""")
+        self.configuration.set("rights", "file", rights_file_path)
+        self._test_rights("from_file", "", "/other", "r", 401)
+        self._test_rights("from_file", "tmp", "/other", "r", 403)
+        self._test_rights("from_file", "", "/custom/sub", "r", 404)
+        self._test_rights("from_file", "tmp", "/custom/sub", "r", 404)
+        self._test_rights("from_file", "", "/custom/sub", "w", 401)
+        self._test_rights("from_file", "tmp", "/custom/sub", "w", 403)
+
+    def test_custom(self):
+        """Custom rights management."""
+        self._test_rights("tests.custom.rights", "", "/", "r", 401)
+        self._test_rights("tests.custom.rights", "", "/tmp", "r", 207)