Fixes (#18897)

* Fix showing review items that span over multiple days

* Simplify

* Fix tests

* Fix unchanged value

* Allow admin as default role and viewer as passed header for proxy auth

---------

Co-authored-by: Josh Hawkins <32435876+hawkeye217@users.noreply.github.com>

frigate/api/auth.py

@@ -33,6 +33,7 @@ from frigate.models import User
 logger = logging.getLogger(__name__)
 
 router = APIRouter(tags=[Tags.auth])
+VALID_ROLES = ["admin", "viewer"]
 
 
 class RateLimiter:
@@ -272,12 +273,13 @@ def auth(request: Request):
             else proxy_config.default_role
         )
 
-        # if comma-separated with "admin", use "admin", else use default role
-        success_response.headers["remote-role"] = (
-            "admin"
-            if role
-            and "admin" in [r.strip() for r in role.split(proxy_config.separator)]
-            else proxy_config.default_role
+        # if comma-separated with "admin", use "admin",
+        # if comma-separated with "viewer", use "viewer",
+        # else use default role
+
+        roles = [r.strip() for r in role.split(proxy_config.separator)] if role else []
+        success_response.headers["remote-role"] = next(
+            (r for r in VALID_ROLES if r in roles), proxy_config.default_role
         )
 
         return success_response
@@ -402,7 +404,7 @@ def login(request: Request, body: AppPostLoginBody):
     password_hash = db_user.password_hash
     if verify_password(password, password_hash):
         role = getattr(db_user, "role", "viewer")
-        if role not in ["admin", "viewer"]:
+        if role not in VALID_ROLES:
             role = "viewer"  # Enforce valid roles
         expiration = int(time.time()) + JWT_SESSION_LENGTH
         encoded_jwt = create_encoded_jwt(user, role, expiration, request.app.jwt_token)
@@ -432,7 +434,7 @@ def create_user(
     if not re.match("^[A-Za-z0-9._]+$", body.username):
         return JSONResponse(content={"message": "Invalid username"}, status_code=400)
 
-    role = body.role if body.role in ["admin", "viewer"] else "viewer"
+    role = body.role if body.role in VALID_ROLES else "viewer"
     password_hash = hash_password(body.password, iterations=HASH_ITERATIONS)
     User.insert(
         {
@@ -503,7 +505,7 @@ async def update_role(
         return JSONResponse(
             content={"message": "Cannot modify admin user's role"}, status_code=403
         )
-    if body.role not in ["admin", "viewer"]:
+    if body.role not in VALID_ROLES:
         return JSONResponse(
             content={"message": "Role must be 'admin' or 'viewer'"}, status_code=400
         )

frigate/api/review.py

@@ -58,9 +58,8 @@ async def review(
     )
 
     clauses = [
-        (ReviewSegment.start_time > after)
-        & (ReviewSegment.start_time < before)
-        & ((ReviewSegment.end_time.is_null(True)) | (ReviewSegment.end_time < before))
+        (ReviewSegment.start_time < before)
+        & ((ReviewSegment.end_time.is_null(True)) | (ReviewSegment.end_time > after))
     ]
 
     if cameras != "all":

frigate/test/http_api/test_http_review.py

@@ -48,8 +48,9 @@ class TestHttpReview(BaseTestHttp):
     ###################################  GET /review Endpoint   ########################################################
     ####################################################################################################################
 
-    # Does not return any data point since the end time (before parameter) is not passed and the review segment end_time is 2 seconds from now
-    def test_get_review_no_filters_no_matches(self):
+    def test_get_review_that_overlaps_default_period(self):
+        """Test that a review item that starts during the default period
+        but ends after is included in the results."""
         now = datetime.now().timestamp()
 
         with TestClient(self.app) as client:
@@ -57,7 +58,7 @@ class TestHttpReview(BaseTestHttp):
             response = client.get("/review")
             assert response.status_code == 200
             response_json = response.json()
-            assert len(response_json) == 0
+            assert len(response_json) == 1
 
     def test_get_review_no_filters(self):
         now = datetime.now().timestamp()
@@ -73,11 +74,13 @@ class TestHttpReview(BaseTestHttp):
             assert response_json[0]["has_been_reviewed"] == False
 
     def test_get_review_with_time_filter_no_matches(self):
+        """Test that review items outside the range are not returned."""
         now = datetime.now().timestamp()
 
         with TestClient(self.app) as client:
             id = "123456.random"
-            super().insert_mock_review_segment(id, now, now + 2)
+            super().insert_mock_review_segment(id, now - 2, now - 1)
+            super().insert_mock_review_segment(f"{id}2", now + 4, now + 5)
             params = {
                 "after": now,
                 "before": now + 3,