diff --git a/CHANGELOG.md b/CHANGELOG.md index d72e62b5..3e441527 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/SHARING.md b/SHARING.md index 964f755b..7fc128a9 100644 --- a/SHARING.md +++ b/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) | -| User | str | optional(default:owner) | -| Permissions | str | optional(default:r) | -| Enabled | bool | optional(owner/default:False) | -| Hidden | bool | optional(owner/default:True) | +| Conversion | str | optional (default:none) | +| User | str | optional (default:owner) | +| Permissions | str | optional (default:rp) | +| Enabled | bool | optional (owner/default:False) | +| 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) | -| Enabled | bool | optional(owner/default:False) | -| Hidden | bool | optional(owner/default:True) | +| Permissions | str | optional (default:r) | +| Enabled | bool | optional (owner/default:False) | +| Hidden | bool | optional (owner/default:True) | | Properties | optional | * Output: text/plain|application/json diff --git a/radicale/app/propfind.py b/radicale/app/propfind.py index 41ba6ef9..45f5e18d 100644 --- a/radicale/app/propfind.py +++ b/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: diff --git a/radicale/app/proppatch.py b/radicale/app/proppatch.py index 462f3a0e..0f60dc97 100644 --- a/radicale/app/proppatch.py +++ b/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) diff --git a/radicale/config.py b/radicale/config.py index f7179bad..581d4939 100644 --- a/radicale/config.py +++ b/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", { diff --git a/radicale/sharing/__init__.py b/radicale/sharing/__init__.py index dc926022..41b8067b 100644 --- a/radicale/sharing/__init__.py +++ b/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 diff --git a/radicale/tests/test_sharing.py b/radicale/tests/test_sharing.py index eadedab6..3302680a 100644 --- a/radicale/tests/test_sharing.py +++ b/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="""\ + + + + + """ + color + """ + + +""", 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="""\ + + + + + """ + color + """ + + +""", 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="""\ + + + + + """ + color + """ + + +""", 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="""\ + + + + + """ + color + """ + + +""", 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",