Просмотр исходного кода

Restrict printer secrets to update-authority callers

  GET /api/v1/printers/ and /api/v1/printers/{id} return access_code
  only when the caller holds PRINTERS_UPDATE. Adds PrinterResponseWithSecret
  as the elevated response shape; PrinterResponse no longer carries the
  field. Auth-disabled single-trust mode preserved.
maziggy 2 месяцев назад
Родитель
Сommit
9a432f0050

+ 57 - 9
backend/app/api/routes/printers.py

@@ -8,7 +8,11 @@ from fastapi.responses import Response
 from sqlalchemy import func, select
 from sqlalchemy.ext.asyncio import AsyncSession
 
-from backend.app.core.auth import RequireCameraStreamTokenIfAuthEnabled, RequirePermissionIfAuthEnabled
+from backend.app.core.auth import (
+    RequireCameraStreamTokenIfAuthEnabled,
+    RequirePermissionIfAuthEnabled,
+    is_auth_enabled,
+)
 from backend.app.core.config import settings
 from backend.app.core.database import get_db
 from backend.app.core.permissions import Permission
@@ -16,6 +20,7 @@ from backend.app.core.tasks import spawn_background_task
 from backend.app.models.ams_label import AmsLabel
 from backend.app.models.printer import Printer
 from backend.app.models.slot_preset import SlotPresetMapping
+from backend.app.models.user import User
 from backend.app.schemas.printer import (
     AmsLabelBody,
     AMSTray,
@@ -28,6 +33,7 @@ from backend.app.schemas.printer import (
     PrinterCreate,
     PrinterDiagnosticResult,
     PrinterResponse,
+    PrinterResponseWithSecret,
     PrinterStatus,
     PrinterUpdate,
     PrintOptionsResponse,
@@ -55,14 +61,50 @@ logger = logging.getLogger(__name__)
 router = APIRouter(prefix="/printers", tags=["printers"])
 
 
-@router.get("/", response_model=list[PrinterResponse])
+async def _caller_can_view_printer_secrets(user: User | None, db: AsyncSession) -> bool:
+    """Whether the caller is trusted enough to see ``access_code`` on a printer
+    response. Fail-CLOSED: anything that isn't an authenticated user holding
+    PRINTERS_UPDATE returns False.
+
+    - Auth disabled  → True (single trust domain — same as today's local UI).
+    - JWT user with PRINTERS_UPDATE → True (Admin or Operator; the same roles
+      that already manage printers and the Virtual Printer card UX that
+      surfaces a target's code for slicer configuration).
+    - JWT Viewer → False (the bug fix: Viewers must not be able to read
+      access_code via PRINTERS_READ and then go around Bambuddy to MQTT).
+    - API-key principal (``user is None`` because the dep returns None for
+      API keys) → False. PRINTERS_UPDATE is admin-only and absent from
+      ``_APIKEY_SCOPE_BY_PERMISSION``, so no API key can hold it.
+    """
+    if not await is_auth_enabled(db):
+        return True
+    if user is None:
+        return False
+    return user.has_permission(Permission.PRINTERS_UPDATE.value)
+
+
+def _serialize_printer(printer: Printer, *, include_secret: bool):
+    """Build the response shape that matches the caller's authority."""
+    if include_secret:
+        return PrinterResponseWithSecret.model_validate(printer)
+    return PrinterResponse.model_validate(printer)
+
+
+@router.get("/")
 async def list_printers(
-    _=RequirePermissionIfAuthEnabled(Permission.PRINTERS_READ),
+    user: User | None = RequirePermissionIfAuthEnabled(Permission.PRINTERS_READ),
     db: AsyncSession = Depends(get_db),
 ):
-    """List all configured printers."""
+    """List all configured printers.
+
+    ``access_code`` is included in each item only when the caller is trusted
+    to see it (Admin / Operator JWT, or auth-disabled mode). Viewers and
+    API keys never receive it.
+    """
     result = await db.execute(select(Printer).order_by(Printer.name))
-    return list(result.scalars().all())
+    printers = list(result.scalars().all())
+    include_secret = await _caller_can_view_printer_secrets(user, db)
+    return [_serialize_printer(p, include_secret=include_secret) for p in printers]
 
 
 @router.post("/", response_model=PrinterResponse)
@@ -262,18 +304,24 @@ async def get_developer_mode_warnings(
     return warnings
 
 
-@router.get("/{printer_id}", response_model=PrinterResponse)
+@router.get("/{printer_id}")
 async def get_printer(
     printer_id: int,
-    _=RequirePermissionIfAuthEnabled(Permission.PRINTERS_READ),
+    user: User | None = RequirePermissionIfAuthEnabled(Permission.PRINTERS_READ),
     db: AsyncSession = Depends(get_db),
 ):
-    """Get a specific printer."""
+    """Get a specific printer.
+
+    ``access_code`` is included only when the caller is trusted to see it
+    (Admin / Operator JWT, or auth-disabled mode). Viewers and API keys
+    never receive it.
+    """
     result = await db.execute(select(Printer).where(Printer.id == printer_id))
     printer = result.scalar_one_or_none()
     if not printer:
         raise HTTPException(404, "Printer not found")
-    return printer
+    include_secret = await _caller_can_view_printer_secrets(user, db)
+    return _serialize_printer(printer, include_secret=include_secret)
 
 
 @router.patch("/{printer_id}", response_model=PrinterResponse)

+ 16 - 3
backend/app/schemas/printer.py

@@ -29,7 +29,6 @@ class PrinterBase(BaseModel):
         max_length=253,
         pattern=r"^(\d{1,3}(\.\d{1,3}){3}|[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?)*)$",
     )
-    access_code: str = Field(..., min_length=1, max_length=20)
     model: str | None = None
     location: str | None = None  # Group/location name
     auto_archive: bool = True
@@ -41,7 +40,10 @@ class PrinterBase(BaseModel):
 
 
 class PrinterCreate(PrinterBase):
-    pass
+    # access_code lives on the input shapes only — never on the default
+    # PrinterResponse. Direct exposure on PRINTERS_READ would let a Viewer
+    # connect to the printer's MQTT and bypass Bambuddy's RBAC.
+    access_code: str = Field(..., min_length=1, max_length=20)
 
 
 class PlateDetectionROI(BaseModel):
@@ -101,7 +103,6 @@ class PrinterResponse(PrinterBase):
             "name": printer.name,
             "serial_number": printer.serial_number,
             "ip_address": printer.ip_address,
-            "access_code": printer.access_code,
             "model": printer.model,
             "location": printer.location,
             "auto_archive": printer.auto_archive,
@@ -135,6 +136,18 @@ class PrinterResponse(PrinterBase):
         return cls(**data)
 
 
+class PrinterResponseWithSecret(PrinterResponse):
+    """PrinterResponse + access_code. Returned ONLY to callers with
+    PRINTERS_UPDATE (Admin / Operator JWTs, or single-trust auth-disabled mode).
+
+    Viewers and API keys never receive this shape — they get the bare
+    PrinterResponse without access_code, since holding the access_code lets
+    the caller talk to the printer's MQTT directly and bypass Bambuddy's RBAC.
+    """
+
+    access_code: str
+
+
 class HMSErrorResponse(BaseModel):
     code: str
     attr: int = 0  # Attribute value for constructing wiki URL

+ 186 - 0
backend/tests/integration/test_printers_api.py

@@ -2997,3 +2997,189 @@ class TestConfigureAmsSlotPersistsKProfile:
         assert response.status_code == 200
         # MQTT was indeed called
         mock_client.extrusion_cali_sel.assert_called_once()
+
+
+class TestPrinterAccessCodeVisibility:
+    """Regression coverage: GET /printers and GET /printers/{id} must NOT
+    return ``access_code`` to callers without PRINTERS_UPDATE authority.
+
+    Holding ``access_code`` lets the caller talk to the printer's MQTT
+    directly with serial+code, bypassing every PRINTERS_CONTROL /
+    PRINTERS_FILES / PRINTERS_AMS_RFID check Bambuddy enforces.
+
+    Trust matrix encoded here:
+      - Auth disabled                  → access_code visible (single-trust mode)
+      - JWT Admin                      → access_code visible
+      - JWT Operator (has *_UPDATE)    → access_code visible (VP-card UX)
+      - JWT Viewer                     → access_code STRIPPED
+      - API key with can_read_status   → access_code STRIPPED
+    """
+
+    @pytest.fixture
+    async def auth_setup(self, async_client: AsyncClient):
+        await async_client.post(
+            "/api/v1/auth/setup",
+            json={
+                "auth_enabled": True,
+                "admin_username": "pcadmin",
+                "admin_password": "AdminPass1!",
+            },
+        )
+
+        async def _login(username, password):
+            resp = await async_client.post(
+                "/api/v1/auth/login",
+                json={"username": username, "password": password},
+            )
+            return resp.json()["access_token"]
+
+        admin_token = await _login("pcadmin", "AdminPass1!")
+
+        groups = (
+            await async_client.get(
+                "/api/v1/groups/",
+                headers={"Authorization": f"Bearer {admin_token}"},
+            )
+        ).json()
+        operators_group = next(g for g in groups if g["name"] == "Operators")
+        viewers_group = next(g for g in groups if g["name"] == "Viewers")
+
+        for username, password, group in (
+            ("pcoperator", "Operpass1!", operators_group["id"]),
+            ("pcviewer", "Viewpass1!", viewers_group["id"]),
+        ):
+            await async_client.post(
+                "/api/v1/users/",
+                headers={"Authorization": f"Bearer {admin_token}"},
+                json={"username": username, "password": password, "group_ids": [group]},
+            )
+
+        operator_token = await _login("pcoperator", "Operpass1!")
+        viewer_token = await _login("pcviewer", "Viewpass1!")
+
+        return {
+            "admin_token": admin_token,
+            "operator_token": operator_token,
+            "viewer_token": viewer_token,
+        }
+
+    async def _seed_printer_with_known_code(self, async_client: AsyncClient, admin_token: str) -> int:
+        resp = await async_client.post(
+            "/api/v1/printers/",
+            headers={"Authorization": f"Bearer {admin_token}"},
+            json={
+                "name": "AC-Visibility",
+                "serial_number": "00M09AVISIBILITY",
+                "ip_address": "192.168.42.42",
+                "access_code": "SECRET-CODE",
+                "is_active": True,
+                "model": "X1C",
+            },
+        )
+        assert resp.status_code == 200, resp.text
+        return resp.json()["id"]
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_auth_disabled_includes_access_code(self, async_client: AsyncClient, printer_factory):
+        """Single-trust mode: behaviour preserved, code is visible."""
+        printer = await printer_factory(name="AuthOff", access_code="LOCAL-CODE")
+
+        list_resp = await async_client.get("/api/v1/printers/")
+        detail_resp = await async_client.get(f"/api/v1/printers/{printer.id}")
+
+        assert list_resp.status_code == 200
+        assert detail_resp.status_code == 200
+        match = next(p for p in list_resp.json() if p["id"] == printer.id)
+        assert match["access_code"] == "LOCAL-CODE"
+        assert detail_resp.json()["access_code"] == "LOCAL-CODE"
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_admin_jwt_includes_access_code(self, async_client: AsyncClient, auth_setup):
+        printer_id = await self._seed_printer_with_known_code(async_client, auth_setup["admin_token"])
+        headers = {"Authorization": f"Bearer {auth_setup['admin_token']}"}
+
+        list_resp = await async_client.get("/api/v1/printers/", headers=headers)
+        detail_resp = await async_client.get(f"/api/v1/printers/{printer_id}", headers=headers)
+
+        assert list_resp.status_code == 200
+        assert detail_resp.status_code == 200
+        match = next(p for p in list_resp.json() if p["id"] == printer_id)
+        assert match["access_code"] == "SECRET-CODE"
+        assert detail_resp.json()["access_code"] == "SECRET-CODE"
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_operator_jwt_includes_access_code(self, async_client: AsyncClient, auth_setup):
+        """Operators hold PRINTERS_UPDATE (default role) — the VP-card UX
+        surfaces the target printer's access_code so they can configure
+        their slicer. The visibility predicate must keep working for them.
+        """
+        printer_id = await self._seed_printer_with_known_code(async_client, auth_setup["admin_token"])
+        headers = {"Authorization": f"Bearer {auth_setup['operator_token']}"}
+
+        list_resp = await async_client.get("/api/v1/printers/", headers=headers)
+        detail_resp = await async_client.get(f"/api/v1/printers/{printer_id}", headers=headers)
+
+        assert list_resp.status_code == 200
+        assert detail_resp.status_code == 200
+        match = next(p for p in list_resp.json() if p["id"] == printer_id)
+        assert match["access_code"] == "SECRET-CODE"
+        assert detail_resp.json()["access_code"] == "SECRET-CODE"
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_viewer_jwt_excludes_access_code(self, async_client: AsyncClient, auth_setup):
+        """The fix: Viewers hold PRINTERS_READ but not PRINTERS_UPDATE, and
+        must NOT be able to read the printer's secret.
+        """
+        printer_id = await self._seed_printer_with_known_code(async_client, auth_setup["admin_token"])
+        headers = {"Authorization": f"Bearer {auth_setup['viewer_token']}"}
+
+        list_resp = await async_client.get("/api/v1/printers/", headers=headers)
+        detail_resp = await async_client.get(f"/api/v1/printers/{printer_id}", headers=headers)
+
+        assert list_resp.status_code == 200
+        assert detail_resp.status_code == 200
+        match = next(p for p in list_resp.json() if p["id"] == printer_id)
+        # Field absent OR null — both are acceptable (no usable secret reaches the wire).
+        assert "access_code" not in match or match["access_code"] is None
+        body = detail_resp.json()
+        assert "access_code" not in body or body["access_code"] is None
+        # And the rest of the payload still arrives so the UI keeps working.
+        assert match["name"] == "AC-Visibility"
+        assert body["name"] == "AC-Visibility"
+
+    @pytest.mark.asyncio
+    @pytest.mark.integration
+    async def test_api_key_excludes_access_code(self, async_client: AsyncClient, auth_setup, db_session):
+        """API keys with can_read_status hold PRINTERS_READ but the predicate
+        gates on PRINTERS_UPDATE (admin-only / API-key-unmapped). The key
+        must NOT be able to exfiltrate access_code.
+        """
+        from backend.app.core.auth import generate_api_key
+        from backend.app.models.api_key import APIKey
+
+        printer_id = await self._seed_printer_with_known_code(async_client, auth_setup["admin_token"])
+
+        full_key, key_hash, key_prefix = generate_api_key()
+        api_key = APIKey(
+            name="visibility-key",
+            key_hash=key_hash,
+            key_prefix=key_prefix,
+            can_read_status=True,
+            enabled=True,
+        )
+        db_session.add(api_key)
+        await db_session.commit()
+
+        list_resp = await async_client.get("/api/v1/printers/", headers={"X-API-Key": full_key})
+        detail_resp = await async_client.get(f"/api/v1/printers/{printer_id}", headers={"X-API-Key": full_key})
+
+        assert list_resp.status_code == 200
+        assert detail_resp.status_code == 200
+        match = next(p for p in list_resp.json() if p["id"] == printer_id)
+        assert "access_code" not in match or match["access_code"] is None
+        body = detail_resp.json()
+        assert "access_code" not in body or body["access_code"] is None

+ 4 - 1
frontend/src/api/client.ts

@@ -308,7 +308,10 @@ export interface Printer {
   name: string;
   serial_number: string;
   ip_address: string;
-  access_code: string;
+  // Optional because the backend only returns access_code when the caller has
+  // PRINTERS_UPDATE — Admin / Operator JWTs or auth-disabled mode. Viewers and
+  // API keys receive a Printer without this field.
+  access_code?: string;
   model: string | null;
   location: string | null;  // Group/location name
   nozzle_count: number;  // 1 or 2, auto-detected from MQTT