diff --git a/CHANGELOG.md b/CHANGELOG.md index e47df7974..15d74c9cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to Bambuddy will be documented in this file. - **Orca Cloud profile sync now connects by approving a code instead of the copy-paste sign-in** — Connecting Bambuddy to Orca Cloud used to mean opening an OAuth sign-in in a new tab, watching it redirect to a `localhost` URL that fails to load, then copying that dead URL out of the address bar and pasting it back into Bambuddy. That dance existed only because Orca's auth backend (Supabase) accepts no redirect target other than `localhost`, and the deliberately-broken redirect page confused nearly everyone who reached it. OrcaSlicer has since shipped a first-class external-app pairing API (the OAuth 2.0 Device Authorization Grant, RFC 8628), so the flow is now: click **Connect**, approve a short code on your Orca Cloud settings page, and Bambuddy pairs itself — no redirect, no paste, no client secret, and it behaves identically from a LAN IP, `localhost`, or behind a reverse proxy. Bambuddy requests **read-only** access (it only lists and views your Orca Cloud profiles), keeps the pairing alive with the API's rotating refresh tokens (validated end-to-end against Orca's staging and production servers), and stores nothing beyond the issued token pair. The profile list and detail views are unchanged, so nothing downstream of the connect step looks different. The old paste-based sign-in and the email/password fallback are removed. Points at production Orca Cloud by default; `ORCA_CLOUD_API_BASE` overrides the endpoint for testing. ### Fixed +- **Bambu Cloud kept dropping to "sign-in expired" and forcing constant re-logins, even while cloud features still worked** — Since the #2562 status rework, the Bambu Cloud sign-in would flip to "expired" shortly after logging in, over and over, with nothing in the logs to explain it. **Root cause.** The rework made a genuine 401 from Bambu durably record the stored token as dead (`cloud_token_invalid_at`) — correct in principle, but it treated **any** HTTP 401 from **any** cloud or MakerWorld call as a dead token. Bambu returns 401 for plenty of non-fatal reasons (an endpoint-, region- or scope-specific refusal; a Cloudflare-edge blip; a brief backend hiccup), so a single stray 401 from any one call — including a background poll — durably signed the whole cloud integration out until the next manual re-login. Because the flag lives in the database, a setup running more than one Bambuddy instance against the same database made it worse: a stray 401 seen by either instance signed the user out in both. **Fix.** Invalidation now fires only for Bambu's documented token-expiry response — `{"code":4,"error":"Please login."}` — and never for a plain or unparseable 401, which is treated as transient (the request fails, but the session is left signed in). The same signature gate is applied to the MakerWorld path, which shares the token. A genuinely expired token is still detected and surfaced exactly as before; what stops is the false "expired" on a working session. Covered by service tests for both engines: `code:4` and the "Please login." text invalidate, while a benign `code:1`/`forbidden` 401, an unparseable 401, and (for MakerWorld) a signature-less 401 with a token do not. - **Every SpoolBuddy screen crashed the moment a text field was focused (#2616, reporters @MartinNYHC, @agentdr8)** — Tapping the Search box on the SpoolBuddy inventory, or the Search / Color Name / Brand fields on the write-tag New Spool tab, blanked the UI with a minified React error #130 ("Element type is invalid… but got: object"). It hit both internal and Spoolman inventories, so it was not data-specific. **Root cause.** The SpoolBuddy shell mounts an on-screen keyboard (`VirtualKeyboard`) that pops up on `focusin` for any text input — which is why every field on every SpoolBuddy page tripped it, while the main app (no on-screen keyboard) was fine. That component does `import Keyboard from 'react-simple-keyboard'`, a CommonJS package, and under the current bundler's CJS→ESM interop the default import resolves to the module **namespace object** (`{ KeyboardReact, default }`) rather than the component itself. Rendering that object as `` put an object where an element type belongs, and React threw. (The discrepancy is interop-specific: the test runner hands back the real component, so it only manifested in the browser build — which is why it needed a runtime, not a type, fix.) **Fix.** A small `resolveInteropDefault` helper unwraps such an interop-wrapped default: it returns the value as-is when it's already a usable element type (function/class, tag string, or a `$$typeof`-marked forwardRef/memo/lazy) and otherwise falls through to `.default` and named exports. `VirtualKeyboard` resolves the real `react-simple-keyboard` component through it, so the keyboard renders under any interop shape. Covered by unit tests for the resolver against the object shape, a named-only export, a forwardRef object, and a bare component, plus a render test that mounts the keyboard on input focus. - **The streaming overlay (`/overlay`) showed nothing in OBS when login was enabled (#2613, reporter @MartinNYHC)** — With authentication on, the overlay page worked when opened in a browser where you were already signed in, but stayed blank in OBS. The reporter suspected their Cloudflare/remote setup; it was unrelated. **Root cause.** The `/overlay/{id}` *route* renders without a login, but every piece of data it draws is auth-gated — printer status and name (`PRINTERS_READ`), one setting (`SETTINGS_READ`), and the camera stream (a camera-stream token). In your own browser those ride the JWT from local storage and the app-wide stream-token sync; OBS is a fresh browser with **no session**, so the status calls 401'd and the overlay never populated (the same would happen in any private/incognito window — remote access was never the cause). Unlike the Cam Wall (`/camwall?token=…`), the overlay had no token mode, and a long-lived token couldn't help because the JWT-gated status/settings endpoints reject it. **Fix.** The overlay is now a self-contained kiosk surface. A new **Streaming Overlay** long-lived-token scope is offered under Settings → API Keys (with a ready-made `/overlay/{id}?token=…` URL copied once on creation); the overlay page reads `?token=` from the URL and, in that mode, authenticates its status and camera calls with the token instead of a JWT (and skips the WebSocket, falling back to its existing 2 s poll). A new token-authenticated `GET /printers/{id}/overlay-status` returns exactly the fields the overlay draws — name, camera rotation, live print state, and the one setting — and nothing else. The scope is deliberately **separate from `camwall`**: the overlay names the file on screen, which the Cam Wall is trusted never to expose, so folding it in would have silently widened every existing wall token. The logged-in path (opening the overlay while signed in) is unchanged. Docs updated to explain the token and stop claiming the overlay needs no authentication. Covered by backend tests (scope boundaries in both directions — an overlay token can't reach the Cam Wall feed and a camwall/camera-stream token can't reach the overlay feed — plus the payload shape, disconnected-printer shape, and revoked/absent/garbage-token rejection) and frontend tests (kiosk mode reads the token feed and carries the token to the camera, never touches the JWT-only status endpoint or a socket; the mint UI offers the scope and hands over the assembled OBS URL). - **Reassigning a queue item while it was dispatching split it across two printers (#2615, reporter @Jostxxl)** — Editing a queue item's printer while its FTP upload was already in flight left the queue row pointing at one printer while the archive, expected-print registration, and the physical `project_file` command had gone to another. On a farm this made the reassigned-to printer look broken (marked `printing` but never sent the job), left the row permanently inconsistent, and could trigger a duplicate dispatch after a restart. **Root cause.** A queue row stays `status='pending'` for the entire (multi-minute) FTP upload — status only flips to `printing` at the very end. The edit route only blocked non-`pending` rows, so a `PATCH` during the upload window was accepted; the in-flight dispatch kept using the printer it had snapshotted at the start, while the DB row's `printer_id` changed underneath it. The existing #1853 CAS guards *cancellation* mid-dispatch, not *reassignment*. **Fix.** A `dispatching_at` claim is stamped atomically on the row (`WHERE status='pending' AND dispatching_at IS NULL`) the moment the scheduler begins dispatching, before any slow I/O, and cleared when dispatch ends. While it's held, both edit routes reject changes — the single-item `PATCH` returns **409** (re-checked immediately before the write to close the read-modify-write gap), and bulk edits skip the row — and the scheduler's selection query won't re-pick it. Startup reconciliation clears any claim orphaned by a crash mid-dispatch (no dispatch coroutine survives a restart, so every claim present at boot is stale), so a stale token can never wedge an item out of the queue. The row stays `pending` throughout, so no status-consumer, UI, completion, or reconciliation path had to change. To move a dispatching item, cancel it first (the coordinated escape) and re-queue. New column `print_queue.dispatching_at` (nullable timestamp, dialect-safe DDL — SQLite `DATETIME` / Postgres `TIMESTAMP`). Covered by scheduler tests (claim is exclusive, fails on non-pending rows, releases on every exit, skips an already-claimed row, startup clears stale claims) and API tests (reassign returns 409 with `printer_id` unchanged, bulk skips the claimed row, an unclaimed pending row still edits normally). diff --git a/backend/app/services/bambu_cloud.py b/backend/app/services/bambu_cloud.py index 98edb0a82..0f69177a1 100644 --- a/backend/app/services/bambu_cloud.py +++ b/backend/app/services/bambu_cloud.py @@ -32,6 +32,33 @@ def _token_digest(token: str) -> str: return hashlib.sha256(token.encode("utf-8")).hexdigest() +def is_expiry_401(response: httpx.Response) -> bool: + """Whether a 401 is Bambu's genuine "token expired" signal. + + Bambu answers an expired/revoked token with ``{"code":4,"error":"Please + login.","message":""}``. Not every 401 means that: individual endpoints + return 401 for resource-, region- or scope-specific reasons, and a working + token still draws the occasional transient 401 (Cloudflare edge, a brief + backend blip). Treating *any* 401 as a dead credential signs the user out on + a single stray rejection — the #2562 follow-up regression. We trust only the + documented expiry body, so a benign 401 no longer nukes the whole cloud + integration. An unparseable / unsigned 401 is deliberately NOT expiry. + + Shared by the Bambu Cloud and MakerWorld services — both carry the same + token and see the same expiry body. + """ + try: + body = response.json() + except Exception: + return False + if not isinstance(body, dict): + return False + if body.get("code") == 4: + return True + text = f"{body.get('error', '')} {body.get('message', '')}".lower() + return "please login" in text + + def invalidate_validation_cache(token: str | None = None) -> None: """Drop cached validation verdicts. @@ -197,16 +224,25 @@ class BambuCloudService: return False return not (self.token_expiry and datetime.now(timezone.utc) > self.token_expiry) - async def _note_response(self, response: httpx.Response) -> None: - """Record a 401 from Bambu as "this stored credential is dead". + async def _note_response(self, response: httpx.Response) -> bool: + """Record Bambu's genuine token-expiry 401 as "this credential is dead". - Bambu answers an expired/revoked token with 401 and a body of - ``{"code":4,"error":"Please login.","message":""}``. Reported at most - once per service instance so a route that makes several calls doesn't - write the flag several times. + Returns ``True`` only for the real expiry signal (see + :meth:`_is_expiry_401`); a plain/transient 401 returns ``False`` and is + left alone so it can't durably sign the user out. The durable flag is + written at most once per service instance so a route making several + calls doesn't write it repeatedly. """ - if response.status_code != 401 or self._on_auth_failure is None or self._auth_failure_reported: - return + if response.status_code != 401: + return False + if not is_expiry_401(response): + logger.info( + "Bambu Cloud returned 401 without the expiry signature — treating as transient, " + "not signing the stored token out" + ) + return False + if self._on_auth_failure is None or self._auth_failure_reported: + return True self._auth_failure_reported = True if self.access_token: _validation_cache[_token_digest(self.access_token)] = ( @@ -219,6 +255,7 @@ class BambuCloudService: # Recording the failure is best-effort — the caller still needs the # real error (a 401) rather than a bookkeeping exception on top. logger.exception("Failed to record Bambu Cloud auth failure") + return True async def validate_token(self) -> bool | None: """Ask Bambu whether the loaded token is still accepted. @@ -249,8 +286,11 @@ class BambuCloudService: return None if response.status_code == 401: - await self._note_response(response) - return False + # Only a 401 carrying Bambu's expiry signature is a real sign-out. + # A signature-less 401 here is transient/edge noise — report unknown + # (last-known state) rather than expiring a working session. + expired = await self._note_response(response) + return False if expired else None if response.status_code >= 500: logger.info( "Bambu Cloud returned %s while validating the token — treating as unknown", response.status_code diff --git a/backend/app/services/makerworld.py b/backend/app/services/makerworld.py index 7ea2b55fd..592285b6c 100644 --- a/backend/app/services/makerworld.py +++ b/backend/app/services/makerworld.py @@ -28,6 +28,8 @@ from urllib.parse import urlparse import certifi import httpx +from backend.app.services.bambu_cloud import is_expiry_401 + logger = logging.getLogger(__name__) @@ -255,8 +257,18 @@ class MakerWorldService: if self._owns_client: await self._client.aclose() - async def _note_auth_failure(self) -> None: - """Record that Bambu rejected the token we sent. Best-effort, once.""" + async def _note_auth_failure(self, response: httpx.Response) -> None: + """Durably record a dead credential — only for Bambu's genuine expiry 401. + + A MakerWorld 401 without the ``{"code":4,"error":"Please login."}`` + signature is endpoint- or edge-specific noise, not an expired token; + invalidating on it would sign the user out of the whole cloud + integration on a single stray rejection (the #2562 follow-up + regression). Best-effort, once per service instance. + """ + if not is_expiry_401(response): + logger.info("MakerWorld returned 401 without the expiry signature — not signing the stored token out") + return if self._on_auth_failure is None or self._auth_failure_reported: return self._auth_failure_reported = True @@ -307,7 +319,7 @@ class MakerWorldService: # We sent a token and Bambu refused it — the credential is dead, # not merely absent. Record that before raising so the rest of the # app stops claiming the user is connected. - await self._note_auth_failure() + await self._note_auth_failure(response) raise MakerWorldAuthError(_SIGN_IN_EXPIRED_MESSAGE) raise MakerWorldAuthError(f"Signing in to Bambu Cloud is required for {path}") if response.status_code == 403: @@ -469,7 +481,7 @@ class MakerWorldService: raise MakerWorldUnavailableError(f"Bambu Lab API request failed: {exc}") from exc if response.status_code == 401: - await self._note_auth_failure() + await self._note_auth_failure(response) raise MakerWorldAuthError(_SIGN_IN_EXPIRED_MESSAGE) if response.status_code == 403: upstream = _extract_upstream_error(response) diff --git a/backend/tests/unit/services/test_makerworld.py b/backend/tests/unit/services/test_makerworld.py index 318b42452..55ac33cf1 100644 --- a/backend/tests/unit/services/test_makerworld.py +++ b/backend/tests/unit/services/test_makerworld.py @@ -199,6 +199,32 @@ class TestGetDesign: assert "Profiles" in message assert marked == [True], "a rejected token must be recorded as dead" + @pytest.mark.asyncio + async def test_transient_401_with_token_does_not_invalidate(self): + """A 401 WITHOUT Bambu's expiry signature (endpoint/edge noise) must fail + the request but NOT durably sign the user out — otherwise one stray 401 + from any single MakerWorld call kills the whole cloud integration.""" + marked: list[bool] = [] + + async def _on_auth_failure() -> None: + marked.append(True) + + svc = MakerWorldService( + client=MagicMock(spec=httpx.AsyncClient), + auth_token="tok-abc", + on_auth_failure=_on_auth_failure, + ) + svc._client.get = AsyncMock() + resp = MagicMock() + resp.status_code = 401 + resp.json.return_value = {"code": 1, "error": "forbidden"} + svc._client.get.return_value = resp + + with pytest.raises(MakerWorldAuthError): + await svc.get_design(1) + + assert marked == [], "a benign 401 must not record the credential as dead" + @pytest.mark.asyncio async def test_maps_403_to_forbidden_with_upstream_reason(self, service): """403 is distinct from 401: auth was valid, MakerWorld refuses the diff --git a/backend/tests/unit/test_cloud_token_expiry.py b/backend/tests/unit/test_cloud_token_expiry.py index 6ed7eb3e3..b3e9f2c90 100644 --- a/backend/tests/unit/test_cloud_token_expiry.py +++ b/backend/tests/unit/test_cloud_token_expiry.py @@ -45,10 +45,27 @@ def _clear_validation_cache(): bc.invalidate_validation_cache() -def _service(status_code: int = 200, *, on_auth_failure=None, raises: Exception | None = None): +# Bambu's genuine "token expired" 401 body — the only 401 that means sign-out. +_EXPIRY_401_BODY = {"code": 4, "error": "Please login.", "message": ""} + + +def _service( + status_code: int = 200, + *, + on_auth_failure=None, + raises: Exception | None = None, + body: object | None = None, + json_raises: bool = False, +): svc = BambuCloudService(client=MagicMock(spec=httpx.AsyncClient), on_auth_failure=on_auth_failure) resp = MagicMock() resp.status_code = status_code + # A 401 defaults to Bambu's expiry body so existing "rejected token" cases + # mean a real expiry; pass body= to exercise a transient/benign 401. + if json_raises: + resp.json = MagicMock(side_effect=ValueError("not json")) + else: + resp.json = MagicMock(return_value=_EXPIRY_401_BODY if (body is None and status_code == 401) else (body or {})) svc._client.get = AsyncMock(side_effect=raises) if raises else AsyncMock(return_value=resp) return svc @@ -79,7 +96,30 @@ class TestValidateToken: @pytest.mark.asyncio async def test_rejected_token_returns_false(self): - svc = _service(401) + svc = _service(401) # defaults to Bambu's genuine expiry body + svc.set_token("dead-token") + assert await svc.validate_token() is False + + @pytest.mark.asyncio + async def test_transient_401_is_unknown_not_invalid(self): + """A 401 WITHOUT Bambu's expiry signature is edge/endpoint noise, not a + dead token — it must read as unknown, never sign the user out. This is + the regression that logged users out on a single stray 401.""" + svc = _service(401, body={"code": 1, "error": "forbidden"}) + svc.set_token("good-token") + assert await svc.validate_token() is None + + @pytest.mark.asyncio + async def test_unparseable_401_is_unknown_not_invalid(self): + svc = _service(401, json_raises=True) + svc.set_token("good-token") + assert await svc.validate_token() is None + + @pytest.mark.asyncio + async def test_expiry_signature_via_please_login_text(self): + """The `code:4` field is primary, but the "Please login." text alone + (no/other code) is still accepted as the expiry signal.""" + svc = _service(401, body={"error": "Please login.", "message": ""}) svc.set_token("dead-token") assert await svc.validate_token() is False @@ -170,11 +210,26 @@ class TestAuthFailureCallback: svc.set_token("dead-token") resp = MagicMock() resp.status_code = 401 + resp.json = MagicMock(return_value=_EXPIRY_401_BODY) await svc._note_response(resp) await svc._note_response(resp) await svc._note_response(resp) assert calls == [1] + @pytest.mark.asyncio + async def test_transient_401_does_not_fire_the_callback(self): + """A benign 401 must not durably invalidate — the callback that persists + the dead-token flag stays untouched.""" + calls: list[int] = [] + + async def _cb() -> None: + calls.append(1) + + svc = _service(401, on_auth_failure=_cb, body={"code": 1, "error": "forbidden"}) + svc.set_token("good-token") + await svc.validate_token() + assert calls == [] + @pytest.mark.asyncio async def test_success_does_not_fire_the_callback(self): calls: list[int] = []