Make it so users cannot remove themselves nor the original creator fr…

…om creators lists

pydatalab/pydatalab/routes/v0_1/items.py

@@ -654,10 +654,22 @@ def update_item_permissions(refcode: str):
             400,
         )
 
+    # Make sure a user cannot remove their own access to an item
+    current_user_id = current_user.person.immutable_id
+    try:
+        creator_ids.remove(current_user_id)
+    except ValueError:
+        pass
+    creator_ids.insert(0, current_user_id)
+
+    # The first ID in the creator list takes precedence; always make sure this is included to avoid orphaned items
     if current_creator_ids:
         base_owner = current_creator_ids[0]
-        if base_owner not in creator_ids:
-            creator_ids.append(base_owner)
+    try:
+        creator_ids.remove(base_owner)
+    except ValueError:
+        pass
+    creator_ids.insert(0, base_owner)
 
     LOGGER.warning("Setting permissions for item %s to %s", refcode, creator_ids)
     result = flask_mongo.db.items.update_one(

pydatalab/tests/server/test_permissions.py

@@ -7,7 +7,7 @@ def test_unverified_user_permissions(unverified_client):
     response = client.get("/samples/")
     assert response.status_code == 200
 
-    response = client.post("/new-sample/", json={"item_id": "test"})
+    response = client.post("/new-sample/", json={"type": "samples", "item_id": "test"})
     assert response.status_code == 401
 
     response = client.get("/starting-materials/")
@@ -20,7 +20,7 @@ def test_deactivated_user_permissions(deactivated_client):
     response = client.get("/samples/")
     assert response.status_code == 200
 
-    response = client.post("/new-sample/", json={"item_id": "test"})
+    response = client.post("/new-sample/", json={"type": "samples", "item_id": "test"})
     assert response.status_code == 401
 
     response = client.get("/starting-materials/")
@@ -33,7 +33,7 @@ def test_unauthenticated_user_permissions(unauthenticated_client):
     response = client.get("/samples/")
     assert response.status_code == 401
 
-    response = client.post("/new-sample/", json={"item_id": "test"})
+    response = client.post("/new-sample/", json={"type": "samples", "item_id": "test"})
     assert response.status_code == 401
 
     response = client.get("/starting-materials/")
@@ -43,24 +43,42 @@ def test_unauthenticated_user_permissions(unauthenticated_client):
 def test_basic_permissions_update(admin_client, admin_user_id, client, user_id):
     """Test that an admin can share an item with a normal user."""
 
-    response = admin_client.get("/new-sample/", json={"item_id": "test-admin-sample"})
-    assert response.status_code == 200
+    response = admin_client.post(
+        "/new-sample/", json={"type": "samples", "item_id": "test-admin-sample"}
+    )
+    assert response.status_code == 201
 
     response = admin_client.get("/get-item-data/test-admin-sample")
     assert response.status_code == 200
-    refcode = response["item_data"]["refcode"]
+    refcode = response.json["item_data"]["refcode"]
 
     response = client.get(f"/items/{refcode}")
     assert response.status_code == 404
 
-    admin_client.patch(f"/items/{refcode}", json={"creators": [{"immutable_id": user_id}]})
+    # Add normal user to the item
+    response = admin_client.patch(
+        f"/items/{refcode}/permissions", json={"creators": [{"immutable_id": str(user_id)}]}
+    )
+    assert response.status_code == 200
+
+    # Check that they can now see it
+    response = client.get(f"/items/{refcode}")
+    assert response.status_code == 200
+
+    # Check that they cannot remove themselves/the admin from the creators
+    client.patch(f"/items/{refcode}/permissions", json={"creators": []})
     assert response.status_code == 200
 
     response = client.get(f"/items/{refcode}")
     assert response.status_code == 200
 
-    client.patch(f"/items/{refcode}", json={"creators": []})
+    # Check that the admin can remove the user from the permissions
+    response = admin_client.patch(f"/items/{refcode}/permissions", json={"creators": []})
     assert response.status_code == 200
 
     response = client.get(f"/items/{refcode}")
     assert response.status_code == 404
+
+    # but that the admin still remains the creator
+    response = admin_client.get(f"/items/{refcode}")
+    assert response.status_code == 200