ADGroup: Changing group membership management mechanism (dsccommunity…

…#620) This is intended to change the way that the ADGroup resource manages group membership. The new implementation abandons usage of Add-ADGroupMember and Remove-ADGroupMember due to limitations with Foreign Security Principals. Instead we opt to utilize Set-ADGroup with the Add and Remove parameters, passing a hash object with the member key and a list of formatted SID values (e.g. - "<SID=SID_VALUE>").
nanderh · Oct 10, 2020 · 87b1308 · 87b1308
1 parent f30a845
commit 87b1308
Show file tree

Hide file tree

Showing 14 changed files with 1,082 additions and 381 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,7 +8,10 @@ For older change log history see the [historic changelog](HISTORIC_CHANGELOG.md)
 ## [Unreleased]
 
 ### Added
-
+- ADGroup
+  - Added support for managing AD group membership of Foreign Security Principals. This involved completely
+    refactoring group membership management to utilize the `Set-ADGroup` cmdlet and referencing SID values.
+    ([issue #619](https://github.com/dsccommunity/ActiveDirectoryDsc/issues/619)).
 - ADFineGrainedPasswordPolicy
   - New resource for creating and updating Fine Grained Password Policies for AD principal subjects.
     ([issue #584](https://github.com/dsccommunity/ActiveDirectoryDsc/issues/584)).

diff --git a/source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1 b/source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1
@@ -655,8 +655,6 @@ function Set-TargetResource
 
     Assert-MemberParameters @assertMemberParameters
 
-    $membersInMultipleDomains = $false
-
     if ($MembershipAttribute -eq 'DistinguishedName')
     {
         $allMembers = $Members + $MembersToInclude + $MembersToExclude
@@ -676,7 +674,6 @@ function Set-TargetResource
         if ($GroupMemberDomainCount -gt 1 -or ($groupMemberDomains -ine (Get-DomainName)).Count -gt 0)
         {
             Write-Verbose -Message ($script:localizedData.GroupMembershipMultipleDomains -f $GroupMemberDomainCount)
-            $membersInMultipleDomains = $true
         }
     }
 
@@ -842,12 +839,24 @@ function Set-TargetResource
                         {
                             Write-Verbose -Message ($script:localizedData.RemovingGroupMembers -f $adGroupMembers.Count, $GroupName)
 
-                            Remove-ADGroupMember @commonParameters -Members $adGroupMembers -Confirm:$false -ErrorAction 'Stop'
+                            $setADCommonGroupMemberParms = @{
+                                Members             = $adGroupMembers
+                                MembershipAttribute = $MembershipAttribute
+                                Parameters          = $commonParameters
+                                Action              = 'Remove'
+                            }
+                            Set-ADCommonGroupMember @setADCommonGroupMemberParms
                         }
 
                         Write-Verbose -Message ($script:localizedData.AddingGroupMembers -f $Members.Count, $GroupName)
 
-                        Add-ADCommonGroupMember -Parameters $commonParameters -Members $Members -MembersInMultipleDomains:$membersInMultipleDomains
+                        $setADCommonGroupMemberParms = @{
+                            Members             = $Members
+                            MembershipAttribute = $MembershipAttribute
+                            Parameters          = $commonParameters
+                            Action              = 'Add'
+                        }
+                        Set-ADCommonGroupMember @setADCommonGroupMemberParms
                     }
 
                     if ($PSBoundParameters.ContainsKey('MembersToInclude') -and -not [System.String]::IsNullOrEmpty($MembersToInclude))
@@ -856,7 +865,13 @@ function Set-TargetResource
 
                         Write-Verbose -Message ($script:localizedData.AddingGroupMembers -f $MembersToInclude.Count, $GroupName)
 
-                        Add-ADCommonGroupMember -Parameters $commonParameters -Members $MembersToInclude -MembersInMultipleDomains:$membersInMultipleDomains
+                        $setADCommonGroupMemberParms = @{
+                            Members             = $MembersToInclude
+                            MembershipAttribute = $MembershipAttribute
+                            Parameters          = $commonParameters
+                            Action              = 'Add'
+                        }
+                        Set-ADCommonGroupMember @setADCommonGroupMemberParms
                     }
 
                     if ($PSBoundParameters.ContainsKey('MembersToExclude') -and -not [System.String]::IsNullOrEmpty($MembersToExclude))
@@ -865,7 +880,13 @@ function Set-TargetResource
 
                         Write-Verbose -Message ($script:localizedData.RemovingGroupMembers -f $MembersToExclude.Count, $GroupName)
 
-                        Remove-ADGroupMember @commonParameters -Members $MembersToExclude -Confirm:$false -ErrorAction 'Stop'
+                        $setADCommonGroupMemberParms = @{
+                            Members             = $MembersToExclude
+                            MembershipAttribute = $MembershipAttribute
+                            Parameters          = $commonParameters
+                            Action              = 'Remove'
+                        }
+                        Set-ADCommonGroupMember @setADCommonGroupMemberParms
                     }
                 }
             }
@@ -960,15 +981,27 @@ function Set-TargetResource
 
                 Write-Verbose -Message ($script:localizedData.AddingGroupMembers -f $Members.Count, $GroupName)
 
-                Add-ADCommonGroupMember -Parameters $commonParameters -Members $Members -MembersInMultipleDomains:$membersInMultipleDomains
+                $setADCommonGroupMemberParms = @{
+                    Members             = $Members
+                    MembershipAttribute = $MembershipAttribute
+                    Parameters          = $commonParameters
+                    Action              = 'Add'
+                }
+                Set-ADCommonGroupMember @setADCommonGroupMemberParms
             }
             elseif ($PSBoundParameters.ContainsKey('MembersToInclude') -and -not [System.String]::IsNullOrEmpty($MembersToInclude))
             {
                 $MembersToInclude = Remove-DuplicateMembers -Members $MembersToInclude
 
                 Write-Verbose -Message ($script:localizedData.AddingGroupMembers -f $MembersToInclude.Count, $GroupName)
 
-                Add-ADCommonGroupMember -Parameters $commonParameters -Members $MembersToInclude -MembersInMultipleDomains:$membersInMultipleDomains
+                $setADCommonGroupMemberParms = @{
+                    Members             = $MembersToInclude
+                    MembershipAttribute = $MembershipAttribute
+                    Parameters          = $commonParameters
+                    Action              = 'Add'
+                }
+                Set-ADCommonGroupMember @setADCommonGroupMemberParms
             }
         }
     } #end catch

diff --git a/source/Examples/Resources/ADGroup/4-ADGroup_NewGroupOneWayTrust_Config.ps1 b/source/Examples/Resources/ADGroup/4-ADGroup_NewGroupOneWayTrust_Config.ps1
@@ -0,0 +1,41 @@
+<#PSScriptInfo
+.VERSION 1.0.0
+.GUID f2ecc331-e242-4204-a6b1-54fd68c852b7
+.AUTHOR DSC Community
+.COMPANYNAME DSC Community
+.COPYRIGHT DSC Community contributors. All rights reserved.
+.TAGS DSCConfiguration
+.LICENSEURI https://github.com/dsccommunity/ActiveDirectoryDsc/blob/master/LICENSE
+.PROJECTURI https://github.com/dsccommunity/ActiveDirectoryDsc
+.ICONURI https://dsccommunity.org/images/DSC_Logo_300p.png
+.RELEASENOTES
+Initial release
+#>
+
+#Requires -Module ActiveDirectoryDsc
+
+<#
+    .DESCRIPTION
+        This configuration will create a new domain-local group in contoso with
+        two members; one from the contoso domain and one from the fabrikam domain.
+        This qualified SamAccountName format is required if any of the users are in a
+        one-way trusted forest/external domain.
+#>
+Configuration ADGroup_NewGroupOneWayTrust_Config
+{
+    Import-DscResource -ModuleName ActiveDirectoryDsc
+
+    node localhost
+    {
+        ADGroup 'ExampleExternalTrustGroup'
+        {
+            GroupName           = 'ExampleExternalTrustGroup'
+            GroupScope          = 'DomainLocal'
+            MembershipAttribute = 'SamAccountName'
+            Members             = @(
+                'contoso\john'
+                'fabrikam\toby'
+            )
+        }
+    }
+}
diff --git a/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psd1 b/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psd1
@@ -37,7 +37,7 @@
         'ConvertTo-DeploymentDomainMode'
         'Restore-ADCommonObject'
         'Get-ADDomainNameFromDistinguishedName'
-        'Add-ADCommonGroupMember'
+        'Set-ADCommonGroupMember'
         'Get-DomainControllerObject'
         'Test-IsDomainController'
         'Convert-PropertyMapToObjectProperties'
@@ -53,6 +53,8 @@
         'Get-ActiveDirectoryDomain'
         'Get-ActiveDirectoryForest'
         'Resolve-SamAccountName'
+        'Resolve-SecurityIdentifier'
+        'Resolve-MembersSecurityIdentifier'
     )
 
     # Cmdlets to export from this module, for best performance, do not use wildcards and do not delete the entry, use an empty array if there are no cmdlets to export.