BloodHoundAD / AzureHound

Azure Data Exporter for BloodHound
GNU General Public License v3.0
556 stars 75 forks source link

Fix to correctly fetch "onPremisesSecurityIdentifier" and onPremisesSyncEnabled" and ""accountEnabled" #42

Closed LuemmelSec closed 1 year ago

LuemmelSec commented 1 year ago

Added an array containing "onPremisesSecurityIdentifier" and onPremisesSyncEnabled" to fill the $select variable when querying the Graph API at /v1.0/users

As both, Microsoft and the AzureHound, document and state, both datasets are not included in the "default" query and need to be specified as a GET parameter of $select. Please refer to here: https://learn.microsoft.com/en-us/graph/api/user-list?view=graph-rest-1.0&tabs=http#optional-query-parameters

And here: https://github.com/BloodHoundAD/AzureHound/blob/main/models/azure/user.go?plain=1#L323

So this was not fetched, as AzureHound is currently only doing the "default" fetch without $select, resulting in the following data always returned:

image

But BloodHound and the ingestor part are already taken both values into consideration. Please see: https://github.com/BloodHoundAD/BloodHound/blob/69786fa46fa18090e7641e086cd2aed70a530748/src/js/ingestion_types.js?plain=1#L523

and: https://github.com/BloodHoundAD/BloodHound/blob/69786fa46fa18090e7641e086cd2aed70a530748/src/js/newingestion.js?plain=1#L2896

With the changes implemented it now looks "correct": image

UPDATE: I also found out that the enabled info was also not fetched. However, BloodHound in the default configuration "wants" to show this info but was actually never feeded what was needed. Please refer to: https://github.com/BloodHoundAD/BloodHound/blob/master/src/js/ingestion_types.js?plain=1#L520

and: https://github.com/BloodHoundAD/BloodHound/blob/master/src/js/newingestion.js?plain=1#L2934

It now shows up like intended: image

ddlees commented 1 year ago

@LuemmelSec I pulled your PR and made some modifications in #48. Thank you for your contribution!