Add verdict, verdict_id to Incident Findings class

[zschmerber]

Related Issue:

#902

Description of changes:

In this PR I am adding verdict, verdict_id, vedict_type, and verdict_type_id. the new fields will all be used in the incident finding and should be the last PR entered in V1.1

[zschmerber]

Thoughts on adding Duplicate to the verdict_id?

[pagbabian-splunk]

Thoughts on adding Duplicate to the verdict_id?

Good idea.

I just looked at the merged PR for Incident Finding, and there is a resolution_id which seems to overlap a bit with our proposed verdict_id - but it doesn't have a True Positive.

I'm not sure I like the term resolution as much as verdict since we also have a status that includes resolved but separately has a closed. Which would always mean two distinct events in order to have a verdict/resolution and then close the incident. If every attribute state change equates to a separate incident_finding event, that would make sense, but it seems like incident_finding is one of the more stateful classes we have, and multiple attribute changes could be committed together.

[zschmerber]

Thoughts on adding Duplicate to the verdict_id?

Good idea. I just looked at the merged PR for Incident, and there is a resolution_id which seems to overlap a bit with verdict_id - but it doesn't have a True Positive.

That is a pretty big overlap. I think we need to choose one or the other to add that values too.

[pagbabian-splunk]

Yes it is - I would just add to the resolution_id True Positive and Duplicate. I'm not so sure about having a Resolved for status_id though, see my comment above. Does Resolved also imply Closed? Also, would Duplicate be for a Detection Finding or other Finding, but not Incident Finding?

[zschmerber]

Yes it is - I would just add to the resolution_id True Positive and Duplicate. I'm not so sure about having a Resolved for status_id though, see my comment above. Does Resolved also imply Closed? Also, would Duplicate be for a Detection Finding or other Finding, but not Incident Finding?

Ok I can do that, do we want to have a resolution_type_id field that works in parallel that contains analyst , provider ect...?

[floydtree]

@zschmerber @pagbabian-splunk let's chat on slack for resolution vs verdict, easier and quicker to arrive at a consensus.

[zschmerber]

Removed resolution_id and resolution and added the fields into verdict_id and verdict

[pagbabian-splunk]

Looks good. A few nits on the verdict_id labels in the dictionary.

The language isn't quite right for: 'The incident verdict can be disregarded' and related. i.e. it's the incident that can be disregarded, not the incident verdict. Similar to others in the list: the incident is a True Positive, or the incident verdict is True Positive (not incident verdict is a True Positive). I know these things are small.

Also, although we haven't been perfectly consistent, and it won't matter in the browser, we usually have 0 - Unknown at the top of the list, and 99 - Other at the bottom, but if not in that order, we have 99 - Other and then 0 - Unknown as the first two in the list followed by the actual values.

[zschmerber]

Looks good. A few nits on the verdict_id labels in the dictionary.

The language isn't quite right for: 'The incident verdict can be disregarded' and related. i.e. it's the incident that can be disregarded, not the incident verdict. Similar to others in the list: the incident is a True Positive, or the incident verdict is True Positive (not incident verdict is a True Positive). I know these things are small.

Also, although we haven't been perfectly consistent, and it won't matter in the browser, we usually have 0 - Unknown at the top of the list, and 99 - Other at the bottom, but if not in that order, we have 99 - Other and then 0 - Unknown as the first two in the list followed by the actual values.

fixed this, also thoughts on verdict_type_id fields:
internal and external vs analyst verdict and provider?

[pagbabian-splunk]

Not that I want to copy, but UDM says 'Analyst Verdict' and 'Provider Verdict' - if we were to map those events, it would be straightforward. Hard to say how I would decide whether Analyst Verdict was internal or external.

[floydtree]

Looks good!

[Aniak5]

LGTM