Merge pull request #2156 from pbiering/sharing-block-update-conversion

Sharing block update conversion

radicale/sharing/__init__.py

@@ -947,7 +947,7 @@ class BaseSharing:
             Conversion = request_data['Conversion']
             # verify against whitelist
             if Conversion not in CONVERSIONS_WHITELIST:
-                return httputils.bad_request("Conversion not supported: %r" % Conversion)
+                return httputils.bad_request("Conversion is not supported: %r" % Conversion)
 
         if 'Actions' in request_data:
             valid = True  # default
@@ -1060,11 +1060,11 @@ class BaseSharing:
                     return httputils.NOT_FOUND
                 if not isinstance(item, storage.BaseCollection):
                     logger.warning(api_info + ": PathMapped=%r is not a collection", PathMapped)
-                    return httputils.METHOD_NOT_ALLOWED
+                    return httputils.bad_request("PathMapped is not a collection")
                 if Conversion == "bday":
                     if item.tag != "VADDRESSBOOK":
                         logger.warning(api_info + ": PathMapped=%r is not a VADDRESSBOOK collection (mandatory for Conversion=%r)", PathMapped, Conversion)
-                        return httputils.METHOD_NOT_ALLOWED
+                        return httputils.bad_request("Conversion is not supported for collection type")
 
             if Permissions is None:
                 if ShareType == "token":
@@ -1081,7 +1081,7 @@ class BaseSharing:
                     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
+                            return httputils.bad_request("Permissions are not supported for conversion")
 
             if Enabled is None:
                 Enabled = False # security by default
@@ -1299,7 +1299,12 @@ class BaseSharing:
                     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
+                            return httputils.bad_request("Permissions are not supported for conversion")
+
+            if Conversion is not None and share['Conversion'] is not None:
+                if Conversion != share['Conversion']:
+                    logger.warning(api_info + ": PathMapped=%r change of Conversion %r -> %r is not supported", PathMapped, share['Conversion'], Conversion)
+                    return httputils.bad_request("Change of conversion is not supported")
 
             if user == share['Owner']:
                 if PathMapped is not None:
@@ -1324,8 +1329,9 @@ class BaseSharing:
                            User=User,
                            Timestamp=Timestamp,
                            Properties=Properties,
+                           Conversion=Conversion,
                            Actions=Actions,
-                           Conversion=Conversion)
+                           )
                 else:
                     result = self.database_update_sharing(
                            ShareType=ShareType,
@@ -1338,8 +1344,9 @@ class BaseSharing:
                            User=User,
                            Timestamp=Timestamp,
                            Properties=Properties,
+                           Conversion=Conversion,
                            Actions=Actions,
-                           Conversion=Conversion)
+                           )
 
             elif user == share['User']:
                 # User is only allowed to update Properties

radicale/tests/test_sharing.py

@@ -4756,7 +4756,7 @@ permissions: RrWw""")
             json_dict['Enabled'] = True
             json_dict['Hidden'] = False
             json_dict['Properties'] = {"D:displayname": "Test-BDAY"}
-            _, headers, answer = self._sharing_api_json("map", "create", check=405, login="owner:ownerpw", json_dict=json_dict)
+            _, headers, answer = self._sharing_api_json("map", "create", check=400, login="owner:ownerpw", json_dict=json_dict)
 
             # create map
             logging.info("\n*** create map(bday) user/owner:r -> ok")
@@ -4774,6 +4774,24 @@ permissions: RrWw""")
             answer_dict = json.loads(answer)
             assert answer_dict['Status'] == "success"
 
+            # update map with (denied) conversion change
+            logging.info("\n*** update map(bday), try to change Conversion -> fail")
+            json_dict = {}
+            json_dict['User'] = "user"
+            json_dict['PathMapped'] = path_mapped
+            json_dict['PathOrToken'] = path_shared_r
+            json_dict['Conversion'] = "none"
+            _, headers, answer = self._sharing_api_json("map", "update", check=400, login="owner:ownerpw", json_dict=json_dict)
+
+            # update map with same conversion
+            logging.info("\n*** update map(bday) with equal Conversion -> ok")
+            json_dict = {}
+            json_dict['User'] = "user"
+            json_dict['PathMapped'] = path_mapped
+            json_dict['PathOrToken'] = path_shared_r
+            json_dict['Conversion'] = "bday"
+            _, headers, answer = self._sharing_api_json("map", "update", check=200, login="owner:ownerpw", json_dict=json_dict)
+
             # enable map by user
             logging.info("\n*** enable map(bday) by user")
             json_dict = {}
@@ -6243,7 +6261,7 @@ permissions: RrWw""")
             json_dict['Permissions'] = "rw"
             json_dict['Hidden'] = False
             json_dict['Conversion'] = "bday"
-            _, headers, answer = self._sharing_api_json("token", "create", check=405, login="owner:ownerpw", json_dict=json_dict)
+            _, headers, answer = self._sharing_api_json("token", "create", check=400, login="owner:ownerpw", json_dict=json_dict)
 
             # check PROPFIND item with token
             logging.info("\n*** PROPFIND item with token -> calendar")
@@ -6378,7 +6396,7 @@ permissions: RrWw""")
             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)
+            _, headers, answer = self._sharing_api_json("token", "update", check=400, login="owner:ownerpw", json_dict=json_dict)
 
             # update map to "rPE"
             logging.info("\n*** update token with bday conversion ('rPE' permissions) -> not supported")
@@ -6386,7 +6404,7 @@ permissions: RrWw""")
             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)
+            _, headers, answer = self._sharing_api_json("token", "update", check=400, login="owner:ownerpw", json_dict=json_dict)
 
             # update map Conversion
             logging.info("\n*** update token remove Conversion -> not supported")
@@ -6432,7 +6450,7 @@ permissions: RrWw""")
             json_dict['Enabled'] = True
             json_dict['Hidden'] = False
             json_dict['Conversion'] = "bday"
-            _, headers, answer = self._sharing_api_json("token", "create", check=405, login="owner:ownerpw", json_dict=json_dict)
+            _, headers, answer = self._sharing_api_json("token", "create", check=400, login="owner:ownerpw", json_dict=json_dict)
 
     def test_sharing_api_map_properies_overlay_unicode(self) -> None:
         """share-by-map API usage tests related to properties overlay using unicode."""