add tests for being able to delete documents

concierge_backend_lib/document_collections.py

@@ -102,7 +102,7 @@ async def get_documents(token, collection_id):
 
 async def delete_document(token, collection_id, document_type, document_id):
     if auth_enabled:
-        authorized = await authorize(token, collection_id, "delete")
+        authorized = await authorize(token, collection_id, "update")
         if not authorized:
             raise UnauthorizedOperationError()
     return await asyncify(

concierge_shiny/collection_management.py

@@ -164,9 +164,9 @@ def collection_view():
                     ui.accordion_panel(
                         ui.markdown(
                             "#### You don't have permission to ingest documents into this collection"
-                        )
-                    ),
-                    value="ingest_documents",
+                        ),
+                        value="ingest_documents",
+                    )
                 )
             accordion_elements.append(
                 ui.accordion_panel(

requirements.txt

@@ -16,4 +16,4 @@ shinyswatch~=0.8.0
 isi-util==0.2a1
 concierge-util==0.2a2
 python-keycloak~=5.1.1
-httpx~=0.28.1
+httpx~=0.23.2

tests/test_rbac.py

@@ -11,6 +11,7 @@
     create_collection,
     delete_collection,
     get_documents,
+    delete_document,
 )
 from concierge_backend_lib.ingesting import insert_document
 from concierge_backend_lib.loading import load_file
@@ -100,8 +101,6 @@ async def test_cannot_read_collection(user, collection_name):
         await get_documents(token["access_token"], collection_lookup[collection_name])
 
 
-# we will use the documents created in the next tests
-document_lookup = {}
 # we will use the same file for each test
 doc = load_file(os.path.join(os.path.dirname(__file__), "test_doc.txt"))
 
@@ -112,7 +111,6 @@ async def ingest_document(user, collection_name):
         token["access_token"], collection_lookup[collection_name], doc
     ):
         pass
-    document_lookup[f"{collection_name} document"] = doc_id
     return doc_id
 
 
@@ -147,6 +145,48 @@ async def test_cannot_ingest_document(user, collection_name):
         await ingest_document(user, collection_name)
 
 
+async def delete_document_with_user(user, collection_name):
+    # create a new entry each time to avoid accidentally trying to delete the same one multiple times
+    doc_id = await ingest_document(
+        "testadmin", collection_name
+    )  # testadmin should be able to ingest documents into any collection
+    token = keycloak_client.token(user, "test")
+    return await delete_document(
+        token["access_token"], collection_lookup[collection_name], "plaintext", doc_id
+    )
+
+
+@pytest.mark.parametrize(
+    "user,collection_name",
+    [
+        ("testadmin", "testadmin's shared collection"),
+        ("testadmin", "testadmin's private collection"),
+        ("testadmin", "testprivate's private collection"),
+        ("testshared", "testadmin's shared collection"),
+        ("testprivate", "testprivate's private collection"),
+    ],
+)
+async def test_can_delete_document(user, collection_name):
+    assert await delete_document_with_user(user, collection_name)
+
+
+@pytest.mark.parametrize(
+    "user,collection_name",
+    [
+        ("testsharedread", "testadmin's private collection"),
+        ("testsharedread", "testadmin's shared collection"),
+        ("testshared", "testadmin's private collection"),
+        ("testprivate", "testadmin's private collection"),
+        ("testprivate", "testadmin's shared collection"),
+        ("testnothing", "testadmin's shared collection"),
+        ("testnothing", "testadmin's private collection"),
+    ],
+)
+async def test_cannot_delete_document(user, collection_name):
+    with pytest.raises((UnauthorizedOperationError, KeycloakPostError)):
+        await delete_document_with_user(user, collection_name)
+
+
 async def teardown():
     token = get_keycloak_admin_openid_token()
     for id in collection_lookup.values():