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 <[email protected]>
wixtoolset · Jun 30, 2024 · 354f86b · 354f86b
1 parent 96b546f
commit 354f86b
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 9 deletions.
diff --git 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<LPVOID*>(&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<LPBYTE*>(pUserInfo));
+ er = ::NetUserGetInfo(wzDomain, psu->wzName, 0, reinterpret_cast<LPBYTE*>(&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
@@ -43,13 +43,13 @@ public static class SIDStrings
  /// <param name="userName"></param>
  /// <param name="password"></param>
  /// <remarks>Has to be run as an Admin</remarks>
- 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
@@ -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()]