Fix review summary report analysis creation to be scoped for users with full camera access only (#24056)

* Fix review summary analysis

* Add ability to scope based on full camera access

docs/static/frigate-api.yaml

@@ -2316,7 +2316,7 @@ paths:
         - Review
       summary: Generate Review Summary
       description: |-
-        **Access:** Admin role required.
+        **Access:** Authenticated user with access to all cameras.
 
         Use GenAI to summarize review items over a period of time.
       operationId: 
@@ -2347,8 +2347,8 @@ paths:
               schema:
                 $ref: '#/components/schemas/HTTPValidationError'
       security:
-        - frigateAdminAuth: []
-      x-required-role: admin
+        - frigateUserAuth: []
+      x-required-role: all_cameras
   /:
     get:
       tags:

frigate/api/auth.py

@@ -1251,3 +1251,23 @@ async def get_allowed_cameras_for_filter(request: Request):
     all_camera_names = set(request.app.frigate_config.cameras.keys())
     roles_dict = request.app.frigate_config.auth.roles
     return User.get_allowed_cameras(role, roles_dict, all_camera_names)
+
+
+async def require_full_camera_access(
+    request: Request,
+    allowed_cameras: list[str] = Depends(get_allowed_cameras_for_filter),
+):
+    """Dependency for endpoints returning data that spans every camera.
+
+    Some responses cannot be meaningfully scoped to a subset of cameras, so
+    rather than filter them the endpoint is limited to callers who can already
+    see every camera. Admin and viewer always qualify; a custom role qualifies
+    only when its camera list covers all configured cameras.
+    """
+    all_camera_names = set(request.app.frigate_config.cameras.keys())
+
+    if not all_camera_names.issubset(allowed_cameras):
+        raise HTTPException(
+            status_code=403,
+            detail="Access to all cameras is required for this endpoint",
+        )

frigate/api/review.py

@@ -17,6 +17,7 @@ from frigate.api.auth import (
     get_allowed_cameras_for_filter,
     get_current_user,
     require_camera_access,
+    require_full_camera_access,
     require_role,
 )
 from frigate.api.defs.query.review_query_parameters import (
@@ -743,9 +744,12 @@ async def set_not_reviewed(
     )
 
 
+# Intentionally not camera scoped, as the summary correlates each flagged event
+# with overlapping activity on other cameras. Restricted to callers who can
+# already see every camera, so the unscoped query discloses nothing.
 @router.post(
     "/review/summarize/start/{start_ts}/end/{end_ts}",
-    dependencies=[Depends(require_role(["admin"]))],
+    dependencies=[Depends(require_full_camera_access)],
     description="Use GenAI to summarize review items over a period of time.",
 )
 def generate_review_summary(request: Request, start_ts: float, end_ts: float):

frigate/test/http_api/test_http_camera_access.py

@@ -440,3 +440,68 @@ class TestGo2rtcStreamAccess(BaseTestHttp):
             f"limited_user should be denied on alias back_door_main; "
             f"got {resp.status_code}"
         )
+
+
+class TestReviewSummaryAccess(BaseTestHttp):
+    """Tests for POST /review/summarize/start/{start_ts}/end/{end_ts}.
+
+    The summary correlates each flagged event with overlapping activity on
+    other cameras, so it is gated on full camera access rather than scoped to
+    the caller's cameras. These tests pin that decision so the dependency is
+    not loosened without first scoping the query.
+
+    GenAI is not configured in unit tests, so an authorized request returns 400
+    while an unauthorized one is rejected with 403 before the handler runs.
+    """
+
+    def setUp(self):
+        super().setUp([Event, ReviewSegment, Recordings])
+        self.minimal_config = _MULTI_CAMERA_CONFIG
+        self.app = super().create_app()
+
+    def tearDown(self):
+        self.app.dependency_overrides.clear()
+        super().tearDown()
+
+    def _summarize(self, allowed_cameras: list[str]):
+        async def mock_cameras(request: Request):
+            return allowed_cameras
+
+        self.app.dependency_overrides[get_allowed_cameras_for_filter] = mock_cameras
+        with AuthTestClient(self.app) as client:
+            return client.post("/review/summarize/start/0/end/9999999999")
+
+    def _assert_allowed(self, resp):
+        assert resp.status_code not in (401, 403), (
+            f"Caller should not be blocked; got {resp.status_code}"
+        )
+
+    def test_partial_camera_access_blocked(self):
+        assert self._summarize(["front_door"]).status_code == 403
+
+    def test_no_camera_access_blocked(self):
+        assert self._summarize([]).status_code == 403
+
+    def test_full_camera_access_allowed(self):
+        # Covers admin and viewer, which always resolve to every camera, and a
+        # custom role whose list happens to name them all.
+        self._assert_allowed(self._summarize(["front_door", "back_door"]))
+
+    def _summarize_as_role(self, role: str):
+        """Summarize using the real role to allowed-cameras resolution."""
+        self.app.dependency_overrides.pop(get_allowed_cameras_for_filter, None)
+        with AuthTestClient(self.app) as client:
+            return client.post(
+                "/review/summarize/start/0/end/9999999999",
+                headers={"remote-user": "test", "remote-role": role},
+            )
+
+    def test_viewer_role_allowed(self):
+        # viewer is never camera restricted, so it resolves to every camera.
+        self._assert_allowed(self._summarize_as_role("viewer"))
+
+    def test_admin_role_allowed(self):
+        self._assert_allowed(self._summarize_as_role("admin"))
+
+    def test_restricted_role_blocked(self):
+        assert self._summarize_as_role("limited_user").status_code == 403

generate_api_auth_spec.py

@@ -94,6 +94,7 @@ SPEC_SERVERS = [
 PUBLIC = "public"
 AUTHENTICATED = "any"
 CAMERA = "camera"
+ALL_CAMERAS = "all_cameras"
 ADMIN = "admin"
 
 ADMIN_SCHEME = "frigateAdminAuth"
@@ -128,6 +129,7 @@ ACCESS_NOTES = {
     PUBLIC: "**Access:** Public — no authentication required.",
     AUTHENTICATED: "**Access:** Any authenticated user.",
     CAMERA: "**Access:** Authenticated user with access to the referenced camera.",
+    ALL_CAMERAS: "**Access:** Authenticated user with access to all cameras.",
     ADMIN: "**Access:** Admin role required.",
 }
 
@@ -197,6 +199,8 @@ def _route_markers(route: APIRoute) -> tuple[set[str], list[str] | None]:
                 pass
         elif name in ("require_camera_access", "require_go2rtc_stream_access"):
             markers.add(CAMERA)
+        elif name == "require_full_camera_access":
+            markers.add(ALL_CAMERAS)
         elif "auth_checker" in qualname:
             markers.add(AUTHENTICATED)
         elif "public_checker" in qualname:
@@ -254,6 +258,8 @@ def _classify_base(
     # Explicit route-level markers win, in order of specificity.
     if ADMIN in markers:
         return ADMIN, admin_roles or ["admin"], None
+    if ALL_CAMERAS in markers:
+        return ALL_CAMERAS, None, None
     if CAMERA in markers:
         return CAMERA, None, None
     if AUTHENTICATED in markers:
@@ -337,8 +343,8 @@ def security_for(level: str) -> list:
         return []
     if level == ADMIN:
         return [{ADMIN_SCHEME: []}]
-    # AUTHENTICATED and CAMERA both require any authenticated session; the
-    # camera-specific scoping is conveyed in the note and x-required-role.
+    # AUTHENTICATED, CAMERA and ALL_CAMERAS all require any authenticated
+    # session; the camera scoping is conveyed in the note and x-required-role.
     return [{USER_SCHEME: []}]