Fix a couple minor bug in when adding/removing user comments
Plus some code clean up so error reporting is consistent.
Rob Mensching committed
Oct 15, 2022 at 19:53 UTC
5589a8081bbeb2f449339be23684e583b6df1c81
2 files changed
+62
-63
src/ext/Util/ca/scaexec.cpp
+53
-57
@@ -519,10 +519,8 @@ static HRESULT ModifyUserLocalBatchRight(
519
return hr;
520
}
521
522
-static HRESULT ApplyAttributes(int iAttributes, DWORD* pFlags)
522
+static void ApplyAttributes(int iAttributes, DWORD* pFlags)
523
{
524
- HRESULT hr = S_OK;
525
-
524
if (SCAU_DONT_EXPIRE_PASSWRD & iAttributes)
525
{
526
*pFlags |= UF_DONT_EXPIRE_PASSWD;
@@ -558,14 +556,10 @@ static HRESULT ApplyAttributes(int iAttributes, DWORD* pFlags)
556
{
557
*pFlags &= ~UF_PASSWORD_EXPIRED;
558
}
561
-
562
- return hr;
559
}
560
565
-static HRESULT ApplyComment(int iAttributes, LPWSTR pwzComment, LPWSTR* ppComment)
561
+static void ApplyComment(int iAttributes, LPWSTR pwzComment, LPWSTR* ppComment)
562
{
567
- HRESULT hr = S_OK;
568
-
563
if (SCAU_REMOVE_COMMENT & iAttributes)
564
{
565
*ppComment = L"";
@@ -574,32 +568,36 @@ static HRESULT ApplyComment(int iAttributes, LPWSTR pwzComment, LPWSTR* ppCommen
568
{
569
*ppComment = pwzComment;
570
}
577
-
578
- return hr;
571
}
572
573
static NET_API_STATUS SetUserPassword(__in LPWSTR pwzServerName, __in LPWSTR pwzName, __in LPWSTR pwzPassword)
574
{
583
- _USER_INFO_1003 userInfo1003;
575
+ NET_API_STATUS er = NERR_Success;
576
+ _USER_INFO_1003 userInfo1003 = { };
577
578
userInfo1003.usri1003_password = pwzPassword;
586
- return ::NetUserSetInfo(pwzServerName, pwzName, 1003, reinterpret_cast<LPBYTE>(&userInfo1003), NULL);
579
+ er = ::NetUserSetInfo(pwzServerName, pwzName, 1003, reinterpret_cast<LPBYTE>(&userInfo1003), NULL);
580
+ return HRESULT_FROM_WIN32(er);
581
}
582
589
-static NET_API_STATUS SetUserComment(__in LPWSTR pwzServerName, __in LPWSTR pwzName, __in LPWSTR pwzComment)
583
+static HRESULT SetUserComment(__in LPWSTR pwzServerName, __in LPWSTR pwzName, __in LPWSTR pwzComment)
584
{
591
- _USER_INFO_1007 userInfo1007;
585
+ NET_API_STATUS er = NERR_Success;
586
+ _USER_INFO_1007 userInfo1007 = { };
587
588
userInfo1007.usri1007_comment = pwzComment;
594
- return ::NetUserSetInfo(pwzServerName, pwzName, 1007, reinterpret_cast<LPBYTE>(&userInfo1007), NULL);
589
+ er = ::NetUserSetInfo(pwzServerName, pwzName, 1007, reinterpret_cast<LPBYTE>(&userInfo1007), NULL);
590
+ return HRESULT_FROM_WIN32(er);
591
}
592
597
-static NET_API_STATUS SetUserFlags(__in LPWSTR pwzServerName, __in LPWSTR pwzName, __in DWORD flags)
593
+static HRESULT SetUserFlags(__in LPWSTR pwzServerName, __in LPWSTR pwzName, __in DWORD flags)
594
{
599
- _USER_INFO_1008 userInfo1008;
595
+ NET_API_STATUS er = NERR_Success;
596
+ _USER_INFO_1008 userInfo1008 = { };
597
598
userInfo1008.usri1008_flags = flags;
602
- return ::NetUserSetInfo(pwzServerName, pwzName, 1008, reinterpret_cast<LPBYTE>(&userInfo1008), NULL);
599
+ er = ::NetUserSetInfo(pwzServerName, pwzName, 1008, reinterpret_cast<LPBYTE>(&userInfo1008), NULL);
600
+ return HRESULT_FROM_WIN32(er);
601
}
602
603
static HRESULT RemoveUserInternal(
@@ -717,12 +715,10 @@ LExit:
715
return hr;
716
}
717
720
-static HRESULT GetServerName(LPWSTR pwzDomain, LPWSTR* ppwzServerName)
718
+static void GetServerName(LPWSTR pwzDomain, LPWSTR* ppwzServerName)
719
{
722
- HRESULT hr = S_OK;
723
-
720
+ DWORD er = ERROR_SUCCESS;
721
PDOMAIN_CONTROLLER_INFOW pDomainControllerInfo = NULL;
725
- UINT er;
722
723
if (pwzDomain && *pwzDomain)
724
{
@@ -732,12 +728,18 @@ static HRESULT GetServerName(LPWSTR pwzDomain, LPWSTR* ppwzServerName)
728
// MSDN says, if we get the above error code, try again with the "DS_FORCE_REDISCOVERY" flag
729
er = ::DsGetDcNameW(NULL, (LPCWSTR)pwzDomain, NULL, NULL, DS_FORCE_REDISCOVERY, &pDomainControllerInfo);
730
}
735
- if (ERROR_SUCCESS == er
736
- && 2 <= wcslen(pDomainControllerInfo->DomainControllerName)
737
- && '\\' == *pDomainControllerInfo->DomainControllerName
738
- && '\\' == *pDomainControllerInfo->DomainControllerName + 1)
731
+
732
+ if (ERROR_SUCCESS == er && pDomainControllerInfo->DomainControllerName)
733
{
740
- *ppwzServerName = pDomainControllerInfo->DomainControllerName + 2; // Skip the \\ prefix
734
+ // Skip the \\ prefix if present.
735
+ if ('\\' == *pDomainControllerInfo->DomainControllerName && '\\' == *pDomainControllerInfo->DomainControllerName + 1)
736
+ {
737
+ *ppwzServerName = pDomainControllerInfo->DomainControllerName + 2;
738
+ }
739
+ else
740
+ {
741
+ *ppwzServerName = pDomainControllerInfo->DomainControllerName;
742
+ }
743
}
744
else
745
{
@@ -749,8 +751,6 @@ static HRESULT GetServerName(LPWSTR pwzDomain, LPWSTR* ppwzServerName)
751
{
752
::NetApiBufferFree((LPVOID)pDomainControllerInfo);
753
}
752
-
753
- return hr;
754
}
755
756
/********************************************************************
@@ -837,30 +837,28 @@ extern "C" UINT __stdcall CreateUser(
837
pUserInfo1->usri1_password = pwzPassword;
838
839
// Set the user's comment
840
- hr = ApplyComment(iAttributes, pwzComment, &pUserInfo1->usri1_comment);
841
- ExitOnFailure(hr, "failed to apply comment");
840
+ ApplyComment(iAttributes, pwzComment, &pUserInfo1->usri1_comment);
841
842
// Set the user's flags
844
- hr = ApplyAttributes(iAttributes, &pUserInfo1->usri1_flags);
845
- ExitOnFailure(hr, "failed to apply attributes");
843
+ ApplyAttributes(iAttributes, &pUserInfo1->usri1_flags);
844
845
//
846
// Create the User
847
//
850
- hr = GetServerName(pwzDomain, &pwzServerName);
851
- ExitOnFailure(hr, "failed to get server name");
848
+ GetServerName(pwzDomain, &pwzServerName);
849
850
er = ::NetUserAdd(pwzServerName, 1, reinterpret_cast<LPBYTE>(pUserInfo1), &dw);
851
if (NERR_UserExists == er)
852
{
856
- er = ERROR_SUCCESS; // Make sure that we don't report this situation as an error
857
- // if we fall through the tests that follow.
853
if (SCAU_FAIL_IF_EXISTS & iAttributes)
854
{
855
hr = HRESULT_FROM_WIN32(er);
856
ExitOnFailure(hr, "User was not supposed to exist, but does.");
857
}
858
859
+ er = ERROR_SUCCESS; // Make sure that we don't report this situation as an error
860
+ // if we fall through the tests that follow.
861
+
862
if (SCAU_UPDATE_IF_EXISTS & iAttributes)
863
{
864
pUserInfo1 = NULL;
@@ -890,6 +888,7 @@ extern "C" UINT __stdcall CreateUser(
888
if (FAILED(hr))
889
{
890
WcaLogError(hr, "failed to get existing user rights: %ls, continuing anyway.", pwzName);
891
+ hr = S_OK;
892
}
893
else
894
{
@@ -923,41 +922,41 @@ extern "C" UINT __stdcall CreateUser(
922
923
if (ERROR_SUCCESS == er)
924
{
926
- hr = HRESULT_FROM_WIN32(::SetUserPassword(pwzServerName, pwzName, pwzPassword));
925
+ hr = SetUserPassword(pwzServerName, pwzName, pwzPassword);
926
if (FAILED(hr))
927
{
928
WcaLogError(hr, "failed to set user password for user %ls\\%ls, continuing anyway.", pwzServerName, pwzName);
929
+ hr = S_OK;
930
}
931
932
if (SCAU_REMOVE_COMMENT & iAttributes)
933
{
934
- hr = HRESULT_FROM_WIN32(SetUserComment(pwzServerName, pwzName, L""));
934
+ hr = SetUserComment(pwzServerName, pwzName, L"");
935
if (FAILED(hr))
936
{
937
WcaLogError(hr, "failed to clear user comment for user %ls\\%ls, continuing anyway.", pwzServerName, pwzName);
938
+ hr = S_OK;
939
}
940
}
941
else if (pwzComment && *pwzComment)
942
{
942
- hr = HRESULT_FROM_WIN32(SetUserComment(pwzServerName, pwzName, pwzComment));
943
+ hr = SetUserComment(pwzServerName, pwzName, pwzComment);
944
if (FAILED(hr))
945
{
946
WcaLogError(hr, "failed to set user comment to %ls for user %ls\\%ls, continuing anyway.", pwzComment, pwzServerName, pwzName);
947
+ hr = S_OK;
948
}
949
}
950
951
DWORD flags = pUserInfo1->usri1_flags;
952
951
- hr = ApplyAttributes(iAttributes, &flags);
952
- if (FAILED(hr))
953
- {
954
- WcaLogError(hr, "failed to apply attributes for user %ls\\%ls, continuing anyway.", pwzServerName, pwzName);
955
- }
953
+ ApplyAttributes(iAttributes, &flags);
954
957
- hr = HRESULT_FROM_WIN32(SetUserFlags(pwzServerName, pwzName, flags));
955
+ hr = SetUserFlags(pwzServerName, pwzName, flags);
956
if (FAILED(hr))
957
{
958
WcaLogError(hr, "failed to set user flags for user %ls\\%ls, continuing anyway.", pwzServerName, pwzName);
959
+ hr = S_OK;
960
}
961
}
962
}
@@ -985,13 +984,13 @@ extern "C" UINT __stdcall CreateUser(
984
MessageExitOnFailure(hr, msierrUSRFailedGrantLogonAsService, "Failed to grant logon as batch job rights to user: %ls", pwzName);
985
}
986
988
-//
989
-// Add the users to groups
990
-//
991
-while (S_OK == (hr = WcaReadStringFromCaData(&pwz, &pwzGroup)))
992
-{
993
- hr = WcaReadStringFromCaData(&pwz, &pwzGroupDomain);
994
- ExitOnFailure(hr, "failed to get domain for group: %ls", pwzGroup);
987
+ //
988
+ // Add the users to groups
989
+ //
990
+ while (S_OK == (hr = WcaReadStringFromCaData(&pwz, &pwzGroup)))
991
+ {
992
+ hr = WcaReadStringFromCaData(&pwz, &pwzGroupDomain);
993
+ ExitOnFailure(hr, "failed to get domain for group: %ls", pwzGroup);
994
995
WcaLog(LOGMSG_STANDARD, "Adding user %ls\\%ls to group %ls\\%ls", pwzDomain, pwzName, pwzGroupDomain, pwzGroup);
996
hr = AddUserToGroup(pwzName, pwzDomain, pwzGroup, pwzGroupDomain);
@@ -1001,10 +1000,7 @@ while (S_OK == (hr = WcaReadStringFromCaData(&pwz, &pwzGroup)))
1000
{
1001
hr = S_OK;
1002
}
1004
-
1005
- ExitOnFailure(hr, "failed to get next group in which to include user:%ls", pwzName);
1006
-
1007
-ExitOnFailure(hr, "failed to get next group in which to include user:%ls", pwzName);
1003
+ ExitOnFailure(hr, "failed to get next group in which to include user: %ls", pwzName);
1004
1005
LExit:
1006
WcaCaScriptClose(hRollbackScript, WCA_CASCRIPT_CLOSE_PRESERVE);
src/ext/Util/ca/scauser.cpp
+9
-6
@@ -533,13 +533,16 @@ HRESULT ScaUserExecute(
533
// MSDN says, if we get the above error code, try again with the "DS_FORCE_REDISCOVERY" flag
534
er = ::DsGetDcNameW(NULL, wzDomain, NULL, NULL, DS_FORCE_REDISCOVERY, &pDomainControllerInfo);
535
}
536
- if (ERROR_SUCCESS == er)
536
+ if (ERROR_SUCCESS == er && pDomainControllerInfo->DomainControllerName)
537
{
538
- if (2 <= wcslen(pDomainControllerInfo->DomainControllerName))
538
+ // If the \\ prefix on the queried domain was present, skip it.
539
+ if ('\\' == *pDomainControllerInfo->DomainControllerName && '\\' == *pDomainControllerInfo->DomainControllerName + 1)
540
{
540
- wzDomain = pDomainControllerInfo->DomainControllerName + 2; // Add 2 so that we don't get the \\ prefix.
541
- // Pass the entire string if it is too short
542
- // to have a \\ prefix.
541
+ wzDomain = pDomainControllerInfo->DomainControllerName + 2;
542
+ }
543
+ else
544
+ {
545
+ wzDomain = pDomainControllerInfo->DomainControllerName;
546
}
547
}
548
}
@@ -672,7 +675,7 @@ HRESULT ScaUserExecute(
675
// CustomAction.
676
hr = WcaDoDeferredAction(CUSTOM_ACTION_DECORATION(L"RemoveUser"), pwzActionData, COST_USER_DELETE);
677
ExitOnFailure(hr, "failed to schedule RemoveUser");
675
- }
678
+ }
679
680
ReleaseNullStr(pwzScriptKey);
681
ReleaseNullStr(pwzActionData);