From ea576e74682ea8bc086195372ed90ca78fc2f54d Mon Sep 17 00:00:00 2001 From: Nicolas Mowen Date: Thu, 26 Jun 2025 13:01:09 -0600 Subject: [PATCH] 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 | 20 +++++++++++--------- frigate/api/review.py | 5 ++--- frigate/test/http_api/test_http_review.py | 11 +++++++---- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/frigate/api/auth.py b/frigate/api/auth.py index db12c04bb..6577f9c23 100644 --- a/frigate/api/auth.py +++ b/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 ) diff --git a/frigate/api/review.py b/frigate/api/review.py index b90365595..e6d010db7 100644 --- a/frigate/api/review.py +++ b/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": diff --git a/frigate/test/http_api/test_http_review.py b/frigate/test/http_api/test_http_review.py index 9fd0d1ea7..469e012b2 100644 --- a/frigate/test/http_api/test_http_review.py +++ b/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,