Merge branch 'main' into bes/uv-vis-reformat-for-plugin-dev

Author: ml-evs

pydatalab/src/pydatalab/routes/v0_1/files.py

@@ -22,14 +22,22 @@ def _(): ...
 
 @FILES.route("/files/<string:file_id>/<string:filename>", methods=["GET"])
 def get_file(file_id: str, filename: str):
+    """If this user has the appropriate permissions, return the file with the
+    given database ID and filename.
+
+    Parameters:
+        file_id: The file ID in the database.
+        filename: The filename in the database.
+
+    """
     try:
         _file_id = ObjectId(file_id)
     except InvalidId:
         # If the ID is invalid, then there will be no results in the database anyway,
         # so just 401
         _file_id = file_id
-    if not pydatalab.mongo.flask_mongo.db.files.find_one(
-        {"_id": _file_id, **get_default_permissions(user_only=False)}
+    if not pydatalab.mongo.flask_mongo.db.items.find_one(
+        {"file_ObjectIds": {"$in": [_file_id]}, **get_default_permissions(user_only=False)}
     ):
         return (
             jsonify(
@@ -41,6 +49,7 @@ def get_file(file_id: str, filename: str):
             ),
             401,
         )
+
     path = os.path.join(CONFIG.FILE_DIRECTORY, secure_filename(file_id))
     return send_from_directory(path, filename)
 

pydatalab/tests/server/conftest.py

@@ -162,6 +162,12 @@ def client(app, user_api_key):
     yield client_factory(app, user_api_key)
 
 
+@pytest.fixture(scope="function")
+def another_client(app, another_user_api_key):
+    """Returns a test client for the API with a second normal user access."""
+    yield client_factory(app, another_user_api_key)
+
+
 @pytest.fixture(scope="function")
 def unauthenticated_client(app):
     """Returns an unauthenticated test client for the API."""
@@ -201,6 +207,11 @@ def user_api_key() -> str:
     return generate_api_key()
 
 
+@pytest.fixture(scope="session")
+def another_user_api_key() -> str:
+    return generate_api_key()
+
+
 @pytest.fixture(scope="session")
 def unverified_user_api_key() -> str:
     return generate_api_key()
@@ -216,6 +227,11 @@ def user_id():
     yield ObjectId(24 * "1")
 
 
+@pytest.fixture(scope="session")
+def another_user_id():
+    yield ObjectId(24 * "7")
+
+
 @pytest.fixture(scope="session")
 def admin_user_id():
     yield ObjectId(24 * "8")
@@ -253,6 +269,8 @@ def insert_demo_users(
     app,
     user_id,
     user_api_key,
+    another_user_id,
+    another_user_api_key,
     admin_user_id,
     admin_api_key,
     deactivated_user_id,
@@ -262,6 +280,7 @@ def insert_demo_users(
     real_mongo_client,
 ):
     insert_user(user_id, user_api_key, "user", real_mongo_client)
+    insert_user(another_user_id, another_user_api_key, "user", real_mongo_client)
     insert_user(admin_user_id, admin_api_key, "admin", real_mongo_client)
     insert_user(
         deactivated_user_id,

pydatalab/tests/server/test_files.py

@@ -52,7 +52,7 @@ def test_upload(client, default_filepath, insert_default_sample, default_sample)
         )
     assert isinstance(response.json["file_id"], str)
     assert response.json["file_information"]
-    assert response.json["status"], "success"
+    assert response.json["status"] == "success"
     assert response.status_code == 201
 
 
@@ -99,6 +99,7 @@ def test_get_file_and_delete(client, default_filepath, default_sample):
     assert not response.json["files_data"]
 
 
+@pytest.mark.dependency(depends=["test_get_file_and_delete"])
 def test_upload_new_version(
     client, default_filepath, insert_default_sample, default_sample, tmpdir
 ):  # pylint: disable=unused-argument
@@ -120,7 +121,7 @@ def test_upload_new_version(
     file_id = response.json["file_id"]
     assert file_id
     assert response.json["file_information"]
-    assert response.json["status"], "success"
+    assert response.json["status"] == "success"
     assert response.status_code == 201
 
     # Copy the file to a new temp directory so its fs metadata changes
@@ -142,10 +143,61 @@ def test_upload_new_version(
         )
     assert isinstance(response_reup.json["file_id"], str)
     assert response_reup.json["file_information"]
-    assert response_reup.json["status"], "success"
+    assert response_reup.json["status"] == "success"
     assert response_reup.status_code == 201
     assert (
         response_reup.json["file_information"]["location"]
         == response.json["file_information"]["location"]
     )
     assert response_reup.json["file_id"] == response.json["file_id"]
+
+
+@pytest.mark.dependency(depends=["test_upload_new_version"])
+def test_file_permissions(
+    client,
+    another_client,
+    another_user_id,
+    default_filepath,
+    insert_default_sample,
+    default_sample,
+    tmpdir,
+):  # pylint: disable=unused-argument
+    """Upload a file as one user, then test access as two different users."""
+    filename = "my_secret_file.txt"
+    with open(default_filepath, "rb") as f:
+        response = client.post(
+            "/upload-file/",
+            buffered=True,
+            content_type="multipart/form-data",
+            data={
+                "item_id": default_sample.item_id,
+                "file": [(f, filename)],
+                "type": "application/octet-stream",
+                "replace_file": "null",
+                "relativePath": "null",
+            },
+        )
+
+    assert response.status_code == 201
+    resp_json = response.json
+    assert resp_json["status"] == "success"
+    file_id = resp_json["file_id"]
+
+    # Test that a random user cannot access the file directly
+    response = another_client.get(f"/files/{file_id}/{filename}")
+    assert response.status_code == 401
+    assert response.json["status"] == "error"
+
+    # Give the user access to the item, then check again
+    # First get refcode for item ID
+    response = client.get(f"/get-item-data/{default_sample.item_id}")
+    refcode = response.json["item_data"]["refcode"]
+
+    # Add normal user to the item
+    response = client.patch(
+        f"/items/{refcode}/permissions", json={"creators": [{"immutable_id": str(another_user_id)}]}
+    )
+
+    # Now check they have access to the file
+    response = another_client.get(f"/files/{file_id}/{filename}")
+    assert response.status_code == 200