From 354f86b57cadd16e8a4557e1a7dc429f2aac0010 Mon Sep 17 00:00:00 2001 From: Bevan Weiss Date: Sun, 30 Jun 2024 11:52:25 +1000 Subject: [PATCH] Add a few more checks on rollback of Util User. Tidy up the memory management around the ::NetUserGetInfo to align with MS guidance (https://learn.microsoft.com/en-us/windows/win32/api/lmaccess/nf-lmaccess-netusergetinfo#examples) Signed-off-by: Bevan Weiss --- src/ext/Util/ca/scauser.cpp | 6 +----- src/test/burn/WixTestTools/UserVerifier.cs | 4 ++-- .../msi/WixToolsetTest.MsiE2E/UtilExtensionUserTests.cs | 9 +++++++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/ext/Util/ca/scauser.cpp b/src/ext/Util/ca/scauser.cpp index b643a8429..fd0eb403a 100644 --- a/src/ext/Util/ca/scauser.cpp +++ b/src/ext/Util/ca/scauser.cpp @@ -520,10 +520,6 @@ HRESULT ScaUserExecute( // Check to see if the user already exists since we have to be very careful when adding // and removing users. Note: MSDN says that it is safe to call these APIs from any // user, so we should be safe calling it during immediate mode. - er = ::NetApiBufferAllocate(sizeof(USER_INFO_0), reinterpret_cast(&pUserInfo)); - hr = HRESULT_FROM_WIN32(er); - ExitOnFailure(hr, "Failed to allocate memory to check existence of user: %ls", psu->wzName); - LPCWSTR wzDomain = psu->wzDomain; if (wzDomain && *wzDomain) { @@ -547,7 +543,7 @@ HRESULT ScaUserExecute( } } - er = ::NetUserGetInfo(wzDomain, psu->wzName, 0, reinterpret_cast(pUserInfo)); + er = ::NetUserGetInfo(wzDomain, psu->wzName, 0, reinterpret_cast(&pUserInfo)); if (NERR_Success == er) { ueUserExists = USER_EXISTS_YES; diff --git a/src/test/burn/WixTestTools/UserVerifier.cs b/src/test/burn/WixTestTools/UserVerifier.cs index 51c6c31e5..d192a8b8f 100644 --- a/src/test/burn/WixTestTools/UserVerifier.cs +++ b/src/test/burn/WixTestTools/UserVerifier.cs @@ -43,13 +43,13 @@ public static class SIDStrings /// /// /// Has to be run as an Admin - public static void CreateLocalUser(string userName, string password) + public static void CreateLocalUser(string userName, string password, string comment = "") { DeleteLocalUser(userName); UserPrincipal newUser = new UserPrincipal(new PrincipalContext(ContextType.Machine)); newUser.SetPassword(password); newUser.Name = userName; - newUser.Description = String.Empty; + newUser.Description = comment; newUser.UserCannotChangePassword = true; newUser.PasswordNeverExpires = false; newUser.Save(); diff --git a/src/test/msi/WixToolsetTest.MsiE2E/UtilExtensionUserTests.cs b/src/test/msi/WixToolsetTest.MsiE2E/UtilExtensionUserTests.cs index 30bc53e8f..08b7cee18 100644 --- a/src/test/msi/WixToolsetTest.MsiE2E/UtilExtensionUserTests.cs +++ b/src/test/msi/WixToolsetTest.MsiE2E/UtilExtensionUserTests.cs @@ -48,7 +48,8 @@ public void CanInstallAndUninstallUsers() [RuntimeFact] public void CanRollbackUsers() { - UserVerifier.CreateLocalUser("testName3", "test123!@#"); + UserVerifier.CreateLocalUser("testName3", "test123!@#", "User3 comment"); + UserVerifier.AddUserToGroup("testName3", "Backup Operators"); var productFail = this.CreatePackageInstaller("ProductFail"); // make sure the user accounts are deleted before we start @@ -63,6 +64,10 @@ public void CanRollbackUsers() // Verify that user added to power users group is removed from power users group on rollback. UserVerifier.VerifyUserIsNotMemberOf("", "testName3", "Power Users"); + // but is not removed from Backup Operators + UserVerifier.VerifyUserIsMemberOf(string.Empty, "testName3", "Backup Operators"); + // and has their original comment set back + UserVerifier.VerifyUserComment(string.Empty, "testName3", "User3 comment"); // clean up UserVerifier.DeleteLocalUser("testName1"); @@ -71,7 +76,7 @@ public void CanRollbackUsers() } - // Verify that command-line parameters aer not blocked by repair switches. + // Verify that command-line parameters are not blocked by repair switches. // Original code signalled repair mode by using "-f ", which silently // terminated the command-line parsing, ignoring any parameters that followed. [RuntimeFact()]