diff --git a/backend/app/api/routes/cloud.py b/backend/app/api/routes/cloud.py index 28ac259c9..238189db9 100644 --- a/backend/app/api/routes/cloud.py +++ b/backend/app/api/routes/cloud.py @@ -36,7 +36,7 @@ from backend.app.schemas.cloud import ( from backend.app.services.bambu_cloud import ( BambuCloudAuthError, BambuCloudError, - get_cloud_service, + BambuCloudService, ) from backend.app.utils.filament_ids import filament_id_to_setting_id @@ -48,40 +48,57 @@ router = APIRouter(prefix="/cloud", tags=["cloud"]) # Keys for storing cloud credentials in settings CLOUD_TOKEN_KEY = "bambu_cloud_token" CLOUD_EMAIL_KEY = "bambu_cloud_email" +CLOUD_REGION_KEY = "bambu_cloud_region" -async def get_stored_token(db: AsyncSession, user: User | None = None) -> tuple[str | None, str | None]: - """Get stored cloud token and email. +def _normalise_region(region: str | None) -> str: + """Treat NULL/empty as 'global' for legacy rows that predate the region column.""" + return region if region in ("global", "china") else "global" + + +async def get_stored_token(db: AsyncSession, user: User | None = None) -> tuple[str | None, str | None, str]: + """Get stored cloud token, email, and region. When a user is provided (auth enabled), returns that user's per-user credentials. When user is None (auth disabled), falls back to global Settings table. + Region defaults to ``"global"`` when unset (including for rows that predate + the ``cloud_region`` column). """ if user is not None: - return user.cloud_token, user.cloud_email + return user.cloud_token, user.cloud_email, _normalise_region(user.cloud_region) # Fallback: global storage (auth disabled) - result = await db.execute(select(Settings).where(Settings.key.in_([CLOUD_TOKEN_KEY, CLOUD_EMAIL_KEY]))) + result = await db.execute( + select(Settings).where(Settings.key.in_([CLOUD_TOKEN_KEY, CLOUD_EMAIL_KEY, CLOUD_REGION_KEY])) + ) settings = {s.key: s.value for s in result.scalars().all()} - return settings.get(CLOUD_TOKEN_KEY), settings.get(CLOUD_EMAIL_KEY) + return ( + settings.get(CLOUD_TOKEN_KEY), + settings.get(CLOUD_EMAIL_KEY), + _normalise_region(settings.get(CLOUD_REGION_KEY)), + ) -async def store_token(db: AsyncSession, token: str, email: str, user: User | None = None) -> None: - """Store cloud token and email. +async def store_token(db: AsyncSession, token: str, email: str, region: str, user: User | None = None) -> None: + """Store cloud token, email, and region. When a user is provided (auth enabled), stores on the user record. When user is None (auth disabled), stores in global Settings table. """ + region = _normalise_region(region) if user is not None: # User object is from the auth dependency's session (detached), # so use a direct UPDATE via the route's db session. from sqlalchemy import update - await db.execute(update(User).where(User.id == user.id).values(cloud_token=token, cloud_email=email)) + await db.execute( + update(User).where(User.id == user.id).values(cloud_token=token, cloud_email=email, cloud_region=region) + ) await db.commit() return # Fallback: global storage (auth disabled) - for key, value in [(CLOUD_TOKEN_KEY, token), (CLOUD_EMAIL_KEY, email)]: + for key, value in [(CLOUD_TOKEN_KEY, token), (CLOUD_EMAIL_KEY, email), (CLOUD_REGION_KEY, region)]: result = await db.execute(select(Settings).where(Settings.key == key)) setting = result.scalar_one_or_none() if setting: @@ -92,7 +109,7 @@ async def store_token(db: AsyncSession, token: str, email: str, user: User | Non async def clear_token(db: AsyncSession, user: User | None = None) -> None: - """Clear stored cloud token and email. + """Clear stored cloud token, email, and region. When a user is provided (auth enabled), clears that user's credentials. When user is None (auth disabled), clears from global Settings table. @@ -100,33 +117,62 @@ async def clear_token(db: AsyncSession, user: User | None = None) -> None: if user is not None: from sqlalchemy import update - await db.execute(update(User).where(User.id == user.id).values(cloud_token=None, cloud_email=None)) + await db.execute( + update(User).where(User.id == user.id).values(cloud_token=None, cloud_email=None, cloud_region=None) + ) await db.commit() return # Fallback: global storage (auth disabled) - result = await db.execute(select(Settings).where(Settings.key.in_([CLOUD_TOKEN_KEY, CLOUD_EMAIL_KEY]))) + result = await db.execute( + select(Settings).where(Settings.key.in_([CLOUD_TOKEN_KEY, CLOUD_EMAIL_KEY, CLOUD_REGION_KEY])) + ) for setting in result.scalars().all(): await db.delete(setting) await db.commit() +async def build_authenticated_cloud(db: AsyncSession, user: User | None) -> BambuCloudService | None: + """Build a per-request cloud service seeded with the caller's stored token + region. + + Returns ``None`` when no token is stored, so callers can 401 without constructing + (and then closing) a useless client. Caller is responsible for ``await cloud.close()``. + """ + token, _email, region = await get_stored_token(db, user) + if not token: + return None + cloud = BambuCloudService(region=region) + cloud.set_token(token) + return cloud + + @router.get("/status", response_model=CloudAuthStatus) async def get_auth_status( db: AsyncSession = Depends(get_db), current_user: User | None = RequirePermissionIfAuthEnabled(Permission.CLOUD_AUTH), ): - """Get current cloud authentication status.""" - token, email = await get_stored_token(db, current_user) - cloud = get_cloud_service() + """Get current cloud authentication status. - if token: - cloud.set_token(token) + Reads the stored credentials in one DB round-trip (we used to call + ``get_stored_token`` twice — once here and once inside + ``build_authenticated_cloud``). ``region`` is exposed so the frontend can + show "Connected (China)" after a reload without relying on local state. + """ + token, email, region = await get_stored_token(db, current_user) + if not token: + return CloudAuthStatus(is_authenticated=False, email=None, region=None) - return CloudAuthStatus( - is_authenticated=cloud.is_authenticated, - email=email if cloud.is_authenticated else None, - ) + cloud = BambuCloudService(region=region) + cloud.set_token(token) + try: + authenticated = cloud.is_authenticated + return CloudAuthStatus( + is_authenticated=authenticated, + email=email if authenticated else None, + region=region if authenticated else None, + ) + finally: + await cloud.close() @router.post("/login", response_model=CloudLoginResponse) @@ -145,14 +191,14 @@ async def login( After receiving/generating the code, call /cloud/verify to complete the login. For TOTP, include the tfa_key from this response in the verify request. """ - cloud = get_cloud_service() + cloud = BambuCloudService(region=request.region) try: result = await cloud.login_request(request.email, request.password) if result.get("success") and cloud.access_token: # Direct login succeeded (rare) - await store_token(db, cloud.access_token, request.email, current_user) + await store_token(db, cloud.access_token, request.email, request.region, current_user) return CloudLoginResponse( success=result.get("success", False), @@ -165,6 +211,8 @@ async def login( raise HTTPException(status_code=401, detail=str(e)) except BambuCloudError as e: raise HTTPException(status_code=500, detail=str(e)) + finally: + await cloud.close() @router.post("/verify", response_model=CloudLoginResponse) @@ -183,8 +231,11 @@ async def verify_code( For TOTP verification: - The user enters the 6-digit code from their authenticator app - Include the tfa_key from the /cloud/login response + + ``request.region`` must match the region used in /cloud/login so that the + TOTP call hits the correct TFA endpoint (bambulab.com vs bambulab.cn). """ - cloud = get_cloud_service() + cloud = BambuCloudService(region=request.region) try: # Use TOTP verification if tfa_key is provided @@ -194,7 +245,7 @@ async def verify_code( result = await cloud.verify_code(request.email, request.code) if result.get("success") and cloud.access_token: - await store_token(db, cloud.access_token, request.email, current_user) + await store_token(db, cloud.access_token, request.email, request.region, current_user) return CloudLoginResponse( success=result.get("success", False), @@ -205,6 +256,8 @@ async def verify_code( raise HTTPException(status_code=401, detail=str(e)) except BambuCloudError as e: raise HTTPException(status_code=500, detail=str(e)) + finally: + await cloud.close() @router.post("/token", response_model=CloudAuthStatus) @@ -216,19 +269,22 @@ async def set_token( """ Set access token directly. - For users who already have a token (e.g., from Bambu Studio). + For users who already have a token (e.g., from Bambu Studio). The + selected ``region`` is persisted alongside the token so every subsequent + request hits the right Bambu API endpoint, including after a restart. """ - cloud = get_cloud_service() + cloud = BambuCloudService(region=request.region) cloud.set_token(request.access_token) - # Verify token works by trying to get profile try: + # Verify token works by trying to get profile await cloud.get_user_profile() - await store_token(db, request.access_token, "token-auth", current_user) + await store_token(db, request.access_token, "token-auth", request.region, current_user) return CloudAuthStatus(is_authenticated=True, email="token-auth") except BambuCloudError: - cloud.logout() raise HTTPException(status_code=401, detail="Invalid token") + finally: + await cloud.close() @router.post("/logout") @@ -237,8 +293,6 @@ async def logout( current_user: User | None = RequirePermissionIfAuthEnabled(Permission.CLOUD_AUTH), ): """Log out of Bambu Cloud.""" - cloud = get_cloud_service() - cloud.logout() await clear_token(db, current_user) return {"success": True} @@ -254,14 +308,8 @@ async def get_slicer_settings( Requires authentication. """ - token, _ = await get_stored_token(db, current_user) - if not token: - raise HTTPException(status_code=401, detail="Not authenticated") - - cloud = get_cloud_service() - cloud.set_token(token) - - if not cloud.is_authenticated: + cloud = await build_authenticated_cloud(db, current_user) + if cloud is None or not cloud.is_authenticated: raise HTTPException(status_code=401, detail="Not authenticated") try: @@ -316,6 +364,8 @@ async def get_slicer_settings( raise HTTPException(status_code=401, detail="Authentication expired") except BambuCloudError as e: raise HTTPException(status_code=500, detail=str(e)) + finally: + await cloud.close() @router.get("/settings/{setting_id}") @@ -329,14 +379,8 @@ async def get_setting_detail( Returns the full preset configuration. """ - token, _ = await get_stored_token(db, current_user) - if not token: - raise HTTPException(status_code=401, detail="Not authenticated") - - cloud = get_cloud_service() - cloud.set_token(token) - - if not cloud.is_authenticated: + cloud = await build_authenticated_cloud(db, current_user) + if cloud is None or not cloud.is_authenticated: raise HTTPException(status_code=401, detail="Not authenticated") try: @@ -347,6 +391,8 @@ async def get_setting_detail( raise HTTPException(status_code=401, detail="Authentication expired") except BambuCloudError as e: raise HTTPException(status_code=500, detail=str(e)) + finally: + await cloud.close() @router.get("/filaments", response_model=list[SlicerSetting]) @@ -581,12 +627,9 @@ async def get_filament_info( # Phase 2: Try cloud for uncached IDs if unresolved_ids: - token, _ = await get_stored_token(db, current_user) - if token: - cloud = get_cloud_service() - cloud.set_token(token) - - if cloud.is_authenticated: + cloud = await build_authenticated_cloud(db, current_user) + if cloud is not None and cloud.is_authenticated: + try: still_unresolved: list[str] = [] for setting_id in unresolved_ids: try: @@ -615,6 +658,10 @@ async def get_filament_info( still_unresolved.append(setting_id) unresolved_ids = still_unresolved + finally: + await cloud.close() + elif cloud is not None: + await cloud.close() # Phase 3: Try local profiles for any IDs still without a name if unresolved_ids: @@ -633,14 +680,8 @@ async def get_devices( Returns printers registered to the user's Bambu account. """ - token, _ = await get_stored_token(db, current_user) - if not token: - raise HTTPException(status_code=401, detail="Not authenticated") - - cloud = get_cloud_service() - cloud.set_token(token) - - if not cloud.is_authenticated: + cloud = await build_authenticated_cloud(db, current_user) + if cloud is None or not cloud.is_authenticated: raise HTTPException(status_code=401, detail="Not authenticated") try: @@ -662,6 +703,8 @@ async def get_devices( raise HTTPException(status_code=401, detail="Authentication expired") except BambuCloudError as e: raise HTTPException(status_code=500, detail=str(e)) + finally: + await cloud.close() @router.get("/firmware-updates", response_model=FirmwareUpdatesResponse) @@ -680,14 +723,8 @@ async def get_firmware_updates( Requires cloud authentication. """ - token, _ = await get_stored_token(db, current_user) - if not token: - raise HTTPException(status_code=401, detail="Not authenticated") - - cloud = get_cloud_service() - cloud.set_token(token) - - if not cloud.is_authenticated: + cloud = await build_authenticated_cloud(db, current_user) + if cloud is None or not cloud.is_authenticated: raise HTTPException(status_code=401, detail="Not authenticated") try: @@ -741,6 +778,8 @@ async def get_firmware_updates( raise HTTPException(status_code=401, detail="Authentication expired") except BambuCloudError as e: raise HTTPException(status_code=500, detail=str(e)) + finally: + await cloud.close() @router.post("/settings") @@ -757,14 +796,8 @@ async def create_setting( Type should be: 'filament', 'print', or 'printer' """ - token, _ = await get_stored_token(db, current_user) - if not token: - raise HTTPException(status_code=401, detail="Not authenticated") - - cloud = get_cloud_service() - cloud.set_token(token) - - if not cloud.is_authenticated: + cloud = await build_authenticated_cloud(db, current_user) + if cloud is None or not cloud.is_authenticated: raise HTTPException(status_code=401, detail="Not authenticated") try: @@ -781,6 +814,8 @@ async def create_setting( raise HTTPException(status_code=401, detail="Authentication expired") except BambuCloudError as e: raise HTTPException(status_code=500, detail=str(e)) + finally: + await cloud.close() @router.put("/settings/{setting_id}") @@ -795,14 +830,8 @@ async def update_setting( Updates the preset's name and/or settings on Bambu Cloud. """ - token, _ = await get_stored_token(db, current_user) - if not token: - raise HTTPException(status_code=401, detail="Not authenticated") - - cloud = get_cloud_service() - cloud.set_token(token) - - if not cloud.is_authenticated: + cloud = await build_authenticated_cloud(db, current_user) + if cloud is None or not cloud.is_authenticated: raise HTTPException(status_code=401, detail="Not authenticated") try: @@ -817,6 +846,8 @@ async def update_setting( raise HTTPException(status_code=401, detail="Authentication expired") except BambuCloudError as e: raise HTTPException(status_code=500, detail=str(e)) + finally: + await cloud.close() @router.delete("/settings/{setting_id}", response_model=SlicerSettingDeleteResponse) @@ -830,14 +861,8 @@ async def delete_setting( Removes the preset from Bambu Cloud. This cannot be undone. """ - token, _ = await get_stored_token(db, current_user) - if not token: - raise HTTPException(status_code=401, detail="Not authenticated") - - cloud = get_cloud_service() - cloud.set_token(token) - - if not cloud.is_authenticated: + cloud = await build_authenticated_cloud(db, current_user) + if cloud is None or not cloud.is_authenticated: raise HTTPException(status_code=401, detail="Not authenticated") try: @@ -851,6 +876,8 @@ async def delete_setting( raise HTTPException(status_code=401, detail="Authentication expired") except BambuCloudError as e: raise HTTPException(status_code=500, detail=str(e)) + finally: + await cloud.close() # Path to field definition files @@ -927,13 +954,10 @@ async def get_filament_id_map( if _filament_id_name_cache and time.time() - _filament_id_name_cache_time < FILAMENT_CACHE_TTL: return _filament_id_name_cache - token, _ = await get_stored_token(db, current_user) - if not token: - return _filament_id_name_cache or {} - - cloud = get_cloud_service() - cloud.set_token(token) - if not cloud.is_authenticated: + cloud = await build_authenticated_cloud(db, current_user) + if cloud is None or not cloud.is_authenticated: + if cloud is not None: + await cloud.close() return _filament_id_name_cache or {} try: @@ -961,6 +985,8 @@ async def get_filament_id_map( return result except Exception: return _filament_id_name_cache or {} + finally: + await cloud.close() @router.get("/fields/{preset_type}") diff --git a/backend/app/api/routes/inventory.py b/backend/app/api/routes/inventory.py index 3b7c23424..939730a60 100644 --- a/backend/app/api/routes/inventory.py +++ b/backend/app/api/routes/inventory.py @@ -786,7 +786,7 @@ async def list_assignments( async def assign_spool( data: SpoolAssignmentCreate, db: AsyncSession = Depends(get_db), - _: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_UPDATE), + current_user: User | None = RequirePermissionIfAuthEnabled(Permission.INVENTORY_UPDATE), ): """Assign a spool to an AMS slot and auto-configure via MQTT.""" from backend.app.services.printer_manager import printer_manager @@ -911,34 +911,39 @@ async def assign_spool( # Use base_sf (version suffix stripped) for cloud API + MQTT setting_id = base_sf try: - from backend.app.services.bambu_cloud import get_cloud_service + from backend.app.api.routes.cloud import build_authenticated_cloud - cloud = get_cloud_service() - if cloud.is_authenticated: - detail = await cloud.get_setting_detail(base_sf) - if detail.get("filament_id"): - tray_info_idx = detail["filament_id"] - logger.info( - "Spool assign: resolved filament_id=%r from cloud for setting_id=%r", - tray_info_idx, - sf, - ) - # Use cloud preset name for tray_sub_brands if available - cloud_name = detail.get("name", "") - if cloud_name: - tray_sub_brands = cloud_name.replace(r"@.*$", "").split("@")[0].strip() - elif detail.get("base_id"): - # Derive from base_id (e.g. "GFSL05" → "GFL05") - bid = detail["base_id"].split("_")[0] - if bid.startswith("GFS") and len(bid) >= 5: - tray_info_idx = f"GF{bid[3:]}" - else: - tray_info_idx = bid - logger.info( - "Spool assign: derived filament_id=%r from base_id=%r", - tray_info_idx, - detail["base_id"], - ) + cloud = await build_authenticated_cloud(db, current_user) + if cloud is not None and cloud.is_authenticated: + try: + detail = await cloud.get_setting_detail(base_sf) + if detail.get("filament_id"): + tray_info_idx = detail["filament_id"] + logger.info( + "Spool assign: resolved filament_id=%r from cloud for setting_id=%r", + tray_info_idx, + sf, + ) + # Use cloud preset name for tray_sub_brands if available + cloud_name = detail.get("name", "") + if cloud_name: + tray_sub_brands = cloud_name.replace(r"@.*$", "").split("@")[0].strip() + elif detail.get("base_id"): + # Derive from base_id (e.g. "GFSL05" → "GFL05") + bid = detail["base_id"].split("_")[0] + if bid.startswith("GFS") and len(bid) >= 5: + tray_info_idx = f"GF{bid[3:]}" + else: + tray_info_idx = bid + logger.info( + "Spool assign: derived filament_id=%r from base_id=%r", + tray_info_idx, + detail["base_id"], + ) + finally: + await cloud.close() + elif cloud is not None: + await cloud.close() except Exception as e: logger.warning("Spool assign: cloud lookup failed for %r: %s", sf, e) diff --git a/backend/app/core/database.py b/backend/app/core/database.py index b2fdb47c2..186130e01 100644 --- a/backend/app/core/database.py +++ b/backend/app/core/database.py @@ -1273,6 +1273,7 @@ async def run_migrations(conn): # Migration: Add per-user Bambu Cloud credential columns await _safe_execute(conn, "ALTER TABLE users ADD COLUMN cloud_token VARCHAR(500)") await _safe_execute(conn, "ALTER TABLE users ADD COLUMN cloud_email VARCHAR(255)") + await _safe_execute(conn, "ALTER TABLE users ADD COLUMN cloud_region VARCHAR(10)") # Cleanup: Remove obsolete settings keys that are no longer used obsolete_keys = ["slicer_binary_path"] diff --git a/backend/app/main.py b/backend/app/main.py index bff6536ae..b66d5298a 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -3965,6 +3965,19 @@ async def lifespan(app: FastAPI): # Startup await init_db() + # Register an app-scoped httpx client for Bambu Cloud services so + # per-request BambuCloudService instances reuse the same connection pool + # (important for routes like /cloud/filament-info that chain many + # get_setting_detail calls). The shared client stores no region/token + # state, so the per-request ownership pattern that fixed the region-bleed + # bug is preserved. + import httpx as _httpx + + from backend.app.services.bambu_cloud import set_shared_http_client + + _shared_cloud_http_client = _httpx.AsyncClient(timeout=30.0) + set_shared_http_client(_shared_cloud_http_client) + # Fix queue items stuck with invalid "aborted" status (should be "cancelled"). # This can happen when a print was cancelled mid-print on versions before this fix. try: @@ -4200,6 +4213,10 @@ async def lifespan(app: FastAPI): await mqtt_relay.disconnect(timeout=2) + # Drop the shared Bambu Cloud HTTP client we registered at startup. + set_shared_http_client(None) + await _shared_cloud_http_client.aclose() + # Checkpoint WAL (SQLite only) and close all database connections from backend.app.core.db_dialect import is_sqlite diff --git a/backend/app/models/user.py b/backend/app/models/user.py index 4de0406d9..82d4ef79d 100644 --- a/backend/app/models/user.py +++ b/backend/app/models/user.py @@ -42,6 +42,8 @@ class User(Base): # Per-user Bambu Cloud credentials (when auth is enabled, each user has their own) cloud_token: Mapped[str | None] = mapped_column(String(500), nullable=True, default=None) cloud_email: Mapped[str | None] = mapped_column(String(255), nullable=True, default=None) + # "global" or "china"; NULL treated as "global" for legacy rows. + cloud_region: Mapped[str | None] = mapped_column(String(10), nullable=True, default=None) # Relationship to groups through association table groups: Mapped[list[Group]] = relationship( diff --git a/backend/app/schemas/cloud.py b/backend/app/schemas/cloud.py index f315b4000..e59b3a844 100644 --- a/backend/app/schemas/cloud.py +++ b/backend/app/schemas/cloud.py @@ -1,12 +1,16 @@ +from typing import Literal + from pydantic import BaseModel, Field +Region = Literal["global", "china"] + class CloudLoginRequest(BaseModel): """Request to initiate cloud login.""" email: str = Field(..., description="Bambu Lab account email") password: str = Field(..., description="Account password") - region: str = Field(default="global", description="Region: 'global' or 'china'") + region: Region = Field(default="global", description="Region: 'global' or 'china'") class CloudVerifyRequest(BaseModel): @@ -15,6 +19,7 @@ class CloudVerifyRequest(BaseModel): email: str = Field(..., description="Bambu Lab account email") code: str = Field(..., description="6-digit verification code") tfa_key: str | None = Field(None, description="TFA key for TOTP verification (from login response)") + region: Region = Field(default="global", description="Region: 'global' or 'china'") class CloudLoginResponse(BaseModel): @@ -32,12 +37,14 @@ class CloudAuthStatus(BaseModel): is_authenticated: bool email: str | None = None + region: Region | None = None class CloudTokenRequest(BaseModel): """Request to set access token directly.""" access_token: str = Field(..., description="Bambu Lab access token") + region: Region = Field(default="global", description="Region: 'global' or 'china'") class SlicerSetting(BaseModel): diff --git a/backend/app/services/bambu_cloud.py b/backend/app/services/bambu_cloud.py index 35ce86d28..bde5cc8c5 100644 --- a/backend/app/services/bambu_cloud.py +++ b/backend/app/services/bambu_cloud.py @@ -27,15 +27,41 @@ class BambuCloudAuthError(BambuCloudError): pass +_shared_http_client: httpx.AsyncClient | None = None + + +def set_shared_http_client(client: httpx.AsyncClient | None) -> None: + """Register an app-scoped ``httpx.AsyncClient`` so per-request + ``BambuCloudService`` instances can reuse its connection pool. + + Pass ``None`` during shutdown to unregister. The service only holds a + reference (never closes a client it does not own), so region + token + state still stays per-request — this only shares the transport pool. + """ + global _shared_http_client + _shared_http_client = client + + class BambuCloudService: """Service for interacting with Bambu Lab Cloud API.""" - def __init__(self, region: str = "global"): + def __init__(self, region: str = "global", client: httpx.AsyncClient | None = None): self.base_url = BAMBU_API_BASE if region == "global" else BAMBU_API_BASE_CN self.access_token: str | None = None self.refresh_token: str | None = None self.token_expiry: datetime | None = None - self._client = httpx.AsyncClient(timeout=30.0) + # Prefer an explicitly-injected client (tests), else fall back to the + # app-scoped shared client (production), and finally create our own so + # scripts / tests that skip the lifespan still get a working service. + if client is not None: + self._client = client + self._owns_client = False + elif _shared_http_client is not None: + self._client = _shared_http_client + self._owns_client = False + else: + self._client = httpx.AsyncClient(timeout=30.0) + self._owns_client = True @property def is_authenticated(self) -> bool: @@ -511,17 +537,16 @@ class BambuCloudService: raise BambuCloudError(f"Request failed: {e}") async def close(self): - """Close the HTTP client.""" - await self._client.aclose() + """Close the HTTP client we own. No-op when sharing an app-scoped client.""" + if self._owns_client: + await self._client.aclose() -# Singleton instance -_cloud_service: BambuCloudService | None = None - - -def get_cloud_service() -> BambuCloudService: - """Get the singleton cloud service instance.""" - global _cloud_service - if _cloud_service is None: - _cloud_service = BambuCloudService() - return _cloud_service +# Previously this module exposed a process-wide ``_cloud_service`` singleton +# via ``get_cloud_service()`` / ``reset_cloud_service()``. That pattern leaked +# region and token state across users (a China-region login would pin the +# singleton to api.bambulab.cn until the next explicit reset), so the singleton +# has been removed. Callers should construct a per-request +# ``BambuCloudService(region=...)`` from the stored region and ``await +# cloud.close()`` it when done. See ``routes.cloud.build_authenticated_cloud`` +# for the standard pattern. diff --git a/backend/app/services/github_backup.py b/backend/app/services/github_backup.py index 82e172508..d71f94185 100644 --- a/backend/app/services/github_backup.py +++ b/backend/app/services/github_backup.py @@ -22,7 +22,6 @@ from backend.app.models.printer import Printer from backend.app.models.settings import Settings from backend.app.models.spool import Spool from backend.app.models.spool_usage_history import SpoolUsageHistory -from backend.app.services.bambu_cloud import get_cloud_service from backend.app.services.printer_manager import printer_manager logger = logging.getLogger(__name__) @@ -415,16 +414,15 @@ class GitHubBackupService: async def _collect_cloud_profiles(self, db: AsyncSession, files: dict): """Collect Bambu Cloud profiles if authenticated.""" - # Check if cloud is authenticated - cloud = get_cloud_service() + # Backup runs without a user context, so fall back to the auth-disabled + # Settings storage. ``build_authenticated_cloud`` honours the stored + # region so China-region tokens are validated against api.bambulab.cn. + from backend.app.api.routes.cloud import build_authenticated_cloud - # Try to restore token from DB - result = await db.execute(select(Settings).where(Settings.key == "bambu_cloud_token")) - setting = result.scalar_one_or_none() - if setting and setting.value: - cloud.set_token(setting.value) - - if not cloud.is_authenticated: + cloud = await build_authenticated_cloud(db, user=None) + if cloud is None or not cloud.is_authenticated: + if cloud is not None: + await cloud.close() logger.info("Cloud not authenticated, skipping cloud profiles") return @@ -472,6 +470,8 @@ class GitHubBackupService: except Exception as e: logger.warning("Failed to collect cloud profiles: %s", e) + finally: + await cloud.close() async def _collect_settings(self, db: AsyncSession, files: dict): """Collect app settings.""" diff --git a/backend/tests/integration/test_cloud_auth.py b/backend/tests/integration/test_cloud_auth.py index 6a47c94d7..96fe4cb17 100644 --- a/backend/tests/integration/test_cloud_auth.py +++ b/backend/tests/integration/test_cloud_auth.py @@ -6,7 +6,7 @@ Regression tests for: - Cloud endpoints use CLOUD_AUTH permission (not SETTINGS_READ) """ -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest from httpx import AsyncClient @@ -267,19 +267,21 @@ class TestCloudTokenStorage: """get_stored_token with user=None and no global token returns (None, None).""" from backend.app.api.routes.cloud import get_stored_token - token, email = await get_stored_token(db_session, user=None) + token, email, region = await get_stored_token(db_session, user=None) assert token is None assert email is None + assert region == "global" # default for missing rows @pytest.mark.asyncio async def test_store_and_get_global_token(self, db_session): """store_token with user=None stores in global Settings table.""" from backend.app.api.routes.cloud import get_stored_token, store_token - await store_token(db_session, "test-token-123", "test@example.com", user=None) - token, email = await get_stored_token(db_session, user=None) + await store_token(db_session, "test-token-123", "test@example.com", "global", user=None) + token, email, region = await get_stored_token(db_session, user=None) assert token == "test-token-123" assert email == "test@example.com" + assert region == "global" @pytest.mark.asyncio async def test_store_and_get_per_user_token(self, db_session): @@ -293,7 +295,7 @@ class TestCloudTokenStorage: await db_session.commit() await db_session.refresh(user) - await store_token(db_session, "user-token-abc", "user@example.com", user=user) + await store_token(db_session, "user-token-abc", "user@example.com", "global", user=user) # Re-fetch user to verify persistence from sqlalchemy import select @@ -302,6 +304,7 @@ class TestCloudTokenStorage: refreshed = result.scalar_one() assert refreshed.cloud_token == "user-token-abc" assert refreshed.cloud_email == "user@example.com" + assert refreshed.cloud_region == "global" @pytest.mark.asyncio async def test_per_user_token_does_not_affect_global(self, db_session): @@ -316,17 +319,17 @@ class TestCloudTokenStorage: await db_session.refresh(user) # Store per-user token - await store_token(db_session, "per-user-token", "per-user@test.com", user=user) + await store_token(db_session, "per-user-token", "per-user@test.com", "global", user=user) # Global should still be empty - global_token, global_email = await get_stored_token(db_session, user=None) + global_token, global_email, _ = await get_stored_token(db_session, user=None) assert global_token is None assert global_email is None @pytest.mark.asyncio async def test_clear_per_user_token(self, db_session): """clear_token with user clears only that user's credentials.""" - from backend.app.api.routes.cloud import clear_token, get_stored_token, store_token + from backend.app.api.routes.cloud import clear_token, store_token from backend.app.core.auth import get_password_hash from backend.app.models.user import User @@ -335,7 +338,7 @@ class TestCloudTokenStorage: await db_session.commit() await db_session.refresh(user) - await store_token(db_session, "to-clear", "clear@test.com", user=user) + await store_token(db_session, "to-clear", "clear@test.com", "china", user=user) await clear_token(db_session, user=user) from sqlalchemy import select @@ -344,22 +347,24 @@ class TestCloudTokenStorage: refreshed = result.scalar_one() assert refreshed.cloud_token is None assert refreshed.cloud_email is None + assert refreshed.cloud_region is None @pytest.mark.asyncio async def test_clear_global_token(self, db_session): """clear_token with user=None clears from global Settings.""" from backend.app.api.routes.cloud import clear_token, get_stored_token, store_token - await store_token(db_session, "global-token", "global@test.com", user=None) + await store_token(db_session, "global-token", "global@test.com", "global", user=None) await clear_token(db_session, user=None) - token, email = await get_stored_token(db_session, user=None) + token, email, region = await get_stored_token(db_session, user=None) assert token is None assert email is None + assert region == "global" # normalised default @pytest.mark.asyncio async def test_two_users_independent_tokens(self, db_session): - """Two users should have completely independent cloud tokens.""" + """Two users should have completely independent cloud tokens and regions.""" from backend.app.api.routes.cloud import get_stored_token, store_token from backend.app.core.auth import get_password_hash from backend.app.models.user import User @@ -371,8 +376,10 @@ class TestCloudTokenStorage: await db_session.refresh(user_a) await db_session.refresh(user_b) - await store_token(db_session, "token-a", "a@test.com", user=user_a) - await store_token(db_session, "token-b", "b@test.com", user=user_b) + # Different regions on purpose — a China user and a Global user must not + # bleed their region into each other's lookups. + await store_token(db_session, "token-a", "a@test.com", "china", user=user_a) + await store_token(db_session, "token-b", "b@test.com", "global", user=user_b) # Verify each user reads their own token (re-fetch from DB) from sqlalchemy import select @@ -382,10 +389,209 @@ class TestCloudTokenStorage: fresh_a = result_a.scalar_one() fresh_b = result_b.scalar_one() - token_a, email_a = await get_stored_token(db_session, user=fresh_a) - token_b, email_b = await get_stored_token(db_session, user=fresh_b) + token_a, email_a, region_a = await get_stored_token(db_session, user=fresh_a) + token_b, email_b, region_b = await get_stored_token(db_session, user=fresh_b) assert token_a == "token-a" assert email_a == "a@test.com" + assert region_a == "china" assert token_b == "token-b" assert email_b == "b@test.com" + assert region_b == "global" + + +class TestCloudRegionPersistence: + """Region must survive a DB round-trip so restarts don't silently flip users to api.bambulab.com.""" + + @pytest.mark.asyncio + async def test_region_survives_roundtrip_per_user(self, db_session): + """Stored China region is returned on subsequent get_stored_token calls.""" + from backend.app.api.routes.cloud import get_stored_token, store_token + from backend.app.core.auth import get_password_hash + from backend.app.models.user import User + + user = User(username="region-user", password_hash=get_password_hash("pass"), role="user") + db_session.add(user) + await db_session.commit() + await db_session.refresh(user) + + await store_token(db_session, "cn-token", "token-auth", "china", user=user) + + # Simulate "next request": re-fetch the user fresh from the DB. + from sqlalchemy import select + + result = await db_session.execute(select(User).where(User.id == user.id)) + refreshed = result.scalar_one() + + _token, _email, region = await get_stored_token(db_session, user=refreshed) + assert region == "china" + + @pytest.mark.asyncio + async def test_region_survives_roundtrip_global_fallback(self, db_session): + """Stored China region in auth-disabled Settings fallback survives too.""" + from backend.app.api.routes.cloud import get_stored_token, store_token + + await store_token(db_session, "cn-token", "token-auth", "china", user=None) + _token, _email, region = await get_stored_token(db_session, user=None) + assert region == "china" + + @pytest.mark.asyncio + async def test_invalid_region_is_normalised_to_global(self, db_session): + """Unknown region values fall back to 'global' rather than mis-route.""" + from backend.app.api.routes.cloud import get_stored_token, store_token + + await store_token(db_session, "t", "x@test.com", "mars", user=None) + _token, _email, region = await get_stored_token(db_session, user=None) + assert region == "global" + + @pytest.mark.asyncio + async def test_build_authenticated_cloud_uses_stored_region(self, db_session): + """build_authenticated_cloud wires the stored region into the per-request service.""" + from backend.app.api.routes.cloud import build_authenticated_cloud, store_token + from backend.app.core.auth import get_password_hash + from backend.app.models.user import User + + user = User(username="cn-build", password_hash=get_password_hash("pass"), role="user") + db_session.add(user) + await db_session.commit() + await db_session.refresh(user) + + await store_token(db_session, "cn-token", "token-auth", "china", user=user) + + from sqlalchemy import select + + result = await db_session.execute(select(User).where(User.id == user.id)) + refreshed = result.scalar_one() + + cloud = await build_authenticated_cloud(db_session, refreshed) + assert cloud is not None + try: + assert cloud.base_url == "https://api.bambulab.cn" + assert cloud.access_token == "cn-token" + finally: + await cloud.close() + + +class TestCloudRouteRegionPlumbing: + """Route-level proof that region=china on the wire actually steers outbound + HTTP calls to api.bambulab.cn / bambulab.cn. This is the core bug the PR + fixes — unit tests prove the service does the right thing given the region, + storage tests prove the region persists, but only these tests prove the + route handlers plumb the region through end-to-end. + + Auth is disabled (Settings-fallback path) to keep the fixture footprint + minimal; the region plumbing code path is identical for the per-user path. + """ + + @staticmethod + def _make_response(json_body: dict, status: int = 200): + """Build a MagicMock httpx.Response stand-in for patched posts/gets.""" + response = MagicMock() + response.status_code = status + response.text = "{}" + response.json.return_value = json_body + response.cookies = {} + return response + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_set_token_route_with_china_region_hits_cn_endpoint(self, async_client: AsyncClient): + """POST /cloud/token with region=china routes get_user_profile to api.bambulab.cn.""" + import httpx + + with ( + patch("backend.app.core.auth.is_auth_enabled", return_value=False), + patch.object(httpx.AsyncClient, "get", new_callable=AsyncMock) as mock_get, + ): + mock_get.return_value = self._make_response({"uid": "123", "email": "x"}) + + response = await async_client.post( + "/api/v1/cloud/token", + json={"access_token": "cn-token", "region": "china"}, + ) + + assert response.status_code == 200 + # The profile check call must have hit api.bambulab.cn, never .com + called_urls = [str(call.args[0]) for call in mock_get.call_args_list if call.args] + assert any("api.bambulab.cn" in url for url in called_urls), called_urls + assert not any("api.bambulab.com" in url for url in called_urls), called_urls + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_login_route_with_china_region_hits_cn_endpoint(self, async_client: AsyncClient): + """POST /cloud/login with region=china routes login_request to api.bambulab.cn.""" + import httpx + + with ( + patch("backend.app.core.auth.is_auth_enabled", return_value=False), + patch.object(httpx.AsyncClient, "post", new_callable=AsyncMock) as mock_post, + ): + mock_post.return_value = self._make_response({"loginType": "verifyCode"}) + + response = await async_client.post( + "/api/v1/cloud/login", + json={"email": "user@example.com", "password": "x", "region": "china"}, + ) + + assert response.status_code == 200 + called_urls = [str(call.args[0]) for call in mock_post.call_args_list if call.args] + assert any("api.bambulab.cn" in url for url in called_urls), called_urls + assert not any("api.bambulab.com" in url for url in called_urls), called_urls + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_verify_route_with_china_region_hits_cn_tfa_endpoint(self, async_client: AsyncClient): + """POST /cloud/verify with region=china + tfa_key routes TOTP to bambulab.cn.""" + import httpx + + with ( + patch("backend.app.core.auth.is_auth_enabled", return_value=False), + patch.object(httpx.AsyncClient, "post", new_callable=AsyncMock) as mock_post, + ): + mock_post.return_value = self._make_response({"token": "t"}) + + response = await async_client.post( + "/api/v1/cloud/verify", + json={ + "email": "user@example.com", + "code": "123456", + "tfa_key": "tfa-xyz", + "region": "china", + }, + ) + + assert response.status_code == 200 + called_urls = [str(call.args[0]) for call in mock_post.call_args_list if call.args] + # TOTP endpoint lives on bambulab.cn (without the api. prefix), + # NOT bambulab.com — that's exactly the bug we just fixed. + assert any("bambulab.cn/api/sign-in/tfa" in url for url in called_urls), called_urls + assert not any("bambulab.com" in url for url in called_urls), called_urls + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_cloud_status_exposes_stored_region(self, async_client: AsyncClient): + """GET /cloud/status returns the stored region so the UI can render + 'Connected (China)' after a reload.""" + from backend.app.api.routes.cloud import store_token + from backend.app.core.database import async_session + + with patch("backend.app.core.auth.is_auth_enabled", return_value=False): + async with async_session() as db: + await store_token(db, "cn-token", "token-auth", "china", user=None) + + response = await async_client.get("/api/v1/cloud/status") + assert response.status_code == 200 + data = response.json() + assert data["is_authenticated"] is True + assert data["region"] == "china" + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_cloud_status_region_is_null_when_unauthenticated(self, async_client: AsyncClient): + """No stored token ⇒ no region in the status payload.""" + with patch("backend.app.core.auth.is_auth_enabled", return_value=False): + response = await async_client.get("/api/v1/cloud/status") + assert response.status_code == 200 + data = response.json() + assert data["is_authenticated"] is False + assert data["region"] is None diff --git a/backend/tests/unit/services/test_bambu_cloud.py b/backend/tests/unit/services/test_bambu_cloud.py index 59b9371d4..15d5bae37 100644 --- a/backend/tests/unit/services/test_bambu_cloud.py +++ b/backend/tests/unit/services/test_bambu_cloud.py @@ -236,3 +236,58 @@ class TestBambuCloudTOTPVerification: headers = call_args[1]["headers"] assert "User-Agent" in headers assert "Mozilla" in headers["User-Agent"] + + +class TestBambuCloudRegion: + """Region routing — China-region instances must hit api.bambulab.cn.""" + + def test_global_region_uses_com_base(self): + """Default / 'global' region should use api.bambulab.com.""" + cloud = BambuCloudService() # default region + assert cloud.base_url == "https://api.bambulab.com" + + cloud_explicit = BambuCloudService(region="global") + assert cloud_explicit.base_url == "https://api.bambulab.com" + + def test_china_region_uses_cn_base(self): + """'china' region should use api.bambulab.cn.""" + cloud = BambuCloudService(region="china") + assert cloud.base_url == "https://api.bambulab.cn" + + @pytest.mark.asyncio + async def test_china_region_login_hits_cn_endpoint(self): + """A login_request from a China-region instance must POST to api.bambulab.cn.""" + cloud = BambuCloudService(region="china") + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"loginType": "verifyCode"} + + with patch.object(cloud._client, "post", new_callable=AsyncMock) as mock_post: + mock_post.return_value = mock_response + + await cloud.login_request("test@example.com", "password") + + url = mock_post.call_args[0][0] + assert "api.bambulab.cn" in url + assert "api.bambulab.com" not in url + + @pytest.mark.asyncio + async def test_china_region_totp_hits_cn_tfa_endpoint(self): + """TOTP verification from a China-region instance uses the CN TFA endpoint.""" + cloud = BambuCloudService(region="china") + + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.text = '{"token": "t"}' + mock_response.json.return_value = {"token": "t"} + mock_response.cookies = {} + + with patch.object(cloud._client, "post", new_callable=AsyncMock) as mock_post: + mock_post.return_value = mock_response + + await cloud.verify_totp("tfa-key", "123456") + + url = mock_post.call_args[0][0] + assert "bambulab.cn/api/sign-in/tfa" in url + assert "bambulab.com" not in url diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index ab2ccb383..ade787ef3 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -957,6 +957,7 @@ export interface MQTTStatus { export interface CloudAuthStatus { is_authenticated: boolean; email: string | null; + region?: 'global' | 'china' | null; } export interface CloudLoginResponse { @@ -3642,15 +3643,15 @@ export const api = { method: 'POST', body: JSON.stringify({ email, password, region }), }), - cloudVerify: (email: string, code: string, tfaKey?: string) => + cloudVerify: (email: string, code: string, tfaKey?: string, region: string = 'global') => request('/cloud/verify', { method: 'POST', - body: JSON.stringify({ email, code, tfa_key: tfaKey }), + body: JSON.stringify({ email, code, tfa_key: tfaKey, region }), }), - cloudSetToken: (access_token: string) => + cloudSetToken: (access_token: string, region: string = 'global') => request('/cloud/token', { method: 'POST', - body: JSON.stringify({ access_token }), + body: JSON.stringify({ access_token, region }), }), cloudLogout: () => request<{ success: boolean }>('/cloud/logout', { method: 'POST' }), diff --git a/frontend/src/pages/ProfilesPage.tsx b/frontend/src/pages/ProfilesPage.tsx index edfefb181..cb5d74229 100644 --- a/frontend/src/pages/ProfilesPage.tsx +++ b/frontend/src/pages/ProfilesPage.tsx @@ -128,7 +128,7 @@ function LoginForm({ onSuccess, t }: { onSuccess: () => void; t: TFunction }) { }); const verifyMutation = useMutation({ - mutationFn: () => api.cloudVerify(email, code, tfaKey || undefined), + mutationFn: () => api.cloudVerify(email, code, tfaKey || undefined, region), onSuccess: (result) => { if (result.success) { showToast(t('profiles.login.toast.loggedIn')); @@ -141,7 +141,7 @@ function LoginForm({ onSuccess, t }: { onSuccess: () => void; t: TFunction }) { }); const tokenMutation = useMutation({ - mutationFn: () => api.cloudSetToken(token), + mutationFn: () => api.cloudSetToken(token, region), onSuccess: () => { showToast(t('profiles.login.toast.tokenSet')); onSuccess(); @@ -231,18 +231,31 @@ function LoginForm({ onSuccess, t }: { onSuccess: () => void; t: TFunction }) { )} {step === 'token' && ( -
- -

{t('profiles.login.accessTokenHint')}

-