Explorar el Código

fix(cloud): only sign out on Bambu's real token-expiry 401, not any 401

Since #2562, a Bambu Cloud sign-in flipped to "expired" and forced constant
re-logins even while cloud features worked. #2562 made a 401 durably record
the stored token as dead, but treated *any* 401 from any cloud/MakerWorld call
as expiry. Bambu 401s for benign reasons (endpoint/region/scope refusals,
Cloudflare edge, transient blips), so one stray 401 -- including from a
background poll -- signed the whole cloud integration out until manual re-login.
The flag lives in the DB, so a setup with more than one instance against the
same database signed the user out across all of them.

Invalidate only on Bambu's documented expiry body {"code":4,"error":"Please
login."}. A plain/unparseable 401 is treated as transient: the request fails
but the session stays signed in. validate_token maps a signature-less 401 to
None (unknown), never expired. A shared is_expiry_401() gates both the Bambu
Cloud and MakerWorld services (same token). Genuine expiry is still detected
and surfaced exactly as before.
maziggy hace 1 mes
padre
commit
ba40f5731d

La diferencia del archivo ha sido suprimido porque es demasiado grande
+ 1 - 0
CHANGELOG.md


+ 50 - 10
backend/app/services/bambu_cloud.py

@@ -32,6 +32,33 @@ def _token_digest(token: str) -> str:
     return hashlib.sha256(token.encode("utf-8")).hexdigest()
     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:
 def invalidate_validation_cache(token: str | None = None) -> None:
     """Drop cached validation verdicts.
     """Drop cached validation verdicts.
 
 
@@ -197,16 +224,25 @@ class BambuCloudService:
             return False
             return False
         return not (self.token_expiry and datetime.now(timezone.utc) > self.token_expiry)
         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
         self._auth_failure_reported = True
         if self.access_token:
         if self.access_token:
             _validation_cache[_token_digest(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
             # Recording the failure is best-effort — the caller still needs the
             # real error (a 401) rather than a bookkeeping exception on top.
             # real error (a 401) rather than a bookkeeping exception on top.
             logger.exception("Failed to record Bambu Cloud auth failure")
             logger.exception("Failed to record Bambu Cloud auth failure")
+        return True
 
 
     async def validate_token(self) -> bool | None:
     async def validate_token(self) -> bool | None:
         """Ask Bambu whether the loaded token is still accepted.
         """Ask Bambu whether the loaded token is still accepted.
@@ -249,8 +286,11 @@ class BambuCloudService:
             return None
             return None
 
 
         if response.status_code == 401:
         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:
         if response.status_code >= 500:
             logger.info(
             logger.info(
                 "Bambu Cloud returned %s while validating the token — treating as unknown", response.status_code
                 "Bambu Cloud returned %s while validating the token — treating as unknown", response.status_code

+ 16 - 4
backend/app/services/makerworld.py

@@ -28,6 +28,8 @@ from urllib.parse import urlparse
 import certifi
 import certifi
 import httpx
 import httpx
 
 
+from backend.app.services.bambu_cloud import is_expiry_401
+
 logger = logging.getLogger(__name__)
 logger = logging.getLogger(__name__)
 
 
 
 
@@ -255,8 +257,18 @@ class MakerWorldService:
         if self._owns_client:
         if self._owns_client:
             await self._client.aclose()
             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:
         if self._on_auth_failure is None or self._auth_failure_reported:
             return
             return
         self._auth_failure_reported = True
         self._auth_failure_reported = True
@@ -307,7 +319,7 @@ class MakerWorldService:
                 # We sent a token and Bambu refused it — the credential is dead,
                 # We sent a token and Bambu refused it — the credential is dead,
                 # not merely absent. Record that before raising so the rest of the
                 # not merely absent. Record that before raising so the rest of the
                 # app stops claiming the user is connected.
                 # 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(_SIGN_IN_EXPIRED_MESSAGE)
             raise MakerWorldAuthError(f"Signing in to Bambu Cloud is required for {path}")
             raise MakerWorldAuthError(f"Signing in to Bambu Cloud is required for {path}")
         if response.status_code == 403:
         if response.status_code == 403:
@@ -469,7 +481,7 @@ class MakerWorldService:
             raise MakerWorldUnavailableError(f"Bambu Lab API request failed: {exc}") from exc
             raise MakerWorldUnavailableError(f"Bambu Lab API request failed: {exc}") from exc
 
 
         if response.status_code == 401:
         if response.status_code == 401:
-            await self._note_auth_failure()
+            await self._note_auth_failure(response)
             raise MakerWorldAuthError(_SIGN_IN_EXPIRED_MESSAGE)
             raise MakerWorldAuthError(_SIGN_IN_EXPIRED_MESSAGE)
         if response.status_code == 403:
         if response.status_code == 403:
             upstream = _extract_upstream_error(response)
             upstream = _extract_upstream_error(response)

+ 26 - 0
backend/tests/unit/services/test_makerworld.py

@@ -199,6 +199,32 @@ class TestGetDesign:
         assert "Profiles" in message
         assert "Profiles" in message
         assert marked == [True], "a rejected token must be recorded as dead"
         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
     @pytest.mark.asyncio
     async def test_maps_403_to_forbidden_with_upstream_reason(self, service):
     async def test_maps_403_to_forbidden_with_upstream_reason(self, service):
         """403 is distinct from 401: auth was valid, MakerWorld refuses the
         """403 is distinct from 401: auth was valid, MakerWorld refuses the

+ 57 - 2
backend/tests/unit/test_cloud_token_expiry.py

@@ -45,10 +45,27 @@ def _clear_validation_cache():
     bc.invalidate_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)
     svc = BambuCloudService(client=MagicMock(spec=httpx.AsyncClient), on_auth_failure=on_auth_failure)
     resp = MagicMock()
     resp = MagicMock()
     resp.status_code = status_code
     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)
     svc._client.get = AsyncMock(side_effect=raises) if raises else AsyncMock(return_value=resp)
     return svc
     return svc
 
 
@@ -79,7 +96,30 @@ class TestValidateToken:
 
 
     @pytest.mark.asyncio
     @pytest.mark.asyncio
     async def test_rejected_token_returns_false(self):
     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")
         svc.set_token("dead-token")
         assert await svc.validate_token() is False
         assert await svc.validate_token() is False
 
 
@@ -170,11 +210,26 @@ class TestAuthFailureCallback:
         svc.set_token("dead-token")
         svc.set_token("dead-token")
         resp = MagicMock()
         resp = MagicMock()
         resp.status_code = 401
         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)
         await svc._note_response(resp)
         await svc._note_response(resp)
         await svc._note_response(resp)
         assert calls == [1]
         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
     @pytest.mark.asyncio
     async def test_success_does_not_fire_the_callback(self):
     async def test_success_does_not_fire_the_callback(self):
         calls: list[int] = []
         calls: list[int] = []

Algunos archivos no se mostraron porque demasiados archivos cambiaron en este cambio