fix: copy dc links when copying projects (#784)

Author: olevski

components/renku_data_services/data_connectors/db.py

@@ -605,7 +605,7 @@ async def insert_link(
     async def copy_link(
         self,
         user: base_models.APIUser,
-        project_id: ULID,
+        target_project_id: ULID,
         link: models.DataConnectorToProjectLink,
         *,
         session: AsyncSession | None = None,
@@ -615,7 +615,7 @@ async def copy_link(
             user, ResourceType.data_connector, link.data_connector_id, Scope.READ
         )
         allowed_to_write_project = await self.authz.has_permission(
-            user, ResourceType.project, link.project_id, Scope.WRITE
+            user, ResourceType.project, target_project_id, Scope.WRITE
         )
         if not allowed_to_read_dc:
             raise errors.MissingResourceError(
@@ -627,7 +627,7 @@ async def copy_link(
                 message=f"The project with ID {link.project_id} does not exist or you do not have access to it"
             )
         unsaved_link = models.UnsavedDataConnectorToProjectLink(
-            data_connector_id=link.data_connector_id, project_id=project_id
+            data_connector_id=link.data_connector_id, project_id=target_project_id
         )
         return await self.insert_link(user=user, link=unsaved_link, session=session)
 

components/renku_data_services/project/blueprints.py

@@ -146,7 +146,7 @@ async def _copy(
             _: Request, user: base_models.APIUser, project_id: ULID, body: apispec.ProjectPost
         ) -> JSONResponse:
             project = await copy_project(
-                project_id=project_id,
+                source_project_id=project_id,
                 user=user,
                 name=body.name,
                 namespace=body.namespace,

components/renku_data_services/project/core.py

@@ -62,7 +62,7 @@ def validate_project_patch(patch: apispec.ProjectPatch) -> models.ProjectPatch:
 
 
 async def copy_project(
-    project_id: ULID,
+    source_project_id: ULID,
     user: APIUser,
     name: str,
     namespace: str,
@@ -77,7 +77,7 @@ async def copy_project(
     data_connector_repo: DataConnectorRepository,
 ) -> models.Project:
     """Create a copy of a given project."""
-    template = await project_repo.get_project(user=user, project_id=project_id, with_documentation=True)
+    template = await project_repo.get_project(user=user, project_id=source_project_id, with_documentation=True)
     repositories_ = _validate_repositories(repositories)
 
     unsaved_project = models.UnsavedProject(
@@ -96,17 +96,17 @@ async def copy_project(
     project = await project_repo.insert_project(user, unsaved_project)
 
     # NOTE: Copy session launchers
-    launchers = await session_repo.get_project_launchers(user=user, project_id=project_id)
+    launchers = await session_repo.get_project_launchers(user=user, project_id=source_project_id)
     for launcher in launchers:
         await session_repo.copy_launcher(user=user, project_id=project.id, launcher=launcher)
 
     # NOTE: Copy data connector links. If this operation fails due to lack of permission, still proceed to create the
     # copy but return an error code that reflects this
     uncopied_dc_ids: list[ULID] = []
-    dc_links = await data_connector_repo.get_links_to(user=user, project_id=project_id)
+    dc_links = await data_connector_repo.get_links_to(user=user, project_id=source_project_id)
     for dc_link in dc_links:
         try:
-            await data_connector_repo.copy_link(user=user, project_id=project.id, link=dc_link)
+            await data_connector_repo.copy_link(user=user, target_project_id=project.id, link=dc_link)
         except errors.MissingResourceError:
             uncopied_dc_ids.append(dc_link.data_connector_id)
 

test/bases/renku_data_services/data_api/conftest.py

@@ -1,10 +1,12 @@
 import json
-from collections.abc import AsyncGenerator
+from collections.abc import AsyncGenerator, Callable
 from copy import deepcopy
 from typing import Any
 
+import pytest
 import pytest_asyncio
 from authzed.api.v1 import Relationship, RelationshipUpdate, SubjectReference, WriteRelationshipsRequest
+from httpx import Response
 from sanic import Sanic
 from sanic_testing.testing import SanicASGITestClient
 from ulid import ULID
@@ -167,6 +169,34 @@ async def unauthorized_headers() -> dict[str, str]:
     return {"Authorization": "Bearer {}"}
 
 
+@pytest.fixture
+def headers_from_user(
+    admin_user: UserInfo,
+    admin_headers: dict[str, str],
+    regular_user: UserInfo,
+    user_headers: dict[str, str],
+    member_1_user: UserInfo,
+    member_1_headers: dict[str, str],
+    member_2_user: UserInfo,
+    member_2_headers: dict[str, str],
+    unauthorized_headers: dict[str, str],
+) -> Callable[[UserInfo], dict[str, str]]:
+    def _headers_from_user(user: UserInfo) -> dict[str, str]:
+        match user.id:
+            case admin_user.id:
+                return admin_headers
+            case regular_user.id:
+                return user_headers
+            case member_1_user.id:
+                return member_1_headers
+            case member_2_user.id:
+                return member_2_headers
+            case _:
+                return unauthorized_headers
+
+    return _headers_from_user
+
+
 @pytest_asyncio.fixture
 async def bootstrap_admins(
     sanic_client_with_migrations, app_config_instance: Config, event_loop, admin_user: UserInfo
@@ -258,7 +288,7 @@ async def create_project_helper(
 
 
 @pytest_asyncio.fixture
-async def create_project_copy(sanic_client, user_headers, admin_headers, regular_user, admin_user):
+async def create_project_copy(sanic_client, user_headers, headers_from_user):
     async def create_project_copy_helper(
         id: str,
         namespace: str,
@@ -268,7 +298,7 @@ async def create_project_copy_helper(
         members: list[dict[str, str]] = None,
         **payload,
     ) -> dict[str, Any]:
-        headers = user_headers if user is None or user is regular_user else admin_headers
+        headers = headers_from_user(user) if user is not None else user_headers
         copy_payload = {"slug": Slug.from_name(name).value}
         copy_payload.update(payload)
         copy_payload.update({"namespace": namespace, "name": name})
@@ -391,7 +421,7 @@ async def create_data_connector_helper(
 
 @pytest_asyncio.fixture
 async def create_data_connector_and_link_project(
-    sanic_client, regular_user, user_headers, admin_user, admin_headers, create_data_connector
+    regular_user, user_headers, admin_user, admin_headers, create_data_connector, link_data_connector
 ):
     async def create_data_connector_and_link_project_helper(
         name: str, project_id: str, admin: bool = False, **payload
@@ -401,20 +431,27 @@ async def create_data_connector_and_link_project_helper(
 
         data_connector = await create_data_connector(name, user=user, headers=headers, **payload)
         data_connector_id = data_connector["id"]
-        payload = {"project_id": project_id}
-
-        _, response = await sanic_client.post(
-            f"/api/data/data_connectors/{data_connector_id}/project_links", headers=headers, json=payload
-        )
-
-        assert response.status_code == 201, response.text
+        response = await link_data_connector(project_id, data_connector_id, headers=headers)
         data_connector_link = response.json
 
         return data_connector, data_connector_link
 
     return create_data_connector_and_link_project_helper
 
 
+@pytest.fixture
+def link_data_connector(sanic_client: SanicASGITestClient):
+    async def _link_data_connector(project_id: str, dc_id: str, headers: dict[str, str]) -> Response:
+        payload = {"project_id": project_id}
+        _, response = await sanic_client.post(
+            f"/api/data/data_connectors/{dc_id}/project_links", headers=headers, json=payload
+        )
+        assert response.status_code == 201, response.text
+        return response
+
+    return _link_data_connector
+
+
 @pytest_asyncio.fixture
 async def create_resource_pool(sanic_client, user_headers, admin_headers, valid_resource_pool_payload):
     async def create_resource_pool_helper(admin: bool = False, **payload) -> dict[str, Any]:

test/bases/renku_data_services/data_api/test_projects.py

@@ -7,6 +7,7 @@
 
 import pytest
 from httpx import Response
+from sanic_testing.testing import SanicASGITestClient
 from sqlalchemy import select
 from syrupy.filters import props
 from ulid import ULID
@@ -1484,6 +1485,49 @@ async def test_project_copy_includes_data_connector_links(
     assert {d["id"] for d in data_connector_links} != {link_1["id"], link_2["id"]}
 
 
+@pytest.mark.asyncio
+async def test_project_copy_includes_public_data_connector_links_owned_by_others(
+    sanic_client: SanicASGITestClient,
+    user_headers: dict[str, str],
+    regular_user: UserInfo,
+    member_1_headers: dict[str, str],
+    member_1_user: UserInfo,
+    member_2_headers: dict[str, str],
+    member_2_user: UserInfo,
+    create_project,
+    create_project_copy,
+    create_data_connector,
+    link_data_connector,
+) -> None:
+    project = await create_project("Project", visibility="public")
+    project_id = project["id"]
+    dc1 = await create_data_connector("dc1", member_1_user, member_1_headers, visibility="public")
+    dc2 = await create_data_connector("dc2", member_1_user, member_1_headers, visibility="public")
+    assert "id" in dc1
+    assert "id" in dc2
+    link1_res = await link_data_connector(project_id, dc1["id"], user_headers)
+    link2_res = await link_data_connector(project_id, dc2["id"], user_headers)
+    link1 = link1_res.json
+    link2 = link2_res.json
+
+    copy_project = await create_project_copy(
+        project_id,
+        member_2_user.namespace.path.serialize(),
+        "Copy Project",
+        user=member_2_user,
+    )
+    project_copy_id = copy_project["id"]
+    _, response = await sanic_client.get(
+        f"/api/data/projects/{project_copy_id}/data_connector_links", headers=member_2_headers
+    )
+    assert response.status_code == 200, response.text
+    data_connector_links = response.json
+    assert {d["data_connector_id"] for d in data_connector_links} == {dc1["id"], dc2["id"]}
+    assert data_connector_links[0]["project_id"] == data_connector_links[1]["project_id"] == project_copy_id
+    # NOTE: Check that new data connector links are created
+    assert {d["id"] for d in data_connector_links} != {link1["id"], link2["id"]}
+
+
 @pytest.mark.asyncio
 async def test_project_get_all_copies(
     sanic_client, admin_user, regular_user, admin_headers, user_headers, create_project, create_project_copy