From 6ee09a14e49c7ce50585f90552f2980301a91955 Mon Sep 17 00:00:00 2001 From: Tristan legion Date: Thu, 19 Feb 2026 13:07:25 -0500 Subject: [PATCH 1/2] Require and validate source_uid when creating officers; link to Source Ensure each Officer created via POST has a valid source_uid. Validate source exists, connect it via the existing citation relationship, and add tests for missing/invalid source_uid. Fixes: #507 Co-authored-by: Cursor --- backend/routes/officers.py | 24 ++++++++++-- backend/routes/tmp/pydantic/officers.py | 1 + backend/tests/test_officers.py | 52 ++++++++++++++++++++++++- 3 files changed, 71 insertions(+), 6 deletions(-) diff --git a/backend/routes/officers.py b/backend/routes/officers.py index 3a8c27c5b..a95d538d9 100644 --- a/backend/routes/officers.py +++ b/backend/routes/officers.py @@ -1,4 +1,5 @@ import logging +from datetime import datetime from typing import Optional, List from backend.auth.jwt import min_role_required @@ -7,6 +8,7 @@ add_pagination_wrapper) from backend.database.models.user import UserRole, User from backend.database.models.officer import Officer +from backend.database.models.source import Source from backend.routes.search import create_officer_result from .tmp.pydantic.officers import CreateOfficer, UpdateOfficer from flask import Blueprint, abort, request, jsonify @@ -134,16 +136,30 @@ class AddEmploymentListSchema(BaseModel): @validate_request(CreateOfficer) def create_officer(): """Create an officer profile. + + Requires a valid source_uid. The officer will be linked to the + specified data source via an UPDATED_BY citation. """ logger = logging.getLogger("create_officer") body: CreateOfficer = request.validated_body jwt_decoded = get_jwt() current_user = User.get(jwt_decoded["sub"]) - # try: - officer = Officer.from_dict(body.dict()) - # except Exception as e: - # abort(400, description=str(e)) + source = Source.nodes.get_or_none(uid=body.source_uid) + if source is None: + abort( + 422, + description=( + f"Source with UID '{body.source_uid}' not found. " + "A valid data source is required to create an officer." + ), + ) + + officer_data = body.dict() + officer_data.pop("source_uid", None) + officer = Officer.from_dict(officer_data) + + officer.citations.connect(source, {"date": datetime.now()}) logger.info(f"Officer {officer.uid} created by User {current_user.uid}") track_to_mp( diff --git a/backend/routes/tmp/pydantic/officers.py b/backend/routes/tmp/pydantic/officers.py index 572761def..944dc2f94 100644 --- a/backend/routes/tmp/pydantic/officers.py +++ b/backend/routes/tmp/pydantic/officers.py @@ -83,6 +83,7 @@ class CreateOfficer(BaseOfficer, BaseModel): gender: Optional[str] = Field(None, description="The gender of the officer") date_of_birth: Optional[str] = Field(None, description="The date of birth of the officer") state_ids: Optional[List[StateId]] = Field(None, description="The state ids of the officer") + source_uid: str = Field(..., description="The UID of the data source for this officer record") class UpdateOfficer(BaseOfficer, BaseModel): diff --git a/backend/tests/test_officers.py b/backend/tests/test_officers.py index e98077a0e..fec689f8f 100644 --- a/backend/tests/test_officers.py +++ b/backend/tests/test_officers.py @@ -141,15 +141,16 @@ def example_officers(): def test_create_officer( client, contributor_access_token, + example_source, example_agency ): - # Test that we can create an officer without an agency association request = { "first_name": "Max", "last_name": "Payne", "ethnicity": "White", - "gender": "Male" + "gender": "Male", + "source_uid": example_source.uid, } res = client.post( "/api/v1/officers/", @@ -169,6 +170,53 @@ def test_create_officer( assert officer_obj.ethnicity == request["ethnicity"] assert officer_obj.gender == request["gender"] + source = officer_obj.citations.all() + assert len(source) == 1 + assert source[0].uid == example_source.uid + + +def test_create_officer_without_source_uid( + client, + contributor_access_token, + ): + """POST officer without source_uid should fail with 422.""" + request = { + "first_name": "Max", + "last_name": "Payne", + "ethnicity": "White", + "gender": "Male", + } + res = client.post( + "/api/v1/officers/", + json=request, + headers={ + "Authorization": "Bearer {0}".format(contributor_access_token) + }, + ) + assert res.status_code == 422 + + +def test_create_officer_with_invalid_source_uid( + client, + contributor_access_token, + ): + """POST officer with a non-existent source_uid should fail with 422.""" + request = { + "first_name": "Max", + "last_name": "Payne", + "ethnicity": "White", + "gender": "Male", + "source_uid": "nonexistent-uid-12345", + } + res = client.post( + "/api/v1/officers/", + json=request, + headers={ + "Authorization": "Bearer {0}".format(contributor_access_token) + }, + ) + assert res.status_code == 422 + def test_get_officer(client, example_officer, access_token): # Test that we can get it From 5fb49c9ecc7fdec9c6ff1543bdecb5578a9d6848 Mon Sep 17 00:00:00 2001 From: Tristan legion Date: Mon, 23 Mar 2026 14:28:59 -0400 Subject: [PATCH 2/2] Enforce source membership when creating officers --- backend/routes/officers.py | 53 ++++---- backend/tests/test_officers.py | 214 +++++++++++++++++++++------------ 2 files changed, 168 insertions(+), 99 deletions(-) diff --git a/backend/routes/officers.py b/backend/routes/officers.py index a95d538d9..2492e99b2 100644 --- a/backend/routes/officers.py +++ b/backend/routes/officers.py @@ -4,8 +4,11 @@ from backend.auth.jwt import min_role_required from backend.mixpanel.mix import track_to_mp -from backend.schemas import (validate_request, ordered_jsonify, - add_pagination_wrapper) +from backend.schemas import ( + validate_request, + ordered_jsonify, + add_pagination_wrapper, +) from backend.database.models.user import UserRole, User from backend.database.models.officer import Officer from backend.database.models.source import Source @@ -34,8 +37,8 @@ class Config: json_schema_extra = { "example": { "officerName": "John Doe", - "location" : "New York", - "badgeNumber" : 1234, + "location": "New York", + "badgeNumber": 1234, "page": 1, "perPage": 20, } @@ -155,19 +158,28 @@ def create_officer(): ), ) + if not source.members.is_connected(current_user): + abort( + 403, description="User does not have permission to use this source" + ) + + member_rel = source.members.relationship(current_user) + if member_rel is None or not member_rel.may_publish(): + abort( + 403, description="User does not have permission to use this source" + ) + officer_data = body.dict() officer_data.pop("source_uid", None) officer = Officer.from_dict(officer_data) - officer.citations.connect(source, {"date": datetime.now()}) + officer.citations.connect(source, {"timestamp": datetime.now()}) logger.info(f"Officer {officer.uid} created by User {current_user.uid}") track_to_mp( request, "create_officer", - { - "officer_id": officer.uid - }, + {"officer_id": officer.uid}, ) return officer.to_json() @@ -177,8 +189,7 @@ def create_officer(): @jwt_required() @min_role_required(UserRole.PUBLIC) def get_officer(officer_uid: int): - """Get an officer profile. - """ + """Get an officer profile.""" o = Officer.nodes.get_or_none(uid=officer_uid) if o is None: abort(404, description="Officer not found") @@ -205,8 +216,10 @@ def get_all_officers(): limit = q_per_page # Build full name - officer_name_parts = [args.get(k, "").strip() for k in - ["firstName", "middleName", "lastName", "suffix"]] + officer_name_parts = [ + args.get(k, "").strip() + for k in ["firstName", "middleName", "lastName", "suffix"] + ] officer_name = " AND ".join([p for p in officer_name_parts if p]) or None officer_rank = " ".join(args.getlist("rank")) @@ -293,7 +306,7 @@ def get_all_officers(): results, _ = db.cypher_query(cypher_query, params) # Check mode — full node or SearchResult - if args.get("searchResult", "").lower() == 'true': # default is full node + if args.get("searchResult", "").lower() == "true": # default is full node all_officers = [create_officer_result(o[0]) for o in results] page = [item.model_dump() for item in all_officers if item] return_func = jsonify @@ -304,8 +317,7 @@ def get_all_officers(): # Add pagination wrapper response = add_pagination_wrapper( - page_data=page, total=row_count, - page_number=q_page, per_page=q_per_page + page_data=page, total=row_count, page_number=q_page, per_page=q_per_page ) logging.warning("API response: %s", response) @@ -319,8 +331,7 @@ def get_all_officers(): @min_role_required(UserRole.CONTRIBUTOR) @validate_request(UpdateOfficer) def update_officer(officer_uid: str): - """Update an officer profile. - """ + """Update an officer profile.""" body: UpdateOfficer = request.validated_body o = Officer.nodes.get_or_none(uid=officer_uid) if o is None: @@ -335,9 +346,7 @@ def update_officer(officer_uid: str): track_to_mp( request, "update_officer", - { - "officer_id": o.uid - }, + {"officer_id": o.uid}, ) return o.to_json() @@ -359,9 +368,7 @@ def delete_officer(officer_uid: str): track_to_mp( request, "delete_officer", - { - "officer_id": uid - }, + {"officer_id": uid}, ) return {"message": "Officer deleted successfully"} except Exception as e: diff --git a/backend/tests/test_officers.py b/backend/tests/test_officers.py index fec689f8f..af15e13a9 100644 --- a/backend/tests/test_officers.py +++ b/backend/tests/test_officers.py @@ -2,9 +2,7 @@ import pytest import math from datetime import date, datetime -from backend.database import ( - Officer, Unit, Agency -) +from backend.database import Officer, Unit, Agency from neomodel import db @@ -13,20 +11,20 @@ "first_name": "John", "last_name": "Doe", "ethnicity": "White", - "gender": "Male" + "gender": "Male", }, "hazel": { "first_name": "Hazel", "last_name": "Nutt", "ethnicity": "White", - "gender": "Female" + "gender": "Female", }, "frank": { "first_name": "Frank", "last_name": "Furter", "ethnicity": "Black/African American", - "gender": "Male" - } + "gender": "Male", + }, } mock_agencies = { @@ -36,7 +34,7 @@ "hq_address": "3510 S Michigan Ave", "hq_city": "Chicago", "hq_zip": "60653", - "jurisdiction": "MUNICIPAL" + "jurisdiction": "MUNICIPAL", }, "nypd": { "name": "New York Police Department", @@ -44,8 +42,8 @@ "hq_address": "1 Police Plaza", "hq_city": "New York", "hq_zip": "10038", - "jurisdiction": "MUNICIPAL" - } + "jurisdiction": "MUNICIPAL", + }, } mock_units = { @@ -62,7 +60,7 @@ "zip": "60001", "agency_url": "https://agency.gov", "officers_url": "https://agency.gov/unit-alpha/officers", - "date_established": date(2001, 5, 14) + "date_established": date(2001, 5, 14), }, "unit_bravo": { "name": "Unit Bravo", @@ -77,7 +75,7 @@ "zip": "75001", "agency_url": "https://agency.gov", "officers_url": "https://agency.gov/unit-bravo/officers", - "date_established": date(1998, 9, 3) + "date_established": date(1998, 9, 3), }, "unit_charlie": { "name": "Unit Charlie", @@ -92,40 +90,44 @@ "zip": "43001", "agency_url": "https://agency.gov", "officers_url": "https://agency.gov/unit-charlie/officers", - "date_established": date(2010, 2, 28) - } + "date_established": date(2010, 2, 28), + }, } mock_unit_memberships = { "john": { "earliest_date": datetime.strptime( - "2015-03-04 00:00:00", "%Y-%m-%d %H:%M:%S").date(), + "2015-03-04 00:00:00", "%Y-%m-%d %H:%M:%S" + ).date(), "latest_date": datetime.strptime( - "2020-03-04 00:00:00", "%Y-%m-%d %H:%M:%S").date(), + "2020-03-04 00:00:00", "%Y-%m-%d %H:%M:%S" + ).date(), "badge_number": "1234", - "highest_rank": 'Officer' + "highest_rank": "Officer", }, "hazel": { "earliest_date": datetime.strptime( - "2018-08-12 00:00:00", "%Y-%m-%d %H:%M:%S").date(), + "2018-08-12 00:00:00", "%Y-%m-%d %H:%M:%S" + ).date(), "latest_date": datetime.strptime( - "2021-04-04 00:00:00", "%Y-%m-%d %H:%M:%S").date(), + "2021-04-04 00:00:00", "%Y-%m-%d %H:%M:%S" + ).date(), "badge_number": "5678", - "highest_rank": 'Sergeant', + "highest_rank": "Sergeant", }, "frank": { "earliest_date": datetime.strptime( - "2019-05-03 00:00:00", "%Y-%m-%d %H:%M:%S").date(), + "2019-05-03 00:00:00", "%Y-%m-%d %H:%M:%S" + ).date(), "latest_date": datetime.strptime( - "2025-05-04 00:00:00", "%Y-%m-%d %H:%M:%S").date(), + "2025-05-04 00:00:00", "%Y-%m-%d %H:%M:%S" + ).date(), "badge_number": "1234", - "highest_rank": 'Lieutenant' - } + "highest_rank": "Lieutenant", + }, } -mock_sources = { - "cpdp": {"name": "Citizens Police Data Project"} -} +mock_sources = {"cpdp": {"name": "Citizens Police Data Project"}} @pytest.fixture @@ -139,11 +141,8 @@ def example_officers(): def test_create_officer( - client, - contributor_access_token, - example_source, - example_agency - ): + client, contributor_access_token, example_source, example_agency +): request = { "first_name": "Max", @@ -162,9 +161,7 @@ def test_create_officer( assert res.status_code == 200 response = res.json - officer_obj = ( - Officer.nodes.get(uid=response["uid"]) - ) + officer_obj = Officer.nodes.get(uid=response["uid"]) assert officer_obj.first_name == request["first_name"] assert officer_obj.last_name == request["last_name"] assert officer_obj.ethnicity == request["ethnicity"] @@ -176,9 +173,9 @@ def test_create_officer( def test_create_officer_without_source_uid( - client, - contributor_access_token, - ): + client, + contributor_access_token, +): """POST officer without source_uid should fail with 422.""" request = { "first_name": "Max", @@ -197,9 +194,9 @@ def test_create_officer_without_source_uid( def test_create_officer_with_invalid_source_uid( - client, - contributor_access_token, - ): + client, + contributor_access_token, +): """POST officer with a non-existent source_uid should fail with 422.""" request = { "first_name": "Max", @@ -218,6 +215,52 @@ def test_create_officer_with_invalid_source_uid( assert res.status_code == 422 +def test_create_officer_user_not_member(client, access_token, example_source): + request = { + "first_name": "Max", + "last_name": "Payne", + "ethnicity": "White", + "gender": "Male", + "source_uid": example_source.uid, + } + res = client.post( + "/api/v1/officers/", + json=request, + headers={"Authorization": "Bearer {0}".format(access_token)}, + ) + assert res.status_code == 403 + + +def test_create_officer_member_not_publisher( + client, + example_source_member, + example_source, +): + login_res = client.post( + "api/v1/auth/login", + json={ + "email": example_source_member.email, + "password": "my_password", + }, + ) + assert login_res.status_code == 200 + member_access_token = login_res.json["access_token"] + + request = { + "first_name": "Max", + "last_name": "Payne", + "ethnicity": "White", + "gender": "Male", + "source_uid": example_source.uid, + } + res = client.post( + "/api/v1/officers/", + json=request, + headers={"Authorization": "Bearer {0}".format(member_access_token)}, + ) + assert res.status_code == 403 + + def test_get_officer(client, example_officer, access_token): # Test that we can get it res = client.get(f"/api/v1/officers/{example_officer.uid}") @@ -308,7 +351,7 @@ def test_officer_pagination(client, db_session, access_token, example_officers): # Create Officers in the database officers = Officer.nodes.all() per_page = 1 - expected_total_pages = math.ceil(len(officers)//per_page) + expected_total_pages = math.ceil(len(officers) // per_page) for page in range(1, expected_total_pages + 1): res = client.get( @@ -335,9 +378,9 @@ def test_officer_pagination2(client, db_session, access_token): # Create Officers in the database for i in range(1, 36): mock_officer = { - "first_name": f"John{i}", - # "last_name": "Doe" - } + "first_name": f"John{i}", + # "last_name": "Doe" + } Officer(**mock_officer).save() officers = Officer.nodes.all() @@ -566,12 +609,15 @@ def create_officers_units_agencies(): agencies[key] = Agency(**mock).save() # Link officers to existing unit objects - units["unit_alpha"].officers.connect(officers["john"], - mock_unit_memberships["john"]) - units["unit_bravo"].officers.connect(officers["hazel"], - mock_unit_memberships["hazel"]) - units["unit_charlie"].officers.connect(officers["frank"], - mock_unit_memberships["frank"]) + units["unit_alpha"].officers.connect( + officers["john"], mock_unit_memberships["john"] + ) + units["unit_bravo"].officers.connect( + officers["hazel"], mock_unit_memberships["hazel"] + ) + units["unit_charlie"].officers.connect( + officers["frank"], mock_unit_memberships["frank"] + ) # Link units to agencies (one direction is enough) units["unit_alpha"].agency.connect(agencies["cpd"]) @@ -580,8 +626,9 @@ def create_officers_units_agencies(): return officers -def test_get_officers_with_unit(client, db_session, access_token, - create_officers_units_agencies): +def test_get_officers_with_unit( + client, db_session, access_token, create_officers_units_agencies +): results, meta = db.cypher_query(""" MATCH (o:Officer) @@ -625,8 +672,9 @@ def test_get_officers_with_unit(client, db_session, access_token, assert res.json["total"] == len(officers_with_units) -def test_get_officers_with_unit_and_agency(client, db_session, access_token, - create_officers_units_agencies): +def test_get_officers_with_unit_and_agency( + client, db_session, access_token, create_officers_units_agencies +): results, meta = db.cypher_query(""" MATCH (o:Officer) @@ -651,16 +699,22 @@ def test_get_officers_with_unit_and_agency(client, db_session, access_token, assert res.json["total"] == len(officers_with_unit) -@pytest.mark.parametrize("query,expect_results", [ - ("active_after=2018-01-01", True), - ("active_before=2022-01-01", True), - ("active_before=2010-01-01", False), - ("active_after=2035-01-01", False), -]) -def test_get_officers_dates(client, access_token, query, expect_results, - create_officers_units_agencies): - res = client.get(f"/api/v1/officers/?{query}", - headers={"Authorization": f"Bearer {access_token}"}) +@pytest.mark.parametrize( + "query,expect_results", + [ + ("active_after=2018-01-01", True), + ("active_before=2022-01-01", True), + ("active_before=2010-01-01", False), + ("active_after=2035-01-01", False), + ], +) +def test_get_officers_dates( + client, access_token, query, expect_results, create_officers_units_agencies +): + res = client.get( + f"/api/v1/officers/?{query}", + headers={"Authorization": f"Bearer {access_token}"}, + ) assert res.status_code == 200 if expect_results: @@ -675,8 +729,9 @@ def test_get_officers_dates(client, access_token, query, expect_results, assert res.json == {"message": "No results found matching the query"} -def test_get_officers_with_rank(client, db_session, access_token, - create_officers_units_agencies): +def test_get_officers_with_rank( + client, db_session, access_token, create_officers_units_agencies +): res = client.get( "/api/v1/officers/?rank=Officer", headers={"Authorization ": "Bearer {0}".format(access_token)}, @@ -705,11 +760,12 @@ def test_get_officers_with_rank(client, db_session, access_token, assert res.json["results"][0]["last_name"] is not None -def test_filter_by_ethnicity(client, db_session, access_token, - create_officers_units_agencies): +def test_filter_by_ethnicity( + client, db_session, access_token, create_officers_units_agencies +): res = client.get( "/api/v1/officers/?ethnicity=White", - headers={"Authorization": f"Bearer {access_token}"} + headers={"Authorization": f"Bearer {access_token}"}, ) assert res.status_code == 200 @@ -720,7 +776,7 @@ def test_filter_by_ethnicity(client, db_session, access_token, # Multiple ethnicities res = client.get( "/api/v1/officers/?ethnicity=Whiteðnicity=Hispanic", - headers={"Authorization": f"Bearer {access_token}"} + headers={"Authorization": f"Bearer {access_token}"}, ) assert res.status_code == 200 assert res.json != [] @@ -735,14 +791,20 @@ def test_filter_by_ethnicity(client, db_session, access_token, ("John", "Doe", True), ("Hazel", "Nutt", True), ("Alice", "Johnson", False), - ] + ], ) -def test_filter_by_officer_name(client, db_session, access_token, - create_officers_units_agencies, - first_name, last_name, expect_results): +def test_filter_by_officer_name( + client, + db_session, + access_token, + create_officers_units_agencies, + first_name, + last_name, + expect_results, +): res = client.get( f"/api/v1/officers/?firstName={first_name}&lastName={last_name}", - headers={"Authorization": f"Bearer {access_token}"} + headers={"Authorization": f"Bearer {access_token}"}, ) assert res.status_code == 200