From 835754970f1aff5bf0996034efa9f1ab2d976825 Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Sat, 10 Apr 2021 10:25:39 +0100 Subject: [PATCH 1/2] ADGroup Child Domain Fix --- .../MSFT_ADGroup/MSFT_ADGroup.psm1 | 50 ++++++++++++++++--- .../ActiveDirectoryDsc.Common.psm1 | 35 +++++++++++-- 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1 b/source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1 index cdc22533..6ea4f410 100644 --- a/source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1 +++ b/source/DSCResources/MSFT_ADGroup/MSFT_ADGroup.psm1 @@ -106,6 +106,7 @@ function Get-TargetResource if ($_.FullyQualifiedErrorId -eq $oneWayTrustFullyQualifiedErrorId) { + Write-Debug -Message 'Processing Group Members - One-Way Trust' # Get-ADGroupMember returns property name 'SID' while Get-ADObject returns property name 'ObjectSID' if ($MembershipAttribute -eq 'SID') { @@ -130,20 +131,51 @@ function Get-TargetResource 'ObjectSID' ) - $adObject = Get-ADObject @getADObjectParameters + $adObject = Get-ADObject @getADObjectParameters -Server :3268 - # Perform SID translation to a readable name as the SamAccountName if the member is - # of objectClass "foreignSecurityPrincipal" - $classMatchForResolve = $adObject.objectClass -eq 'foreignSecurityPrincipal' - $attributeMatchForResolve = $MembershipAttribute -eq 'SamAccountName' - - if ($classMatchForResolve -and $attributeMatchForResolve) + if ($adObject.objectClass -eq 'foreignSecurityPrincipal' -and + $MembershipAttribute -eq 'SamAccountName') { Resolve-SamAccountName -ObjectSid $adObject.objectSid } else { - $adObject.$selectProperty + if ($selectProperty -eq 'SamAccountName') + { + $adGroupDomain = ([RegEx]::Matches($adgroup.DistinguishedName, '(?i)DC=\w{1,}?\b') | + ForEach-Object { $_.Value }) + $memberDomain = ([RegEx]::Matches($adobject.DistinguishedName, '(?i)DC=\w{1,}?\b') | + ForEach-Object { $_.Value }) + + if ($adGroupDomain -ne $memberDomain) + { + $forestDomains = (Get-ADForest).Domains + foreach ($domain in $forestDomains) + { + $domainDetails = Get-ADDomain -Identity $domain + $domainDN = $domainDetails.DistinguishedName + if ($memberDomain = $domainDN) + { + $domainNetBIOSName = (Get-ADDomain -Identity $domainDetails.DNSRoot).NetBIOSName + break + } + } + + if (-not [System.String]::IsNullOrEmpty($domainNetBIOSName)) + { + "$domainNetBIOSName\$($adObject.SamAccountName)" + } + else + { + $errorMessage = 'Domain $domain not found in Forest' + New-InvalidResultException -Message $errorMessage + } + } + } + else + { + $adObject.$selectProperty + } } } } @@ -154,6 +186,8 @@ function Get-TargetResource } } + Write-Debug -Message "Group Members: $($adGroupMembers -Join ',')" + $targetResource = @{ Ensure = 'Present' GroupName = $adGroup.Name diff --git a/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1 b/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1 index 43cd30a2..ef70c91c 100644 --- a/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1 +++ b/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1 @@ -1117,6 +1117,8 @@ function Set-ADCommonGroupMember $Action = 'Add' ) + Write-Debug 'Entering Set-ADCommonGroupMember' + Assert-Module -ModuleName ActiveDirectory $setADGroupParameters = $Parameters.Clone() @@ -2599,6 +2601,8 @@ function Resolve-MembersSecurityIdentifier begin { + Write-Debug 'Entering Resolve-MembersSecurityIdentifier' + Assert-Module -ModuleName ActiveDirectory $property = 'ObjectSID' @@ -2643,30 +2647,51 @@ function Resolve-MembersSecurityIdentifier { if ($MembershipAttribute -eq 'SamAccountName' -and $member -match '\\') { - Write-Debug -Message ($script:localizedData.TranslatingMembershipAttribute -f + $memberDomainName = $member.split('\')[0] + $memberSamAccountName = $member.split('\')[1] + try + { + $memberDomainDetails = Get-ADDomain -Identity $memberDomainName + } + catch [System.Security.Authentication.AuthenticationException] + { + Write-Debug -Message ($script:localizedData.TranslatingMembershipAttribute -f $MembershipAttribute, $member, $property) - $securityIdentifier = Resolve-SecurityIdentifier -SamAccountName $member + $securityIdentifier = Resolve-SecurityIdentifier -SamAccountName $member + $sid = $true + $memberDomainDetails = $null + } + + if ($memberDomainDetails) + { + $securityIdentifier = (Get-ADObject -SearchBase $memberDomainDetails.DistinguishedName -Filter "SamAccountName -eq '$memberSamAccountName'" -Server :3268).DistinguishedName + $sid = $false + } } elseif ($MembershipAttribute -eq 'DistinguishedName' -and ($member -split ',')[1] -eq $fspADContainer) { Write-Debug -Message ($script:localizedData.ParsingCommonNameFromDN -f $member) $securityIdentifier = ($member -split ',')[0] -replace '^CN[=]' + $sid=$true } else { Write-Debug -Message ($script:localizedData.ADObjectPropertyLookup -f $property, $MembershipAttribute, $member) - $getADObjectParms['Filter'] = "$($MembershipAttribute) -eq '$($member)'" + #$getADObjectParms['Filter'] = "$($MembershipAttribute) -eq '$($member)'" + $getADObjectParms['Identity'] = $member - $securityIdentifier = [string](Get-ADObject @getADObjectParms).$property + #$securityIdentifier = [string](Get-ADObject @getADObjectParms -Server :3268).$property + $securityIdentifier = $member + $sid=$false } if (-not ([string]::IsNullOrEmpty($securityIdentifier))) { - if ($PrepareForMembership.IsPresent) + if ($PrepareForMembership.IsPresent -and $sid -eq $true) { "" } From c4d30d204c873494dd3d0d2da66ba83122c3a290 Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Sat, 10 Apr 2021 10:32:31 +0100 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0b2571b..fe2a0319 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ For older change log history see the [historic changelog](HISTORIC_CHANGELOG.md) - ADGroup - Refactored Module. - Refactored Unit and Integration Tests. + - Fixed management of child domain user membership. + ([issue #633](https://github.com/dsccommunity/ActiveDirectoryDsc/issues/633)) ### Added