fix(auth/ui): sidebar accepts granular *_read tiers for archives/queue/files (#1755)

navPermissions in Layout.tsx gated three resources on the LEGACY *:read flag.
  Default Operators group is seeded with *_own only (and the migration map flips
  legacy → _own on existing groups), so non-admin users never held the legacy
  permission and the sidebar hid Archives / Queue / Files even though the
  underlying API accepted their requests. Reporter only spotted Files; same bug
  shape applied to Archives and Queue.

  Fix: navPermissions accepts Permission | Permission[]; the three affected
  resources list all three tiers. isHidden checks .some(hasPermission) for
  arrays. Permission type extended with the matching *_own / *_all variants —
  backend already shipped them, the TS type just didn't declare them.
This commit is contained in:
maziggy
2026-06-16 07:44:22 +02:00
parent ead6c37147
commit 804fe470fa
6 changed files with 111 additions and 11 deletions
+2
View File
@@ -22,6 +22,8 @@ All notable changes to Bambuddy will be documented in this file.
- **VP bridge-synthesised reply trace (#1622 round 3)** — The round-2 cmd.jsonl from shaddowlink's P1S vs H2D capture proves the actual failure mode: on P1S in archive mode the slicer issues `extrusion_cali_set` (push K/n directly) and the printer responds `fail`, on H2D and on the P1S second round the slicer takes the `extrusion_cali_sel` flow (select by `filament_id` / `cali_idx`) and the printer responds `success`. Both flows traverse the bridge cleanly — `ams_filament_setting` round-trips with `result=success` and the cached push_status carries `tray_info_idx=GFA11`, `tray_type=PLA-AERO`, K/n, and `cali_idx=-1` intact. So the bridge is innocent on every layer the dump can see, and the open question becomes: what makes the slicer pick `_set` vs `_sel`? Likely candidates are the `info.get_version` answer Bambuddy synthesises (slicer fingerprints on `sw_ver` / `hw_ver` / `module` to decide its command flow) or the first cached `pushall` response the slicer reads to bootstrap its UI. Round 2 captured neither — the JSONL had `slicer_to_bridge` and `printer_to_slicer` directions but no `bridge_to_slicer` direction for the bridge's own synthesised replies. Same env flag (`BAMBUDDY_VP_DUMP_WIRE=1`) now also appends every bridge-synthesised reply (info.get_version answer, project_file ack, on-demand pushall response) to `<log_dir>/vp_wire/<vp_name>_cmd.jsonl` under direction `bridge_to_slicer`. Capture lives in `mqtt_server.py::_publish_to_report` — the single chokepoint every synthesised reply already passes through — gated on a new `log_event: bool = True` parameter; the 1Hz periodic-push path threads `log_event=False` so the JSONL isn't flooded with ~60 lines/min per VP (snapshot dump already covers cache shape). The on-demand pushall response from `_send_status_report` IS logged because that's the bootstrap-fingerprint reply the slicer reads on first connect. Two additional unit tests in `test_vp_mqtt_bridge.py::TestWireFormat` pin the event-on-default and skip-when-`log_event=False` posture; `test_vp_wire_dump.py` already covers the underlying `append_event` shape and the new direction is documented in `_debug.py`'s docstring. Diagnostic-only — does not change the publish data path; the new param defaults preserve every existing call site's behaviour.
### Fixed
- **Sidebar entries for Files / Archives / Queue no longer hide from non-admin users with granular read access (#1755, reported by @knifesk)** — The reporter noticed the **File Manager** sidebar entry was hidden for a default Operators user even though the same user could load `/files` directly and the backend API accepted their requests. Root cause is broader than reported: `frontend/src/components/Layout.tsx::navPermissions` mapped `files → 'library:read'`, `archives → 'archives:read'`, `queue → 'queue:read'` — the LEGACY permission flags — but the default Operators group at `backend/app/core/permissions.py:368-380` is seeded with the GRANULAR variants only (`ARCHIVES_READ_OWN.value`, `QUEUE_READ_OWN.value`, `LIBRARY_READ_OWN.value`). The migration path at `backend/app/core/database.py:3034-3041` also flips legacy `*:read` → `*:read_own` on existing non-admin groups. So a non-admin user never holds the legacy permission, `hasPermission('library:read')` returns false, sidebar entry is suppressed — for all three resources, not just Files. Admins get `ALL_PERMISSIONS` which includes the legacy variant, so the sidebar always renders for them, which is why this regression went unnoticed until a real non-admin Operator account landed in #1755. **Fix:** `navPermissions` now accepts `Permission | Permission[]` and the three affected resources list all three tiers (`*:read`, `*:read_own`, `*:read_all`). The `isHidden` check switches on the array type — `some(hasPermission)` for arrays, current behavior for single values. Nothing else in the gate logic changed. `frontend/src/api/client.ts` Permission type extended with the missing granular variants (`archives:read_own`, `archives:read_all`, `queue:read_own`, `queue:read_all`, `library:read_own`, `library:read_all`) — these existed in the backend enum and were already being shipped to the frontend in `/auth/me`, but the TS type didn't declare them so any new code wanting to gate on the granular tier would TypeScript-error. **What this also fixes downstream:** any future feature that needs to gate UI on `*:read_own` / `*:read_all` can now do so without re-adding the same type entries. **Tests:** 5 new cases in `Layout.test.tsx::'Sidebar gate accepts granular read tiers (#1755)'` — Files visible with only `library:read_own`, Files visible with only `library:read_all`, Archives visible with only `archives:read_own`, Queue visible with only `queue:read_own`, and the negative case (`printers:read` only — none of Files / Archives / Queue render). 22/22 Layout vitests green; ESLint clean; `npm run build` clean. No backend change, no DB migration, no new i18n keys. No new permission — just unmasks UI for users who already had backend access.
- **Push notification for "Printer offline" now actually fires (#1752, reported by @saint-hh)** — The notification provider's `on_printer_offline` toggle has shipped since the notifications feature landed: schema field, DB column, `notification_template.py` entry, and the dispatcher `NotificationService.on_printer_offline(printer_id, printer_name, db)` are all in place. What was missing was the caller — nothing in the codebase actually invoked the dispatcher when a printer went offline. The reporter (P2S, smart-plug-cuts-power scenario) confirmed turning the toggle on did nothing; only the print-failure notification fired when power was restored, via the firmware's `gcode_state=FAILED` report on MQTT reconnect. **Why the toggle was orphan:** every other provider event (`on_print_start`, `on_print_complete`, `on_print_progress`, `on_printer_error`, etc.) has a clear call site under `main.py::on_printer_status_change` or alongside the print-lifecycle hooks. The offline event was the only edge-triggered toggle without one — the dispatcher and template predated the wiring step and were silently shipped. Both upstream offline-trigger paths (`smart_plug_manager` → `printer_manager.mark_printer_offline()` and `bambu_mqtt.py::check_staleness` after the 30s STALE_RECONNECT_COOLDOWN) route through `_on_status_change` already and reach `on_printer_status_change`; the handler just didn't act on the disconnect edge. **Fix:** edge detection in `on_printer_status_change` watches `state.connected` against the previous observation per printer (`_printer_last_connected: dict[int, bool]`). On the True → False transition it schedules `_maybe_notify_printer_offline(printer_id)` as a background asyncio task; on the next True observation it cancels any pending task. The helper sleeps `_PRINTER_OFFLINE_NOTIFY_DEBOUNCE_SECONDS = 60.0` then re-checks `printer_manager.is_connected(printer_id)` — only fires the notification if the printer is still offline. **Why 60s debounce:** sized against `bambu_mqtt.py::STALE_RECONNECT_COOLDOWN = 30s` — a single stale-trigger + reconnect cycle isn't enough to fire, only a real outage that survives one full cooldown notifies. Transient MQTT blips (WiFi roam, broker reload, brief packet loss) recover within the window and the cancellation path kicks in. **Edge-case handling:** initial observation with no prior connected state doesn't fire (covers Bambuddy startup with an already-offline printer); a False → False repeat doesn't reschedule (the in-flight task stays in place rather than resetting the clock on every status callback, which would otherwise mean the notification never fires); the task entry pops from `_printer_offline_notify_tasks` in the finally block whether the notification fired, the printer reconnected, or the task was cancelled mid-await. **No symmetric `on_printer_online` event:** the reporter explicitly noted the "printer lost power and interrupted the print" notification already fires when power is restored — that's the print-failure notification, triggered by the firmware reporting `gcode_state=FAILED` for the interrupted print on MQTT reconnect. That covers the "printer is back" channel without a new toggle. If the user then resumes the print, no print_start notification fires (Bambuddy's `bambu_mqtt.py:3039` explicitly suppresses `is_new_print` for PAUSE → RUNNING to prevent duplicates when resuming from pause), but that's a separate scope from offline-detection. **Tests:** 9 new cases in `test_printer_offline_notification.py` split across two classes. `TestMaybeNotifyPrinterOffline` pins the debounced helper: fires notification when still offline at end of window, doesn't fire when printer reconnected during debounce, doesn't fire when the printer disappeared from the DB (uninstall mid-window), clears `_printer_offline_notify_tasks[printer_id]` after run. `TestOfflineEdgeDetection` pins the edge logic inside `on_printer_status_change`: first observation (connected) doesn't schedule, first observation (disconnected) doesn't schedule (the no-prior-True case — important for startup), True → False schedules a task, reconnect cancels the pending task, repeated False observations don't replace the in-flight task. Full backend suite still green; ruff clean.
- **Virtual Printer queue mode: multi-plate "Send All" now enqueues one queue item per plate** — BambuStudio / OrcaSlicer's "Send All" packs every plate of the project into a SINGLE 3MF and uploads it with one FTP STOR — `slice_info.config` inside the file carries N `<plate>` blocks (one per plate), each with its own `<metadata key="index" value="N"/>` and its own `Metadata/plate_N.gcode` payload. Previously the VP queue path only ever extracted the FIRST plate's index via `_extract_plate_id` and created exactly ONE PrintQueueItem with that single `plate_id`; plates 2..N silently dropped on the floor. Indistinguishable from the user's perspective from "Send" of a single plate — except they expected 3 items in the queue and got 1, with no log line to explain why. **Confirmed against the wire** on the live H2D-1 Proxy VP: `Cube.gcode.3mf` carrying three `<plate>` blocks (indices 1, 2, 3) + three per-plate gcode payloads in the same zip, identical filename whether "Send" or "Send All" was clicked — the only signal of intent is the count of `<plate>` blocks inside the file. **Fix:** replaced `_extract_plate_id` (returning `int | None`) with `_extract_plate_ids` (returning `list[int]`). The list contains every `<plate>` block's `index` metadata, in order; falls back to `[1]` for files missing `slice_info.config` or with no parseable plates so the single-plate path is preserved. `_add_to_print_queue` now loops over the list — each iteration calls `extract_filament_requirements(file_path, plate_id)` per-plate (the plate-aware path was already there from the #1697 work) and creates a PrintQueueItem with that plate's filament types / overrides, plate-specific position = `MAX(position) + iteration`. Single-plate "Send" hits the loop once → exactly today's behaviour (one queue item, plate_id from the slicer, same archive). Multi-plate "Send All" of a 3-plate file → 3 queue items, plate_id 1/2/3, consecutive positions, all pointing at the same backing archive (one upload = one archive). **What stays the same:** the single archive row per upload (the archive backs the queue items via `archive_id`); the `auto_dispatch=False` / `manual_start=true` posture inherited from the VP config (so multi-plate items still require manual start); the `queue_force_color_match` per-VP toggle (now applies per-plate). **What this also fixed downstream:** the `required_filament_types` / `filament_overrides` JSON on each queue item now reflects THAT plate's filaments, not the file's first plate — so the scheduler's per-printer "Any X" matching dispatches each plate onto a printer with the right colours loaded for THAT plate, not for plate 1's filament set. **Tests:** 1 new regression case in `test_virtual_printer.py::TestVirtualPrinterInstance::test_add_to_print_queue_multi_plate_send_all_enqueues_one_per_plate` — builds a 3-plate 3MF (writes the per-plate `<plate>` blocks into `slice_info.config` and the per-plate gcode payloads), runs `_add_to_print_queue`, asserts 3 PrintQueueItems with `plate_id == [1, 2, 3]`, `position == [1, 2, 3]`, shared `archive_id`, all `manual_start=True`. 126 existing single-plate VP tests stay green (loop runs once when input has one plate). Full backend suite 5962/5962 green; ruff clean; frontend untouched. **Live-verified** on the H2D-1 Proxy VP — a Send All of the 3-plate Cube project now produces 3 queue items + 1 archive instead of 1 queue item + 1 archive.
@@ -420,4 +420,91 @@ describe('Layout', () => {
});
});
});
describe('Sidebar gate accepts granular read tiers (#1755)', () => {
// Default Operators group is seeded with `*:read_own` only — never the
// legacy `*:read`. Previously the sidebar gate checked the legacy alone,
// so Archives / Queue / Files were hidden from every non-admin even
// though the underlying API endpoints accepted their requests. These
// tests pin that the gate accepts ANY of the three tiers (legacy /
// _own / _all) for the three resources that ship granular variants.
const enableAuthWithUser = (permissions: string[]) => {
server.use(
http.get('/api/v1/auth/status', () =>
HttpResponse.json({ auth_enabled: true, requires_setup: false }),
),
http.get('/api/v1/auth/me', () =>
HttpResponse.json({
id: 1,
username: 'tester',
role: 'user',
is_active: true,
is_admin: false,
groups: [{ id: 2, name: 'Operators' }],
permissions,
created_at: '2026-01-01T00:00:00Z',
}),
),
);
window.localStorage.setItem('auth_token', 'test-token');
};
const sidebarLink = (href: string) =>
document.querySelector(`aside a[href="${href}"]`);
it('shows Files in the sidebar when the user only has library:read_own', async () => {
enableAuthWithUser(['library:read_own']);
render(<Layout />);
await waitFor(() => {
expect(document.querySelector('aside')).toBeInTheDocument();
expect(sidebarLink('/files')).toBeInTheDocument();
});
});
it('shows Files in the sidebar when the user only has library:read_all', async () => {
enableAuthWithUser(['library:read_all']);
render(<Layout />);
await waitFor(() => {
expect(sidebarLink('/files')).toBeInTheDocument();
});
});
it('shows Archives in the sidebar when the user only has archives:read_own', async () => {
enableAuthWithUser(['archives:read_own']);
render(<Layout />);
await waitFor(() => {
expect(sidebarLink('/archives')).toBeInTheDocument();
});
});
it('shows Queue in the sidebar when the user only has queue:read_own', async () => {
enableAuthWithUser(['queue:read_own']);
render(<Layout />);
await waitFor(() => {
expect(sidebarLink('/queue')).toBeInTheDocument();
});
});
it('still hides Files when the user has none of the three read tiers', async () => {
enableAuthWithUser(['printers:read']);
render(<Layout />);
await waitFor(() => {
expect(document.querySelector('aside')).toBeInTheDocument();
});
expect(sidebarLink('/files')).toBeNull();
expect(sidebarLink('/archives')).toBeNull();
expect(sidebarLink('/queue')).toBeNull();
});
});
});
+3 -3
View File
@@ -2898,13 +2898,13 @@ export interface ExternalLinkUpdate {
// Permission type - all available permissions
export type Permission =
| 'printers:read' | 'printers:create' | 'printers:update' | 'printers:delete' | 'printers:control' | 'printers:files' | 'printers:ams_rfid' | 'printers:clear_plate'
| 'archives:read' | 'archives:create'
| 'archives:read' | 'archives:read_own' | 'archives:read_all' | 'archives:create'
| 'archives:update_own' | 'archives:update_all' | 'archives:delete_own' | 'archives:delete_all'
| 'archives:reprint_own' | 'archives:reprint_all' | 'archives:purge'
| 'queue:read' | 'queue:create'
| 'queue:read' | 'queue:read_own' | 'queue:read_all' | 'queue:create'
| 'queue:update_own' | 'queue:update_all' | 'queue:delete_own' | 'queue:delete_all'
| 'queue:reorder'
| 'library:read' | 'library:upload'
| 'library:read' | 'library:read_own' | 'library:read_all' | 'library:upload'
| 'library:update_own' | 'library:update_all' | 'library:delete_own' | 'library:delete_all'
| 'library:purge'
| 'projects:read' | 'projects:create' | 'projects:update' | 'projects:delete'
+17 -6
View File
@@ -279,23 +279,34 @@ export function Layout() {
const result: string[] = [];
const seen = new Set<string>();
// Map nav item IDs to the permission required to see them
const navPermissions: Record<string, Permission> = {
archives: 'archives:read',
queue: 'queue:read',
// Map nav item IDs to the permission(s) required to see them. Resources
// that ship in three tiers (legacy `*:read` + granular `*:read_own` /
// `*:read_all`) list all three: the default Operators group is seeded
// with `_own` only, so gating on the legacy alone hides the entry from
// every non-admin user even though the underlying API accepts their
// request (#1755).
const navPermissions: Record<string, Permission | Permission[]> = {
archives: ['archives:read', 'archives:read_own', 'archives:read_all'],
queue: ['queue:read', 'queue:read_own', 'queue:read_all'],
stats: 'stats:read',
profiles: 'kprofiles:read',
maintenance: 'maintenance:read',
projects: 'projects:read',
inventory: 'inventory:read',
files: 'library:read',
files: ['library:read', 'library:read_own', 'library:read_all'],
makerworld: 'makerworld:view',
settings: 'settings:read',
notifications: 'notifications:user_email',
};
const isHidden = (id: string) => {
if (authEnabled && id in navPermissions && !hasPermission(navPermissions[id])) return true;
if (authEnabled && id in navPermissions) {
const required = navPermissions[id];
const granted = Array.isArray(required)
? required.some((p) => hasPermission(p))
: hasPermission(required);
if (!granted) return true;
}
// notifications nav item also requires advanced auth to be enabled and user_notifications_enabled setting
if (id === 'notifications' && (!authEnabled || !advancedAuthStatus?.advanced_auth_enabled || (settings?.user_notifications_enabled === false))) return true;
return false;
File diff suppressed because one or more lines are too long
+1 -1
View File
@@ -26,7 +26,7 @@
<!-- Splash screens for iOS -->
<link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
<script type="module" crossorigin src="/assets/index-DiPrOrl_.js"></script>
<script type="module" crossorigin src="/assets/index-BMUh7cW4.js"></script>
<link rel="stylesheet" crossorigin href="/assets/index-45eedLWT.css">
</head>
<body>