From 08cdc6aa2b9dd0e273a3c3a22893616d26342a0e Mon Sep 17 00:00:00 2001 From: Ron Martin Date: Mon, 29 Aug 2022 18:38:07 -0400 Subject: Support add, modify and remove comments on user accounts Fixes 5371 --- src/ext/Iis/ca/sca.h | 1 + src/ext/Util/ca/sca.h | 3 +- src/ext/Util/ca/scaexec.cpp | 375 +++++++++++++++------ src/ext/Util/ca/scauser.cpp | 62 +++- src/ext/Util/ca/scauser.h | 7 +- .../TestData/CreateUser/Package.wxs | 15 + .../TestData/CreateUser/PackageComponents.wxs | 76 +++++ .../WixToolsetTest.Util/UtilExtensionFixture.cs | 76 +++++ src/ext/Util/wixext/Symbols/UserSymbol.cs | 10 +- src/ext/Util/wixext/UtilCompiler.cs | 22 ++ src/ext/Util/wixext/UtilDecompiler.cs | 6 +- src/ext/Util/wixext/UtilTableDefinitions.cs | 1 + src/libs/wcautil/WixToolset.WcaUtil/wcawrap.cpp | 11 +- src/test/burn/WixTestTools/MSIExec.cs | 2 +- src/test/burn/WixTestTools/UserVerifier.cs | 41 ++- .../ProductA/ProductA.wixproj | 2 +- .../UtilExtensionUserTests/ProductA/product.wxs | 6 +- .../ProductAddCommentToExistingUser.wixproj | 13 + .../ProductAddCommentToExistingUser/product.wxs | 25 ++ .../ProductCommentDelete.wixproj | 13 + .../ProductCommentDelete/product.wxs | 18 + .../ProductCommentFail/ProductCommentFail.wixproj | 13 + .../ProductCommentFail/product_fail.wxs | 22 ++ .../ProductFail/product_fail.wxs | 2 +- .../ProductNewUserWithComment.wixproj | 13 + .../ProductNewUserWithComment/product.wxs | 25 ++ .../ProductWithCommandLineParameters.wixproj | 13 + .../ProductWithCommandLineParameters.wxs | 21 ++ .../UtilExtensionUserTests.cs | 186 ++++++++-- 29 files changed, 900 insertions(+), 180 deletions(-) create mode 100644 src/ext/Util/test/WixToolsetTest.Util/TestData/CreateUser/Package.wxs create mode 100644 src/ext/Util/test/WixToolsetTest.Util/TestData/CreateUser/PackageComponents.wxs create mode 100644 src/test/msi/TestData/UtilExtensionUserTests/ProductAddCommentToExistingUser/ProductAddCommentToExistingUser.wixproj create mode 100644 src/test/msi/TestData/UtilExtensionUserTests/ProductAddCommentToExistingUser/product.wxs create mode 100644 src/test/msi/TestData/UtilExtensionUserTests/ProductCommentDelete/ProductCommentDelete.wixproj create mode 100644 src/test/msi/TestData/UtilExtensionUserTests/ProductCommentDelete/product.wxs create mode 100644 src/test/msi/TestData/UtilExtensionUserTests/ProductCommentFail/ProductCommentFail.wixproj create mode 100644 src/test/msi/TestData/UtilExtensionUserTests/ProductCommentFail/product_fail.wxs create mode 100644 src/test/msi/TestData/UtilExtensionUserTests/ProductNewUserWithComment/ProductNewUserWithComment.wixproj create mode 100644 src/test/msi/TestData/UtilExtensionUserTests/ProductNewUserWithComment/product.wxs create mode 100644 src/test/msi/TestData/UtilExtensionUserTests/ProductWithCommandLineParameters/ProductWithCommandLineParameters.wixproj create mode 100644 src/test/msi/TestData/UtilExtensionUserTests/ProductWithCommandLineParameters/ProductWithCommandLineParameters.wxs diff --git a/src/ext/Iis/ca/sca.h b/src/ext/Iis/ca/sca.h index 64567dcb..6921613b 100644 --- a/src/ext/Iis/ca/sca.h +++ b/src/ext/Iis/ca/sca.h @@ -121,4 +121,5 @@ enum SCAU_ATTRIBUTES SCAU_DONT_REMOVE_ON_UNINSTALL = 0x00000100, SCAU_DONT_CREATE_USER = 0x00000200, SCAU_NON_VITAL = 0x00000400, + SCAU_REMOVE_COMMENT = 0x00000800, }; 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(&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(&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(&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(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(&userInfo), &dw); - if (NERR_UserExists == er) - { if (SCAU_UPDATE_IF_EXISTS & iAttributes) { - er = ::NetUserGetInfo(wz, pwzName, 1, reinterpret_cast(&puserInfo)); - if (NERR_Success == er) + pUserInfo1 = NULL; + er = ::NetUserGetInfo(pwzServerName, pwzName, 1, reinterpret_cast(&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(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( diff --git a/src/ext/Util/test/WixToolsetTest.Util/TestData/CreateUser/Package.wxs b/src/ext/Util/test/WixToolsetTest.Util/TestData/CreateUser/Package.wxs new file mode 100644 index 00000000..af861986 --- /dev/null +++ b/src/ext/Util/test/WixToolsetTest.Util/TestData/CreateUser/Package.wxs @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + diff --git a/src/ext/Util/test/WixToolsetTest.Util/TestData/CreateUser/PackageComponents.wxs b/src/ext/Util/test/WixToolsetTest.Util/TestData/CreateUser/PackageComponents.wxs new file mode 100644 index 00000000..9fc7ba23 --- /dev/null +++ b/src/ext/Util/test/WixToolsetTest.Util/TestData/CreateUser/PackageComponents.wxs @@ -0,0 +1,76 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/ext/Util/test/WixToolsetTest.Util/UtilExtensionFixture.cs b/src/ext/Util/test/WixToolsetTest.Util/UtilExtensionFixture.cs index 24641fce..3da5f671 100644 --- a/src/ext/Util/test/WixToolsetTest.Util/UtilExtensionFixture.cs +++ b/src/ext/Util/test/WixToolsetTest.Util/UtilExtensionFixture.cs @@ -296,6 +296,82 @@ namespace WixToolsetTest.Util } } + [Fact] + public void CanCreateUserAccountWithComment() + { + var folder = TestData.Get(@"TestData\CreateUser"); + var build = new Builder(folder, typeof(UtilExtensionFactory), new[] { folder }); + + var results = build.BuildAndQuery(Build, "Wix4User"); + WixAssert.CompareLineByLine(new[] + { + "Wix4User:TEST_USER00\tComponent1\ttestName00\t\ttest123!@#\tTest Comment 1\t1792", + "Wix4User:TEST_USER01\tComponent1\ttestName01\t\ttest123!@#\tTest Comment 1\t1796", + "Wix4User:TEST_USER02\tComponent1\ttestName02\t\ttest123!@#\t\t3840", + "Wix4User:TEST_USER03\tComponent1\ttestName03\t\ttest123!@#\t\t3844", + "Wix4User:TEST_USER04\tComponent1\ttestName04\t\ttest123!@#\tTest Comment 1\t1280", + "Wix4User:TEST_USER05\tComponent1\ttestName05\t\ttest123!@#\tTest Comment 1\t1284", + "Wix4User:TEST_USER06\tComponent1\ttestName06\t\ttest123!@#\t\t3328", + "Wix4User:TEST_USER07\tComponent1\ttestName07\t\ttest123!@#\t\t3332", + "Wix4User:TEST_USER10\tComponent1\ttestName10\t\ttest123!@#\tTest Comment 1\t1808", + "Wix4User:TEST_USER11\tComponent1\ttestName11\t\ttest123!@#\tTest Comment 1\t1812", + "Wix4User:TEST_USER12\tComponent1\ttestName12\t\ttest123!@#\t\t3856", + "Wix4User:TEST_USER13\tComponent1\ttestName13\t\ttest123!@#\t\t3860", + "Wix4User:TEST_USER14\tComponent1\ttestName14\t\ttest123!@#\tTest Comment 1\t1296", + "Wix4User:TEST_USER15\tComponent1\ttestName15\t\ttest123!@#\tTest Comment 1\t1300", + "Wix4User:TEST_USER16\tComponent1\ttestName16\t\ttest123!@#\t\t3344", + "Wix4User:TEST_USER17\tComponent1\ttestName17\t\ttest123!@#\t\t3348", + "Wix4User:TEST_USER20\tComponent1\ttestName20\t\ttest123!@#\tTest Comment 1\t768", + "Wix4User:TEST_USER21\tComponent1\ttestName21\t\ttest123!@#\tTest Comment 1\t772", + "Wix4User:TEST_USER22\tComponent1\ttestName22\t\ttest123!@#\t\t2816", + "Wix4User:TEST_USER23\tComponent1\ttestName23\t\ttest123!@#\t\t2820", + "Wix4User:TEST_USER24\tComponent1\ttestName24\t\ttest123!@#\tTest Comment 1\t256", + "Wix4User:TEST_USER25\tComponent1\ttestName25\t\ttest123!@#\tTest Comment 1\t260", + "Wix4User:TEST_USER26\tComponent1\ttestName26\t\ttest123!@#\t\t2304", + "Wix4User:TEST_USER27\tComponent1\ttestName27\t\ttest123!@#\t\t2308", + "Wix4User:TEST_USER30\tComponent1\ttestName30\t\ttest123!@#\tTest Comment 1\t784", + "Wix4User:TEST_USER31\tComponent1\ttestName31\t\ttest123!@#\tTest Comment 1\t788", + "Wix4User:TEST_USER32\tComponent1\ttestName32\t\ttest123!@#\t\t2832", + "Wix4User:TEST_USER33\tComponent1\ttestName33\t\ttest123!@#\t\t2836", + "Wix4User:TEST_USER34\tComponent1\ttestName34\t\ttest123!@#\tTest Comment 1\t272", + "Wix4User:TEST_USER35\tComponent1\ttestName35\t\ttest123!@#\tTest Comment 1\t276", + "Wix4User:TEST_USER36\tComponent1\ttestName36\t\ttest123!@#\t\t2320", + "Wix4User:TEST_USER37\tComponent1\ttestName37\t\ttest123!@#\t\t2324", + "Wix4User:TEST_USER40\tComponent1\ttestName40\t\ttest123!@#\tTest Comment 1\t1536", + "Wix4User:TEST_USER41\tComponent1\ttestName41\t\ttest123!@#\tTest Comment 1\t1540", + "Wix4User:TEST_USER42\tComponent1\ttestName42\t\ttest123!@#\t\t3584", + "Wix4User:TEST_USER43\tComponent1\ttestName43\t\ttest123!@#\t\t3588", + "Wix4User:TEST_USER44\tComponent1\ttestName44\t\ttest123!@#\tTest Comment 1\t1024", + "Wix4User:TEST_USER45\tComponent1\ttestName45\t\ttest123!@#\tTest Comment 1\t1028", + "Wix4User:TEST_USER46\tComponent1\ttestName46\t\ttest123!@#\t\t3072", + "Wix4User:TEST_USER47\tComponent1\ttestName47\t\ttest123!@#\t\t3076", + "Wix4User:TEST_USER50\tComponent1\ttestName50\t\ttest123!@#\tTest Comment 1\t1552", + "Wix4User:TEST_USER51\tComponent1\ttestName51\t\ttest123!@#\tTest Comment 1\t1556", + "Wix4User:TEST_USER52\tComponent1\ttestName52\t\ttest123!@#\t\t3600", + "Wix4User:TEST_USER53\tComponent1\ttestName53\t\ttest123!@#\t\t3604", + "Wix4User:TEST_USER54\tComponent1\ttestName54\t\ttest123!@#\tTest Comment 1\t1040", + "Wix4User:TEST_USER55\tComponent1\ttestName55\t\ttest123!@#\tTest Comment 1\t1044", + "Wix4User:TEST_USER56\tComponent1\ttestName56\t\ttest123!@#\t\t3088", + "Wix4User:TEST_USER57\tComponent1\ttestName57\t\ttest123!@#\t\t3092", + "Wix4User:TEST_USER60\tComponent1\ttestName60\t\ttest123!@#\tTest Comment 1\t512", + "Wix4User:TEST_USER61\tComponent1\ttestName61\t\ttest123!@#\tTest Comment 1\t516", + "Wix4User:TEST_USER62\tComponent1\ttestName62\t\ttest123!@#\t\t2560", + "Wix4User:TEST_USER63\tComponent1\ttestName63\t\ttest123!@#\t\t2564", + "Wix4User:TEST_USER64\tComponent1\ttestName64\t\ttest123!@#\tTest Comment 1\t0", + "Wix4User:TEST_USER65\tComponent1\ttestName65\t\ttest123!@#\tTest Comment 1\t4", + "Wix4User:TEST_USER66\tComponent1\ttestName66\t\ttest123!@#\t\t2048", + "Wix4User:TEST_USER67\tComponent1\ttestName67\t\ttest123!@#\t\t2052", + "Wix4User:TEST_USER70\tComponent1\ttestName70\t\ttest123!@#\tTest Comment 1\t528", + "Wix4User:TEST_USER71\tComponent1\ttestName71\t\ttest123!@#\tTest Comment 1\t532", + "Wix4User:TEST_USER72\tComponent1\ttestName72\t\ttest123!@#\t\t2576", + "Wix4User:TEST_USER73\tComponent1\ttestName73\t\ttest123!@#\t\t2580", + "Wix4User:TEST_USER74\tComponent1\ttestName74\t\ttest123!@#\tTest Comment 1\t16", + "Wix4User:TEST_USER75\tComponent1\ttestName75\t\ttest123!@#\tTest Comment 1\t20", + "Wix4User:TEST_USER76\tComponent1\ttestName76\t\ttest123!@#\t\t2064", + "Wix4User:TEST_USER77\tComponent1\ttestName77\t\ttest123!@#\t\t2068", + }, results.OrderBy(s => s).ToArray()); + } + [Fact] public void CanBuildBundleWithWarningsWithSearchesUsingDiscouragedVariableNames() { diff --git a/src/ext/Util/wixext/Symbols/UserSymbol.cs b/src/ext/Util/wixext/Symbols/UserSymbol.cs index 5f00064b..6ea810de 100644 --- a/src/ext/Util/wixext/Symbols/UserSymbol.cs +++ b/src/ext/Util/wixext/Symbols/UserSymbol.cs @@ -15,6 +15,7 @@ namespace WixToolset.Util new IntermediateFieldDefinition(nameof(UserSymbolFields.Name), IntermediateFieldType.String), new IntermediateFieldDefinition(nameof(UserSymbolFields.Domain), IntermediateFieldType.String), new IntermediateFieldDefinition(nameof(UserSymbolFields.Password), IntermediateFieldType.String), + new IntermediateFieldDefinition(nameof(UserSymbolFields.Comment), IntermediateFieldType.String), new IntermediateFieldDefinition(nameof(UserSymbolFields.Attributes), IntermediateFieldType.Number), }, typeof(UserSymbol)); @@ -31,6 +32,7 @@ namespace WixToolset.Util.Symbols Name, Domain, Password, + Comment, Attributes, } @@ -70,10 +72,16 @@ namespace WixToolset.Util.Symbols set => this.Set((int)UserSymbolFields.Password, value); } + public string Comment + { + get => this.Fields[(int)UserSymbolFields.Comment].AsString(); + set => this.Set((int)UserSymbolFields.Comment, value); + } + public int Attributes { get => this.Fields[(int)UserSymbolFields.Attributes].AsNumber(); set => this.Set((int)UserSymbolFields.Attributes, value); } } -} \ No newline at end of file +} diff --git a/src/ext/Util/wixext/UtilCompiler.cs b/src/ext/Util/wixext/UtilCompiler.cs index a6e4b835..d937b4f1 100644 --- a/src/ext/Util/wixext/UtilCompiler.cs +++ b/src/ext/Util/wixext/UtilCompiler.cs @@ -34,6 +34,7 @@ namespace WixToolset.Util internal const int UserDontRemoveOnUninstall = 0x00000100; internal const int UserDontCreateUser = 0x00000200; internal const int UserNonVital = 0x00000400; + internal const int UserRemoveComment = 0x00000800; private static readonly Regex FindPropertyBrackets = new Regex(@"\[(?!\\|\])|(? + /// Sets the user comment for a given user + /// + /// domain name for the user, empty for local users + /// the user name + /// comment to be set for the user + public static void SetUserComment(string domainName, string userName, string comment) + { + UserPrincipal user = GetUser(domainName, userName); + + Assert.False(null == user, String.Format("User '{0}' was not found under domain '{1}'.", userName, domainName)); + + var directoryEntry = user.GetUnderlyingObject() as DirectoryEntry; + Assert.False(null == directoryEntry); + directoryEntry.Properties["Description"].Value = comment; + user.Save(); + } + /// /// Adds the specified user to the specified local group /// @@ -162,7 +180,24 @@ namespace WixTestTools } /// - /// Verify that a givin user is member of a local group + /// Verifies the user comment for a given user + /// + /// domain name for the user, empty for local users + /// the user name + /// the comment to be verified + public static void VerifyUserComment(string domainName, string userName, string comment) + { + UserPrincipal user = GetUser(domainName, userName); + + Assert.False(null == user, String.Format("User '{0}' was not found under domain '{1}'.", userName, domainName)); + + var directoryEntry = user.GetUnderlyingObject() as DirectoryEntry; + Assert.False(null == directoryEntry); + Assert.True(comment == (string)(directoryEntry.Properties["Description"].Value)); + } + + /// + /// Verify that a given user is member of a local group /// /// domain name for the user, empty for local users /// the user name @@ -322,7 +357,6 @@ namespace WixTestTools missedAGroup = true; message += String.Format("Local group '{0}' was not found. \r\n", groupName); } - } Assert.False(missedAGroup, message); } @@ -346,3 +380,4 @@ namespace WixTestTools } } } + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductA/ProductA.wixproj b/src/test/msi/TestData/UtilExtensionUserTests/ProductA/ProductA.wixproj index fbc6f292..3895b853 100644 --- a/src/test/msi/TestData/UtilExtensionUserTests/ProductA/ProductA.wixproj +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductA/ProductA.wixproj @@ -1,7 +1,7 @@ - {1A1795A6-87C0-4A9A-ABD5-DF9BED697037} + {A3E0B539-63F9-4B43-9E34-F33AE1C6E06D} true diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductA/product.wxs b/src/test/msi/TestData/UtilExtensionUserTests/ProductA/product.wxs index a7bec54e..be6e16a4 100644 --- a/src/test/msi/TestData/UtilExtensionUserTests/ProductA/product.wxs +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductA/product.wxs @@ -15,10 +15,10 @@ - + - + @@ -27,7 +27,7 @@ - + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductAddCommentToExistingUser/ProductAddCommentToExistingUser.wixproj b/src/test/msi/TestData/UtilExtensionUserTests/ProductAddCommentToExistingUser/ProductAddCommentToExistingUser.wixproj new file mode 100644 index 00000000..5938e525 --- /dev/null +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductAddCommentToExistingUser/ProductAddCommentToExistingUser.wixproj @@ -0,0 +1,13 @@ + + + + {B33D3140-4AA5-469D-9DEE-AAF8F0C626DA} + true + + + + + + + + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductAddCommentToExistingUser/product.wxs b/src/test/msi/TestData/UtilExtensionUserTests/ProductAddCommentToExistingUser/product.wxs new file mode 100644 index 00000000..ce8c4cae --- /dev/null +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductAddCommentToExistingUser/product.wxs @@ -0,0 +1,25 @@ + + + + + + + + + + + + + + + + + + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentDelete/ProductCommentDelete.wixproj b/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentDelete/ProductCommentDelete.wixproj new file mode 100644 index 00000000..63bb2370 --- /dev/null +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentDelete/ProductCommentDelete.wixproj @@ -0,0 +1,13 @@ + + + + {9E4C301E-5F36-4A86-85BE-776E067D929D} + true + + + + + + + + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentDelete/product.wxs b/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentDelete/product.wxs new file mode 100644 index 00000000..f0fbc55e --- /dev/null +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentDelete/product.wxs @@ -0,0 +1,18 @@ + + + + + + + + + + + + + + + + + + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentFail/ProductCommentFail.wixproj b/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentFail/ProductCommentFail.wixproj new file mode 100644 index 00000000..66f308ae --- /dev/null +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentFail/ProductCommentFail.wixproj @@ -0,0 +1,13 @@ + + + + {85F698E0-F542-4CB4-80A1-6630D2DEB647} + true + + + + + + + + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentFail/product_fail.wxs b/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentFail/product_fail.wxs new file mode 100644 index 00000000..f36d5bd5 --- /dev/null +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductCommentFail/product_fail.wxs @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductFail/product_fail.wxs b/src/test/msi/TestData/UtilExtensionUserTests/ProductFail/product_fail.wxs index c5da862c..82472d4e 100644 --- a/src/test/msi/TestData/UtilExtensionUserTests/ProductFail/product_fail.wxs +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductFail/product_fail.wxs @@ -31,7 +31,7 @@ - + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductNewUserWithComment/ProductNewUserWithComment.wixproj b/src/test/msi/TestData/UtilExtensionUserTests/ProductNewUserWithComment/ProductNewUserWithComment.wixproj new file mode 100644 index 00000000..aeac903a --- /dev/null +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductNewUserWithComment/ProductNewUserWithComment.wixproj @@ -0,0 +1,13 @@ + + + + {549E1829-BBDE-42E1-968A-BEB8FC12BFC7} + true + + + + + + + + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductNewUserWithComment/product.wxs b/src/test/msi/TestData/UtilExtensionUserTests/ProductNewUserWithComment/product.wxs new file mode 100644 index 00000000..dde23aab --- /dev/null +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductNewUserWithComment/product.wxs @@ -0,0 +1,25 @@ + + + + + + + + + + + + + + + + + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductWithCommandLineParameters/ProductWithCommandLineParameters.wixproj b/src/test/msi/TestData/UtilExtensionUserTests/ProductWithCommandLineParameters/ProductWithCommandLineParameters.wixproj new file mode 100644 index 00000000..93a56216 --- /dev/null +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductWithCommandLineParameters/ProductWithCommandLineParameters.wixproj @@ -0,0 +1,13 @@ + + + + {79F2CB65-1E71-42EB-AA30-51BD70C29B23} + true + + + + + + + + diff --git a/src/test/msi/TestData/UtilExtensionUserTests/ProductWithCommandLineParameters/ProductWithCommandLineParameters.wxs b/src/test/msi/TestData/UtilExtensionUserTests/ProductWithCommandLineParameters/ProductWithCommandLineParameters.wxs new file mode 100644 index 00000000..564ce4f0 --- /dev/null +++ b/src/test/msi/TestData/UtilExtensionUserTests/ProductWithCommandLineParameters/ProductWithCommandLineParameters.wxs @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + diff --git a/src/test/msi/WixToolsetTest.MsiE2E/UtilExtensionUserTests.cs b/src/test/msi/WixToolsetTest.MsiE2E/UtilExtensionUserTests.cs index fcdfde52..30bc53e8 100644 --- a/src/test/msi/WixToolsetTest.MsiE2E/UtilExtensionUserTests.cs +++ b/src/test/msi/WixToolsetTest.MsiE2E/UtilExtensionUserTests.cs @@ -11,21 +11,14 @@ namespace WixToolsetTest.MsiE2E { public UtilExtensionUserTests(ITestOutputHelper testOutputHelper) : base(testOutputHelper) { } - const string TempDomain = "USERDOMAIN"; - const string TempUsername = "USERNAME"; - // Verify that the users specified in the authoring are created as expected. [RuntimeFact] public void CanInstallAndUninstallUsers() { - var arguments = new string[] - { - $"TEMPDOMAIN={Environment.GetEnvironmentVariable(TempDomain)}", - $"TEMPUSERNAME={Environment.GetEnvironmentVariable(TempUsername)}", - }; + UserVerifier.CreateLocalUser("testName3", "test123!@#"); var productA = this.CreatePackageInstaller("ProductA"); - productA.InstallProduct(MSIExec.MSIExecReturnCode.SUCCESS, arguments); + productA.InstallProduct(MSIExec.MSIExecReturnCode.SUCCESS); // Validate New User Information. UserVerifier.VerifyUserInformation(String.Empty, "testName1", true, false, false); @@ -34,62 +27,90 @@ namespace WixToolsetTest.MsiE2E UserVerifier.VerifyUserInformation(String.Empty, "testName2", true, true, true); UserVerifier.VerifyUserIsMemberOf(String.Empty, "testName2", "Power Users"); - UserVerifier.VerifyUserIsMemberOf(Environment.GetEnvironmentVariable(TempDomain), Environment.GetEnvironmentVariable(TempUsername), "Power Users"); + UserVerifier.VerifyUserIsMemberOf("", "testName3", "Power Users"); - productA.UninstallProduct(MSIExec.MSIExecReturnCode.SUCCESS, arguments); + productA.UninstallProduct(MSIExec.MSIExecReturnCode.SUCCESS); // Verify Users marked as RemoveOnUninstall were removed. Assert.False(UserVerifier.UserExists(String.Empty, "testName1"), String.Format("User '{0}' was not removed on Uninstall", "testName1")); Assert.True(UserVerifier.UserExists(String.Empty, "testName2"), String.Format("User '{0}' was removed on Uninstall", "testName2")); + // Verify that user added to power users group is removed on uninstall. + UserVerifier.VerifyUserIsNotMemberOf("", "testName3", "Power Users"); + // clean up + UserVerifier.DeleteLocalUser("testName1"); UserVerifier.DeleteLocalUser("testName2"); - - UserVerifier.VerifyUserIsNotMemberOf(Environment.GetEnvironmentVariable(TempDomain), Environment.GetEnvironmentVariable(TempUsername), "Power Users"); + UserVerifier.DeleteLocalUser("testName3"); } // Verify the rollback action reverts all Users changes. [RuntimeFact] public void CanRollbackUsers() { - var arguments = new string[] - { - $"TEMPDOMAIN={Environment.GetEnvironmentVariable(TempDomain)}", - $"TEMPUSERNAME={Environment.GetEnvironmentVariable(TempUsername)}", - }; + UserVerifier.CreateLocalUser("testName3", "test123!@#"); var productFail = this.CreatePackageInstaller("ProductFail"); // make sure the user accounts are deleted before we start UserVerifier.DeleteLocalUser("testName1"); UserVerifier.DeleteLocalUser("testName2"); - UserVerifier.VerifyUserIsNotMemberOf(Environment.GetEnvironmentVariable(TempDomain), Environment.GetEnvironmentVariable(TempUsername), "Power Users"); - productFail.InstallProduct(MSIExec.MSIExecReturnCode.ERROR_INSTALL_FAILURE, arguments); + productFail.InstallProduct(MSIExec.MSIExecReturnCode.ERROR_INSTALL_FAILURE); - // Verify Users marked as RemoveOnUninstall were removed. + // Verify added Users were removed on rollback. Assert.False(UserVerifier.UserExists(String.Empty, "testName1"), String.Format("User '{0}' was not removed on Rollback", "testName1")); Assert.False(UserVerifier.UserExists(String.Empty, "testName2"), String.Format("User '{0}' was not removed on Rollback", "testName2")); - UserVerifier.VerifyUserIsNotMemberOf(Environment.GetEnvironmentVariable(TempDomain), Environment.GetEnvironmentVariable(TempUsername), "Power Users"); + // Verify that user added to power users group is removed from power users group on rollback. + UserVerifier.VerifyUserIsNotMemberOf("", "testName3", "Power Users"); + + // clean up + UserVerifier.DeleteLocalUser("testName1"); + UserVerifier.DeleteLocalUser("testName2"); + UserVerifier.DeleteLocalUser("testName3"); } - // Verify that the users specified in the authoring are created as expected on repair. - [RuntimeFact(Skip = "Test demonstrates failure")] - public void CanRepairUsers() + + // Verify that command-line parameters aer 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()] + public void CanRepairUsersWithCommandLineParameters() { var arguments = new string[] { - $"TEMPDOMAIN={Environment.GetEnvironmentVariable(TempDomain)}", - $"TEMPUSERNAME={Environment.GetEnvironmentVariable(TempUsername)}", + "TESTPARAMETER1=testName1", }; + var productWithCommandLineParameters = this.CreatePackageInstaller("ProductWithCommandLineParameters"); + + // Make sure that the user doesn't exist when we start the test. + UserVerifier.DeleteLocalUser("testName1"); + + // Install + productWithCommandLineParameters.InstallProduct(MSIExec.MSIExecReturnCode.SUCCESS, arguments); + + // Repair + productWithCommandLineParameters.RepairProduct(MSIExec.MSIExecReturnCode.SUCCESS, arguments); + + // Clean up + UserVerifier.DeleteLocalUser("testName1"); + } + + + // Verify that the users specified in the authoring are created as expected on repair. + [RuntimeFact()] + public void CanRepairUsers() + { + UserVerifier.CreateLocalUser("testName3", "test123!@#"); var productA = this.CreatePackageInstaller("ProductA"); - productA.InstallProduct(MSIExec.MSIExecReturnCode.SUCCESS, arguments); + productA.InstallProduct(MSIExec.MSIExecReturnCode.SUCCESS); + // Validate New User Information. UserVerifier.DeleteLocalUser("testName1"); UserVerifier.SetUserInformation(String.Empty, "testName2", true, false, false); - productA.RepairProduct(MSIExec.MSIExecReturnCode.SUCCESS, arguments); + productA.RepairProduct(MSIExec.MSIExecReturnCode.SUCCESS); // Validate New User Information. UserVerifier.VerifyUserInformation(String.Empty, "testName1", true, false, false); @@ -98,21 +119,24 @@ namespace WixToolsetTest.MsiE2E UserVerifier.VerifyUserInformation(String.Empty, "testName2", true, true, true); UserVerifier.VerifyUserIsMemberOf(String.Empty, "testName2", "Power Users"); - UserVerifier.VerifyUserIsMemberOf(Environment.GetEnvironmentVariable(TempDomain), Environment.GetEnvironmentVariable(TempUsername), "Power Users"); + UserVerifier.VerifyUserIsMemberOf("", "testName3", "Power Users"); - productA.UninstallProduct(MSIExec.MSIExecReturnCode.SUCCESS, arguments); + productA.UninstallProduct(MSIExec.MSIExecReturnCode.SUCCESS); // Verify Users marked as RemoveOnUninstall were removed. Assert.False(UserVerifier.UserExists(String.Empty, "testName1"), String.Format("User '{0}' was not removed on Uninstall", "testName1")); Assert.True(UserVerifier.UserExists(String.Empty, "testName2"), String.Format("User '{0}' was removed on Uninstall", "testName2")); + // Verify that user added to power users group is removed on uninstall. + UserVerifier.VerifyUserIsNotMemberOf("", "testName3", "Power Users"); + // clean up + UserVerifier.DeleteLocalUser("testName1"); UserVerifier.DeleteLocalUser("testName2"); - - UserVerifier.VerifyUserIsNotMemberOf(Environment.GetEnvironmentVariable(TempDomain), Environment.GetEnvironmentVariable(TempUsername), "Power Users"); + UserVerifier.DeleteLocalUser("testName3"); } - // Verify that Installation fails if FailIfExisits is set. + // Verify that Installation fails if FailIfExists is set. [RuntimeFact] public void FailsIfUserExists() { @@ -135,7 +159,6 @@ namespace WixToolsetTest.MsiE2E // clean up UserVerifier.DeleteLocalUser("existinguser"); } - } // Verify that a user cannot be created on a domain on which you dont have create user permission. @@ -158,5 +181,98 @@ namespace WixToolsetTest.MsiE2E productNonVitalGroup.InstallProduct(); } + + // Verify that a user can be created with a user comment + [RuntimeFact] + public void CanCreateNewUserWithComment() + { + var productNewUserWithComment = this.CreatePackageInstaller("ProductNewUserWithComment"); + + productNewUserWithComment.InstallProduct(); + UserVerifier.VerifyUserComment(String.Empty, "testName1", "testComment1"); + + // clean up + UserVerifier.DeleteLocalUser("testName1"); + } + + // Verify that a comment can be added to an existing user + [RuntimeFact] + public void CanAddCommentToExistingUser() + { + UserVerifier.CreateLocalUser("testName1", "test123!@#"); + var productAddCommentToExistingUser = this.CreatePackageInstaller("ProductAddCommentToExistingUser"); + + productAddCommentToExistingUser.InstallProduct(); + + UserVerifier.VerifyUserComment(String.Empty, "testName1", "testComment1"); + + // clean up + UserVerifier.DeleteLocalUser("testName1"); + } + + // Verify that a comment can be repaired for a new user + [RuntimeFact] + public void CanRepairCommentOfNewUser() + { + var productNewUserWithComment = this.CreatePackageInstaller("ProductNewUserWithComment"); + + productNewUserWithComment.InstallProduct(); + UserVerifier.SetUserComment(String.Empty, "testName1", ""); + + productNewUserWithComment.RepairProduct(); + UserVerifier.VerifyUserComment(String.Empty, "testName1", "testComment1"); + + // clean up + UserVerifier.DeleteLocalUser("testName1"); + } + + // Verify that a comment can be changed for an existing user + [RuntimeFact] + public void CanChangeCommentOfExistingUser() + { + UserVerifier.CreateLocalUser("testName1", "test123!@#"); + UserVerifier.SetUserComment(String.Empty, "testName1", "initialTestComment1"); + var productNewUserWithComment = this.CreatePackageInstaller("ProductNewUserWithComment"); + + productNewUserWithComment.InstallProduct(); + UserVerifier.VerifyUserComment(String.Empty, "testName1", "testComment1"); + + // clean up + UserVerifier.DeleteLocalUser("testName1"); + } + + // Verify that a comment can be rolled back for an existing user + [RuntimeFact] + public void CanRollbackCommentOfExistingUser() + { + UserVerifier.CreateLocalUser("testName1", "test123!@#"); + UserVerifier.SetUserComment(String.Empty, "testName1", "initialTestComment1"); + var productCommentFail = this.CreatePackageInstaller("ProductCommentFail"); + + productCommentFail.InstallProduct(MSIExec.MSIExecReturnCode.ERROR_INSTALL_FAILURE); + + // Verify that comment change was rolled back. + UserVerifier.VerifyUserComment(String.Empty, "testName1", "initialTestComment1"); + + // clean up + UserVerifier.DeleteLocalUser("testName1"); + } + + // Verify that a comment can be deleted for an existing user + [RuntimeFact] + public void CanDeleteCommentOfExistingUser() + { + UserVerifier.CreateLocalUser("testName1", "test123!@#"); + UserVerifier.SetUserComment(String.Empty, "testName1", "testComment1"); + var productCommentDelete = this.CreatePackageInstaller("ProductCommentDelete"); + + productCommentDelete.InstallProduct(MSIExec.MSIExecReturnCode.SUCCESS); + + // Verify that comment was removed. + UserVerifier.VerifyUserComment(String.Empty, "testName1", ""); + + // clean up + UserVerifier.DeleteLocalUser("testName1"); + } } } -- cgit v1.2.3-55-g6feb