4 files changed, 317 insertions, 130 deletions

diff --git a/src/ext/Util/ca/sca.h b/src/ext/Util/ca/sca.h
index 599122ff..84f5ffd9 100644
--- a/src/ext/Util/ca/sca.h
+++ b/src/ext/Util/ca/sca.h
@@ -16,4 +16,5 @@ enum SCAU_ATTRIBUTES
     SCAU_DONT_REMOVE_ON_UNINSTALL = 0x00000100,
     SCAU_DONT_CREATE_USER = 0x00000200,
     SCAU_NON_VITAL = 0x00000400,
-};
\ No newline at end of file
+    SCAU_REMOVE_COMMENT = 0x00000800,
+};
diff --git a/src/ext/Util/ca/scaexec.cpp b/src/ext/Util/ca/scaexec.cpp
index 5845c1b4..7bd271d1 100644
--- a/src/ext/Util/ca/scaexec.cpp
+++ b/src/ext/Util/ca/scaexec.cpp
@@ -2,7 +2,6 @@
 
 #include "precomp.h"
 
-
 /********************************************************************
  * CreateSmb - CUSTOM ACTION ENTRY POINT for creating fileshares
  *
@@ -520,55 +519,88 @@ static HRESULT ModifyUserLocalBatchRight(
     return hr;
 }
 
-static void SetUserPasswordAndAttributes(
-    __in USER_INFO_1* puserInfo,
-    __in LPWSTR wzPassword,
-    __in int iAttributes
-    )
+static HRESULT ApplyAttributes(int iAttributes, DWORD* pFlags)
 {
-    Assert(puserInfo);
-
-    // Set the User's password
-    puserInfo->usri1_password = wzPassword;
+    HRESULT hr = S_OK;
 
-    // Apply the Attributes
     if (SCAU_DONT_EXPIRE_PASSWRD & iAttributes)
     {
-        puserInfo->usri1_flags |= UF_DONT_EXPIRE_PASSWD;
+        *pFlags |= UF_DONT_EXPIRE_PASSWD;
     }
     else
     {
-        puserInfo->usri1_flags &= ~UF_DONT_EXPIRE_PASSWD;
+        *pFlags &= ~UF_DONT_EXPIRE_PASSWD;
     }
 
     if (SCAU_PASSWD_CANT_CHANGE & iAttributes)
     {
-        puserInfo->usri1_flags |= UF_PASSWD_CANT_CHANGE;
+        *pFlags |= UF_PASSWD_CANT_CHANGE;
     }
     else
     {
-        puserInfo->usri1_flags &= ~UF_PASSWD_CANT_CHANGE;
+        *pFlags &= ~UF_PASSWD_CANT_CHANGE;
     }
 
     if (SCAU_DISABLE_ACCOUNT & iAttributes)
     {
-        puserInfo->usri1_flags |= UF_ACCOUNTDISABLE;
+        *pFlags |= UF_ACCOUNTDISABLE;
     }
     else
     {
-        puserInfo->usri1_flags &= ~UF_ACCOUNTDISABLE;
+        *pFlags &= ~UF_ACCOUNTDISABLE;
     }
 
     if (SCAU_PASSWD_CHANGE_REQD_ON_LOGIN & iAttributes) // TODO: for some reason this doesn't work
     {
-        puserInfo->usri1_flags |= UF_PASSWORD_EXPIRED;
+        *pFlags |= UF_PASSWORD_EXPIRED;
     }
     else
     {
-        puserInfo->usri1_flags &= ~UF_PASSWORD_EXPIRED;
+        *pFlags &= ~UF_PASSWORD_EXPIRED;
+    }
+
+    return hr;
+}
+
+static HRESULT ApplyComment(int iAttributes, LPWSTR pwzComment, LPWSTR* ppComment)
+{
+    HRESULT hr = S_OK;
+
+    if (SCAU_REMOVE_COMMENT & iAttributes)
+    {
+        *ppComment = L"";
+    }
+    else if (pwzComment && *pwzComment)
+    {
+        *ppComment = pwzComment;
     }
+
+    return hr;
+}
+
+static NET_API_STATUS SetUserPassword(__in LPWSTR pwzServerName, __in LPWSTR pwzName, __in LPWSTR pwzPassword)
+{
+    _USER_INFO_1003 userInfo1003;
+
+    userInfo1003.usri1003_password = pwzPassword;
+    return ::NetUserSetInfo(pwzServerName, pwzName, 1003, reinterpret_cast<LPBYTE>(&userInfo1003), NULL);
 }
 
+static NET_API_STATUS SetUserComment(__in LPWSTR pwzServerName, __in LPWSTR pwzName, __in LPWSTR pwzComment)
+{
+    _USER_INFO_1007 userInfo1007;
+
+    userInfo1007.usri1007_comment = pwzComment;
+    return ::NetUserSetInfo(pwzServerName, pwzName, 1007, reinterpret_cast<LPBYTE>(&userInfo1007), NULL);
+}
+
+static NET_API_STATUS SetUserFlags(__in LPWSTR pwzServerName, __in LPWSTR pwzName, __in DWORD flags)
+{
+    _USER_INFO_1008 userInfo1008;
+
+    userInfo1008.usri1008_flags = flags;
+    return ::NetUserSetInfo(pwzServerName, pwzName, 1008, reinterpret_cast<LPBYTE>(&userInfo1008), NULL);
+}
 
 static HRESULT RemoveUserInternal(
     LPWSTR wzGroupCaData,
@@ -624,7 +656,12 @@ static HRESULT RemoveUserInternal(
             }
             if (ERROR_SUCCESS == er)
             {
-                wz = pDomainControllerInfo->DomainControllerName + 2;  //Add 2 so that we don't get the \\ prefix
+                if (2 <= wcslen(pDomainControllerInfo->DomainControllerName))
+                {
+                    wz = pDomainControllerInfo->DomainControllerName + 2; // Add 2 so that we don't get the \\ prefix.
+                                                                          // Pass the entire string if it is too short
+                                                                          // to have a \\ prefix.
+                }
             }
             else
             {
@@ -680,6 +717,41 @@ LExit:
     return hr;
 }
 
+static HRESULT GetServerName(LPWSTR pwzDomain, LPWSTR* ppwzServerName)
+{
+    HRESULT hr = S_OK;
+
+    PDOMAIN_CONTROLLER_INFOW pDomainControllerInfo = NULL;
+    UINT er;
+
+    if (pwzDomain && *pwzDomain)
+    {
+        er = ::DsGetDcNameW(NULL, (LPCWSTR)pwzDomain, NULL, NULL, NULL, &pDomainControllerInfo);
+        if (RPC_S_SERVER_UNAVAILABLE == er)
+        {
+            // MSDN says, if we get the above error code, try again with the "DS_FORCE_REDISCOVERY" flag
+            er = ::DsGetDcNameW(NULL, (LPCWSTR)pwzDomain, NULL, NULL, DS_FORCE_REDISCOVERY, &pDomainControllerInfo);
+        }
+        if (ERROR_SUCCESS == er
+         && 2 <= wcslen(pDomainControllerInfo->DomainControllerName)
+         && '\\' == *pDomainControllerInfo->DomainControllerName
+         && '\\' == *pDomainControllerInfo->DomainControllerName + 1)
+        {
+            *ppwzServerName = pDomainControllerInfo->DomainControllerName + 2;  // Skip the \\ prefix
+        }
+        else
+        {
+            *ppwzServerName = pwzDomain;
+        }
+    }
+
+    if (pDomainControllerInfo)
+    {
+        ::NetApiBufferFree((LPVOID)pDomainControllerInfo);
+    }
+
+    return hr;
+}
 
 /********************************************************************
  CreateUser - CUSTOM ACTION ENTRY POINT for creating users
@@ -688,7 +760,7 @@ LExit:
  * *****************************************************************/
 extern "C" UINT __stdcall CreateUser(
     __in MSIHANDLE hInstall
-    )
+)
 {
     //AssertSz(0, "Debug CreateUser");
 
@@ -699,11 +771,11 @@ extern "C" UINT __stdcall CreateUser(
     LPWSTR pwz = NULL;
     LPWSTR pwzName = NULL;
     LPWSTR pwzDomain = NULL;
+    LPWSTR pwzComment = NULL;
     LPWSTR pwzScriptKey = NULL;
     LPWSTR pwzPassword = NULL;
     LPWSTR pwzGroup = NULL;
     LPWSTR pwzGroupDomain = NULL;
-    PDOMAIN_CONTROLLER_INFOW pDomainControllerInfo = NULL;
     int iAttributes = 0;
     BOOL fInitializedCom = FALSE;
 
@@ -711,10 +783,10 @@ extern "C" UINT __stdcall CreateUser(
     int iOriginalAttributes = 0;
     int iRollbackAttributes = 0;
 
-    USER_INFO_1 userInfo;
-    USER_INFO_1* puserInfo = NULL;
+    USER_INFO_1 userInfo1;
+    USER_INFO_1* pUserInfo1 = NULL;
     DWORD dw;
-    LPCWSTR wz = NULL;
+    LPWSTR pwzServerName = NULL;
 
     hr = WcaInitialize(hInstall, "CreateUser");
     ExitOnFailure(hr, "failed to initialize");
@@ -723,7 +795,7 @@ extern "C" UINT __stdcall CreateUser(
     ExitOnFailure(hr, "failed to initialize COM");
     fInitializedCom = TRUE;
 
-    hr = WcaGetProperty( L"CustomActionData", &pwzData);
+    hr = WcaGetProperty(L"CustomActionData", &pwzData);
     ExitOnFailure(hr, "failed to get CustomActionData");
 
     WcaLog(LOGMSG_TRACEONLY, "CustomActionData: %ls", pwzData);
@@ -738,6 +810,9 @@ extern "C" UINT __stdcall CreateUser(
     hr = WcaReadStringFromCaData(&pwz, &pwzDomain);
     ExitOnFailure(hr, "failed to read domain from custom action data");
 
+    hr = WcaReadStringFromCaData(&pwz, &pwzComment);
+    ExitOnFailure(hr, "failed to read user comment from custom action data");
+
     hr = WcaReadIntegerFromCaData(&pwz, &iAttributes);
     ExitOnFailure(hr, "failed to read attributes from custom action data");
 
@@ -747,96 +822,155 @@ extern "C" UINT __stdcall CreateUser(
     hr = WcaReadStringFromCaData(&pwz, &pwzPassword);
     ExitOnFailure(hr, "failed to read password from custom action data");
 
-    // There is no rollback scheduled if the key is empty.
-    // Best effort to get original configuration and save it in the script so rollback can restore it.
-    if (*pwzScriptKey)
-    {
-        hr = WcaCaScriptCreate(WCA_ACTION_INSTALL, WCA_CASCRIPT_ROLLBACK, FALSE, pwzScriptKey, FALSE, &hRollbackScript);
-        ExitOnFailure(hr, "Failed to open rollback CustomAction script.");
-
-        iRollbackAttributes = 0;
-        hr = GetExistingUserRightsAssignments(pwzDomain, pwzName, &iOriginalAttributes);
-        if (FAILED(hr))
-        {
-            WcaLogError(hr, "failed to get existing user rights: %ls, continuing anyway.", pwzName);
-        }
-        else
-        {
-            if (!(SCAU_ALLOW_LOGON_AS_SERVICE & iOriginalAttributes) && (SCAU_ALLOW_LOGON_AS_SERVICE & iAttributes))
-            {
-                iRollbackAttributes |= SCAU_ALLOW_LOGON_AS_SERVICE;
-            }
-            if (!(SCAU_ALLOW_LOGON_AS_BATCH & iOriginalAttributes) && (SCAU_ALLOW_LOGON_AS_BATCH & iAttributes))
-            {
-                iRollbackAttributes |= SCAU_ALLOW_LOGON_AS_BATCH;
-            }
-        }
-
-        hr = WcaCaScriptWriteNumber(hRollbackScript, iRollbackAttributes);
-        ExitOnFailure(hr, "Failed to add data to rollback script.");
-
-        // Nudge the system to get all our rollback data written to disk.
-        WcaCaScriptFlush(hRollbackScript);
-    }
-
     if (!(SCAU_DONT_CREATE_USER & iAttributes))
     {
-        ::ZeroMemory(&userInfo, sizeof(USER_INFO_1));
-        userInfo.usri1_name = pwzName;
-        userInfo.usri1_priv = USER_PRIV_USER;
-        userInfo.usri1_flags = UF_SCRIPT;
-        userInfo.usri1_home_dir = NULL;
-        userInfo.usri1_comment = NULL;
-        userInfo.usri1_script_path = NULL;
-
-        SetUserPasswordAndAttributes(&userInfo, pwzPassword, iAttributes);
+        pUserInfo1 = &userInfo1;
+        ::ZeroMemory(pUserInfo1, sizeof(USER_INFO_1));
+        pUserInfo1->usri1_name = pwzName;
+        pUserInfo1->usri1_priv = USER_PRIV_USER;
+        pUserInfo1->usri1_flags = UF_SCRIPT;
+        pUserInfo1->usri1_home_dir = NULL;
+        pUserInfo1->usri1_comment = NULL;
+        pUserInfo1->usri1_script_path = NULL;
+
+        // Set the user's password
+        pUserInfo1->usri1_password = pwzPassword;
+
+        // Set the user's comment
+        hr = ApplyComment(iAttributes, pwzComment, &pUserInfo1->usri1_comment);
+        ExitOnFailure(hr, "failed to apply comment");
+
+        // Set the user's flags
+        hr = ApplyAttributes(iAttributes, &pUserInfo1->usri1_flags);
+        ExitOnFailure(hr, "failed to apply attributes");
 
         //
         // Create the User
         //
-        if (pwzDomain && *pwzDomain)
+        hr = GetServerName(pwzDomain, &pwzServerName);
+        ExitOnFailure(hr, "failed to get server name");
+
+        er = ::NetUserAdd(pwzServerName, 1, reinterpret_cast<LPBYTE>(pUserInfo1), &dw);
+        if (NERR_UserExists == er)
         {
-            er = ::DsGetDcNameW( NULL, (LPCWSTR)pwzDomain, NULL, NULL, NULL, &pDomainControllerInfo );
-            if (RPC_S_SERVER_UNAVAILABLE == er)
-            {
-                // MSDN says, if we get the above error code, try again with the "DS_FORCE_REDISCOVERY" flag
-                er = ::DsGetDcNameW( NULL, (LPCWSTR)pwzDomain, NULL, NULL, DS_FORCE_REDISCOVERY, &pDomainControllerInfo );
-            }
-            if (ERROR_SUCCESS == er)
+            er = ERROR_SUCCESS; // Make sure that we don't report this situation as an error
+                                // if we fall through the tests that follow.
+            if (SCAU_FAIL_IF_EXISTS & iAttributes)
             {
-                wz = pDomainControllerInfo->DomainControllerName + 2;  //Add 2 so that we don't get the \\ prefix
+                hr = HRESULT_FROM_WIN32(er);
+                ExitOnFailure(hr, "User was not supposed to exist, but does.");
             }
-            else
-            {
-                wz = pwzDomain;
-            }
-        }
 
-        er = ::NetUserAdd(wz, 1, reinterpret_cast<LPBYTE>(&userInfo), &dw);
-        if (NERR_UserExists == er)
-        {
             if (SCAU_UPDATE_IF_EXISTS & iAttributes)
             {
-                er = ::NetUserGetInfo(wz, pwzName, 1, reinterpret_cast<LPBYTE*>(&puserInfo));
-                if (NERR_Success == er)
+                pUserInfo1 = NULL;
+                er = ::NetUserGetInfo(pwzServerName, pwzName, 1, reinterpret_cast<LPBYTE*>(&pUserInfo1));
+                if (ERROR_SUCCESS == er)
                 {
-                    // Change the existing user's password and attributes again then try
-                    // to update user with this new data
-                    SetUserPasswordAndAttributes(puserInfo, pwzPassword, iAttributes);
+                    // There is no rollback scheduled if the key is empty.
+                    // Best effort to get original configuration and save it in the script so rollback can restore it.
+
+                    if (*pwzScriptKey)
+                    {
+                        // Try to open the rollback script
+                        hr = WcaCaScriptOpen(WCA_ACTION_INSTALL, WCA_CASCRIPT_ROLLBACK, FALSE, pwzScriptKey, &hRollbackScript);
+
+                        if (INVALID_HANDLE_VALUE != hRollbackScript)
+                        {
+                            WcaCaScriptClose(hRollbackScript, WCA_CASCRIPT_CLOSE_PRESERVE);
+                        }
+                        else
+                        {
+                            hRollbackScript = NULL;
+                            hr = WcaCaScriptCreate(WCA_ACTION_INSTALL, WCA_CASCRIPT_ROLLBACK, FALSE, pwzScriptKey, FALSE, &hRollbackScript);
+                            ExitOnFailure(hr, "Failed to open rollback CustomAction script.");
+
+                            iRollbackAttributes = 0;
+                            hr = GetExistingUserRightsAssignments(pwzDomain, pwzName, &iOriginalAttributes);
+                            if (FAILED(hr))
+                            {
+                                WcaLogError(hr, "failed to get existing user rights: %ls, continuing anyway.", pwzName);
+                            }
+                            else
+                            {
+                                if (!(SCAU_ALLOW_LOGON_AS_SERVICE & iOriginalAttributes) && (SCAU_ALLOW_LOGON_AS_SERVICE & iAttributes))
+                                {
+                                    iRollbackAttributes |= SCAU_ALLOW_LOGON_AS_SERVICE;
+                                }
+
+                                if (!(SCAU_ALLOW_LOGON_AS_BATCH & iOriginalAttributes) && (SCAU_ALLOW_LOGON_AS_BATCH & iAttributes))
+                                {
+                                    iRollbackAttributes |= SCAU_ALLOW_LOGON_AS_BATCH;
+                                }
+                            }
+
+                            hr = WcaCaScriptWriteString(hRollbackScript, pUserInfo1->usri1_comment);
+                            ExitOnFailure(hr, "Failed to add rollback comment to rollback script.");
+
+                            if (!pUserInfo1->usri1_comment || !*pUserInfo1->usri1_comment)
+                            {
+                                iRollbackAttributes |= SCAU_REMOVE_COMMENT;
+                            }
+
+                            hr = WcaCaScriptWriteNumber(hRollbackScript, iRollbackAttributes);
+                            ExitOnFailure(hr, "Failed to add rollback attributes to rollback script.");
+
+                            // Nudge the system to get all our rollback data written to disk.
+                            WcaCaScriptFlush(hRollbackScript);
+                        }
+                    }
+                }
 
-                    er = ::NetUserSetInfo(wz, pwzName, 1, reinterpret_cast<LPBYTE>(puserInfo), &dw);
+                if (ERROR_SUCCESS == er)
+                {
+                    hr = HRESULT_FROM_WIN32(::SetUserPassword(pwzServerName, pwzName, pwzPassword));
+                    if (FAILED(hr))
+                    {
+                        WcaLogError(hr, "failed to set user password for user %ls\\%ls, continuing anyway.", pwzServerName, pwzName);
+                    }
+
+                    if (SCAU_REMOVE_COMMENT & iAttributes)
+                    {
+                        hr = HRESULT_FROM_WIN32(SetUserComment(pwzServerName, pwzName, L""));
+                        if (FAILED(hr))
+                        {
+                            WcaLogError(hr, "failed to clear user comment for user %ls\\%ls, continuing anyway.", pwzServerName, pwzName);
+                        }
+                    }
+                    else if (pwzComment && *pwzComment)
+                    {
+                        hr = HRESULT_FROM_WIN32(SetUserComment(pwzServerName, pwzName, pwzComment));
+                        if (FAILED(hr))
+                        {
+                            WcaLogError(hr, "failed to set user comment to %ls for user %ls\\%ls, continuing anyway.", pwzComment, pwzServerName, pwzName);
+                        }
+                    }
+
+                    DWORD flags = pUserInfo1->usri1_flags;
+
+                    hr = ApplyAttributes(iAttributes, &flags);
+                    if (FAILED(hr))
+                    {
+                        WcaLogError(hr, "failed to apply attributes for user %ls\\%ls, continuing anyway.", pwzServerName, pwzName);
+                    }
+
+                    hr = HRESULT_FROM_WIN32(SetUserFlags(pwzServerName, pwzName, flags));
+                    if (FAILED(hr))
+                    {
+                        WcaLogError(hr, "failed to set user flags for user %ls\\%ls, continuing anyway.", pwzServerName, pwzName);
+                    }
                 }
             }
-            else if (!(SCAU_FAIL_IF_EXISTS & iAttributes))
-            {
-                er = NERR_Success;
-            }
         }
         else if (NERR_PasswordTooShort == er || NERR_PasswordTooLong == er)
         {
             MessageExitOnFailure(hr = HRESULT_FROM_WIN32(er), msierrUSRFailedUserCreatePswd, "failed to create user: %ls due to invalid password.", pwzName);
         }
-        MessageExitOnFailure(hr = HRESULT_FROM_WIN32(er), msierrUSRFailedUserCreate, "failed to create user: %ls", pwzName);
+
+        if (ERROR_SUCCESS != er)
+        {
+            MessageExitOnFailure(hr = HRESULT_FROM_WIN32(er), msierrUSRFailedUserCreate, "failed to create user: %ls", pwzName);
+        }
     }
 
     if (SCAU_ALLOW_LOGON_AS_SERVICE & iAttributes)
@@ -851,14 +985,15 @@ extern "C" UINT __stdcall CreateUser(
         MessageExitOnFailure(hr, msierrUSRFailedGrantLogonAsService, "Failed to grant logon as batch job rights to user: %ls", pwzName);
     }
 
-    //
-    // Add the users to groups
-    //
-    while (S_OK == (hr = WcaReadStringFromCaData(&pwz, &pwzGroup)))
-    {
-        hr = WcaReadStringFromCaData(&pwz, &pwzGroupDomain);
-        ExitOnFailure(hr, "failed to get domain for group: %ls", pwzGroup);
+//
+// Add the users to groups
+//
+while (S_OK == (hr = WcaReadStringFromCaData(&pwz, &pwzGroup)))
+{
+    hr = WcaReadStringFromCaData(&pwz, &pwzGroupDomain);
+    ExitOnFailure(hr, "failed to get domain for group: %ls", pwzGroup);
 
+        WcaLog(LOGMSG_STANDARD, "Adding user %ls\\%ls to group %ls\\%ls", pwzDomain, pwzName, pwzGroupDomain, pwzGroup);
         hr = AddUserToGroup(pwzName, pwzDomain, pwzGroup, pwzGroupDomain);
         MessageExitOnFailure(hr, msierrUSRFailedUserGroupAdd, "failed to add user: %ls to group %ls", pwzName, pwzGroup);
     }
@@ -866,24 +1001,23 @@ extern "C" UINT __stdcall CreateUser(
     {
         hr = S_OK;
     }
+
     ExitOnFailure(hr, "failed to get next group in which to include user:%ls", pwzName);
 
+ExitOnFailure(hr, "failed to get next group in which to include user:%ls", pwzName);
+
 LExit:
     WcaCaScriptClose(hRollbackScript, WCA_CASCRIPT_CLOSE_PRESERVE);
 
-    if (puserInfo)
+    if (pUserInfo1 && pUserInfo1 != &userInfo1)
     {
-        ::NetApiBufferFree((LPVOID)puserInfo);
-    }
-
-    if (pDomainControllerInfo)
-    {
-        ::NetApiBufferFree((LPVOID)pDomainControllerInfo);
+        ::NetApiBufferFree((LPVOID)pUserInfo1);
     }
 
     ReleaseStr(pwzData);
     ReleaseStr(pwzName);
     ReleaseStr(pwzDomain);
+    ReleaseStr(pwzComment);
     ReleaseStr(pwzScriptKey);
     ReleaseStr(pwzPassword);
     ReleaseStr(pwzGroup);
@@ -922,15 +1056,17 @@ extern "C" UINT __stdcall CreateUserRollback(
 
     LPWSTR pwzData = NULL;
     LPWSTR pwz = NULL;
+    LPWSTR pwzScriptKey = NULL;
     LPWSTR pwzName = NULL;
     LPWSTR pwzDomain = NULL;
-    LPWSTR pwzScriptKey = NULL;
+    LPWSTR pwzComment = NULL;
     int iAttributes = 0;
     BOOL fInitializedCom = FALSE;
 
     WCA_CASCRIPT_HANDLE hRollbackScript = NULL;
     LPWSTR pwzRollbackData = NULL;
     int iOriginalAttributes = 0;
+    LPWSTR pwzOriginalComment = NULL;
 
     hr = WcaInitialize(hInstall, "CreateUserRollback");
     ExitOnFailure(hr, "failed to initialize");
@@ -957,6 +1093,9 @@ extern "C" UINT __stdcall CreateUserRollback(
     hr = WcaReadStringFromCaData(&pwz, &pwzDomain);
     ExitOnFailure(hr, "failed to read domain from custom action data");
 
+    hr = WcaReadStringFromCaData(&pwz, &pwzComment);
+    ExitOnFailure(hr, "failed to read comment from custom action data");
+
     hr = WcaReadIntegerFromCaData(&pwz, &iAttributes);
     ExitOnFailure(hr, "failed to read attributes from custom action data");
 
@@ -978,6 +1117,15 @@ extern "C" UINT __stdcall CreateUserRollback(
             WcaLog(LOGMSG_TRACEONLY, "Rollback Data: %ls", pwzRollbackData);
 
             pwz = pwzRollbackData;
+            hr = WcaReadStringFromCaData(&pwz, &pwzOriginalComment);
+            if (FAILED(hr))
+            {
+                WcaLogError(hr, "failed to read comment from rollback data, continuing anyway");
+            }
+            else
+            {
+                pwzComment = pwzOriginalComment;
+            }
             hr = WcaReadIntegerFromCaData(&pwz, &iOriginalAttributes);
             if (FAILED(hr))
             {
@@ -998,8 +1146,10 @@ LExit:
     ReleaseStr(pwzData);
     ReleaseStr(pwzName);
     ReleaseStr(pwzDomain);
+    ReleaseStr(pwzComment);
     ReleaseStr(pwzScriptKey);
     ReleaseStr(pwzRollbackData);
+    ReleaseStr(pwzOriginalComment);
 
     if (fInitializedCom)
     {
@@ -1033,6 +1183,7 @@ extern "C" UINT __stdcall RemoveUser(
     LPWSTR pwz = NULL;
     LPWSTR pwzName = NULL;
     LPWSTR pwzDomain = NULL;
+    LPWSTR pwzComment = NULL;
     int iAttributes = 0;
     BOOL fInitializedCom = FALSE;
 
@@ -1058,6 +1209,9 @@ extern "C" UINT __stdcall RemoveUser(
     hr = WcaReadStringFromCaData(&pwz, &pwzDomain);
     ExitOnFailure(hr, "failed to read domain from custom action data");
 
+    hr = WcaReadStringFromCaData(&pwz, &pwzComment);
+    ExitOnFailure(hr, "failed to read comment from custom action data");
+
     hr = WcaReadIntegerFromCaData(&pwz, &iAttributes);
     ExitOnFailure(hr, "failed to read attributes from custom action data");
 
@@ -1067,6 +1221,7 @@ LExit:
     ReleaseStr(pwzData);
     ReleaseStr(pwzName);
     ReleaseStr(pwzDomain);
+    ReleaseStr(pwzComment);
 
     if (fInitializedCom)
     {
diff --git a/src/ext/Util/ca/scauser.cpp b/src/ext/Util/ca/scauser.cpp
index f92ebf1b..dc5bebba 100644
--- a/src/ext/Util/ca/scauser.cpp
+++ b/src/ext/Util/ca/scauser.cpp
@@ -2,8 +2,8 @@
 
 #include "precomp.h"
 
-LPCWSTR vcsUserQuery = L"SELECT `User`, `Component_`, `Name`, `Domain`, `Password` FROM `Wix4User` WHERE `User`=?";
-enum eUserQuery { vuqUser = 1, vuqComponent, vuqName, vuqDomain, vuqPassword };
+LPCWSTR vcsUserQuery = L"SELECT `User`, `Component_`, `Name`, `Domain`, `Comment`, `Password` FROM `Wix4User` WHERE `User`=?";
+enum eUserQuery { vuqUser = 1, vuqComponent, vuqName, vuqDomain, vuqComment, vuqPassword };
 
 LPCWSTR vcsGroupQuery = L"SELECT `Group`, `Component_`, `Name`, `Domain` FROM `Wix4Group` WHERE `Group`=?";
 enum eGroupQuery { vgqGroup = 1, vgqComponent, vgqName, vgqDomain };
@@ -11,8 +11,8 @@ enum eGroupQuery { vgqGroup = 1, vgqComponent, vgqName, vgqDomain };
 LPCWSTR vcsUserGroupQuery = L"SELECT `User_`, `Group_` FROM `Wix4UserGroup` WHERE `User_`=?";
 enum eUserGroupQuery { vugqUser = 1, vugqGroup };
 
-LPCWSTR vActionableQuery = L"SELECT `User`,`Component_`,`Name`,`Domain`,`Password`,`Attributes` FROM `Wix4User` WHERE `Component_` IS NOT NULL";
-enum eActionableQuery { vaqUser = 1, vaqComponent, vaqName, vaqDomain, vaqPassword, vaqAttributes };
+LPCWSTR vActionableQuery = L"SELECT `User`,`Component_`,`Name`,`Domain`,`Password`,`Comment`,`Attributes` FROM `Wix4User` WHERE `Component_` IS NOT NULL";
+enum eActionableQuery { vaqUser = 1, vaqComponent, vaqName, vaqDomain, vaqPassword, vaqComment, vaqAttributes };
 
 
 static HRESULT AddUserToList(
@@ -78,6 +78,11 @@ HRESULT __stdcall ScaGetUser(
         hr = ::StringCchCopyW(pscau->wzDomain, countof(pscau->wzDomain), pwzData);
         ExitOnFailure(hr, "Failed to copy domain string to user object");
 
+        hr = WcaGetRecordFormattedString(hRec, vuqComment, &pwzData);
+        ExitOnFailure(hr, "Failed to get Wix4User.Comment");
+        hr = ::StringCchCopyW(pscau->wzComment, countof(pscau->wzComment), pwzData);
+        ExitOnFailure(hr, "Failed to copy comment string to user object");
+
         hr = WcaGetRecordFormattedString(hRec, vuqPassword, &pwzData);
         ExitOnFailure(hr, "Failed to get Wix4User.Password");
         hr = ::StringCchCopyW(pscau->wzPassword, countof(pscau->wzPassword), pwzData);
@@ -154,6 +159,11 @@ HRESULT __stdcall ScaGetUserDeferred(
         hr = ::StringCchCopyW(pscau->wzDomain, countof(pscau->wzDomain), pwzData);
         ExitOnFailure(hr, "Failed to copy domain string to user object (in deferred CA)");
 
+        hr = WcaGetRecordString(hRec, vuqComment, &pwzData);
+        ExitOnFailure(hr, "Failed to get Wix4User.Comment");
+        hr = ::StringCchCopyW(pscau->wzComment, countof(pscau->wzComment), pwzData);
+        ExitOnFailure(hr, "Failed to copy comment string to user object (in deferred CA)");
+
         hr = WcaGetRecordString(hRec, vuqPassword, &pwzData);
         ExitOnFailure(hr, "Failed to get Wix4User.Password");
         hr = ::StringCchCopyW(pscau->wzPassword, countof(pscau->wzPassword), pwzData);
@@ -316,7 +326,7 @@ HRESULT ScaUserRead(
         ExitOnFailure(hr, "failed to get Component state for Wix4User");
 
         // don't bother if we aren't installing or uninstalling this component
-        if (WcaIsInstalling(isInstalled,  isAction) || WcaIsUninstalling(isInstalled, isAction))
+        if (WcaIsInstalling(isInstalled, isAction) || WcaIsUninstalling(isInstalled, isAction))
         {
             //
             // Add the user to the list and populate it's values
@@ -345,6 +355,10 @@ HRESULT ScaUserRead(
             ExitOnFailure(hr, "failed to get Wix4User.Domain");
             hr = ::StringCchCopyW(psu->wzDomain, countof(psu->wzDomain), pwzData);
             ExitOnFailure(hr, "failed to copy user domain: %ls", pwzData);
+            hr = WcaGetRecordFormattedString(hRec, vaqComment, &pwzData);
+            ExitOnFailure(hr, "failed to get Wix4User.Comment");
+            hr = ::StringCchCopyW(psu->wzComment, countof(psu->wzComment), pwzData);
+            ExitOnFailure(hr, "failed to copy user comment: %ls", pwzData);
 
             hr = WcaGetRecordFormattedString(hRec, vaqPassword, &pwzData);
             ExitOnFailure(hr, "failed to get Wix4User.Password");
@@ -492,15 +506,16 @@ HRESULT ScaUserExecute(
     {
         USER_EXISTS ueUserExists = USER_EXISTS_INDETERMINATE;
 
-        // Always put the User Name and Domain plus Attributes on the front of the CustomAction
-        // data.  Sometimes we'll add more data.
+        // Always put the User Name, Domain, and Comment on the front of the CustomAction data.
+        // The attributes will be added when we have finished adjusting them. Sometimes we'll
+        // add more data.
         Assert(psu->wzName);
         hr = WcaWriteStringToCaData(psu->wzName, &pwzActionData);
         ExitOnFailure(hr, "Failed to add user name to custom action data: %ls", psu->wzName);
         hr = WcaWriteStringToCaData(psu->wzDomain, &pwzActionData);
         ExitOnFailure(hr, "Failed to add user domain to custom action data: %ls", psu->wzDomain);
-        hr = WcaWriteIntegerToCaData(psu->iAttributes, &pwzActionData);
-        ExitOnFailure(hr, "failed to add user attributes to custom action data for user: %ls", psu->wzKey);
+        hr = WcaWriteStringToCaData(psu->wzComment, &pwzActionData);
+        ExitOnFailure(hr, "Failed to add user comment to custom action data: %ls", psu->wzComment);
 
         // 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
@@ -520,7 +535,12 @@ HRESULT ScaUserExecute(
             }
             if (ERROR_SUCCESS == er)
             {
-                wzDomain = pDomainControllerInfo->DomainControllerName + 2;  //Add 2 so that we don't get the \\ prefix
+                if (2 <= wcslen(pDomainControllerInfo->DomainControllerName))
+                {
+                    wzDomain = pDomainControllerInfo->DomainControllerName + 2; // Add 2 so that we don't get the \\ prefix.
+                                                                                // Pass the entire string if it is too short
+                                                                                // to have a \\ prefix.
+                }
             }
         }
 
@@ -544,23 +564,32 @@ HRESULT ScaUserExecute(
 
         if (WcaIsInstalling(psu->isInstalled, psu->isAction))
         {
-            // If the user exists, check to see if we are supposed to fail if user the exists before
+            // If the user exists, check to see if we are supposed to fail if the user exists before
             // the install.
             if (USER_EXISTS_YES == ueUserExists)
             {
-                // Reinstalls will always fail if we don't remove the check for "fail if exists".
+                // Re-installs will always fail if we don't remove the check for "fail if exists".
                 if (WcaIsReInstalling(psu->isInstalled, psu->isAction))
                 {
                     psu->iAttributes &= ~SCAU_FAIL_IF_EXISTS;
+
+                    // If install would create the user, re-install should be able to update the user.
+                    if (!(psu->iAttributes & SCAU_DONT_CREATE_USER))
+                    {
+                        psu->iAttributes |= SCAU_UPDATE_IF_EXISTS;
+                    }
                 }
 
-                if ((SCAU_FAIL_IF_EXISTS & (psu->iAttributes)) && !(SCAU_UPDATE_IF_EXISTS & (psu->iAttributes)))
+                if (SCAU_FAIL_IF_EXISTS & psu->iAttributes && !(SCAU_UPDATE_IF_EXISTS & psu->iAttributes))
                 {
                     hr = HRESULT_FROM_WIN32(NERR_UserExists);
                     MessageExitOnFailure(hr, msierrUSRFailedUserCreateExists, "Failed to create user: %ls because user already exists.", psu->wzName);
                 }
             }
 
+            hr = WcaWriteIntegerToCaData(psu->iAttributes, &pwzActionData);
+            ExitOnFailure(hr, "failed to add user attributes to custom action data for user: %ls", psu->wzKey);
+
             // Rollback only if the user already exists, we couldn't determine if the user exists, or we are going to create the user
             if ((USER_EXISTS_YES == ueUserExists) || (USER_EXISTS_INDETERMINATE == ueUserExists) || !(psu->iAttributes & SCAU_DONT_CREATE_USER))
             {
@@ -597,7 +626,6 @@ HRESULT ScaUserExecute(
                 ExitOnFailure(hr, "Failed to add user domain to rollback custom action data: %ls", psu->wzDomain);
                 hr = WcaWriteIntegerToCaData(iRollbackUserAttributes, &pwzRollbackData);
                 ExitOnFailure(hr, "failed to add user attributes to rollback custom action data for user: %ls", psu->wzKey);
-
                 // If the user already exists, add relevant group information to rollback data
                 if (USER_EXISTS_YES == ueUserExists || USER_EXISTS_INDETERMINATE == ueUserExists)
                 {
@@ -630,11 +658,13 @@ HRESULT ScaUserExecute(
         }
         else if (((USER_EXISTS_YES == ueUserExists) || (USER_EXISTS_INDETERMINATE == ueUserExists)) && WcaIsUninstalling(psu->isInstalled, psu->isAction) && !(psu->iAttributes & SCAU_DONT_REMOVE_ON_UNINSTALL))
         {
+            hr = WcaWriteIntegerToCaData(psu->iAttributes, &pwzActionData);
+            ExitOnFailure(hr, "failed to add user attributes to custom action data for user: %ls", psu->wzKey);
+
             // Add user's group information - this will ensure the user can be removed from any groups they were added to, if the user isn't be deleted
             hr = WriteGroupInfo(psu->psgGroups, &pwzActionData);
             ExitOnFailure(hr, "failed to add group information to custom action data");
 
-            //
             // Schedule the removal because the user exists and we don't have any flags set
             // that say, don't remove the user on uninstall.
             //
@@ -642,7 +672,7 @@ HRESULT ScaUserExecute(
             // CustomAction.
             hr = WcaDoDeferredAction(CUSTOM_ACTION_DECORATION(L"RemoveUser"), pwzActionData, COST_USER_DELETE);
             ExitOnFailure(hr, "failed to schedule RemoveUser");
-        }
+       }
 
         ReleaseNullStr(pwzScriptKey);
         ReleaseNullStr(pwzActionData);
diff --git a/src/ext/Util/ca/scauser.h b/src/ext/Util/ca/scauser.h
index a5fd5ea8..3da847b5 100644
--- a/src/ext/Util/ca/scauser.h
+++ b/src/ext/Util/ca/scauser.h
@@ -31,6 +31,7 @@ struct SCA_USER
     WCHAR wzDomain[MAX_DARWIN_COLUMN + 1];
     WCHAR wzName[MAX_DARWIN_COLUMN + 1];
     WCHAR wzPassword[MAX_DARWIN_COLUMN + 1];
+    WCHAR wzComment[MAX_DARWIN_COLUMN + 1];
     INT iAttributes;
 
     SCA_GROUP *psgGroups;
@@ -41,16 +42,16 @@ struct SCA_USER
 
 // prototypes
 HRESULT __stdcall ScaGetUser(
-    __in LPCWSTR wzUser, 
+    __in LPCWSTR wzUser,
     __out SCA_USER* pscau
     );
 HRESULT __stdcall ScaGetUserDeferred(
-    __in LPCWSTR wzUser, 
+    __in LPCWSTR wzUser,
     __in WCA_WRAPQUERY_HANDLE hUserQuery,
     __out SCA_USER* pscau
     );
 HRESULT __stdcall ScaGetGroup(
-    __in LPCWSTR wzGroup, 
+    __in LPCWSTR wzGroup,
     __out SCA_GROUP* pscag
     );
 void ScaUserFreeList(