Browse Source

fix(settings): expose UI rendering fields without requiring SETTINGS_READ (#1293)

  The Clear Plate button (and 4 other features on the Printers page) read
  their state from /settings, which requires SETTINGS_READ. Granting that
  permission also adds the Settings nav item and leaks SMTP/LDAP/MQTT
  credentials — exactly what users were trying to avoid by giving an
  operator only printers:clear_plate.

  New /settings/ui-preferences endpoint returns a curated, opt-in subset
  of non-sensitive fields. Matches the existing /default-sidebar-order
  precedent. PrintersPage switched to the new endpoint; admin pages still
  use /settings for full access.
maziggy 2 weeks ago
parent
commit
8a6fcf5cbd

File diff suppressed because it is too large
+ 0 - 0
CHANGELOG.md


+ 38 - 0
backend/app/api/routes/settings.py

@@ -261,6 +261,44 @@ async def get_default_sidebar_order(
     return {"default_sidebar_order": value or ""}
     return {"default_sidebar_order": value or ""}
 
 
 
 
+# Fields exposed via /ui-preferences without SETTINGS_READ. Each entry MUST be
+# non-sensitive (no credentials, no PII, no secret tokens) — granting SETTINGS_READ
+# also grants visibility of SMTP/LDAP/MQTT passwords and similar, so the goal of
+# this endpoint is exactly to NOT require that permission for UI rendering hints.
+# When adding a field here, confirm it doesn't carry anything sensitive.
+_UI_PREFERENCE_FIELDS: tuple[str, ...] = (
+    "require_plate_clear",
+    "check_printer_firmware",
+    "camera_view_mode",
+    "time_format",
+    "date_format",
+    "drying_presets",
+    "ams_humidity_good",
+    "ams_humidity_fair",
+    "ams_temp_good",
+    "ams_temp_fair",
+    "bed_cooled_threshold",
+)
+
+
+@router.get("/ui-preferences")
+async def get_ui_preferences(db: AsyncSession = Depends(get_db)):
+    """Get the curated subset of settings that any page needs to render correctly.
+
+    Intentionally not gated on SETTINGS_READ — every authenticated user (and
+    every page that loads for them) needs these fields, but granting SETTINGS_READ
+    would also grant visibility of secrets (SMTP/LDAP/MQTT credentials, etc.).
+    Same pattern as /default-sidebar-order (#1293).
+
+    Reuses _build_settings_response so the typed values match what /settings
+    returns for fields with the same name — bool/int/float/str types stay in
+    sync without a separate type-coercion path.
+    """
+    full = await _build_settings_response(db, is_api_key=False)
+    dumped = full.model_dump()
+    return {key: dumped[key] for key in _UI_PREFERENCE_FIELDS if key in dumped}
+
+
 @router.get("/check-ffmpeg")
 @router.get("/check-ffmpeg")
 async def check_ffmpeg():
 async def check_ffmpeg():
     """Check if ffmpeg is installed and available."""
     """Check if ffmpeg is installed and available."""

+ 119 - 0
backend/tests/integration/test_settings_ui_preferences.py

@@ -0,0 +1,119 @@
+"""Tests for the public /settings/ui-preferences endpoint (#1293).
+
+Reporter @Tivonfeng: granting `printers:clear_plate` alone wasn't enough to
+make the Clear Plate button work — the frontend also needs `require_plate_clear`
+from /settings, which requires SETTINGS_READ (and also surfaces SMTP/LDAP/MQTT
+secrets). The fix is a public subset endpoint that returns only UI rendering
+fields, so the frontend doesn't have to demand SETTINGS_READ for non-admin UX.
+
+The two guarantees pinned here:
+1. The endpoint is accessible without SETTINGS_READ.
+2. The endpoint NEVER returns sensitive fields (SMTP/LDAP/MQTT credentials,
+   API tokens, HA bearer token, etc.) — even if a future commit accidentally
+   adds one of those keys to _UI_PREFERENCE_FIELDS, this test fails loudly.
+"""
+
+import pytest
+from httpx import AsyncClient
+
+# Anything in this list MUST NOT appear in the /ui-preferences response.
+# Mirror of _SENSITIVE_FIELDS_FOR_API_KEY in backend/app/api/routes/settings.py
+# plus a wider net for any *_password / *_token / *_key suffix.
+_SENSITIVE_KEYS = {
+    "smtp_password",
+    "smtp_username",
+    "smtp_from_email",
+    "smtp_host",
+    "smtp_port",
+    "mqtt_password",
+    "mqtt_username",
+    "mqtt_broker",
+    "ha_token",
+    "ha_url",
+    "prometheus_token",
+    "virtual_printer_access_code",
+    "ldap_bind_password",
+    "ldap_bind_dn",
+    "ldap_server_url",
+    "external_url",
+    "bambu_studio_api_url",
+    "orcaslicer_api_url",
+    "local_backup_path",
+    "github_token",
+    "gitea_token",
+    "obico_api_key",
+    "obico_endpoint_url",
+}
+
+
+class TestUiPreferencesEndpoint:
+    """The new public endpoint must work without SETTINGS_READ and must
+    never return sensitive fields."""
+
+    @pytest.mark.asyncio
+    async def test_endpoint_returns_200_without_auth(self, async_client: AsyncClient):
+        """No SETTINGS_READ required — that's the whole point of the endpoint."""
+        response = await async_client.get("/api/v1/settings/ui-preferences")
+        assert response.status_code == 200
+        data = response.json()
+        assert isinstance(data, dict)
+
+    @pytest.mark.asyncio
+    async def test_returns_require_plate_clear(self, async_client: AsyncClient):
+        """The field that drove #1293: PrintersPage gates the Clear Plate button
+        on this. Must be present in the response."""
+        response = await async_client.get("/api/v1/settings/ui-preferences")
+        assert response.status_code == 200
+        data = response.json()
+        assert "require_plate_clear" in data
+        # Type must be bool (frontend does === true checks)
+        assert isinstance(data["require_plate_clear"], bool)
+
+    @pytest.mark.asyncio
+    async def test_returns_expected_field_set(self, async_client: AsyncClient):
+        """Pin the exact set of fields the endpoint exposes — adding a sensitive
+        field to _UI_PREFERENCE_FIELDS by accident should fail this assert and
+        force the author to reconsider."""
+        response = await async_client.get("/api/v1/settings/ui-preferences")
+        data = response.json()
+        expected = {
+            "require_plate_clear",
+            "check_printer_firmware",
+            "camera_view_mode",
+            "time_format",
+            "date_format",
+            "drying_presets",
+            "ams_humidity_good",
+            "ams_humidity_fair",
+            "ams_temp_good",
+            "ams_temp_fair",
+            "bed_cooled_threshold",
+        }
+        assert set(data.keys()) == expected
+
+    @pytest.mark.asyncio
+    async def test_response_excludes_sensitive_fields(self, async_client: AsyncClient, db_session):
+        """Even with sensitive fields seeded in the DB, none of them must
+        appear in the response — the endpoint is opt-in, not opt-out."""
+        from backend.app.models.settings import Settings
+
+        # Seed every sensitive field with a unique recognizable value so a leak
+        # would be obvious in failure output.
+        for i, key in enumerate(_SENSITIVE_KEYS):
+            db_session.add(Settings(key=key, value=f"SECRET_VALUE_{i}_DO_NOT_LEAK"))
+        await db_session.commit()
+
+        response = await async_client.get("/api/v1/settings/ui-preferences")
+        assert response.status_code == 200
+        data = response.json()
+
+        # No sensitive key should appear in the response keys
+        leaked_keys = _SENSITIVE_KEYS & set(data.keys())
+        assert leaked_keys == set(), f"Leaked sensitive fields: {leaked_keys}"
+
+        # And the recognizable values shouldn't appear in any value either
+        response_text = response.text
+        for i in range(len(_SENSITIVE_KEYS)):
+            assert f"SECRET_VALUE_{i}_DO_NOT_LEAK" not in response_text, (
+                f"Sensitive value index {i} leaked into response body"
+            )

+ 20 - 0
frontend/src/__tests__/pages/PrintersPage.test.tsx

@@ -96,6 +96,17 @@ describe('PrintersPage', () => {
           require_plate_clear: true,
           require_plate_clear: true,
         });
         });
       }),
       }),
+      // PrintersPage now reads UI rendering fields from the public ui-preferences
+      // endpoint instead of /settings (#1293) — admin pages still hit /settings.
+      http.get('/api/v1/settings/ui-preferences', () => {
+        return HttpResponse.json({
+          ams_humidity_good: 40,
+          ams_humidity_fair: 60,
+          ams_temp_good: 30,
+          ams_temp_fair: 35,
+          require_plate_clear: true,
+        });
+      }),
       http.get('/api/v1/queue/', () => {
       http.get('/api/v1/queue/', () => {
         return HttpResponse.json([]);
         return HttpResponse.json([]);
       })
       })
@@ -360,6 +371,15 @@ describe('PrintersPage', () => {
             require_plate_clear: false,
             require_plate_clear: false,
           });
           });
         }),
         }),
+        http.get('/api/v1/settings/ui-preferences', () => {
+          return HttpResponse.json({
+            ams_humidity_good: 40,
+            ams_humidity_fair: 60,
+            ams_temp_good: 30,
+            ams_temp_fair: 35,
+            require_plate_clear: false,
+          });
+        }),
         http.get('/api/v1/printers/:id/status', () => {
         http.get('/api/v1/printers/:id/status', () => {
           return HttpResponse.json({ ...mockPrinterStatus, state: 'FINISH', awaiting_plate_clear: true });
           return HttpResponse.json({ ...mockPrinterStatus, state: 'FINISH', awaiting_plate_clear: true });
         })
         })

+ 18 - 0
frontend/src/api/client.ts

@@ -3990,6 +3990,24 @@ export const api = {
   // Settings
   // Settings
   getSettings: () => request<AppSettings>('/settings/'),
   getSettings: () => request<AppSettings>('/settings/'),
   getDefaultSidebarOrder: () => request<{ default_sidebar_order: string }>('/settings/default-sidebar-order'),
   getDefaultSidebarOrder: () => request<{ default_sidebar_order: string }>('/settings/default-sidebar-order'),
+  // Public subset of settings for UI rendering — no settings:read required.
+  // Used by pages whose users may not have SETTINGS_READ (e.g. operators with
+  // only printers:clear_plate). Keep in sync with _UI_PREFERENCE_FIELDS in
+  // backend/app/api/routes/settings.py.
+  getUiPreferences: () =>
+    request<{
+      require_plate_clear?: boolean;
+      check_printer_firmware?: boolean;
+      camera_view_mode?: 'window' | 'embedded';
+      time_format?: 'system' | '12h' | '24h';
+      date_format?: string;
+      drying_presets?: string;
+      ams_humidity_good?: number;
+      ams_humidity_fair?: number;
+      ams_temp_good?: number;
+      ams_temp_fair?: number;
+      bed_cooled_threshold?: number;
+    }>('/settings/ui-preferences'),
   updateSettings: (data: AppSettingsUpdate) =>
   updateSettings: (data: AppSettingsUpdate) =>
     request<AppSettings>('/settings/', {
     request<AppSettings>('/settings/', {
       method: 'PUT',
       method: 'PUT',

+ 5 - 3
frontend/src/pages/PrintersPage.tsx

@@ -6455,10 +6455,12 @@ export function PrintersPage() {
     queryFn: api.getPrinters,
     queryFn: api.getPrinters,
   });
   });
 
 
-  // Fetch app settings for AMS thresholds
+  // Fetch the UI-rendering subset of settings. Uses /ui-preferences (not /settings)
+  // so users with printers:read but no settings:read still get the values needed
+  // to render the clear-plate button, drying presets, AMS thresholds, etc. (#1293).
   const { data: settings } = useQuery({
   const { data: settings } = useQuery({
-    queryKey: ['settings'],
-    queryFn: api.getSettings,
+    queryKey: ['ui-preferences'],
+    queryFn: api.getUiPreferences,
   });
   });
 
 
   // Compute drying presets: user-configured (from settings) merged over built-in defaults
   // Compute drying presets: user-configured (from settings) merged over built-in defaults

File diff suppressed because it is too large
+ 0 - 0
static/assets/index-CcnHDYu5.js


+ 1 - 1
static/index.html

@@ -26,7 +26,7 @@
 
 
     <!-- Splash screens for iOS -->
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-DPn2Wuq0.js"></script>
+    <script type="module" crossorigin src="/assets/index-CcnHDYu5.js"></script>
     <link rel="stylesheet" crossorigin href="/assets/index-BkYu3kLs.css">
     <link rel="stylesheet" crossorigin href="/assets/index-BkYu3kLs.css">
   </head>
   </head>
   <body>
   <body>

Some files were not shown because too many files changed in this diff