Fixed Test-UserAdminMembership so the result is correct regardless if…

[dwknight70]

Fix to the Test-UserAdminMembership function. The current version is only checking if the current user is a direct member of local administrators. If the user member of any groups that are in local administrators but is not a direct member, it erroneously returns false. See the discussion @ https://smsagent.blog/2016/06/30/testing-for-local-administrator-privilege-with-powershell/.

[skycommand]

Hello. 😊 Please address the problems I've raised.

[dwknight70]

I don't see where you have raised any problems - please clarify - thanks.

[skycommand]

Strange. Re-submitting my code review...

[dwknight70]

The code you committed is fine, but that comment truly does not apply. The elevation context is irrelevant for that function, and it will not request elevation.

[skycommand]

@dwknight70 Thanks a lot for your contribution. 🙏 I shall remember you fondly as the first person who improved my code. 🎉

I will work on improving the language and tone of the comment to which you are objecting but I believe the user must know that Test-UserAdminMembership is not the function they must use to determine whether they can run such commands as Get-ProvisionedAppxPackage -Online.

[dwknight70]

Glad to be of help. Yeah, I can see how people might be confused. Code leveraging these would really want to use a combination of both functions - Test-ProcessAdminRight to see if it's in an elevated admin-context already, and Test-UserAdminMembership to check if it's even possible for them to get an elevated context. If it is, then something like this (https://gist.github.com/fearthecowboy/0781e8914ad9b3a1b104) can be used to re-launch a script in an elevated context, which will automatically trigger the UAC prompt.

[dwknight70]

Oh, and I should mention, not only do I happen to be your first contributor, I've also never contributed to any other repos before. It's a first all the way around ;)