briandelmsft / STAT-Function

Azure Function for the Microsoft Sentinel Triage AssistanT (STAT)
https://aka.ms/mstat
MIT License
9 stars 1 forks source link

[BUG] Entra ID Risks Module - Values are set to null instead of 0 #66

Closed piaudonn closed 5 months ago

piaudonn commented 6 months ago

This make logic app Control Actions fails if we check for UserFailedMFACount or UserMFAFraudCount or SuspiciousActivityReportCount greater than 0 as Null is not an integer.

https://github.com/briandelmsft/STAT-Function/blob/48f9183ac44d0415c28813d6279f4f7a281b3276/modules/aadrisks.py#L20C3-L22C56

This is only when the data has accounts which do not exist in Entra ID (I think?).

briandelmsft commented 5 months ago

@piaudonn how do you think we should handle this? On the one hand the check may not be performed based on the input settings to the module, so in that case null makes sense. And, if someone is turning off the check, they really shouldn't be evaluating it. But, if the check is performed and there's no data, I think 0 is the right outcome, but isn't that already happening?

https://github.com/briandelmsft/STAT-Function/blob/48f9183ac44d0415c28813d6279f4f7a281b3276/modules/aadrisks.py#L62-L68

L66 would throw an exception if the property wasn't there, and I don't think there's anyway the count() KQL function is returning anything but an int when there are results

briandelmsft commented 5 months ago

@piaudonn after our discussion I looked into this issue further with the unenriched entities in mind. If the entity is found via IdentityInfo, the base module still then takes that UPN and goes to the graph to finish the enrichments, if the graph call fails it is passed as a 'RawEntity' only. If we don't find it via IdentityInfo it is also passed as a 'RawEntity' only.

If it's a 'RawEntity' only, this module doesn't touch it as there won't be an id property: https://github.com/briandelmsft/STAT-Function/blob/48f9183ac44d0415c28813d6279f4f7a281b3276/modules/aadrisks.py#L15-L17

If there's no entities processed, we don't execute the sums: https://github.com/briandelmsft/STAT-Function/blob/48f9183ac44d0415c28813d6279f4f7a281b3276/modules/aadrisks.py#L85-L92

and the return will look like this:

{
  "AnalyzedEntities": 0,
  "FailedMFATotalCount": null,
  "HighestRiskLevel": "",
  "MFAFraudTotalCount": null,
  "SuspiciousActivityReportTotalCount": null,
  "ModuleName": "AADRisksModule",
  "DetailedResults": [],
  "RiskDetectionTotalCount": null
}

which is coming from the class definition: https://github.com/briandelmsft/STAT-Function/blob/48f9183ac44d0415c28813d6279f4f7a281b3276/classes/__init__.py#L420-L431