From e6d8a02e953ab11e18682baeb65ee5c8c2a4a48d Mon Sep 17 00:00:00 2001 From: "Hannappel, Christoph" <christoph.hannappel@dataport.de> Date: Mon, 18 Nov 2024 12:47:53 +0100 Subject: [PATCH 1/9] Fix: Member comparison was case sensitive. --- CHANGELOG.md | 5 ++++ .../MSFT_SPShellAdmins.psm1 | 24 +++++++++---------- .../SharePointDsc.SPShellAdmins.Tests.ps1 | 24 ++++++++++++------- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 627d75d9d..53208f4da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- SPShellAdmins + - Fixed that the Member comparison was not case insensitive. + ## [5.5.0] - 2024-04-22 ### Added diff --git a/SharePointDsc/DSCResources/MSFT_SPShellAdmins/MSFT_SPShellAdmins.psm1 b/SharePointDsc/DSCResources/MSFT_SPShellAdmins/MSFT_SPShellAdmins.psm1 index 999c7d374..daf88e08a 100644 --- a/SharePointDsc/DSCResources/MSFT_SPShellAdmins/MSFT_SPShellAdmins.psm1 +++ b/SharePointDsc/DSCResources/MSFT_SPShellAdmins/MSFT_SPShellAdmins.psm1 @@ -424,7 +424,7 @@ function Set-TargetResource { foreach ($member in $params.MembersToInclude) { - if (-not $shellAdmins.UserName.Contains($member)) + if ($shellAdmins.UserName -notcontains $member) { try { @@ -479,7 +479,7 @@ function Set-TargetResource { foreach ($member in $params.MembersToExclude) { - if ($shellAdmins.UserName.Contains($member)) + if ($shellAdmins.UserName -contains $member) { try { @@ -651,7 +651,7 @@ function Set-TargetResource { foreach ($member in $database.MembersToInclude) { - if (-not $dbShellAdmins.UserName.Contains($member)) + if ($dbShellAdmins.UserName -notcontains $member) { try { @@ -745,7 +745,7 @@ function Set-TargetResource { foreach ($member in $database.MembersToExclude) { - if ($dbShellAdmins.UserName.Contains($member)) + if ($dbShellAdmins.UserName -contains $member) { try { @@ -937,7 +937,7 @@ function Set-TargetResource { foreach ($member in $params.MembersToInclude) { - if (-not $dbShellAdmins.UserName.Contains($member)) + if ($dbShellAdmins.UserName -notcontains $member) { try { @@ -1031,7 +1031,7 @@ function Set-TargetResource { foreach ($member in $params.MembersToExclude) { - if ($dbShellAdmins.UserName.Contains($member)) + if ($dbShellAdmins.UserName -contains $member) { try { @@ -1164,7 +1164,7 @@ function Test-TargetResource foreach ($member in $MembersToInclude) { - if (-not($CurrentValues.Members.Contains($member))) + if ($CurrentValues.Members -notcontains $member) { $message = "$member is not a Shell Admin." Write-Verbose -Message $message @@ -1187,7 +1187,7 @@ function Test-TargetResource { foreach ($member in $MembersToExclude) { - if ($CurrentValues.Members.Contains($member)) + if ($CurrentValues.Members -contains $member) { $message = "$member is a Shell Admin." Write-Verbose -Message $message @@ -1260,7 +1260,7 @@ function Test-TargetResource foreach ($member in $MembersToInclude) { - if (-not($database.Members.Contains($member))) + if ($database.Members -notcontains $member) { $message = "$member is not a Shell Admin." Write-Verbose -Message $message @@ -1282,7 +1282,7 @@ function Test-TargetResource { foreach ($member in $MembersToExclude) { - if ($database.Members.Contains($member)) + if ($database.Members -contains $member) { $message = "$member is a Shell Admin." Write-Verbose -Message $message @@ -1365,7 +1365,7 @@ function Test-TargetResource foreach ($member in $database.MembersToInclude) { - if (-not($currentCDB.Members.Contains($member))) + if ($currentCDB.Members -notcontains $member) { $message = "$member is not a Shell Admin." Write-Verbose -Message $message @@ -1388,7 +1388,7 @@ function Test-TargetResource { foreach ($member in $database.MembersToExclude) { - if ($currentCDB.Members.Contains($member)) + if ($currentCDB.Members -contains $member) { $message = "$member is a Shell Admin." Write-Verbose -Message $message diff --git a/tests/Unit/SharePointDsc/SharePointDsc.SPShellAdmins.Tests.ps1 b/tests/Unit/SharePointDsc/SharePointDsc.SPShellAdmins.Tests.ps1 index 17dbfb47b..0c2822d71 100644 --- a/tests/Unit/SharePointDsc/SharePointDsc.SPShellAdmins.Tests.ps1 +++ b/tests/Unit/SharePointDsc/SharePointDsc.SPShellAdmins.Tests.ps1 @@ -517,8 +517,9 @@ try else { # Database parameter not used, return general permissions + # The Username "contoso\User2" is mocked with Upercase to test if the resource is case insenitive. return @{ - UserName = "contoso\user1", "contoso\user2" + UserName = "contoso\user1", "contoso\User2" } } } @@ -623,15 +624,17 @@ try if ($database) { # Database parameter used, return database permissions + # The Username "contoso\User2" is mocked with Upercase to test if the resource is case insenitive. return @{ - UserName = "contoso\user1", "contoso\user2" + UserName = "contoso\user1", "contoso\User2" } } else { # Database parameter not used, return general permissions + # The Username "contoso\User2" is mocked with Upercase to test if the resource is case insenitive. return @{ - UserName = "contoso\user1", "contoso\user2" + UserName = "contoso\user1", "contoso\User2" } } } @@ -720,8 +723,9 @@ try else { # Database parameter not used, return general permissions + # The Username "contoso\User2" is mocked with Upercase to test if the resource is case insenitive. return @{ - UserName = "contoso\user1", "contoso\user2", "contoso\user3" + UserName = "contoso\user1", "contoso\User2", "contoso\user3" } } } @@ -825,15 +829,17 @@ try if ($database) { # Database parameter used, return database permissions + # The Username "contoso\User2" is mocked with Upercase to test if the resource is case insenitive. return @{ - UserName = "contoso\user1", "contoso\user2", "contoso\user3" + UserName = "contoso\user1", "contoso\User2", "contoso\user3" } } else { # Database parameter not used, return general permissions + # The Username "contoso\User2" is mocked with Upercase to test if the resource is case insenitive. return @{ - UserName = "contoso\user1", "contoso\user2" + UserName = "contoso\user1", "contoso\User2" } } } @@ -964,15 +970,17 @@ try if ($database) { # Database parameter used, return database permissions + # The Username "contoso\User2" is mocked with Upercase to test if the resource is case insenitive. return @{ - UserName = "contoso\user1", "contoso\user2" + UserName = "contoso\user1", "contoso\User2" } } else { # Database parameter not used, return general permissions + # The Username "contoso\User2" is mocked with Upercase to test if the resource is case insenitive. return @{ - UserName = "contoso\user1", "contoso\user2" + UserName = "contoso\user1", "contoso\User2" } } } From 14bff26767f8ea9bb3d356185512b0323a6523e7 Mon Sep 17 00:00:00 2001 From: Christoph Hannappel <37103421+ChristophHannappel@users.noreply.github.com> Date: Mon, 18 Nov 2024 19:04:57 +0100 Subject: [PATCH 2/9] Fix: Set GitVersion to specific Version --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index d9c9994cb..76580a55b 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -26,7 +26,7 @@ stages: vmImage: "windows-latest" steps: - pwsh: | - dotnet tool install --global GitVersion.Tool + dotnet tool install --global GitVersion.Tool --version 5.12.0 $gitVersionObject = dotnet-gitversion | ConvertFrom-Json $gitVersionObject.PSObject.Properties.ForEach{ Write-Host -Object "Setting Task Variable '$($_.Name)' with value '$($_.Value)'." From da32151031ff2d37fc6f3c0d28802cbd198fd21c Mon Sep 17 00:00:00 2001 From: "Hannappel, Christoph" <christoph.hannappel@dataport.de> Date: Tue, 19 Nov 2024 10:20:37 +0100 Subject: [PATCH 3/9] Fix: Exclude SharePointDsc.Reverse.psm1 from HQRm Tests --- build.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/build.yaml b/build.yaml index b8ef68c99..2a0b854f6 100644 --- a/build.yaml +++ b/build.yaml @@ -92,6 +92,7 @@ DscTest: Tag: ExcludeSourceFile: - output + - SharePointDsc/Modules/SharePointDsc.Reverse/SharePointDsc.Reverse.psm1 ExcludeModuleFile: - Modules/DscResource.Common From 64e349c4c074c3a37f2e0c04ba6ee244db731c48 Mon Sep 17 00:00:00 2001 From: "Hannappel, Christoph" <christoph.hannappel@dataport.de> Date: Tue, 19 Nov 2024 10:48:40 +0100 Subject: [PATCH 4/9] Fix: yaml syntax --- build.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.yaml b/build.yaml index 2a0b854f6..09b9e3cf9 100644 --- a/build.yaml +++ b/build.yaml @@ -92,7 +92,7 @@ DscTest: Tag: ExcludeSourceFile: - output - - SharePointDsc/Modules/SharePointDsc.Reverse/SharePointDsc.Reverse.psm1 + - 'SharePointDsc/Modules/SharePointDsc.Reverse/SharePointDsc.Reverse.psm1' ExcludeModuleFile: - Modules/DscResource.Common From c72695e73a9dbef863072087d5ff776f91993372 Mon Sep 17 00:00:00 2001 From: "Hannappel, Christoph" <christoph.hannappel@dataport.de> Date: Tue, 19 Nov 2024 13:01:45 +0100 Subject: [PATCH 5/9] Fix: Allow DscResource.Test Prerelease to fix HQRM Test on SharePointDsc.Reverse --- RequiredModules.psd1 | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/RequiredModules.psd1 b/RequiredModules.psd1 index 93a5515bb..111143eaf 100644 --- a/RequiredModules.psd1 +++ b/RequiredModules.psd1 @@ -17,7 +17,13 @@ 'Sampler.GitHubTasks' = 'latest' MarkdownLinkCheck = 'latest' 'DscResource.Common' = 'latest' - 'DscResource.Test' = 'latest' + # Using Prerelease to Fix HQRM Test for SharePointDsc.Reverse: https://github.com/dsccommunity/SharePointDsc/pull/1440#issuecomment-2485466538 + 'DscResource.Test' = @{ + Version = 'latest' + Parameters = @{ + AllowPrerelease = $true + } + } 'DscResource.AnalyzerRules' = 'latest' xDscResourceDesigner = 'latest' 'DscResource.DocGenerator' = 'latest' From a8e91b851a3c34826518fcd89937272799f75068 Mon Sep 17 00:00:00 2001 From: "Hannappel, Christoph" <christoph.hannappel@dataport.de> Date: Tue, 19 Nov 2024 13:37:44 +0100 Subject: [PATCH 6/9] Fix: HQRM Exclusion --- build.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/build.yaml b/build.yaml index 09b9e3cf9..3cf482dff 100644 --- a/build.yaml +++ b/build.yaml @@ -93,6 +93,8 @@ DscTest: ExcludeSourceFile: - output - 'SharePointDsc/Modules/SharePointDsc.Reverse/SharePointDsc.Reverse.psm1' + - 'SharePointDsc/Modules/SharePointDsc.Reverse/SharePointDsc.Reverse' + - SharePointDsc.Reverse ExcludeModuleFile: - Modules/DscResource.Common From b7dfd254b280cd1b82306a43fc1ee80a59cfbb98 Mon Sep 17 00:00:00 2001 From: "Hannappel, Christoph" <christoph.hannappel@dataport.de> Date: Tue, 19 Nov 2024 14:09:13 +0100 Subject: [PATCH 7/9] Fix: Try to exclude SharePointDsc.Reverse.psm1 with Fileextension --- build.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/build.yaml b/build.yaml index 3cf482dff..9e25bb445 100644 --- a/build.yaml +++ b/build.yaml @@ -95,6 +95,7 @@ DscTest: - 'SharePointDsc/Modules/SharePointDsc.Reverse/SharePointDsc.Reverse.psm1' - 'SharePointDsc/Modules/SharePointDsc.Reverse/SharePointDsc.Reverse' - SharePointDsc.Reverse + - SharePointDsc.Reverse.psm1 ExcludeModuleFile: - Modules/DscResource.Common From eef72c960e2d1b587101870a225b2d7e2fc4acfc Mon Sep 17 00:00:00 2001 From: "Hannappel, Christoph" <christoph.hannappel@dataport.de> Date: Thu, 21 Nov 2024 08:55:09 +0100 Subject: [PATCH 8/9] Fix: Corrected Exclusion Path --- build.yaml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/build.yaml b/build.yaml index 9e25bb445..a615f665e 100644 --- a/build.yaml +++ b/build.yaml @@ -92,10 +92,7 @@ DscTest: Tag: ExcludeSourceFile: - output - - 'SharePointDsc/Modules/SharePointDsc.Reverse/SharePointDsc.Reverse.psm1' - - 'SharePointDsc/Modules/SharePointDsc.Reverse/SharePointDsc.Reverse' - - SharePointDsc.Reverse - - SharePointDsc.Reverse.psm1 + - Modules/SharePointDsc.Reverse/SharePointDsc.Reverse.psm1 ExcludeModuleFile: - Modules/DscResource.Common From db9df9bd5b8d5bebe83736fb154762cac7dbd980 Mon Sep 17 00:00:00 2001 From: "Hannappel, Christoph" <christoph.hannappel@dataport.de> Date: Thu, 21 Nov 2024 15:07:25 +0100 Subject: [PATCH 9/9] Fix: Adds UnitTest for AllDatabases and MemberToInclue and MemberToExclude --- .../SharePointDsc.SPShellAdmins.Tests.ps1 | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/tests/Unit/SharePointDsc/SharePointDsc.SPShellAdmins.Tests.ps1 b/tests/Unit/SharePointDsc/SharePointDsc.SPShellAdmins.Tests.ps1 index 0c2822d71..de3833aa2 100644 --- a/tests/Unit/SharePointDsc/SharePointDsc.SPShellAdmins.Tests.ps1 +++ b/tests/Unit/SharePointDsc/SharePointDsc.SPShellAdmins.Tests.ps1 @@ -459,6 +459,116 @@ try } } + Context -Name "AllDatabases parameter is used with MembersToInclude and permissions do not match" -Fixture { + BeforeAll { + $testParams = @{ + IsSingleInstance = "Yes" + MembersToInclude = "contoso\user1", "contoso\user2", "contoso\sa_farm" + AllDatabases = $true + } + + Mock -CommandName Get-SPShellAdmin -MockWith { + if ($database) + { + # Database parameter used, return database permissions + return @{ + UserName = "contoso\user3", "contoso\user4" + } + } + else + { + # Database parameter not used, return general permissions + return @{ + UserName = "contoso\user3", "contoso\user4" + } + } + } + + Mock -CommandName Get-SPDatabase -MockWith { + return @( + @{ + Name = "SharePoint_Content_Contoso1" + Id = "F9168C5E-CEB2-4faa-B6BF-329BF39FA1E4" + NormalizedDataSource = 'SQL01' + }, + @{ + Name = "SharePoint_Content_Contoso2" + Id = "936DA01F-9ABD-4d9d-80C7-02AF85C822A8" + NormalizedDataSource = 'SQL01' + } + ) + } + } + + It "Should return null from the get method" { + Get-TargetResource @testParams | Should -Not -BeNullOrEmpty + } + + It "Should return false from the test method" { + Test-TargetResource @testParams | Should -Be $false + } + + It "Should throw an exception in the set method" { + Set-TargetResource @testParams + Assert-MockCalled Add-SPShellAdmin + } + } + + Context -Name "AllDatabases parameter is used with MembersToExclude and permissions do not match" -Fixture { + BeforeAll { + $testParams = @{ + IsSingleInstance = "Yes" + MembersToExclude = "contoso\user3", "contoso\user4" + AllDatabases = $true + } + + Mock -CommandName Get-SPShellAdmin -MockWith { + if ($database) + { + # Database parameter used, return database permissions + return @{ + UserName = "contoso\user1", "contoso\user3", "contoso\user4" + } + } + else + { + # Database parameter not used, return general permissions + return @{ + UserName = "contoso\user2", "contoso\user3", "contoso\user4" + } + } + } + + Mock -CommandName Get-SPDatabase -MockWith { + return @( + @{ + Name = "SharePoint_Content_Contoso1" + Id = "F9168C5E-CEB2-4faa-B6BF-329BF39FA1E4" + NormalizedDataSource = 'SQL01' + }, + @{ + Name = "SharePoint_Content_Contoso2" + Id = "936DA01F-9ABD-4d9d-80C7-02AF85C822A8" + NormalizedDataSource = 'SQL01' + } + ) + } + } + + It "Should return null from the get method" { + Get-TargetResource @testParams | Should -Not -BeNullOrEmpty + } + + It "Should return false from the test method" { + Test-TargetResource @testParams | Should -Be $false + } + + It "Should throw an exception in the set method" { + Set-TargetResource @testParams + Assert-MockCalled Remove-SPShellAdmin + } + } + Context -Name "Configured Members do not match the actual members - General permissions" -Fixture { BeforeAll { $testParams = @{