From c604cfdf3bd452ab989ac0220362cef03e6b31d8 Mon Sep 17 00:00:00 2001 From: Melvin Strobl Date: Fri, 16 Oct 2026 19:26:05 +0200 Subject: [PATCH] Device security and robustness (openpanel C0): check code, pnc, lenient report - POST /devices/pair answers a 2-character `check`, listed by /devices/pending and shown beside each waiting entry; five codes a minute per address, and at 50 waiting the oldest gives way instead of a 429 - portal-minted device tokens are asked for with the device's nonce (`pnc`) and held against it, so unpair and delete revoke them; one without `pnc` stands only while the nonce is 0 - a device credential acts as the device (`device_principal`), not as the approving superuser, so deleting that account unpairs nothing - the report is validated port by port: unknown enum values read as `unknown`, unreadable ports and sections are left out and logged, `config` may name an entry of `config_schemas`; 422 only for no schema 1 or no object - devices.json is parsed once per change (cached by stat), delete drops the report and keeps the nonce, a busy recheck runs in the background - scope and io only from enabled flows (start/stop tells the devices), a port bound twice gives one io row - the Devices tab's saves cancel and hold back device refetches until settled --- backend/fluksio/api/deps.py | 59 +++-- backend/fluksio/api/routes/devices.py | 111 +++++++-- backend/fluksio/cloud/config.py | 11 +- backend/fluksio/core/security.py | 12 +- backend/fluksio/flow/controller.py | 18 +- backend/fluksio/flow/devices.py | 221 +++++++++++++++-- backend/fluksio/models.py | 3 + backend/tests/api/routes/test_devices.py | 222 +++++++++++++++++- backend/tests/flow/test_device_store.py | 15 ++ backend/tests/test_cloud.py | 37 ++- backend/tests/utils/portal.py | 2 + docs/code/api.md | 8 +- docs/interface/dashboards.md | 56 +++-- frontend/src/client/schemas.gen.ts | 10 + frontend/src/client/sdk.gen.ts | 14 +- frontend/src/client/types.gen.ts | 2 + .../src/components/Devices/DeviceDetail.tsx | 21 +- frontend/src/components/Devices/ports.ts | 2 +- frontend/src/components/Devices/queries.ts | 39 ++- frontend/src/components/Flow/useFlowSocket.ts | 14 +- frontend/src/routes/panel.index.tsx | 15 ++ 21 files changed, 773 insertions(+), 119 deletions(-) diff --git a/backend/fluksio/api/deps.py b/backend/fluksio/api/deps.py index 0c061d8..f585e33 100644 --- a/backend/fluksio/api/deps.py +++ b/backend/fluksio/api/deps.py @@ -53,9 +53,10 @@ def _device_for(payload: dict[str, Any], app: Any) -> DeviceDef: effect at once. Or the device's nonce moved on, which revokes exactly one screen's and leaves the device and its dashboards alone. - The nonce is only held against a credential this instance signed. One the - portal minted for a remote screen carries none, the portal names the - device and nothing else, and is revoked at the hub instead. + The nonce is held against a credential the portal minted for a remote + screen as well, which carries the nonce this instance named when it asked + for it. One from an older portal carries none and reads as 0, so it + stands only until the device is first unpaired or deleted. The claim is still called ``panel``: tokens in circulation say it, and nothing pairs again for a rename. @@ -71,13 +72,12 @@ def _device_for(payload: dict[str, Any], app: Any) -> DeviceDef: device = store.find(str(payload.get("panel", ""))) if device is None: raise InvalidTokenError("This device no longer exists") - if payload.get("aud") == security.PANEL_AUDIENCE: - try: - nonce = store.nonce(device.id) - except (OSError, ValueError) as exc: - raise DeviceUnavailable(f"The device nonces are unreadable: {exc}") - if int(payload.get("pnc") or 0) != nonce: - raise InvalidTokenError("This device was paired again") + try: + nonce = store.nonce(device.id) + except (OSError, ValueError) as exc: + raise DeviceUnavailable(f"The device nonces are unreadable: {exc}") + if int(payload.get("pnc") or 0) != nonce: + raise InvalidTokenError("This device was paired again") presence.seen(device.id) return device @@ -189,8 +189,9 @@ def _device_digests(device: DeviceDef, request: Request) -> set[str]: def _device_may(payload: dict[str, Any], request: Request) -> None: """Refuse anything a wall panel has no business asking for. - A panel credential names the account that approved the pairing, so without - this it would be that person's session hanging on a wall. What a panel + A panel credential acts as the device itself, an account of no rights + (``device_principal``), and what that account may reach is decided here + and nowhere else. What a panel genuinely needs is small and worth writing out: the dashboards it was assigned, its own definition and report, the messages its own widgets and ports bind to, and the bytes those messages currently point at, for a tile @@ -214,9 +215,9 @@ def _device_may(payload: dict[str, Any], request: Request) -> None: The device is left on ``request.state`` for the route, which credits a value published on one of its ports to the node holding that port. - # ponytail: the device file, its nonce and the published dashboards are - # re-read per request. Cache them behind the store's revision if this - # shows up in a profile. + # ponytail: the device file and the published dashboards are re-read per + # request (the nonces are cached by the file's stat). Cache them behind + # the store's revision if this shows up in a profile. """ device = _device_for(payload, request.app) request.state.device = device @@ -402,9 +403,35 @@ def _gate_panel( return payload +#: Where the ids of device principals come from, so each device has its own. +_DEVICE_IDS = uuid.UUID("5e1f2a0c-6d3b-4c1e-9f0a-0d3c1e5b7a20") + + +def device_principal(device_id: str) -> User: + """The account a device credential acts as: the device, and no person. + + Not stored. A device's credential is checked against the device itself + (``_device_for``) and bounded by route (``_device_may``), so no account's + rights are in play, and deleting or deactivating the superuser who + approved the pairing leaves the device paired. Never a superuser, so a + route gated on one refuses it even past the scope check. + """ + return User( + id=uuid.uuid5(_DEVICE_IDS, device_id), + email=f"{device_id}@device.invalid", + hashed_password="", + full_name=device_id, + is_active=True, + is_superuser=False, + ) + + def _user_for(session: Session, token_data: TokenPayload) -> User | None: """The local account a payload names, by id or by portal identity. + A device credential, already checked by ``decode_token``, names the + device instead, which acts as itself. + A token the portal minted names a person on the portal, not a user here, so a mapping is what turns one into the other. There are two ways one comes to exist: a superuser typed their code in, which makes the account up front, @@ -418,6 +445,8 @@ def _user_for(session: Session, token_data: TokenPayload) -> User | None: the account it named, and a person dropped at the portal is not adopted again. """ + if token_data.panel: + return device_principal(token_data.panel) if token_data.portal_sub: user = session.exec( select(User).where(User.portal_sub == token_data.portal_sub) diff --git a/backend/fluksio/api/routes/devices.py b/backend/fluksio/api/routes/devices.py index 9193c74..59ae4a9 100644 --- a/backend/fluksio/api/routes/devices.py +++ b/backend/fluksio/api/routes/devices.py @@ -6,6 +6,10 @@ device's page to say which device it is, or picks the request from the list of devices waiting on this network. The device polls, collects the credential the approval minted, and never asks again. +A request from the list carries a two-character check that the device shows +beside its code, so whoever picks it can tell it is the screen in front of +them: behind some proxies every address looks private. + A screen hanging somewhere this instance is not reachable from does the same thing through the portal, which forwards the two pairing calls down the tunnel without a session and mints the credential itself when the approval @@ -16,7 +20,8 @@ The credential is scoped: ``fluksio.api.deps`` lets it reach the dashboards that device was assigned, the messages their widgets bind to, the messages the device nodes in published flows wire its own ports to, and nothing else. Deleting the device revokes it, and so does unpairing, which bumps the -device's nonce and leaves what it shows as it was. +device's nonce and leaves what it shows as it was. That holds for a credential +the portal minted too, which carries the nonce the device had when it paired. Once paired, a device says what it has with ``PUT /devices/{id}/report``: its board, its screen and the ports it serves, which the device node's ports are @@ -29,6 +34,8 @@ device has updated. from __future__ import annotations +import asyncio +import logging import secrets import time from contextlib import suppress @@ -47,6 +54,7 @@ from fluksio.api.deps import ( get_current_active_superuser, get_current_user, ) +from fluksio.api.routes.oauth import too_many from fluksio.cloud import config as cloud_config from fluksio.core import security from fluksio.core.config import settings @@ -57,16 +65,18 @@ from fluksio.flow.devices import ( DeviceExists, DeviceIo, DeviceNotFound, - DeviceReport, DeviceStore, is_lan, presence, + read_report, ) from fluksio.flow.events import event_bus from fluksio.flow.state import CONNECT_TIMEOUT_S, SOCKET_TIMEOUT_S from fluksio.flow.store import StaleVersion from fluksio.models import Message, User +logger = logging.getLogger(__name__) + #: Gated per route rather than on the router: the pairing endpoints are the #: only unauthenticated ones in the app, because a device with no credential is #: the whole point of them. @@ -84,10 +94,18 @@ PAIR_TTL = 600 #: No I/O/0/1: the code is read off a wall and typed on a keyboard. CODE_ALPHABET = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789" CODE_LENGTH = 6 +#: The check a device shows beside its code, to compare with the pending list. +CHECK_LENGTH = 2 -#: Refuse to grow past this. Codes are free to ask for and cost memory. +#: Never more waiting than this: codes are free to ask for and cost memory. +#: The oldest makes room for a new one, so a flood cannot lock a device out. MAX_PENDING = 50 +#: Codes one address may ask for per window. A device asks once per code, +#: every ten minutes, and again after a failure. +PAIR_PER_ADDRESS = 5 +PAIR_WINDOW_S = 60.0 + #: Ids a device cannot take: the routes beside ``/{device_id}``. RESERVED = {"pair", "pending"} @@ -107,6 +125,8 @@ class _Pending(BaseModel): #: Names the request in the pending list, so it can be adopted without #: the code being handed to anyone. request: str = "" + #: Shown by the device and beside its entry in the list. Not a secret. + check: str = "" #: Wall-clock rather than monotonic: an entry outlives the process that #: minted it. expires: float @@ -164,7 +184,11 @@ class _PendingStore: return f"{_PENDING_KEY}:{code}" def _codes(self) -> list[str]: - """The live codes, the expired ones dropped first.""" + """The live codes, oldest first, the expired ones dropped first. + + Oldest first because every code lives as long: the index is scored by + expiry, and the dictionary keeps the order codes were added in. + """ now = time.time() if self._redis is None: for code in [c for c, e in self._local.items() if e.expires < now]: @@ -173,6 +197,12 @@ class _PendingStore: self._redis.zremrangebyscore(_PENDING_INDEX, "-inf", now) return list(cast("list[str]", self._redis.zrange(_PENDING_INDEX, 0, -1))) + def make_room(self, room: int) -> None: + """Drop the oldest codes until fewer than ``room`` are left.""" + codes = self._codes() + for code in codes[: max(0, len(codes) - room + 1)]: + self.drop(code) + def count(self) -> int: return len(self._codes()) @@ -251,6 +281,9 @@ class PairStarted(BaseModel): #: Held by the device, never shown. Without it the code alone would let #: anyone who reads it off the wall collect the credential first. secret: str + #: Shown beside the code. The pending list shows it next to this request, + #: so whoever adopts it from there can tell it is this screen. + check: str = "" expires_in: int = PAIR_TTL @@ -275,6 +308,8 @@ class PendingDevice(BaseModel): #: What ``POST /devices/{id}/pair`` takes to adopt it without the code. request: str = "" + #: What the device shows beside its code, to compare before adopting it. + check: str = "" device: str name: str = "" board: str = "" @@ -289,6 +324,7 @@ class PendingDevice(BaseModel): def _pending_public(entry: _Pending) -> PendingDevice: return PendingDevice( request=entry.request, + check=entry.check, device=entry.device, name=entry.name, board=entry.board, @@ -356,13 +392,14 @@ def _describe(request: Request) -> str: return " ยท ".join(part for part in (agent or "Unknown device", address) if part) -def _mint_at_hub(device_id: str) -> str: +def _mint_at_hub(device_id: str, nonce: int) -> str: """Ask the portal for this device's credential. A device that arrived through the portal cannot reach this instance, so a token this instance signed would be one it could never present: the portal verifies what crosses its tunnel against its own key. It mints, we say - which device, and the scope check here decides the rest. + which device and its nonce, and the checks here decide the rest: the + nonce is held against the device's as for a credential minted here. """ config = cloud_config.load() if config is None: @@ -375,7 +412,7 @@ def _mint_at_hub(device_id: str) -> str: response = httpx.post( f"{config.portal_url.rstrip('/')}/api/v1/panel-tokens/", headers={"Authorization": f"Bearer {config.token}"}, - json={"panel": device_id}, + json={"panel": device_id, "pnc": nonce}, timeout=15.0, ) except httpx.HTTPError as exc: @@ -403,16 +440,28 @@ def _document(device: DeviceDef, store: DeviceStore) -> DeviceDocument: ) +#: A recheck waiting for a rebuild to finish. One is enough: it reads the +#: devices as they are once it runs. +_rechecks: set[asyncio.Task[None]] = set() + + async def _recheck(request: Request) -> None: """Validate the flows again: a device node may name this device. The device is saved either way, so a busy engine is not this request's - failure: the next rebuild validates again. + failure. The check waits for the rebuild in the background instead, since + one that started before this change may have read the devices before it. """ controller = getattr(request.app.state, "flow_controller", None) - if controller is not None: - with suppress(RebuildBusy): - await controller.recheck() + if controller is None: + return + try: + await controller.recheck() + except RebuildBusy: + if not _rechecks: + task = asyncio.create_task(controller.recheck(wait=None)) + _rechecks.add(task) + task.add_done_callback(_rechecks.discard) def _changed() -> None: @@ -433,19 +482,24 @@ def start_pairing(request: Request, body: PairHello | None = None) -> Any: All this hands out is a code that means nothing until somebody with an account approves it, so the worst an unwelcome caller achieves is an entry that expires ten minutes later. Reachable from the internet when this - instance is enrolled with a portal, which is what the cap and the portal's - own per-address limits are between. + instance is enrolled with a portal, which is what the per-address limit, + the cap and the portal's own limits are between. At the cap the oldest + request makes room, so a flood delays a device rather than locking it out. """ - if _pending.count() >= MAX_PENDING: + if too_many(request, "pair", PAIR_PER_ADDRESS, PAIR_WINDOW_S): raise HTTPException( - status_code=429, detail="Too many devices are waiting to be paired" + status_code=429, + detail="Too many pairing codes asked for from here. Try again shortly.", + headers={"Retry-After": str(int(PAIR_WINDOW_S))}, ) + _pending.make_room(MAX_PENDING) remote = cloud_config.via_portal(request.headers) hello = body or PairHello() entry = _Pending( secret=secrets.token_urlsafe(16), request=secrets.token_urlsafe(9), + check="".join(secrets.choice(CODE_ALPHABET) for _ in range(CHECK_LENGTH)), expires=time.time() + PAIR_TTL, device=_describe(request), name=hello.name, @@ -462,7 +516,7 @@ def start_pairing(request: Request, body: PairHello | None = None) -> Any: break else: raise HTTPException(status_code=503, detail="Could not mint a pairing code") - return PairStarted(code=code, secret=entry.secret) + return PairStarted(code=code, secret=entry.secret, check=entry.check) def poll_pairing(code: str, secret: str = "") -> Any: @@ -642,7 +696,7 @@ async def put_report(device_id: str, request: Request, store: DeviceStoreDep) -> detail=f"A report is at most {REPORT_MAX // 1024} KB", ) try: - report = DeviceReport.model_validate_json(bytes(raw)) + report, left_out = read_report(bytes(raw)) except ValidationError as exc: raise HTTPException( status_code=422, @@ -650,6 +704,8 @@ async def put_report(device_id: str, request: Request, store: DeviceStoreDep) -> include_url=False, include_input=False, include_context=False ), ) + for part in left_out: + logger.warning("Device '%s' reported %s, left out", device_id, part) first = await run_in_threadpool(store.report, device_id, report) if ( first @@ -666,7 +722,8 @@ async def put_report(device_id: str, request: Request, store: DeviceStoreDep) -> ) _changed() await _recheck(request) - return Message(message=f"{len(report.ports)} ports reported") + said = f"{len(report.ports)} ports reported" + return Message(message=f"{said}, {len(left_out)} left out" if left_out else said) @router.put( @@ -719,15 +776,17 @@ def _adopt( ) -> str: """Mint the credential the device behind ``entry`` collects on its next poll. - One minted here names the approver, so what the device does stays - attributable to a person. One minted by the portal names the account this - instance was enrolled with instead. + Either one carries the device's nonce, so unpairing or deleting the + device refuses it. One minted here also names the approver, for the + record: the device acts as itself and not as that account, so deleting + the account leaves the device paired. """ + nonce = store.nonce(device_id) if entry.remote: - entry.token = _mint_at_hub(device_id) + entry.token = _mint_at_hub(device_id, nonce) else: entry.token = security.create_panel_token( - device_id, user.id, timedelta(days=TOKEN_DAYS), store.nonce(device_id) + device_id, user.id, timedelta(days=TOKEN_DAYS), nonce ) entry.device_id = device_id _pending.save(code, entry) @@ -791,9 +850,9 @@ async def unpair_device(device_id: str, store: DeviceStoreDep) -> Any: The screen hanging there is refused on its next request and goes back to showing a pairing code, while its dashboards and their arrangement stay. - - A credential the portal minted for a remote screen carries no nonce, so - this does not reach it. That one is revoked at the hub. + The same holds for one the portal minted. An older portal minted without + the nonce, and such a credential stands only while the nonce is still 0, + so this refuses it too. """ if await run_in_threadpool(store.find, device_id) is None: raise _not_found(device_id) diff --git a/backend/fluksio/cloud/config.py b/backend/fluksio/cloud/config.py index d561014..14d2d03 100644 --- a/backend/fluksio/cloud/config.py +++ b/backend/fluksio/cloud/config.py @@ -166,15 +166,14 @@ def decode_portal_token(token: str) -> dict[str, Any]: scope = claims.get("scope") if scope == "panel": # A screen that paired through the portal. The portal named the panel - # and nothing else; what that panel may read is decided here, by the - # same check a panel paired on this network passes. It acts as the - # enrolling account for want of any other, but the scope check is what - # actually bounds it โ€” so a token of this scope that names no panel is - # refused rather than left holding the account it borrows. + # and the nonce this instance gave it when asking, nothing else; what + # that panel may read is decided here, by the same checks a panel + # paired on this network passes, the nonce included. A token of this + # scope that names no panel is refused. panel = str(claims.get("sub") or "") if not panel: raise InvalidTokenError("a panel token must name its panel") - return {"sub": config.local_user_id, "panel": panel} + return {"panel": panel, "pnc": claims.get("pnc")} if scope != "proxy": raise InvalidTokenError("not a proxy token") # A portal session names the person holding it, and that name is the whole diff --git a/backend/fluksio/core/security.py b/backend/fluksio/core/security.py index ab0c7ae..78f5e92 100644 --- a/backend/fluksio/core/security.py +++ b/backend/fluksio/core/security.py @@ -186,12 +186,12 @@ def create_panel_token( ) -> str: """The credential a paired wall panel holds. - Signed with the app's own secret like a browser session, because it names a - person in exactly the same way: ``sub`` is the account that approved the - pairing, so everything the panel does is attributable to them. What keeps - it from being a full session is the ``panel`` claim โ€” the request filter in - ``fluksio.api.deps`` lets it reach only that panel's dashboards and the message - endpoints its widgets need. + Signed with the app's own secret like a browser session, and told apart + from one by its audience. ``sub`` records the account that approved the + pairing. The panel acts as itself rather than as that account (see + ``deps.device_principal``), so deleting the account leaves it paired. The + ``panel`` claim is what the request filter in ``fluksio.api.deps`` holds + it to: that panel's dashboards and the message endpoints its widgets need. ``pnc`` is the panel's nonce at the moment of pairing, and the filter refuses a credential naming any other. Bumping the panel's nonce is diff --git a/backend/fluksio/flow/controller.py b/backend/fluksio/flow/controller.py index 3a75f49..2492a00 100644 --- a/backend/fluksio/flow/controller.py +++ b/backend/fluksio/flow/controller.py @@ -619,6 +619,8 @@ class FlowController: finally: self._lock.release() + # A stopped flow wires no device ports. + self._rewired() self._publish( { "type": "pipeline_rebuilt", @@ -836,7 +838,7 @@ class FlowController: ) self._rewired() - async def recheck(self) -> None: + async def recheck(self, wait: float | None = FLOW_REBUILD_WAIT) -> None: """Validate the built graph again without rebuilding anything. For what validation reads from outside the flows: a device created or @@ -844,7 +846,7 @@ class FlowController: """ if self.pipeline is None: return - await self._acquire(FLOW_REBUILD_WAIT) + await self._acquire(wait) try: self.issues = _collect_issues( self.loaded, self.pipeline, self._all_flow_inputs(), self.devices @@ -2124,6 +2126,14 @@ def _collect_issues( _NUMBERS = frozenset({"int", "float"}) +def _ways(said: dict[str, Any]) -> list[str]: + """The directions a reported port may be wired in, none when it does not + say: a direction this version does not know is no reason to refuse one.""" + if said.get("modes"): + return list(said["modes"]) + return [said["direction"]] if said.get("direction") in ("in", "out") else [] + + def _device_issues( loaded: dict[str, LoadedNode], devices: Any ) -> list[ValidationIssue]: @@ -2190,9 +2200,7 @@ def _device_issues( elif said is None: problem = f"'{device}' does not report a port '{spec.port}'." advisory = True - elif said.get("direction") and direction not in ( - said.get("modes") or [said["direction"]] - ): + elif (ways := _ways(said)) and direction not in ways: problem = ( f"'{spec.port}' of '{device}' is an {said['direction']} " f"port, wired here as {direction}." diff --git a/backend/fluksio/flow/devices.py b/backend/fluksio/flow/devices.py index f61911e..7b0a8d5 100644 --- a/backend/fluksio/flow/devices.py +++ b/backend/fluksio/flow/devices.py @@ -34,7 +34,16 @@ from datetime import UTC, datetime from pathlib import Path from typing import Any, Literal -from pydantic import BaseModel, Field, TypeAdapter, ValidationError, field_validator +from pydantic import ( + BaseModel, + Field, + TypeAdapter, + ValidationError, + ValidationInfo, + ValidatorFunctionWrapHandler, + field_validator, + model_validator, +) from fluksio.flow.dashboards import DashboardDef, DashboardNotFound, DashboardStore from fluksio.flow.events import event_bus @@ -167,21 +176,48 @@ class DeviceState(BaseModel): _STATES = TypeAdapter(dict[str, DeviceState]) +#: Read in place of a value this version does not know, such as a newer +#: firmware's display shape or port direction: no reason to refuse the rest of +#: what the device says. +UNKNOWN = "unknown" + + +def _known(value: Any, known: tuple[str, ...]) -> Any: + return value if value in known else UNKNOWN + + +def _left_out(info: ValidationInfo, what: str, why: str) -> None: + """Note a part of a report that was dropped, for whoever reads it to log.""" + if isinstance(info.context, dict): + info.context.setdefault("left_out", []).append(f"{what} ({why})") + + +def _why(exc: ValidationError) -> str: + error = exc.errors(include_url=False)[0] + where = ".".join(str(part) for part in error["loc"]) + return f"{where}: {error['msg']}" if where else str(error["msg"]) + + class ReportDisplay(BaseModel): """The device's screen, in pixels.""" w: int = Field(ge=0) h: int = Field(ge=0) - shape: Literal["rect", "round"] = "rect" + shape: Literal["rect", "round", "unknown"] = "rect" touch: bool = False + @field_validator("shape", mode="before") + @classmethod + def _shape(cls, value: Any) -> Any: + return _known(value, ("rect", "round")) + class ReportPort(BaseModel): """One port the device has, and how it may be wired.""" port: str = Field(pattern=PORT_PATTERN.pattern) #: Its default direction: ``in`` publishes, ``out`` follows. - direction: Literal["in", "out"] + direction: Literal["in", "out", "unknown"] dtype: DType #: A JSON schema of the ``config`` the device reads for this port. config: dict[str, Any] | None = None @@ -190,6 +226,19 @@ class ReportPort(BaseModel): #: The directions it may be wired in, when it may be either: a GPIO pin. modes: list[Literal["in", "out"]] | None = None + @field_validator("direction", mode="before") + @classmethod + def _direction(cls, value: Any) -> Any: + return _known(value, ("in", "out")) + + @field_validator("modes", mode="before") + @classmethod + def _modes(cls, value: Any) -> Any: + # The ones this version wires. Another is not said, rather than wrong. + if isinstance(value, list): + return [mode for mode in value if mode in ("in", "out")] + return value + class ReportCamera(BaseModel): """The camera sensor the device found, and what it delivers.""" @@ -203,8 +252,16 @@ class ReportCamera(BaseModel): class DeviceReport(BaseModel): """What a device says it has, version 1. - Keys this version does not know are dropped, so a newer firmware still - reports to an older instance. The whole report replaces the one before. + Read leniently, so a newer firmware still reports to an older instance. + Keys this version does not know are dropped, a value of a closed set it + does not know reads as ``unknown``, and a port or a section it cannot read + is left out on its own. Only a report that is not version 1, or not an + object at all, is refused. :func:`read_report` says what was left out. + + A port's ``config`` may name an entry of ``config_schemas`` instead of + spelling the schema out, so a board with thirty pins sends the pin schema + once. It is put in place when the report is read. The whole report + replaces the one before. """ version: Literal[1] = Field(alias="schema") @@ -217,11 +274,85 @@ class DeviceReport(BaseModel): #: The build's features, true when there: ``{"camera": true}``. features: dict[str, bool] = Field(default_factory=dict) + @model_validator(mode="before") + @classmethod + def _named_configs(cls, data: Any, info: ValidationInfo) -> Any: + if not isinstance(data, dict) or not isinstance(data.get("ports"), list): + return data + schemas = data.get("config_schemas") + schemas = schemas if isinstance(schemas, dict) else {} + ports = [] + for port in data["ports"]: + if isinstance(port, dict) and isinstance(port.get("config"), str): + schema = schemas.get(port["config"]) + if not isinstance(schema, dict): + _left_out( + info, + f"the config of port {port.get('port')!r}", + f"no schema named {port['config']!r}", + ) + schema = None + port = {**port, "config": schema} + ports.append(port) + return {**data, "ports": ports} + + @field_validator("ports", mode="before") + @classmethod + def _each_port(cls, value: Any, info: ValidationInfo) -> Any: + if not isinstance(value, list): + _left_out(info, "ports", "not a list") + return [] + kept = [] + for raw in value: + try: + kept.append(ReportPort.model_validate(raw)) + except ValidationError as exc: + name = raw.get("port") if isinstance(raw, dict) else raw + _left_out(info, f"port {name!r}", _why(exc)) + return kept + + @field_validator("features", mode="before") + @classmethod + def _flags(cls, value: Any, info: ValidationInfo) -> Any: + if not isinstance(value, dict): + _left_out(info, "features", "not an object") + return {} + flags = {} + for name, flag in value.items(): + if isinstance(flag, bool): + flags[str(name)] = flag + else: + _left_out(info, f"feature {name!r}", f"{flag!r} is not true or false") + return flags + + @field_validator("platform", "board", "firmware", "display", "camera", mode="wrap") + @classmethod + def _or_left_out( + cls, value: Any, handler: ValidatorFunctionWrapHandler, info: ValidationInfo + ) -> Any: + try: + return handler(value) + except ValidationError as exc: + name = info.field_name or "" + _left_out(info, name, _why(exc)) + return cls.model_fields[name].get_default() + def stored(self) -> dict[str, Any]: """As ``devices.json`` keeps it: the device's own keys, nothing unset.""" return self.model_dump(mode="json", by_alias=True, exclude_none=True) +def read_report(raw: bytes) -> tuple[DeviceReport, list[str]]: + """A device's report, and each part of it that was left out. + + Raises ``ValidationError`` only for a report that is not version 1 or + not an object. + """ + context: dict[str, list[str]] = {"left_out": []} + report = DeviceReport.model_validate_json(raw, context=context) + return report, context["left_out"] + + class DeviceNotFound(KeyError): def __init__(self, device_id: str) -> None: super().__init__(device_id) @@ -265,6 +396,11 @@ class DeviceStore: self._wiring: dict[str, list[Binding]] = {} self._wiring_at = -1 self._wiring_lock = threading.Lock() + #: ``devices.json`` as last parsed, by the file's stat. Every device + #: request holds its nonce against it. + self._states_seen: ( + tuple[tuple[int, int, int], dict[str, DeviceState]] | None + ) = None def _file(self, device_id: str) -> Path: return self.root / f"{checked(device_id)}.json" @@ -281,18 +417,24 @@ class DeviceStore: # -- what the published flows wire ------------------------------------ def wiring(self) -> dict[str, list[Binding]]: - """Every device's bound ports, keyed by device id. + """Every device's bound ports in enabled published flows, by device id. + + A stopped flow wires nothing: its ports go out of the device's ``io`` + and out of its credential's reach until it is started again. Each list is in one fixed order, port first, so what a device reads is the same bytes for as long as the flows are the same: the firmware compares the whole document and rebuilds its screen on any change. - Read again when the flow store commits, a device save included. + Read again when the flow store commits, a device save, starting and + stopping a flow included. """ with self._wiring_lock: revision = self.flows.revision if self._wiring_at != revision: found: dict[str, list[Binding]] = {} for flow in self.flows.read_all(): + if not self.flows.read_enabled(flow.name): + continue for binding in bindings_of(flow): found.setdefault(binding.device, []).append(binding) for rows in found.values(): @@ -303,7 +445,19 @@ class DeviceStore: return self._wiring def bound(self, device_id: str) -> list[Binding]: - return self.wiring().get(device_id, []) + """What the device's ports are wired to, one binding per port. + + A port wired twice is an issue on both nodes. Until it is fixed the + device follows the first of the two in the fixed order, for its + ``io`` and its credential's reach alike, so neither flips between them. + """ + found: list[Binding] = [] + seen: set[str] = set() + for binding in self.wiring().get(device_id, []): + if binding.port not in seen: + seen.add(binding.port) + found.append(binding) + return found def io(self, device_id: str) -> list[DeviceIo]: """The ports as the device reads them.""" @@ -315,7 +469,10 @@ class DeviceStore: ] def bindings(self, device_id: str) -> list[DeviceBinding]: - """The same ports by the node holding each, for whoever wires them.""" + """The same ports by the node holding each, for whoever wires them. + + Every binding, a port wired twice twice, so the one to remove shows. + """ return sorted( ( DeviceBinding( @@ -325,7 +482,7 @@ class DeviceStore: message=b.message, direction=b.direction, ) - for b in self.bound(device_id) + for b in self.wiring().get(device_id, []) ), key=lambda b: (b.flow, b.node, b.port, b.message), ) @@ -403,7 +560,8 @@ class DeviceStore: The nonce is bumped rather than dropped: a device created again under the same id, or brought back by a revert, must not take the old - screen's credential back with it. + screen's credential back with it. Its report goes, since a device + created again is not the board that made it. """ with self.flows._write_lock: path = self._file(device_id) @@ -411,14 +569,15 @@ class DeviceStore: raise DeviceNotFound(device_id) path.unlink() self.flows._commit(f"Delete device '{device_id}'") - self.bump(device_id) + self.bump(device_id, forget=True) presence.forget(device_id) # -- outside git ------------------------------------------------------- def _read_states(self) -> dict[str, DeviceState]: - """Raises on a file that does not parse, rather than reading it as empty. + """The file as it is now, for a writer to change and write back. + Raises on a file that does not parse, rather than reading it as empty. Empty would put every nonce back to 0, which honours again every credential an unpair refused. """ @@ -426,6 +585,26 @@ class DeviceStore: return {} return _STATES.validate_json(self.state_file.read_bytes()) + def _states(self) -> dict[str, DeviceState]: + """The same for reading only, parsed again only once the file changed. + + Every device request reads its nonce here. A write from this process + replaces what is held at once, and one from anywhere else changes the + file's stat. Shared between callers, so never changed in place. + """ + try: + seen = self.state_file.stat() + except FileNotFoundError: + return {} + key = (seen.st_mtime_ns, seen.st_size, seen.st_ino) + held = self._states_seen + if held is not None and held[0] == key: + return held[1] + # Stat before read, so what is held is never older than its key. + states = _STATES.validate_json(self.state_file.read_bytes()) + self._states_seen = (key, states) + return states + def _write_states(self, states: dict[str, DeviceState]) -> None: # Through a temporary file, so a crash mid-write cannot leave a torn # file behind. @@ -433,19 +612,27 @@ class DeviceStore: tmp = self.state_file.with_name(self.state_file.name + ".tmp") tmp.write_bytes(_STATES.dump_json(states, indent=2)) os.replace(tmp, self.state_file) + seen = self.state_file.stat() + self._states_seen = ((seen.st_mtime_ns, seen.st_size, seen.st_ino), states) def state(self, device_id: str) -> DeviceState: - return self._read_states().get(device_id, DeviceState()) + return self._states().get(device_id, DeviceState()) def nonce(self, device_id: str) -> int: return self.state(device_id).nonce - def bump(self, device_id: str) -> int: - """Refuse the credential this device holds now. Returns the new nonce.""" + def bump(self, device_id: str, *, forget: bool = False) -> int: + """Refuse the credential this device holds now. Returns the new nonce. + + ``forget`` drops everything else kept about the device as well. + """ with self._state_lock: states = self._read_states() - state = states.setdefault(device_id, DeviceState()) + state = states.get(device_id) or DeviceState() + if forget: + state = DeviceState(nonce=state.nonce) state.nonce += 1 + states[device_id] = state self._write_states(states) return state.nonce diff --git a/backend/fluksio/models.py b/backend/fluksio/models.py index f40b9cd..01b41bb 100644 --- a/backend/fluksio/models.py +++ b/backend/fluksio/models.py @@ -94,6 +94,9 @@ class TokenPayload(SQLModel): #: up from it. A portal identity nobody here was mapped to resolves to no #: user at all, which is the refusal. portal_sub: str | None = None + #: The device a device credential stands for, which acts as itself rather + #: than as the account ``sub`` names. + panel: str | None = None class NewPassword(SQLModel): diff --git a/backend/tests/api/routes/test_devices.py b/backend/tests/api/routes/test_devices.py index e99c937..fc86238 100644 --- a/backend/tests/api/routes/test_devices.py +++ b/backend/tests/api/routes/test_devices.py @@ -7,6 +7,7 @@ from fastapi.testclient import TestClient from starlette.websockets import WebSocketDisconnect from fluksio.api.routes import devices as devices_route +from fluksio.api.routes import oauth from fluksio.cloud import config as cloud_config from fluksio.core.config import settings from fluksio.flow.devices import DeviceState @@ -18,6 +19,14 @@ PANELS = f"{settings.API_V1_STR}/panels" DASHBOARDS = f"{settings.API_V1_STR}/dashboards" +@pytest.fixture(autouse=True) +def _pairing_unlimited() -> None: + """Every test pairs from the test client's one address, which the + per-address limit is about. The test of that limit spends it itself.""" + for bucket in [b for b in oauth._hits if b.startswith("pair:")]: + del oauth._hits[bucket] + + def _panels(client: TestClient, headers: dict[str, str], config: dict) -> None: """Make the stored devices exactly ``config["panels"]``, one call each.""" wanted = {device["id"]: device for device in config["panels"]} @@ -754,6 +763,16 @@ def test_a_port_wired_twice_or_an_unknown_device_is_an_issue( ("device_port", "gpio.4", False), ("unknown_device", None, False), } + # The device follows the first of the two, and the tab shows both. + read = client.get(f"{PREFIX}/porch_io", headers=superuser_token_headers).json() + assert [(row["port"], row["message"]) for row in read["io"]] == [ + ("backlight", "porch.backlight"), + ("gpio.4", "porch.doorbell"), + ] + assert [b["flow"] for b in read["bindings"] if b["port"] == "gpio.4"] == [ + "porch", + "porch_too", + ] # Creating the device is enough, nothing has to be published again. _panels( client, @@ -1192,6 +1211,13 @@ def test_a_pending_code_is_found_by_whichever_worker_is_polled() -> None: assert polled.get("STALE1") is None assert polled.count() == 0 + # Making room drops the oldest, whichever worker minted it. + for offset, code in enumerate(("OLD111", "MID222", "NEW333")): + later = devices_route._Pending(secret="s", expires=time.time() + 60 + offset) + assert minted.add(code, later) + polled.make_room(3) + assert minted.get("OLD111") is None and polled.count() == 2 + def test_removing_the_panel_revokes_its_credential( client: TestClient, superuser_token_headers: dict[str, str] @@ -1262,6 +1288,10 @@ def test_a_device_on_this_network_is_adopted_from_the_pending_list( mine = next(entry for entry in pending if entry["name"] == "knob-3f2a") assert mine["board"] == "knob" and mine["firmware"] == "1.4" assert mine["needs_code"] is False + # The check the device shows, to compare before adopting it from here. + assert len(started["check"]) == devices_route.CHECK_LENGTH + assert set(started["check"]) <= set(devices_route.CODE_ALPHABET) + assert mine["check"] == started["check"] portal = next(entry for entry in pending if entry["remote"]) assert portal["needs_code"] is True assert len(portal["name"]) == devices_route.HELLO_LENGTH @@ -1375,7 +1405,9 @@ def test_a_remote_device_is_paired_at_the_portal( assert calls[0]["url"].endswith("/api/v1/panel-tokens/") # type: ignore[union-attr] assert calls[0]["headers"]["Authorization"] == "Bearer instance-token" # type: ignore[index] - assert calls[0]["json"] == {"panel": "hallway"} # type: ignore[index] + # With the device's nonce, which the hub puts in the credential. + nonce = app.state.device_store.nonce("hallway") + assert calls[0]["json"] == {"panel": "hallway", "pnc": nonce} # type: ignore[index] collected = client.get( f"{PREFIX}/pair/{started['code']}", params={"secret": started["secret"]} @@ -1646,3 +1678,191 @@ def test_only_the_newest_frame_of_a_batch_is_pushed(tmp_path) -> None: length = int.from_bytes(frames[0][:4], "big") assert orjson.loads(frames[0][4 : 4 + length])["digest"] == second["digest"] assert frames[0][4 + length :] == b"two" + + +# -------------------------------------------------------------------------- +# C0: limits on asking, a lenient report, and what a credential stands on +# -------------------------------------------------------------------------- + + +def test_one_address_asks_for_a_few_codes_a_minute(client: TestClient) -> None: + lan = TestClient(app, client=("192.168.1.77", 50000)) + for _ in range(devices_route.PAIR_PER_ADDRESS): + assert lan.post(f"{PREFIX}/pair").status_code == 200 + refused = lan.post(f"{PANELS}/pair") + assert refused.status_code == 429 + assert refused.headers["retry-after"] + # Another address is not held to this one's. + assert client.post(f"{PREFIX}/pair").status_code == 200 + + +def test_at_the_cap_the_oldest_request_makes_room( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + store = devices_route._PendingStore() + store._redis = None + monkeypatch.setattr(devices_route, "_pending", store) + monkeypatch.setattr(devices_route, "PAIR_PER_ADDRESS", 1000) + + started = [ + client.post(f"{PREFIX}/pair").json() + for _ in range(devices_route.MAX_PENDING + 1) + ] + assert store.count() == devices_route.MAX_PENDING + oldest, newest = started[0], started[-1] + assert ( + client.get( + f"{PREFIX}/pair/{oldest['code']}", params={"secret": oldest["secret"]} + ).status_code + == 404 + ) + assert ( + client.get( + f"{PREFIX}/pair/{newest['code']}", params={"secret": newest["secret"]} + ).status_code + == 200 + ) + + +def test_a_report_is_read_port_by_port( + client: TestClient, + superuser_token_headers: dict[str, str], + caplog: pytest.LogCaptureFixture, +) -> None: + """What this version cannot read is left out and logged, not refused. + + A shape or direction it does not know reads as unknown, and a named config + schema is put in place. + """ + _panels(client, superuser_token_headers, {"panels": [{"id": "lenient"}]}) + device = _pair(client, superuser_token_headers, "lenient") + pin = {"type": "object", "properties": {"invert": {"type": "boolean"}}} + report = { + **REPORT, + "display": {"w": 466, "h": 466, "shape": "hexagon", "touch": False}, + "config_schemas": {"pin": pin}, + "ports": [ + {"port": "gpio.4", "direction": "in", "dtype": "bool", "config": "pin"}, + {"port": "gpio.5", "direction": "both", "dtype": "bool", "config": "pin"}, + {"port": "Not A Port", "direction": "in", "dtype": "bool"}, + {"port": "lidar", "direction": "in", "dtype": "pointcloud"}, + ], + "features": {"display": True, "camera": "imx415"}, + } + with caplog.at_level("WARNING", logger="fluksio.api.routes.devices"): + put = client.put(f"{PREFIX}/lenient/report", headers=device, json=report) + assert put.status_code == 200, put.text + assert "2 ports reported, 3 left out" in put.json()["message"] + logged = " ".join(r.getMessage() for r in caplog.records) + for part in ("'Not A Port'", "'lidar'", "feature 'camera'"): + assert part in logged + + listed = client.get(f"{PREFIX}/", headers=superuser_token_headers).json() + stored = next(d for d in listed["data"] if d["id"] == "lenient")["report"] + assert stored["display"]["shape"] == "unknown" + assert stored["ports"] == [ + {"port": "gpio.4", "direction": "in", "dtype": "bool", "config": pin}, + {"port": "gpio.5", "direction": "unknown", "dtype": "bool", "config": pin}, + ] + assert stored["features"] == {"display": True} + assert "config_schemas" not in stored + + # Only no structure at all is refused. + for body in ([], "report", {"ports": []}): + refused = client.put(f"{PREFIX}/lenient/report", headers=device, json=body) + assert refused.status_code == 422, body + + # Deleting the device drops the report and keeps the nonce moving. + store = app.state.device_store + nonce = store.nonce("lenient") + client.delete(f"{PREFIX}/lenient", headers=superuser_token_headers) + assert store.state("lenient").report is None + assert store.nonce("lenient") == nonce + 1 + + +def test_a_busy_engine_rechecks_once_it_is_free( + client: TestClient, + superuser_token_headers: dict[str, str], + monkeypatch: pytest.MonkeyPatch, +) -> None: + from fluksio.flow.controller import RebuildBusy + + _panels(client, superuser_token_headers, {"panels": [{"id": "busy"}]}) + device = _pair(client, superuser_token_headers, "busy") + controller = app.state.flow_controller + waits: list[float | None] = [] + + async def recheck(wait: float | None = 5.0) -> None: + waits.append(wait) + if wait is not None: + raise RebuildBusy("busy") + + monkeypatch.setattr(controller, "recheck", recheck) + put = client.put(f"{PREFIX}/busy/report", headers=device, json=REPORT) + assert put.status_code == 200, put.text + # The background recheck waits for the lock rather than giving up. + client.get(f"{PREFIX}/busy", headers=device) + assert waits == [5.0, None] + + +def test_a_device_outlives_the_account_that_paired_it( + client: TestClient, superuser_token_headers: dict[str, str] +) -> None: + """It acts as itself, not as whoever approved it.""" + from tests.utils.user import user_authentication_headers + from tests.utils.utils import random_email, random_lower_string + + email, password = random_email(), random_lower_string() + created = client.post( + f"{settings.API_V1_STR}/users/", + headers=superuser_token_headers, + json={"email": email, "password": password, "is_superuser": True}, + ) + assert created.status_code == 200, created.text + approver = user_authentication_headers( + client=client, email=email, password=password + ) + _dashboard(client, superuser_token_headers, "outlived") + _panels( + client, + superuser_token_headers, + {"panels": [{"id": "orphan", "dashboards": ["outlived"]}]}, + ) + device = _pair(client, approver, "orphan") + + gone = client.delete( + f"{settings.API_V1_STR}/users/{created.json()['id']}", + headers=superuser_token_headers, + ) + assert gone.status_code == 200, gone.text + assert client.get(f"{PREFIX}/orphan", headers=device).status_code == 200 + assert client.get(f"{DASHBOARDS}/outlived", headers=device).status_code == 200 + # Still no more than a device: nothing a superuser's route serves. + assert client.get(f"{PREFIX}/", headers=device).status_code == 403 + client.delete(f"{PREFIX}/orphan", headers=superuser_token_headers) + + +def test_a_stopped_flow_wires_no_ports( + client: TestClient, superuser_token_headers: dict[str, str] +) -> None: + _panels(client, superuser_token_headers, {"panels": [{"id": "porch_io"}]}) + _publish_flow(client, superuser_token_headers, "porch", [PORCH_NODE]) + device = _pair(client, superuser_token_headers, "porch_io") + messages = f"{settings.API_V1_STR}/messages" + + def io() -> list[str]: + read = client.get(f"{PREFIX}/porch_io", headers=device).json() + return [row["port"] for row in read["io"]] + + assert io() == ["backlight", "gpio.4"] + client.post(f"{FLOWS}/porch/stop", headers=superuser_token_headers) + assert io() == [] + assert ( + client.post( + f"{messages}/porch.doorbell", headers=device, json={"value": True} + ).status_code + == 403 + ) + client.post(f"{FLOWS}/porch/start", headers=superuser_token_headers) + assert io() == ["backlight", "gpio.4"] + client.delete(f"{FLOWS}/porch", headers=superuser_token_headers) diff --git a/backend/tests/flow/test_device_store.py b/backend/tests/flow/test_device_store.py index 6c76956..4384fed 100644 --- a/backend/tests/flow/test_device_store.py +++ b/backend/tests/flow/test_device_store.py @@ -239,3 +239,18 @@ def test_panels_with_io_reach_the_flows_in_one_start(tmp_path: Path) -> None: assert [(r.port, r.message, r.config) for r in store.io("knob")] == [ ("backlight", "kiosk.brightness", {"wake_s": 30}) ] + + +def test_the_nonces_are_read_again_only_when_the_file_changes(tmp_path: Path) -> None: + """Every device request reads its nonce, so the file is parsed once per + change, and a change from outside this process is still seen.""" + store = DeviceStore(FlowStore(tmp_path / "flows"), tmp_path / "devices.json") + assert store.nonce("hall") == 0 + store.bump("hall") + assert store.nonce("hall") == 1 + assert store._states() is store._states() + + raw = json.loads(store.state_file.read_text()) + raw["hall"]["nonce"] = 7 + store.state_file.write_text(json.dumps(raw, indent=4)) + assert store.nonce("hall") == 7 diff --git a/backend/tests/test_cloud.py b/backend/tests/test_cloud.py index 8e7fd60..9712294 100644 --- a/backend/tests/test_cloud.py +++ b/backend/tests/test_cloud.py @@ -24,6 +24,7 @@ from sqlmodel import Session from fluksio.api.deps import decode_token, user_from_token from fluksio.cloud import config as cloud_config from fluksio.core.config import settings +from fluksio.main import app from fluksio.models import User from tests.utils.portal import ISSUER, jwks, portal_token from tests.utils.user import create_random_user @@ -290,7 +291,9 @@ def test_a_panel_scoped_portal_token_reaches_only_its_panel( json={"version": version}, ) - token = portal_token(portal_key, subject="hallway", scope="panel") + # As the hub mints it: with the nonce this instance named when it asked. + nonce = app.state.device_store.nonce("hallway") + token = portal_token(portal_key, subject="hallway", scope="panel", pnc=nonce) headers = {"Authorization": f"Bearer {token}"} assert client.get(f"{devices}/hallway", headers=headers).status_code == 200 @@ -312,9 +315,39 @@ def test_a_panel_scoped_portal_token_reaches_only_its_panel( client.get(f"{settings.API_V1_STR}/flows/", headers=headers).status_code == 403 ) - # Deleting the device is how the screen is retired, whoever minted its token. + # Unpairing refuses it as it refuses one minted here, and so does + # deleting the device, which a device created again does not undo. + client.post(f"{devices}/hallway/unpair", headers=superuser_token_headers) + assert client.get(f"{devices}/hallway", headers=headers).status_code == 401 + nonce = app.state.device_store.nonce("hallway") + token = portal_token(portal_key, subject="hallway", scope="panel", pnc=nonce) + headers = {"Authorization": f"Bearer {token}"} + assert client.get(f"{devices}/hallway", headers=headers).status_code == 200 client.delete(f"{devices}/hallway", headers=superuser_token_headers) assert client.get(f"{devices}/hallway", headers=headers).status_code == 401 + client.post(f"{devices}/hallway", headers=superuser_token_headers) + assert client.get(f"{devices}/hallway", headers=headers).status_code == 401 + client.delete(f"{devices}/hallway", headers=superuser_token_headers) + + +def test_a_portal_token_from_before_nonces_stands_until_an_unpair( + client: TestClient, + enrolled: User, # noqa: ARG001 (fixture installs the enrolment) + portal_key: rsa.RSAPrivateKey, + superuser_token_headers: dict[str, str], +) -> None: + """An older hub minted without ``pnc``, which reads as nonce 0.""" + devices = f"{settings.API_V1_STR}/devices" + device_id = f"porch_{uuid.uuid4().hex[:6]}" + assert ( + client.post(f"{devices}/{device_id}", headers=superuser_token_headers) + ).status_code == 200 + token = portal_token(portal_key, subject=device_id, scope="panel") + headers = {"Authorization": f"Bearer {token}"} + assert client.get(f"{devices}/{device_id}", headers=headers).status_code == 200 + client.post(f"{devices}/{device_id}/unpair", headers=superuser_token_headers) + assert client.get(f"{devices}/{device_id}", headers=headers).status_code == 401 + client.delete(f"{devices}/{device_id}", headers=superuser_token_headers) def test_a_panel_token_naming_no_panel_is_refused( diff --git a/backend/tests/utils/portal.py b/backend/tests/utils/portal.py index 7f424f8..00b510a 100644 --- a/backend/tests/utils/portal.py +++ b/backend/tests/utils/portal.py @@ -31,6 +31,7 @@ def portal_token( audience: str = INSTANCE_ID, issuer: str = ISSUER, scope: str = "proxy", + **claims: Any, ) -> str: now = datetime.now(UTC) pem = key.private_bytes( @@ -46,6 +47,7 @@ def portal_token( "iat": now, "exp": now + timedelta(hours=1), "scope": scope, + **claims, }, pem, algorithm="RS256", diff --git a/docs/code/api.md b/docs/code/api.md index 9fb3ec3..68e8986 100644 --- a/docs/code/api.md +++ b/docs/code/api.md @@ -192,13 +192,13 @@ on its own, so this is for reclaiming space sooner. | `POST` | `/dashboards/{name}/duplicate` | copy into an unpublished `{name}_copy` | | `GET` | `/devices/` | list devices, with `online`, `last_seen`, `report` and `reported_at` | | `POST`, `GET`, `PUT`, `DELETE` | `/devices/{id}` | create, read, save or delete one; a save sends the `version` it read and gets 409 when someone saved first. A read adds `io` and `bindings`, its ports as the device nodes of published flows wire them | -| `POST` | `/devices/pair` | start device pairing, optionally with `{name, board, firmware}`; unauthenticated | +| `POST` | `/devices/pair` | start device pairing, optionally with `{name, board, firmware}`; answers `code`, `secret` and the two-character `check` the device shows beside its code; unauthenticated, five a minute per address, the oldest of 50 waiting requests gives way | | `GET` | `/devices/pair/{code}` | poll pairing with its `secret`; answers `device` (and `panel` for older firmware); unauthenticated | -| `GET` | `/devices/pending` | devices waiting for a code, as they describe themselves | +| `GET` | `/devices/pending` | devices waiting for a code, as they describe themselves, each with its `check` | | `GET` | `/devices/pair/{code}/device` | what is waiting on a code, before it is approved | | `POST` | `/devices/{id}/pair` | adopt by `{code}`, or by `{request}` from the pending list for a request from your network | -| `POST` | `/devices/{id}/unpair` | revoke the device credential | -| `PUT` | `/devices/{id}/report` | what the device has, from its own credential only; at most 16 KB, replaces the last report | +| `POST` | `/devices/{id}/unpair` | revoke the device credential, one the portal issued included | +| `PUT` | `/devices/{id}/report` | what the device has, from its own credential only; at most 16 KB, replaces the last report, read leniently (see [What a device reports](../interface/dashboards.md#what-a-device-reports)) | `/panels/pair`, `/panels/pair/{code}` and `GET /panels/{id}` answer as their `/devices/` counterparts, for firmware that has not updated yet. diff --git a/docs/interface/dashboards.md b/docs/interface/dashboards.md index fc23451..5605134 100644 --- a/docs/interface/dashboards.md +++ b/docs/interface/dashboards.md @@ -455,8 +455,9 @@ As a superuser: 4. Open the pairing link on the device. It shows a six-character code. 5. Enter that code under **Pairing**, check the device description, then press **Pair**. A device asking from your own network also appears in the list - below, with what it says about itself, and **Pair this one** adopts it - without the code. + below, with what it says about itself and a two-character check. The device + shows the same check beside its code. When the two match, **Pair this one** + adopts it without the code. The screen collects its credential and shows the assigned published dashboards. Several dashboards get a navigation rail. The page says whether the device is @@ -466,16 +467,22 @@ since the instance started. Each save carries the version it was read at. When someone else saved the device first, the save is refused and the page shows their version. +One address may ask for five codes a minute. At most 50 requests wait at once, +and a new one pushes out the oldest. + A device credential reads its assigned dashboards, their bound messages and the artifacts they show. It writes only the targets of their controls and query charts, and the messages of its own `in` [ports](#the-devices-own-ports). A message its tiles only draw, or a dashboard setting is bound to, stays read only. On a locked dashboard only the query charts may still publish. -**Unpair screen** revokes a locally paired credential and keeps the device and -its dashboards. Deleting the device removes the assignment and its access, and -a device added again under the same name does not bring the old credential -back. +A device acts as itself and not as the superuser who paired it, so deleting or +deactivating that account leaves the device paired. + +**Unpair screen** revokes the device's credential and keeps the device and its +dashboards. Deleting the device removes the assignment, its access and its +report, and a device added again under the same name does not bring the old +credential back. The device definitions are stored in the flow repository (`_devices/.json`), so their history is in git with the flows. The credential generation lives in @@ -517,8 +524,11 @@ ports it reports and what the published flows wire, each linked to its node. | type | The payload type, checked like any message's | | config | A JSON object passed to the device unread, such as `{"pull": "up", "debounce_ms": 50}` | -Only published flows count. A port wired in two of them, or a device node -naming a device that does not exist, is an error on the canvas. Until a device +Only enabled published flows count, so stopping a flow takes its ports out of +the device's `io` and its credential's reach. A port wired in two of them, or a +device node naming a device that does not exist, is an error on the canvas. +Until it is fixed the device follows the first binding of the port, by message +name. Until a device reports what it has, a port name is free text; once it has, a port it does not list is a warning and one wired the wrong way round, or to another type, an error. `int` and `float` pass for each other. @@ -537,8 +547,17 @@ Home, not a fault, since a panel switched off or asleep breaks nothing. A paired device says what it has with `PUT /devices/{id}/report`, from its own credential only. openpanel sends it after pairing and once per boot. The report -replaces the one before, is at most 16 KB and keys this version does not know -are dropped: +replaces the one before and is at most 16 KB. It is read leniently, so a newer +firmware still reports to an older instance: + +- keys this version does not know are dropped +- a value it does not know for a display `shape` or a port `direction` reads as + `unknown`, and a `features` value that is not true or false is left out +- a port it cannot read, such as one with an unknown `dtype`, is left out on its + own and named in the server log + +Only a report that is not `schema` 1, or not an object at all, is refused with +422. ```json { @@ -558,9 +577,12 @@ are dropped: ``` `config` is a JSON schema of the settings the device reads for that port, which -the Device node turns into a form. `modes` lists the directions a port may be -wired in when it may be either, as a GPIO pin. `on_demand` marks a port that -sends only while somebody wants it. +the Device node turns into a form. It may instead name an entry of a top-level +`config_schemas` object, so pins that share a schema send it once: +`"config_schemas": {"pin": {...}}` and `"config": "pin"` on each port. The +schema is put in place when the report is read. `modes` lists the directions a +port may be wired in when it may be either, as a GPIO pin. `on_demand` marks a +port that sends only while somebody wants it. The device's page shows the report under **Capabilities**, with each port's binding or **Use in flow** for it. A reported screen becomes a canvas preset in @@ -591,9 +613,11 @@ a request that came through it, or from outside your network, is adopted by its code only and never from the list. The instance enforces the same dashboard and message limits. Remote -credentials are issued by the portal, so **Unpair screen** does not revoke -them; revoke them at the portal. Deleting the device removes access on the -instance. +credentials are issued by the portal with the device's credential generation +in them, so **Unpair screen** and deleting the device revoke them as they do a +local one. A credential from a portal older than this carries none and keeps +working only while the device was never unpaired or deleted. Pair such a +screen again once the portal is updated. ## See also diff --git a/frontend/src/client/schemas.gen.ts b/frontend/src/client/schemas.gen.ts index 5a3196c..11f8dd1 100644 --- a/frontend/src/client/schemas.gen.ts +++ b/frontend/src/client/schemas.gen.ts @@ -3257,6 +3257,11 @@ export const PairStartedSchema = { type: 'string', title: 'Secret' }, + check: { + type: 'string', + title: 'Check', + default: '' + }, expires_in: { type: 'integer', title: 'Expires In', @@ -3317,6 +3322,11 @@ export const PendingDeviceSchema = { title: 'Request', default: '' }, + check: { + type: 'string', + title: 'Check', + default: '' + }, device: { type: 'string', title: 'Device' diff --git a/frontend/src/client/sdk.gen.ts b/frontend/src/client/sdk.gen.ts index 6caf50a..526d073 100644 --- a/frontend/src/client/sdk.gen.ts +++ b/frontend/src/client/sdk.gen.ts @@ -812,8 +812,9 @@ export class DevicesService { * All this hands out is a code that means nothing until somebody with an * account approves it, so the worst an unwelcome caller achieves is an entry * that expires ten minutes later. Reachable from the internet when this - * instance is enrolled with a portal, which is what the cap and the portal's - * own per-address limits are between. + * instance is enrolled with a portal, which is what the per-address limit, + * the cap and the portal's own limits are between. At the cap the oldest + * request makes room, so a flood delays a device rather than locking it out. * @param data The data for the request. * @param data.requestBody * @returns PairStarted Successful Response @@ -1016,7 +1017,8 @@ export class DevicesService { * * The first report of a touch screen makes the device touch friendly, as * the switch would have been set by hand. Later ones leave the setting to - * whoever changed it since. + * whoever changed it since, and so does the first report of a device paired + * before pairings were noted: its switch was set by hand already. * @param data The data for the request. * @param data.deviceId * @returns Message Successful Response @@ -1065,9 +1067,9 @@ export class DevicesService { * * The screen hanging there is refused on its next request and goes back to * showing a pairing code, while its dashboards and their arrangement stay. - * - * A credential the portal minted for a remote screen carries no nonce, so - * this does not reach it. That one is revoked at the hub. + * The same holds for one the portal minted. An older portal minted without + * the nonce, and such a credential stands only while the nonce is still 0, + * so this refuses it too. * @param data The data for the request. * @param data.deviceId * @returns Message Successful Response diff --git a/frontend/src/client/types.gen.ts b/frontend/src/client/types.gen.ts index aaae749..7df2e73 100644 --- a/frontend/src/client/types.gen.ts +++ b/frontend/src/client/types.gen.ts @@ -1093,6 +1093,7 @@ export type PairRequest = { export type PairStarted = { code: string; secret: string; + check?: string; expires_in?: number; }; @@ -1110,6 +1111,7 @@ export type PairStatus = { */ export type PendingDevice = { request?: string; + check?: string; device: string; name?: string; board?: string; diff --git a/frontend/src/components/Devices/DeviceDetail.tsx b/frontend/src/components/Devices/DeviceDetail.tsx index e19f590..ba57ff8 100644 --- a/frontend/src/components/Devices/DeviceDetail.tsx +++ b/frontend/src/components/Devices/DeviceDetail.tsx @@ -477,11 +477,29 @@ function Pairing({ ? "Waiting to be paired" : "No device is waiting"} + {pending?.some((entry) => !entry.needs_code) ? ( +

+ Compare the check beside each with the one the device shows next + to its code before choosing Pair this one. +

+ ) : null} {(pending ?? []).map((entry) => (
+ {entry.check ? ( + + + + Check {entry.check.split("").join(" ")} + + + ) : null} {entry.name || entry.board || "Unnamed device"} @@ -546,8 +564,7 @@ function Pairing({ Sends the screen back to a pairing code and keeps the device and - its dashboards. A screen paired through the portal is revoked - there instead. + its dashboards, also for a screen paired through the portal.
diff --git a/frontend/src/components/Devices/ports.ts b/frontend/src/components/Devices/ports.ts index fbac94b..d3b7460 100644 --- a/frontend/src/components/Devices/ports.ts +++ b/frontend/src/components/Devices/ports.ts @@ -25,7 +25,7 @@ export type DeviceReport = { display?: { w: number h: number - shape?: "rect" | "round" + shape?: "rect" | "round" | "unknown" touch?: boolean } | null camera?: { sensor?: string; w?: number; h?: number; fps?: number } | null diff --git a/frontend/src/components/Devices/queries.ts b/frontend/src/components/Devices/queries.ts index 238e10b..a3269f2 100644 --- a/frontend/src/components/Devices/queries.ts +++ b/frontend/src/components/Devices/queries.ts @@ -1,4 +1,8 @@ -import { useMutation, useQueryClient } from "@tanstack/react-query" +import { + type QueryClient, + useMutation, + useQueryClient, +} from "@tanstack/react-query" import { type DeviceDef, @@ -11,6 +15,20 @@ export const deviceKeys = { all: ["devices"] as const, detail: (id: string) => ["devices", id] as const, pending: ["devices-pending"] as const, + /** Every save of a device, whichever one. */ + save: ["devices", "save"] as const, +} + +/** + * Read the devices again, unless a save is under way. + * + * A refetch started during a save can land before the save does, which shows + * the change undone and hands the next save an older version, which the + * server refuses with 409. The last save to settle reads the list instead. + */ +export function refetchDevices(client: QueryClient, exact = true) { + if (client.isMutating({ mutationKey: deviceKeys.save })) return + client.invalidateQueries({ queryKey: deviceKeys.all, exact }) } /** Every device, what it shows and whether it is connected. */ @@ -43,8 +61,10 @@ export const pendingQueryOptions = () => ({ * A change is shown at once, and the saves of one device go one at a time, * each laid over the device as the cache holds it when it is sent and * against the version the save before it left. So a title and a switch - * changed in one gesture neither refuse nor undo each other. Any failure, a - * 409 from someone else's save included, reads the list again. + * changed in one gesture neither refuse nor undo each other. A refetch the + * socket asks for meanwhile waits (`refetchDevices`), and the last save to + * settle reads the list again, after a failure too, a 409 from someone else's + * save included. */ export function useSaveDevice(id: string) { const queryClient = useQueryClient() @@ -64,8 +84,13 @@ export function useSaveDevice(id: string) { : all, ) return useMutation({ + mutationKey: deviceKeys.save, scope: { id: `device:${id}` }, - onMutate: (changes: Partial) => patch(changes), + onMutate: async (changes: Partial) => { + // One already on its way would land over this change. + await queryClient.cancelQueries({ queryKey: deviceKeys.all, exact: true }) + patch(changes) + }, mutationFn: (changes: Partial) => { const current = stored() if (!current) throw new Error(`No device named ${id}`) @@ -87,7 +112,11 @@ export function useSaveDevice(id: string) { // The version alone: the cache already shows this change, and a save // queued behind it may have shown another since. onSuccess: (saved) => patch({ version: saved.version }), - onError: () => queryClient.invalidateQueries({ queryKey: deviceKeys.all }), + // Still counted while it settles, so 1 is this one alone. + onSettled: () => { + if (queryClient.isMutating({ mutationKey: deviceKeys.save }) === 1) + queryClient.invalidateQueries({ queryKey: deviceKeys.all, exact: true }) + }, }) } diff --git a/frontend/src/components/Flow/useFlowSocket.ts b/frontend/src/components/Flow/useFlowSocket.ts index e80325d..bf4824d 100644 --- a/frontend/src/components/Flow/useFlowSocket.ts +++ b/frontend/src/components/Flow/useFlowSocket.ts @@ -4,7 +4,7 @@ import { useEffect } from "react" import { ApiError, FlowsService, OpenAPI } from "@/client" import { dashboardKeys } from "@/components/Dashboard/queries" -import { deviceKeys } from "@/components/Devices/queries" +import { deviceKeys, refetchDevices } from "@/components/Devices/queries" import { healthKeys } from "@/components/Health/queries" import { runKeys } from "@/components/Runs/queries" import { connectionStore } from "@/lib/connectionStore" @@ -418,16 +418,16 @@ function open(url: string) { // `exact` matters โ€” the list key is a prefix of every detail key, // including the draft an editor may have open. client?.invalidateQueries({ queryKey: dashboardKeys.all, exact: true }) - client?.invalidateQueries({ - queryKey: message.dashboard - ? dashboardKeys.detail(message.dashboard) - : deviceKeys.all, - }) + if (message.dashboard) + client?.invalidateQueries({ + queryKey: dashboardKeys.detail(message.dashboard), + }) + else if (client) refetchDevices(client, false) break case "device_status": // A device's socket opened or closed: the Devices tab reads presence // off the list. - client?.invalidateQueries({ queryKey: deviceKeys.all, exact: true }) + if (client) refetchDevices(client) break case "run_started": case "run_finished": diff --git a/frontend/src/routes/panel.index.tsx b/frontend/src/routes/panel.index.tsx index 92faa12..d9a5674 100644 --- a/frontend/src/routes/panel.index.tsx +++ b/frontend/src/routes/panel.index.tsx @@ -170,6 +170,21 @@ function PairPanel() { ) : null} + {/* Through the portal only the code pairs, so there is no list to + pick this screen from. */} + {started?.check && !isPortal() ? ( +

+ Picked from the waiting list instead? Its check is{" "} + + {started.check} + + . +

+ ) : null} +

The code lasts ten minutes; this screen replaces it when it runs out.