Carve out exception for delete permission for group-wide items (#1023)

* Update permissions to have special carve out to avoid deletion of starting-materials/equipment

* Update deletion error message from webapp to mention permissions

pydatalab/src/pydatalab/permissions.py

@@ -60,7 +60,7 @@ def wrapped_route(*args, **kwargs):
     return wrapped_route
 
 
-def get_default_permissions(user_only: bool = True) -> Dict[str, Any]:
+def get_default_permissions(user_only: bool = True, deleting: bool = False) -> Dict[str, Any]:
     """Return the MongoDB query terms corresponding to the current user.
 
     Will return open permissions if a) the `CONFIG.TESTING` parameter is `True`,
@@ -109,6 +109,10 @@ def get_default_permissions(user_only: bool = True) -> Dict[str, Any]:
             # TODO: remove this hack when permissions are refactored. Currently starting_materials and equipment
             # are a special case that should be group editable, so even when the route has asked to only edit this
             # user's stuff, we can also let starting materials and equipment through.
+
+            # If we are trying to delete, then make sure they cannot delete items that do not match their user
+            if deleting:
+                return user_perm
             user_perm = {"$or": [user_perm, {"type": {"$in": ["starting_materials", "equipment"]}}]}
             return user_perm
         return {"$or": [user_perm, null_perm]}

pydatalab/src/pydatalab/routes/v0_1/collections.py

@@ -293,7 +293,10 @@ def delete_collection(collection_id: str):
                 projection={"_id": 1},
             )["_id"]
             result = flask_mongo.db.collections.delete_one(
-                {"collection_id": collection_id, **get_default_permissions(user_only=True)}
+                {
+                    "collection_id": collection_id,
+                    **get_default_permissions(user_only=True, deleting=True),
+                }
             )
             if result.deleted_count != 1:
                 return (

pydatalab/src/pydatalab/routes/v0_1/items.py

@@ -703,7 +703,7 @@ def delete_sample():
     item_id = request_json["item_id"]
 
     result = flask_mongo.db.items.delete_one(
-        {"item_id": item_id, **get_default_permissions(user_only=True)}
+        {"item_id": item_id, **get_default_permissions(user_only=True, deleting=True)}
     )
 
     if result.deleted_count != 1:

pydatalab/tests/server/test_starting_materials.py

@@ -192,3 +192,18 @@ def test_starting_material_permissions(
         json={"item_id": default_starting_material_dict["item_id"], "block_id": block_id},
     )
     assert response.status_code == 200
+
+    # Check that a normal user cannot entirely delete a starting material that has no user
+    response = client.post(
+        "/delete-sample/",
+        json={"item_id": default_starting_material_dict["item_id"]},
+    )
+
+    assert response.status_code == 401
+
+    # Check that the admin can delete a starting material that has no user
+    response = admin_client.post(
+        "/delete-sample/",
+        json={"item_id": default_starting_material_dict["item_id"]},
+    )
+    assert response.status_code == 200

webapp/src/server_fetch_utils.js

@@ -371,7 +371,13 @@ export function deleteSample(item_id) {
       console.log("delete successful" + response_json);
       store.commit("deleteFromSampleList", item_id);
     })
-    .catch((error) => alert("Sample delete failed for " + item_id + ": " + error));
+    .catch(() =>
+      alert(
+        "Unable to delete item with ID '" +
+          item_id +
+          "': check that you have the appropriate permissions.",
+      ),
+    );
 }
 
 export function deleteStartingMaterial(item_id) {
@@ -382,7 +388,13 @@ export function deleteStartingMaterial(item_id) {
       console.log("delete successful" + response_json);
       store.commit("deleteFromStartingMaterialList", item_id);
     })
-    .catch((error) => alert("Item delete failed for " + item_id + ": " + error));
+    .catch(() =>
+      alert(
+        "Unable to delete item with ID '" +
+          item_id +
+          "': check that you have the appropriate permissions.",
+      ),
+    );
 }
 
 export function deleteCollection(collection_id, collection_summary) {
@@ -391,7 +403,13 @@ export function deleteCollection(collection_id, collection_summary) {
       console.log("delete successful" + response_json);
       store.commit("deleteFromCollectionList", collection_summary);
     })
-    .catch((error) => alert("Collection delete failed for " + collection_id + ": " + error));
+    .catch(() =>
+      alert(
+        "Unable to delete collection with ID '" +
+          collection_id +
+          "': check that you have the appropriate permissions.",
+      ),
+    );
 }
 
 export function deleteEquipment(item_id) {
@@ -402,7 +420,13 @@ export function deleteEquipment(item_id) {
       console.log("delete successful" + response_json);
       store.commit("deleteFromEquipmentList", item_id);
     })
-    .catch((error) => alert("Item delete failed for " + item_id + ": " + error));
+    .catch(() =>
+      alert(
+        "Unable to delete item with ID '" +
+          item_id +
+          "': check that you have the appropriate permissions.",
+      ),
+    );
 }
 
 export function deletSampleFromCollection(collection_id, collection_summary) {