search

huntresslabs / deployment-scripts

RMM deployment scripts for Huntress
37 stars 33 forks source link

fix testAdministrator test for local admin #48

Closed abstract-surfer closed 1 year ago

abstract-surfer commented 1 year ago

When testing locally I found a bug where the testAdministrator method passed even when not running in an elevated PowerShell window. Upon further digging, it appears this check was arranged incorrectly.

My second commit, which I'm fully willing to remove, adds a check during install for proper permissions. The bug I found during testing is that the script attempted to run and got as far as checking for registry keys before it failed due to not having an elevated command prompt.

See screenshot below for error during install if permissions are missing:

image

Alan-Huntress commented 1 year ago

This was done on purpose as some parts of the script do work without admin. The design decision was made to log that admin priv's are missing but continue as far as it can go, but I'm certainly open to other opinions.

abstract-surfer commented 1 year ago

@Alan-Huntress thanks for that context. The admin check was broken, though, as the syntax was wrong. Do we want to provide a script that will fail when admin privileges are not available, rather than warning the user? I realize some parts of the script may work, but if the agent won't install completely, that seems less than desirable. @AspenScythe is there background I'm missing here?

Alan-Huntress commented 1 year ago

The original script (v1) did not even check for admin, it simply failed silently. I've made improvements here and there as my busy schedule allows, but there are still a ton of things we could do better with all our deploy methods.