Parcourir la source

fix(projects): a project's BOM no longer shows archives from printers outside the user's groups

The BOM routes stored any archive id on a line and answered with that
archive's print name, looked up by id alone. A member of a group limited to
some printers could read the name of any archive (404 when asked for
directly) by linking a BOM line to it, and every reader of the project saw
the names of archives from printers they have no access to. A link to an
archive that does not exist was saved too: a dangling id on SQLite, a
foreign key error (500) on PostgreSQL.

A line now links only to a live archive on the caller's printers (400
"Archive not found" otherwise, the same for hidden and missing), and shows
its archive's id and name only to callers who can see it, as the project's
archive list does. Every non-zero id is checked on edit, also an unchanged
one, so resending a hidden link's id answers like a missing archive. 0 means
no archive on create too. The list looks the names up in one query.
maziggy il y a 1 semaine
Parent
commit
9c9278e673

+ 1 - 0
CHANGELOG.md

@@ -162,6 +162,7 @@ All notable changes to Bambuddy will be documented in this file.
 - **The frontend build no longer warns about `path` and `crypto` being externalized for the STEP previewer (#2976)** — `occt-import-js`, the Emscripten build behind STEP previews, requires both modules, but only inside its `ENVIRONMENT_IS_NODE` branches; in the browser it loads its `.wasm` from the URL the preview worker passes and draws randomness from `crypto.getRandomValues`. Vite still externalized both and printed two warnings on every build. `vite.config.ts` now drops exactly those two warnings for that one package through `build.rolldownOptions.onLog`, so an externalization anywhere else, or of any other module, still shows.
 
 ### Fixed
+- **A project's bill of materials no longer shows archives from printers outside the user's groups** — A BOM line can be linked to an archive through the API, and the BOM answered with that archive's name whichever printer it came from. A member of a group limited to some printers could read the name of any archive by linking a line to it, and everyone reading the project saw the names of archives from printers they have no access to. A link to an archive that does not exist was saved too (a server error on PostgreSQL). A BOM line now links only to an archive the user can see, and shows a linked archive only to users who can see it.
 - **Adding archives or queue jobs to a project keeps to the user's own printers and prints** — Adding archives to a project (**Assign to project**), removing them, and adding queue jobs took any archive or job by its id. A member of a group limited to some printers could link another team's archives and jobs to a project or unlink them, and the reply's count told them which hidden archives exist; any project editor could also relink other users' prints. These now change only the archives and jobs the user could edit directly: on their printers, and only their own unless they may edit everyone's (Operators: their own). Others are skipped like missing ones, and **Assign to project** now reports how many were actually added. An API key with project access still needs no archive or queue access for this, but changes only the archives and jobs of its printers that its owner could change.
 - **Editing an archive can no longer move it to a printer outside the user's groups** — A member of a group limited to some printers could move an archive of one of their printers to any other printer through the archive edit, into the history of a team they have no access to, although uploading an archive to such a printer was already refused. Any edit could also link an archive to a printer or project that does not exist (a server error on PostgreSQL, a broken link on SQLite). Such an edit is now refused with "Printer not found" or "Project not found". Moving an archive between the user's own printers, and removing its printer or project, work as before.
 - **A smart plug or virtual printer can no longer be linked to printer 0** — Adding or editing a smart plug and adding a virtual printer skipped the "Printer not found" check when the printer id was `0`. On PostgreSQL the save then failed with a server error; on SQLite the plug or virtual printer was saved linked to a printer that does not exist. Such a request is now refused with "Printer not found", as for any other unknown printer.

+ 59 - 76
backend/app/api/routes/projects.py

@@ -1577,11 +1577,54 @@ async def delete_project_cover_image(
 # ============ Phase 7: BOM Endpoints ============
 
 
+async def _visible_archive_names(db: AsyncSession, archive_ids, printer_scope: PrinterScope) -> dict[int, str | None]:
+    """Print names of the archives among ``archive_ids`` the caller can see.
+
+    As the project's archive list: live archives on the caller's printers
+    (#1727). An archive missing from the result reads as missing.
+    """
+    ids = {i for i in archive_ids if i}
+    if not ids:
+        return {}
+    query = select(PrintArchive.id, PrintArchive.print_name).where(PrintArchive.id.in_(ids), _LIVE_ARCHIVE)
+    if (clause := printer_scope.where(PrintArchive.printer_id)) is not None:
+        query = query.where(clause)
+    return dict((await db.execute(query)).all())
+
+
+async def _ensure_linkable_archive(db: AsyncSession, archive_id: int, printer_scope: PrinterScope) -> None:
+    """Refuse a link to an archive the caller cannot see; hidden reads as missing."""
+    if archive_id not in await _visible_archive_names(db, [archive_id], printer_scope):
+        raise HTTPException(status_code=400, detail="Archive not found")
+
+
+def _bom_response(item: ProjectBOMItem, visible_names: dict[int, str | None]) -> BOMItemResponse:
+    archive_id = item.archive_id if item.archive_id in visible_names else None
+    return BOMItemResponse(
+        id=item.id,
+        project_id=item.project_id,
+        name=item.name,
+        quantity_needed=item.quantity_needed,
+        quantity_acquired=item.quantity_acquired,
+        unit_price=item.unit_price,
+        sourcing_url=item.sourcing_url,
+        archive_id=archive_id,
+        archive_name=visible_names[archive_id] if archive_id is not None else None,
+        stl_filename=item.stl_filename,
+        remarks=item.remarks,
+        sort_order=item.sort_order,
+        is_complete=item.quantity_acquired >= item.quantity_needed,
+        created_at=item.created_at,
+        updated_at=item.updated_at,
+    )
+
+
 @router.get("/{project_id}/bom", response_model=list[BOMItemResponse])
 async def list_bom_items(
     project_id: int,
     db: AsyncSession = Depends(get_db),
     _: User | None = RequirePermissionIfAuthEnabled(Permission.PROJECTS_READ),
+    printer_scope: PrinterScope = RequestPrinterScope,
 ):
     """List all BOM items for a project."""
     # Verify project exists
@@ -1597,35 +1640,8 @@ async def list_bom_items(
     )
     items = result.scalars().all()
 
-    response = []
-    for item in items:
-        # Get archive name if linked
-        archive_name = None
-        if item.archive_id:
-            archive_result = await db.execute(select(PrintArchive.print_name).where(PrintArchive.id == item.archive_id))
-            archive_name = archive_result.scalar()
-
-        response.append(
-            BOMItemResponse(
-                id=item.id,
-                project_id=item.project_id,
-                name=item.name,
-                quantity_needed=item.quantity_needed,
-                quantity_acquired=item.quantity_acquired,
-                unit_price=item.unit_price,
-                sourcing_url=item.sourcing_url,
-                archive_id=item.archive_id,
-                archive_name=archive_name,
-                stl_filename=item.stl_filename,
-                remarks=item.remarks,
-                sort_order=item.sort_order,
-                is_complete=item.quantity_acquired >= item.quantity_needed,
-                created_at=item.created_at,
-                updated_at=item.updated_at,
-            )
-        )
-
-    return response
+    names = await _visible_archive_names(db, [item.archive_id for item in items], printer_scope)
+    return [_bom_response(item, names) for item in items]
 
 
 @router.post("/{project_id}/bom", response_model=BOMItemResponse)
@@ -1634,6 +1650,7 @@ async def create_bom_item(
     data: BOMItemCreate,
     db: AsyncSession = Depends(get_db),
     _: User | None = RequirePermissionIfAuthEnabled(Permission.PROJECTS_UPDATE),
+    printer_scope: PrinterScope = RequestPrinterScope,
 ):
     """Add a BOM item to a project."""
     # Verify project exists
@@ -1641,6 +1658,11 @@ async def create_bom_item(
     if not result.scalar_one_or_none():
         raise HTTPException(status_code=404, detail="Project not found")
 
+    # 0 means no archive, as on update.
+    archive_id = data.archive_id or None
+    if archive_id is not None:
+        await _ensure_linkable_archive(db, archive_id, printer_scope)
+
     # Get max sort order
     max_order_result = await db.execute(
         select(func.max(ProjectBOMItem.sort_order)).where(ProjectBOMItem.project_id == project_id)
@@ -1653,7 +1675,7 @@ async def create_bom_item(
         quantity_needed=data.quantity_needed,
         unit_price=data.unit_price,
         sourcing_url=data.sourcing_url,
-        archive_id=data.archive_id,
+        archive_id=archive_id,
         stl_filename=data.stl_filename,
         remarks=data.remarks,
         sort_order=max_order + 1,
@@ -1662,29 +1684,7 @@ async def create_bom_item(
     await db.flush()
     await db.refresh(item)
 
-    # Get archive name if linked
-    archive_name = None
-    if item.archive_id:
-        archive_result = await db.execute(select(PrintArchive.print_name).where(PrintArchive.id == item.archive_id))
-        archive_name = archive_result.scalar()
-
-    return BOMItemResponse(
-        id=item.id,
-        project_id=item.project_id,
-        name=item.name,
-        quantity_needed=item.quantity_needed,
-        quantity_acquired=item.quantity_acquired,
-        unit_price=item.unit_price,
-        sourcing_url=item.sourcing_url,
-        archive_id=item.archive_id,
-        archive_name=archive_name,
-        stl_filename=item.stl_filename,
-        remarks=item.remarks,
-        sort_order=item.sort_order,
-        is_complete=item.quantity_acquired >= item.quantity_needed,
-        created_at=item.created_at,
-        updated_at=item.updated_at,
-    )
+    return _bom_response(item, await _visible_archive_names(db, [item.archive_id], printer_scope))
 
 
 @router.patch("/{project_id}/bom/{item_id}", response_model=BOMItemResponse)
@@ -1694,6 +1694,7 @@ async def update_bom_item(
     data: BOMItemUpdate,
     db: AsyncSession = Depends(get_db),
     _: User | None = RequirePermissionIfAuthEnabled(Permission.PROJECTS_UPDATE),
+    printer_scope: PrinterScope = RequestPrinterScope,
 ):
     """Update a BOM item."""
     result = await db.execute(
@@ -1718,6 +1719,10 @@ async def update_bom_item(
     if data.sourcing_url is not None:
         item.sourcing_url = data.sourcing_url if data.sourcing_url else None
     if data.archive_id is not None:
+        # Also when unchanged: a hidden current link must answer as a missing one,
+        # or resending its id would confirm it.
+        if data.archive_id != 0:
+            await _ensure_linkable_archive(db, data.archive_id, printer_scope)
         item.archive_id = data.archive_id if data.archive_id != 0 else None
     if data.stl_filename is not None:
         item.stl_filename = data.stl_filename if data.stl_filename else None
@@ -1727,29 +1732,7 @@ async def update_bom_item(
     await db.flush()
     await db.refresh(item)
 
-    # Get archive name if linked
-    archive_name = None
-    if item.archive_id:
-        archive_result = await db.execute(select(PrintArchive.print_name).where(PrintArchive.id == item.archive_id))
-        archive_name = archive_result.scalar()
-
-    return BOMItemResponse(
-        id=item.id,
-        project_id=item.project_id,
-        name=item.name,
-        quantity_needed=item.quantity_needed,
-        quantity_acquired=item.quantity_acquired,
-        unit_price=item.unit_price,
-        sourcing_url=item.sourcing_url,
-        archive_id=item.archive_id,
-        archive_name=archive_name,
-        stl_filename=item.stl_filename,
-        remarks=item.remarks,
-        sort_order=item.sort_order,
-        is_complete=item.quantity_acquired >= item.quantity_needed,
-        created_at=item.created_at,
-        updated_at=item.updated_at,
-    )
+    return _bom_response(item, await _visible_archive_names(db, [item.archive_id], printer_scope))
 
 
 @router.delete("/{project_id}/bom/{item_id}")

+ 154 - 0
backend/tests/integration/test_project_bom_archive_link.py

@@ -0,0 +1,154 @@
+"""A BOM line links only to an archive the caller can see, and shows no other.
+
+The BOM routes stored any archive id and answered with that archive's print
+name, looked up by id alone. A member of a group limited to some printers
+could read the name of any archive (404 when asked for directly) by linking a
+BOM line to it, and every reader of the project saw the names of archives
+from printers they have no access to. An unknown id was stored as well (a
+server error on PostgreSQL).
+"""
+
+from __future__ import annotations
+
+import pytest
+from httpx import AsyncClient
+
+from backend.tests.integration.test_printer_scope_1727 import TEAM_PERMISSIONS, _admin_token, _auth, _group, _user
+
+pytestmark = [pytest.mark.asyncio, pytest.mark.integration]
+
+
+async def _member(client: AsyncClient, admin: str, printer_ids: list[int]) -> dict[str, str]:
+    perms = await _group(client, admin, "perms", permissions=[*TEAM_PERMISSIONS, "projects:read", "projects:update"])
+    team = await _group(client, admin, "team", printer_ids=printer_ids)
+    return _auth((await _user(client, admin, "member", [perms, team]))[0])
+
+
+async def _project(client: AsyncClient, headers: dict[str, str] | None = None) -> int:
+    response = await client.post("/api/v1/projects/", headers=headers or {}, json={"name": "P"})
+    assert response.status_code == 200, response.text
+    return response.json()["id"]
+
+
+async def test_a_member_cannot_link_an_archive_of_another_printer(
+    async_client: AsyncClient, printer_factory, archive_factory
+):
+    a, b = await printer_factory(name="A"), await printer_factory(name="B")
+    admin = await _admin_token(async_client)
+    member = await _member(async_client, admin, [a.id])
+    project = await _project(async_client, _auth(admin))
+    hidden = await archive_factory(b.id, print_name="team-b")
+
+    created = await async_client.post(
+        f"/api/v1/projects/{project}/bom", headers=member, json={"name": "x", "archive_id": hidden.id}
+    )
+    line = await async_client.post(f"/api/v1/projects/{project}/bom", headers=member, json={"name": "y"})
+    updated = await async_client.patch(
+        f"/api/v1/projects/{project}/bom/{line.json()['id']}", headers=member, json={"archive_id": hidden.id}
+    )
+
+    assert created.status_code == 400, created.text
+    assert created.json()["detail"] == "Archive not found"
+    assert updated.status_code == 400, updated.text
+    assert updated.json()["detail"] == "Archive not found"
+    listed = (await async_client.get(f"/api/v1/projects/{project}/bom", headers=_auth(admin))).json()
+    assert [(i["name"], i["archive_id"]) for i in listed] == [("y", None)]
+
+
+async def test_a_hidden_archive_linked_by_an_admin_reads_as_missing(
+    async_client: AsyncClient, printer_factory, archive_factory
+):
+    a, b = await printer_factory(name="A"), await printer_factory(name="B")
+    admin = await _admin_token(async_client)
+    member = await _member(async_client, admin, [a.id])
+    project = await _project(async_client, _auth(admin))
+    hidden, visible = await archive_factory(b.id, print_name="team-b"), await archive_factory(a.id, print_name="ours")
+    for name, archive in (("hidden", hidden), ("visible", visible)):
+        response = await async_client.post(
+            f"/api/v1/projects/{project}/bom", headers=_auth(admin), json={"name": name, "archive_id": archive.id}
+        )
+        assert response.json()["archive_name"] == archive.print_name
+
+    as_member = (await async_client.get(f"/api/v1/projects/{project}/bom", headers=member)).json()
+    as_admin = (await async_client.get(f"/api/v1/projects/{project}/bom", headers=_auth(admin))).json()
+
+    assert [(i["name"], i["archive_id"], i["archive_name"]) for i in as_member] == [
+        ("hidden", None, None),
+        ("visible", visible.id, "ours"),
+    ]
+    assert [i["archive_name"] for i in as_admin] == ["team-b", "ours"]
+
+
+async def test_editing_a_line_shows_no_hidden_archive(async_client: AsyncClient, printer_factory, archive_factory):
+    a, b = await printer_factory(name="A"), await printer_factory(name="B")
+    admin = await _admin_token(async_client)
+    member = await _member(async_client, admin, [a.id])
+    project = await _project(async_client, _auth(admin))
+    hidden = await archive_factory(b.id, print_name="team-b")
+    line = await async_client.post(
+        f"/api/v1/projects/{project}/bom", headers=_auth(admin), json={"name": "x", "archive_id": hidden.id}
+    )
+
+    response = await async_client.patch(
+        f"/api/v1/projects/{project}/bom/{line.json()['id']}", headers=member, json={"quantity_acquired": 1}
+    )
+
+    assert response.status_code == 200, response.text
+    assert (response.json()["archive_id"], response.json()["archive_name"]) == (None, None)
+    # The link itself stays: the member only does not see it.
+    as_admin = (await async_client.get(f"/api/v1/projects/{project}/bom", headers=_auth(admin))).json()
+    assert as_admin[0]["archive_id"] == hidden.id
+
+
+@pytest.mark.parametrize("archive_id", [9999])
+async def test_an_unknown_archive_is_refused(async_client: AsyncClient, archive_id):
+    project = await _project(async_client)
+
+    response = await async_client.post(f"/api/v1/projects/{project}/bom", json={"name": "x", "archive_id": archive_id})
+
+    assert response.status_code == 400, response.text
+    assert response.json()["detail"] == "Archive not found"
+
+
+async def test_archive_0_means_no_archive(async_client: AsyncClient):
+    project = await _project(async_client)
+
+    response = await async_client.post(f"/api/v1/projects/{project}/bom", json={"name": "x", "archive_id": 0})
+
+    assert response.status_code == 200, response.text
+    assert response.json()["archive_id"] is None
+
+
+async def test_a_visible_archive_can_be_linked_and_cleared(async_client: AsyncClient, printer_factory, archive_factory):
+    a = await printer_factory(name="A")
+    project = await _project(async_client)
+    archive = await archive_factory(a.id, print_name="ours")
+
+    created = await async_client.post(f"/api/v1/projects/{project}/bom", json={"name": "x", "archive_id": archive.id})
+    cleared = await async_client.patch(f"/api/v1/projects/{project}/bom/{created.json()['id']}", json={"archive_id": 0})
+
+    assert (created.json()["archive_id"], created.json()["archive_name"]) == (archive.id, "ours")
+    assert (cleared.json()["archive_id"], cleared.json()["archive_name"]) == (None, None)
+
+
+async def test_resending_a_hidden_link_answers_as_a_missing_archive(
+    async_client: AsyncClient, printer_factory, archive_factory
+):
+    # Otherwise 200 for the hidden current id against 400 for any other id would
+    # tell the member which archive the line links to.
+    a, b = await printer_factory(name="A"), await printer_factory(name="B")
+    admin = await _admin_token(async_client)
+    member = await _member(async_client, admin, [a.id])
+    project = await _project(async_client, _auth(admin))
+    hidden = await archive_factory(b.id)
+    line = await async_client.post(
+        f"/api/v1/projects/{project}/bom", headers=_auth(admin), json={"name": "x", "archive_id": hidden.id}
+    )
+    url = f"/api/v1/projects/{project}/bom/{line.json()['id']}"
+
+    same = await async_client.patch(url, headers=member, json={"archive_id": hidden.id})
+    other = await async_client.patch(url, headers=member, json={"archive_id": 9999})
+
+    assert (
+        (same.status_code, same.json()) == (other.status_code, other.json()) == (400, {"detail": "Archive not found"})
+    )