PSGPPreferences overwrites / wipes out existing data including WMI filters when making changes
Closed this issue · 2 comments
Hi,
Sorry to raise another one - I think I hinted at this earlier in #32 but never got around to posting it!
I have encounted a number of occasions where adding a new entry to a GPO that has already been configured via the GUI results in data loss. Here is one example:
Say I have a more complex GP Preferences setup from the GUI (using environment variables and WMI filters) within a GPO. I'll attach an example below. I can 'get' basic information from it using Get-GPPGroup although that information does not include the WMI filters etc which is fair enough.
Get-GPPGroup -GPOName "TEST GPO"
However when I try to add a new entry (without touching the existing ones in any way!) using New-GPPGroupMember and New-GPPGroup:
$a = New-GPPGroupMember -Name "Domain Admins" -A ADD
New-GPPGroup -GPOName "TEST GPO" -Update -Name "Administrators" -Members $a
PSGPPreferences adds the new entry, but also changes the entirety of the existing XML and messes up a load of things:
- The member '%DS_userPrincipalName%' added to 'Administrators (built in)' is no longer visible under 'Members' when you open the GPO in the Group Policy Editor for editing
- The WMI filters are completely removed
- (Looking at Groups.xml, a whole load of other values relating to the original entries which may or may not be essential for them to properly function are added, removed or updated).
Is it not possible to have GPPreferences perform changes in a more 'light touch' way? Instead of reading the data, trying to process every attribute, then output it again (which would involve correctly implementing the entire specification, which I'm not sure is public knowledge), wouldn't it be better to read the XML in and only edit the parts that the command actually related to, and writing out the rest exactly as it was before? At present running GPPreferences against an existing GPO that is at all complex is nerve wracking, since you never know what might be accidentally lost.
Here's a slightly more complicated Groups.xml generated by the GP Preferences GUI to see how attributes (like bypassErrors, userContext, sid and the WMI filters) are added/removed/generally altered:
Original Groups.xml (GUI only):
<Groups clsid="{3125E937-EB16-4b4c-9934-544FC6D24D26}">
<Group clsid="{6d4a79e4-529c-4481-abd0-f5bd7ea93ba7}" changed="2022-09-01 16:46:31" disabled="0" image="2" name="Administrators (built-in)" uid="{F89B8BA2-C15D-4CC6-B5DB-B2B95C688EE0}" userContext="0" removePolicy="0">
<Properties action="U" newName="" description="Administrators have complete and unrestricted access to the computer/domain" deleteAllUsers="0" deleteAllGroups="0" removeAccounts="0" groupSid="S-1-5-32-544" groupName="Administrators (built-in)">
<Members>
<Member name="%DS_userPrincipalName%" action="ADD" sid=""/>
</Members>
</Properties>
<Filters>
<FilterWmi bool="AND" not="0" query="SELECT DS_managedBy FROM ds_computer WHERE DS_name = "%ComputerName%"" nameSpace="Root\directory\LDAP" property="DS_managedBy" variableName="DS_managedBy"/>
<FilterWmi bool="AND" not="0" query="SELECT DS_userPrincipalName FROM ds_user WHERE DS_distinguishedName = "%DS_managedBy%"" nameSpace="Root\directory\LDAP" property="DS_userPrincipalName" variableName="DS_userPrincipalName"/>
</Filters>
</Group>
<Group clsid="{6d4a79e4-529c-4481-abd0-f5bd7ea93ba7}" changed="2022-07-08 15:51:00" disabled="0" image="2" name="Administrators (built-in)" uid="{61F454FB-5EF7-47DB-A3D1-726F8275BA52}" userContext="0" removePolicy="0">
<Properties action="U" newName="" description="Administrators have complete and unrestricted access to the computer/domain" deleteAllUsers="0" deleteAllGroups="0" removeAccounts="0" groupSid="S-1-5-32-544" groupName="Administrators (built-in)">
<Members>
<Member name="%DomainName%\%DS_cn%" action="ADD" sid=""/>
</Members>
</Properties>
<Filters>
<FilterWmi bool="AND" not="0" query="SELECT DS_cn FROM ds_group WHERE DS_name = "%ComputerName% Administrators"" nameSpace="Root\directory\LDAP" property="DS_cn" variableName="DS_cn"/>
</Filters>
</Group>
</Groups>
I agree that's an inexcusable behavior: the challenge here is that the module convert everything from XML into PowerShell objects. So just to keep filters as they are the module needs to have classes for all of them.
Another quick and dirty option would be to keep the Filters section as a string, without parsing it. I think I should implement this first and then code proper parsing in a separate branch.
Thank you @Borgquite !!
Fixed in #46