Miscellaneous fixes (#24352)

* fix recordings unavailable endpoint when no params are provided

we already import the datetime class directly, so those attribute lookups raised AttributeError and the request returned 500

* reject JWTs whose role is no longer in the config

`/auth` trusted the role inside the JWT and re-signed it on refresh without checking the config, so a user whose restricted role was deleted kept a session carrying a role that isn't in `auth.roles`. The media, clip, recording, export and go2rtc checks treat a missing role the same as a role with no camera list, so that session could open every camera. `/auth` now returns 401 for a token whose role isn't configured, which sends the user back through login, and login already falls back to `viewer` for a role that's gone.

* delete the deleted camera group's layout, not the open one's

Deleting a camera group removed the layout of the group being viewed, because the dialog's delete was bound to `${activeGroup}-draggable-layout`. It also cleared the saved group, and both ran before the `config/set` request whether or not it succeeded, so deleting one group while viewing another lost the open group's layout and left the deleted group's layout behind. The deleted group's own layout is now removed after a successful save, and the saved group is only cleared when it was the open group.

* don't block API when querying PTZ info

camera_ptz_info is async but waited on the ONVIF controller's future with future.result(), blocking the API event loop for as long as a slow or unreachable ONVIF camera took to answer (including reconnect attempts), so every other async request stalled with it. Await the future with asyncio.wrap_future instead. The coroutine runs on the controller's own loop and thread, so this cannot deadlock.

* for custom exports, only allow admin users to add to existing cases

follows the existing convention where attaching an export to an existing case is admin-only on `POST /export/{camera}/...` and `POST /exports/batch`

* drop pending edits for a camera or profile that no longer exists

* match cached preview frames to their camera exactly

https://github.com/blakeblackshear/frigate/pull/22594 added a trailing `-` to the `preview_{camera}` prefix so `camera` stopped matching `camera2`'s frames, but camera names can contain `-`, so `front` still matched `front-door`'s. After a restart, `front`'s preview recorder deleted this hour's frames of a matching camera that sorted before it and added the timestamps of one that sorted after it, so ffmpeg was asked for files that don't exist and that hour's preview was lost. The offline fallback for `latest.jpg` could also return `front-door`'s frame for `front`, even to a user without access to `front-door`, and a short export could take its fallback thumbnail from the other camera. These now compare the full camera name taken from the file name.

frigate/api/auth.py

@@ -786,6 +786,13 @@ def auth(request: Request):
 
         user = token.claims.get("sub")
         role = token.claims.get("role")
+
+        # the token keeps the role it was issued with, so a role removed from
+        # the config since then must send the user back through login
+        if role not in auth_config.roles:
+            logger.debug("jwt role %s is not in the config", role)
+            return fail_response
+
         current_time = int(time.time())
 
         # if the jwt is expired

frigate/api/export.py

@@ -1038,6 +1038,10 @@ def export_recording_custom(
     if camera_validation_error is not None:
         return camera_validation_error
 
+    # Validate user-provided ffmpeg args to prevent injection and add to cases.
+    # Admin users are trusted and skip validation.
+    is_admin = request.headers.get("remote-role", "") == "admin"
+
     playback_source = body.source
     friendly_name = body.name
     existing_image, image_validation_error = _sanitize_existing_image(body.image_path)
@@ -1048,6 +1052,16 @@ def export_recording_custom(
     cpu_fallback = body.cpu_fallback
 
     export_case_id = body.export_case_id
+
+    if export_case_id is not None and not is_admin:
+        return JSONResponse(
+            content={
+                "success": False,
+                "message": "Only admins can attach exports to an existing case.",
+            },
+            status_code=403,
+        )
+
     case_validation_error = _validate_export_case(export_case_id)
     if case_validation_error is not None:
         return case_validation_error
@@ -1064,10 +1078,6 @@ def export_recording_custom(
             status_code=400,
         )
 
-    # Validate user-provided ffmpeg args to prevent injection.
-    # Admin users are trusted and skip validation.
-    is_admin = request.headers.get("remote-role", "") == "admin"
-
     if not is_admin:
         for args_label, args_value in [
             ("input", ffmpeg_input_args),

frigate/api/media.py

@@ -189,7 +189,7 @@ async def camera_ptz_info(request: Request, camera_name: str):
         future = asyncio.run_coroutine_threadsafe(
             request.app.onvif.get_camera_info(camera_name), request.app.onvif.loop
         )
-        result = future.result()
+        result = await asyncio.wrap_future(future)
         return JSONResponse(content=result)
     else:
         return JSONResponse(

frigate/api/record.py

@@ -412,11 +412,8 @@ async def no_recordings(
     if not camera_list:
         return JSONResponse(content=[])
 
-    before = params.before or datetime.datetime.now().timestamp()
-    after = (
-        params.after
-        or (datetime.datetime.now() - datetime.timedelta(hours=1)).timestamp()
-    )
+    before = params.before or datetime.now().timestamp()
+    after = params.after or (datetime.now() - timedelta(hours=1)).timestamp()
     scale = params.scale
 
     recordings: list[tuple[float, float]] = []

frigate/output/preview.py

@@ -66,6 +66,12 @@ def get_cache_image_name(camera: str, frame_time: float) -> str:
     )
 
 
+def is_camera_preview_frame(file_name: str, camera: str) -> bool:
+    """Check whether a cached preview frame file belongs to the camera."""
+    # camera names may contain "-", so a prefix match would include "front-door"
+    return file_name.rsplit("-", 1)[0] == f"preview_{camera}"
+
+
 def get_most_recent_preview_frame(
     camera: str, before: float | None = None
 ) -> str | None:
@@ -79,7 +85,7 @@ def get_most_recent_preview_frame(
         preview_files = [
             f
             for f in os.listdir(PREVIEW_CACHE_DIR)
-            if f.startswith(f"preview_{camera}-")
+            if is_camera_preview_frame(f, camera)
             and f.endswith(f".{PREVIEW_FRAME_TYPE}")
         ]
 
@@ -276,7 +282,7 @@ class PreviewRecorder:
         start_file = f"{file_start}{start_ts}.webp"
 
         for file in sorted(os.listdir(os.path.join(CACHE_DIR, FOLDER_PREVIEW_FRAMES))):
-            if not file.startswith(file_start):
+            if not is_camera_preview_frame(file, self.camera_name):
                 continue
 
             if file < start_file:

frigate/record/export.py

@@ -36,6 +36,7 @@ from frigate.ffmpeg_presets import (
     parse_preset_hardware_acceleration_encode,
 )
 from frigate.models import Export, Previews, Recordings, ReviewSegment
+from frigate.output.preview import is_camera_preview_frame
 from frigate.util.ffmpeg import run_ffmpeg_with_progress
 from frigate.util.ownership import chown_to_runtime
 from frigate.util.recording_coverage import (
@@ -1098,7 +1099,7 @@ class RecordingExporter(threading.Thread):
             fallback_preview = None
 
             for file in sorted(os.listdir(preview_dir)):
-                if not file.startswith(file_start):
+                if not is_camera_preview_frame(file, self.camera):
                     continue
 
                 if file < start_file:

frigate/test/http_api/test_http_auth_jwt_role.py

@@ -0,0 +1,81 @@
+"""Tests that /auth only accepts a JWT whose role is still in the config."""
+
+import os
+import time
+from unittest.mock import MagicMock, Mock, patch
+
+from frigate.api.auth import create_encoded_jwt
+from frigate.api.fastapi_app import create_fastapi_app
+from frigate.config import FrigateConfig
+from frigate.config.camera.updater import CameraConfigUpdatePublisher
+from frigate.const import JWT_SECRET_ENV_VAR
+from frigate.models import Event, Recordings, ReviewSegment
+from frigate.test.http_api.base_http_test import AuthTestClient, BaseTestHttp
+
+
+@patch.dict(os.environ, {JWT_SECRET_ENV_VAR: "test-secret"})
+class TestAuthJwtRole(BaseTestHttp):
+    def setUp(self):
+        super().setUp(models=[Event, Recordings, ReviewSegment])
+        self.minimal_config = {
+            "mqtt": {"host": "mqtt"},
+            "auth": {"enabled": True, "roles": {"garage": ["front_door"]}},
+            "networking": {"listen": {"internal": 5000, "external": 8971}},
+            "cameras": {
+                "front_door": {
+                    "ffmpeg": {
+                        "inputs": [
+                            {"path": "rtsp://10.0.0.1:554/video", "roles": ["detect"]}
+                        ]
+                    },
+                    "detect": {
+                        "height": 1080,
+                        "width": 1920,
+                        "fps": 5,
+                    },
+                }
+            },
+        }
+
+    def _create_app(self):
+        mock_publisher = Mock(spec=CameraConfigUpdatePublisher)
+        mock_publisher.publisher = MagicMock()
+
+        return create_fastapi_app(
+            FrigateConfig(**self.minimal_config),
+            self.db,
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+            mock_publisher,
+            None,
+            enforce_default_admin=False,
+        )
+
+    def _auth(self, app, role: str):
+        token = create_encoded_jwt("bob", role, int(time.time()) + 3600, app.jwt_token)
+
+        with AuthTestClient(app) as client:
+            return client.get(
+                "/auth",
+                headers={
+                    "x-server-port": "8971",
+                    "authorization": f"Bearer {token}",
+                },
+            )
+
+    def test_configured_role_is_accepted(self):
+        resp = self._auth(self._create_app(), "garage")
+
+        self.assertEqual(resp.status_code, 202)
+        self.assertEqual(resp.headers["remote-user"], "bob")
+        self.assertEqual(resp.headers["remote-role"], "garage")
+
+    def test_role_removed_from_config_is_rejected(self):
+        resp = self._auth(self._create_app(), "removed_role")
+
+        self.assertEqual(resp.status_code, 401)
+        self.assertNotIn("remote-role", resp.headers)

frigate/test/test_preview_loader.py

@@ -75,6 +75,21 @@ class TestPreviewLoader(unittest.TestCase):
 
         self.assertIsNone(get_most_recent_preview_frame(camera))
 
+    def test_get_most_recent_preview_frame_hyphenated_camera(self):
+        for name in ("preview_front-2000.0", "preview_front-door-3000.0"):
+            with open(
+                os.path.join(PREVIEW_CACHE_DIR, f"{name}.{PREVIEW_FRAME_TYPE}"), "w"
+            ) as f:
+                f.write("test")
+
+        expected_path = os.path.join(
+            PREVIEW_CACHE_DIR, f"preview_front-2000.0.{PREVIEW_FRAME_TYPE}"
+        )
+        self.assertEqual(get_most_recent_preview_frame("front"), expected_path)
+        self.assertEqual(
+            get_most_recent_preview_frame("front", before=5000.0), expected_path
+        )
+
     def test_get_most_recent_preview_frame_no_directory(self):
         shutil.rmtree(PREVIEW_CACHE_DIR)
         self.assertIsNone(get_most_recent_preview_frame("test_camera"))

web/src/components/filter/CameraGroupSelector.tsx

@@ -10,6 +10,7 @@ import { MdHome } from "react-icons/md";
 import { Button, buttonVariants } from "../ui/button";
 import {
   useCallback,
+  useContext,
   useEffect,
   useLayoutEffect,
   useMemo,
@@ -66,7 +67,8 @@ import { z } from "zod";
 import { Toaster } from "@/components/ui/sonner";
 import { toast } from "sonner";
 import ActivityIndicator from "../indicators/activity-indicator";
-import { useUserPersistence } from "@/hooks/use-user-persistence";
+import { deleteUserNamespacedKey } from "@/hooks/use-user-persistence";
+import { AuthContext } from "@/context/auth-context";
 import { TooltipPortal } from "@radix-ui/react-tooltip";
 import { cn } from "@/lib/utils";
 import * as LuIcons from "react-icons/lu";
@@ -499,9 +501,8 @@ function NewGroupDialog({
   const [editState, setEditState] = useState<"none" | "add" | "edit">("none");
   const [isLoading, setIsLoading] = useState(false);
 
-  const [, , , deleteGridLayout] = useUserPersistence(
-    `${activeGroup}-draggable-layout`,
-  );
+  const { auth } = useContext(AuthContext);
+  const username = auth?.user?.username;
 
   useEffect(() => {
     if (!open) {
@@ -513,15 +514,14 @@ function NewGroupDialog({
 
   const onDeleteGroup = useCallback(
     async (name: string) => {
-      deleteGridLayout();
-      deleteGroup();
-
       await axios
         .put(`config/set?camera_groups.${name}`, { requires_restart: 0 })
         .then((res) => {
           if (res.status === 200) {
+            deleteUserNamespacedKey(`${name}-draggable-layout`, username);
             if (activeGroup == name) {
               // deleting current group
+              deleteGroup();
               setGroup("default");
             }
             updateConfig();
@@ -557,15 +557,7 @@ function NewGroupDialog({
           setIsLoading(false);
         });
     },
-    [
-      updateConfig,
-      activeGroup,
-      setGroup,
-      setOpen,
-      deleteGroup,
-      deleteGridLayout,
-      t,
-    ],
+    [updateConfig, activeGroup, setGroup, setOpen, deleteGroup, username, t],
   );
 
   const onSave = () => {

web/src/pages/Settings.tsx

@@ -540,6 +540,35 @@ export default function Settings() {
   const [editingProfile, setEditingProfile] = useState<
     Record<string, string | null>
   >({});
+
+  // drop pending edits for a camera or profile that no longer exists
+  useEffect(() => {
+    if (!config) return;
+    const isStale = (camera: string, profile?: string | null) =>
+      !config.cameras[camera] || (!!profile && !config.profiles?.[profile]);
+    const prune = <T,>(
+      prev: Record<string, T>,
+      stale: (key: string, value: T) => boolean,
+    ) => {
+      const next = Object.fromEntries(
+        Object.entries(prev).filter(([key, value]) => !stale(key, value)),
+      );
+      return Object.keys(next).length === Object.keys(prev).length
+        ? prev
+        : next;
+    };
+    setEditingProfile((prev) => prune(prev, isStale));
+    setPendingDataBySection((prev) =>
+      prune(prev, (key) => {
+        const [camera, section] = key.split("::");
+        return (
+          section !== undefined &&
+          isStale(camera, parseProfileFromSectionPath(section).profileName)
+        );
+      }),
+    );
+  }, [config]);
+
   const [profilesUIEnabled, setProfilesUIEnabled] = useState(false);
 
   const allProfileNames = useMemo(() => {