From b322a8cd95b810f566e7234e4436582303975acb Mon Sep 17 00:00:00 2001 From: Kenny Scelfo Date: Mon, 6 Nov 2023 10:43:30 -0600 Subject: [PATCH 1/5] Refactored Principal Resolution Functions for Enhanced Virtual Account Support - Enhanced Test-IsLocalMachine to recognize 'IIS APPPOOL' and 'NT VIRTUAL MACHINE' as local machine scopes, improving virtual account type coverage. - Updated Split-MemberName to preserve original scope, allowing local accounts that do not use the computer name to be resolved. - Extended Find-Principal with a new Scope parameter, enabling searches additional local machine scopes. - Augmented Find-Principal to handle a predefined list of NT account scopes ('NT Authority', 'NT Service', 'IIS APPPOOL', 'NT Virtual Machine'), utilizing System.Security.Principal.NTAccount for SID resolution. This enhancement focuses on local virtual account identification. - Implemented new integration tests for ConvertTo-Principal to verify local, virtual account resolution. - Introduced Get-ScopeFromDistinguishedName to accurately parse scope from distinguished names, addressing issues with multiple domain components and escaped commas. - Added unit tests for Get-ScopeFromDistinguishedName, ensuring functional correctness. - Adjusted outside domain test to expect 'domain.com' instead of 'domain'. --- .../DSC_xGroupResource.psm1 | 232 +++++++++++------- .../DSC_xGroupResource.Integration.Tests.ps1 | 20 ++ tests/Unit/DSC_xGroupResource.Tests.ps1 | 52 +++- 3 files changed, 204 insertions(+), 100 deletions(-) diff --git a/source/DSCResources/DSC_xGroupResource/DSC_xGroupResource.psm1 b/source/DSCResources/DSC_xGroupResource/DSC_xGroupResource.psm1 index c6f971754..7a11c0ff6 100644 --- a/source/DSCResources/DSC_xGroupResource/DSC_xGroupResource.psm1 +++ b/source/DSCResources/DSC_xGroupResource/DSC_xGroupResource.psm1 @@ -67,8 +67,8 @@ $modulePath = Join-Path -Path (Split-Path -Path (Split-Path -Path $PSScriptRoot # Import the shared modules Import-Module -Name (Join-Path -Path $modulePath ` - -ChildPath (Join-Path -Path 'xPSDesiredStateConfiguration.Common' ` - -ChildPath 'xPSDesiredStateConfiguration.Common.psm1')) + -ChildPath (Join-Path -Path 'xPSDesiredStateConfiguration.Common' ` + -ChildPath 'xPSDesiredStateConfiguration.Common.psm1')) Import-Module -Name (Join-Path -Path $modulePath -ChildPath 'DscResource.Common') @@ -647,10 +647,10 @@ function Set-TargetResourceOnFullSKU if ($groupOriginallyExists) { $actualMembersAsPrincipals = @( Get-MembersAsPrincipalsList ` - -Group $group ` - -PrincipalContextCache $principalContextCache ` - -Disposables $disposables ` - -Credential $Credential + -Group $group ` + -PrincipalContextCache $principalContextCache ` + -Disposables $disposables ` + -Credential $Credential ) } @@ -666,10 +666,10 @@ function Set-TargetResourceOnFullSKU # Resolve the names to actual principal objects. $membersAsPrincipals = @( ConvertTo-UniquePrincipalsList ` - -MemberNames $uniqueMembers ` - -PrincipalContextCache $principalContextCache ` - -Disposables $disposables ` - -Credential $Credential ) + -MemberNames $uniqueMembers ` + -PrincipalContextCache $principalContextCache ` + -Disposables $disposables ` + -Credential $Credential ) if ($null -ne $actualMembersAsPrincipals -and $actualMembersAsPrincipals.Count -gt 0) { @@ -684,20 +684,20 @@ function Set-TargetResourceOnFullSKU foreach ($actualMemberAsPrincipal in $actualMembersAsPrincipals) { - if ($membersAsPrincipals -notcontains $actualMemberAsPrincipal) - { - Remove-GroupMember -Group $group -MemberAsPrincipal $actualMemberAsPrincipal - $saveChanges = $true - } + if ($membersAsPrincipals -notcontains $actualMemberAsPrincipal) + { + Remove-GroupMember -Group $group -MemberAsPrincipal $actualMemberAsPrincipal + $saveChanges = $true } + } } else { # Set the members of the group foreach ($memberAsPrincipal in $membersAsPrincipals) { - Add-GroupMember -Group $group -MemberAsPrincipal $memberAsPrincipal - } + Add-GroupMember -Group $group -MemberAsPrincipal $memberAsPrincipal + } $saveChanges = $true } @@ -712,10 +712,10 @@ function Set-TargetResourceOnFullSKU if ($groupOriginallyExists) { $actualMembersAsPrincipals = @( Get-MembersAsPrincipalsList ` - -Group $group ` - -PrincipalContextCache $principalContextCache ` - -Disposables $disposables ` - -Credential $Credential + -Group $group ` + -PrincipalContextCache $principalContextCache ` + -Disposables $disposables ` + -Credential $Credential ) } @@ -730,10 +730,10 @@ function Set-TargetResourceOnFullSKU { # Resolve the names to actual principal objects. $membersToIncludeAsPrincipals = @( ConvertTo-UniquePrincipalsList ` - -MemberNames $uniqueMembersToInclude ` - -PrincipalContextCache $principalContextCache ` - -Disposables $disposables ` - -Credential $Credential + -MemberNames $uniqueMembersToInclude ` + -PrincipalContextCache $principalContextCache ` + -Disposables $disposables ` + -Credential $Credential ) } @@ -748,10 +748,10 @@ function Set-TargetResourceOnFullSKU { # Resolve the names to actual principal objects. $membersToExcludeAsPrincipals = @( ConvertTo-UniquePrincipalsList ` - -MemberNames $uniqueMembersToExclude ` - -PrincipalContextCache $principalContextCache ` - -Disposables $disposables ` - -Credential $Credential + -MemberNames $uniqueMembersToExclude ` + -PrincipalContextCache $principalContextCache ` + -Disposables $disposables ` + -Credential $Credential ) } @@ -765,7 +765,7 @@ function Set-TargetResourceOnFullSKU { New-InvalidArgumentException -ArgumentName 'MembersToInclude and MembersToExclude' ` -Message ($script:localizedData.IncludeAndExcludeConflict -f $includedPrincipal.SamAccountName, - 'MembersToInclude', 'MembersToExclude') + 'MembersToInclude', 'MembersToExclude') } if ($actualMembersAsPrincipals -notcontains $includedPrincipal) @@ -944,16 +944,16 @@ function Set-TargetResourceOnNanoServer if ($Ensure -eq 'Present') { $whatIfShouldProcess = - if ($groupOriginallyExists) - { - $PSCmdlet.ShouldProcess(($script:localizedData.GroupWithName -f $GroupName), - $script:localizedData.SetOperation) - } - else - { - $PSCmdlet.ShouldProcess(($script:localizedData.GroupWithName -f $GroupName), - $script:localizedData.AddOperation) - } + if ($groupOriginallyExists) + { + $PSCmdlet.ShouldProcess(($script:localizedData.GroupWithName -f $GroupName), + $script:localizedData.SetOperation) + } + else + { + $PSCmdlet.ShouldProcess(($script:localizedData.GroupWithName -f $GroupName), + $script:localizedData.AddOperation) + } if ($whatIfShouldProcess) { @@ -1023,7 +1023,7 @@ function Set-TargetResourceOnNanoServer { New-InvalidArgumentException -ArgumentName 'MembersToInclude and MembersToExclude' ` -Message ($script:localizedData.IncludeAndExcludeConflict -f $includedMember, 'MembersToInclude', - 'MembersToExclude') + 'MembersToExclude') } } } @@ -1207,10 +1207,10 @@ function Test-TargetResourceOnFullSKU } $actualMembersAsPrincipals = @( Get-MembersAsPrincipalsList ` - -Group $group ` - -PrincipalContextCache $principalContextCache ` - -Disposables $disposables ` - -Credential $Credential + -Group $group ` + -PrincipalContextCache $principalContextCache ` + -Disposables $disposables ` + -Credential $Credential ) $uniqueMembers = $Members | Select-Object -Unique @@ -1228,10 +1228,10 @@ function Test-TargetResourceOnFullSKU # Resolve the names to actual principal objects. $expectedMembersAsPrincipals = @( ConvertTo-UniquePrincipalsList ` - -MemberNames $uniqueMembers ` - -PrincipalContextCache $principalContextCache ` - -Disposables $disposables ` - -Credential $Credential + -MemberNames $uniqueMembers ` + -PrincipalContextCache $principalContextCache ` + -Disposables $disposables ` + -Credential $Credential ) if ($expectedMembersAsPrincipals.Count -ne $actualMembersAsPrincipals.Count) @@ -1256,10 +1256,10 @@ function Test-TargetResourceOnFullSKU elseif ($PSBoundParameters.ContainsKey('MembersToInclude') -or $PSBoundParameters.ContainsKey('MembersToExclude')) { $actualMembersAsPrincipals = @( Get-MembersAsPrincipalsList ` - -Group $group ` - -PrincipalContextCache $principalContextCache ` - -Disposables $disposables ` - -Credential $Credential + -Group $group ` + -PrincipalContextCache $principalContextCache ` + -Disposables $disposables ` + -Credential $Credential ) $membersToIncludeAsPrincipals = $null @@ -1273,10 +1273,10 @@ function Test-TargetResourceOnFullSKU { # Resolve the names to actual principal objects. $membersToIncludeAsPrincipals = @( ConvertTo-UniquePrincipalsList ` - -MemberNames $uniqueMembersToInclude ` - -PrincipalContextCache $principalContextCache ` - -Disposables $disposables ` - -Credential $Credential + -MemberNames $uniqueMembersToInclude ` + -PrincipalContextCache $principalContextCache ` + -Disposables $disposables ` + -Credential $Credential ) } @@ -1291,10 +1291,10 @@ function Test-TargetResourceOnFullSKU { # Resolve the names to actual principal objects. $membersToExcludeAsPrincipals = @( ConvertTo-UniquePrincipalsList ` - -MemberNames $uniqueMembersToExclude ` - -PrincipalContextCache $principalContextCache ` - -Disposables $disposables ` - -Credential $Credential + -MemberNames $uniqueMembersToExclude ` + -PrincipalContextCache $principalContextCache ` + -Disposables $disposables ` + -Credential $Credential ) } @@ -1308,7 +1308,7 @@ function Test-TargetResourceOnFullSKU { New-InvalidArgumentException -ArgumentName 'MembersToInclude and MembersToExclude' ` -Message ($script:localizedData.IncludeAndExcludeConflict -f $includedPrincipal.SamAccountName, - 'MembersToInclude', 'MembersToExclude') + 'MembersToInclude', 'MembersToExclude') } if ($actualMembersAsPrincipals -notcontains $includedPrincipal) @@ -1516,7 +1516,7 @@ function Test-TargetResourceOnNanoServer { New-InvalidArgumentException -ArgumentName 'MembersToInclude and MembersToExclude' ` -Message ($script:localizedData.IncludeAndExcludeConflict -f $includedMember, 'MembersToInclude', - 'MembersToExclude') + 'MembersToExclude') } } } @@ -1633,10 +1633,10 @@ function Get-MembersOnFullSKU $members = New-Object -TypeName 'System.Collections.ArrayList' $membersAsPrincipals = @( Get-MembersAsPrincipalsList ` - -Group $Group ` - -PrincipalContextCache $PrincipalContextCache ` - -Disposables $Disposables ` - -Credential $Credential + -Group $Group ` + -PrincipalContextCache $PrincipalContextCache ` + -Disposables $Disposables ` + -Credential $Credential ) foreach ($memberAsPrincipal in $membersAsPrincipals) @@ -2019,7 +2019,7 @@ function ConvertTo-Principal try { - $principal = Find-Principal -PrincipalContext $principalContext -IdentityValue $identityValue + $principal = Find-Principal -Scope $scope -PrincipalContext $principalContext -IdentityValue $identityValue } catch [System.Runtime.InteropServices.COMException] { @@ -2069,7 +2069,7 @@ function Resolve-SidToPrincipal $Scope ) - $principal = Find-Principal -PrincipalContext $PrincipalContext -IdentityValue $Sid.Value -IdentityType ([System.DirectoryServices.AccountManagement.IdentityType]::Sid) + $principal = Find-Principal -Scope $Scope -PrincipalContext $PrincipalContext -IdentityValue $Sid.Value -IdentityType ([System.DirectoryServices.AccountManagement.IdentityType]::Sid) if ($null -eq $principal) { @@ -2176,7 +2176,7 @@ function Get-PrincipalContext $principalContext = New-Object -TypeName 'System.DirectoryServices.AccountManagement.PrincipalContext' ` -ArgumentList @( [System.DirectoryServices.AccountManagement.ContextType]::Domain, $Scope, - $principalContextName, $Credential.GetNetworkCredential().Password ) + $principalContextName, $Credential.GetNetworkCredential().Password ) # Cache the PrincipalContext for this scope for subsequent calls. $null = $PrincipalContextCache.Add($Scope, $principalContext) @@ -2215,7 +2215,7 @@ function Test-IsLocalMachine $Scope ) - $localMachineScopes = @( '.', $env:computerName, 'localhost', '127.0.0.1', 'NT Authority', 'NT Service', 'BuiltIn' ) + $localMachineScopes = @( '.', $env:computerName, 'localhost', '127.0.0.1', 'NT Authority', 'NT Service', 'BuiltIn', 'IIS APPPOOL', 'NT Virtual Machine' ) if ($localMachineScopes -icontains $Scope) { @@ -2247,6 +2247,62 @@ function Test-IsLocalMachine return $false } +<# +.SYNOPSIS + This function extracts the scope from a distinguished name string. +.DESCRIPTION + This function takes a distinguished name string as input and extracts the scope from it. +.PARAMETER DistinguishedName + Specifies the distinguished name string from which the scope is to be extracted. +.EXAMPLE + Get-ScopeFromDistinguishedName -DistinguishedName "CN=John Doe,OU=Users,DC=example,DC=com" + This example extracts the scope "example.com" from the distinguished name string "CN=John Doe,OU=Users,DC=example,DC=com". +#> +function Get-ScopeFromDistinguishedName +{ + param ( + [Parameter(Mandatory = $true)] + [string]$DistinguishedName + ) + + $domainComponents = @() + $escaped = $false + $currentComponent = "" + + foreach ($char in $DistinguishedName.ToCharArray()) + { + if (-not $escaped -and $char -eq '\') + { + $escaped = $true + $currentComponent += $char + } + elseif ($escaped) + { + $currentComponent += $char + $escaped = $false + } + elseif ($char -eq ',') + { + if ($currentComponent.StartsWith('DC=')) + { + $domainComponents += $currentComponent + } + $currentComponent = "" + } + else + { + $currentComponent += $char + } + } + if ($currentComponent.StartsWith('DC=')) + { + $domainComponents += $currentComponent + } + + $scope = ($domainComponents -replace '^DC=') -join '.' + return $scope +} + <# .SYNOPSIS Splits a member name into the scope and the account name. @@ -2312,11 +2368,6 @@ function Split-MemberName { $scope = $MemberName.Substring(0, $separatorIndex) - if (Test-IsLocalMachine -Scope $scope) - { - $scope = $env:computerName - } - $accountName = $MemberName.Substring($separatorIndex + 1) return [System.String[]] @( $scope, $accountName ) @@ -2333,9 +2384,9 @@ function Split-MemberName } # Parse distinguished name for the scope - $distinguishedNamePrefix = 'DC=' + $domainComponentPrefix = 'DC=' - $separatorIndex = $MemberName.IndexOf($distinguishedNamePrefix, [System.StringComparison]::OrdinalIgnoreCase) + $separatorIndex = $MemberName.IndexOf($domainComponentPrefix, [System.StringComparison]::OrdinalIgnoreCase) if ($separatorIndex -ne -1) { <# @@ -2344,16 +2395,8 @@ function Split-MemberName See the initialization of $accountName above. #> - $startScopeIndex = $separatorIndex + $distinguishedNamePrefix.Length - $endScopeIndex = $MemberName.IndexOf(',', $startScopeIndex) - - if ($endScopeIndex -gt $startScopeIndex) - { - $scopeLength = $endScopeIndex - $separatorIndex - $distinguishedNamePrefix.Length - $scope = $MemberName.Substring($startScopeIndex, $scopeLength) - - return [System.String[]] @( $scope, $accountName ) - } + $scope = Get-ScopeFromDistinguishedName -DistinguishedName $MemberName + return [System.String[]] @( $scope, $accountName ) } return [System.String[]] @( $scope, $accountName ) @@ -2383,6 +2426,10 @@ function Find-Principal [System.DirectoryServices.AccountManagement.PrincipalContext] $PrincipalContext, + [Parameter(Mandatory = $true)] + [string] + $Scope, + [Parameter(Mandatory = $true)] [System.String] $IdentityValue, @@ -2392,6 +2439,15 @@ function Find-Principal $IdentityType ) + $ntAccountScopes = @( 'NT Authority', 'NT Service', 'IIS APPPOOL', 'NT Virtual Machine' ) + + if ($ntAccountScopes -icontains $Scope -and $IdentityValue -notlike 'S-*') + { + $fullIdentityValue = "$Scope\$IdentityValue" + $ntAccount = New-Object System.Security.Principal.NTAccount($fullIdentityValue) + $sid = $ntAccount.Translate([System.Security.Principal.SecurityIdentifier]).Value + return [System.DirectoryServices.AccountManagement.Principal]::FindByIdentity($PrincipalContext, [System.DirectoryServices.AccountManagement.IdentityType]::Sid, $sid) + } if ($PSBoundParameters.ContainsKey('IdentityType')) { return [System.DirectoryServices.AccountManagement.Principal]::FindByIdentity($PrincipalContext, $IdentityType, $IdentityValue) diff --git a/tests/Integration/DSC_xGroupResource.Integration.Tests.ps1 b/tests/Integration/DSC_xGroupResource.Integration.Tests.ps1 index d1795f15d..f7ac0b7a5 100644 --- a/tests/Integration/DSC_xGroupResource.Integration.Tests.ps1 +++ b/tests/Integration/DSC_xGroupResource.Integration.Tests.ps1 @@ -19,6 +19,7 @@ $script:testEnvironment = Initialize-TestEnvironment ` -ResourceType 'Mof' ` -TestType 'Integration' +Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath '..\..\output\xPSDesiredStateConfiguration\*\DSCResources\DSC_xGroupResource\DSC_xGroupResource.psm1') Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath '..\TestHelpers\CommonTestHelper.psm1') Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath '..\TestHelpers\DSC_xGroupResource.TestHelper.psm1') @@ -85,6 +86,25 @@ try } } + $accounts = @( + # Not every computer will have these accounts, so we'll skip them for now + # @{accountName = 'IIS APPPOOL\DefaultAppPool' }, + # @{accountName = 'NT SERVICE\MSSQL$SQLEXPRESS' }, + # @{accountName = 'NT Virtual Machine\Virtual Machines'}, + @{accountName = 'BuiltIn\Users' }, + @{accountName = 'NT AUTHORITY\NETWORK SERVICE' }, + @{accountName = 'NT AUTHORITY\LOCAL SERVICE' }, + @{accountName = 'NT AUTHORITY\SYSTEM' } + ) + + It 'Should succeed when calling ConvertTo-Principal with account: ' -TestCases $accounts { + param($accountName) + + $result = DSC_xGroupResource\ConvertTo-Principal -MemberName $accountName -PrincipalContextCache @{} -Disposables @() + + $result | Should -Not -BeNullOrEmpty + } + It 'Should not change the state of the present built-in Users group when no Members specified' { $configurationName = 'BuiltInGroup' $testGroupName = 'Users' diff --git a/tests/Unit/DSC_xGroupResource.Tests.ps1 b/tests/Unit/DSC_xGroupResource.Tests.ps1 index 874366bd3..e5abaaabd 100644 --- a/tests/Unit/DSC_xGroupResource.Tests.ps1 +++ b/tests/Unit/DSC_xGroupResource.Tests.ps1 @@ -276,9 +276,7 @@ try $testMemberName = 'domain\username' $splitMemberNameResult = Split-MemberName -MemberName $testMemberName - Assert-MockCalled -CommandName 'Test-IsLocalMachine' - - $splitMemberNameResult | Should -Be @( $script:localDomain, 'username' ) + $splitMemberNameResult | Should -Be @( 'domain', 'username' ) } Mock -CommandName 'Test-IsLocalMachine' -MockWith { return $false } @@ -287,8 +285,6 @@ try $testMemberName = 'domain\username' $splitMemberNameResult = Split-MemberName -MemberName $testMemberName - Assert-MockCalled -CommandName 'Test-IsLocalMachine' - $splitMemberNameResult | Should -Be @( 'domain', 'username' ) } @@ -303,14 +299,14 @@ try $testMemberName = 'CN=username,DC=domain' $splitMemberNameResult = Split-MemberName -MemberName $testMemberName - $splitMemberNameResult | Should -Be @( $script:localDomain, $testMemberName ) + $splitMemberNameResult | Should -Be @( 'domain', $testMemberName ) } - It 'Should split a member name in the CN=username,DC=domain format with outisde domain' { + It 'Should split a member name in the CN=username,DC=domain,DC=com format with outside domain' { $testMemberName = 'CN=username,DC=domain,DC=com' $splitMemberNameResult = Split-MemberName -MemberName $testMemberName - $splitMemberNameResult | Should -Be @( 'domain', $testMemberName ) + $splitMemberNameResult | Should -Be @( 'domain.com', $testMemberName ) } It 'Should split a member name in the local username format' { @@ -321,6 +317,38 @@ try } } + Context 'xGroupResource\Get-ScopeFromDistinguishedName' { + + It 'when given a valid distinguished name it returns the correct domain name' { + $distinguishedName = 'CN=John Doe,OU=Users,DC=example,DC=com' + $expectedDomain = 'example.com' + $result = Get-ScopeFromDistinguishedName -DistinguishedName $distinguishedName + $result | Should -Be $expectedDomain + } + + It 'when given a distinguished name with escaped characters, it returns the correct domain name' { + $distinguishedName = 'CN=Jane\, Doe,OU=Users,DC=example,DC=com' + $expectedDomain = 'example.com' + $result = Get-ScopeFromDistinguishedName -DistinguishedName $distinguishedName + $result | Should -Be $expectedDomain + } + + It 'when given a distinguished name with multiple domain components, returns the correct domain name' { + $distinguishedName = 'CN=John Doe,OU=Users,DC=example,DC=org' + $expectedDomain = 'example.org' + $result = Get-ScopeFromDistinguishedName -DistinguishedName $distinguishedName + $result | Should -Be $expectedDomain + } + + It 'when given a distinguished name with no domain components, it returns an empty string' { + $distinguishedName = 'CN=John Doe,OU=Users' + $expectedDomain = '' + $result = Get-ScopeFromDistinguishedName -DistinguishedName $distinguishedName + $result | Should -Be $expectedDomain + } + + } + if ($script:onNanoServer) { Context 'xGroupResource\Get-TargetResourceOnNanoServer' { @@ -1088,7 +1116,7 @@ try Assert-MockCalled -CommandName 'Remove-DisposableObject' } - Mock -CommandName 'Get-Group' -MockWith { return $script:testGroup } + Mock -CommandName 'Get-Group' -MockWith { return $script:testGroup } It 'Should add a member to an existing group with no members using Members' { $testMembers = @( $script:testUserPrincipal1.Name ) @@ -1902,9 +1930,9 @@ try $errorMessage = ($script:localizedData.CouldNotFindPrincipal -f $script:testUserPrincipal1.Name) { $null = ConvertTo-Principal ` - -MemberName $script:testUserPrincipal1.Name ` - -PrincipalContextCache $principalContextCache ` - -Disposables $disposables } | Should -Throw -ExpectedMessage $errorMessage + -MemberName $script:testUserPrincipal1.Name ` + -PrincipalContextCache $principalContextCache ` + -Disposables $disposables } | Should -Throw -ExpectedMessage $errorMessage } } From 22c71effcc7e7723aefae3cd399cd088ff5ada86 Mon Sep 17 00:00:00 2001 From: Kenny Scelfo Date: Mon, 6 Nov 2023 12:16:45 -0600 Subject: [PATCH 2/5] Updating chagelog --- CHANGELOG.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51fa94b86..09883f2e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,13 +5,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- xGroup + - Fixed a bug where the resource would fail if any of the group members were + local, virtual accounts. + - Fixed a bug where members with distinguished names that contained multiple + domain components would only have the first domain component included in + the account scope. + - Fixed a bug where members with distinguished names that contained an + escaped comma would not be parsed correctly. - xPackage - Fixed a bug not allowing using the file hash of an installer [Issue #702](https://github.com/dsccommunity/xPSDesiredStateConfiguration/issues/702). - xPSDesiredStateConfiguration - Updated CI pipeline to remove Azure DevOps deprecated Windows Server 2016 image and add Windows Server 2022 - Fixes [Issue #752](https://github.com/dsccommunity/xPSDesiredStateConfiguration/issues/752). - xDSCWebService - - Fixed a bug where the variable ```DscWebServiceDefaultAppPoolName``` is not set in the resource xDSCWebService since version 9.0.0 as a result of module refactoring. + - Fixed a bug where the variable ```DscWebServiceDefaultAppPoolName``` is not set in the resource xDSCWebService since version 9.0.0 as a result of module refactoring ### Fixed - xPSDesiredStateConfiguration From 56c887aa4940385490a44c23fe35d31d7063fbf0 Mon Sep 17 00:00:00 2001 From: Kenny Scelfo Date: Mon, 6 Nov 2023 16:01:59 -0600 Subject: [PATCH 3/5] Fixing assertion in xDSCWebService for mock call count. --- tests/Unit/DSC_xDSCWebService.Tests.ps1 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Unit/DSC_xDSCWebService.Tests.ps1 b/tests/Unit/DSC_xDSCWebService.Tests.ps1 index 218b86656..11ee6a7e1 100644 --- a/tests/Unit/DSC_xDSCWebService.Tests.ps1 +++ b/tests/Unit/DSC_xDSCWebService.Tests.ps1 @@ -720,7 +720,7 @@ try Assert-MockCalled -Exactly -Times 3 -CommandName Get-Command Assert-MockCalled -Exactly -Times 1 -CommandName Get-Culture Assert-MockCalled -Exactly -Times 2 -CommandName Get-Website -ModuleName xPSDesiredStateConfiguration.PSWSIIS - Assert-MockCalled -Exactly -Times 3 -CommandName Test-Path + Assert-MockCalled -Exactly -Times 2 -CommandName Test-Path Assert-MockCalled -Exactly -Times 2 -CommandName Get-OsVersion Assert-MockCalled -Exactly -Times 0 -CommandName Add-PullServerFirewallConfiguration Assert-MockCalled -Exactly -Times 3 -CommandName Update-LocationTagInApplicationHostConfigForAuthentication @@ -784,7 +784,7 @@ try Assert-MockCalled -Exactly -Times 3 -CommandName Get-Command Assert-MockCalled -Exactly -Times 1 -CommandName Get-Culture - Assert-MockCalled -Exactly -Times 3 -CommandName Test-Path + Assert-MockCalled -Exactly -Times 2 -CommandName Test-Path Assert-MockCalled -Exactly -Times 2 -CommandName Get-OsVersion Assert-MockCalled -Exactly -Times 3 -CommandName Update-LocationTagInApplicationHostConfigForAuthentication Assert-MockCalled -Exactly -Times 5 -CommandName Set-AppSettingsInWebconfig @@ -823,7 +823,7 @@ try Assert-MockCalled -Exactly -Times 3 -CommandName Get-Command Assert-MockCalled -Exactly -Times 1 -CommandName Get-Culture Assert-MockCalled -Exactly -Times 2 -CommandName Get-Website -ModuleName xPSDesiredStateConfiguration.PSWSIIS - Assert-MockCalled -Exactly -Times 3 -CommandName Test-Path + Assert-MockCalled -Exactly -Times 2 -CommandName Test-Path Assert-MockCalled -Exactly -Times 2 -CommandName Get-OsVersion Assert-MockCalled -Exactly -Times 3 -CommandName Update-LocationTagInApplicationHostConfigForAuthentication Assert-MockCalled -Exactly -Times 5 -CommandName Set-AppSettingsInWebconfig @@ -855,7 +855,7 @@ try Assert-MockCalled -Exactly -Times 3 -CommandName Get-Command Assert-MockCalled -Exactly -Times 1 -CommandName Get-Culture Assert-MockCalled -Exactly -Times 2 -CommandName Get-Website -ModuleName xPSDesiredStateConfiguration.PSWSIIS - Assert-MockCalled -Exactly -Times 3 -CommandName Test-Path + Assert-MockCalled -Exactly -Times 2 -CommandName Test-Path Assert-MockCalled -Exactly -Times 2 -CommandName Get-OsVersion Assert-MockCalled -Exactly -Times 3 -CommandName Update-LocationTagInApplicationHostConfigForAuthentication Assert-MockCalled -Exactly -Times 5 -CommandName Set-AppSettingsInWebconfig From 566d5dfa759a0614ebdacbdec9c731636ce66ad4 Mon Sep 17 00:00:00 2001 From: Kenny Scelfo Date: Tue, 7 Nov 2023 08:56:51 -0600 Subject: [PATCH 4/5] - Modify assertion for the number of calls to the Test-Path mock to account for the short-circuiting of the if conditions on line 449 of DSC_xDSCWebService.psm1 preventing the third condition from being evaluated. --- tests/Unit/DSC_xDSCWebService.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/DSC_xDSCWebService.Tests.ps1 b/tests/Unit/DSC_xDSCWebService.Tests.ps1 index 11ee6a7e1..3e8f5ca0d 100644 --- a/tests/Unit/DSC_xDSCWebService.Tests.ps1 +++ b/tests/Unit/DSC_xDSCWebService.Tests.ps1 @@ -888,7 +888,7 @@ try Assert-MockCalled -Exactly -Times 1 -CommandName Get-Command Assert-MockCalled -Exactly -Times 1 -CommandName Get-Culture Assert-MockCalled -Exactly -Times 2 -CommandName Get-Website -ModuleName xPSDesiredStateConfiguration.PSWSIIS - Assert-MockCalled -Exactly -Times 2 -CommandName Test-Path + Assert-MockCalled -Exactly -Times 1 -CommandName Test-Path Assert-MockCalled -Exactly -Times 2 -CommandName Get-OsVersion Assert-MockCalled -Exactly -Times 3 -CommandName Update-LocationTagInApplicationHostConfigForAuthentication Assert-MockCalled -Exactly -Times 5 -CommandName Set-AppSettingsInWebconfig From c7261bb2f600b05399d251dad720e64234933295 Mon Sep 17 00:00:00 2001 From: Kenny Scelfo Date: Fri, 10 Nov 2023 11:30:27 -0600 Subject: [PATCH 5/5] - Fixing issues identified during the pull request review process --- CHANGELOG.md | 10 +++------- .../DSC_xGroupResource.psm1 | 20 ++++++++++++------- tests/Unit/DSC_xGroupResource.Tests.ps1 | 11 +++++----- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09883f2e9..6adc1acb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,13 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] - xGroup - - Fixed a bug where the resource would fail if any of the group members were - local, virtual accounts. - - Fixed a bug where members with distinguished names that contained multiple - domain components would only have the first domain component included in - the account scope. - - Fixed a bug where members with distinguished names that contained an - escaped comma would not be parsed correctly. + - Fixed a bug where the resource would fail if any of the group members were local, virtual accounts. - Fixes [Issue #763](https://github.com/dsccommunity/xPSDesiredStateConfiguration/issues/763) + - Fixed a bug where members with distinguished names that contained multiple domain components would only have the first domain component included in the account scope. + - Fixed a bug where members with distinguished names that contained an escaped comma would not be parsed correctly. - xPackage - Fixed a bug not allowing using the file hash of an installer [Issue #702](https://github.com/dsccommunity/xPSDesiredStateConfiguration/issues/702). - xPSDesiredStateConfiguration diff --git a/source/DSCResources/DSC_xGroupResource/DSC_xGroupResource.psm1 b/source/DSCResources/DSC_xGroupResource/DSC_xGroupResource.psm1 index 7a11c0ff6..ca308fd58 100644 --- a/source/DSCResources/DSC_xGroupResource/DSC_xGroupResource.psm1 +++ b/source/DSCResources/DSC_xGroupResource/DSC_xGroupResource.psm1 @@ -2255,19 +2255,23 @@ function Test-IsLocalMachine .PARAMETER DistinguishedName Specifies the distinguished name string from which the scope is to be extracted. .EXAMPLE - Get-ScopeFromDistinguishedName -DistinguishedName "CN=John Doe,OU=Users,DC=example,DC=com" - This example extracts the scope "example.com" from the distinguished name string "CN=John Doe,OU=Users,DC=example,DC=com". + Get-ScopeFromDistinguishedName -DistinguishedName 'CN=John Doe,OU=Users,DC=example,DC=com' + This example extracts the scope 'example.com' from the distinguished name string 'CN=John Doe,OU=Users,DC=example,DC=com'. + + Get-ScopeFromDistinguishedName -DistinguishedName 'CN=Doe\, Jane,OU=Users,DC=example,DC=com' + This example extracts the scope 'example.com' from the distinguished name string 'CN=Doe\, Jane,OU=Users,DC=example,DC=com'. #> function Get-ScopeFromDistinguishedName { param ( [Parameter(Mandatory = $true)] - [string]$DistinguishedName + [System.String] + $DistinguishedName ) $domainComponents = @() $escaped = $false - $currentComponent = "" + $currentComponent = '' foreach ($char in $DistinguishedName.ToCharArray()) { @@ -2287,7 +2291,7 @@ function Get-ScopeFromDistinguishedName { $domainComponents += $currentComponent } - $currentComponent = "" + $currentComponent = '' } else { @@ -2427,7 +2431,7 @@ function Find-Principal $PrincipalContext, [Parameter(Mandatory = $true)] - [string] + [System.String] $Scope, [Parameter(Mandatory = $true)] @@ -2444,10 +2448,12 @@ function Find-Principal if ($ntAccountScopes -icontains $Scope -and $IdentityValue -notlike 'S-*') { $fullIdentityValue = "$Scope\$IdentityValue" - $ntAccount = New-Object System.Security.Principal.NTAccount($fullIdentityValue) + $ntAccount = New-Object -TypeName 'System.Security.Principal.NTAccount' ` + -ArgumentList @($fullIdentityValue) $sid = $ntAccount.Translate([System.Security.Principal.SecurityIdentifier]).Value return [System.DirectoryServices.AccountManagement.Principal]::FindByIdentity($PrincipalContext, [System.DirectoryServices.AccountManagement.IdentityType]::Sid, $sid) } + if ($PSBoundParameters.ContainsKey('IdentityType')) { return [System.DirectoryServices.AccountManagement.Principal]::FindByIdentity($PrincipalContext, $IdentityType, $IdentityValue) diff --git a/tests/Unit/DSC_xGroupResource.Tests.ps1 b/tests/Unit/DSC_xGroupResource.Tests.ps1 index e5abaaabd..2ea093ae3 100644 --- a/tests/Unit/DSC_xGroupResource.Tests.ps1 +++ b/tests/Unit/DSC_xGroupResource.Tests.ps1 @@ -318,29 +318,28 @@ try } Context 'xGroupResource\Get-ScopeFromDistinguishedName' { - - It 'when given a valid distinguished name it returns the correct domain name' { + It 'Should return the correct domain when given a valid distinguished name' { $distinguishedName = 'CN=John Doe,OU=Users,DC=example,DC=com' $expectedDomain = 'example.com' $result = Get-ScopeFromDistinguishedName -DistinguishedName $distinguishedName $result | Should -Be $expectedDomain } - It 'when given a distinguished name with escaped characters, it returns the correct domain name' { - $distinguishedName = 'CN=Jane\, Doe,OU=Users,DC=example,DC=com' + It 'Should return the correct domain when given a valid distinguished name containing escaped characters' { + $distinguishedName = 'CN=Doe\, Jane,OU=Users,DC=example,DC=com' $expectedDomain = 'example.com' $result = Get-ScopeFromDistinguishedName -DistinguishedName $distinguishedName $result | Should -Be $expectedDomain } - It 'when given a distinguished name with multiple domain components, returns the correct domain name' { + It 'Should return the correct domain when given a valid distinguished name containing multiple domain components' { $distinguishedName = 'CN=John Doe,OU=Users,DC=example,DC=org' $expectedDomain = 'example.org' $result = Get-ScopeFromDistinguishedName -DistinguishedName $distinguishedName $result | Should -Be $expectedDomain } - It 'when given a distinguished name with no domain components, it returns an empty string' { + It 'Should return an empty string when given a valid distinguished name with no domain components' { $distinguishedName = 'CN=John Doe,OU=Users' $expectedDomain = '' $result = Get-ScopeFromDistinguishedName -DistinguishedName $distinguishedName