Skip to content

Commit

Permalink
Merge pull request #1440 from ChristophHannappel/fix/SPShellAdmins
Browse files Browse the repository at this point in the history 
SPShellAdmins: Member comparison was case sensitive.
  • Loading branch information
@ykuijs
ykuijs authored Nov 25, 2024
2 parents 13408f4 + db9df9b commit fd20715
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 22 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ function Set-TargetResource
{
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 @@ function Set-TargetResource
{
foreach ($member in $params.MembersToExclude)
{
if ($shellAdmins.UserName.Contains($member))
if ($shellAdmins.UserName -contains $member)
{
try
{
Expand Down Expand Up @@ -651,7 +651,7 @@ function Set-TargetResource
{
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 @@ function Set-TargetResource
{
foreach ($member in $database.MembersToExclude)
{
if ($dbShellAdmins.UserName.Contains($member))
if ($dbShellAdmins.UserName -contains $member)
{
try
{
Expand Down Expand Up @@ -937,7 +937,7 @@ function Set-TargetResource
{
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 @@ function Set-TargetResource
{
foreach ($member in $params.MembersToExclude)
{
if ($dbShellAdmins.UserName.Contains($member))
if ($dbShellAdmins.UserName -contains $member)
{
try
{
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion azure-pipelines.yml
View file
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
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
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

0 comments on commit fd20715

Please sign in to comment.