Fix incorrect fields in detection AdminPromoAfterRoleMgmtAppPermissionGrant.yaml

[ep3p]

Required items, please complete

Change(s):

1. Use distinct query_frequency and query_period parameters.
2. Interchange left and right join sides.
3. Use join kind inner instead of innerunique.
4. Parse ServicePrincipal.ObjectID by name instead of position modifiedProperties[3].
5. Remove the AADOperationType == "AssignEligibleRole".
6. Take into account any "RoleManagement" and "Assign" operation.

Reason for Change(s):

1. If the permission grant operation happened 2h10m before the query execution, and the role assignment operation happened 1h50m before the query execution, this detection would have not triggered any alert, thus the detection was not working in some cases.
2. This detection should trigger only if a "RoleManagement" and "Assign" operation happens, it should be the first thing to check as it has the lower timescope.
3. innerunique kind deduplicates left side instead of the right side, so there could have been (very improbable) cases were the condition TimeRoleMgGrant < TimeAdminPromo does not evaluate to true, thus the detection was not working in some cases.
4. Not every event or environment has ServicePrincipal.ObjectID in position 3 of modifiedProperties, thus the detection was not working in some cases.
5. Operation Add eligible member to role in Core Directory has AADOperationType == "Assign", not "AssignEligibleRole".
6. Custom role definition assignments could be considered by this detection too if the custom role contains Admin.

Version Updated:

• Yes

Testing Completed:

• Need help, because I don't have available events of this privilege escalation. I have tried this query by adding some columns so the join works.

Checked that the validations are passing and have addressed any issues that are present:

• Yes

[v-spadarthi]

@aprakash13 : Please review the Analytics and provide your feedback. Thanks!

[v-mchatla]

Hi @devikamehra, Can you please review and share your feedback. Thanks

[aprakash13]

Reached out to Roberto to review this PR.

[v-atulyadav]

Waiting for Roberto' s feedback. Thanks.

[v-atulyadav]

Waiting for Roberto' s feedback. Thanks

[v-atulyadav]

Hi @devikamehra, can you please review and share your feedback. Thanks

[v-atulyadav]

Hi @aprakash13 we still waiting for Roberto' s feedback. Thanks

[v-atulyadav]

Hi @aprakash13 we still waiting for Roberto' s feedback. Thanks

[v-atulyadav]

Hi @aprakash13 we still waiting for Roberto' s feedback. Thanks

[v-atulyadav]

Hi @aprakash13, waiting for Roberto' s feedback. Thanks

[v-atulyadav]

Hi @aprakash13, please review this long pending PR. Thanks

[v-atulyadav]

Hi @devikamehra @rushriva @vakohl, can you please look into this long pending PR. Thanks.

[v-atulyadav]

Hi @devikamehra @rushriva @vakohl, could you please review this PR and provide your comments. Thanks

[v-atulyadav]

Hi @devikamehra @rushriva @vakohl, can you please look into this long pending PR. Thanks.

[v-atulyadav]

Hi @devikamehra @rushriva @vakohl, could you please review this PR and provide your comments. Thanks

[v-atulyadav]

Hi @devikamehra @rushriva @vakohl, can you please look into this long pending PR. Thanks.

[v-atulyadav]

Hi @devikamehra @rushriva @vakohl, could you please review this PR and provide your comments. Thanks

[devikamehra]

@v-atulyadav We are working on improving the code of this query and the interpretation of the query made by PR author doesn't seems to be correct.

[ep3p]

Please, could I know what doesn't seem to be correct?

[ep3p]

Would you prefer the alternate order of the join sides?

[v-atulyadav]

Hi @devikamehra, please respond on @ep3p's comment. Thanks

[ep3p]

I have reviewed the query for correctness but please note, the author is already working to improve the query and thus this version might not get merged. Also, why do we require to change the order of join?

@devikamehra thank you very much.

The order of the join is not strictly required to be changed, but, if one side is going to check 1h of events and the other side is going to check 2h of events, AND the join type is inner; then, it is more efficient to check first the side that has 1h of events.

I am okay if this version does not get merged, as long as the improvements get passed.

I will alternate the order of the join sides, for clarity.

[v-atulyadav]

Hi @devikamehra, please provide your feedback as @ep3p has made some changes. Thanks

[v-atulyadav]

Hi @devikamehra, could you please provide your feedback. Thanks

[devikamehra]

@v-atulyadav The author is already working on this and thus we would have to wait.

[v-atulyadav]

Hi @ep3p, waiting for your response. Thanks

[ep3p]

Hi @ep3p, waiting for your response. Thanks

@v-atulyadav by author they must mean Roberto haha I am not the original author of this detection. This PR version might not get merged.

[v-dvedak]

Hi @ep3p, waiting for your response. Thanks

@v-atulyadav by author they must mean Roberto haha I am not the original author of this detection. This PR version might not get merged.

@ep3p, we would like to know if this PR is needed in that case since you mentioned it might not get merged. In that case, we can close it if no longer needed. Thanks!

[ep3p]

Hi @ep3p, waiting for your response. Thanks

@v-atulyadav by author they must mean Roberto haha I am not the original author of this detection. This PR version might not get merged.

@ep3p, we would like to know if this PR is needed in that case since you mentioned it might not get merged. In that case, we can close it if no longer needed. Thanks!

@v-dvedak Please, I am not the one who initially said it might not get merged, it was @devikamehra

I personally consider this detection needs a change. I will not be the one who has to decide if that change will come by this PR or another.

[v-atulyadav]

Hi @ep3p, would it be possible to mark this PR as a draft until we hear from the author?. Thanks.