diff --git a/.gitignore b/.gitignore index bd8ef4707..fd36b15bc 100644 --- a/.gitignore +++ b/.gitignore @@ -98,3 +98,8 @@ security/ test_pipeline_archive_source.3mf test_pipeline_run_1.3mf + +# Python coverage artifacts +.coverage +.coverage.* +htmlcov/ diff --git a/backend/app/api/routes/auth.py b/backend/app/api/routes/auth.py index aac5c57d9..4ffbf59a3 100644 --- a/backend/app/api/routes/auth.py +++ b/backend/app/api/routes/auth.py @@ -348,10 +348,8 @@ async def setup_auth(request: SetupRequest, db: AsyncSession = Depends(get_db)): # (#2530). Only migrate when there is exactly one obvious owner: # handing another admin's session a Bambu credential is not a # guess worth making. - from backend.app.api.routes.cloud import ( - get_stored_token, - migrate_global_cloud_token_to_user, - ) + from backend.app.api.routes.cloud import migrate_global_cloud_token_to_user + from backend.app.services.bambu_cloud_credentials import get_stored_token if admin_created: cloud_owner = admin_user diff --git a/backend/app/api/routes/cloud.py b/backend/app/api/routes/cloud.py index 2dcad6c4f..5f97985ef 100644 --- a/backend/app/api/routes/cloud.py +++ b/backend/app/api/routes/cloud.py @@ -7,7 +7,6 @@ Handles authentication and profile management with Bambu Cloud. import asyncio import json import logging -from datetime import datetime, timezone from pathlib import Path from typing import Literal @@ -23,7 +22,7 @@ from backend.app.core.auth import ( require_permission_if_auth_enabled, security, ) -from backend.app.core.database import async_session, get_db +from backend.app.core.database import get_db from backend.app.core.permissions import Permission from backend.app.models.api_key import APIKey from backend.app.models.settings import Settings @@ -50,6 +49,22 @@ from backend.app.services.bambu_cloud import ( BambuCloudService, invalidate_validation_cache, ) + +# Credential read/write lives in the services layer so feature packages can +# consume it without importing the route layer. Imported here for this +# module's own use; consumers should import from bambu_cloud_credentials +# directly rather than through this route module. +from backend.app.services.bambu_cloud_credentials import ( + CLOUD_EMAIL_KEY, + CLOUD_REGION_KEY, + CLOUD_TOKEN_INVALID_KEY, + CLOUD_TOKEN_KEY, + _clear_cloud_token_invalid, + _normalise_region, + get_stored_token, + is_cloud_token_invalid, + mark_cloud_token_invalid, +) from backend.app.utils.filament_ids import filament_id_to_setting_id logger = logging.getLogger(__name__) @@ -166,100 +181,6 @@ async def resolve_api_key_cloud_owner( router = APIRouter(prefix="/cloud", tags=["cloud"], dependencies=[Depends(_cloud_api_key_gate)]) -# 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" -# Global (auth-disabled) counterpart of ``User.cloud_token_invalid_at``. Stores -# an ISO timestamp; absent/empty means "not known to be dead". -CLOUD_TOKEN_INVALID_KEY = "bambu_cloud_token_invalid_at" - - -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 is_cloud_token_invalid(db: AsyncSession, user: User | None = None) -> bool: - """Whether the stored Bambu token is known to have been rejected. - - Set by :func:`mark_cloud_token_invalid` the first time Bambu answers 401, - cleared on a fresh login/logout. This is the only durable record we have: - Bambu's access token is opaque (no readable expiry) and Bambuddy does not - persist the refresh token, so without this flag a dead credential looks - exactly like a live one. - """ - if user is not None: - return user.cloud_token_invalid_at is not None - result = await db.execute(select(Settings).where(Settings.key == CLOUD_TOKEN_INVALID_KEY)) - row = result.scalar_one_or_none() - return bool(row and row.value) - - -async def mark_cloud_token_invalid(user_id: int | None) -> None: - """Record that Bambu rejected the stored token. - - Opens its own session on purpose. This runs from - ``BambuCloudService._on_auth_failure``, i.e. in the middle of a route that - is about to fail — writing through that route's session would tie the flag - to a transaction the route may still roll back, and the fact that the - credential is dead is true regardless of how the request ends. - - Best-effort: a bookkeeping failure must never replace the 401 the caller - actually needs to see. - """ - now = datetime.now(timezone.utc) - try: - async with async_session() as db: - if user_id is not None: - await db.execute(update(User).where(User.id == user_id).values(cloud_token_invalid_at=now)) - else: - result = await db.execute(select(Settings).where(Settings.key == CLOUD_TOKEN_INVALID_KEY)) - row = result.scalar_one_or_none() - if row: - row.value = now.isoformat() - else: - db.add(Settings(key=CLOUD_TOKEN_INVALID_KEY, value=now.isoformat())) - await db.commit() - logger.warning("Bambu Cloud rejected the stored token (user_id=%s) — marking the sign-in as expired", user_id) - except Exception: - logger.exception("Could not record the Bambu Cloud token as invalid") - - -async def _clear_cloud_token_invalid(db: AsyncSession, user: User | None) -> None: - """Clear the rejected-token flag — called on every fresh login and logout.""" - if user is not None: - await db.execute(update(User).where(User.id == user.id).values(cloud_token_invalid_at=None)) - return - result = await db.execute(select(Settings).where(Settings.key == CLOUD_TOKEN_INVALID_KEY)) - row = result.scalar_one_or_none() - if row: - await db.delete(row) - - -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, _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, 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), - _normalise_region(settings.get(CLOUD_REGION_KEY)), - ) - - async def store_token(db: AsyncSession, token: str, email: str, region: str, user: User | None = None) -> None: """Store cloud token, email, and region. diff --git a/backend/app/api/routes/makerworld.py b/backend/app/api/routes/makerworld.py index 5124e5fbc..2a5400aca 100644 --- a/backend/app/api/routes/makerworld.py +++ b/backend/app/api/routes/makerworld.py @@ -1,13 +1,20 @@ """MakerWorld integration routes. -User pastes a MakerWorld URL → Bambuddy resolves it → shows plate list → -one-click import/print. The URL-paste flow covers the actual discovery -pattern (Reddit/YouTube/shared links) without needing to replicate -MakerWorld's whole search UI. +User pastes a model URL (MakerWorld or other supported host) → Bambuddy resolves +it → shows plate list → one-click import/print. The URL-paste flow covers the +actual discovery pattern (Reddit/YouTube/shared links) without needing to +replicate the host's whole search UI. Search/browse endpoints are intentionally NOT exposed: the public-facing ``design/search`` endpoint returns empty results from server-originated requests (see memory/makerworld-integration.md for the investigation). + +These are still the *MakerWorld* routes: they consult the shared seams where +one exists — URL routing via :class:`ModelProviderRegistry`, permissions and +folder naming from the provider descriptor, already-imported matching via +:meth:`ModelProvider.source_url_filter` — but request/response shapes remain +MakerWorld-specific. The fully shared import API that makes new hosts work +with zero route changes arrives with #2793. """ from __future__ import annotations @@ -16,19 +23,20 @@ import logging import os from urllib.parse import unquote -from fastapi import APIRouter, Depends, HTTPException, Query +from fastapi import APIRouter, Depends, Header, HTTPException, Query from fastapi.responses import Response +from fastapi.security import HTTPAuthorizationCredentials from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from backend.app.api.routes.cloud import ( - get_stored_token, - is_cloud_token_invalid, - mark_cloud_token_invalid, - resolve_api_key_cloud_owner, -) +from backend.app.api.routes.cloud import resolve_api_key_cloud_owner from backend.app.api.routes.library import save_3mf_bytes_to_library -from backend.app.core.auth import RequirePermissionIfAuthEnabled +from backend.app.core.auth import ( + RequirePermissionIfAuthEnabled, + require_auth_if_enabled, + require_permission_if_auth_enabled, + security, +) from backend.app.core.database import get_db from backend.app.core.permissions import Permission from backend.app.models.library import LibraryFile, LibraryFolder @@ -41,74 +49,119 @@ from backend.app.schemas.makerworld import ( MakerWorldResolveRequest, MakerWorldStatus, ) -from backend.app.services.makerworld import ( - MakerWorldAuthError, - MakerWorldError, - MakerWorldForbiddenError, - MakerWorldNotFoundError, - MakerWorldService, - MakerWorldUnavailableError, - MakerWorldUrlError, +from backend.app.services.model_providers import makerworld_provider, registry +from backend.app.services.model_providers.base import ( + ModelProvider, + ProviderAuthError, + ProviderError, + ProviderForbiddenError, + ProviderNotFoundError, + ProviderResourceRef, + ProviderService, + ProviderUnavailableError, + ProviderUrlError, ) +from backend.app.services.model_providers.makerworld.service import MakerWorldService logger = logging.getLogger(__name__) router = APIRouter(prefix="/makerworld", tags=["makerworld"]) -_SOURCE_TYPE = "makerworld" +def _provider_for_url(url: str) -> ModelProvider: + """Return the registered model provider that claims *url*. -async def _build_service(db: AsyncSession, user: User | None) -> MakerWorldService: - """Construct a per-request MakerWorldService seeded with the caller's - stored Bambu Cloud bearer token when available. - - Mirrors ``cloud.build_authenticated_cloud`` — the token is entirely - optional; anonymous calls (metadata, URL resolution) still work — and, - like it, records a rejected token so the whole app agrees the sign-in is - dead rather than each feature failing on its own. + A pasted link for an unsupported host is a clean 400 — the registry is + the routing seam, and "nobody supports this URL" is a client-input + problem, not a server error. """ - token, _email, _region = await get_stored_token(db, user) - user_id = user.id if user is not None else None - return MakerWorldService( - auth_token=token, - on_auth_failure=lambda: mark_cloud_token_invalid(user_id), - ) + provider = registry.find_for_url(url) + if provider is None: + msg = f"No registered model provider supports {url!r}" + raise HTTPException(status_code=400, detail=msg) + return provider -def _canonical_url(model_id: int, profile_id: int | None = None) -> str: - """Build a stable source_url we use for dedupe. +def _provider_for_source(source_type: str) -> ModelProvider: + """Return the registered model provider with this ``source_type``. - Dedupe is keyed per *plate* (profile) rather than per model, since the - ``/iot-service/.../profile/{profileId}`` download returns a specific - plate — not the full multi-plate zip — so two different plates of the - same design should become two separate library entries. Canonical - shape uses the locale-free path with the ``#profileId-`` fragment so - all URL variants of the same plate still collapse (e.g. ``/en/models/ - 123-slug?from=search#profileId-456`` and ``/de/models/123#profileId- - 456`` both map to ``https://makerworld.com/models/123#profileId- - 456``). Plate-less imports (legacy or whole-design) keep the old - model-only shape for backwards compatibility with existing rows. + Import identifies a resource by numeric id, not by URL, so there is + nothing to route on except the source type the caller names. The detail + is built here rather than via ``str(KeyError)`` — KeyError's ``__str__`` + is the *repr* of its argument and would ship the quotes to the client. """ - if profile_id: - return f"https://makerworld.com/models/{model_id}#profileId-{profile_id}" - return f"https://makerworld.com/models/{model_id}" + try: + return registry.get(source_type) + except KeyError as exc: + msg = f"No model provider registered for source_type {source_type!r}" + raise HTTPException(status_code=400, detail=msg) from exc -def _map_service_error(exc: MakerWorldError) -> HTTPException: - """Translate service exceptions into HTTP responses.""" - if isinstance(exc, MakerWorldUrlError): +async def _authorize_for_provider( + provider: ModelProvider, + permission: Permission | None, + credentials: HTTPAuthorizationCredentials | None, + x_api_key: str | None, +) -> User | None: + """Apply *provider*'s own permission to a request that named it. + + This cannot live in the route signature. FastAPI resolves dependencies + before the body exists, so a dependency can only ever bake in one + provider's permission — MakerWorld's — while the provider actually being + used comes from the request (``source_type`` on import, the pasted URL on + resolve). Importing from a second provider would then be gated on + ``makerworld:import``, which is nobody's intent. + + The check runs through the same ``require_permission_if_auth_enabled`` + the decorator would have built, so JWT users, API keys (scope gate plus + the owner-outranks-key rule) and auth-disabled installs behave exactly as + before. The routes keep a permission-free ``require_auth_if_enabled`` + dependency so an anonymous caller is still refused before the body is + read. + + A provider that declares no permission is refused rather than waved + through: the descriptor's permission fields are optional, and "unset" + must not read as "unrestricted". + """ + if permission is None: + raise HTTPException( + status_code=500, + detail=f"Model provider {provider.source_type!r} declares no permission for this operation", + ) + checker = require_permission_if_auth_enabled(permission) + return await checker(credentials=credentials, x_api_key=x_api_key) + + +async def _build_service( + db: AsyncSession, + provider: ModelProvider, + current_user: User | None, + api_key_cloud_owner: User | None = None, +) -> ProviderService: + """Construct a per-request service via *provider*. + + Identity resolution (JWT user vs API-key owner vs anonymous) and + credential seeding live inside ``provider.build_service`` — the single + place every provider resolves them, so the routes never re-implement it. + """ + return await provider.build_service(db=db, user=current_user, api_key_owner=api_key_cloud_owner) + + +def _map_service_error(exc: ProviderError) -> HTTPException: + """Translate provider service exceptions into HTTP responses.""" + if isinstance(exc, ProviderUrlError): return HTTPException(status_code=400, detail=str(exc)) - if isinstance(exc, MakerWorldAuthError): + if isinstance(exc, ProviderAuthError): return HTTPException(status_code=401, detail=str(exc)) - if isinstance(exc, MakerWorldForbiddenError): - # 403 forwards MakerWorld's own refusal message (content-gated, + if isinstance(exc, ProviderForbiddenError): + # 403 forwards the provider's own refusal message (content-gated, # region-locked, requires points, etc.) — UI surfaces it verbatim. return HTTPException(status_code=403, detail=str(exc)) - if isinstance(exc, MakerWorldNotFoundError): + if isinstance(exc, ProviderNotFoundError): return HTTPException(status_code=404, detail=str(exc)) - if isinstance(exc, MakerWorldUnavailableError): + if isinstance(exc, ProviderUnavailableError): return HTTPException(status_code=502, detail=str(exc)) - return HTTPException(status_code=500, detail=f"MakerWorld error: {exc}") + return HTTPException(status_code=500, detail=f"Model provider error: {exc}") @router.get("/thumbnail") @@ -133,10 +186,10 @@ async def proxy_thumbnail( URLs are content-addressable (filename contains a hash), so the aggressive ``immutable`` cache-control is safe. """ - service = MakerWorldService() + service = MakerWorldService(thumbnail_hosts=makerworld_provider.thumbnail_hosts()) try: payload, content_type = await service.fetch_thumbnail(url) - except MakerWorldError as exc: + except ProviderError as exc: raise _map_service_error(exc) from exc finally: await service.close() @@ -153,7 +206,7 @@ async def proxy_thumbnail( @router.get("/status", response_model=MakerWorldStatus) async def get_status( db: AsyncSession = Depends(get_db), - current_user: User | None = RequirePermissionIfAuthEnabled(Permission.MAKERWORLD_VIEW), + current_user: User | None = RequirePermissionIfAuthEnabled(makerworld_provider.view_permission), api_key_cloud_owner: User | None = Depends(resolve_api_key_cloud_owner), ): """Report whether the caller can import 3MFs (needs a Bambu Cloud token). @@ -164,26 +217,37 @@ async def get_status( stored token rather than always reporting ``False`` (#1777, same shape as the cloud-presets fix in #1182). """ - cloud_token_user = current_user or api_key_cloud_owner - token, _email, _region = await get_stored_token(db, cloud_token_user) - has_token = bool(token) - # A token Bambu has already rejected downloads nothing. ``can_download`` - # used to be a bare alias for ``has_cloud_token``, so the import button - # stayed enabled against a dead credential and the user found out via a - # 401 toast (#2562 follow-up). - expired = has_token and await is_cloud_token_invalid(db, cloud_token_user) + service = await _build_service(db, makerworld_provider, current_user, api_key_cloud_owner) + try: + status = await service.get_status(db) + finally: + await service.close() return MakerWorldStatus( - has_cloud_token=has_token, - can_download=has_token and not expired, - sign_in_expired=expired, + has_cloud_token=status.authenticated, + can_download=status.can_download, + # ``credential_rejected`` is the machine-readable "your sign-in + # expired" state the provider set exactly when a stored token exists + # *and* was rejected — no token means there is no sign-in to have + # expired. It is read instead of ``auth_error is not None`` because + # the latter is a human-readable reason that providers may also set + # for non-credential failures (network, rate limit). + sign_in_expired=status.credential_rejected, ) -@router.post("/resolve", response_model=MakerWorldResolvedModel) +@router.post( + "/resolve", + response_model=MakerWorldResolvedModel, + # Authentication only — the permission belongs to whichever provider the + # pasted URL routes to, which is not known until the body is parsed (see + # ``_authorize_for_provider``). + dependencies=[Depends(require_auth_if_enabled)], +) async def resolve_url( body: MakerWorldResolveRequest, db: AsyncSession = Depends(get_db), - current_user: User | None = RequirePermissionIfAuthEnabled(Permission.MAKERWORLD_VIEW), + credentials: HTTPAuthorizationCredentials | None = Depends(security), + x_api_key: str | None = Header(default=None, alias="X-API-Key"), api_key_cloud_owner: User | None = Depends(resolve_api_key_cloud_owner), ): """Resolve a MakerWorld URL to full model metadata + plate list. @@ -192,68 +256,34 @@ async def resolve_url( exist for the same model URL, so the UI can show an "Already imported" badge and skip a redundant download. """ + # Strategy pattern: select provider based on URL instead of hardcoding. + # Routing runs before the permission check because the permission *is* the + # provider's; all an unpermitted caller learns from the ordering is which + # hosts Bambuddy supports, which the UI states anyway. + provider = _provider_for_url(body.url) + current_user = await _authorize_for_provider(provider, provider.view_permission, credentials, x_api_key) try: - model_id, profile_id = MakerWorldService.parse_url(body.url) - except MakerWorldError as exc: + ref = provider.parse_url(body.url) + except ProviderError as exc: raise _map_service_error(exc) from exc + model_id = int(ref.external_id) + profile_id = int(ref.sub_id) if ref.sub_id else None - # API-keyed callers carry identity on the key, not in current_user — see - # the /status handler comment and #1777 / #1182. - cloud_token_user = current_user or api_key_cloud_owner - service = await _build_service(db, cloud_token_user) + service = await _build_service(db, provider, current_user, api_key_cloud_owner) try: - design = await service.get_design(model_id) - instances_envelope = await service.get_design_instances(model_id) - except MakerWorldError as exc: + resolved = await service.resolve(ref) + except ProviderError as exc: raise _map_service_error(exc) from exc finally: await service.close() - # MakerWorld's instances payload is ``{"total": N, "hits": [...]}``; callers - # only care about the hits, and we normalise the null case to an empty list - # so the frontend doesn't have to handle null vs [] both ways. - instances = instances_envelope.get("hits") or [] - if not isinstance(instances, list): - instances = [] - - # /instances/hits omits the per-instance printer compatibility info that - # /design.instances[].extention.modelInfo carries (compatibility + - # otherCompatibility). Merge it in so the frontend can show "this - # instance was sliced for A1" + "also marked compatible with: H2D, P1S, - # …" before the user picks one — without that, every instance row looks - # identical in the UI and users blindly pick the first one regardless of - # whether it matches their printer. - design_instances = design.get("instances") or [] - if isinstance(design_instances, list): - compat_by_id = {} - for di in design_instances: - if not isinstance(di, dict): - continue - iid = di.get("id") - if iid is None: - continue - ext = (di.get("extention") or {}).get("modelInfo") or {} - compat_by_id[iid] = { - "compatibility": ext.get("compatibility"), - "otherCompatibility": ext.get("otherCompatibility"), - } - for inst in instances: - if not isinstance(inst, dict): - continue - iid = inst.get("id") - extra = compat_by_id.get(iid) - if extra: - inst["compatibility"] = extra["compatibility"] - inst["otherCompatibility"] = extra["otherCompatibility"] - - # Find every library row whose source_url is either the model-level - # canonical URL (legacy whole-model imports) or any plate-level URL - # (``...#profileId-{n}``) under this model. The frontend surfaces this + # Find every library row whose source_url belongs to this resource — + # the provider's :meth:`source_url_filter` owns what "belongs" means + # (whole-model key, per-plate keys, ...). The frontend surfaces the ids # to mark imported plates in the instance picker. - model_prefix = _canonical_url(model_id) existing_q = await db.execute( select(LibraryFile.id).where( - (LibraryFile.source_url == model_prefix) | (LibraryFile.source_url.like(f"{model_prefix}#profileId-%")), + provider.source_url_filter(LibraryFile.source_url, str(model_id)), LibraryFile.deleted_at.is_(None), ) ) @@ -262,17 +292,24 @@ async def resolve_url( return MakerWorldResolvedModel( model_id=model_id, profile_id=profile_id, - design=design, - instances=instances, + design=resolved.design, + instances=resolved.instances, already_imported_library_ids=already_imported, ) -@router.post("/import", response_model=MakerWorldImportResponse) +@router.post( + "/import", + response_model=MakerWorldImportResponse, + # Authentication only — the permission belongs to the provider named by + # ``source_type`` (see ``_authorize_for_provider``). + dependencies=[Depends(require_auth_if_enabled)], +) async def import_instance( body: MakerWorldImportRequest, db: AsyncSession = Depends(get_db), - current_user: User | None = RequirePermissionIfAuthEnabled(Permission.MAKERWORLD_IMPORT), + credentials: HTTPAuthorizationCredentials | None = Depends(security), + x_api_key: str | None = Header(default=None, alias="X-API-Key"), api_key_cloud_owner: User | None = Depends(resolve_api_key_cloud_owner), ): """Download a specific MakerWorld instance (plate configuration) and save @@ -282,6 +319,15 @@ async def import_instance( was imported before (any plate), that existing LibraryFile is returned and no new download happens. """ + # Resolve the provider first: an unknown ``source_type`` must 400 before + # the default-destination folder gets auto-created as a side effect — and + # the permission that applies is the resolved provider's, not MakerWorld's, + # so it cannot be checked any earlier. All that costs is telling an + # authenticated-but-unpermitted caller which source types are registered, + # which the UI lists anyway; anonymous callers never get this far. + provider = _provider_for_source(body.source_type) + current_user = await _authorize_for_provider(provider, provider.import_permission, credentials, x_api_key) + if body.folder_id is not None: folder_q = await db.execute(select(LibraryFolder).where(LibraryFolder.id == body.folder_id)) target_folder = folder_q.scalar_one_or_none() @@ -294,90 +340,78 @@ async def import_instance( ) effective_folder_id: int | None = body.folder_id else: - # Default destination: a dedicated top-level "MakerWorld" folder. Keeps - # imports out of the library root so power users can still organise - # manually in subfolders, and auto-creates the folder on the first - # import so users don't have to set it up themselves. - mw_folder_q = await db.execute( - select(LibraryFolder).where( - LibraryFolder.name == "MakerWorld", - LibraryFolder.parent_id.is_(None), - LibraryFolder.is_external.is_(False), + # Default destination: the resolved provider's dedicated top-level + # folder (``default_folder_name`` — read off *provider*, not the + # MakerWorld singleton, so the second provider lands in its own + # folder). Keeps imports out of the library root so power users can + # still organise manually in subfolders, and auto-creates the folder + # on the first import so users don't have to set it up themselves. A + # provider that leaves it unset imports into the library root rather + # than minting a NULL-named folder. + default_folder_name = provider.default_folder_name + if default_folder_name is None: + effective_folder_id = None + else: + default_folder_q = await db.execute( + select(LibraryFolder).where( + LibraryFolder.name == default_folder_name, + LibraryFolder.parent_id.is_(None), + LibraryFolder.is_external.is_(False), + ) ) - ) - mw_folder = mw_folder_q.scalar_one_or_none() - if mw_folder is None: - mw_folder = LibraryFolder(name="MakerWorld", parent_id=None) - db.add(mw_folder) - await db.flush() - effective_folder_id = mw_folder.id + default_folder = default_folder_q.scalar_one_or_none() + if default_folder is None: + default_folder = LibraryFolder(name=default_folder_name, parent_id=None) + db.add(default_folder) + await db.flush() + effective_folder_id = default_folder.id - # API-keyed callers carry identity on the key, not in current_user — see - # the /status handler comment and #1777 / #1182. The same resolved user - # is reused for owner_id on save_3mf_bytes_to_library below so the - # library row is attributed to the key's owner rather than NULL. - cloud_token_user = current_user or api_key_cloud_owner - service = await _build_service(db, cloud_token_user) + service = await _build_service(db, provider, current_user, api_key_cloud_owner) # YASTL#51's iot-service endpoint needs the *alphanumeric* modelId - # (e.g. "US2bb73b106683e5"), not the integer design id from /models/{N}. - # Fetch design metadata to resolve it, and — in the same call — pick a - # default profileId from the response if the frontend didn't specify one. + # (e.g. "US2bb73b106683e5"), not the integer design id from /models/{N} — + # resolving that, plus picking a default profile when the frontend didn't + # specify one, lives inside ``get_download``. The route only orchestrates + # dedupe + persistence so every provider shares those concerns here. + ref = ProviderResourceRef( + source_type=provider.source_type, + external_id=str(body.model_id), + sub_id=str(body.profile_id) if body.profile_id else None, + ) + try: - design = await service.get_design(body.model_id) - except MakerWorldError as exc: - await service.close() - raise _map_service_error(exc) from exc + info = await service.get_download(ref) + # The provider enriches ``sub_id`` with the actually-resolved profile + # when the caller omitted one. + resolved_profile_id = int(info.ref.sub_id) if info.ref.sub_id else None - alphanumeric_model_id = design.get("modelId") - if not isinstance(alphanumeric_model_id, str) or not alphanumeric_model_id: - await service.close() - raise HTTPException( - status_code=502, - detail="MakerWorld design metadata missing the modelId field", - ) + # Canonical URL includes profile_id so each plate gets its own library + # entry (see ``ModelProvider.canonical_url``). + source_url = provider.canonical_url(info.ref) - profile_id = body.profile_id - if profile_id is None: - for instance in design.get("instances") or []: - pid = instance.get("profileId") - if isinstance(pid, int) and pid > 0: - profile_id = pid - break - if profile_id is None: - try: - envelope = await service.get_design_instances(body.model_id) - except MakerWorldError as exc: - await service.close() - raise _map_service_error(exc) from exc - for hit in envelope.get("hits") or []: - pid = hit.get("profileId") - if isinstance(pid, int) and pid > 0: - profile_id = pid - break - if profile_id is None: - await service.close() - raise HTTPException( - status_code=502, - detail="MakerWorld returned no instances for this model", + # Dedupe check upfront so we don't burn bandwidth re-downloading. + existing_q = await db.execute(LibraryFile.active().where(LibraryFile.source_url == source_url).limit(1)) + existing_row = existing_q.scalar_one_or_none() + if existing_row is not None: + return MakerWorldImportResponse( + library_file_id=existing_row.id, + filename=existing_row.filename, + folder_id=existing_row.folder_id, + profile_id=resolved_profile_id, + was_existing=True, ) - # Canonical URL includes profile_id so each plate gets its own library - # entry (see ``_canonical_url`` docstring). - source_url = _canonical_url(body.model_id, profile_id) - - try: - manifest = await service.get_profile_download(profile_id, alphanumeric_model_id) - except MakerWorldError as exc: - await service.close() + download = await service.download(info) + except ProviderError as exc: raise _map_service_error(exc) from exc + finally: + await service.close() - signed_url = manifest.get("url") # Basename-strip any path components from the upstream filename so a # malicious response (``name: "../../evil.3mf"``) can't persist a suspect # string into the library row or the UI. On-disk storage uses a UUID # filename regardless (see library.py), so this is defence-in-depth. - raw_name = manifest.get("name") + raw_name = info.suggested_filename if isinstance(raw_name, str) and raw_name.strip(): # MakerWorld emits percent-encoded names (`%20` for spaces, etc.) # because the same string round-trips through HTTP URLs in the @@ -387,44 +421,24 @@ async def import_instance( suggested_name = os.path.basename(unquote(raw_name.strip())) or f"makerworld-{body.model_id}.3mf" else: suggested_name = f"makerworld-{body.model_id}.3mf" - if not signed_url or not isinstance(signed_url, str): - await service.close() - raise HTTPException(status_code=502, detail="MakerWorld did not return a download URL") - - # Dedupe check upfront so we don't burn bandwidth re-downloading. - if source_url: - existing_q = await db.execute(LibraryFile.active().where(LibraryFile.source_url == source_url).limit(1)) - existing_row = existing_q.scalar_one_or_none() - if existing_row is not None: - await service.close() - return MakerWorldImportResponse( - library_file_id=existing_row.id, - filename=existing_row.filename, - folder_id=existing_row.folder_id, - profile_id=profile_id, - was_existing=True, - ) - - try: - file_bytes, download_filename = await service.download_3mf(signed_url) - except MakerWorldError as exc: - await service.close() - raise _map_service_error(exc) from exc - finally: - await service.close() # Prefer the server-provided human-readable filename; the signed URL's # path ends in a UUID that's not meaningful to users. Decode the # fallback path-tail too — same percent-encoding round-trip applies # there as on the manifest-supplied name. - filename = suggested_name if suggested_name.endswith(".3mf") else unquote(download_filename) + filename = suggested_name if suggested_name.endswith(".3mf") else unquote(download.filename) + # API-keyed callers carry identity on the key, not in current_user (#1777); + # this collapse stays route-side solely so the library row is attributed + # to the key's owner rather than NULL. Credential identity is resolved + # inside the provider. + cloud_token_user = current_user or api_key_cloud_owner library_file, was_existing = await save_3mf_bytes_to_library( db, - file_bytes=file_bytes, + file_bytes=download.file_bytes, filename=filename, folder_id=effective_folder_id, - source_type=_SOURCE_TYPE, + source_type=provider.source_type, source_url=source_url, owner_id=cloud_token_user.id if cloud_token_user else None, ) @@ -433,7 +447,7 @@ async def import_instance( library_file_id=library_file.id, filename=library_file.filename, folder_id=library_file.folder_id, - profile_id=profile_id, + profile_id=resolved_profile_id, was_existing=was_existing, ) @@ -442,23 +456,27 @@ async def import_instance( async def recent_imports( limit: int = 10, db: AsyncSession = Depends(get_db), - current_user: User | None = RequirePermissionIfAuthEnabled(Permission.MAKERWORLD_VIEW), + current_user: User | None = RequirePermissionIfAuthEnabled(makerworld_provider.view_permission), ): """Last N MakerWorld imports, newest first. Surfaces files whose ``source_type`` is ``"makerworld"`` so the MakerWorld page can show a 'Recent imports' sidebar that persists across resolves. + Widening this to all registered providers is a behaviour change that + belongs with the provider that needs it. ``limit`` is clamped to ``[1, 50]`` to keep payloads sensible. """ _ = current_user # permission gate only capped = max(1, min(50, int(limit))) + result = await db.execute( LibraryFile.active() - .where(LibraryFile.source_type == _SOURCE_TYPE) + .where(LibraryFile.source_type == makerworld_provider.source_type) .order_by(LibraryFile.created_at.desc()) .limit(capped) ) rows = result.scalars().all() + return [ MakerWorldRecentImport( library_file_id=row.id, diff --git a/backend/app/api/routes/slicer_presets.py b/backend/app/api/routes/slicer_presets.py index 657d1c1ab..e55c58858 100644 --- a/backend/app/api/routes/slicer_presets.py +++ b/backend/app/api/routes/slicer_presets.py @@ -20,7 +20,7 @@ from fastapi import APIRouter, Depends, HTTPException, Query from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession -from backend.app.api.routes.cloud import get_stored_token, resolve_api_key_cloud_owner +from backend.app.api.routes.cloud import resolve_api_key_cloud_owner from backend.app.api.routes.orca_cloud import ( _ORCA_TYPE_TO_BAMBU, _build_authenticated_service as _build_orca_service, @@ -43,6 +43,7 @@ from backend.app.services.bambu_cloud import ( BambuCloudError, BambuCloudService, ) +from backend.app.services.bambu_cloud_credentials import get_stored_token from backend.app.services.orca_cloud import ( OrcaCloudAuthError, OrcaCloudError, diff --git a/backend/app/main.py b/backend/app/main.py index fb6cc3ab8..5926d2ddc 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -8925,7 +8925,7 @@ async def lifespan(app: FastAPI): import httpx as _httpx from backend.app.services.bambu_cloud import set_shared_http_client - from backend.app.services.makerworld import ( + from backend.app.services.model_providers.makerworld.service import ( set_shared_http_client as set_shared_makerworld_http_client, ) from backend.app.services.orca_cloud import ( diff --git a/backend/app/schemas/makerworld.py b/backend/app/schemas/makerworld.py index 2d31583ac..4633da343 100644 --- a/backend/app/schemas/makerworld.py +++ b/backend/app/schemas/makerworld.py @@ -42,6 +42,16 @@ class MakerWorldImportRequest(BaseModel): ..., description="The MakerWorld design ID (the number in /models/{id}).", ) + source_type: str = Field( + default="makerworld", + description=( + "Which registered model provider owns the resource. Import " + "identifies a model by numeric id rather than URL, so there is no " + "URL for the provider registry to route on — the caller names the " + "provider instead. Defaults to 'makerworld' so existing callers " + "stay unchanged." + ), + ) profile_id: int | None = Field( default=None, description=( diff --git a/backend/app/services/bambu_cloud_credentials.py b/backend/app/services/bambu_cloud_credentials.py new file mode 100644 index 000000000..260f112ce --- /dev/null +++ b/backend/app/services/bambu_cloud_credentials.py @@ -0,0 +1,118 @@ +"""Bambu Cloud credential storage. + +Single seam for reading and bookkeeping the stored Bambu Cloud bearer token: +per-user columns when auth is enabled, global ``Settings`` rows otherwise +(auth-disabled single-user installs). Lives in the services layer so feature +packages (e.g. ``model_providers``) can consume credentials without importing +the route layer — routes are just one consumer among several here. +""" + +from __future__ import annotations + +import logging +from datetime import datetime, timezone + +from sqlalchemy import select, update +from sqlalchemy.ext.asyncio import AsyncSession + +from backend.app.core.database import async_session +from backend.app.models.settings import Settings +from backend.app.models.user import User + +logger = logging.getLogger(__name__) + +# 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" +# Global (auth-disabled) counterpart of ``User.cloud_token_invalid_at``. Stores +# an ISO timestamp; absent/empty means "not known to be dead". +CLOUD_TOKEN_INVALID_KEY = "bambu_cloud_token_invalid_at" + + +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 is_cloud_token_invalid(db: AsyncSession, user: User | None = None) -> bool: + """Whether the stored Bambu token is known to have been rejected. + + Set by :func:`mark_cloud_token_invalid` the first time Bambu answers 401, + cleared on a fresh login/logout. This is the only durable record we have: + Bambu's access token is opaque (no readable expiry) and Bambuddy does not + persist the refresh token, so without this flag a dead credential looks + exactly like a live one. + """ + if user is not None: + return user.cloud_token_invalid_at is not None + result = await db.execute(select(Settings).where(Settings.key == CLOUD_TOKEN_INVALID_KEY)) + row = result.scalar_one_or_none() + return bool(row and row.value) + + +async def mark_cloud_token_invalid(user_id: int | None) -> None: + """Record that Bambu rejected the stored token. + + Opens its own session on purpose. This runs from + ``BambuCloudService._on_auth_failure``, i.e. in the middle of a route that + is about to fail — writing through that route's session would tie the flag + to a transaction the route may still roll back, and the fact that the + credential is dead is true regardless of how the request ends. + + Best-effort: a bookkeeping failure must never replace the 401 the caller + actually needs to see. ``user_id=None`` (auth-disabled single-user setup) + records the global flag — those installs *do* hold a token + (:func:`get_stored_token` reads it from ``Settings``), so the rejection + must land somewhere the status endpoints can see it. + """ + now = datetime.now(timezone.utc) + try: + async with async_session() as db: + if user_id is not None: + await db.execute(update(User).where(User.id == user_id).values(cloud_token_invalid_at=now)) + else: + result = await db.execute(select(Settings).where(Settings.key == CLOUD_TOKEN_INVALID_KEY)) + row = result.scalar_one_or_none() + if row: + row.value = now.isoformat() + else: + db.add(Settings(key=CLOUD_TOKEN_INVALID_KEY, value=now.isoformat())) + await db.commit() + logger.warning("Bambu Cloud rejected the stored token (user_id=%s) — marking the sign-in as expired", user_id) + except Exception: + logger.exception("Could not record the Bambu Cloud token as invalid") + + +async def _clear_cloud_token_invalid(db: AsyncSession, user: User | None) -> None: + """Clear the rejected-token flag — called on every fresh login and logout.""" + if user is not None: + await db.execute(update(User).where(User.id == user.id).values(cloud_token_invalid_at=None)) + return + result = await db.execute(select(Settings).where(Settings.key == CLOUD_TOKEN_INVALID_KEY)) + row = result.scalar_one_or_none() + if row: + await db.delete(row) + + +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, _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, 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), + _normalise_region(settings.get(CLOUD_REGION_KEY)), + ) diff --git a/backend/app/services/github_backup.py b/backend/app/services/github_backup.py index 5fbd04245..32e8bac7e 100644 --- a/backend/app/services/github_backup.py +++ b/backend/app/services/github_backup.py @@ -535,8 +535,8 @@ class GitHubBackupService: Both stores are read regardless: a ``Settings`` row survives enabling auth later, and dropping it silently would lose that account's presets. """ - from backend.app.api.routes.cloud import get_stored_token from backend.app.api.routes.orca_cloud import _load_credentials + from backend.app.services.bambu_cloud_credentials import get_stored_token bambu: list = [] orca: list = [] diff --git a/backend/app/services/model_providers/__init__.py b/backend/app/services/model_providers/__init__.py new file mode 100644 index 000000000..a11c0b421 --- /dev/null +++ b/backend/app/services/model_providers/__init__.py @@ -0,0 +1,51 @@ +"""Model-provider interface + registry. + +The shared seam for "import a model from a 3D model website". Providers +implement the interface (a ``ModelProvider`` descriptor + a per-request +``ProviderService`` transport) and register an instance here; the registry +routes pasted URLs to the owning provider via ``find_for_url``. MakerWorld is +the first (and currently only) registered provider. +""" + +from backend.app.services.model_providers.base import ( + ModelProvider, + ProviderAuthConfig, + ProviderAuthError, + ProviderAuthType, + ProviderDownload, + ProviderDownloadInfo, + ProviderError, + ProviderForbiddenError, + ProviderNotFoundError, + ProviderResolvedModel, + ProviderResourceRef, + ProviderService, + ProviderStatus, + ProviderUnavailableError, + ProviderUrlError, +) +from backend.app.services.model_providers.makerworld import makerworld_provider +from backend.app.services.model_providers.registry import ModelProviderRegistry, registry + +registry.register(makerworld_provider) + +__all__ = [ + "ModelProvider", + "ModelProviderRegistry", + "ProviderAuthConfig", + "ProviderAuthError", + "ProviderAuthType", + "ProviderDownload", + "ProviderDownloadInfo", + "ProviderError", + "ProviderForbiddenError", + "ProviderNotFoundError", + "ProviderResolvedModel", + "ProviderResourceRef", + "ProviderService", + "ProviderStatus", + "ProviderUnavailableError", + "ProviderUrlError", + "makerworld_provider", + "registry", +] diff --git a/backend/app/services/model_providers/base.py b/backend/app/services/model_providers/base.py new file mode 100644 index 000000000..230dee93e --- /dev/null +++ b/backend/app/services/model_providers/base.py @@ -0,0 +1,327 @@ +"""Model-provider interface. + +A *model provider* is a website that hosts 3D printer models (MakerWorld, +Thingiverse, Printables, ...) whose files Bambuddy can resolve and import +into the library. This module defines the contract every provider must +fulfil — the split being: + + * :class:`ModelProvider` — the static, provider-wide descriptor: identity + (``source_type``, ``display_name``), URL routing (``host_patterns``), + the auth it needs (or explicitly doesn't), and a factory that builds a + per-request :class:`ProviderService` seeded with the caller's stored + credentials. + * :class:`ProviderService` — one HTTP client per request, mirroring the + ``BambuCloudService`` construction pattern: resolve a model URL to + metadata + importable files, resolve + fetch a concrete download, and + proxy thumbnail images. Providers are *thin transports*: shared concerns + (library dedupe, folder auto-creation, ``save_3mf_bytes_to_library``) + stay in the route layer so every provider benefits from them. + +The interface deliberately covers everything the MakerWorld integration +needs today (see ``model_providers/makerworld/``) so that adding a new site +is: implement ``ModelProvider`` + ``ProviderService``, register it, and the +shared import API routes pasted URLs to it via ``registry.find_for_url``. + +Only interoperability — not affiliated with or endorsed by MakerWorld or any +other provider, and not intended to circumvent any access control. +""" + +from __future__ import annotations + +from abc import ABC, abstractmethod +from dataclasses import dataclass, field +from typing import TYPE_CHECKING, Any +from urllib.parse import urlparse + +import httpx + +from backend.app.core.compat import StrEnum + +if TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession + + from backend.app.core.permissions import Permission + from backend.app.models.user import User + + +class ProviderAuthType(StrEnum): + """The kind of credentials a model provider may (optionally) require.""" + + NONE = "none" + ACCESS_TOKEN = "access_token" + USERNAME_PASSWORD = "username_password" + BAMBU_CLOUD_BEARER = "bambu_cloud_bearer" # MakerWorld today: shared Bambu Cloud token + COOKIE = "cookie" # reserved for sites without a first-party API + + +@dataclass(frozen=True) +class ProviderAuthConfig: + """Declarative description of a provider's authentication requirement. + + Describes *what* the provider needs so the UI can prompt for it; the + actual storage/retrieval of credentials stays provider-specific for now + (MakerWorld reads the Bambu Cloud token the user already configured). + ``credential_fields`` names the inputs a future generic credential vault + would collect (e.g. ``("access_token",)`` or ``("username", "password")``). + """ + + auth_type: ProviderAuthType + display_label: str + description: str = "" + credential_fields: tuple[str, ...] = () + setup_hint: str = "" + + +@dataclass(frozen=True) +class ProviderResourceRef: + """Provider-agnostic handle for one model resource. + + ``external_id`` is the provider-native model identifier (MakerWorld's + integer design id as a string); ``sub_id`` is an optional secondary key + such as MakerWorld's ``profileId`` for a specific plate. + + Both ids must be **numeric strings** today: the shared route layer casts + them with ``int()`` when shaping API responses. Providers whose native + ids are not numeric need route-layer changes first — keep this contract + in mind when implementing one. + """ + + source_type: str + external_id: str + sub_id: str | None = None + original_url: str | None = None + + +@dataclass +class ProviderStatus: + """Whether the caller can use this provider right now. + + ``auth_error`` carries a human-readable reason when the caller is signed + in but the stored credential has been rejected (e.g. expired); ``None`` + when there is no error to report. ``credential_rejected`` is the + machine-readable counterpart — set exactly when the stored credential + exists *and* was refused by the provider — so callers (e.g. a route + reporting "sign-in expired") never have to infer it from ``auth_error``, + which may legitimately be set for other failures (network, rate limit). + """ + + authenticated: bool + can_download: bool + auth_error: str | None = None + credential_rejected: bool = False + + +@dataclass +class ProviderResolvedModel: + """Result of resolving a model URL. + + ``design`` and ``instances`` are provider-specific dicts passed through + verbatim — the frontend reads fields a provider may add over time, so we + don't re-shape them here. Which library rows already hold this resource + is the route layer's concern (it owns the library query) and stays out of + the resolved payload. + """ + + ref: ProviderResourceRef + design: dict[str, Any] + instances: list[dict[str, Any]] = field(default_factory=list) + + +@dataclass(frozen=True) +class ProviderDownloadInfo: + """A concrete, short-lived download for one file/plate. + + ``ref`` may be enriched by the provider with the ``sub_id`` it resolved + (e.g. the actual MakerWorld profile selected when the caller omitted + one) so the route can build the canonical dedupe URL. + """ + + ref: ProviderResourceRef + url: str + suggested_filename: str + + +@dataclass +class ProviderDownload: + """Downloaded file bytes plus the final suggested filename.""" + + file_bytes: bytes + filename: str + + +class ProviderError(Exception): + """Base exception for model-provider API errors.""" + + +class ProviderAuthError(ProviderError): + """Raised when a provider requires credentials and we have none (or the + stored one was rejected). True auth failure.""" + + +class ProviderForbiddenError(ProviderError): + """Raised when a provider refuses access despite valid authentication — + content-gated (purchase/points required, region restricted, ...).""" + + +class ProviderNotFoundError(ProviderError): + """Raised when a model / file / profile doesn't exist.""" + + +class ProviderUnavailableError(ProviderError): + """Raised on 5xx, network errors, or malformed payloads.""" + + +class ProviderUrlError(ProviderError): + """Raised when a URL isn't a model page of this provider.""" + + +class ModelProvider(ABC): + """Static descriptor + factory for one model-hosting site. + + Instances are shared (one per provider); all mutable state lives in the + per-request :class:`ProviderService` built by :meth:`build_service`. + """ + + source_type: str + display_name: str + host_patterns: tuple[str, ...] = () + auth: ProviderAuthConfig | None = None + #: Top-level library folder imports land in when the caller names no + #: folder. ``None`` imports into the library root — the route will not + #: mint a folder without a name. + default_folder_name: str | None = None + #: The permissions the routes enforce for this provider's read and import + #: operations. Optional only so the base class has a default: a provider + #: that leaves them unset is refused at the gate rather than treated as + #: unrestricted (see ``makerworld._authorize_for_provider``). + view_permission: Permission | None = None + import_permission: Permission | None = None + + @abstractmethod + async def build_service( + self, + *, + db: AsyncSession, + user: User | None, + api_key_owner: User | None = None, + client: httpx.AsyncClient | None = None, + ) -> ProviderService: + """Build a per-request service seeded with the caller's credentials. + + ``api_key_owner`` is the API key's owning user for API-keyed calls + (see ``resolve_api_key_cloud_owner``); providers use it as the + fallback identity when ``user`` is None. + """ + + @abstractmethod + def parse_url(self, url: str) -> ProviderResourceRef: + """Extract a :class:`ProviderResourceRef` from a model URL. + + Raises :class:`ProviderUrlError` when the URL isn't a model page of + this provider. + """ + + @abstractmethod + def canonical_url(self, ref: ProviderResourceRef) -> str: + """Stable dedupe key for a resource (library ``source_url``). + + All URL variants of the same resource must collapse to this string; + different resources (e.g. different plates of one model) must differ. + """ + + def source_url_filter(self, column: Any, external_id: str) -> Any: + """SQL predicate over ``LibraryFile.source_url`` selecting every row + that belongs to this resource — the whole-model canonical URL plus, + when the provider keys dedupe per sub-resource (plate/profile), every + such variant. Drives the resolve flow's already-imported detection. + + The default matches the model-level canonical URL only; providers with + recognisable per-plate URL shapes override this (see MakerWorld). + """ + prefix = self.canonical_url(ProviderResourceRef(source_type=self.source_type, external_id=external_id)) + return column == prefix + + def supports_url(self, url: str) -> bool: + """Whether ``url`` points at this provider (host-suffix match). + + Accepts scheme-less input (``makerworld.com/models/1``) the same way + :meth:`parse_url` does, so ``find_for_url`` routes exactly the URLs + the provider will then accept. + """ + if not url or not isinstance(url, str): + return False + candidate = url.strip() + if "://" not in candidate: + candidate = "https://" + candidate + try: + host = (urlparse(candidate).hostname or "").lower() + except ValueError: + return False + return any(host == pattern or host.endswith("." + pattern) for pattern in self.host_patterns) + + def thumbnail_hosts(self) -> tuple[str, ...]: + """Hosts whose image URLs may be proxied by ``fetch_thumbnail``. + + Serves as the SSRF allowlist for the provider's image proxy; empty + means the provider has no server-side thumbnail proxy. + """ + return () + + def download_hosts(self) -> tuple[str, ...]: + """Hosts whose file URLs may be fetched by the download path. + + Serves as the SSRF allowlist for :meth:`ProviderService.download`, + symmetric to :meth:`thumbnail_hosts`; empty means the provider has no + server-side file fetch (so no allowlist constraint applies). Providers + whose service fetches files must override this — a new provider gets + the same structural hint the thumbnail proxy gives its counterpart. + """ + return () + + +class ProviderService(ABC): + """Per-request client for a single provider. + + Built by :meth:`ModelProvider.build_service`, never constructed directly. + Providers must be closed after use (:meth:`close`); the shared connection + pool is only closed by the owner. + """ + + @abstractmethod + async def close(self) -> None: + """Close the client if this service instance owns it.""" + + @abstractmethod + async def get_status(self, db: AsyncSession) -> ProviderStatus: + """Report whether the caller can use this provider (credential state).""" + + @abstractmethod + async def resolve(self, ref: ProviderResourceRef) -> ProviderResolvedModel: + """Fetch metadata + the importable file/plate list for a resource.""" + + @abstractmethod + async def get_download(self, ref: ProviderResourceRef) -> ProviderDownloadInfo: + """Resolve the concrete download for a resource/file. + + May need provider-specific lookups (e.g. MakerWorld's alphanumeric + ``modelId``) and must enrich ``ref.sub_id`` with the actually-resolved + file/plate so the route can build the canonical dedupe key. + Raises ``ProviderAuthError`` when the provider requires credentials + and the caller has none. + """ + + @abstractmethod + async def download(self, info: ProviderDownloadInfo) -> ProviderDownload: + """Fetch the file bytes for a :class:`ProviderDownloadInfo`. + + Must restrict the upstream URL host to :meth:`ModelProvider.download_hosts` + (SSRF guard — the symmetric counterpart to ``fetch_thumbnail``). + """ + + @abstractmethod + async def fetch_thumbnail(self, url: str) -> tuple[bytes, str]: + """Proxy a provider CDN image, returning ``(bytes, content_type)``. + + Must restrict the upstream host to :meth:`ModelProvider.thumbnail_hosts` + (SSRF guard). + """ diff --git a/backend/app/services/model_providers/makerworld/__init__.py b/backend/app/services/model_providers/makerworld/__init__.py new file mode 100644 index 000000000..44124e89f --- /dev/null +++ b/backend/app/services/model_providers/makerworld/__init__.py @@ -0,0 +1,12 @@ +"""MakerWorld model provider package. + +Exports the provider instance the registry consumes; the implementation lives +in the sibling modules (``service``, ``http``, ``url``, ``errors``, ``auth``). +""" + +from backend.app.services.model_providers.makerworld.provider import ( + MakerWorldProvider, + makerworld_provider, +) + +__all__ = ["MakerWorldProvider", "makerworld_provider"] diff --git a/backend/app/services/model_providers/makerworld/auth.py b/backend/app/services/model_providers/makerworld/auth.py new file mode 100644 index 000000000..637c63d44 --- /dev/null +++ b/backend/app/services/model_providers/makerworld/auth.py @@ -0,0 +1,18 @@ +"""MakerWorld credential handling. + +MakerWorld downloads run on the same Bambu Cloud bearer token as the rest of +the Bambu cloud integration — there is no separate MakerWorld OAuth flow. This +module is the single seam where the MakerWorld provider reads the caller's +stored token, reports a rejected/expired credential, and records a 401 so the +whole app agrees the sign-in is dead (see ``cloud.mark_cloud_token_invalid``). +""" + +from __future__ import annotations + +from backend.app.services.bambu_cloud_credentials import ( + get_stored_token, + is_cloud_token_invalid, + mark_cloud_token_invalid, +) + +__all__ = ["get_stored_token", "is_cloud_token_invalid", "mark_cloud_token_invalid"] diff --git a/backend/app/services/model_providers/makerworld/errors.py b/backend/app/services/model_providers/makerworld/errors.py new file mode 100644 index 000000000..b266af0a3 --- /dev/null +++ b/backend/app/services/model_providers/makerworld/errors.py @@ -0,0 +1,44 @@ +"""MakerWorld error types. + +Subclasses of the generic provider hierarchy so route layers can map errors +with the provider-agnostic classes (:class:`ProviderError` and friends) while +callers that know they're talking to MakerWorld get the specific types. +""" + +from __future__ import annotations + +from backend.app.services.model_providers.base import ( + ProviderAuthError, + ProviderError, + ProviderForbiddenError, + ProviderNotFoundError, + ProviderUnavailableError, + ProviderUrlError, +) + + +class MakerWorldError(ProviderError): + """Base exception for MakerWorld API errors.""" + + +class MakerWorldAuthError(ProviderAuthError, MakerWorldError): + """Raised when MakerWorld requires a Bambu Cloud token and we don't have + one (or the one we sent was rejected). True auth failure.""" + + +class MakerWorldForbiddenError(ProviderForbiddenError, MakerWorldError): + """Raised when MakerWorld refuses access despite valid authentication — + content-gated (points required, purchase required, region restricted, + early-access, etc.).""" + + +class MakerWorldNotFoundError(ProviderNotFoundError, MakerWorldError): + """Raised when a design / profile / instance doesn't exist.""" + + +class MakerWorldUnavailableError(ProviderUnavailableError, MakerWorldError): + """Raised on 5xx, network errors, or malformed payloads.""" + + +class MakerWorldUrlError(ProviderUrlError, MakerWorldError): + """Raised when a URL isn't a makerworld.com model page.""" diff --git a/backend/app/services/model_providers/makerworld/http.py b/backend/app/services/model_providers/makerworld/http.py new file mode 100644 index 000000000..b56b5e230 --- /dev/null +++ b/backend/app/services/model_providers/makerworld/http.py @@ -0,0 +1,166 @@ +"""MakerWorld HTTP layer. + +Constants and the low-level transport helpers for the MakerWorld / Bambu Lab +APIs: the S3 presigned-download path that must reach the transport +byte-for-byte, upstream error extraction, and the CDN SSRF guard helpers used +by :class:`MakerWorldService`. + +The app-scoped shared ``httpx`` client lives with its consumer instead (see +``service.set_shared_http_client``) — same-module so the service reads the +live value, matching ``bambu_cloud`` / ``orca_cloud`` / ``slicer_api``. +""" + +from __future__ import annotations + +import asyncio +import ssl + +import certifi +import httpx + +from backend.app.services.model_providers.makerworld.errors import MakerWorldUnavailableError + +# API base: ``api.bambulab.com/v1/design-service`` — the same Bambu Cloud +# backend that the MakerWorld web UI talks to, but not behind Cloudflare +# (the website ``makerworld.com`` is, and plain httpx requests there get +# fingerprinted as bot traffic and served "Please log in"). +MAKERWORLD_API_BASE = "https://api.bambulab.com/v1/design-service" + +# Besides MakerWorld's own CDN, Bambu Cloud also issues AWS S3 presigned +# URLs (e.g. ``s3.us-west-2.amazonaws.com``) from the iot-service download +# endpoint. The suffix check matches any regional S3 endpoint. +# +# Deliberately NOT part of the ``download_hosts()`` seam: that seam is an +# exact-host allowlist a provider declares, and this is a suffix family +# belonging to Bambu's signed-URL infrastructure specifically. It stays a +# constant of *this* provider's transport: ``download_3mf`` accepts the +# injected hosts or an S3 endpoint, while a second provider brings its own +# service and declares its own ``download_hosts()``. +_ALLOWED_DOWNLOAD_SUFFIXES = (".amazonaws.com",) + +# The shared default SSRF allowlist for MakerWorld CDN traffic. The thumbnail +# proxy and the 3MF download path are both driven by the provider descriptor +# instead — ``build_service`` feeds the runner's ``ModelProvider.thumbnail_hosts()`` +# and ``download_hosts()`` into ``MakerWorldService`` — and this tuple is what +# those methods return by default. Lives here with the other transport guards +# so the allowlist is in one place. +MAKERWORLD_CDN_HOSTS = ("makerworld.bblmw.com", "public-cdn.bblmw.com") + +# Client identity sent to MakerWorld / api.bambulab.com. We identify honestly +# as Bambuddy with a source URL so Bambu can distinguish our traffic from +# impersonators — the opposite of what the OrcaSlicer fork was called out for +# in the May 2026 Bambu Lab blog post on cloud access. The Referer is kept +# because MakerWorld's CSRF / origin-check middleware uses it on some +# endpoints — that's distinct from client impersonation. +_CLIENT_HEADERS = { + "User-Agent": "Bambuddy/1.0 (+https://github.com/maziggy/bambuddy)", + "Accept": "text/html,application/json,*/*", + "Accept-Language": "en-US,en;q=0.9", + "Referer": "https://makerworld.com/", +} + +_MAX_3MF_BYTES = 200 * 1024 * 1024 # 200 MB hard cap +_MAX_THUMBNAIL_BYTES = 10 * 1024 * 1024 # 10 MB hard cap — MakerWorld's "thumbnails" can be 2–3 MB source images + +_IMAGE_EXT_TO_MIME = { + ".png": "image/png", + ".jpg": "image/jpeg", + ".jpeg": "image/jpeg", + ".gif": "image/gif", + ".webp": "image/webp", + ".bmp": "image/bmp", +} +# Content types we refuse even if the URL extension looks image-y — prevents +# forwarding an upstream error page or JSON blob with image framing. +_REFUSED_THUMBNAIL_MIMES = ("text/html", "text/plain", "application/json") + + +def _s3_ssl_context() -> ssl.SSLContext: + """Build the TLS context used for the S3 presigned download (#2562). + + ``urllib.request`` verifies against the *OS* trust store, while httpx — + every other network call in Bambuddy — verifies against the bundled + ``certifi`` CA bundle. On Windows those two disagree: Python's + ``ssl.load_default_certs()`` only enumerates the roots already cached in + the Windows ROOT store, and Windows populates that store lazily via + CryptoAPI's auto-update, which Python never triggers. If the Amazon root + signing the S3 chain isn't cached on that machine yet, verification fails + with ``unable to get local issuer certificate`` — even though the + api.bambulab.com calls that preceded it (httpx) succeeded. + + Pinning urllib to certifi makes the S3 hop trust exactly what the rest of + the app already trusts. Built per call rather than at import so a certifi + refresh doesn't require a restart; construction is cheap relative to the + download that follows. + """ + return ssl.create_default_context(cafile=certifi.where()) + + +async def _download_s3_urllib(url: str, filename_fallback: str) -> tuple[bytes, str]: + """Fetch an AWS S3 presigned URL without touching the query string. + + ``urllib.request`` passes the URL to the transport verbatim — which is + essential for S3 presigned URLs where the signature is computed over + the exact query-string bytes. httpx's ``URL`` class and curl_cffi's + libcurl layer both normalise encodings and produce + ``SignatureDoesNotMatch`` 400s from S3. + + Runs the blocking urllib call in a thread executor so we don't stall + the event loop. + """ + from urllib.request import HTTPRedirectHandler, HTTPSHandler, Request, build_opener + + # Don't follow redirects: the host allowlist is only enforced on + # the initial URL. A 302 from S3 to any other host would otherwise + # transparently bypass the allowlist — so insist S3 resolve directly. + class _NoRedirect(HTTPRedirectHandler): + def redirect_request(self, *args, **kwargs): # type: ignore[override] + return None + + # HTTPSHandler swaps only the TLS context — the URL still reaches the + # transport verbatim, which is what the S3 signature depends on. + opener = build_opener(_NoRedirect, HTTPSHandler(context=_s3_ssl_context())) + + def _blocking_fetch() -> bytes: + req = Request(url, headers={"User-Agent": _CLIENT_HEADERS["User-Agent"]}) + with opener.open(req, timeout=60.0) as resp: + if resp.status != 200: + raise MakerWorldUnavailableError(f"3MF download returned HTTP {resp.status}") + data = b"" + while True: + chunk = resp.read(65536) + if not chunk: + break + data += chunk + if len(data) > _MAX_3MF_BYTES: + raise MakerWorldUnavailableError(f"3MF exceeds {_MAX_3MF_BYTES // (1024 * 1024)} MB cap") + return data + + try: + data = await asyncio.to_thread(_blocking_fetch) + except MakerWorldUnavailableError: + raise + except Exception as exc: # noqa: BLE001 — urllib throws a zoo of exceptions + raise MakerWorldUnavailableError(f"S3 download failed: {exc}") from exc + return data, filename_fallback + + +def _extract_upstream_error(response: httpx.Response) -> str | None: + """Pull MakerWorld's own error text out of a 4xx/5xx response body. + + MakerWorld returns ``{"code": N, "error": "text"}`` on auth/perm failures + and sometimes ``{"message": "..."}`` on other errors. Returns ``None`` if + the body isn't JSON or doesn't have a recognised error field — callers + should fall back to a generic message in that case. + """ + try: + data = response.json() + except ValueError: + return None + if not isinstance(data, dict): + return None + for key in ("error", "message", "detail"): + value = data.get(key) + if isinstance(value, str) and value.strip(): + return value.strip() + return None diff --git a/backend/app/services/model_providers/makerworld/provider.py b/backend/app/services/model_providers/makerworld/provider.py new file mode 100644 index 000000000..9393c3dde --- /dev/null +++ b/backend/app/services/model_providers/makerworld/provider.py @@ -0,0 +1,117 @@ +"""MakerWorld model provider. + +Static descriptor + per-request service factory for makerworld.com. The +``MakerWorldProvider`` instance is what gets registered in the shared +:class:`ModelProviderRegistry`; the actual API work lives in ``service.py`` +(the per-request :class:`ProviderService`) and ``url.py`` (URL parsing and +canonicalisation). Credential handling is centralised here so route layers +never touch MakerWorld specifics. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import httpx + +from backend.app.core.permissions import Permission +from backend.app.services.model_providers.base import ( + ModelProvider, + ProviderAuthConfig, + ProviderAuthType, + ProviderResourceRef, + ProviderService, +) +from backend.app.services.model_providers.makerworld import url as mw_url +from backend.app.services.model_providers.makerworld.auth import ( + get_stored_token, + mark_cloud_token_invalid, +) +from backend.app.services.model_providers.makerworld.http import MAKERWORLD_CDN_HOSTS +from backend.app.services.model_providers.makerworld.service import MakerWorldService + +if TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession + + from backend.app.models.user import User + + +class MakerWorldProvider(ModelProvider): + """MakerWorld descriptor: identity, URL routing, auth requirements, and the + factory that builds a per-request :class:`MakerWorldService` seeded with the + caller's stored Bambu Cloud bearer token. + """ + + source_type = "makerworld" + display_name = "MakerWorld" + host_patterns = ("makerworld.com",) + auth = ProviderAuthConfig( + auth_type=ProviderAuthType.BAMBU_CLOUD_BEARER, + display_label="Bambu Cloud sign-in", + description=( + "MakerWorld downloads reuse the Bambu Cloud account already stored in Bambuddy — " + "there is no separate MakerWorld sign-in." + ), + setup_hint="Open the Profiles page and sign in to Bambu Cloud.", + ) + default_folder_name = "MakerWorld" + view_permission = Permission.MAKERWORLD_VIEW + import_permission = Permission.MAKERWORLD_IMPORT + + async def build_service( + self, + *, + db: AsyncSession, + user: User | None, + api_key_owner: User | None = None, + client: httpx.AsyncClient | None = None, + ) -> ProviderService: + """Build a per-request service seeded with the caller's stored Bambu + Cloud bearer, mirroring ``cloud.build_authenticated_cloud``. + + ``api_key_owner`` is the API key's owning user for API-keyed calls + (see ``resolve_api_key_cloud_owner``); MakerWorld uses it as the + fallback identity when ``user`` is None. Like the cloud integration, a + rejected token is recorded so the whole app agrees the sign-in is dead + rather than each feature failing on its own — including auth-disabled + single-user installs, where ``user_id=None`` records the *global* + flag those installs read back on the status endpoints. + """ + identity = user if user is not None else api_key_owner + token, _email, _region = await get_stored_token(db, identity) + user_id = identity.id if identity is not None else None + return MakerWorldService( + client=client, + auth_token=token, + user=identity, + on_auth_failure=lambda: mark_cloud_token_invalid(user_id), + # The SSRF allowlists are the provider's declared seams — the + # service must not hardcode its own copies (symmetric pair, + # ``fetch_thumbnail`` / ``download``). + thumbnail_hosts=self.thumbnail_hosts(), + download_hosts=self.download_hosts(), + ) + + def parse_url(self, url: str) -> ProviderResourceRef: + return mw_url.parse_url(url) + + def canonical_url(self, ref: ProviderResourceRef) -> str: + return mw_url.canonical_url(ref) + + def source_url_filter(self, column, external_id: str): + """Whole-model key plus every per-plate key — MakerWorld's canonical + shape appends ``#profileId-{n}`` for plate-level dedupe (see + ``url.canonical_url``), so the already-imported detection must match + both. The ``#profileId-`` fragment lives here with the descriptor + because it is part of this provider's URL contract.""" + prefix = mw_url.canonical_url(ProviderResourceRef(source_type=self.source_type, external_id=external_id)) + return (column == prefix) | (column.like(f"{prefix}#profileId-%")) + + def thumbnail_hosts(self) -> tuple[str, ...]: + return MAKERWORLD_CDN_HOSTS + + def download_hosts(self) -> tuple[str, ...]: + return MAKERWORLD_CDN_HOSTS + + +makerworld_provider = MakerWorldProvider() diff --git a/backend/app/services/makerworld.py b/backend/app/services/model_providers/makerworld/service.py similarity index 68% rename from backend/app/services/makerworld.py rename to backend/app/services/model_providers/makerworld/service.py index bd19e041a..fba1ba8c1 100644 --- a/backend/app/services/makerworld.py +++ b/backend/app/services/model_providers/makerworld/service.py @@ -9,7 +9,11 @@ The endpoints and header set were reverse-engineered from the `kloshi-io/makerworld-api-reverse` TypeScript project (Apache-2.0) and cross-validated against live MakerWorld traffic. Authenticated calls reuse Bambuddy's existing Bambu Cloud bearer token (same SSO backend — no separate -OAuth flow needed). +OAuth flow needed; see ``model_providers/makerworld/auth.py``). + +Implements the :class:`ProviderService` interface — the route layer drives it +through ``resolve`` / ``get_download`` / ``download`` so the same flow can be +reused for future providers. Only interoperability — not affiliated with or endorsed by MakerWorld or Bambu Lab, and not intended to circumvent any access control. @@ -19,49 +23,59 @@ from __future__ import annotations import asyncio import logging -import re -import ssl from collections.abc import Awaitable, Callable +from dataclasses import replace from typing import Any from urllib.parse import urlparse -import certifi import httpx from backend.app.services.bambu_cloud import is_captcha_challenge, is_expiry_401 +from backend.app.services.model_providers.base import ( + ProviderDownload, + ProviderDownloadInfo, + ProviderResolvedModel, + ProviderResourceRef, + ProviderService, + ProviderStatus, +) +from backend.app.services.model_providers.makerworld.auth import is_cloud_token_invalid +from backend.app.services.model_providers.makerworld.errors import ( + MakerWorldAuthError, + MakerWorldForbiddenError, + MakerWorldNotFoundError, + MakerWorldUnavailableError, + MakerWorldUrlError, +) +from backend.app.services.model_providers.makerworld.http import ( + _ALLOWED_DOWNLOAD_SUFFIXES, + _CLIENT_HEADERS, + _IMAGE_EXT_TO_MIME, + _MAX_3MF_BYTES, + _MAX_THUMBNAIL_BYTES, + _REFUSED_THUMBNAIL_MIMES, + MAKERWORLD_API_BASE, + MAKERWORLD_CDN_HOSTS, + _download_s3_urllib, + _extract_upstream_error, +) logger = logging.getLogger(__name__) +_shared_http_client: httpx.AsyncClient | None = None -# API base: ``api.bambulab.com/v1/design-service`` — the same Bambu Cloud -# backend that the MakerWorld web UI talks to, but not behind Cloudflare -# (the website ``makerworld.com`` is, and plain httpx requests there get -# fingerprinted as bot traffic and served "Please log in"). Confirmed by -# Pr0zak/YASTL#51 and verified with direct curl. -MAKERWORLD_API_BASE = "https://api.bambulab.com/v1/design-service" -MAKERWORLD_HOST = "makerworld.com" # Used only for URL parsing (input validation) -MAKERWORLD_CDN_HOSTS = ("makerworld.bblmw.com", "public-cdn.bblmw.com") -# Hosts that the iot-service download endpoint may return presigned URLs -# for. Besides MakerWorld's own CDN, Bambu Cloud also issues AWS S3 -# presigned URLs (e.g. ``s3.us-west-2.amazonaws.com``) — confirmed by -# Pr0zak/YASTL#52. The suffix check matches any regional S3 endpoint. -_ALLOWED_DOWNLOAD_SUFFIXES = (".amazonaws.com",) +def set_shared_http_client(client: httpx.AsyncClient | None) -> None: + """Register an app-scoped ``httpx.AsyncClient`` for service reuse. + + Same pattern as ``bambu_cloud.set_shared_http_client`` — lets the FastAPI + lifespan share one connection pool across per-request service instances. + Must live in the same module as the service class so ``__init__`` reads + the live value rather than an import-time snapshot. + """ + global _shared_http_client + _shared_http_client = client -# Client identity sent to MakerWorld / api.bambulab.com. We identify honestly -# as Bambuddy with a source URL so Bambu can distinguish our traffic from -# impersonators — the opposite of what the OrcaSlicer fork was called out for -# in the May 2026 Bambu Lab blog post on cloud access. Verified 2026-05-12 via -# curl that MakerWorld treats this UA identically to a Firefox UA at the -# Cloudflare edge (same response shape on /api/v1/design-service/* paths). -# The Referer is kept because MakerWorld's CSRF / origin-check middleware uses -# it on some endpoints — that's distinct from client impersonation. -_CLIENT_HEADERS = { - "User-Agent": "Bambuddy/1.0 (+https://github.com/maziggy/bambuddy)", - "Accept": "text/html,application/json,*/*", - "Accept-Language": "en-US,en;q=0.9", - "Referer": "https://makerworld.com/", -} # Shown whenever Bambu rejects the stored bearer. Bambu's own 401 body is # ``{"code":4,"error":"Please login.","message":""}`` and we used to forward that @@ -75,155 +89,8 @@ _SIGN_IN_EXPIRED_MESSAGE = ( "Your Bambu Cloud sign-in has expired. Open the Profiles page and sign in to Bambu Cloud again." ) -_MODEL_ID_RE = re.compile(r"/models/(\d+)") -_PROFILE_ID_RE = re.compile(r"#profileId[-=](\d+)") -_MAX_3MF_BYTES = 200 * 1024 * 1024 # 200 MB hard cap -_MAX_THUMBNAIL_BYTES = 10 * 1024 * 1024 # 10 MB hard cap — MakerWorld's "thumbnails" can be 2–3 MB source images -_IMAGE_EXT_TO_MIME = { - ".png": "image/png", - ".jpg": "image/jpeg", - ".jpeg": "image/jpeg", - ".gif": "image/gif", - ".webp": "image/webp", - ".bmp": "image/bmp", -} -# Content types we refuse even if the URL extension looks image-y — prevents -# forwarding an upstream error page or JSON blob with image framing. -_REFUSED_THUMBNAIL_MIMES = ("text/html", "text/plain", "application/json") -_shared_http_client: httpx.AsyncClient | None = None - - -def _s3_ssl_context() -> ssl.SSLContext: - """Build the TLS context used for the S3 presigned download (#2562). - - ``urllib.request`` verifies against the *OS* trust store, while httpx — - every other network call in Bambuddy — verifies against the bundled - ``certifi`` CA bundle. On Windows those two disagree: Python's - ``ssl.load_default_certs()`` only enumerates the roots already cached in - the Windows ROOT store, and Windows populates that store lazily via - CryptoAPI's auto-update, which Python never triggers. If the Amazon root - signing the S3 chain isn't cached on that machine yet, verification fails - with ``unable to get local issuer certificate`` — even though the - api.bambulab.com calls that preceded it (httpx) succeeded. - - Pinning urllib to certifi makes the S3 hop trust exactly what the rest of - the app already trusts. Built per call rather than at import so a certifi - refresh doesn't require a restart; construction is cheap relative to the - download that follows. - """ - return ssl.create_default_context(cafile=certifi.where()) - - -def set_shared_http_client(client: httpx.AsyncClient | None) -> None: - """Register an app-scoped ``httpx.AsyncClient`` for service reuse. - - Same pattern as ``bambu_cloud.set_shared_http_client`` — lets the FastAPI - lifespan share one connection pool across per-request service instances. - """ - global _shared_http_client - _shared_http_client = client - - -class MakerWorldError(Exception): - """Base exception for MakerWorld API errors.""" - - -class MakerWorldAuthError(MakerWorldError): - """Raised when the endpoint requires a Bambu Cloud token and we don't have - one (or the one we sent was rejected). True auth failure.""" - - -class MakerWorldForbiddenError(MakerWorldError): - """Raised when MakerWorld refuses access despite valid authentication — - content-gated (points required, purchase required, region restricted, - early-access, etc.). The message includes MakerWorld's own reason text - when provided.""" - - -class MakerWorldNotFoundError(MakerWorldError): - """Raised when a design / profile / instance doesn't exist.""" - - -class MakerWorldUnavailableError(MakerWorldError): - """Raised on 5xx, network errors, or malformed payloads.""" - - -class MakerWorldUrlError(MakerWorldError): - """Raised when a URL isn't a makerworld.com model page.""" - - -async def _download_s3_urllib(url: str, filename_fallback: str) -> tuple[bytes, str]: - """Fetch an AWS S3 presigned URL without touching the query string. - - ``urllib.request`` passes the URL to the transport verbatim — which is - essential for S3 presigned URLs where the signature is computed over - the exact query-string bytes. httpx's ``URL`` class and curl_cffi's - libcurl layer both normalise encodings and produce - ``SignatureDoesNotMatch`` 400s from S3. - - Runs the blocking urllib call in a thread executor so we don't stall - the event loop. - """ - from urllib.request import HTTPRedirectHandler, HTTPSHandler, Request, build_opener - - # Don't follow redirects: the host allowlist above is only enforced on - # the initial URL. A 302 from S3 to any other host would otherwise - # transparently bypass the allowlist — so insist S3 resolve directly. - class _NoRedirect(HTTPRedirectHandler): - def redirect_request(self, *args, **kwargs): # type: ignore[override] - return None - - # HTTPSHandler swaps only the TLS context — the URL still reaches the - # transport verbatim, which is what the S3 signature depends on. - opener = build_opener(_NoRedirect, HTTPSHandler(context=_s3_ssl_context())) - - def _blocking_fetch() -> bytes: - req = Request(url, headers={"User-Agent": _CLIENT_HEADERS["User-Agent"]}) - with opener.open(req, timeout=60.0) as resp: - if resp.status != 200: - raise MakerWorldUnavailableError(f"3MF download returned HTTP {resp.status}") - data = b"" - while True: - chunk = resp.read(65536) - if not chunk: - break - data += chunk - if len(data) > _MAX_3MF_BYTES: - raise MakerWorldUnavailableError(f"3MF exceeds {_MAX_3MF_BYTES // (1024 * 1024)} MB cap") - return data - - try: - data = await asyncio.to_thread(_blocking_fetch) - except MakerWorldUnavailableError: - raise - except Exception as exc: # noqa: BLE001 — urllib throws a zoo of exceptions - raise MakerWorldUnavailableError(f"S3 download failed: {exc}") from exc - return data, filename_fallback - - -def _extract_upstream_error(response: httpx.Response) -> str | None: - """Pull MakerWorld's own error text out of a 4xx/5xx response body. - - MakerWorld returns ``{"code": N, "error": "text"}`` on auth/perm failures - and sometimes ``{"message": "..."}`` on other errors. Returns ``None`` if - the body isn't JSON or doesn't have a recognised error field — callers - should fall back to a generic message in that case. - """ - try: - data = response.json() - except ValueError: - return None - if not isinstance(data, dict): - return None - for key in ("error", "message", "detail"): - value = data.get(key) - if isinstance(value, str) and value.strip(): - return value.strip() - return None - - -class MakerWorldService: +class MakerWorldService(ProviderService): """Per-request MakerWorld API client. Mirrors ``BambuCloudService``'s construction pattern so callers can @@ -233,15 +100,26 @@ class MakerWorldService: def __init__( self, + *, client: httpx.AsyncClient | None = None, auth_token: str | None = None, + user: Any | None = None, on_auth_failure: Callable[[], Awaitable[None]] | None = None, + thumbnail_hosts: tuple[str, ...] = MAKERWORLD_CDN_HOSTS, + download_hosts: tuple[str, ...] = MAKERWORLD_CDN_HOSTS, ): # Fired when Bambu rejects the stored token (401). MakerWorld runs on the # same Bambu Cloud bearer as everything else, so a rejection here means # the credential is dead app-wide — see ``build_authenticated_cloud``. self._on_auth_failure = on_auth_failure self._auth_failure_reported = False + # SSRF allowlists for the thumbnail proxy and the 3MF download guard. + # Default to MakerWorld's CDN hosts; ``MakerWorldProvider.build_service`` + # passes ``ModelProvider.thumbnail_hosts()`` / ``download_hosts()`` so + # the guards are driven by the provider descriptor rather than enforced + # by coincidence (interface contract on ``ProviderService``). + self._thumbnail_hosts = tuple(thumbnail_hosts) + self._download_hosts = tuple(download_hosts) if client is not None: self._client = client self._owns_client = False @@ -252,6 +130,7 @@ class MakerWorldService: self._client = httpx.AsyncClient(timeout=30.0) self._owns_client = True self._auth_token = auth_token + self._user = user async def close(self) -> None: if self._owns_client: @@ -283,6 +162,121 @@ class MakerWorldService: headers["Authorization"] = f"Bearer {self._auth_token}" return headers + # ------------------------------------------------------------- interface + + async def get_status(self, db: Any) -> ProviderStatus: + """Whether the caller can download: needs a stored, non-rejected Bambu + Cloud token. ``credential_rejected`` is the machine-readable expired + state; ``auth_error`` names it for humans so the UI can say "your + sign-in expired" rather than a bare "sign in".""" + has_token = bool(self._auth_token) + expired = has_token and await is_cloud_token_invalid(db, self._user) + return ProviderStatus( + authenticated=has_token, + can_download=has_token and not expired, + auth_error=_SIGN_IN_EXPIRED_MESSAGE if expired else None, + credential_rejected=expired, + ) + + async def resolve(self, ref: ProviderResourceRef) -> ProviderResolvedModel: + """Fetch full model metadata + the plate list, merging per-instance + printer compatibility so the frontend can show "sliced for A1 / also + compatible with H2D, P1S" before the user picks a plate.""" + model_id = int(ref.external_id) + design = await self.get_design(model_id) + instances_envelope = await self.get_design_instances(model_id) + + # MakerWorld's instances payload is ``{"total": N, "hits": [...]}``; + # normalise the null case to an empty list so the frontend doesn't + # have to handle null vs [] both ways. + instances = instances_envelope.get("hits") or [] + if not isinstance(instances, list): + instances = [] + + # /instances/hits omits the per-instance printer compatibility info + # that /design.instances[].extention.modelInfo carries. Merge it in. + design_instances = design.get("instances") or [] + if isinstance(design_instances, list): + compat_by_id = {} + for di in design_instances: + if not isinstance(di, dict): + continue + iid = di.get("id") + if iid is None: + continue + ext = (di.get("extention") or {}).get("modelInfo") or {} + compat_by_id[iid] = { + "compatibility": ext.get("compatibility"), + "otherCompatibility": ext.get("otherCompatibility"), + } + for inst in instances: + if not isinstance(inst, dict): + continue + iid = inst.get("id") + extra = compat_by_id.get(iid) + if extra: + inst["compatibility"] = extra["compatibility"] + inst["otherCompatibility"] = extra["otherCompatibility"] + + return ProviderResolvedModel(ref=ref, design=design, instances=instances) + + async def get_download(self, ref: ProviderResourceRef) -> ProviderDownloadInfo: + """Resolve the signed 3MF download for a specific MakerWorld profile. + + Handles the provider-specific dance: the iot-service endpoint needs + the *alphanumeric* ``modelId`` (e.g. ``"US2bb73b106683e5"``) from the + design, not the integer design id, and picks a default profile when + the caller didn't specify one. Enriches ``ref.sub_id`` with the actual + profile used so the route can build the per-plate dedupe key. + """ + model_id = int(ref.external_id) + design = await self.get_design(model_id) + + alphanumeric_model_id = design.get("modelId") + if not isinstance(alphanumeric_model_id, str) or not alphanumeric_model_id: + raise MakerWorldUnavailableError("MakerWorld design metadata missing the modelId field") + + profile_id = int(ref.sub_id) if ref.sub_id else None + if profile_id is None: + for instance in design.get("instances") or []: + pid = instance.get("profileId") + if isinstance(pid, int) and pid > 0: + profile_id = pid + break + if profile_id is None: + envelope = await self.get_design_instances(model_id) + for hit in envelope.get("hits") or []: + pid = hit.get("profileId") + if isinstance(pid, int) and pid > 0: + profile_id = pid + break + if profile_id is None: + raise MakerWorldUnavailableError("MakerWorld returned no instances for this model") + + manifest = await self.get_profile_download(profile_id, alphanumeric_model_id) + + signed_url = manifest.get("url") + if not signed_url or not isinstance(signed_url, str): + raise MakerWorldUnavailableError("MakerWorld did not return a download URL") + + # Raw upstream name — the route layer basenames / percent-decodes it + # as defence-in-depth before persisting. + raw_name = manifest.get("name") + suggested_filename = raw_name if isinstance(raw_name, str) and raw_name.strip() else "" + + return ProviderDownloadInfo( + ref=replace(ref, sub_id=str(profile_id)), + url=signed_url, + suggested_filename=suggested_filename, + ) + + async def download(self, info: ProviderDownloadInfo) -> ProviderDownload: + """Fetch the 3MF bytes for a signed URL, returning ``(bytes, filename)``.""" + file_bytes, download_filename = await self.download_3mf(info.url) + return ProviderDownload(file_bytes=file_bytes, filename=download_filename) + + # ---------------------------------------------------------------- endpoints + async def _get_json(self, path: str) -> dict[str, Any]: """GET ``{MAKERWORLD_API_BASE}{path}`` returning the decoded JSON body. @@ -376,49 +370,6 @@ class MakerWorldService: ) return data - # ------------------------------------------------------------------ URL parse - - @staticmethod - def parse_url(url: str) -> tuple[int, int | None]: - """Extract ``(model_id, profile_id_or_None)`` from a MakerWorld URL. - - Accepts any of: - - ``https://makerworld.com/en/models/1400373`` - - ``https://makerworld.com/en/models/1400373-slug-with-dashes`` - - ``https://makerworld.com/en/models/1400373#profileId-1452154`` - - ``makerworld.com/models/1400373`` (scheme optional) - - Rejects non-makerworld hosts. - """ - if not url or not isinstance(url, str): - raise MakerWorldUrlError("URL is empty or not a string") - candidate = url.strip() - if "://" not in candidate: - candidate = "https://" + candidate - try: - parsed = urlparse(candidate) - except ValueError as exc: - raise MakerWorldUrlError(f"Could not parse URL: {exc}") from exc - - host = (parsed.hostname or "").lower() - if host != MAKERWORLD_HOST and not host.endswith("." + MAKERWORLD_HOST): - raise MakerWorldUrlError(f"Not a MakerWorld URL (host={host!r}); expected makerworld.com") - - model_match = _MODEL_ID_RE.search(parsed.path) - if not model_match: - raise MakerWorldUrlError("URL does not contain a /models/{id} segment") - model_id = int(model_match.group(1)) - - profile_id: int | None = None - if parsed.fragment: - profile_match = _PROFILE_ID_RE.search("#" + parsed.fragment) - if profile_match: - profile_id = int(profile_match.group(1)) - - return model_id, profile_id - - # ---------------------------------------------------------------- endpoints - async def get_design(self, model_id: int) -> dict[str, Any]: """Fetch full model metadata. Works anonymously. @@ -510,8 +461,13 @@ class MakerWorldService: async def download_3mf(self, signed_url: str) -> tuple[bytes, str]: """Fetch the 3MF bytes from a signed MakerWorld CDN URL. - Validates that the URL's host is one of the known MakerWorld CDN hosts - (SSRF guard — pattern matches ``_spoolman_helpers.assert_safe_spoolman_url``). + Validates that the URL's host is one of the declared download hosts + (SSRF guard — driven by ``ModelProvider.download_hosts()`` via + ``build_service``, the symmetric counterpart to the thumbnail + allowlist) *or* matches ``_ALLOWED_DOWNLOAD_SUFFIXES``, Bambu's S3 + regional endpoints, which are this provider's own signed-URL family + rather than part of the injectable seam; pattern matches + ``_spoolman_helpers.assert_safe_spoolman_url``. Enforces a 200 MB cap so a single bad response can't exhaust disk. Returns ``(file_bytes, suggested_filename)``. @@ -522,7 +478,7 @@ class MakerWorldService: raise MakerWorldUrlError(f"Invalid download URL: {exc}") from exc host = (parsed.hostname or "").lower() - is_allowed = host in MAKERWORLD_CDN_HOSTS or any(host.endswith(suffix) for suffix in _ALLOWED_DOWNLOAD_SUFFIXES) + is_allowed = host in self._download_hosts or any(host.endswith(suffix) for suffix in _ALLOWED_DOWNLOAD_SUFFIXES) if not is_allowed: raise MakerWorldUrlError(f"Refusing to download from non-MakerWorld host: {host!r}") @@ -570,8 +526,9 @@ class MakerWorldService: SPA's ``img-src`` CSP and keeps users' IP addresses out of MakerWorld's access logs. - Validates that the URL's host is one of the known MakerWorld CDN - hosts (SSRF guard — same allowlist as :meth:`download_3mf`). Caps + Validates that the URL's host is one of the declared thumbnail hosts + (SSRF guard — symmetric to :meth:`download_3mf`; both allowlists are + fed from the provider descriptor by ``build_service``). Caps payload at 5 MB. Returns ``(bytes, content_type)``; content type defaults to ``image/jpeg`` if the upstream didn't set one. """ @@ -581,7 +538,7 @@ class MakerWorldService: raise MakerWorldUrlError(f"Invalid thumbnail URL: {exc}") from exc host = (parsed.hostname or "").lower() - if host not in MAKERWORLD_CDN_HOSTS: + if host not in self._thumbnail_hosts: raise MakerWorldUrlError(f"Refusing to fetch thumbnail from non-MakerWorld host: {host!r}") # ``follow_redirects=False``: the host allowlist above is only diff --git a/backend/app/services/model_providers/makerworld/url.py b/backend/app/services/model_providers/makerworld/url.py new file mode 100644 index 000000000..add2bdc52 --- /dev/null +++ b/backend/app/services/model_providers/makerworld/url.py @@ -0,0 +1,82 @@ +"""MakerWorld URL parsing and canonicalisation. + +Extracts ``(model_id, profile_id_or_None)`` from model URLs and builds the +stable dedupe key used as the library ``source_url``. Rejects non-makerworld +hosts — this is the input-validation surface, so it is deliberately strict. +""" + +from __future__ import annotations + +import re +from urllib.parse import urlparse + +from backend.app.services.model_providers.base import ProviderResourceRef +from backend.app.services.model_providers.makerworld.errors import MakerWorldUrlError + +MAKERWORLD_HOST = "makerworld.com" # Used only for URL parsing (input validation) + +_MODEL_ID_RE = re.compile(r"/models/(\d+)") +_PROFILE_ID_RE = re.compile(r"#profileId[-=](\d+)") + + +def parse_url(url: str) -> ProviderResourceRef: + """Extract a :class:`ProviderResourceRef` from a MakerWorld URL. + + Accepts any of: + - ``https://makerworld.com/en/models/1400373`` + - ``https://makerworld.com/en/models/1400373-slug-with-dashes`` + - ``https://makerworld.com/en/models/1400373#profileId-1452154`` + - ``makerworld.com/models/1400373`` (scheme optional) + + Rejects non-makerworld hosts. + """ + if not url or not isinstance(url, str): + raise MakerWorldUrlError("URL is empty or not a string") + candidate = url.strip() + if "://" not in candidate: + candidate = "https://" + candidate + try: + parsed = urlparse(candidate) + except ValueError as exc: + raise MakerWorldUrlError(f"Could not parse URL: {exc}") from exc + + host = (parsed.hostname or "").lower() + if host != MAKERWORLD_HOST and not host.endswith("." + MAKERWORLD_HOST): + raise MakerWorldUrlError(f"Not a MakerWorld URL (host={host!r}); expected makerworld.com") + + model_match = _MODEL_ID_RE.search(parsed.path) + if not model_match: + raise MakerWorldUrlError("URL does not contain a /models/{id} segment") + model_id = int(model_match.group(1)) + + profile_id: int | None = None + if parsed.fragment: + profile_match = _PROFILE_ID_RE.search("#" + parsed.fragment) + if profile_match: + profile_id = int(profile_match.group(1)) + + return ProviderResourceRef( + source_type="makerworld", + external_id=str(model_id), + sub_id=str(profile_id) if profile_id is not None else None, + original_url=url, + ) + + +def canonical_url(ref: ProviderResourceRef) -> str: + """Build a stable dedupe key for a MakerWorld resource. + + Dedupe is keyed per *plate* (profile) rather than per model, since the + download returns a specific plate — not the full multi-plate zip — so two + different plates of the same design should become two separate library + entries. Canonical shape uses the locale-free path with the + ``#profileId-`` fragment so all URL variants of the same plate still + collapse (e.g. ``/en/models/123-slug?from=search#profileId-456`` and + ``/de/models/123#profileId-456`` both map to + ``https://makerworld.com/models/123#profileId-456``). Plate-less imports + (legacy or whole-design) keep the old model-only shape for backwards + compatibility with existing rows. + """ + if ref.sub_id: + return f"https://makerworld.com/models/{ref.external_id}#profileId-{ref.sub_id}" + return f"https://makerworld.com/models/{ref.external_id}" diff --git a/backend/app/services/model_providers/registry.py b/backend/app/services/model_providers/registry.py new file mode 100644 index 000000000..29128856e --- /dev/null +++ b/backend/app/services/model_providers/registry.py @@ -0,0 +1,60 @@ +"""Provider registry — maps ``source_type`` / URLs to model providers. + +The registry is the routing layer a future *shared* import API uses: a pasted +URL goes through :meth:`ModelProviderRegistry.find_for_url`, which asks each +registered provider ``supports_url`` and returns the one that owns it. Today +the route layer still calls the MakerWorld provider directly (endpoints stay +at ``/makerworld/*``), but registering providers here keeps the seam ready. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from backend.app.services.model_providers.base import ModelProvider + + +class ModelProviderRegistry: + """Holds the registered :class:`ModelProvider` instances. + + Registering is idempotent per provider instance; registering a *different* + provider under an already-taken ``source_type`` is an error. + """ + + def __init__(self) -> None: + self._providers: dict[str, ModelProvider] = {} + + def register(self, provider: ModelProvider) -> None: + existing = self._providers.get(provider.source_type) + if existing is not None and existing is not provider: + raise ValueError(f"A model provider for source_type {provider.source_type!r} is already registered") + self._providers[provider.source_type] = provider + + def get(self, source_type: str) -> ModelProvider: + try: + return self._providers[source_type] + except KeyError as exc: + raise KeyError(f"No model provider registered for source_type {source_type!r}") from exc + + def all(self) -> tuple[ModelProvider, ...]: + return tuple(self._providers.values()) + + def find_for_url(self, url: str) -> ModelProvider | None: + """Return the provider that claims ``url``, or ``None`` if none do. + + Iterates in registration (dict insertion) order; when more than one + provider ``supports_url`` the *first registered* one wins. Providers + overlap rarely (``host_patterns`` are usually disjoint), so this + tie-break is documented rather than policed — a "generic" provider + must register after the specific ones it might shadow. + """ + for provider in self._providers.values(): + if provider.supports_url(url): + return provider + return None + + +# App-wide registry. Providers register themselves on package import (see +# ``backend/app/services/model_providers/__init__.py``). +registry = ModelProviderRegistry() diff --git a/backend/app/services/preset_resolver.py b/backend/app/services/preset_resolver.py index 944090f42..dd8607730 100644 --- a/backend/app/services/preset_resolver.py +++ b/backend/app/services/preset_resolver.py @@ -29,7 +29,6 @@ import logging from fastapi import HTTPException from sqlalchemy.ext.asyncio import AsyncSession -from backend.app.api.routes.cloud import get_stored_token from backend.app.api.routes.orca_cloud import _build_authenticated_service as _build_orca_service from backend.app.core.permissions import Permission from backend.app.models.local_preset import LocalPreset @@ -40,6 +39,7 @@ from backend.app.services.bambu_cloud import ( BambuCloudError, BambuCloudService, ) +from backend.app.services.bambu_cloud_credentials import get_stored_token from backend.app.services.orca_cloud import OrcaCloudAuthError, OrcaCloudError logger = logging.getLogger(__name__) diff --git a/backend/tests/integration/test_cloud_auth.py b/backend/tests/integration/test_cloud_auth.py index b9c0c5c22..ade088f8b 100644 --- a/backend/tests/integration/test_cloud_auth.py +++ b/backend/tests/integration/test_cloud_auth.py @@ -265,7 +265,7 @@ class TestCloudTokenStorage: @pytest.mark.asyncio async def test_get_stored_token_returns_none_when_no_user_no_global(self, db_session): """get_stored_token with user=None and no global token returns (None, None).""" - from backend.app.api.routes.cloud import get_stored_token + from backend.app.services.bambu_cloud_credentials import get_stored_token token, email, region = await get_stored_token(db_session, user=None) assert token is None @@ -275,7 +275,8 @@ class TestCloudTokenStorage: @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 + from backend.app.api.routes.cloud import store_token + from backend.app.services.bambu_cloud_credentials import get_stored_token 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) @@ -286,9 +287,10 @@ class TestCloudTokenStorage: @pytest.mark.asyncio async def test_store_and_get_per_user_token(self, db_session): """store_token with user stores on the user record.""" - from backend.app.api.routes.cloud import get_stored_token, store_token + from backend.app.api.routes.cloud import store_token from backend.app.core.auth import get_password_hash from backend.app.models.user import User + from backend.app.services.bambu_cloud_credentials import get_stored_token user = User(username="tokentest", password_hash=get_password_hash("pass"), role="user") db_session.add(user) @@ -309,9 +311,10 @@ class TestCloudTokenStorage: @pytest.mark.asyncio async def test_per_user_token_does_not_affect_global(self, db_session): """Storing per-user token should not affect global Settings.""" - from backend.app.api.routes.cloud import get_stored_token, store_token + from backend.app.api.routes.cloud import store_token from backend.app.core.auth import get_password_hash from backend.app.models.user import User + from backend.app.services.bambu_cloud_credentials import get_stored_token user = User(username="isolationtest", password_hash=get_password_hash("pass"), role="user") db_session.add(user) @@ -352,7 +355,8 @@ class TestCloudTokenStorage: @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 + from backend.app.api.routes.cloud import clear_token, store_token + from backend.app.services.bambu_cloud_credentials import get_stored_token await store_token(db_session, "global-token", "global@test.com", "global", user=None) await clear_token(db_session, user=None) @@ -365,9 +369,10 @@ class TestCloudTokenStorage: @pytest.mark.asyncio async def test_two_users_independent_tokens(self, db_session): """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.api.routes.cloud import store_token from backend.app.core.auth import get_password_hash from backend.app.models.user import User + from backend.app.services.bambu_cloud_credentials import get_stored_token user_a = User(username="user_a", password_hash=get_password_hash("pass"), role="user") user_b = User(username="user_b", password_hash=get_password_hash("pass"), role="user") @@ -406,9 +411,10 @@ class TestCloudRegionPersistence: @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.api.routes.cloud import store_token from backend.app.core.auth import get_password_hash from backend.app.models.user import User + from backend.app.services.bambu_cloud_credentials import get_stored_token user = User(username="region-user", password_hash=get_password_hash("pass"), role="user") db_session.add(user) @@ -429,7 +435,8 @@ class TestCloudRegionPersistence: @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 + from backend.app.api.routes.cloud import store_token + from backend.app.services.bambu_cloud_credentials import get_stored_token await store_token(db_session, "cn-token", "token-auth", "china", user=None) _token, _email, region = await get_stored_token(db_session, user=None) @@ -438,7 +445,8 @@ class TestCloudRegionPersistence: @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 + from backend.app.api.routes.cloud import store_token + from backend.app.services.bambu_cloud_credentials import get_stored_token await store_token(db_session, "t", "x@test.com", "mars", user=None) _token, _email, region = await get_stored_token(db_session, user=None) diff --git a/backend/tests/integration/test_cloud_token_auth_migration.py b/backend/tests/integration/test_cloud_token_auth_migration.py index c1e97d769..162a9916c 100644 --- a/backend/tests/integration/test_cloud_token_auth_migration.py +++ b/backend/tests/integration/test_cloud_token_auth_migration.py @@ -20,16 +20,16 @@ from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession from backend.app.api.routes import cloud as cloud_routes -from backend.app.api.routes.cloud import ( +from backend.app.core.auth import get_password_hash +from backend.app.models.settings import Settings +from backend.app.models.user import User +from backend.app.services.bambu_cloud import BambuCloudError +from backend.app.services.bambu_cloud_credentials import ( CLOUD_EMAIL_KEY, CLOUD_REGION_KEY, CLOUD_TOKEN_KEY, get_stored_token, ) -from backend.app.core.auth import get_password_hash -from backend.app.models.settings import Settings -from backend.app.models.user import User -from backend.app.services.bambu_cloud import BambuCloudError async def _seed_global_token(db: AsyncSession, token: str = "tok-global", region: str = "china") -> None: diff --git a/backend/tests/integration/test_makerworld_apikey_auth.py b/backend/tests/integration/test_makerworld_apikey_auth.py index a1f216f1e..0b1da7915 100644 --- a/backend/tests/integration/test_makerworld_apikey_auth.py +++ b/backend/tests/integration/test_makerworld_apikey_auth.py @@ -29,6 +29,12 @@ from backend.app.core.auth import generate_api_key from backend.app.models.api_key import APIKey from backend.app.models.library import LibraryFile from backend.app.models.user import User +from backend.app.services.model_providers.base import ( + ProviderDownload, + ProviderDownloadInfo, + ProviderResolvedModel, + ProviderResourceRef, +) async def _setup_auth_with_admin(client: AsyncClient) -> str: @@ -104,6 +110,20 @@ def _fake_service(**stubs): return svc +def _download_info( + model_id: int = 1400373, + profile_id: int = 298919107, + name: str = "cube.3mf", +) -> ProviderDownloadInfo: + """What ``service.get_download`` hands the route — signed URL + raw + upstream name + enriched ref (sub_id carries the resolved profile).""" + return ProviderDownloadInfo( + ref=ProviderResourceRef(source_type="makerworld", external_id=str(model_id), sub_id=str(profile_id)), + url="https://makerworld.bblmw.com/makerworld/model/X/Y/cube.3mf?exp=1&key=k", + suggested_filename=name, + ) + + class TestStatusEndpoint: @pytest.mark.asyncio @pytest.mark.integration @@ -161,9 +181,13 @@ class TestResolveEndpoint: admin = await _store_admin_cloud_token(db_session, "mwadmin", token="fake-bambu-token") key = await _make_key(db_session, owner=admin, name="resolve-cloud") - design = {"id": 1400373, "modelId": "US2bb73b106683e5", "title": "Cube", "instances": []} - instances = {"total": 0, "hits": []} - svc = _fake_service(get_design=design, get_design_instances=instances) + svc = _fake_service( + resolve=ProviderResolvedModel( + ref=ProviderResourceRef(source_type="makerworld", external_id="1400373"), + design={"id": 1400373, "title": "Cube"}, + instances=[], + ) + ) build = AsyncMock(return_value=svc) with patch("backend.app.api.routes.makerworld._build_service", build): @@ -173,14 +197,22 @@ class TestResolveEndpoint: headers={"X-API-Key": key}, ) assert resp.status_code == 200, resp.text - # _build_service receives (db, user); the user arg must be the owning admin. - # Without the fix it'd be None (the API-key dep value). + # _build_service receives (db, provider, current_user, api_key_cloud_owner). + # Identity resolution lives in the provider now: for an API-keyed + # call current_user is None by design and the key's owner must arrive + # via api_key_cloud_owner — without the fix it'd be dropped entirely. assert build.await_count == 1 - passed_user = ( - build.await_args.args[1] if len(build.await_args.args) > 1 else build.await_args.kwargs.get("user") + jwt_user = ( + build.await_args.args[2] if len(build.await_args.args) > 2 else build.await_args.kwargs.get("current_user") ) - assert passed_user is not None, "resolve_url must pass the API-key owner, not None" - assert passed_user.id == admin.id + key_owner = ( + build.await_args.args[3] + if len(build.await_args.args) > 3 + else build.await_args.kwargs.get("api_key_cloud_owner") + ) + assert jwt_user is None, "API-keyed callers present no JWT user" + assert key_owner is not None, "resolve_url must forward the API-key owner to the provider" + assert key_owner.id == admin.id class TestImportEndpoint: @@ -195,23 +227,12 @@ class TestImportEndpoint: admin = await _store_admin_cloud_token(db_session, "mwadmin", token="fake-bambu-token") key = await _make_key(db_session, owner=admin, name="import-cloud") - design = { - "id": 1400373, - "modelId": "US2bb73b106683e5", - "title": "Cube", - "instances": [{"profileId": 298919107, "title": "default"}], - } - manifest = { - "name": "cube.3mf", - "url": "https://makerworld.bblmw.com/makerworld/model/X/Y/cube.3mf?exp=1&key=k", - } - # 3MF download returns (bytes, filename). The bytes don't have to be a - # valid zip — save_3mf_bytes_to_library stores them as-is and the - # downstream thumbnail extractor swallows errors. + # The 3MF bytes don't have to be a valid zip — + # save_3mf_bytes_to_library stores them as-is and the downstream + # thumbnail extractor swallows errors. svc = _fake_service( - get_design=design, - get_profile_download=manifest, - download_3mf=(b"PK\x03\x04fake-3mf-bytes", "cube.3mf"), + get_download=_download_info(), + download=ProviderDownload(file_bytes=b"PK\x03\x04fake-3mf-bytes", filename="cube.3mf"), ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): @@ -249,16 +270,9 @@ class TestImportEndpoint: admin = await _store_admin_cloud_token(db_session, "mwadmin", token="fake-bambu-token") key = await _make_key(db_session, owner=admin, name="import-no-cloud", can_access_cloud=False) - design = { - "id": 1400373, - "modelId": "US2bb73b106683e5", - "instances": [{"profileId": 298919107}], - } - manifest = {"name": "cube.3mf", "url": "https://makerworld.bblmw.com/x.3mf"} svc = _fake_service( - get_design=design, - get_profile_download=manifest, - download_3mf=(b"PK\x03\x04fake", "cube.3mf"), + get_download=_download_info(), + download=ProviderDownload(file_bytes=b"PK\x03\x04fake", filename="cube.3mf"), ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)) as build: @@ -270,11 +284,19 @@ class TestImportEndpoint: assert resp.status_code == 200, resp.text body = resp.json() - # _build_service got None — same as before the PR for non-cloud keys. - passed_user = ( - build.await_args.args[1] if len(build.await_args.args) > 1 else build.await_args.kwargs.get("user") + # Both identity slots are None — same as before the PR for non-cloud + # keys: no JWT user, and the cloud-scope fence keeps the key's owner + # back, so the provider builds an anonymous service. + jwt_user = ( + build.await_args.args[2] if len(build.await_args.args) > 2 else build.await_args.kwargs.get("current_user") ) - assert passed_user is None + key_owner = ( + build.await_args.args[3] + if len(build.await_args.args) > 3 + else build.await_args.kwargs.get("api_key_cloud_owner") + ) + assert jwt_user is None + assert key_owner is None # And owner_id is NULL because the cloud-scope fence said no. result = await db_session.execute(select(LibraryFile).where(LibraryFile.id == body["library_file_id"])) diff --git a/backend/tests/integration/test_makerworld_permission_gate.py b/backend/tests/integration/test_makerworld_permission_gate.py new file mode 100644 index 000000000..b1af5c535 --- /dev/null +++ b/backend/tests/integration/test_makerworld_permission_gate.py @@ -0,0 +1,180 @@ +"""The /makerworld/* permission gate with auth enabled. + +The gate moved out of the route signature and into the handler: the provider +that a request actually uses comes from the body (``source_type`` on import, +the pasted URL on resolve), and FastAPI resolves dependencies before the body +exists, so a dependency could only ever name one provider's permission. What +must not change is the enforcement itself, so these pin the outcomes rather +than the wiring: anonymous callers are still refused before the body is read, +and a signed-in user without the permission still gets a 403. +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, patch + +import pytest +from httpx import AsyncClient + +from backend.app.services.model_providers.base import ( + ProviderDownload, + ProviderDownloadInfo, + ProviderResolvedModel, + ProviderResourceRef, +) + + +async def _setup_auth_with_admin(client: AsyncClient) -> str: + await client.post( + "/api/v1/auth/setup", + json={"auth_enabled": True, "admin_username": "mwadmin", "admin_password": "AdminPass1!"}, + ) + login = await client.post("/api/v1/auth/login", json={"username": "mwadmin", "password": "AdminPass1!"}) + assert login.status_code == 200, login.text + return login.json()["access_token"] + + +async def _make_user(client: AsyncClient, admin_jwt: str, *, username: str, permissions: list[str]) -> str: + """Create a user in a fresh group holding exactly *permissions*.""" + group = await client.post( + "/api/v1/groups/", + headers={"Authorization": f"Bearer {admin_jwt}"}, + json={"name": f"grp_{username}", "permissions": permissions}, + ) + assert group.status_code in (200, 201), group.text + created = await client.post( + "/api/v1/users/", + headers={"Authorization": f"Bearer {admin_jwt}"}, + json={"username": username, "password": "UserPass1!", "group_ids": [group.json()["id"]]}, + ) + assert created.status_code in (200, 201), created.text + login = await client.post("/api/v1/auth/login", json={"username": username, "password": "UserPass1!"}) + assert login.status_code == 200, login.text + return login.json()["access_token"] + + +def _fake_service(**stubs): + svc = AsyncMock() + svc.close = AsyncMock() + for name, value in stubs.items(): + setattr(svc, name, AsyncMock(return_value=value)) + return svc + + +def _import_service(): + return _fake_service( + get_download=ProviderDownloadInfo( + ref=ProviderResourceRef(source_type="makerworld", external_id="1400373", sub_id="298919107"), + url="https://makerworld.bblmw.com/makerworld/model/X/Y/cube.3mf?exp=1&key=k", + suggested_filename="cube.3mf", + ), + download=ProviderDownload(file_bytes=b"PK\x03\x04fake-3mf-bytes", filename="cube.3mf"), + ) + + +class TestAnonymousIsRefusedFirst: + @pytest.mark.asyncio + @pytest.mark.integration + async def test_anonymous_import_is_401(self, async_client: AsyncClient): + await _setup_auth_with_admin(async_client) + resp = await async_client.post("/api/v1/makerworld/import", json={"model_id": 1400373}) + assert resp.status_code == 401, resp.text + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_anonymous_resolve_is_401(self, async_client: AsyncClient): + await _setup_auth_with_admin(async_client) + resp = await async_client.post( + "/api/v1/makerworld/resolve", + json={"url": "https://makerworld.com/en/models/1400373"}, + ) + assert resp.status_code == 401, resp.text + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_anonymous_with_a_malformed_body_is_still_401_not_422(self, async_client: AsyncClient): + """The permission moved into the handler, but authentication stayed a + route dependency precisely so an unauthenticated caller cannot probe + the request schema through validation errors.""" + await _setup_auth_with_admin(async_client) + resp = await async_client.post("/api/v1/makerworld/import", json={"nonsense": True}) + assert resp.status_code == 401, resp.text + + +class TestPermissionStillBites: + @pytest.mark.asyncio + @pytest.mark.integration + async def test_view_only_user_cannot_import(self, async_client: AsyncClient): + admin = await _setup_auth_with_admin(async_client) + jwt = await _make_user(async_client, admin, username="mwviewer", permissions=["makerworld:view"]) + + with patch( + "backend.app.api.routes.makerworld._build_service", + AsyncMock(return_value=_import_service()), + ): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373}, + headers={"Authorization": f"Bearer {jwt}"}, + ) + assert resp.status_code == 403, resp.text + assert "makerworld:import" in resp.json()["detail"] + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_user_without_view_cannot_resolve(self, async_client: AsyncClient): + admin = await _setup_auth_with_admin(async_client) + jwt = await _make_user(async_client, admin, username="mwnoview", permissions=["printers:read"]) + + resp = await async_client.post( + "/api/v1/makerworld/resolve", + json={"url": "https://makerworld.com/en/models/1400373"}, + headers={"Authorization": f"Bearer {jwt}"}, + ) + assert resp.status_code == 403, resp.text + assert "makerworld:view" in resp.json()["detail"] + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_user_holding_the_permission_gets_through(self, async_client: AsyncClient): + admin = await _setup_auth_with_admin(async_client) + jwt = await _make_user( + async_client, + admin, + username="mwimporter", + permissions=["makerworld:view", "makerworld:import"], + ) + + with patch( + "backend.app.api.routes.makerworld._build_service", + AsyncMock(return_value=_import_service()), + ): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373}, + headers={"Authorization": f"Bearer {jwt}"}, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["was_existing"] is False + + @pytest.mark.asyncio + @pytest.mark.integration + async def test_resolve_passes_for_a_viewer(self, async_client: AsyncClient): + admin = await _setup_auth_with_admin(async_client) + jwt = await _make_user(async_client, admin, username="mwviewer2", permissions=["makerworld:view"]) + + svc = _fake_service( + resolve=ProviderResolvedModel( + ref=ProviderResourceRef(source_type="makerworld", external_id="1400373"), + design={"id": 1400373}, + instances=[], + ) + ) + with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): + resp = await async_client.post( + "/api/v1/makerworld/resolve", + json={"url": "https://makerworld.com/en/models/1400373"}, + headers={"Authorization": f"Bearer {jwt}"}, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["model_id"] == 1400373 diff --git a/backend/tests/unit/services/test_bambu_cloud_credentials.py b/backend/tests/unit/services/test_bambu_cloud_credentials.py new file mode 100644 index 000000000..ece1f2f46 --- /dev/null +++ b/backend/tests/unit/services/test_bambu_cloud_credentials.py @@ -0,0 +1,106 @@ +"""Tests for ``services/bambu_cloud_credentials`` — the credential seam. + +The read paths are covered indirectly by the cloud-token expiry and +migration suites; these pin the write path that review blocker 5 hinged on: +``mark_cloud_token_invalid`` must record a rejection for *both* identity +shapes, because auth-disabled single-user installs (the default) hold their +token in global ``Settings`` — ``user_id=None`` is a real, expected input, +not a degenerate one. +""" + +from __future__ import annotations + +from datetime import datetime + +import pytest +from sqlalchemy import select + +from backend.app.core.auth import get_password_hash +from backend.app.models.settings import Settings +from backend.app.models.user import User +from backend.app.services import bambu_cloud_credentials as creds +from backend.app.services.bambu_cloud_credentials import ( + CLOUD_TOKEN_INVALID_KEY, + mark_cloud_token_invalid, +) + +pytestmark = pytest.mark.asyncio + + +class _SharedSessionCtx: + """Route ``mark`` through the fixture's in-memory session: the function + normally opens its own session against the configured database, which in + tests is a different SQLite than ``db_session``'s in-memory one.""" + + def __init__(self, session): + self._session = session + + async def __aenter__(self): + return self._session + + async def __aexit__(self, *exc): + return False + + +@pytest.fixture(autouse=True) +def shared_session(db_session, monkeypatch): + monkeypatch.setattr(creds, "async_session", lambda: _SharedSessionCtx(db_session)) + + +async def _make_user(db, username: str = "cred-user") -> User: + user = User( + username=username, + password_hash=get_password_hash("AdminPass1!"), + role="admin", + is_active=True, + ) + db.add(user) + await db.commit() + await db.refresh(user) + return user + + +async def test_mark_sets_the_per_user_flag(db_session): + """user_id set → the rejection lands on that user's column.""" + user = await _make_user(db_session) + + await mark_cloud_token_invalid(user.id) + await db_session.refresh(user) + + assert user.cloud_token_invalid_at is not None + + +async def test_mark_none_writes_the_global_settings_flag(db_session): + """user_id=None (auth-disabled install) → the global ``Settings`` row. + A second call updates the existing row rather than adding another.""" + await mark_cloud_token_invalid(None) + + result = await db_session.execute(select(Settings).where(Settings.key == CLOUD_TOKEN_INVALID_KEY)) + rows = result.scalars().all() + assert len(rows) == 1 + # Stored value parses as ISO — the status endpoints compare it as a date. + datetime.fromisoformat(rows[0].value) + + first_value = rows[0].value + await mark_cloud_token_invalid(None) + result = await db_session.execute(select(Settings).where(Settings.key == CLOUD_TOKEN_INVALID_KEY)) + rows = result.scalars().all() + assert len(rows) == 1 + assert rows[0].value >= first_value + + +async def test_mark_is_best_effort(db_session, monkeypatch): + """A bookkeeping failure must never replace the 401 the caller needs to + see — the function swallows everything.""" + + class _Boom: + async def __aenter__(self): + raise RuntimeError("db gone") + + async def __aexit__(self, *exc): + return False + + monkeypatch.setattr(creds, "async_session", lambda: _Boom()) + + # Must not raise. + await mark_cloud_token_invalid(None) diff --git a/backend/tests/unit/services/test_makerworld.py b/backend/tests/unit/services/test_makerworld.py index 55ac33cf1..09256d7e8 100644 --- a/backend/tests/unit/services/test_makerworld.py +++ b/backend/tests/unit/services/test_makerworld.py @@ -8,55 +8,55 @@ from urllib.error import HTTPError, URLError import httpx import pytest -from backend.app.services.makerworld import ( - _MAX_3MF_BYTES, - MAKERWORLD_API_BASE, +from backend.app.services.model_providers.base import ProviderResourceRef +from backend.app.services.model_providers.makerworld.errors import ( MakerWorldAuthError, MakerWorldForbiddenError, MakerWorldNotFoundError, - MakerWorldService, MakerWorldUnavailableError, MakerWorldUrlError, ) +from backend.app.services.model_providers.makerworld.http import _MAX_3MF_BYTES, MAKERWORLD_API_BASE +from backend.app.services.model_providers.makerworld.service import MakerWorldService, set_shared_http_client +from backend.app.services.model_providers.makerworld.url import parse_url class TestParseUrl: - """MakerWorld URL extraction.""" + """MakerWorld URL extraction — tests parse_url directly.""" def test_strips_locale_prefix_and_slug(self): - model, profile = MakerWorldService.parse_url( - "https://makerworld.com/en/models/1400373-self-watering-seed-starter" - ) - assert model == 1400373 - assert profile is None + ref = parse_url("https://makerworld.com/en/models/1400373-self-watering-seed-starter") + assert ref.external_id == "1400373" + assert ref.sub_id is None def test_extracts_profile_id_from_fragment(self): - model, profile = MakerWorldService.parse_url("https://makerworld.com/en/models/1400373-slug#profileId-1452154") - assert model == 1400373 - assert profile == 1452154 + ref = parse_url("https://makerworld.com/en/models/1400373-slug#profileId-1452154") + assert ref.external_id == "1400373" + assert ref.sub_id == "1452154" def test_accepts_scheme_omitted(self): - model, profile = MakerWorldService.parse_url("makerworld.com/models/999") - assert model == 999 - assert profile is None + ref = parse_url("makerworld.com/models/999") + assert ref.external_id == "999" + assert ref.sub_id is None def test_accepts_subdomain(self): # Defensive: if MakerWorld ever stands up a regional subdomain, still accept it - model, _ = MakerWorldService.parse_url("https://www.makerworld.com/en/models/42") - assert model == 42 + ref = parse_url("https://www.makerworld.com/en/models/42") + assert ref.external_id == "42" + assert ref.sub_id is None def test_rejects_non_makerworld_host(self): with pytest.raises(MakerWorldUrlError): - MakerWorldService.parse_url("https://thingiverse.com/things/123") + parse_url("https://thingiverse.com/things/123") def test_rejects_malformed_url(self): # No /models/ segment anywhere in path with pytest.raises(MakerWorldUrlError): - MakerWorldService.parse_url("https://makerworld.com/en/creators/foo") + parse_url("https://makerworld.com/en/creators/foo") def test_rejects_empty(self): with pytest.raises(MakerWorldUrlError): - MakerWorldService.parse_url("") + parse_url("") class TestApiBase: @@ -269,6 +269,195 @@ class TestGetDesign: await service.get_design(1) +class TestResolve: + """``resolve`` — the interface-level "URL → metadata + plate list" flow the + /makerworld/resolve route drives. The per-instance printer-compatibility + merge lives here (not in the route) so every future provider gets it from + its own ``resolve`` implementation.""" + + @pytest.fixture + def service(self): + return MakerWorldService(client=MagicMock(spec=httpx.AsyncClient)) + + @pytest.mark.asyncio + async def test_merges_compatibility_from_design_into_instances(self, service): + """Per-instance printer compatibility info lives on + ``design.instances[].extention.modelInfo`` but not on + ``/instances/hits``. Resolve enriches each hit with both + ``compatibility`` (primary printer the instance was sliced for) and + ``otherCompatibility`` (extra printers the uploader marked it + compatible with) so the frontend can show "sliced for A1 / also + marked compatible with: H2D, P1S". + """ + design_payload = { + "id": 1400373, + "title": "Seed Starter", + "instances": [ + { + "id": 1452154, + "extention": { + "modelInfo": { + "compatibility": ["A1"], + "otherCompatibility": ["H2D", "P1S"], + } + }, + }, + { + "id": 1452158, + "extention": { + "modelInfo": { + "compatibility": ["X1 Carbon"], + "otherCompatibility": [], + } + }, + }, + ], + } + instances_payload = { + "total": 2, + "hits": [ + {"id": 1452154, "profileId": 298919107, "title": "9 cells"}, + {"id": 1452158, "profileId": 298919564, "title": "12 cells"}, + ], + } + service.get_design = AsyncMock(return_value=design_payload) + service.get_design_instances = AsyncMock(return_value=instances_payload) + + resolved = await service.resolve(ProviderResourceRef(source_type="makerworld", external_id="1400373")) + by_id = {i["id"]: i for i in resolved.instances} + assert by_id[1452154]["compatibility"] == ["A1"] + assert by_id[1452154]["otherCompatibility"] == ["H2D", "P1S"] + assert by_id[1452158]["compatibility"] == ["X1 Carbon"] + assert by_id[1452158]["otherCompatibility"] == [] + assert resolved.design == design_payload + + @pytest.mark.asyncio + async def test_handles_missing_compatibility_gracefully(self, service): + """Older designs (or hits without a matching design.instances entry) + must not crash resolve — they just don't get the compat fields.""" + design_payload = {"id": 1400373, "instances": [{"id": 1452154}]} # no extention + instances_payload = { + "total": 2, + "hits": [ + {"id": 1452154, "profileId": 298919107}, + {"id": 9999999, "profileId": 298919999}, # no design.instances match + ], + } + service.get_design = AsyncMock(return_value=design_payload) + service.get_design_instances = AsyncMock(return_value=instances_payload) + + resolved = await service.resolve(ProviderResourceRef(source_type="makerworld", external_id="1400373")) + # First instance: design entry exists but no extention → fields absent or None. + first = next(i for i in resolved.instances if i["id"] == 1452154) + assert first.get("compatibility") is None + assert first.get("otherCompatibility") is None + # Second instance: no design entry at all → no enrichment, no crash. + second = next(i for i in resolved.instances if i["id"] == 9999999) + assert "compatibility" not in second or second["compatibility"] is None + + @pytest.mark.asyncio + async def test_normalises_null_and_non_list_hits_to_empty(self, service): + service.get_design = AsyncMock(return_value={"id": 1400373}) + service.get_design_instances = AsyncMock(return_value={"total": 0, "hits": None}) + + resolved = await service.resolve(ProviderResourceRef(source_type="makerworld", external_id="1400373")) + assert resolved.instances == [] + + +class TestGetDownload: + """``get_download`` — the interface-level "resource → signed 3MF URL" + flow the /makerworld/import route drives. The provider-specific dance + lives here: the iot-service endpoint needs the *alphanumeric* modelId + (not the integer design id), the profile falls back in two tiers, and + three malformed-upstream shapes must map to UnavailableError (502).""" + + @pytest.fixture + def service(self): + return MakerWorldService(client=MagicMock(spec=httpx.AsyncClient)) + + def _design(self, **overrides): + design = { + "id": 1400373, + "modelId": "US2bb73b106683e5", + "instances": [{"profileId": 298919107, "title": "9 cells"}], + } + design.update(overrides) + return design + + def _manifest(self, url="https://makerworld.bblmw.com/x.3mf?exp=1", name="benchy.3mf"): + return {"url": url, "name": name} + + async def _run(self, service, ref): + return await service.get_download(ref) + + @pytest.mark.asyncio + async def test_resolves_alphanumeric_model_id_and_explicit_profile(self, service): + """Explicit profile_id flows through; get_profile_download receives + the alphanumeric modelId from the design, not the integer id.""" + service.get_design = AsyncMock(return_value=self._design()) + manifest = self._manifest() + service.get_profile_download = AsyncMock(return_value=manifest) + + info = await self._run( + service, ProviderResourceRef(source_type="makerworld", external_id="1400373", sub_id="298919107") + ) + + service.get_profile_download.assert_awaited_once_with(298919107, "US2bb73b106683e5") + assert info.url == manifest["url"] + assert info.suggested_filename == "benchy.3mf" + # The enriched ref carries the resolved profile for the dedupe key. + assert info.ref.sub_id == "298919107" + + @pytest.mark.asyncio + async def test_falls_back_to_first_design_instance_profile(self, service): + """No profile given → first ``design.instances[].profileId`` wins.""" + service.get_design = AsyncMock(return_value=self._design()) + service.get_profile_download = AsyncMock(return_value=self._manifest()) + + info = await self._run(service, ProviderResourceRef(source_type="makerworld", external_id="1400373")) + + service.get_profile_download.assert_awaited_once_with(298919107, "US2bb73b106683e5") + assert info.ref.sub_id == "298919107" + + @pytest.mark.asyncio + async def test_second_tier_falls_back_to_instances_envelope(self, service): + """Design carries no usable profileId → the ``/design/{id}/instances`` + envelope is consulted before giving up.""" + service.get_design = AsyncMock(return_value=self._design(instances=[{"title": "no profileId here"}])) + service.get_design_instances = AsyncMock(return_value={"total": 1, "hits": [{"profileId": 298919564}]}) + service.get_profile_download = AsyncMock(return_value=self._manifest()) + + info = await self._run(service, ProviderResourceRef(source_type="makerworld", external_id="1400373")) + + service.get_design_instances.assert_awaited_once_with(1400373) + service.get_profile_download.assert_awaited_once_with(298919564, "US2bb73b106683e5") + assert info.ref.sub_id == "298919564" + + @pytest.mark.asyncio + async def test_missing_alphanumeric_model_id_is_unavailable(self, service): + """A design without the ``modelId`` field can't reach iot-service.""" + service.get_design = AsyncMock(return_value={"id": 1400373}) + + with pytest.raises(MakerWorldUnavailableError, match="modelId"): + await self._run(service, ProviderResourceRef(source_type="makerworld", external_id="1400373")) + + @pytest.mark.asyncio + async def test_no_profiles_anywhere_is_unavailable(self, service): + service.get_design = AsyncMock(return_value=self._design(instances=[])) + service.get_design_instances = AsyncMock(return_value={"total": 0, "hits": []}) + + with pytest.raises(MakerWorldUnavailableError, match="no instances"): + await self._run(service, ProviderResourceRef(source_type="makerworld", external_id="1400373")) + + @pytest.mark.asyncio + async def test_manifest_without_url_is_unavailable(self, service): + service.get_design = AsyncMock(return_value=self._design()) + service.get_profile_download = AsyncMock(return_value={"name": "benchy.3mf"}) + + with pytest.raises(MakerWorldUnavailableError, match="download URL"): + await self._run(service, ProviderResourceRef(source_type="makerworld", external_id="1400373")) + + class TestGetProfileDownload: """The new auth-gated 3MF manifest endpoint on the Bambu iot-service. @@ -397,11 +586,52 @@ class TestDownload3MF: with pytest.raises(MakerWorldUrlError): await svc.download_3mf(url) + @pytest.mark.asyncio + async def test_download_allowlist_is_driven_by_injected_hosts(self): + """``download_hosts`` is the SSRF seam, not a hardcoded constant (review + round 3 note 2). ``build_service`` passes ``ModelProvider.download_hosts()`` + into ``MakerWorldService`` — prove the injection is live by accepting a + host inside a custom allowlist and refusing a MakerWorld CDN host that + isn't in it.""" + svc = MakerWorldService(client=MagicMock(spec=httpx.AsyncClient), download_hosts=("cdn.example.com",)) + + resp = MagicMock() + resp.status_code = 200 + + async def _chunks(): + yield b"PK\x03\x04" + + resp.aiter_bytes = lambda: _chunks() + svc._client.stream = MagicMock(return_value=self._stream_ctx(resp)) + + payload, _ = await svc.download_3mf("https://cdn.example.com/m/foo.3mf?exp=1&key=k") + assert payload == b"PK\x03\x04" + + with pytest.raises(MakerWorldUrlError): + await svc.download_3mf("https://makerworld.bblmw.com/makerworld/model/X/Y/foo.3mf?exp=1&key=k") + + @pytest.mark.asyncio + async def test_s3_suffix_family_is_allowed_regardless_of_injected_hosts(self): + """``_ALLOWED_DOWNLOAD_SUFFIXES`` is deliberately outside the + ``download_hosts()`` seam: Bambu's presigned S3 endpoints are this + provider's own signed-URL family, not an exact-host allowlist a + provider declares. Pinned so narrowing the injected hosts can never + silently take the S3 download path with it.""" + svc = MakerWorldService(client=MagicMock(spec=httpx.AsyncClient), download_hosts=("cdn.example.com",)) + with patch( + "backend.app.services.model_providers.makerworld.service._download_s3_urllib", + AsyncMock(return_value=(b"PK\x03\x04", "plate.3mf")), + ) as s3: + payload, name = await svc.download_3mf("https://s3.us-west-2.amazonaws.com/bucket/plate.3mf?sig=1") + assert payload == b"PK\x03\x04" + assert name == "plate.3mf" + assert s3.await_count == 1 + @pytest.mark.asyncio async def test_s3_host_delegates_to_urllib_path(self): svc = MakerWorldService(client=MagicMock(spec=httpx.AsyncClient)) with patch( - "backend.app.services.makerworld._download_s3_urllib", + "backend.app.services.model_providers.makerworld.service._download_s3_urllib", new=AsyncMock(return_value=(b"payload", "file.3mf")), ) as mocked: payload, filename = await svc.download_3mf( @@ -499,7 +729,7 @@ class TestS3UrllibDownload: @pytest.mark.asyncio async def test_returns_bytes_and_filename(self): - from backend.app.services.makerworld import _download_s3_urllib + from backend.app.services.model_providers.makerworld.http import _download_s3_urllib fake_resp = MagicMock() fake_resp.status = 200 @@ -524,7 +754,7 @@ class TestS3UrllibDownload: """The ``_NoRedirect`` handler returns ``None`` from ``redirect_request``, which makes ``urllib`` raise ``HTTPError`` instead of following. The wrapper must surface that as ``MakerWorldUnavailableError``.""" - from backend.app.services.makerworld import _download_s3_urllib + from backend.app.services.model_providers.makerworld.http import _download_s3_urllib fake_opener = MagicMock() fake_opener.open = MagicMock( @@ -548,7 +778,7 @@ class TestS3UrllibDownload: @pytest.mark.asyncio async def test_non_200_raises_unavailable(self): - from backend.app.services.makerworld import _download_s3_urllib + from backend.app.services.model_providers.makerworld.http import _download_s3_urllib fake_resp = MagicMock() fake_resp.status = 403 @@ -570,7 +800,7 @@ class TestS3UrllibDownload: @pytest.mark.asyncio async def test_size_cap_enforced(self): - from backend.app.services.makerworld import _download_s3_urllib + from backend.app.services.model_providers.makerworld.http import _download_s3_urllib fake_resp = MagicMock() fake_resp.status = 200 @@ -593,7 +823,7 @@ class TestS3UrllibDownload: @pytest.mark.asyncio async def test_network_error_mapped_to_unavailable(self): - from backend.app.services.makerworld import _download_s3_urllib + from backend.app.services.model_providers.makerworld.http import _download_s3_urllib fake_opener = MagicMock() fake_opener.open = MagicMock(side_effect=URLError("dns fail")) @@ -699,3 +929,33 @@ class TestFetchThumbnail: with pytest.raises(MakerWorldUnavailableError): await service.fetch_thumbnail("https://makerworld.bblmw.com/makerworld/model/X/blob") + + +class TestSharedHttpClient: + """The app-scoped httpx client registered via ``set_shared_http_client`` + must be reused by per-request services (one shared connection pool, same + pattern as ``bambu_cloud``). The setter has to live in the same module as + the service class, or the import-time snapshot never sees the lifespan's + late registration and every request spins up its own client.""" + + @pytest.mark.asyncio + async def test_reuses_registered_client(self): + client = MagicMock(spec=httpx.AsyncClient) + set_shared_http_client(client) + try: + svc = MakerWorldService() + assert svc._client is client + assert svc._owns_client is False + # close() must NOT close a client it doesn't own + await svc.close() + client.aclose.assert_not_called() + finally: + set_shared_http_client(None) + + @pytest.mark.asyncio + async def test_creates_and_owns_own_client_when_none_registered(self): + set_shared_http_client(None) + svc = MakerWorldService() + assert svc._owns_client is True + await svc.close() + assert svc._client.is_closed diff --git a/backend/tests/unit/services/test_model_provider_interface.py b/backend/tests/unit/services/test_model_provider_interface.py new file mode 100644 index 000000000..03406c0d6 --- /dev/null +++ b/backend/tests/unit/services/test_model_provider_interface.py @@ -0,0 +1,221 @@ +"""Tests for the MakerWorld provider descriptor (``provider.py``). + +The descriptor is the interface between the route layer and the per-request +service: identity, URL routing, auth requirements, and the factory that seeds +a service with the caller's stored Bambu Cloud bearer token. +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, patch + +import pytest + +from backend.app.core.permissions import Permission +from backend.app.services.model_providers import makerworld_provider +from backend.app.services.model_providers.base import ModelProvider +from backend.app.services.model_providers.makerworld.service import MakerWorldService + + +class TestMakerWorldProviderDescriptor: + def test_identity_fields(self): + assert makerworld_provider.source_type == "makerworld" + assert makerworld_provider.display_name == "MakerWorld" + assert makerworld_provider.host_patterns == ("makerworld.com",) + assert makerworld_provider.default_folder_name == "MakerWorld" + + def test_permissions(self): + assert makerworld_provider.view_permission == Permission.MAKERWORLD_VIEW + assert makerworld_provider.import_permission == Permission.MAKERWORLD_IMPORT + + def test_auth_descriptor(self): + assert makerworld_provider.auth is not None + assert makerworld_provider.auth.auth_type == "bambu_cloud_bearer" + assert makerworld_provider.auth.credential_fields == () + + def test_supports_url_host_suffix_match(self): + assert makerworld_provider.supports_url("https://makerworld.com/en/models/1400373") + assert makerworld_provider.supports_url("https://www.makerworld.com/models/1400373") + assert makerworld_provider.supports_url("makerworld.com/models/1") + + def test_rejects_foreign_hosts_and_garbage(self): + assert not makerworld_provider.supports_url("https://thingiverse.com/thing/123") + assert not makerworld_provider.supports_url("https://makerworld.com.evil.example/x") + assert not makerworld_provider.supports_url("") + assert not makerworld_provider.supports_url(None) # type: ignore[arg-type] + assert not makerworld_provider.supports_url(123) # type: ignore[arg-type] + + def test_thumbnail_hosts_is_the_cdn_allowlist(self): + assert "makerworld.bblmw.com" in makerworld_provider.thumbnail_hosts() + assert "public-cdn.bblmw.com" in makerworld_provider.thumbnail_hosts() + + def test_download_hosts_is_the_cdn_allowlist(self): + """The download-guard SSRF seam mirrors the thumbnail one — a provider + that fetches files server-side declares the hosts its service may + fetch from (review round 3, note 2).""" + assert "makerworld.bblmw.com" in makerworld_provider.download_hosts() + assert "public-cdn.bblmw.com" in makerworld_provider.download_hosts() + + def test_parse_and_canonical_roundtrip(self): + ref = makerworld_provider.parse_url("https://makerworld.com/en/models/1400373-slug#profileId-1452154") + assert ref.external_id == "1400373" + assert ref.sub_id == "1452154" + assert ref.source_type == "makerworld" + assert makerworld_provider.canonical_url(ref) == "https://makerworld.com/models/1400373#profileId-1452154" + + def test_canonical_plate_less_shape(self): + ref = makerworld_provider.parse_url("https://makerworld.com/models/999") + assert makerworld_provider.canonical_url(ref) == "https://makerworld.com/models/999" + + def test_source_url_filter_matches_model_and_any_plate(self): + """The already-imported predicate must cover the whole-model key and + every per-plate key — MakerWorld's canonical shape appends + ``#profileId-{n}`` (review round 3: shape knowledge belongs to the + provider, not the route).""" + from sqlalchemy import String, column as sa_column + + expr = makerworld_provider.source_url_filter(sa_column("source_url", String), "1400373") + sql = str(expr.compile(compile_kwargs={"literal_binds": True})) + assert "source_url = 'https://makerworld.com/models/1400373'" in sql + assert "LIKE 'https://makerworld.com/models/1400373#profileId-%'" in sql + + def test_default_source_url_filter_is_exact_match_only(self): + """A provider without plate-shaped keys inherits the default: exact + match on its whole-model canonical URL, nothing else.""" + from sqlalchemy import String, column as sa_column + + expr = _WholeModelProvider().source_url_filter(sa_column("source_url", String), "42") + sql = str(expr.compile(compile_kwargs={"literal_binds": True})) + assert sql == "source_url = 'https://example.com/models/42'" + assert "LIKE" not in sql + + +class _WholeModelProvider(ModelProvider): + """Minimal concrete provider that keys dedupe at whole-model granularity + only — exercises the inherited default ``source_url_filter``.""" + + source_type = "wholemodel" + display_name = "WholeModel" + + async def build_service(self, *, db, user, api_key_owner=None, client=None): + raise NotImplementedError + + def parse_url(self, url): + raise NotImplementedError + + def canonical_url(self, ref): + return f"https://example.com/models/{ref.external_id}" + + +class TestBuildService: + """build_service must reproduce exactly what the old route helper did: + read the caller's stored Bambu Cloud token and wire the rejected-token + callback so a 401 invalidates the shared credential app-wide.""" + + @pytest.mark.asyncio + async def test_seeds_token_and_auth_failure_callback(self): + db = AsyncMock() + user = AsyncMock() + user.id = 7 + + with ( + patch( + "backend.app.services.model_providers.makerworld.provider.get_stored_token", + AsyncMock(return_value=("tok-abc", "e@x.com", "global")), + ), + patch( + "backend.app.services.model_providers.makerworld.provider.mark_cloud_token_invalid", + AsyncMock(), + ) as mark_invalid, + ): + svc = await makerworld_provider.build_service(db=db, user=user) + assert isinstance(svc, MakerWorldService) + assert svc._auth_token == "tok-abc" + assert svc._user is user + await svc._on_auth_failure() + mark_invalid.assert_awaited_once_with(7) + await svc.close() + + @pytest.mark.asyncio + async def test_anonymous_user_still_gets_auth_callback(self): + """No user ≠ nothing to invalidate. Auth-disabled single-user installs + hold their token in global Settings (``get_stored_token(db, None)`` + reads it), so a rejection must still be recorded — ``user_id=None`` + writes the global flag (review blocker 5). The callback stays wired; + only a *stray* non-expiry 401 keeps it a no-op.""" + db = AsyncMock() + with ( + patch( + "backend.app.services.model_providers.makerworld.provider.get_stored_token", + AsyncMock(return_value=(None, None, "global")), + ), + patch( + "backend.app.services.model_providers.makerworld.provider.mark_cloud_token_invalid", + AsyncMock(), + ) as mark_invalid, + ): + svc = await makerworld_provider.build_service(db=db, user=None) + + assert svc._auth_token is None + assert svc._on_auth_failure is not None + # Firing it records the *global* flag (user_id=None), not a per-user row. + await svc._on_auth_failure() + mark_invalid.assert_awaited_once_with(None) + await svc.close() + + @pytest.mark.asyncio + async def test_build_service_passes_declared_thumbnail_hosts(self): + """The SSRF seam contract: ``fetch_thumbnail``'s allowlist must come + from ``ModelProvider.thumbnail_hosts()`` via build_service — not from + a hardcoded copy inside the service (review round 2, item 1).""" + db = AsyncMock() + with patch( + "backend.app.services.model_providers.makerworld.provider.get_stored_token", + AsyncMock(return_value=(None, None, "global")), + ): + svc = await makerworld_provider.build_service(db=db, user=None) + + assert svc._thumbnail_hosts == makerworld_provider.thumbnail_hosts() + assert len(svc._thumbnail_hosts) > 0 + await svc.close() + + @pytest.mark.asyncio + async def test_build_service_passes_declared_download_hosts(self): + """Symmetric SSRF seam contract: ``download``'s allowlist must come + from ``ModelProvider.download_hosts()`` via build_service — not from + a hardcoded copy inside the service (review round 3, note 2).""" + db = AsyncMock() + with patch( + "backend.app.services.model_providers.makerworld.provider.get_stored_token", + AsyncMock(return_value=(None, None, "global")), + ): + svc = await makerworld_provider.build_service(db=db, user=None) + + assert svc._download_hosts == makerworld_provider.download_hosts() + assert len(svc._download_hosts) > 0 + await svc.close() + + @pytest.mark.asyncio + async def test_api_key_owner_is_the_fallback_identity(self): + """API-keyed callers carry identity on the key (#1777) — build_service + must use the key's owner when ``user`` is None.""" + db = AsyncMock() + owner = AsyncMock() + owner.id = 11 + + with ( + patch( + "backend.app.services.model_providers.makerworld.provider.get_stored_token", + AsyncMock(return_value=("owner-tok", "owner@x.com", "global")), + ), + patch( + "backend.app.services.model_providers.makerworld.provider.mark_cloud_token_invalid", + AsyncMock(), + ) as mark_invalid, + ): + svc = await makerworld_provider.build_service(db=db, user=None, api_key_owner=owner) + assert svc._auth_token == "owner-tok" + assert svc._user is owner + await svc._on_auth_failure() + mark_invalid.assert_awaited_once_with(11) + await svc.close() diff --git a/backend/tests/unit/test_cloud_captcha_2790.py b/backend/tests/unit/test_cloud_captcha_2790.py index bbe5da712..0dfec0c1a 100644 --- a/backend/tests/unit/test_cloud_captcha_2790.py +++ b/backend/tests/unit/test_cloud_captcha_2790.py @@ -205,7 +205,8 @@ class TestMakerWorldSharesTheDetector: async def test_a_challenge_worded_differently_is_still_named(self): """MakerWorld used to require the literal word "robot" in the error text and reported anything else as an unexplained block.""" - from backend.app.services.makerworld import MakerWorldService, MakerWorldUnavailableError + from backend.app.services.model_providers.makerworld.errors import MakerWorldUnavailableError + from backend.app.services.model_providers.makerworld.service import MakerWorldService svc = MakerWorldService(client=MagicMock(spec=httpx.AsyncClient), auth_token="tok") svc._client.get = AsyncMock(return_value=_response(418, {"captchaId": "abc", "error": "verification required"})) diff --git a/backend/tests/unit/test_cloud_token_expiry.py b/backend/tests/unit/test_cloud_token_expiry.py index b3e9f2c90..692939d81 100644 --- a/backend/tests/unit/test_cloud_token_expiry.py +++ b/backend/tests/unit/test_cloud_token_expiry.py @@ -23,18 +23,17 @@ from unittest.mock import AsyncMock, MagicMock import httpx import pytest -from backend.app.api.routes.cloud import ( +from backend.app.api.routes.cloud import clear_token, store_token +from backend.app.models.settings import Settings +from backend.app.services import bambu_cloud as bc +from backend.app.services.bambu_cloud import BambuCloudService +from backend.app.services.bambu_cloud_credentials import ( CLOUD_EMAIL_KEY, CLOUD_REGION_KEY, CLOUD_TOKEN_INVALID_KEY, CLOUD_TOKEN_KEY, - clear_token, is_cloud_token_invalid, - store_token, ) -from backend.app.models.settings import Settings -from backend.app.services import bambu_cloud as bc -from backend.app.services.bambu_cloud import BambuCloudService @pytest.fixture(autouse=True) diff --git a/backend/tests/unit/test_github_backup_cloud_profiles.py b/backend/tests/unit/test_github_backup_cloud_profiles.py index 03571d02b..90c627b2c 100644 --- a/backend/tests/unit/test_github_backup_cloud_profiles.py +++ b/backend/tests/unit/test_github_backup_cloud_profiles.py @@ -83,7 +83,7 @@ class TestCloudAccountEnumeration: Settings table and the account is keyed ``global``.""" with ( patch( - "backend.app.api.routes.cloud.get_stored_token", + "backend.app.services.bambu_cloud_credentials.get_stored_token", new_callable=AsyncMock, return_value=("bambu-token", "a@b.c", "global"), ), @@ -113,7 +113,7 @@ class TestCloudAccountEnumeration: with ( patch( - "backend.app.api.routes.cloud.get_stored_token", + "backend.app.services.bambu_cloud_credentials.get_stored_token", new_callable=AsyncMock, return_value=(None, None, "global"), ), @@ -137,7 +137,7 @@ class TestCloudAccountEnumeration: with ( patch( - "backend.app.api.routes.cloud.get_stored_token", + "backend.app.services.bambu_cloud_credentials.get_stored_token", new_callable=AsyncMock, return_value=("legacy-global", None, "global"), ), @@ -405,7 +405,7 @@ class TestCollectorAndMetadata: files: dict = {} with ( patch( - "backend.app.api.routes.cloud.get_stored_token", + "backend.app.services.bambu_cloud_credentials.get_stored_token", new_callable=AsyncMock, return_value=(None, None, "global"), ), @@ -435,7 +435,7 @@ class TestCollectorAndMetadata: files: dict = {} with ( patch( - "backend.app.api.routes.cloud.get_stored_token", + "backend.app.services.bambu_cloud_credentials.get_stored_token", new_callable=AsyncMock, return_value=(None, None, "global"), ), @@ -500,7 +500,7 @@ class TestSettingsFallbackIsStillHonoured: await db_session.commit() with patch( - "backend.app.api.routes.cloud.get_stored_token", + "backend.app.services.bambu_cloud_credentials.get_stored_token", new_callable=AsyncMock, return_value=(None, None, "global"), ): diff --git a/backend/tests/unit/test_makerworld_routes.py b/backend/tests/unit/test_makerworld_routes.py index bd8364291..12aa7c3c2 100644 --- a/backend/tests/unit/test_makerworld_routes.py +++ b/backend/tests/unit/test_makerworld_routes.py @@ -14,8 +14,33 @@ from unittest.mock import AsyncMock, patch import pytest -from backend.app.api.routes.makerworld import _canonical_url +from backend.app.api.routes import makerworld as makerworld_routes +from backend.app.core.permissions import Permission from backend.app.models.library import LibraryFile, LibraryFolder +from backend.app.services.model_providers.base import ( + ModelProvider, + ProviderDownload, + ProviderDownloadInfo, + ProviderResolvedModel, + ProviderResourceRef, + ProviderStatus, +) +from backend.app.services.model_providers.makerworld import makerworld_provider + + +def _download_info( + model_id: int = 1400373, + profile_id: int = 298919107, + name: str = "benchy.3mf", + url: str = "https://makerworld.bblmw.com/makerworld/model/X/Y/f.3mf?exp=1&key=k", +) -> ProviderDownloadInfo: + """What ``service.get_download`` hands the route: signed URL + raw upstream + name + the enriched resource ref (``sub_id`` carries the resolved profile).""" + return ProviderDownloadInfo( + ref=ProviderResourceRef(source_type="makerworld", external_id=str(model_id), sub_id=str(profile_id)), + url=url, + suggested_filename=name, + ) def _fake_service(**stubs): @@ -30,37 +55,117 @@ def _fake_service(**stubs): return svc -def _default_design(alphanumeric: str = "US2bb73b106683e5", model_id: int = 1400373): - """Shape the backend needs from ``/design/{id}``: the alphanumeric - ``modelId`` field that iot-service requires, plus at least one instance - so the importer has a ``profile_id`` to fall back on.""" - return { - "id": model_id, - "modelId": alphanumeric, - "title": "Seed Starter", - "instances": [{"profileId": 298919107, "title": "9 cells"}], - } +class _DummyProvider(ModelProvider): + """Stand-in for a second registered model provider. + + Lets the route tests exercise behaviour that differs from the MakerWorld + singleton — a provider-specific default folder name (or none at all), and + its own permissions — without registering anything in the app-wide + registry. The permissions deliberately are *not* the MakerWorld ones: the + routes must gate on whichever provider the request resolved to. + """ + + source_type = "dummy" + display_name = "Dummy" + + def __init__( + self, + default_folder_name: str | None = "Dummy Imports", + view_permission: Permission | None = Permission.LIBRARY_READ, + import_permission: Permission | None = Permission.LIBRARY_UPLOAD, + ): + self.default_folder_name = default_folder_name + self.view_permission = view_permission + self.import_permission = import_permission + + async def build_service(self, *, db, user, api_key_owner=None, client=None): + raise NotImplementedError + + def parse_url(self, url): + return ProviderResourceRef(source_type=self.source_type, external_id="1400373", original_url=url) + + def canonical_url(self, ref): + return f"https://dummy.example.com/models/{ref.external_id}" -def _default_manifest(name: str = "benchy.3mf"): - return { - "name": name, - "url": "https://makerworld.bblmw.com/makerworld/model/X/Y/f.3mf?exp=1&key=k", - } +def _permission_spy(): + """Record which permission the route hands the shared gate. + + The gate itself still runs — the spy delegates to the real factory — so a + test using it proves the wiring without loosening the check. + """ + seen: list = [] + real = makerworld_routes.require_permission_if_auth_enabled + + def factory(*permissions): + seen.extend(permissions) + return real(*permissions) + + return seen, factory -class TestCanonicalUrl: - """Unit test the dedupe-key builder directly — regressions break dedupe - silently so it's worth pinning the exact shape.""" +class TestThumbnail: + """GET /makerworld/thumbnail — the anonymous CDN image proxy.""" - def test_without_profile_id(self): - assert _canonical_url(1400373) == "https://makerworld.com/models/1400373" + def _patch_service(self, svc): + return patch("backend.app.api.routes.makerworld.MakerWorldService", return_value=svc) - def test_without_profile_id_when_none(self): - assert _canonical_url(1400373, None) == "https://makerworld.com/models/1400373" + @pytest.mark.asyncio + async def test_proxies_image_with_immutable_cache(self, async_client): + from unittest.mock import MagicMock - def test_with_profile_id(self): - assert _canonical_url(1400373, 298919107) == ("https://makerworld.com/models/1400373#profileId-298919107") + svc = MagicMock() + svc.fetch_thumbnail = AsyncMock(return_value=(b"png-bytes", "image/png")) + svc.close = AsyncMock() + + with self._patch_service(svc): + resp = await async_client.get( + "/api/v1/makerworld/thumbnail", + params={"url": "https://makerworld.bblmw.com/img/x.png"}, + ) + assert resp.status_code == 200, resp.text + assert resp.content == b"png-bytes" + assert resp.headers["content-type"] == "image/png" + assert "immutable" in resp.headers["cache-control"] + # The SSRF allowlist is the provider's declared seam, not a local + # copy inside the route (review round 2). + assert svc.fetch_thumbnail.await_args.args[0] == "https://makerworld.bblmw.com/img/x.png" + svc.close.assert_awaited_once() + + @pytest.mark.asyncio + async def test_allowlist_comes_from_the_provider_descriptor(self, async_client): + from unittest.mock import MagicMock + + svc = MagicMock() + svc.fetch_thumbnail = AsyncMock(return_value=(b"x", "image/png")) + svc.close = AsyncMock() + + with self._patch_service(svc) as cls: + await async_client.get( + "/api/v1/makerworld/thumbnail", + params={"url": "https://makerworld.bblmw.com/img/x.png"}, + ) + assert cls.call_args.kwargs["thumbnail_hosts"] == makerworld_provider.thumbnail_hosts() + + @pytest.mark.asyncio + async def test_non_cdn_host_is_a_clean_400(self, async_client): + from unittest.mock import MagicMock + + from backend.app.services.model_providers.makerworld.errors import MakerWorldUrlError + + svc = MagicMock() + svc.fetch_thumbnail = AsyncMock( + side_effect=MakerWorldUrlError("Refusing to fetch thumbnail from non-MakerWorld host: 'evil.example'") + ) + svc.close = AsyncMock() + + with self._patch_service(svc): + resp = await async_client.get( + "/api/v1/makerworld/thumbnail", + params={"url": "https://evil.example/x.png"}, + ) + assert resp.status_code == 400 + svc.close.assert_awaited_once() class TestStatus: @@ -79,8 +184,8 @@ class TestStatus: used to be a bare alias for ``has_cloud_token``, so the import button stayed live against a dead credential and the user only found out via a 401 toast.""" - from backend.app.api.routes.cloud import CLOUD_TOKEN_INVALID_KEY, CLOUD_TOKEN_KEY from backend.app.models.settings import Settings + from backend.app.services.bambu_cloud_credentials import CLOUD_TOKEN_INVALID_KEY, CLOUD_TOKEN_KEY db_session.add(Settings(key=CLOUD_TOKEN_KEY, value="dead-token")) db_session.add(Settings(key=CLOUD_TOKEN_INVALID_KEY, value="2026-07-14T07:00:00+00:00")) @@ -94,6 +199,33 @@ class TestStatus: "sign_in_expired": True, } + @pytest.mark.asyncio + async def test_sign_in_expired_reads_credential_rejected_not_auth_error(self, async_client): + """The route keys ``sign_in_expired`` off the machine-readable + ``credential_rejected`` flag, not ``auth_error`` (review round 3 note 1): + ``auth_error`` is the human-readable reason and may be set for non- + credential failures too. A service reporting an expired credential + *without* a message must still surface ``sign_in_expired=True``.""" + svc = AsyncMock() + svc.close = AsyncMock() + svc.get_status = AsyncMock( + return_value=ProviderStatus( + authenticated=True, + can_download=False, + auth_error=None, + credential_rejected=True, + ) + ) + + with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): + resp = await async_client.get("/api/v1/makerworld/status") + assert resp.status_code == 200 + assert resp.json() == { + "has_cloud_token": True, + "can_download": False, + "sign_in_expired": True, + } + class TestResolve: @pytest.mark.asyncio @@ -102,20 +234,25 @@ class TestResolve: "/api/v1/makerworld/resolve", json={"url": "https://thingiverse.com/thing/1"}, ) + # A pasted link for an unsupported host is a clean client-input 400, + # never a 500 — the registry guard runs before any provider call. assert resp.status_code == 400 - assert "makerworld" in resp.json()["detail"].lower() + assert "provider" in resp.json()["detail"].lower() @pytest.mark.asyncio async def test_happy_path_returns_design_and_instances(self, async_client): design_payload = {"id": 1400373, "title": "Seed Starter"} - instances_payload = { - "total": 2, - "hits": [ - {"id": 1452154, "profileId": 298919107, "title": "9 cells"}, - {"id": 1452158, "profileId": 298919564, "title": "12 cells"}, - ], - } - svc = _fake_service(get_design=design_payload, get_design_instances=instances_payload) + instances_payload = [ + {"id": 1452154, "profileId": 298919107, "title": "9 cells"}, + {"id": 1452158, "profileId": 298919564, "title": "12 cells"}, + ] + svc = _fake_service( + resolve=ProviderResolvedModel( + ref=ProviderResourceRef(source_type="makerworld", external_id="1400373", sub_id="1452154"), + design=design_payload, + instances=instances_payload, + ) + ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): resp = await async_client.post( @@ -132,8 +269,10 @@ class TestResolve: @pytest.mark.asyncio async def test_flags_already_imported_library_ids(self, async_client, db_session): - # Seed a matching LibraryFile so resolve() reports it back - existing = LibraryFile( + """Both dedupe shapes must be found through the provider's + ``source_url_filter``: the whole-model canonical URL *and* any + plate-level ``#profileId-`` row.""" + model_row = LibraryFile( filename="prev.3mf", file_path="library/files/prev.3mf", file_type="3mf", @@ -141,13 +280,25 @@ class TestResolve: source_type="makerworld", source_url="https://makerworld.com/models/1400373", ) - db_session.add(existing) + plate_row = LibraryFile( + filename="prev-plate.3mf", + file_path="library/files/prev-plate.3mf", + file_type="3mf", + file_size=100, + source_type="makerworld", + source_url="https://makerworld.com/models/1400373#profileId-298919107", + ) + db_session.add_all([model_row, plate_row]) await db_session.commit() - await db_session.refresh(existing) + await db_session.refresh(model_row) + await db_session.refresh(plate_row) svc = _fake_service( - get_design={"id": 1400373}, - get_design_instances={"total": 0, "hits": []}, + resolve=ProviderResolvedModel( + ref=ProviderResourceRef(source_type="makerworld", external_id="1400373"), + design={"id": 1400373}, + instances=[], + ) ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): @@ -156,93 +307,34 @@ class TestResolve: json={"url": "https://makerworld.com/en/models/1400373"}, ) assert resp.status_code == 200, resp.text - assert resp.json()["already_imported_library_ids"] == [existing.id] + assert sorted(resp.json()["already_imported_library_ids"]) == sorted([model_row.id, plate_row.id]) @pytest.mark.asyncio - async def test_merges_compatibility_from_design_into_instances(self, async_client): - """Per-instance printer compatibility info lives on - ``design.instances[].extention.modelInfo`` but not on - ``/instances/hits``. Resolve enriches each hit with both - ``compatibility`` (primary printer the instance was sliced for) and - ``otherCompatibility`` (extra printers the uploader marked it - compatible with) so the frontend can show "sliced for A1 / also - marked compatible with: H2D, P1S". - """ - design_payload = { - "id": 1400373, - "title": "Seed Starter", - "instances": [ - { - "id": 1452154, - "extention": { - "modelInfo": { - "compatibility": ["A1"], - "otherCompatibility": ["H2D", "P1S"], - } - }, - }, - { - "id": 1452158, - "extention": { - "modelInfo": { - "compatibility": ["X1 Carbon"], - "otherCompatibility": [], - } - }, - }, - ], - } - instances_payload = { - "total": 2, - "hits": [ - {"id": 1452154, "profileId": 298919107, "title": "9 cells"}, - {"id": 1452158, "profileId": 298919564, "title": "12 cells"}, - ], - } - svc = _fake_service(get_design=design_payload, get_design_instances=instances_payload) + async def test_gate_uses_the_permission_of_the_provider_the_url_routes_to(self, async_client): + """Same rule as import, keyed off the pasted URL instead of + ``source_type``: a link that routes to another provider is gated on + that provider's view permission, not ``makerworld:view``.""" + seen, factory = _permission_spy() + dummy = _DummyProvider() + svc = _fake_service( + resolve=ProviderResolvedModel( + ref=ProviderResourceRef(source_type="dummy", external_id="1400373"), + design={"id": 1400373}, + instances=[], + ) + ) - with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): + with ( + patch("backend.app.api.routes.makerworld.require_permission_if_auth_enabled", factory), + patch("backend.app.api.routes.makerworld._provider_for_url", return_value=dummy), + patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)), + ): resp = await async_client.post( "/api/v1/makerworld/resolve", - json={"url": "https://makerworld.com/en/models/1400373"}, + json={"url": "https://dummy.example.com/models/1400373"}, ) assert resp.status_code == 200, resp.text - instances = resp.json()["instances"] - by_id = {i["id"]: i for i in instances} - assert by_id[1452154]["compatibility"] == ["A1"] - assert by_id[1452154]["otherCompatibility"] == ["H2D", "P1S"] - assert by_id[1452158]["compatibility"] == ["X1 Carbon"] - assert by_id[1452158]["otherCompatibility"] == [] - - @pytest.mark.asyncio - async def test_resolve_handles_missing_compatibility_gracefully(self, async_client): - """Older designs (or hits without a matching design.instances entry) - must not crash the resolve response — they just don't get the - compat fields.""" - design_payload = {"id": 1400373, "instances": [{"id": 1452154}]} # no extention - instances_payload = { - "total": 2, - "hits": [ - {"id": 1452154, "profileId": 298919107}, - {"id": 9999999, "profileId": 298919999}, # no design.instances match - ], - } - svc = _fake_service(get_design=design_payload, get_design_instances=instances_payload) - - with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): - resp = await async_client.post( - "/api/v1/makerworld/resolve", - json={"url": "https://makerworld.com/en/models/1400373"}, - ) - assert resp.status_code == 200, resp.text - instances = resp.json()["instances"] - # First instance: design entry exists but no extention → fields absent or None. - first = next(i for i in instances if i["id"] == 1452154) - assert first.get("compatibility") is None - assert first.get("otherCompatibility") is None - # Second instance: no design entry at all → no enrichment, no crash. - second = next(i for i in instances if i["id"] == 9999999) - assert "compatibility" not in second or second["compatibility"] is None + assert seen == [Permission.LIBRARY_READ] class TestImport: @@ -270,11 +362,8 @@ class TestImport: await db_session.commit() await db_session.refresh(existing) - svc = _fake_service( - get_design=_default_design(), - get_profile_download=_default_manifest(), - ) - svc.download_3mf = AsyncMock() # must remain uncalled + svc = _fake_service(get_download=_download_info()) + svc.download = AsyncMock() # must remain uncalled with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): resp = await async_client.post( @@ -287,16 +376,52 @@ class TestImport: assert body["library_file_id"] == existing.id assert body["was_existing"] is True assert body["profile_id"] == 298919107 - svc.download_3mf.assert_not_called() + svc.download.assert_not_called() + + @pytest.mark.asyncio + async def test_unknown_source_type_is_a_clean_400(self, async_client, db_session): + """``source_type`` names the provider (there is no URL to route on); + an unregistered value is a client-input problem — 400 before any + service is built or bytes downloaded.""" + svc = _fake_service( + get_download=_download_info(), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), + ) + svc.download = AsyncMock() + + with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373, "source_type": "thingiverse"}, + ) + assert resp.status_code == 400, resp.text + assert "thingiverse" in resp.json()["detail"].lower() + svc.download.assert_not_called() + + @pytest.mark.asyncio + async def test_unknown_source_type_creates_no_folder_side_effect(self, async_client, db_session): + """Provider resolution must precede destination handling — a rejected + request must not leave an auto-created default folder behind.""" + from sqlalchemy import select + + svc = _fake_service(get_download=_download_info()) + + with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): + await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373, "source_type": "bogus"}, + ) + + result = await db_session.execute(select(LibraryFolder)) + assert result.scalars().all() == [] @pytest.mark.asyncio async def test_autocreates_makerworld_folder_when_folder_id_none(self, async_client, db_session): """Default destination — a top-level "MakerWorld" folder — is created on first import so users don't have to set it up.""" svc = _fake_service( - get_design=_default_design(), - get_profile_download=_default_manifest(), - download_3mf=(self._FAKE_3MF_BYTES, "benchy.3mf"), + get_download=_download_info(), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): @@ -315,6 +440,141 @@ class TestImport: folder = result.scalar_one() assert resp.json()["folder_id"] == folder.id + @pytest.mark.asyncio + async def test_default_folder_comes_from_resolved_provider(self, async_client, db_session): + """``import_instance`` must read ``default_folder_name`` off the provider + it resolved — not the MakerWorld singleton (review round 3 fix). Latent + with one provider, but the difference is visible behind a stand-in: a + second provider's import lands in *its* folder, not "MakerWorld".""" + dummy = _DummyProvider(default_folder_name="Dummy Imports") + svc = _fake_service( + get_download=_download_info(), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), + ) + + with ( + patch("backend.app.api.routes.makerworld._provider_for_source", return_value=dummy), + patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)), + ): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373, "profile_id": 298919107, "source_type": "dummy"}, + ) + assert resp.status_code == 200, resp.text + + from sqlalchemy import select + + result = await db_session.execute( + select(LibraryFolder).where(LibraryFolder.name == "Dummy Imports", LibraryFolder.parent_id.is_(None)) + ) + assert result.scalar_one_or_none() is not None + # The MakerWorld singleton's folder must NOT be auto-created instead. + assert ( + await db_session.execute( + select(LibraryFolder).where(LibraryFolder.name == "MakerWorld", LibraryFolder.parent_id.is_(None)) + ) + ).scalar_one_or_none() is None + + @pytest.mark.asyncio + async def test_none_default_folder_name_imports_to_library_root(self, async_client, db_session): + """A provider that leaves ``default_folder_name`` unset imports into the + library root rather than minting a NULL-named folder (review round 3, + note 3).""" + dummy = _DummyProvider(default_folder_name=None) + svc = _fake_service( + get_download=_download_info(), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), + ) + + with ( + patch("backend.app.api.routes.makerworld._provider_for_source", return_value=dummy), + patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)), + ): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373, "profile_id": 298919107, "source_type": "dummy", "folder_id": None}, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["folder_id"] is None + + from sqlalchemy import select + + result = await db_session.execute(select(LibraryFolder)) + assert result.scalars().all() == [] + + @pytest.mark.asyncio + async def test_gate_uses_the_makerworld_permission_for_makerworld(self, async_client): + """Control for the test below: the default ``source_type`` still gates + on ``makerworld:import``, exactly as the route decorator used to.""" + seen, factory = _permission_spy() + svc = _fake_service( + get_download=_download_info(), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), + ) + + with ( + patch("backend.app.api.routes.makerworld.require_permission_if_auth_enabled", factory), + patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)), + ): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373, "profile_id": 298919107}, + ) + assert resp.status_code == 200, resp.text + assert seen == [Permission.MAKERWORLD_IMPORT] + + @pytest.mark.asyncio + async def test_gate_uses_the_resolved_providers_permission(self, async_client): + """The permission is the resolved provider's, not the MakerWorld + singleton's. It cannot be a route dependency — FastAPI resolves those + before the body exists, so the decorator could only ever name one + provider, and importing from a second one would be gated on + ``makerworld:import``.""" + seen, factory = _permission_spy() + dummy = _DummyProvider() + svc = _fake_service( + get_download=_download_info(), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), + ) + + with ( + patch("backend.app.api.routes.makerworld.require_permission_if_auth_enabled", factory), + patch("backend.app.api.routes.makerworld._provider_for_source", return_value=dummy), + patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)), + ): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373, "profile_id": 298919107, "source_type": "dummy"}, + ) + assert resp.status_code == 200, resp.text + assert seen == [Permission.LIBRARY_UPLOAD] + assert Permission.MAKERWORLD_IMPORT not in seen + + @pytest.mark.asyncio + async def test_provider_without_a_permission_is_refused_not_waved_through(self, async_client, db_session): + """``import_permission`` is optional on the descriptor, so "unset" must + fail closed rather than read as "unrestricted".""" + dummy = _DummyProvider(import_permission=None) + svc = _fake_service( + get_download=_download_info(), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), + ) + + with ( + patch("backend.app.api.routes.makerworld._provider_for_source", return_value=dummy), + patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)), + ): + resp = await async_client.post( + "/api/v1/makerworld/import", + json={"model_id": 1400373, "profile_id": 298919107, "source_type": "dummy"}, + ) + assert resp.status_code == 500 + assert "declares no permission" in resp.json()["detail"] + + from sqlalchemy import select + + assert (await db_session.execute(select(LibraryFile))).scalars().all() == [] + @pytest.mark.asyncio async def test_uses_existing_folder_when_folder_id_provided(self, async_client, db_session): """Caller-supplied ``folder_id`` must be honoured even if the default @@ -325,9 +585,8 @@ class TestImport: await db_session.refresh(folder) svc = _fake_service( - get_design=_default_design(), - get_profile_download=_default_manifest(), - download_3mf=(self._FAKE_3MF_BYTES, "benchy.3mf"), + get_download=_download_info(), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): @@ -343,9 +602,8 @@ class TestImport: """The saved row's ``source_url`` must include ``#profileId-`` so two plates of the same model become two library rows (dedupe is per-plate).""" svc = _fake_service( - get_design=_default_design(), - get_profile_download=_default_manifest(), - download_3mf=(self._FAKE_3MF_BYTES, "benchy.3mf"), + get_download=_download_info(), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): @@ -369,12 +627,8 @@ class TestImport: library row. On-disk storage uses a UUID already, this is belt-and- braces protection for the human-readable field.""" svc = _fake_service( - get_design=_default_design(), - get_profile_download={ - "name": "../../evil.3mf", - "url": "https://makerworld.bblmw.com/makerworld/model/X/Y/f.3mf?exp=1&key=k", - }, - download_3mf=(self._FAKE_3MF_BYTES, "fallback.3mf"), + get_download=_download_info(name="../../evil.3mf"), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="fallback.3mf"), ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): @@ -391,9 +645,8 @@ class TestImport: response field must always be populated, even when the caller provided it explicitly (rather than the backend falling back to design defaults).""" svc = _fake_service( - get_design=_default_design(), - get_profile_download=_default_manifest(), - download_3mf=(self._FAKE_3MF_BYTES, "benchy.3mf"), + get_download=_download_info(), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): @@ -426,9 +679,8 @@ class TestImport: await db_session.refresh(folder) svc = _fake_service( - get_design=_default_design(), - get_profile_download=_default_manifest("seed-starter.3mf"), - download_3mf=(self._FAKE_3MF_BYTES, "seed-starter.3mf"), + get_download=_download_info(name="seed-starter.3mf"), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="seed-starter.3mf"), ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): @@ -453,8 +705,8 @@ class TestImport: @pytest.mark.asyncio async def test_import_to_readonly_external_rejected_at_route(self, async_client, db_session, tmp_path): - """The route-layer gate at ``makerworld.py:256-260`` rejects read-only - externals with 403 before any download happens — so MakerWorld + """The route-layer gate in ``import_instance`` rejects read-only + external folders with 403 before any download happens — so MakerWorld credentials and the upstream download bandwidth aren't wasted.""" ext_dir = tmp_path / "nas-readonly" ext_dir.mkdir() @@ -469,11 +721,8 @@ class TestImport: await db_session.commit() await db_session.refresh(folder) - svc = _fake_service( - get_design=_default_design(), - get_profile_download=_default_manifest(), - ) - svc.download_3mf = AsyncMock() + svc = _fake_service(get_download=_download_info()) + svc.download = AsyncMock() with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): resp = await async_client.post( @@ -481,7 +730,7 @@ class TestImport: json={"model_id": 1400373, "profile_id": 298919107, "folder_id": folder.id}, ) assert resp.status_code == 403, resp.text - svc.download_3mf.assert_not_called() + svc.download.assert_not_called() @pytest.mark.asyncio async def test_import_to_external_with_missing_path_returns_400(self, async_client, db_session, tmp_path): @@ -501,9 +750,8 @@ class TestImport: await db_session.refresh(folder) svc = _fake_service( - get_design=_default_design(), - get_profile_download=_default_manifest(), - download_3mf=(self._FAKE_3MF_BYTES, "benchy.3mf"), + get_download=_download_info(), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): @@ -535,9 +783,8 @@ class TestImport: await db_session.refresh(folder) svc = _fake_service( - get_design=_default_design(), - get_profile_download=_default_manifest("benchy.3mf"), - download_3mf=(self._FAKE_3MF_BYTES, "benchy.3mf"), + get_download=_download_info(name="benchy.3mf"), + download=ProviderDownload(file_bytes=self._FAKE_3MF_BYTES, filename="benchy.3mf"), ) with patch("backend.app.api.routes.makerworld._build_service", AsyncMock(return_value=svc)): diff --git a/backend/tests/unit/test_makerworld_s3_tls.py b/backend/tests/unit/test_makerworld_s3_tls.py index 52ee56bd6..b6c6c4f51 100644 --- a/backend/tests/unit/test_makerworld_s3_tls.py +++ b/backend/tests/unit/test_makerworld_s3_tls.py @@ -1,4 +1,4 @@ -"""Tests for the S3 presigned-download path in ``services/makerworld.py``. +"""Tests for the S3 presigned-download path in ``model_providers/makerworld/http.py``. MakerWorld hands back an AWS presigned URL for the 3MF, and we fetch that one with ``urllib.request`` rather than httpx — httpx re-encodes the query string @@ -25,7 +25,7 @@ from cryptography.hazmat.primitives import hashes, serialization from cryptography.hazmat.primitives.asymmetric import ec from cryptography.x509.oid import NameOID -from backend.app.services import makerworld as mw +from backend.app.services.model_providers.makerworld import http as mw # A presigned URL in the shape Bambu Cloud actually mints: the signature is # computed over these exact query-string bytes, so any re-encoding breaks it. diff --git a/backend/tests/unit/test_model_provider_registry.py b/backend/tests/unit/test_model_provider_registry.py new file mode 100644 index 000000000..473ed39ca --- /dev/null +++ b/backend/tests/unit/test_model_provider_registry.py @@ -0,0 +1,70 @@ +"""Tests for the model-provider registry. + +Pins the routing seam that a future *shared* import API will use: pasted URLs +go through ``find_for_url`` and land on the provider that owns them. +""" + +from __future__ import annotations + +import pytest + +from backend.app.services.model_providers import makerworld_provider, registry +from backend.app.services.model_providers.base import ModelProvider +from backend.app.services.model_providers.registry import ModelProviderRegistry + + +class _DummyProvider(ModelProvider): + source_type = "dummy" + display_name = "Dummy" + + async def build_service(self, *, db, user, api_key_owner=None, client=None): + raise NotImplementedError + + def parse_url(self, url): + raise NotImplementedError + + def canonical_url(self, ref): + raise NotImplementedError + + +class TestAppRegistry: + """The app-wide singleton auto-registers MakerWorld on import.""" + + def test_makerworld_is_registered(self): + assert registry.get("makerworld") is makerworld_provider + assert registry.get("makerworld").display_name == "MakerWorld" + + def test_unknown_source_type_raises_keyerror(self): + with pytest.raises(KeyError): + registry.get("thingiverse") + + def test_find_for_url_routes_makerworld_urls(self): + provider = registry.find_for_url("https://makerworld.com/en/models/1400373#profileId-1452154") + assert provider is makerworld_provider + + def test_find_for_url_returns_none_for_foreign_hosts(self): + assert registry.find_for_url("https://thingiverse.com/thing/123") is None + assert registry.find_for_url("") is None + assert registry.find_for_url(None) is None # type: ignore[arg-type] + + +class TestModelProviderRegistry: + def test_register_is_idempotent_per_instance(self): + reg = ModelProviderRegistry() + provider = _DummyProvider() + reg.register(provider) + reg.register(provider) + assert reg.all() == (provider,) + + def test_register_duplicate_source_type_rejected(self): + reg = ModelProviderRegistry() + reg.register(_DummyProvider()) + with pytest.raises(ValueError): + reg.register(_DummyProvider()) + + def test_all_returns_registered_providers(self): + reg = ModelProviderRegistry() + provider = _DummyProvider() + reg.register(provider) + assert provider in reg.all() + assert len(reg.all()) == 1