diff --git a/api/howler/actions/demote.py b/api/howler/actions/demote.py index 88ed58886..d3540ebc6 100644 --- a/api/howler/actions/demote.py +++ b/api/howler/actions/demote.py @@ -76,7 +76,7 @@ def execute( *hit_helper.demote_hit(escalation=escalation), odm_helper.update("howler.assessment", None), odm_helper.update("howler.rationale", None), - odm_helper.update("howler.assignment", None), + odm_helper.update("howler.triaged", None), ], ) else: @@ -94,7 +94,7 @@ def execute( ds.hit.update_by_query( query, [ - *hit_helper.assess_hit(assessment, rationale, user=(user if user else "automation")), + *hit_helper.assess_hit(assessment, rationale), odm_helper.update( "howler.assignment", user.get("uname", "automation") if user else "automation", diff --git a/api/howler/actions/promote.py b/api/howler/actions/promote.py index 6744b3716..73fe36ccb 100644 --- a/api/howler/actions/promote.py +++ b/api/howler/actions/promote.py @@ -10,7 +10,6 @@ AssessmentEscalationMap, Escalation, ) -from howler.odm.models.user import User from howler.utils.str_utils import sanitize_lucene_query OPERATION_ID = "promote" @@ -27,7 +26,6 @@ def execute( escalation: Escalation = Escalation.ALERT, assessment: Optional[str] = None, rationale: Optional[str] = None, - user: Optional[User] = None, **kwargs, ): """Promote a hit. @@ -75,7 +73,7 @@ def execute( *hit_helper.promote_hit(escalation=escalation), odm_helper.update("howler.assessment", None), odm_helper.update("howler.rationale", None), - odm_helper.update("howler.assignment", None), + odm_helper.update("howler.triaged", None), ], ) else: @@ -90,9 +88,7 @@ def execute( ) return report - ds.hit.update_by_query( - query, hit_helper.assess_hit(assessment, rationale, user=(user if user else "automation")) - ) + ds.hit.update_by_query(query, hit_helper.assess_hit(assessment, rationale)) report.append( { diff --git a/api/howler/helper/hit.py b/api/howler/helper/hit.py index b5541c63f..c188a0bbe 100644 --- a/api/howler/helper/hit.py +++ b/api/howler/helper/hit.py @@ -24,14 +24,11 @@ def assess_hit( assessment: Optional[str] = None, rationale: Optional[str] = None, hit: Optional[Union[dict[str, Any], Hit]] = None, - *, - user: User | str, **kwargs, ) -> list[OdmUpdateOperation]: """Update the assessment and esclation of a hit Args: - user (User | str): The user making the assessment assessment (Optional[str], optional): The assessment to set the hit to. Defaults to None. hit (Optional[Union[dict[str, Any], Hit]], optional): The hit to update. Defaults to None. @@ -55,6 +52,14 @@ def assess_hit( if assessment is None and rationale: rationale = None + if assessment is None: + # reset the timestamp and set state to in progress if removing assessment (re-assessing) + triaged_timestamp = None + status = Status.IN_PROGRESS + else: + triaged_timestamp = "NOW" + status = Status.RESOLVED + logger.debug( "Updating assessment of %s to %s", hit["howler"]["id"] if hit else "unknown", @@ -66,16 +71,12 @@ def assess_hit( escalation, ) - if assessment is None: - assessor_id = None - else: - assessor_id = user.get("uname", user.get("username", None)) if isinstance(user, User) else user - return [ odm_helper.update("howler.assessment", assessment), odm_helper.update("howler.escalation", escalation), odm_helper.update("howler.rationale", rationale, silent=True), - odm_helper.update("howler.assessor", assessor_id, silent=True), + odm_helper.update("howler.triaged", triaged_timestamp), + odm_helper.update("howler.status", status), ] diff --git a/api/howler/helper/workflow.py b/api/howler/helper/workflow.py index 075308737..f412a50f2 100644 --- a/api/howler/helper/workflow.py +++ b/api/howler/helper/workflow.py @@ -86,7 +86,13 @@ def transition(self, current_status: str, transition: str, **kwargs) -> list[Odm updates_dict[update.key] = update - if self.status_prop not in updates_dict and _transition.get("dest", False): + if _transition.get("dest", False): + if self.status_prop in updates_dict and updates_dict[self.status_prop].value != _transition["dest"]: + raise WorkflowException( + f"Transition {transition} attempted to update the status {self.status_prop} with \ + a different value than the transition destination." + ) + updates_dict[self.status_prop] = OdmUpdateOperation( ESCollection.UPDATE_SET, self.status_prop, diff --git a/api/howler/odm/helper.py b/api/howler/odm/helper.py index bbbc81644..650314baa 100644 --- a/api/howler/odm/helper.py +++ b/api/howler/odm/helper.py @@ -178,10 +178,10 @@ def generate_useful_hit( # noqa: C901 hit.howler.assessment = None hit.howler.rationale = None + hit.howler.triaged = None hit.howler.status = "open" hit.howler.assignment = "unassigned" hit.howler.escalation = choice([Escalation.HIT, Escalation.ALERT]) - hit.howler.assessor = None if randint(1, 10) > 9: hit.howler.expiry = datetime.now() + timedelta(days=randint(1, 60)) diff --git a/api/howler/odm/models/howler_data.py b/api/howler/odm/models/howler_data.py index 7d3ade9e3..81c83d2c4 100644 --- a/api/howler/odm/models/howler_data.py +++ b/api/howler/odm/models/howler_data.py @@ -1,4 +1,5 @@ # mypy: ignore-errors +from datetime import datetime from typing import Optional from howler import odm @@ -180,12 +181,6 @@ class HowlerData(odm.Model): description="Unique identifier of the assigned user.", default=DEFAULT_ASSIGNMENT, ) - assessor: Optional[str] = odm.Optional( - odm.Keyword( - description="The most recent person to assess a hit", - default=None, - ) - ) data: list[str] = odm.List( odm.Keyword(description="Raw telemetry records associated with this hit."), default=[], @@ -248,6 +243,7 @@ class HowlerData(odm.Model): ) ) ) + triaged: Optional[datetime] = odm.Optional(odm.Date(description="Timestamp at which the hit was triaged.")) comment: list[Comment] = odm.List( odm.Compound(Comment), default=[], diff --git a/api/howler/odm/random_data.py b/api/howler/odm/random_data.py index 2cede259d..7a7d5c2ce 100644 --- a/api/howler/odm/random_data.py +++ b/api/howler/odm/random_data.py @@ -595,7 +595,6 @@ def create_hits(ds: HowlerDatastore, hit_count: int = 200): hit.howler.id, [ *assess_hit( - user=user, assessment=choice(Assessment.list()), rationale=get_random_string(), hit=hit, diff --git a/api/howler/services/hit_service.py b/api/howler/services/hit_service.py index cf18fd4f8..f1440764b 100644 --- a/api/howler/services/hit_service.py +++ b/api/howler/services/hit_service.py @@ -111,7 +111,7 @@ def get_hit_workflow() -> Workflow: "source": [Status.OPEN, Status.IN_PROGRESS], "transition": HitStatusTransition.ASSESS, "dest": Status.RESOLVED, - "actions": [assess_hit], + "actions": [assess_hit, assign_hit], } ), Transition( diff --git a/api/test/integration/api/test_hit_transition.py b/api/test/integration/api/test_hit_transition.py index f9f7edfe2..036f7643e 100644 --- a/api/test/integration/api/test_hit_transition.py +++ b/api/test/integration/api/test_hit_transition.py @@ -1,5 +1,6 @@ +import datetime import json -from typing import Any, Optional +from typing import Any, Literal, Optional import pytest @@ -42,11 +43,8 @@ def datastore(datastore_connection: HowlerDatastore): wipe_hits(datastore_connection) -def test_full_transition_flow(datastore: HowlerDatastore, login_session): # noqa: C901 - """Test that /api/v1/hit//transitions/start endpoint performs the correct transition""" - session, host = login_session - - assert datastore.hit.get(HIT_ID).howler.status == Status.OPEN +@pytest.fixture(scope="module") +def transition_data(datastore: HowlerDatastore) -> list[dict[str, Any]]: def check_assignment(user: str): def check(): @@ -66,23 +64,30 @@ def check(): return check - def check_assessor(user: str): + def check_triaged(assessment_time: Literal["NOW"] | None): def check(): - assert datastore.hit.get(HIT_ID).howler.assessor == user + tolerance = datetime.timedelta(seconds=10) + + triaged_timestamp = datastore.hit.get(HIT_ID).howler.triaged + if assessment_time is None: + assert triaged_timestamp is None + else: + assert triaged_timestamp is not None + assert abs(triaged_timestamp - datetime.datetime.now(datetime.timezone.utc)) < tolerance return check - transition_data: list[dict[str, Any]] = [ + return [ { "transition": HitStatusTransition.ASSESS, "data": {"assessment": Assessment.AMBIGUOUS}, "dest": Status.RESOLVED, - "check": [check_assessor("admin")], + "check": [check_assignment("admin"), check_triaged("NOW")], }, { "transition": HitStatusTransition.RE_EVALUATE, "dest": Status.IN_PROGRESS, - "check": [check_assessment(None), check_assignment("admin"), check_assessor(None)], + "check": [check_assessment(None), check_assignment("admin"), check_triaged(None)], }, { "transition": HitStatusTransition.RELEASE, @@ -148,12 +153,16 @@ def check(): "transition": HitStatusTransition.ASSESS, "data": {"assessment": Assessment.AMBIGUOUS}, "dest": Status.RESOLVED, - "check": [check_assessment(Assessment.AMBIGUOUS), check_assessor("admin")], + "check": [ + check_assessment(Assessment.AMBIGUOUS), + check_assignment("admin"), + check_triaged("NOW"), + ], }, { "transition": HitStatusTransition.RE_EVALUATE, "dest": Status.IN_PROGRESS, - "check": [check_assessment(None), check_assignment("admin"), check_assessor(None)], + "check": [check_assessment(None), check_assignment("admin"), check_triaged(None)], }, { "transition": HitStatusTransition.RELEASE, @@ -168,6 +177,13 @@ def check(): }, ] + +def test_full_transition_flow(transition_data, datastore, login_session): + """Test that /api/v1/hit//transitions/start endpoint performs the correct transition""" + session, host = login_session + + assert datastore.hit.get(HIT_ID).howler.status == Status.OPEN + for data in transition_data: checks = data.pop("check", None) _, version = datastore.hit.get(HIT_ID, as_obj=False, version=True) @@ -186,7 +202,7 @@ def check(): for c in checks: c() - datastore.hit.get(HIT_ID).howler.status == data["dest"] + assert datastore.hit.get(HIT_ID).howler.status == data["dest"] # hit: Hit = datastore.hit.get(HIT_ID, as_obj=False) # assert hit["howler"]["status"] == Status.IN_PROGRESS diff --git a/api/test/unit/helper/test_workflow.py b/api/test/unit/helper/test_workflow.py index b3993f6b9..77109c98f 100644 --- a/api/test/unit/helper/test_workflow.py +++ b/api/test/unit/helper/test_workflow.py @@ -4,6 +4,8 @@ from howler.datastore.operations import OdmUpdateOperation from howler.helper.workflow import Transition, Workflow, WorkflowException +DUMMY_WORKFLOW_STATE_KEY = "prop" + DUMMY_WORKFLOW_TRANSITIONS = [ Transition({"transition": "first", "source": "state1", "dest": "state2", "actions": []}), Transition( @@ -17,8 +19,52 @@ ] +@pytest.fixture(scope="module") +def workflow_with_state_setting_action(): + return Workflow( + DUMMY_WORKFLOW_STATE_KEY, + [ + *DUMMY_WORKFLOW_TRANSITIONS, + Transition( + { + "transition": "legal-set-state", + "source": "state1", + "dest": "state3", + "actions": [ + lambda **kwargs: [ + OdmUpdateOperation(ESCollection.UPDATE_SET, DUMMY_WORKFLOW_STATE_KEY, "state3") + ] + ], + } + ), + ], + ) + + +@pytest.fixture(scope="module") +def workflow_with_invalid_set_state_action(): + return Workflow( + DUMMY_WORKFLOW_STATE_KEY, + [ + *DUMMY_WORKFLOW_TRANSITIONS, + Transition( + { + "transition": "illegal-set-state", + "source": "state1", + "dest": "state3", + "actions": [ + lambda **kwargs: [ + OdmUpdateOperation(ESCollection.UPDATE_REMOVE, DUMMY_WORKFLOW_STATE_KEY, "state2") + ] + ], + } + ), + ], + ) + + def test_workflow(): - workflow: Workflow = Workflow("prop", DUMMY_WORKFLOW_TRANSITIONS) + workflow: Workflow = Workflow(DUMMY_WORKFLOW_STATE_KEY, DUMMY_WORKFLOW_TRANSITIONS) assert len(workflow.transitions) == 2 @@ -37,6 +83,28 @@ def test_workflow(): assert updates_second[1].value == "state1" +def test_workflow_with_state_setting_action(workflow_with_state_setting_action): + workflow: Workflow = workflow_with_state_setting_action + + assert len(workflow.transitions) == 3 + + # Run the "legal-set-state" transition + updates: list[OdmUpdateOperation] = workflow.transition("state1", "legal-set-state") + assert len(updates) == 1 + assert updates[0].key == DUMMY_WORKFLOW_STATE_KEY + assert updates[0].value == "state3" + + +def test_workflow_with_invalid_set_state_action(workflow_with_invalid_set_state_action): + workflow: Workflow = workflow_with_invalid_set_state_action + + assert len(workflow.transitions) == 3 + + # Run the "illegal-set-state" transition + with pytest.raises(WorkflowException): + workflow.transition("state1", "illegal-set-state") + + def test_workflow_missing_transition_props(): with pytest.raises(WorkflowException): Workflow( diff --git a/ui/src/components/elements/hit/elements/Assigned.tsx b/ui/src/components/elements/hit/elements/Assigned.tsx index e35c59147..5c2f1001d 100644 --- a/ui/src/components/elements/hit/elements/Assigned.tsx +++ b/ui/src/components/elements/hit/elements/Assigned.tsx @@ -48,46 +48,30 @@ const Assigned: FC<{ hit: Hit; layout: HitLayout; hideLabel?: boolean; - showAssessor?: boolean; showAssigned?: boolean; -}> = ({ hit, layout, hideLabel = false, showAssessor = false, showAssigned = false }) => { +}> = ({ hit, layout, hideLabel = false, showAssigned = false }) => { const { t } = useTranslation(); const { user } = useAppUser(); const { viewers } = useContext(SocketContext); const hitViewers = uniq(viewers[hit?.howler?.id] ?? []).filter(viewer => viewer !== user.username); - const assessorVisible = showAssessor || hit.howler.assessment != null; - const assigneeVisible = !hit.howler.assessor && (showAssigned || hit.howler.assignment !== 'unassigned'); + const assigneeVisible = showAssigned || hit.howler.assignment !== 'unassigned'; return ( - - {assigneeVisible && ( - <> - {!hideLabel && {t('app.drawer.hit.assignment.assignee')}:} - - - )} - {assessorVisible && ( - <> - {!hideLabel && {t('app.drawer.hit.assessment.assessor')}:} - - - )} - + {assigneeVisible && ( + <> + {!hideLabel && {t('app.drawer.hit.assignment.assignee')}:} + + + )} {hitViewers.length > 0 && hideLabel && }