Merge pull request #2076 from pbiering/sharing-token-permission-default-rp

Sharing token permission default rp

CHANGELOG.md

@@ -4,6 +4,8 @@
 * Fix: share address book collection as birthday calendar not working on non-DEBUG level
 * Extension: share accept now PATH and USER for matching email addresses as well
 * Adjustment: replace logging/trace_on_debug by new log level "trace"
+* Adjustment: sharing/token: adjust default permissions to "rp"
+* Fix: sharing/propfind+proppatch: permission check related to properties
 
 ## 3.7.0
 

SHARING.md

@@ -310,11 +310,11 @@ Create a share by mapping a collection of an `Owner` to a token.
 | Parameter | Type | Requirement |
 | - | - | - |
 | PathMapped | str | mandatory |
-| Conversion | str | optional(default:none) |
+| Conversion | str | optional (default:none) |
-| User | str | optional(default:owner) |
+| User | str | optional (default:owner) |
-| Permissions | str | optional(default:r) |
+| Permissions | str | optional (default:rp) |
-| Enabled | bool | optional(owner/default:False) |
+| Enabled | bool | optional (owner/default:False) |
-| Hidden | bool | optional(owner/default:True) |
+| Hidden | bool | optional (owner/default:True) |
 | Properties | str | optional |
 
  * Output: text/plain|application/json
@@ -359,11 +359,11 @@ Create a share by mapping a collection of an `Owner` to an `User`.
 | - | - | - |
 | PathOrToken | str | mandatory |
 | PathMapped | str | mandatory |
-| Conversion | str | optional(default:none) |
+| Conversion | str | optional (default:none) |
 | User | str | mandatory |
-| Permissions | str | optional(default:r) |
+| Permissions | str | optional (default:r) |
-| Enabled | bool | optional(owner/default:False) |
+| Enabled | bool | optional (owner/default:False) |
-| Hidden | bool | optional(owner/default:True) |
+| Hidden | bool | optional (owner/default:True) |
 | Properties | optional |
 
  * Output: text/plain|application/json

radicale/app/propfind.py

@@ -313,7 +313,7 @@ def xml_propfind_response(
                     ) and not (
                             "p" in share['Permissions'] or
                             ("p" in raw_permissions and "P" not in share['Permissions']) or
-                            (self._sharing.permit_properties_overlay and "P" not in raw_permissions and "P" not in share['Permissions'])):
+                            (not self._sharing.permit_properties_overlay and "P" not in raw_permissions and "P" not in share['Permissions'])):
                     logger.trace("PROPFIND/xml_propfind_response/current-user-privilege-set: add D:write-properties")
                     privileges.append("D:write-properties")
             elif write:

radicale/app/proppatch.py

@@ -124,7 +124,7 @@ class ApplicationPartProppatch(ApplicationBase):
                     ) and not (
                             "p" in share['Permissions'] or
                             ("p" in raw_permissions and "P" not in share['Permissions']) or
-                            (self._sharing.permit_properties_overlay and "P" not in raw_permissions and "P" not in share['Permissions'])):
+                            (not self._sharing.permit_properties_overlay and "P" not in raw_permissions and "P" not in share['Permissions'])):
                     logger.info("PROPPATCH request on shared %r: write-access", path_orig)
                     if permissions_filter is not None and "e" in permissions_filter:
                         logger.info("PROPPATCH request on shared %r: write-access, overlay enforced, but disabled by share permission 'e'", path_orig)

radicale/config.py

@@ -509,7 +509,7 @@ DEFAULT_CONFIG_SCHEMA: types.CONFIG_SCHEMA = OrderedDict([
             "help": "enforce properties overlay on write access",
             "type": bool}),
         ("default_permissions_create_token", {
-            "value": "r",
+            "value": "rp",
             "help": "default permissions for token-based sharing",
             "type": rights_permission}),
         ("default_permissions_create_map", {

radicale/sharing/__init__.py

@@ -880,9 +880,9 @@ class BaseSharing:
             else:
                 Permissions = str(Permissions)
                 if Conversion == "bday":
-                    # bday is read-only
+                    # bday is read-only and not supporting "Ee"
                     for permission in Permissions:
-                        if permission not in "rPpEe":
+                        if permission not in "rPp":
                             logger.warning(api_info + ": PathMapped=%r Permissions=%r not supported for Conversion=%r", PathMapped, Permissions, Conversion)
                             return httputils.METHOD_NOT_ALLOWED
 
@@ -1081,6 +1081,15 @@ class BaseSharing:
                             logger.trace("" + api_info + ": clear property %r", prop)
                             del Properties[prop]
 
+            if Permissions is not None and share['Conversion'] is not None:
+                Permissions = str(Permissions)
+                if share['Conversion'] == "bday":
+                    # bday is read-only and not supporting "Ee"
+                    for permission in Permissions:
+                        if permission not in "rPp":
+                            logger.warning(api_info + ": PathMapped=%r Permissions=%r not supported for Conversion=%r", PathMapped, Permissions, Conversion)
+                            return httputils.METHOD_NOT_ALLOWED
+
             if user == share['Owner']:
                 if PathMapped is not None:
                     # check access Permissions

radicale/tests/test_sharing.py

@@ -5134,6 +5134,7 @@ permissions: RrWw""")
         for db_type in list(filter(lambda item: item != "none", sharing.INTERNAL_TYPES)):
             logging.info("\n*** test: %s", db_type)
             self.configure({"sharing": {"type": db_type}})
+            self.configure({"sharing": {"permit_properties_overlay": "True"}})
 
             path_mapped = "/owner/adressbook-" + db_type + ".vcf/"
             self.create_addressbook(path_mapped, login="owner:ownerpw")
@@ -5170,7 +5171,7 @@ permissions: RrWw""")
             assert "NICKNAME-C3" in answer
 
             # create map
-            logging.info("\n*** create token with bday conversion -> ok")
+            logging.info("\n*** create token with bday conversion (default permissions) -> ok")
             json_dict = {}
             json_dict['User'] = "owner"
             json_dict['PathMapped'] = path_mapped
@@ -5191,15 +5192,6 @@ permissions: RrWw""")
             assert "Test-FN-C3 (BDAY)" in answer
             assert "Test-FN (BDAY)" in answer
 
-            # check PROPFIND item with token
-            logging.info("\n*** PROPFIND item with token -> calendar")
-            response = self._propfind_allprop(path_shared)
-            logging.debug("response: %r", response)
-            assert "CR:supported-address-data" not in response
-            assert "D:sync-token" not in response
-            assert "C:supported-calendar-component-set" in response
-            assert "D:current-user-privilege-set" in response
-
             # verify content as owner
             logging.info("\n*** GET collection owner -> ok")
             _, headers, answer = self.request("GET", path_shared)
@@ -5209,7 +5201,7 @@ permissions: RrWw""")
             assert 'Content-Disposition' in headers
             assert 'Calendar.ics' in headers['Content-Disposition']
 
-            # create map
+            # create map of ics with conversion -> fail
             logging.info("\n*** create token with bday conversion but unsupported permissions -> fail")
             json_dict = {}
             json_dict['User'] = "owner"
@@ -5220,6 +5212,157 @@ permissions: RrWw""")
             json_dict['Conversion'] = "bday"
             _, headers, answer = self._sharing_api_json("token", "create", check=405, login="owner:ownerpw", json_dict=json_dict)
 
+            # check PROPFIND item with token
+            logging.info("\n*** PROPFIND item with token -> calendar")
+            response = self._propfind_allprop(path_shared)
+            logging.debug("response: %r", response)
+            assert "CR:supported-address-data" not in response
+            assert "D:sync-token" not in response
+            assert "C:supported-calendar-component-set" in response
+            assert "D:current-user-privilege-set" in response
+            status, props = response["D:current-user-privilege-set"]
+            privileges = props.findall(xmlutils.make_clark("D:privilege"))
+            assert len(privileges) >= 1
+            privileges_list = [xmlutils.make_human_tag(privilege.findall("*")[0].tag) for privilege in privileges]
+            assert "D:read" in privileges_list
+            assert "D:write-content" not in privileges_list
+            assert "D:write-properties" not in privileges_list
+            assert "D:write" not in privileges_list
+            assert "D:all" not in privileges_list
+
+            # execute PROPPATCH color as user
+            logging.info("\n*** PROPPATCH color collection with token -> permission denied")
+            color = "#BBBBBB"
+            _, responses = self.proppatch(path=path_shared, data="""\
+<?xml version="1.0" encoding="utf-8"?>
+<D:propertyupdate xmlns:D="DAV:">
+<D:set>
+<D:prop>
+  <I:calendar-color xmlns:I="http://apple.com/ns/ical/">""" + color + """</I:calendar-color>
+</D:prop>
+</D:set>
+</D:propertyupdate>""", check=403)
+
+            # update map
+            logging.info("\n*** update token with bday conversion ("r" permissions) -> ok")
+            json_dict = {}
+            json_dict['User'] = "owner"
+            json_dict['PathOrToken'] = path_shared
+            json_dict['Permissions'] = "r"
+            _, headers, answer = self._sharing_api_json("token", "update", check=200, login="owner:ownerpw", json_dict=json_dict)
+
+            logging.info("\n*** PROPFIND item with token -> calendar")
+            response = self._propfind_allprop(path_shared)
+            logging.debug("response: %r", response)
+            status, props = response["D:current-user-privilege-set"]
+            privileges = props.findall(xmlutils.make_clark("D:privilege"))
+            assert len(privileges) >= 1
+            privileges_list = [xmlutils.make_human_tag(privilege.findall("*")[0].tag) for privilege in privileges]
+            assert "D:read" in privileges_list
+            assert "D:write-content" not in privileges_list
+            assert "D:write-properties" in privileges_list
+            assert "D:write" not in privileges_list
+            assert "D:all" not in privileges_list
+
+            # execute PROPPATCH color as user
+            logging.info("\n*** PROPPATCH color collection with token -> ok")
+            color = "#BBBBBB"
+            _, responses = self.proppatch(path=path_shared, data="""\
+<?xml version="1.0" encoding="utf-8"?>
+<D:propertyupdate xmlns:D="DAV:">
+<D:set>
+<D:prop>
+  <I:calendar-color xmlns:I="http://apple.com/ns/ical/">""" + color + """</I:calendar-color>
+</D:prop>
+</D:set>
+</D:propertyupdate>""", check=207)
+
+            self.configure({"sharing": {"permit_properties_overlay": "False"}})
+
+            logging.info("\n*** PROPFIND item with token (r) -> calendar")
+            response = self._propfind_allprop(path_shared)
+            logging.debug("response: %r", response)
+            status, props = response["D:current-user-privilege-set"]
+            privileges = props.findall(xmlutils.make_clark("D:privilege"))
+            assert len(privileges) >= 1
+            privileges_list = [xmlutils.make_human_tag(privilege.findall("*")[0].tag) for privilege in privileges]
+            assert "D:read" in privileges_list
+            assert "D:write-content" not in privileges_list
+            assert "D:write-properties" not in privileges_list
+            assert "D:write" not in privileges_list
+            assert "D:all" not in privileges_list
+
+            # execute PROPPATCH color as user
+            logging.info("\n*** PROPPATCH color collection with token -> ok")
+            color = "#BBBBBB"
+            _, responses = self.proppatch(path=path_shared, data="""\
+<?xml version="1.0" encoding="utf-8"?>
+<D:propertyupdate xmlns:D="DAV:">
+<D:set>
+<D:prop>
+  <I:calendar-color xmlns:I="http://apple.com/ns/ical/">""" + color + """</I:calendar-color>
+</D:prop>
+</D:set>
+</D:propertyupdate>""", check=403)
+
+            # update map to "rP"
+            logging.info("\n*** update token with bday conversion ('rP' permissions) -> ok")
+            json_dict = {}
+            json_dict['User'] = "owner"
+            json_dict['PathOrToken'] = path_shared
+            json_dict['Permissions'] = "rP"
+            _, headers, answer = self._sharing_api_json("token", "update", check=200, login="owner:ownerpw", json_dict=json_dict)
+
+            logging.info("\n*** PROPFIND item with token -> calendar")
+            response = self._propfind_allprop(path_shared)
+            logging.debug("response: %r", response)
+            status, props = response["D:current-user-privilege-set"]
+            privileges = props.findall(xmlutils.make_clark("D:privilege"))
+            assert len(privileges) >= 1
+            privileges_list = [xmlutils.make_human_tag(privilege.findall("*")[0].tag) for privilege in privileges]
+            assert "D:read" in privileges_list
+            assert "D:write-content" not in privileges_list
+            assert "D:write-properties" in privileges_list
+            assert "D:write" not in privileges_list
+            assert "D:all" not in privileges_list
+
+            # execute PROPPATCH color as user
+            logging.info("\n*** PROPPATCH color collection with token -> ok")
+            color = "#BBBBBB"
+            _, responses = self.proppatch(path=path_shared, data="""\
+<?xml version="1.0" encoding="utf-8"?>
+<D:propertyupdate xmlns:D="DAV:">
+<D:set>
+<D:prop>
+  <I:calendar-color xmlns:I="http://apple.com/ns/ical/">""" + color + """</I:calendar-color>
+</D:prop>
+</D:set>
+</D:propertyupdate>""", check=207)
+
+            # update map to "rPe"
+            logging.info("\n*** update token with bday conversion ('rPe' permissions) -> not supported")
+            json_dict = {}
+            json_dict['User'] = "owner"
+            json_dict['PathOrToken'] = path_shared
+            json_dict['Permissions'] = "rPe"
+            _, headers, answer = self._sharing_api_json("token", "update", check=405, login="owner:ownerpw", json_dict=json_dict)
+
+            # update map to "rPE"
+            logging.info("\n*** update token with bday conversion ('rPE' permissions) -> not supported")
+            json_dict = {}
+            json_dict['User'] = "owner"
+            json_dict['PathOrToken'] = path_shared
+            json_dict['Permissions'] = "rPE"
+            _, headers, answer = self._sharing_api_json("token", "update", check=405, login="owner:ownerpw", json_dict=json_dict)
+
+            # update map Conversion
+            logging.info("\n*** update token remove Conversion -> not supported")
+            json_dict = {}
+            json_dict['User'] = "owner"
+            json_dict['PathOrToken'] = path_shared
+            json_dict['Conversion'] = ""
+            _, headers, answer = self._sharing_api_json("token", "update", check=400, login="owner:ownerpw", json_dict=json_dict)
+
     def test_sharing_api_token_ics_bday(self) -> None:
         """share-by-token ics with bday conversion (has to fail)."""
         self.configure({"auth": {"type": "htpasswd",