EvotecIT / Testimo

Testimo is a PowerShell module for running health checks for Active Directory against a bunch of different tests
549 stars 58 forks source link

DCSMBSharesPermissions not working properly on Non-English systems #190

Closed diecknet closed 2 months ago

diecknet commented 2 months ago

I noticed that the DCSMBSharesPermissions test is not working properly on a German system. Looks like the names are hardcoded.

For example: https://github.com/EvotecIT/Testimo/blob/a358a778f9c8b136cd52960a81e35def6b5629be/Private/SourcesDomainControllers/SMBSharesPermissions.ps1#L45 But it's multiple times in that file.

I can propose a PR soon to fix that, but don't have time right now

PrzemyslawKlys commented 2 months ago

Maybe there is better way to do it then hardcode other languages

SamErde commented 2 months ago

Reference well-known SIDs?

PrzemyslawKlys commented 2 months ago

This requires looking at different source:

image

As the one I have

image

Is all about AccoutnName, probably would need to use COnvertIdentity

image

And make sure the SID is there and then compare for SID.

diecknet commented 2 months ago

There are some .NET methods we can use. I used them in the past. Here are some examples: https://github.com/diecknet/diecknet-scripts/blob/main/Snips/Get-LocalizedNTAuthority.ps1

PrzemyslawKlys commented 2 months ago

Thats what convert-identity does but a bit more

PrzemyslawKlys commented 2 months ago

Convert-Identity just can convert pretty much everything it can into SID/Name and type/domain name including special accounts

image

image

SamErde commented 2 months ago

Nice. Is that part of PSSharedGoods or ADEssentials?

PrzemyslawKlys commented 2 months ago

PSSharedGoods, also cross-forest as far as i remember

diecknet commented 2 months ago

I see a few possible options: a) Change Get-ComputerSMBSharePermissions in PSSharedGoods to return internationalized AccountNames instead of localized. b) Change the DCSMBSharesPermissions test in Testimo to work with localized names by well known SIDs.

Option a could improve future compatibility with other tests/scripts. Option b on the other hand, keeps the localized names in the report. That could be easier to understand for some people.

PrzemyslawKlys commented 2 months ago

Ye, change/improve PSSharedGoods and then modify tests to match it. Just run it thru Convert-Identity and expand with data from it.

PrzemyslawKlys commented 2 months ago

Here's a quick fix: image

PrzemyslawKlys commented 2 months ago

image image

PrzemyslawKlys commented 2 months ago

And woila all done :-)

diecknet commented 2 months ago

Awesome! Thank you @PrzemyslawKlys :)