diff --git a/NOTEPAD.md b/NOTEPAD.md index 032c64a..44a0cc4 100644 --- a/NOTEPAD.md +++ b/NOTEPAD.md @@ -186,7 +186,7 @@ as an em dash. - CHORE/UI: `ROW_HEIGHT` is a fixed 80px while column width follows the canvas, so a 1920-wide panel at 12 columns has 160×80 cells. If that reads too wide, the row height could derive from the canvas too. - CHORE/UI: multi-page and multi-section dashboards still have no UI, and now need none — a panel carries several whole dashboards instead, each with its own canvas and its own publish. `PageDef`/`SectionDef` stay in the schema and the editor still edits `sectionsOf(page)[0]`, so the page `Tabs` in `DashboardEditor` are dead until something writes a second page through the API. - FEAT/UI: a panel does not notice being reassigned until it is reloaded — nothing pushes the panel document or a dashboard publish, so the rail is as stale as the last read. Same gap as the wallpanel hot-reload item above; one event on the bus would answer both. -- CHORE/API: a panel credential may publish *any* message, not only the ones its own widgets bind to — the allowlist is the `/messages/` prefix rather than a walk of the panel's widgets. Enough for a screen in a house; an installation where a panel sits somewhere less trusted would want the narrower check. +- CHORE/API: a panel credential may publish *any* message, not only the ones its own widgets bind to — the allowlist is the `/messages/` prefix rather than a walk of the panel's widgets. The walk now exists: `panels.messages_for()` is what bounds the live socket. Pointing `_panel_may` at it would close this too, but it tightens what already-paired screens may do, so it wants a deliberate look at the query-chart request path first. - CHORE/API: unpairing a device means deleting the panel. A per-panel nonce in the token, bumped on demand, would let one screen be re-paired without disturbing the assignment. - CHORE/API: a panel paired through the portal is revoked here the moment the panel is deleted — `_panel_may` finds nothing and answers 401 — but the hub's copy of the token stays valid until it expires or the installation's generation counter is bumped ("New code"). The hub has no per-panel revocation, and giving it one means telling it which panels exist, which is exactly what this design avoids. The generation bump is the lever; it is blunt, cutting every credential the portal minted for the installation. - CHORE/UI: the device line under a pairing code is the raw user agent plus the address the request came from. Both are self-reported and neither is proof; it is there so an admin can tell the screen they just hung from one they were not expecting, not to authenticate anything. diff --git a/backend/app/api/deps.py b/backend/app/api/deps.py index 3356313..5596dcf 100644 --- a/backend/app/api/deps.py +++ b/backend/app/api/deps.py @@ -142,13 +142,18 @@ def _gate_panel(payload: dict[str, Any], request: Request | None) -> dict[str, A return payload -def user_from_token(session: Session, token: str) -> User | None: +def user_from_token( + session: Session, token: str, request: Request | None = None +) -> User | None: """Resolve a bearer token to its user, or None if it does not hold up. - Shared with the websocket, which cannot use the HTTP security scheme. + Shared with the websocket, which cannot use the HTTP security scheme, and + with the artifact endpoint, which accepts a worker's credential as well as + a person's. Pass the request wherever there is one: a panel's credential is + scoped by route, and without it the scope check cannot run. """ try: - token_data = TokenPayload(**decode_token(token)) + token_data = TokenPayload(**decode_token(token, request)) except (InvalidTokenError, ValidationError): return None user = session.get(User, token_data.sub) diff --git a/backend/app/api/routes/artifacts.py b/backend/app/api/routes/artifacts.py index 64ea46f..874318a 100644 --- a/backend/app/api/routes/artifacts.py +++ b/backend/app/api/routes/artifacts.py @@ -38,7 +38,9 @@ def artifact_caller(request: Request) -> str: else: return f"worker:{claims.get('sub')}" with Session(engine) as session: - user = user_from_token(session, token) + # With the request, so a credential that is scoped by route — a wall + # panel's — is judged against this one rather than waved through. + user = user_from_token(session, token, request) if user is None: raise HTTPException(status_code=401, detail="Not authenticated") return user.email diff --git a/backend/app/api/routes/flows.py b/backend/app/api/routes/flows.py index 916a85e..cae0d94 100644 --- a/backend/app/api/routes/flows.py +++ b/backend/app/api/routes/flows.py @@ -13,12 +13,14 @@ from fastapi import ( WebSocketDisconnect, ) from fastapi.concurrency import run_in_threadpool +from jwt.exceptions import InvalidTokenError from pydantic import BaseModel from sqlmodel import Session from app.api.deps import ( CurrentUser, FlowControllerDep, + decode_token, get_current_user, user_from_token, ) @@ -27,6 +29,7 @@ from app.flow.controller import FlowController from app.flow.dashboards import DashboardStore from app.flow.events import event_bus from app.flow.messages import qualify +from app.flow.panels import messages_for from app.flow.pipeline import ValidationIssue from app.flow.runs import RunRejected from app.flow.schemas import ( @@ -730,14 +733,34 @@ def read_message_history( # ----------------------------------------------------------------------------- -def snapshot_payload(controller: FlowController) -> dict[str, Any]: +def snapshot_payload( + controller: FlowController, only: set[str] | None = None +) -> dict[str, Any]: """Everything a client needs to catch up, sent the moment it connects. Also sent by the tunnel connector, which serves this websocket inline: the two have to agree, so they build the message here rather than each their own. ``emits`` is what the bus counted while nobody was listening — a client that reconnects between two pages would otherwise start from zero. + + ``only`` bounds it to a set of message names, which is what a wall panel + gets: the values its own dashboards draw, and none of the rest — no node + status, no logs, no shape of the graph. A panel renders none of that, and + a screen may be hanging somewhere nobody here can see. """ + if only is not None: + # The same keys, emptied rather than dropped: a screen already hanging + # runs whatever bundle it was paired with, and the shape of this + # message is what that bundle reads. + return { + "type": "snapshot", + "values": {k: v for k, v in controller.values().items() if k in only}, + "nodes": [], + "issues": [], + "paused": [], + "logs": [], + "emits": {}, + } return { "type": "snapshot", "values": controller.values(), @@ -749,6 +772,32 @@ def snapshot_payload(controller: FlowController) -> dict[str, Any]: } +def panel_scope(token: str, app: Any) -> set[str] | None: + """The messages this credential is bounded to, or None if it is a person's. + + The socket is the one authenticated surface the route check cannot reach — + a handshake has no route to judge — so a panel is bounded by what it is + sent instead of by what it asks for. + """ + try: + panel = str(decode_token(token).get("panel") or "") + except InvalidTokenError: + return None + if not panel: + return None + store: DashboardStore | None = getattr(app.state, "dashboard_store", None) + return messages_for(panel, store) if store is not None else set() + + +def event_for_panel(event: dict[str, Any], only: set[str]) -> bool: + """Whether a panel's socket should carry this event. + + The same bound as the snapshot above, applied to the stream that follows + it: a value the panel draws, and nothing else on the bus. + """ + return event.get("type") == "message_value" and str(event.get("name") or "") in only + + @ws_router.websocket("/ws") async def flow_events(websocket: WebSocket, token: str = "") -> None: """Stream values, node status and execution events as they happen. @@ -761,6 +810,7 @@ async def flow_events(websocket: WebSocket, token: str = "") -> None: if user is None: await websocket.close(code=1008) return + only = panel_scope(token, websocket.app) await websocket.accept() @@ -768,7 +818,7 @@ async def flow_events(websocket: WebSocket, token: str = "") -> None: websocket.app.state, "flow_controller", None ) if controller is not None: - await websocket.send_json(snapshot_payload(controller)) + await websocket.send_json(snapshot_payload(controller, only)) async with event_bus.subscribe() as queue: receiver = asyncio.create_task(websocket.receive_text()) @@ -782,7 +832,10 @@ async def flow_events(websocket: WebSocket, token: str = "") -> None: # The client went away. sender.cancel() break - await websocket.send_json(sender.result()) + event = sender.result() + if only is not None and not event_for_panel(event, only): + continue + await websocket.send_json(event) except WebSocketDisconnect: pass finally: diff --git a/backend/app/api/routes/panels.py b/backend/app/api/routes/panels.py index fe284dc..780769e 100644 --- a/backend/app/api/routes/panels.py +++ b/backend/app/api/routes/panels.py @@ -236,7 +236,9 @@ def start_pairing(request: Request) -> Any: while code in _pending: code = "".join(secrets.choice(CODE_ALPHABET) for _ in range(CODE_LENGTH)) - remote = request.headers.get("x-fluksio-via") == "portal" + remote = secrets.compare_digest( + request.headers.get(cloud_config.VIA_HEADER, ""), cloud_config.VIA_PORTAL + ) entry = _Pending(secrets.token_urlsafe(16), _describe(request), remote) _pending[code] = entry return PairStarted(code=code, secret=entry.secret) diff --git a/backend/app/cloud/config.py b/backend/app/cloud/config.py index 0b42a3d..73d179e 100644 --- a/backend/app/cloud/config.py +++ b/backend/app/cloud/config.py @@ -17,6 +17,7 @@ from __future__ import annotations import json import logging import os +import secrets from dataclasses import dataclass from typing import Any @@ -27,6 +28,15 @@ from app.core.config import settings logger = logging.getLogger(__name__) +#: Marks a request the connector replayed off the tunnel. The value is minted +#: per process and never leaves it, because the header itself is not evidence: +#: anything that can reach this API directly can set one, and the difference +#: decides whether a pairing device is handed a credential the whole internet +#: can present. The connector overwrites it on every frame, so a browser +#: sending its own gets nowhere from either direction. +VIA_HEADER = "x-fluksio-via" +VIA_PORTAL = secrets.token_urlsafe(16) + @dataclass(frozen=True) class CloudConfig: diff --git a/backend/app/cloud/connector.py b/backend/app/cloud/connector.py index 59c7418..8f6d52e 100644 --- a/backend/app/cloud/connector.py +++ b/backend/app/cloud/connector.py @@ -222,8 +222,9 @@ class CloudConnector: # Set here rather than trusted from the frame: a browser can send # any header it likes through the proxy, and this one decides where # a pairing device's credential is minted. Arriving on this socket - # is the only thing that makes it true. - headers["x-fluksio-via"] = "portal" + # is the only thing that makes it true, and the value is this + # process's own so nothing off the network can imitate it. + headers[cloud_config.VIA_HEADER] = cloud_config.VIA_PORTAL async with self._client() as client: request = client.build_request( @@ -298,7 +299,11 @@ class CloudConnector: from sqlmodel import Session from app.api.deps import user_from_token - from app.api.routes.flows import snapshot_payload + from app.api.routes.flows import ( + event_for_panel, + panel_scope, + snapshot_payload, + ) from app.core.db import engine from app.flow.events import event_bus @@ -316,6 +321,7 @@ class CloudConnector: _dump({"op": "ws_close", "id": stream_id, "code": 1008}) ) return + only = panel_scope(token, self._app) await socket.send(_dump({"op": "ws_open_ok", "id": stream_id})) @@ -326,7 +332,7 @@ class CloudConnector: { "op": "ws_msg", "id": stream_id, - "text": _dump(snapshot_payload(controller)), + "text": _dump(snapshot_payload(controller, only)), } ) ) @@ -335,6 +341,8 @@ class CloudConnector: async with event_bus.subscribe() as queue: while True: event = await queue.get() + if only is not None and not event_for_panel(event, only): + continue await socket.send( _dump({"op": "ws_msg", "id": stream_id, "text": _dump(event)}) ) diff --git a/backend/app/flow/panels.py b/backend/app/flow/panels.py index 001cc1a..5667695 100644 --- a/backend/app/flow/panels.py +++ b/backend/app/flow/panels.py @@ -17,6 +17,7 @@ from pathlib import Path from pydantic import BaseModel, Field, field_validator from app.core.config import settings +from app.flow.dashboards import DashboardNotFound, DashboardStore from app.flow.schemas import _validate_name @@ -75,3 +76,26 @@ def find(panel_id: str) -> PanelDef | None: if panel.id == panel_id: return panel return None + + +def messages_for(panel_id: str, store: DashboardStore) -> set[str]: + """Every message the widgets of this panel's dashboards read or write. + + What a screen is entitled to see, as its own dashboards define it. Read + from the published documents, since that is what a panel draws, and empty + for a panel that is gone — which is the same answer as "nothing". + """ + panel = find(panel_id) + if panel is None: + return set() + names: set[str] = set() + for dashboard in panel.dashboards: + try: + defn = store.read(dashboard) + except DashboardNotFound: + continue + for widget in defn.widgets: + names.update(widget.messages) + if widget.target: + names.add(widget.target) + return names diff --git a/backend/tests/api/routes/test_panels.py b/backend/tests/api/routes/test_panels.py index 4c10ee1..d71d0c2 100644 --- a/backend/tests/api/routes/test_panels.py +++ b/backend/tests/api/routes/test_panels.py @@ -2,6 +2,7 @@ from fastapi.testclient import TestClient +from app.cloud import config as cloud_config from app.core.config import settings PREFIX = f"{settings.API_V1_STR}/panels" @@ -210,6 +211,11 @@ def test_removing_the_panel_revokes_its_credential( # -------------------------------------------------------------------------- +def _via_portal() -> dict[str, str]: + """The header the connector stamps on a request it replayed off the tunnel.""" + return {cloud_config.VIA_HEADER: cloud_config.VIA_PORTAL} + + def test_the_device_asking_is_named_before_anyone_approves( client: TestClient, superuser_token_headers: dict[str, str] ) -> None: @@ -269,7 +275,7 @@ def test_a_remote_device_is_paired_at_the_portal( monkeypatch.setattr(panels_route.httpx, "post", fake_post) _panels(client, superuser_token_headers, {"panels": [{"id": "hallway"}]}) - started = client.post(f"{PREFIX}/pair", headers={"x-fluksio-via": "portal"}).json() + started = client.post(f"{PREFIX}/pair", headers=_via_portal()).json() looked = client.get( f"{PREFIX}/pair/{started['code']}/device", headers=superuser_token_headers @@ -298,7 +304,7 @@ def test_a_remote_device_needs_an_enrolment( ) -> None: """Unenrolled, there is nowhere to ask — and no token to invent locally.""" _panels(client, superuser_token_headers, {"panels": [{"id": "shed"}]}) - started = client.post(f"{PREFIX}/pair", headers={"x-fluksio-via": "portal"}).json() + started = client.post(f"{PREFIX}/pair", headers=_via_portal()).json() approved = client.post( f"{PREFIX}/shed/pair", @@ -306,3 +312,129 @@ def test_a_remote_device_needs_an_enrolment( json={"code": started["code"]}, ) assert approved.status_code == 409 + + +def test_a_panel_credential_cannot_move_artifacts( + client: TestClient, superuser_token_headers: dict[str, str] +) -> None: + """The artifact store is a person's or a worker's, not a screen's. + + It authenticates for itself rather than through the shared dependency, so + the scope check has to be handed the request there too — without it a panel + would act as whoever approved it. + """ + client.post(f"{DASHBOARDS}/panel_art", headers=superuser_token_headers) + _panels( + client, + superuser_token_headers, + {"panels": [{"id": "hallway", "dashboards": ["panel_art"]}]}, + ) + panel_headers = _pair(client, superuser_token_headers, "hallway") + + assert ( + client.put( + f"{settings.API_V1_STR}/artifacts", + headers=panel_headers, + params={"name": "x"}, + content=b"hello", + ).status_code + == 403 + ) + + +def test_a_local_device_cannot_claim_to_be_remote( + client: TestClient, superuser_token_headers: dict[str, str] +) -> None: + """Otherwise it would be handed a credential the whole internet accepts.""" + started = client.post(f"{PREFIX}/pair", headers={"x-fluksio-via": "portal"}).json() + looked = client.get( + f"{PREFIX}/pair/{started['code']}/device", headers=superuser_token_headers + ) + assert looked.json()["remote"] is False + + +def test_a_panels_socket_carries_only_what_it_draws( + client: TestClient, superuser_token_headers: dict[str, str] +) -> None: + """A screen may hang anywhere, so it is sent its own values and nothing else. + + The handshake has no route for the scope check to judge, so the bound is + applied to what goes out: no node status, no logs, no shape of the graph, + and no value belonging to a dashboard this panel does not show. + """ + from app.api.routes.flows import ( + event_for_panel, + panel_scope, + snapshot_payload, + ) + + client.post(f"{DASHBOARDS}/panel_socket", headers=superuser_token_headers) + saved = client.get( + f"{DASHBOARDS}/panel_socket", headers=superuser_token_headers + ).json() + saved["pages"] = [ + { + "id": "main", + "title": "Overview", + "sections": [ + { + "id": "main", + "widgets": [ + { + "id": "w1", + "type": "stat", + "config": {"message": "house.kitchen.temperature"}, + } + ], + } + ], + } + ] + written = client.put( + f"{DASHBOARDS}/panel_socket", headers=superuser_token_headers, json=saved + ) + assert written.status_code == 200, written.text + # A panel reads the published document, so the draft has to be promoted. + published = client.post( + f"{DASHBOARDS}/panel_socket/publish", + headers=superuser_token_headers, + json={"version": written.json()["version"]}, + ) + assert published.status_code == 200, published.text + + _panels( + client, + superuser_token_headers, + {"panels": [{"id": "hallway", "dashboards": ["panel_socket"]}]}, + ) + token = _pair(client, superuser_token_headers, "hallway")["Authorization"][7:] + + only = panel_scope(token, client.app) + assert only == {"house.kitchen.temperature"} + + class _Controller: + def values(self) -> dict[str, dict[str, str]]: + return { + "house.kitchen.temperature": {"value": "21"}, + "house.safe.code": {"value": "1234"}, + } + + payload = snapshot_payload(_Controller(), only) # type: ignore[arg-type] + assert set(payload["values"]) == {"house.kitchen.temperature"} + # Emptied rather than dropped: a hanging screen runs the bundle it was + # paired with, and that bundle reads these keys. + assert payload["logs"] == [] and payload["nodes"] == [] + assert payload["issues"] == [] and payload["emits"] == {} + + # The stream that follows the snapshot is bounded the same way: a value it + # draws goes out, one it does not stays here, and nothing else travels. + assert event_for_panel( + {"type": "message_value", "name": "house.kitchen.temperature"}, only + ) + assert not event_for_panel( + {"type": "message_value", "name": "house.safe.code"}, only + ) + assert not event_for_panel({"type": "node_log", "text": "a traceback"}, only) + + # A person's credential is not bounded at all. + assert panel_scope(superuser_token_headers["Authorization"][7:], client.app) is None diff --git a/frontend/src/components/Flow/useFlowSocket.ts b/frontend/src/components/Flow/useFlowSocket.ts index f1f2d16..d1ad0de 100644 --- a/frontend/src/components/Flow/useFlowSocket.ts +++ b/frontend/src/components/Flow/useFlowSocket.ts @@ -153,7 +153,7 @@ function connect() { switch (message.type) { case "snapshot": liveStore.setValues(message.values) - liveStore.setStatuses(message.nodes) + liveStore.setStatuses(message.nodes ?? []) liveStore.setPausedFlows(message.paused ?? []) liveStore.setLogs(message.logs ?? []) // Missing against an installation older than this bundle; the graph