Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPShellAdmins: Member comparison was case sensitive. #1440

Merged
merged 10 commits into from
Nov 25, 2024
Merged

SPShellAdmins: Member comparison was case sensitive. #1440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e6d8a02
Fix: Member comparison was case sensitive.
Nov 18, 2024
14bff26
Fix: Set GitVersion to specific Version
ChristophHannappel Nov 18, 2024
da32151
Fix: Exclude SharePointDsc.Reverse.psm1 from HQRm Tests
Nov 19, 2024
4957eba
Merge branch 'fix/SPShellAdmins' of https://github.com/ChristophHanna…
Nov 19, 2024
64e349c
Fix: yaml syntax
Nov 19, 2024
c72695e
Fix: Allow DscResource.Test Prerelease to fix HQRM Test on SharePoint…
Nov 19, 2024
a8e91b8
Fix: HQRM Exclusion
Nov 19, 2024
b7dfd25
Fix: Try to exclude SharePointDsc.Reverse.psm1 with Fileextension
Nov 19, 2024
eef72c9
Fix: Corrected Exclusion Path
Nov 21, 2024
db9df9b
Fix: Adds UnitTest for AllDatabases and MemberToInclue and MemberToEx…
Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion RequiredModules.psd1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
24 changes: 12 additions & 12 deletions SharePointDsc/DSCResources/MSFT_SPShellAdmins/MSFT_SPShellAdmins.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@
{
foreach ($member in $params.MembersToInclude)
{
if (-not $shellAdmins.UserName.Contains($member))
if ($shellAdmins.UserName -notcontains $member)
{
try
{
Expand Down Expand Up @@ -479,7 +479,7 @@
{
foreach ($member in $params.MembersToExclude)
{
if ($shellAdmins.UserName.Contains($member))
if ($shellAdmins.UserName -contains $member)
{
try
{
Expand Down Expand Up @@ -651,7 +651,7 @@
{
foreach ($member in $database.MembersToInclude)
{
if (-not $dbShellAdmins.UserName.Contains($member))
if ($dbShellAdmins.UserName -notcontains $member)
{
try
{
Expand Down Expand Up @@ -745,7 +745,7 @@
{
foreach ($member in $database.MembersToExclude)
{
if ($dbShellAdmins.UserName.Contains($member))
if ($dbShellAdmins.UserName -contains $member)
{
try
{
Expand Down Expand Up @@ -937,7 +937,7 @@
{
foreach ($member in $params.MembersToInclude)
{
if (-not $dbShellAdmins.UserName.Contains($member))
if ($dbShellAdmins.UserName -notcontains $member)
{
try
{
Expand Down Expand Up @@ -1031,7 +1031,7 @@
{
foreach ($member in $params.MembersToExclude)
{
if ($dbShellAdmins.UserName.Contains($member))
if ($dbShellAdmins.UserName -contains $member)
{
try
{
Expand Down Expand Up @@ -1164,7 +1164,7 @@

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
Expand All @@ -1187,7 +1187,7 @@
{
foreach ($member in $MembersToExclude)
{
if ($CurrentValues.Members.Contains($member))
if ($CurrentValues.Members -contains $member)
{
$message = "$member is a Shell Admin."
Write-Verbose -Message $message
Expand Down Expand Up @@ -1260,7 +1260,7 @@

foreach ($member in $MembersToInclude)
{
if (-not($database.Members.Contains($member)))
if ($database.Members -notcontains $member)

Check warning on line 1263 in SharePointDsc/DSCResources/MSFT_SPShellAdmins/MSFT_SPShellAdmins.psm1

View check run for this annotation

Codecov / codecov/patch

SharePointDsc/DSCResources/MSFT_SPShellAdmins/MSFT_SPShellAdmins.psm1#L1263 

Added line #L1263 was not covered by tests
{
$message = "$member is not a Shell Admin."
Write-Verbose -Message $message
Expand All @@ -1282,7 +1282,7 @@
{
foreach ($member in $MembersToExclude)
{
if ($database.Members.Contains($member))
if ($database.Members -contains $member)

Check warning on line 1285 in SharePointDsc/DSCResources/MSFT_SPShellAdmins/MSFT_SPShellAdmins.psm1

View check run for this annotation

Codecov / codecov/patch

SharePointDsc/DSCResources/MSFT_SPShellAdmins/MSFT_SPShellAdmins.psm1#L1285 

Added line #L1285 was not covered by tests
{
$message = "$member is a Shell Admin."
Write-Verbose -Message $message
Expand Down Expand Up @@ -1365,7 +1365,7 @@

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
Expand All @@ -1388,7 +1388,7 @@
{
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
Expand Down
2 changes: 1 addition & 1 deletion azure-pipelines.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)'."
Expand Down
1 change: 1 addition & 0 deletions build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ DscTest:
Tag:
ExcludeSourceFile:
- output
- Modules/SharePointDsc.Reverse/SharePointDsc.Reverse.psm1
ExcludeModuleFile:
- Modules/DscResource.Common

Expand Down
134 changes: 126 additions & 8 deletions tests/Unit/SharePointDsc/SharePointDsc.SPShellAdmins.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = @{
Expand Down Expand Up @@ -517,8 +627,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"
}
}
}
Expand Down Expand Up @@ -623,15 +734,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"
}
}
}
Expand Down Expand Up @@ -720,8 +833,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"
}
}
}
Expand Down Expand Up @@ -825,15 +939,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"
}
}
}
Expand Down Expand Up @@ -964,15 +1080,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"
}
}
}
Expand Down