Domain involved custom actions should use Impersonate=yes so ensure that

they retain the permissions of the executing user instead of trying to perform actions as computer LocalSystem account. The change to Impersonate=yes does not appear to have impacted local user or group membership functions (from the MSI integration tests at least) There is still an outstanding test aspect failing Test says account has PasswordNeverExpires, but this is not true when looking at the user account in the domain. Debugging into the xUnit tests is proving difficult however... Signed-off-by: Bevan Weiss <[email protected]>
wixtoolset · Jul 13, 2024 · c78704a · c78704a
1 parent ccd3b80
commit c78704a
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 7 deletions.
diff --git a/src/ext/Util/ca/scaexec.cpp b/src/ext/Util/ca/scaexec.cpp
@@ -168,13 +168,13 @@ static HRESULT AddUserToGroup(
     IADsGroup *pGroup = NULL;
     BSTR bstrUser = NULL;
     BSTR bstrGroup = NULL;
-    LPCWSTR wz = NULL;
+    LPWSTR wz = NULL;
     LPWSTR pwzUser = NULL;
     LOCALGROUP_MEMBERS_INFO_3 lgmi;
 
     if (*wzGroupDomain)
     {
-        wz = wzGroupDomain;
+        hr = GetDomainServerName(wzGroupDomain, &wz, DS_WRITABLE_REQUIRED);
     }
 
     // Try adding it to the global group first
@@ -189,7 +189,7 @@ static HRESULT AddUserToGroup(
         }
 
         lgmi.lgrmi3_domainandname = (NULL == pwzUser ? wzUser : pwzUser);
-        ui = ::NetLocalGroupAddMembers(wz, wzGroup, 3 , reinterpret_cast<LPBYTE>(&lgmi), 1);
+        ui = ::NetLocalGroupAddMembers(wz, wzGroup, 3, reinterpret_cast<LPBYTE>(&lgmi), 1);
     }
     hr = HRESULT_FROM_WIN32(ui);
     if (HRESULT_FROM_WIN32(ERROR_MEMBER_IN_ALIAS) == hr) // if they're already a member of the group don't report an error
@@ -222,6 +222,7 @@ static HRESULT AddUserToGroup(
     ReleaseObject(pGroup);
     ReleaseBSTR(bstrUser);
     ReleaseBSTR(bstrGroup);
+    ReleaseStr(wz);
 
     return hr;
 }

diff --git a/src/ext/Util/ca/scanet.cpp b/src/ext/Util/ca/scanet.cpp
@@ -21,7 +21,7 @@ HRESULT GetDomainServerName(LPCWSTR pwzDomain, LPWSTR* ppwzServerName, ULONG fla
         if (ERROR_SUCCESS == er && pDomainControllerInfo->DomainControllerName)
         {
             // Skip the \\ prefix if present.
-            if ('\\' == *pDomainControllerInfo->DomainControllerName && '\\' == *pDomainControllerInfo->DomainControllerName + 1)
+            if ('\\' == *pDomainControllerInfo->DomainControllerName && '\\' == *(pDomainControllerInfo->DomainControllerName + 1))
             {
                 hr = StrAllocString(ppwzServerName, pDomainControllerInfo->DomainControllerName + 2, 0);
             }

diff --git a/src/ext/Util/wixlib/UtilExtension_Platform.wxi b/src/ext/Util/wixlib/UtilExtension_Platform.wxi
@@ -136,10 +136,10 @@
         <UIRef Id="ConfigureUsersErrorText" />
 
         <CustomAction Id="$(var.Prefix)ConfigureUsers$(var.Suffix)" DllEntry="ConfigureUsers" Execute="immediate" Return="check" SuppressModularization="yes" BinaryRef="$(var.Prefix)UtilCA$(var.Suffix)" />
-        <CustomAction Id="$(var.Prefix)CreateUser$(var.Suffix)" DllEntry="CreateUser" Impersonate="no" Execute="deferred" Return="check" HideTarget="yes" SuppressModularization="yes" BinaryRef="$(var.Prefix)UtilCA$(var.Suffix)" />
-        <CustomAction Id="$(var.Prefix)CreateUserRollback$(var.Suffix)" DllEntry="CreateUserRollback" Impersonate="no" Execute="rollback" Return="check" HideTarget="yes" SuppressModularization="yes" BinaryRef="$(var.Prefix)UtilCA$(var.Suffix)" />
+        <CustomAction Id="$(var.Prefix)CreateUser$(var.Suffix)" DllEntry="CreateUser" Impersonate="yes" Execute="deferred" Return="check" HideTarget="yes" SuppressModularization="yes" BinaryRef="$(var.Prefix)UtilCA$(var.Suffix)" />
+        <CustomAction Id="$(var.Prefix)CreateUserRollback$(var.Suffix)" DllEntry="CreateUserRollback" Impersonate="yes" Execute="rollback" Return="check" HideTarget="yes" SuppressModularization="yes" BinaryRef="$(var.Prefix)UtilCA$(var.Suffix)" />
         <!-- RemoveUser is a type commit action because it is not possible to rollback the removal of a user -->
-        <CustomAction Id="$(var.Prefix)RemoveUser$(var.Suffix)" DllEntry="RemoveUser" Impersonate="no" Execute="commit" Return="ignore" HideTarget="yes" SuppressModularization="yes" BinaryRef="$(var.Prefix)UtilCA$(var.Suffix)" />
+        <CustomAction Id="$(var.Prefix)RemoveUser$(var.Suffix)" DllEntry="RemoveUser" Impersonate="yes" Execute="commit" Return="ignore" HideTarget="yes" SuppressModularization="yes" BinaryRef="$(var.Prefix)UtilCA$(var.Suffix)" />
 
         <InstallExecuteSequence>
             <Custom Action="virtual $(var.Prefix)ConfigureUsers$(var.Suffix)" Before="InstallFiles" Condition="VersionNT &gt; 400" />