mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-18 11:56:01 +02:00
fix(api-keys): slice + slicer-presets routes resolve cloud token via key owner (#1182 follow-up)
turulix's headless slicing pipeline got cloud preset IDs from /api/v1/cloud/settings (the /cloud/* gate from #1182 worked), but slicing those IDs via POST /library/files/{id}/slice failed with "no Bambu Cloud session is stored" — the slice route lives on a different router, never saw the api_key_owner stash, and _resolve_cloud fell through to the empty auth-disabled global Settings token. Add a permissive route-level dep that returns the API key's owner when the key has the cloud scope and None otherwise (never raises), so non-/cloud/* routes can opt in without breaking the local-preset path. Wire it into POST /library/files/{id}/slice and GET /slicer/presets (same root cause, would hit any UI proxied through an API key). The route picks current_user or api_key_cloud_owner before deriving user_id. Auth gate's None-return for API keys is unchanged — keeping the owner-resolution scoped to the routes that actually need a cloud token prevents scope creep into routes that fence on ``current_user is None``.
This commit is contained in:
@@ -12,6 +12,8 @@ All notable changes to Bambuddy will be documented in this file.
|
||||
- **Filament Track Switch (FTS) support — print modal filament dropdown is no longer empty when an X2D / H2D has the FTS accessory installed** ([#1162](https://github.com/maziggy/bambuddy/issues/1162), reported by @mkavalecz) — When the FTS accessory is installed the printer's MQTT changes one nibble of the per-AMS `info` bitmask: bits 8-11 flip from a fixed extruder ID (0x0 / 0x1) to `0xE` ("uninitialized"), because the AMS is no longer wired to a single nozzle — the FTS dynamically routes any slot to either extruder. Bambuddy's MQTT parser already skipped 0xE entries when building `ams_extruder_map` (matching BambuStudio's reading for boot-time transient state), so with the FTS installed the map ended up empty and the print modal's filament dropdown — which filters by `extruderId === nozzle_id` to prevent cross-nozzle assignment ("position of left hotend is abnormal" failures) — filtered out *every* loaded slot. Net effect: empty Filament Mapping dropdown on every dual-nozzle print with the FTS, even when the AMS was fully loaded with the right material. Detection comes from a new MQTT field — `print.device.fila_switch` — which is non-null only when the accessory is installed; it carries the routing topology as two arrays: `in[track] = currently fed slot (-1 = empty)` and `out[track] = extruder this track terminates at`. The fix surfaces this through a new `FilaSwitchState` dataclass on `PrinterState` (`installed`, `in_slots`, `out_extruders`, `stat`, `info`) and the equivalent `FilaSwitchResponse` Pydantic schema on the `GET /printers/{id}/status` route. Frontend (`useFilamentMapping.ts` + `FilamentMapping.tsx`) skips the per-extruder filter when `printerStatus.fila_switch?.installed === true` so any compatible AMS slot can satisfy any nozzle's filament requirement, since the FTS handles the routing. Slots currently fed into a track also get a routing badge in the dropdown — `[L]` or `[R]` — so the user can tell at a glance which slot the FTS is currently routing where (idle slots get no badge: they can be routed to either extruder on demand). The hard "no cross-nozzle assignment" filter on real dual-nozzle printers without the FTS stays untouched (still trips the same way it always has — `fila_switch == null` keeps the existing behaviour). 4 backend tests in `test_bambu_mqtt.py::TestFilamentTrackSwitchDetection` (default-not-installed, detect-from-MQTT-using-the-reporter's-bundle, no-fila_switch-field-stays-not-installed, missing-in-out-arrays-don't-crash) and 2 frontend tests in `useFilamentMapping.test.ts` (FTS-active drops the nozzle filter; explicit `fila_switch: null` keeps the filter applied). Upstream fila_switch payloads with anything other than the documented shape are tolerated — `installed` flips on the *presence* of the field, the routing arrays default to empty lists if missing, and the dropdown skips the badge for slots not currently in `in_slots`.
|
||||
|
||||
### Fixed
|
||||
- **Slicing a library file via API key fails with "no Bambu Cloud session is stored" even when the key has cloud access** ([#1182](https://github.com/maziggy/bambuddy/issues/1182) follow-up, reported by @turulix) — Tim shipped the headless slicing pipeline #1182 was filed for, then hit a second wall: `GET /api/v1/cloud/settings` returned the cloud preset IDs correctly (the `/cloud/*` router-level gate from #1182 was doing its job), but `POST /api/v1/library/files/{id}/slice` with those IDs in the request body failed the slice job with `error_status: 400, error_detail: "Cloud preset selected for printer, but no Bambu Cloud session is stored. Sign in to Bambu Cloud and retry."` Cause: the `/cloud/*` fix routes the API key's owner User through `cloud_caller` (a router-level gate stashes the owner on `request.state.api_key_owner`, route-level deps pull it back out), but the slice route lives on `/library/*` — different router, no gate, so when the auth dep returned `None` for the API-keyed request the slice route passed `current_user_id=None` straight through to `_run_slicer_with_fallback` → `_resolve_cloud(db, user=None)` → `get_stored_token(db, None)`, which falls back to the auth-disabled global `Settings` table. That table is empty in auth-enabled deployments, so cloud preset resolution failed even though the key's owner User had a perfectly valid `cloud_token` on their User row. Fix is a new route-level dep `resolve_api_key_cloud_owner` in `cloud.py` that's permissive (returns the owner User if the key has `can_access_cloud=true`, otherwise None — never raises) so it can be safely added to non-`/cloud/*` routes without breaking the local-presets path: a request with an API key that lacks the cloud scope still slices fine against local presets, and only fails with the existing "no Bambu Cloud session" error if it actually selects a cloud preset. Wired into `POST /library/files/{id}/slice` (Tim's blocker) and `GET /slicer/presets` (the SliceModal preset dropdown source — same root cause, would have hit anyone using the UI through an API-keyed reverse proxy). Both routes now resolve the cloud-token owner via `current_user or api_key_cloud_owner` instead of `current_user.id if current_user else None`. The auth gate's None-return for API keys is unchanged — keeping that fix scoped to the routes that actually need cloud-token resolution prevents accidental scope creep into other routes that fence on `current_user is None`. 4 new integration tests in `test_api_key_cloud_access.py::TestSliceRouteCloudOwnerResolution` pin the dep contract: returns the owner for a key with `can_access_cloud=True` and a valid owner; returns None for an owned key without the cloud scope (so cloud presets still 400 cleanly, local presets still slice); returns None for legacy ownerless keys; no-op for JWT and anonymous callers.
|
||||
|
||||
- **Project cover photo thumbnail too small to recognise the print** ([#1155](https://github.com/maziggy/bambuddy/issues/1155) follow-up, reported by @smandon) — The 40×40 thumbnail @smandon's MakerWorld download workflow relied on for "is this the model I'm looking for?" wasn't readable at that size; he asked for either a larger thumbnail or a click-to-enlarge full preview. Enlarging the thumbnail itself would shift the card layout and cost the dense grid he chose to use for browsing many projects, so the fix keeps the 40×40 thumbnail and shows a portal-mounted 384×384 popover on hover. The popover renders the full image in `object-contain` so tall portrait MakerWorld photos aren't cropped to a square, has `pointer-events-none` so it can't intercept hover and create a flicker loop, and `z-[100]` so it stacks above every sibling card in the grid. **Why a portal:** ProjectCard carries `overflow-hidden` (for its rounded-corner clipping and the color accent bar), so an in-tree popover gets clipped by the card the moment it extends past the card's bounds — exactly the cut-off behaviour @smandon reported on the second iteration. Rendering via `createPortal(..., document.body)` escapes every ancestor clipping context, and `position: fixed` with measurements from `getBoundingClientRect()` keeps the popover pinned next to the thumbnail regardless of where the card sits in the grid. **Edge handling:** if the thumbnail is near the viewport's right edge the popover flips to the LEFT side of the thumbnail; vertical position is clamped so the popover never overflows the window top or bottom. The thumbnail's own `onClick` is `stopPropagation`'d so hovering the popover area never accidentally triggers the parent card's "open project" navigation. 2 new tests in `ProjectsPage.test.tsx` pin the contract: hovering mounts the popover at document.body level (not nested in the card — a future refactor that drops the portal would re-introduce the clipping bug, and the test catches that); leaving unmounts it; the popover img points at the same cover-image URL as the small thumbnail with `object-contain`; cards without a cover_image_filename never mount the portal-rendering component (so a hover doesn't flash an empty preview).
|
||||
|
||||
- **Spool edit form lost the Extra Colours value on reopen, Dual Color rendered identically to Gradient, and the Sparkle / checkerboard visuals were too subtle** ([#1154](https://github.com/maziggy/bambuddy/issues/1154) follow-up, reported by @maugsburger) — Four issues against the multi-colour swatch work that landed for #1154. **(1) Extra Colours input didn't hydrate on edit reopen:** `ColorSection`'s draft buffer was seeded once via `useState(formData.extra_colors)`, but `SpoolFormModal` opens *before* its own `useEffect` populates `formData` from the spool record — so by the time the saved value landed, the input's local state had already been initialised to `''` and never re-synced. The COLOR preview banner above the input rendered correctly (consumes formData directly), making it obvious the data WAS persisted; only the input was stuck blank, which the user then had to retype to save anything else. Fix: a ref-guarded `useEffect` resyncs `extraColorsDraft` when `formData.extra_colors` changes via an external update (e.g. modal opening with a spool); the ref is updated inside `commitExtraColors` so the user's own typing is round-tripped without the resync clobbering it. **(2) `Dual Color` and `Gradient` produced the same diagonal blend:** `buildColorLayer` in `filamentSwatchHelpers.ts` ran the same `linear-gradient(135deg, ...)` for both effect types, so a "Dual Color" spool was visually indistinguishable from a "Gradient" one. Real dual-colour spools have two distinct bars on the reel — that's the whole point of the variant. Fix: when `effect_type` is `dual-color` or `tri-color`, build the colour layer as `linear-gradient(to right, c1 0% X%, c2 X% Y%, ...)` with CSS double-position stops (so the colour change is a *hard line* rather than a blend region) and equal-width segments across the stops; `gradient` keeps the original 135° smooth blend. The existing `multicolor` conic-gradient path is untouched. **(3) Sparkle effect was almost invisible on card-sized swatches:** the original 4-dot pattern (each ~1px) read fine on the small inline swatch but disappeared on the 60-pixel-tall inventory card banners — exactly where the user actually identifies a spool. Bumped to 13 flecks in mixed sizes (1px / 1.5px / 2px) and varying opacity (0.65 → 1.0) to give a depth-of-field "metal flake" feeling, distinct from solid + multi-colour. **(4) Checkerboard cell density scaled with the swatch:** the previous helper put `repeating-conic-gradient(...)` in the `background-image` and the caller applied `background-size: cover`, so the same 4-cell pattern was either tiny squares on a small swatch or four huge squares on a card-sized banner. Made `buildFilamentBackground()` return `{ backgroundImage, backgroundSize }` with per-layer sizes — painted layers stay `cover`, the checkerboard gets a fixed 12px tile so the cell density stays consistent regardless of element size and clearly reads as a transparency indicator rather than a multi-colour stripe. Updated the three existing call sites (`InventoryPage` group banner + spool card, `ColorSection` preview) to spread the returned style object directly. **8 new frontend tests** cover the four fixes: hard-split contract for Dual/Tri Color (3 tests + 1 regression guard that Dual ≠ Gradient for the same stops); Sparkle prominence (≥ 10 distinct radial-gradient layers in the rendered background); checkerboard density (last `backgroundSize` layer is a fixed pixel value, not `cover`); 4 hydration tests pinning the input restore path (fills when formData arrives via parent update, resyncs when the spool changes mid-form, doesn't clobber live user typing, clears when the new spool has no extra_colors).
|
||||
|
||||
@@ -124,6 +124,41 @@ def cloud_caller(*permissions: Permission):
|
||||
return Depends(resolved)
|
||||
|
||||
|
||||
async def resolve_api_key_cloud_owner(
|
||||
credentials: HTTPAuthorizationCredentials | None = Depends(security),
|
||||
x_api_key: str | None = Header(default=None, alias="X-API-Key"),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
) -> User | None:
|
||||
"""Route-level dep for non-/cloud/* endpoints that need to read the
|
||||
caller's stored Bambu Cloud token (e.g. the slice path resolving cloud
|
||||
presets — #1182 follow-up).
|
||||
|
||||
Returns the API key's owner User when the caller is an API-keyed
|
||||
request *and* the key has ``can_access_cloud=True``; returns None for
|
||||
JWT, anonymous, or API keys without the cloud scope. The caller is
|
||||
expected to fall back to the JWT-authed ``current_user`` first and use
|
||||
this dep's result only when ``current_user`` is None.
|
||||
|
||||
Unlike ``_cloud_api_key_gate`` (which 403s legacy/non-cloud keys at the
|
||||
router level), this dep is permissive: it returns None instead of
|
||||
raising, so a slice request via an API key without cloud scope still
|
||||
runs against local presets. The downstream cloud-token check in
|
||||
``preset_resolver._resolve_cloud`` produces the right 400 if the
|
||||
request actually selects a cloud preset.
|
||||
"""
|
||||
api_key_value: str | None = None
|
||||
if x_api_key:
|
||||
api_key_value = x_api_key
|
||||
elif credentials and credentials.credentials.startswith("bb_"):
|
||||
api_key_value = credentials.credentials
|
||||
if api_key_value is None:
|
||||
return None
|
||||
api_key = await _validate_api_key(db, api_key_value)
|
||||
if api_key is None or api_key.user_id is None or not api_key.can_access_cloud:
|
||||
return None
|
||||
return await _user_from_api_key(db, api_key)
|
||||
|
||||
|
||||
router = APIRouter(prefix="/cloud", tags=["cloud"], dependencies=[Depends(_cloud_api_key_gate)])
|
||||
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ from sqlalchemy import func, select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from sqlalchemy.orm import selectinload
|
||||
|
||||
from backend.app.api.routes.cloud import resolve_api_key_cloud_owner
|
||||
from backend.app.core.auth import (
|
||||
RequireCameraStreamTokenIfAuthEnabled,
|
||||
require_ownership_permission,
|
||||
@@ -3027,6 +3028,7 @@ async def slice_library_file(
|
||||
request: SliceRequest,
|
||||
db: AsyncSession = Depends(get_db),
|
||||
current_user: User | None = Depends(require_permission_if_auth_enabled(Permission.LIBRARY_UPLOAD)),
|
||||
api_key_cloud_owner: User | None = Depends(resolve_api_key_cloud_owner),
|
||||
):
|
||||
"""Enqueue a slice job for a library file. Returns 202 + job_id; the
|
||||
slice runs in the background, the caller polls `GET /slice-jobs/{id}`.
|
||||
@@ -3060,7 +3062,12 @@ async def slice_library_file(
|
||||
model_bytes = src_path.read_bytes()
|
||||
folder_id = lib_file.folder_id
|
||||
source_lib_file_id = lib_file.id
|
||||
user_id = current_user.id if current_user else None
|
||||
# API-keyed callers get None from the auth gate (auth.py keeps that
|
||||
# behaviour to avoid a wider scope expansion). Fall back to the API
|
||||
# key's owner so cloud-preset resolution can read the stored
|
||||
# cloud_token (#1182 follow-up).
|
||||
cloud_token_user = current_user or api_key_cloud_owner
|
||||
user_id = cloud_token_user.id if cloud_token_user else None
|
||||
|
||||
# If the source has a `print_name` in its metadata (BambuStudio always
|
||||
# sets this; OrcaSlicer often leaves it blank), derive the sliced
|
||||
|
||||
@@ -20,7 +20,7 @@ from fastapi import APIRouter, Depends
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from backend.app.api.routes.cloud import get_stored_token
|
||||
from backend.app.api.routes.cloud import get_stored_token, resolve_api_key_cloud_owner
|
||||
from backend.app.core.auth import RequirePermissionIfAuthEnabled
|
||||
from backend.app.core.config import settings as app_settings
|
||||
from backend.app.core.database import get_db
|
||||
@@ -337,6 +337,7 @@ def _dedupe_by_name(
|
||||
async def list_unified_presets(
|
||||
db: AsyncSession = Depends(get_db),
|
||||
current_user: User | None = RequirePermissionIfAuthEnabled(Permission.LIBRARY_UPLOAD),
|
||||
api_key_cloud_owner: User | None = Depends(resolve_api_key_cloud_owner),
|
||||
) -> UnifiedPresetsResponse:
|
||||
"""List slicer presets across cloud / local / standard tiers, deduped by name.
|
||||
|
||||
@@ -346,8 +347,14 @@ async def list_unified_presets(
|
||||
gated on ``CLOUD_AUTH`` inside ``_fetch_cloud_presets`` so a user with
|
||||
only ``LIBRARY_UPLOAD`` doesn't see cloud presets they shouldn't have
|
||||
access to.
|
||||
|
||||
API-keyed callers (which return None from ``current_user``) get the
|
||||
owner User via ``resolve_api_key_cloud_owner`` when the key has the
|
||||
cloud-access scope, so the cloud tier surfaces correctly for them
|
||||
too — matching the slice route (#1182 follow-up).
|
||||
"""
|
||||
cloud, cloud_status = await _fetch_cloud_presets(db, current_user)
|
||||
cloud_token_user = current_user or api_key_cloud_owner
|
||||
cloud, cloud_status = await _fetch_cloud_presets(db, cloud_token_user)
|
||||
local = await _fetch_local_presets(db)
|
||||
standard = await _fetch_bundled_presets(db)
|
||||
|
||||
|
||||
@@ -293,3 +293,124 @@ class TestOwnerDeletionCleanup:
|
||||
db_session.expire_all()
|
||||
result = await db_session.execute(select(APIKey).where(APIKey.id == key_id))
|
||||
assert result.scalar_one_or_none() is None, "API key should have been removed when its owner was deleted"
|
||||
|
||||
|
||||
class TestSliceRouteCloudOwnerResolution:
|
||||
"""The /library/files/{id}/slice route's cloud-token resolver
|
||||
(#1182 follow-up — turulix). The cloud /cloud/* surface gets the API
|
||||
key owner via ``cloud_caller`` (router-level gate), but the slice path
|
||||
is on /library/* and goes through ``resolve_api_key_cloud_owner``.
|
||||
Without this dep the slice path called ``get_stored_token(db, user=None)``
|
||||
and produced "no Bambu Cloud session is stored" because the global
|
||||
Settings cloud_token is empty in auth-enabled deployments — even when
|
||||
the API key's owner had a perfectly valid token on their User row.
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_dep_returns_owner_for_key_with_cloud_scope(
|
||||
self, async_client: AsyncClient, db_session: AsyncSession
|
||||
):
|
||||
"""Bare contract: dep resolves to the owner User when the API key
|
||||
has ``can_access_cloud=True`` and a valid owner."""
|
||||
from backend.app.api.routes.cloud import resolve_api_key_cloud_owner
|
||||
|
||||
await _setup_auth_with_admin(async_client)
|
||||
admin = await _store_admin_cloud_token(db_session, "cloudadmin", token="fake-bambu-token")
|
||||
|
||||
full_key, key_hash, key_prefix = generate_api_key()
|
||||
owned = APIKey(
|
||||
name="slice-cloud",
|
||||
key_hash=key_hash,
|
||||
key_prefix=key_prefix,
|
||||
user_id=admin.id,
|
||||
can_access_cloud=True,
|
||||
)
|
||||
db_session.add(owned)
|
||||
await db_session.commit()
|
||||
|
||||
# Drive the dep directly with the same wiring FastAPI does. We can't
|
||||
# easily fake an HTTPAuthorizationCredentials object without fastapi
|
||||
# internals, so pass the raw token via the X-API-Key header param.
|
||||
owner = await resolve_api_key_cloud_owner(
|
||||
credentials=None,
|
||||
x_api_key=full_key,
|
||||
db=db_session,
|
||||
)
|
||||
assert owner is not None, "Dep must resolve owner for a valid cloud-scope key"
|
||||
assert owner.id == admin.id
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_dep_returns_none_for_key_without_cloud_scope(
|
||||
self, async_client: AsyncClient, db_session: AsyncSession
|
||||
):
|
||||
"""If the key lacks ``can_access_cloud``, the dep refuses to resolve
|
||||
an owner — the slice path then falls through to user_id=None and
|
||||
any cloud-preset references in the slice request will produce the
|
||||
usual "no Bambu Cloud session is stored" error. Local presets
|
||||
still work."""
|
||||
from backend.app.api.routes.cloud import resolve_api_key_cloud_owner
|
||||
|
||||
await _setup_auth_with_admin(async_client)
|
||||
result = await db_session.execute(select(User).where(User.username == "cloudadmin"))
|
||||
admin = result.scalar_one()
|
||||
|
||||
full_key, key_hash, key_prefix = generate_api_key()
|
||||
owned = APIKey(
|
||||
name="slice-no-cloud",
|
||||
key_hash=key_hash,
|
||||
key_prefix=key_prefix,
|
||||
user_id=admin.id,
|
||||
can_access_cloud=False,
|
||||
)
|
||||
db_session.add(owned)
|
||||
await db_session.commit()
|
||||
|
||||
owner = await resolve_api_key_cloud_owner(
|
||||
credentials=None,
|
||||
x_api_key=full_key,
|
||||
db=db_session,
|
||||
)
|
||||
assert owner is None, "Dep must NOT leak owner for a key without cloud scope"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_dep_returns_none_for_legacy_ownerless_key(self, async_client: AsyncClient, db_session: AsyncSession):
|
||||
"""Legacy keys (user_id NULL) created before #1182 must be ignored
|
||||
by this dep — same fence as the /cloud/* gate."""
|
||||
from backend.app.api.routes.cloud import resolve_api_key_cloud_owner
|
||||
|
||||
await _setup_auth_with_admin(async_client)
|
||||
|
||||
full_key, key_hash, key_prefix = generate_api_key()
|
||||
legacy = APIKey(
|
||||
name="slice-legacy",
|
||||
key_hash=key_hash,
|
||||
key_prefix=key_prefix,
|
||||
user_id=None,
|
||||
can_access_cloud=False,
|
||||
)
|
||||
db_session.add(legacy)
|
||||
await db_session.commit()
|
||||
|
||||
owner = await resolve_api_key_cloud_owner(
|
||||
credentials=None,
|
||||
x_api_key=full_key,
|
||||
db=db_session,
|
||||
)
|
||||
assert owner is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.integration
|
||||
async def test_dep_no_op_for_jwt_or_anonymous(self, db_session: AsyncSession):
|
||||
"""JWT-authed and anonymous callers don't hit the API key path —
|
||||
dep returns None unconditionally."""
|
||||
from backend.app.api.routes.cloud import resolve_api_key_cloud_owner
|
||||
|
||||
owner = await resolve_api_key_cloud_owner(
|
||||
credentials=None,
|
||||
x_api_key=None,
|
||||
db=db_session,
|
||||
)
|
||||
assert owner is None
|
||||
|
||||
Reference in New Issue
Block a user