From 620e9b4931a5baf2b3745d1e5a141c2f4cde7a67 Mon Sep 17 00:00:00 2001 From: Brandon Date: Sun, 12 Jan 2020 03:30:00 -0500 Subject: [PATCH] SqlServerRole: Add support for nested roles and remove case-sensitive checks (#1453) - SqlServerRole - Add support for nested role membership (issue #1452). - Removed use of case-sensitive Contains() function when evalutating role membership (issue #1153). - Refactored mocks and unit tests to increase performance (issue #979). --- CHANGELOG.md | 5 + .../MSFT_SqlServerRole.psm1 | 105 ++-- .../en-US/MSFT_SqlServerRole.strings.psd1 | 2 +- .../MSFT_SqlServerRole.Integration.Tests.ps1 | 108 ++++ .../Integration/MSFT_SqlServerRole.config.ps1 | 77 +++ tests/Unit/MSFT_SqlServerRole.Tests.ps1 | 584 +++++++++++++----- 6 files changed, 704 insertions(+), 177 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 911b659dc..3e03fcf7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,11 @@ For older change log history see the [historic changelog](HISTORIC_CHANGELOG.md) - SqlDatabaseRole - Update unit test to have the correct description on the `Describe`-block for the test of `Set-TargetResource`. +- SqlServerRole + - Add support for nested role membership ([issue #1452](https://github.com/dsccommunity/SqlServerDsc/issues/1452)) + - Removed use of case-sensitive Contains() function when evalutating role membership. + ([issue #1153](https://github.com/dsccommunity/SqlServerDsc/issues/1153)) + - Refactored mocks and unit tests to increase performance. ([issue #979](https://github.com/dsccommunity/SqlServerDsc/issues/979)) ### Fixed diff --git a/source/DSCResources/MSFT_SqlServerRole/MSFT_SqlServerRole.psm1 b/source/DSCResources/MSFT_SqlServerRole/MSFT_SqlServerRole.psm1 index f51f1aaaf..3c6f27a78 100644 --- a/source/DSCResources/MSFT_SqlServerRole/MSFT_SqlServerRole.psm1 +++ b/source/DSCResources/MSFT_SqlServerRole/MSFT_SqlServerRole.psm1 @@ -110,7 +110,7 @@ function Get-TargetResource { foreach ($memberToInclude in $MembersToInclude) { - if ( -not ($membersInRole.Contains($memberToInclude))) + if ($membersInRole -notcontains $memberToInclude) { Write-Verbose -Message ( $script:localizedData.MemberNotPresent ` @@ -126,7 +126,7 @@ function Get-TargetResource { foreach ($memberToExclude in $MembersToExclude) { - if ($membersInRole.Contains($memberToExclude)) + if ($membersInRole -contains $memberToExclude) { Write-Verbose -Message ( $script:localizedData.MemberPresent ` @@ -295,20 +295,20 @@ function Set-TargetResource foreach ($memberName in $memberNamesInRoleObject) { - if ( -not ($Members.Contains($memberName))) + if ($Members -notcontains $memberName) { Remove-SqlDscServerRoleMember -SqlServerObject $sqlServerObject ` - -LoginName $memberName ` + -SecurityPrincipal $memberName ` -ServerRoleName $ServerRoleName } } foreach ($memberToAdd in $Members) { - if ( -not ($memberNamesInRoleObject.Contains($memberToAdd))) + if ($memberNamesInRoleObject -notcontains $memberToAdd) { Add-SqlDscServerRoleMember -SqlServerObject $sqlServerObject ` - -LoginName $memberToAdd ` + -SecurityPrincipal $memberToAdd ` -ServerRoleName $ServerRoleName } } @@ -321,10 +321,10 @@ function Set-TargetResource foreach ($memberToInclude in $MembersToInclude) { - if ( -not ($memberNamesInRoleObject.Contains($memberToInclude))) + if ($memberNamesInRoleObject -notcontains $memberToInclude) { Add-SqlDscServerRoleMember -SqlServerObject $sqlServerObject ` - -LoginName $memberToInclude ` + -SecurityPrincipal $memberToInclude ` -ServerRoleName $ServerRoleName } } @@ -336,10 +336,10 @@ function Set-TargetResource foreach ($memberToExclude in $MembersToExclude) { - if ($memberNamesInRoleObject.Contains($memberToExclude)) + if ($memberNamesInRoleObject -contains $memberToExclude) { Remove-SqlDscServerRoleMember -SqlServerObject $sqlServerObject ` - -LoginName $memberToExclude ` + -SecurityPrincipal $memberToExclude ` -ServerRoleName $ServerRoleName } } @@ -471,7 +471,7 @@ function Test-TargetResource .PARAMETER SqlServerObject An object returned from Connect-SQL function. - .PARAMETER LoginName + .PARAMETER SecurityPrincipal String containing the login (user) which should be added as a member to the server role. .PARAMETER ServerRoleName @@ -490,7 +490,7 @@ function Add-SqlDscServerRoleMember [Parameter(Mandatory = $true)] [ValidateNotNullOrEmpty()] [System.String] - $LoginName, + $SecurityPrincipal, [Parameter(Mandatory = $true)] [ValidateNotNullOrEmpty()] @@ -498,27 +498,21 @@ function Add-SqlDscServerRoleMember $ServerRoleName ) - if ( -not ($SqlServerObject.Logins[$LoginName]) ) - { - $errorMessage = $script:localizedData.LoginNotFound ` - -f $LoginName, $ServerName, $InstanceName - - New-ObjectNotFoundException -Message $errorMessage - } - try { + Test-SqlSecurityPrincipal -SqlServerObject $SqlServerObject -SecurityPrincipal $SecurityPrincipal + Write-Verbose -Message ( $script:localizedData.AddMemberToRole ` - -f $LoginName, $ServerRoleName + -f $SecurityPrincipal, $ServerRoleName ) - $SqlServerObject.Roles[$ServerRoleName].AddMember($LoginName) + $SqlServerObject.Roles[$ServerRoleName].AddMember($SecurityPrincipal) } catch { $errorMessage = $script:localizedData.AddMemberServerRoleSetError ` - -f $ServerName, $InstanceName, $ServerRoleName, $LoginName + -f $ServerName, $InstanceName, $ServerRoleName, $SecurityPrincipal New-InvalidOperationException -Message $errorMessage -ErrorRecord $_ } @@ -531,7 +525,7 @@ function Add-SqlDscServerRoleMember .PARAMETER SqlServerObject An object returned from Connect-SQL function. - .PARAMETER LoginName + .PARAMETER SecurityPrincipal String containing the login (user) which should be removed as a member in the server role. .PARAMETER ServerRoleName @@ -550,7 +544,7 @@ function Remove-SqlDscServerRoleMember [Parameter(Mandatory = $true)] [ValidateNotNullOrEmpty()] [System.String] - $LoginName, + $SecurityPrincipal, [Parameter(Mandatory = $true)] [ValidateNotNullOrEmpty()] @@ -558,30 +552,69 @@ function Remove-SqlDscServerRoleMember $ServerRoleName ) - if ( -not ($SqlServerObject.Logins[$LoginName]) ) - { - $errorMessage = $script:localizedData.LoginNotFound ` - -f $LoginName, $ServerName, $InstanceName - - New-ObjectNotFoundException -Message $errorMessage - } - try { + # Determine whether a valid principal has been supplied + Test-SqlSecurityPrincipal -SqlServerObject $SqlServerObject -SecurityPrincipal $SecurityPrincipal + Write-Verbose -Message ( $script:localizedData.RemoveMemberFromRole ` - -f $LoginName, $ServerRoleName + -f $SecurityPrincipal, $ServerRoleName ) - $SqlServerObject.Roles[$ServerRoleName].DropMember($LoginName) + $SqlServerObject.Roles[$ServerRoleName].DropMember($SecurityPrincipal) } catch { $errorMessage = $script:localizedData.DropMemberServerRoleSetError ` - -f $ServerName, $InstanceName, $ServerRoleName, $LoginName + -f $ServerName, $InstanceName, $ServerRoleName, $SecurityPrincipal New-InvalidOperationException -Message $errorMessage -ErrorRecord $_ } } +<# + .SYNOPSIS + Tests whether a security principal is valid on the specified SQL Server instance. + + .PARAMETER SqlServerObject + The object returned from the Connect-SQL function. + + .PARAMETER SecurityPrincipal + String containing the name of the principal to validate. +#> +function Test-SqlSecurityPrincipal +{ + [CmdletBinding()] + [OutputType([System.Boolean])] + param + ( + [Parameter(Mandatory = $true)] + [System.Object] + $SqlServerObject, + + [Parameter(Mandatory = $true)] + [System.String] + $SecurityPrincipal + ) + + if ($SqlServerObject.Logins.Name -notcontains $SecurityPrincipal) + { + if ($SqlServerObject.Roles.Name -notcontains $SecurityPrincipal) + { + $errorMessage = $script:localizedData.SecurityPrincipalNotFound -f ( + $SecurityPrincipal, + $($SqlServerObject.Name) + ) + + # Principal is neither a Login nor a Server role, raise exception + New-ObjectNotFoundException -Message $errorMessage + + return $false + } + } + + return $true +} + Export-ModuleMember -Function *-TargetResource diff --git a/source/DSCResources/MSFT_SqlServerRole/en-US/MSFT_SqlServerRole.strings.psd1 b/source/DSCResources/MSFT_SqlServerRole/en-US/MSFT_SqlServerRole.strings.psd1 index f1d304e22..7b55c13ee 100644 --- a/source/DSCResources/MSFT_SqlServerRole/en-US/MSFT_SqlServerRole.strings.psd1 +++ b/source/DSCResources/MSFT_SqlServerRole/en-US/MSFT_SqlServerRole.strings.psd1 @@ -17,7 +17,7 @@ ConvertFrom-StringData @' CreateRole = Creating the SQL Server role '{0}'. EnsureIsAbsent = Ensure is set to Absent. The existing role '{0}' should be removed. EnsureIsPresent = Ensure is set to Present. Either the role '{0}' is missing and should be created, or members in the role is not in desired state. - LoginNotFound = Login '{0}' does not exist on SQL server '{1}\\{2}'. + SecurityPrincipalNotFound = Security principal '{0}' does not exist on SQL server '{1}'. AddMemberToRole = Adding login '{0}' to role '{1}'. RemoveMemberFromRole = Removing login '{0}' from role '{1}'. '@ diff --git a/tests/Integration/MSFT_SqlServerRole.Integration.Tests.ps1 b/tests/Integration/MSFT_SqlServerRole.Integration.Tests.ps1 index e667531da..971285eb6 100644 --- a/tests/Integration/MSFT_SqlServerRole.Integration.Tests.ps1 +++ b/tests/Integration/MSFT_SqlServerRole.Integration.Tests.ps1 @@ -407,6 +407,114 @@ try Test-DscConfiguration -Verbose | Should -Be 'True' } } + + $configurationName = "$($script:dscResourceName)_AddNestedRole_Config" + + Context ('When using configuration {0}' -f $configurationName) { + It 'Should compile and apply the MOF without throwing an exception' { + $configurationParameters = @{ + OutputPath = $TestDrive + ConfigurationData = $ConfigurationData + } + + { & $configurationName @configurationParameters } | Should -Not -Throw + } + + It 'Should apply the configuration successfully' { + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw + } + + It 'Should be able to call Get-DscConfiguration without throwing an exception' { + { $script:currentConfiguration = Get-DscConfiguration -Verbose -ErrorAction Stop } | + Should -Not -Throw + } + + It "Should have set the resource and all values should match for $($ConfigurationData.AllNodes.Role4Name)." { + $testRoleName = $ConfigurationData.AllNodes.Role4Name + + # Extract just the roles we want from the Configuration + $currentState = $script:currentConfiguration | Where-Object -FilterScript { + $_.ConfigurationName -eq $configurationName -and + $_.ServerRoleName -eq $testRoleName + } + + $currentState.Ensure | Should -Be 'Present' + $currentState.Members | Should -BeNullOrEmpty + $currentState.MembersToInclude | Should -BeNullOrEmpty + $currentState.MembersToExclude | Should -BeNullOrEmpty + } + + It "Should have set the resource and all values should match for $($ConfigurationData.AllNodes.Role5Name)." { + $testRoleName = $ConfigurationData.AllNodes.Role5Name + $testMemberName = $ConfigurationData.AllNodes.Role4Name + + # Extract just the roles we want from the Configuration + $currentstate = $script:currentConfiguration | Where-Object -FilterScript { + $_.ConfigurationName -eq $configurationName -and + $_.ServerRoleName -eq $testRoleName + } + + $currentState.Ensure | Should -Be 'Present' + $currentState.Members | Should -Be @($testMemberName) + $currentState.MembersToInclude | Should -Be @($testMemberName) + $currentState.MembersToExclude | Should -BeNullOrEmpty + } + } + + $configurationName = "$($script:dscResourceName)_RemoveNestedRole_Config" + + Context ('When using configuration {0}' -f $configurationName) { + It 'Should compile and apply the MOF without throwing an exception' { + $configurationParameters = @{ + OutputPath = $TestDrive + ConfigurationData = $ConfigurationData + } + + { & $configurationName @configurationParameters } | Should -Not -Throw + } + + It 'Should apply the configuration successfully' { + $startDscConfigurationParameters = @{ + Path = $TestDrive + ComputerName = 'localhost' + Wait = $true + Verbose = $true + Force = $true + ErrorAction = 'Stop' + } + + { Start-DscConfiguration @startDscConfigurationParameters } | Should -Not -Throw + } + + It 'Should be able to call Get-DscConfiguration without throwing an exception' { + { $script:currentConfiguration = Get-DscConfiguration -Verbose -ErrorAction Stop } | + Should -Not -Throw + } + + It "Should have set the resource and all values should match for $($ConfigurationData.AllNodes.Role5Name)." { + $testRoleName = $ConfigurationData.AllNodes.Role5Name + $testMemberName = $ConfigurationData.AllNodes.Role4Name + + $currentState = $script:currentConfiguration | Where-Object -FilterScript { + $_.ConfigurationName -eq $configurationName -and + $_.ServerRoleName -eq $testRoleName + } + + $currentState.Ensure | Should -Be 'Present' + $currentstate.Members | Should -BeNullOrEmpty + $currentState.MembersToInclude | Should -BeNullOrEmpty + $currentState.MembersToExclude | Should -Be $testMemberName + } + } } } finally diff --git a/tests/Integration/MSFT_SqlServerRole.config.ps1 b/tests/Integration/MSFT_SqlServerRole.config.ps1 index 113a06d7a..7199b45df 100644 --- a/tests/Integration/MSFT_SqlServerRole.config.ps1 +++ b/tests/Integration/MSFT_SqlServerRole.config.ps1 @@ -27,6 +27,8 @@ else Role1Name = 'DscServerRole1' Role2Name = 'DscServerRole2' Role3Name = 'DscServerRole3' + Role4Name = 'DscServerRole4' + Role5Name = 'DscServerRole5' User1Name = '{0}\{1}' -f $env:COMPUTERNAME, 'DscUser1' User2Name = '{0}\{1}' -f $env:COMPUTERNAME, 'DscUser2' @@ -228,3 +230,78 @@ Configuration MSFT_SqlServerRole_RemoveRole3_Config } } } + +<# + .SYNOPSIS + Adds a custom server role to an existing role +#> +Configuration MSFT_SqlServerRole_AddNestedRole_Config +{ + Import-DscResource -ModuleName 'SqlServerDsc' + + Node $AllNodes.NodeName + { + SqlServerRole $Node.Role4Name + { + Ensure = 'Present' + ServerRoleName = $Node.Role4Name + ServerName = $Node.ServerName + InstanceName = $Node.InstanceName + + PsDscRunAsCredential = New-Object ` + -TypeName System.Management.Automation.PSCredential ` + -ArgumentList @( + $Node.Username, + (ConvertTo-SecureString -String $Node.Password -AsPlainText -Force) + ) + } + + SqlServerRole $Node.Role5Name + { + Ensure = 'Present' + ServerRoleName = $Node.Role5Name + ServerName = $Node.ServerName + InstanceName = $Node.InstanceName + + MembersToInclude = $Node.Role4Name + + DependsOn = "[SqlServerRole]$($Node.Role4Name)" + + PsDscRunAsCredential = New-Object ` + -TypeName System.Management.Automation.PSCredential ` + -ArgumentList @( + $Node.Username, + (ConvertTo-SecureString -String $Node.Password -AsPlainText -Force) + ) + } + } +} + +<# + .SYNOPSIS + Removes a custom server role to an existing role +#> +Configuration MSFT_SqlServerRole_RemoveNestedRole_Config +{ + Import-DscResource -ModuleName 'SqlServerDsc' + + Node $AllNodes.NodeName + { + SqlServerRole $Node.Role5Name + { + Ensure = 'Present' + ServerRoleName = $Node.Role5Name + ServerName = $Node.ServerName + InstanceName = $Node.InstanceName + + MembersToExclude = $Node.Role4Name + + PsDscRunAsCredential = New-Object ` + -TypeName System.Management.Automation.PSCredential ` + -ArgumentList @( + $Node.Username, + (ConvertTo-SecureString -String $Node.Password -AsPlainText -Force) + ) + } + } +} diff --git a/tests/Unit/MSFT_SqlServerRole.Tests.ps1 b/tests/Unit/MSFT_SqlServerRole.Tests.ps1 index d0a2c0063..3cab78f67 100644 --- a/tests/Unit/MSFT_SqlServerRole.Tests.ps1 +++ b/tests/Unit/MSFT_SqlServerRole.Tests.ps1 @@ -49,13 +49,25 @@ try $mockServerName = 'localhost' $mockInstanceName = 'MSSQLSERVER' $mockSqlServerRole = 'AdminSqlforBI' + $mockSqlServerChildRole = 'TestChildRole' $mockSqlServerLoginOne = 'CONTOSO\John' $mockSqlServerLoginTwo = 'CONTOSO\Kelly' - $mockSqlServerLoginTree = 'CONTOSO\Lucy' + $mockSqlServerLoginThree = 'CONTOSO\Lucy' $mockSqlServerLoginFour = 'CONTOSO\Steve' - $mockEnumMemberNames = @($mockSqlServerLoginOne, $mockSqlServerLoginTwo) + $mockEnumMemberNames = @( + $mockSqlServerLoginOne, + $mockSqlServerLoginTwo + ) + $mockSecurityPrincipals = @( + $mockSqlServerLoginOne + $mockSqlServerLoginTwo + $mockSqlServerLoginThree + $mockSqlServerLoginFour + $mockSqlServerChildRole + ) $mockSqlServerLoginType = 'WindowsUser' $mockExpectedServerRoleToDrop = 'ServerRoleToDrop' + $mockPrincipalsAsArrays = $false # Default parameters that are used for the It-blocks $mockDefaultParameters = @{ @@ -66,112 +78,156 @@ try #region Function mocks $mockConnectSQL = { - return @( - ( - New-Object -TypeName Object | - Add-Member -MemberType NoteProperty -Name InstanceName -Value $mockInstanceName -PassThru | - Add-Member -MemberType NoteProperty -Name ComputerNamePhysicalNetBIOS -Value $mockServerName -PassThru | - Add-Member -MemberType ScriptProperty -Name Roles -Value { - return @{ - $mockSqlServerRole = ( New-Object -TypeName Object | - Add-Member -MemberType NoteProperty -Name Name -Value $mockSqlServerRole -PassThru | - Add-Member -MemberType ScriptMethod -Name EnumMemberNames -Value { - if ($mockInvalidOperationForEnumMethod) - { - throw 'Mock EnumMemberNames Method was called with invalid operation.' - } - else - { - $mockEnumMemberNames - } - } -PassThru | - Add-Member -MemberType ScriptMethod -Name Drop -Value { - if ($mockInvalidOperationForDropMethod) - { - throw 'Mock Drop Method was called with invalid operation.' - } - - if ( $this.Name -ne $mockExpectedServerRoleToDrop ) - { - throw "Called mocked drop() method without dropping the right server role. Expected '{0}'. But was '{1}'." ` - -f $mockExpectedServerRoleToDrop, $this.Name - } - } -PassThru | - Add-Member -MemberType ScriptMethod -Name AddMember -Value { - if ($mockInvalidOperationForAddMemberMethod) - { - throw 'Mock AddMember Method was called with invalid operation.' - } - - if ( $mockSqlServerLoginToAdd -ne $mockExpectedMemberToAdd ) - { - throw "Called mocked AddMember() method without adding the right login. Expected '{0}'. But was '{1}'." ` - -f $mockExpectedMemberToAdd, $mockSqlServerLoginToAdd - } - } -PassThru | - Add-Member -MemberType ScriptMethod -Name DropMember -Value { - if ($mockInvalidOperationForDropMemberMethod) - { - throw 'Mock DropMember Method was called with invalid operation.' - } - - if ( $mockSqlServerLoginToDrop -ne $mockExpectedMemberToDrop ) - { - throw "Called mocked DropMember() method without removing the right login. Expected '{0}'. But was '{1}'." ` - -f $mockExpectedMemberToDrop, $mockSqlServerLoginToDrop - } - } -PassThru - ) - } - } -PassThru | - Add-Member -MemberType ScriptProperty -Name Logins -Value { - return @{ - $mockSqlServerLoginOne = @(( - New-Object -TypeName Object | - Add-Member -MemberType NoteProperty -Name LoginType -Value $mockSqlServerLoginType -PassThru - )) - $mockSqlServerLoginTwo = @(( - New-Object -TypeName Object | - Add-Member -MemberType NoteProperty -Name LoginType -Value $mockSqlServerLoginType -PassThru - )) - $mockSqlServerLoginTree = @(( - New-Object -TypeName Object | - Add-Member -MemberType NoteProperty -Name LoginType -Value $mockSqlServerLoginType -PassThru - )) - $mockSqlServerLoginFour = @(( - New-Object -TypeName Object | - Add-Member -MemberType NoteProperty -Name LoginType -Value $mockSqlServerLoginType -PassThru - )) - } - } -PassThru -Force - ) - ) + $mockServerObjectHashtable = @{ + InstanceName = $mockInstanceName + ComputerNamePhysicalNetBIOS = $mockServerName + Name = "$mockServerName\$mockInstanceName" + } + + if ($mockPrincipalsAsArrays) + { + $mockServerObjectHashtable += @{ + Logins = @() + Roles = @() + } + } + else + { + $mockServerObjectHashtable += @{ + Logins = @{} + Roles = @{} + } + } + + $mockServerObject = [PSCustomObject]$mockServerObjectHashtable + + # Add the roles to the mock Server objet + foreach ($mockRole in $($mockSqlServerRole, $mockSqlServerChildRole)) + { + # Create the role objet + $mockRoleObject = [PSCustomObject]@{ + Name = $mockRole + } + + # Add mocked methods + $mockRoleObject | Add-Member -MemberType ScriptMethod -Name EnumMemberNames -Value { + if ($mockInvalidOperationForEnumMethod) + { + throw 'Mock EnumMemberNames Method was called with invalid operation.' + } + else + { + $mockEnumMemberNames + } + } -PassThru | + Add-Member -MemberType ScriptMethod -Name Drop -Value { + if ($mockInvalidOperationForDropMethod) + { + throw 'Mock Drop Method was called with invalid operation.' + } + + if ( $this.Name -ne $mockExpectedServerRoleToDrop ) + { + throw "Called mocked drop() method without dropping the right server role. Expected '{0}'. But was '{1}'." ` + -f $mockExpectedServerRoleToDrop, $this.Name + } + } -PassThru | + Add-Member -MemberType ScriptMethod -Name AddMember -Value { + param + ( + [Parameter(Mandatory = $true)] + [String] + $memberName + ) + + if ($mockInvalidOperationForAddMemberMethod) + { + throw 'Mock AddMember Method was called with invalid operation.' + } + + if ($mockExpectedMemberToAdd -ne $memberName) + { + throw "Called mocked AddMember() method without adding the right login. Expected '{0}'. But was '{1}'." ` + -f $mockExpectedMemberToAdd, $memberName + } + } -PassThru | + Add-Member -MemberType ScriptMethod -Name DropMember -Value { + param + ( + [Parameter(Mandatory = $true)] + [String] + $memberName + ) + + if ($mockInvalidOperationForDropMemberMethod) + { + throw 'Mock DropMember Method was called with invalid operation.' + } + + if ($mockExpectedMemberToDrop -ne $memberName) + { + throw "Called mocked DropMember() method without removing the right login. Expected '{0}'. But was '{1}'." ` + -f $mockExpectedMemberToDrop, $memberName + } + } + + # Add the mock role to the roles collection + if ($mockServerObject.Roles -is [array]) + { + $mockServerObject.Roles += $mockRoleObject + } + else + { + $mockServerObject.Roles.Add($mockRole, $mockRoleObject) + } + } + + # Add all mock logins + foreach ($mockLoginName in @($mockSqlServerLoginOne, $mockSqlServerLoginTwo, $mockSqlServerLoginThree, $mockSqlServerLoginFour)) + { + $mockLoginObject = [PSCustomObject]@{ + Name = $mockLoginName + LoginType = $mockSqlServerLoginType + } + + if ($mockServerObject.Logins -is [array]) + { + $mockServerObject.Logins += $mockLoginObject + } + else + { + $mockServerObject.Logins.Add($mockLoginName, $mockLoginObject) + } + } + + return @($mockServerObject) } $mockNewObjectServerRole = { - return @( - ( - New-Object -TypeName Object | - Add-Member -MemberType NoteProperty -Name Name -Value $mockSqlServerRoleAdd -PassThru | - Add-Member -MemberType ScriptMethod -Name Create -Value { - if ($mockInvalidOperationForCreateMethod) - { - throw 'Mock Create Method was called with invalid operation.' - } + $mockObject = [PSCustomObject] @{ + Name = $mockSqlServerRoleAdd + } - if ( $this.Name -ne $mockExpectedServerRoleToCreate ) - { - throw "Called mocked Create() method without adding the right server role. Expected '{0}'. But was '{1}'." ` - -f $mockExpectedServerRoleToCreate, $this.Name - } - } -PassThru -Force - ) - ) + $mockObject | Add-Member -MemberType ScriptMethod -Name Create -Value { + if ($mockInvalidOperationForCreateMethod) + { + throw 'Mock Create Method was called with invalid operation.' + } + + if ( $this.Name -ne $mockExpectedServerRoleToCreate ) + { + throw "Called mocked Create() method without adding the right server role. Expected '{0}'. But was '{1}'." ` + -f $mockExpectedServerRoleToCreate, $this.Name + } + } + + return @($mockObject) } + #endregion Describe "MSFT_SqlServerRole\Get-TargetResource" -Tag 'Get' { - BeforeEach { + BeforeAll { Mock -CommandName Connect-SQL -MockWith $mockConnectSQL -Verifiable } @@ -345,7 +401,7 @@ try $testParameters = $mockDefaultParameters $testParameters += @{ ServerRoleName = $mockSqlServerRole - MembersToExclude = $mockSqlServerLoginTree + MembersToExclude = $mockSqlServerLoginThree } It 'Should return the state as present when the members does not exist' { @@ -372,7 +428,7 @@ try $testParameters += @{ ServerRoleName = $mockSqlServerRole Members = $mockEnumMemberNames - MembersToInclude = $mockSqlServerLoginTree + MembersToInclude = $mockSqlServerLoginThree } $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull @@ -392,7 +448,7 @@ try $testParameters += @{ ServerRoleName = $mockSqlServerRole Members = $mockEnumMemberNames - MembersToExclude = $mockSqlServerLoginTree + MembersToExclude = $mockSqlServerLoginThree } $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull @@ -439,7 +495,7 @@ try $testParameters = $mockDefaultParameters $testParameters += @{ ServerRoleName = $mockSqlServerRole - Members = @($mockSqlServerLoginOne, $mockSqlServerLoginTree) + Members = @($mockSqlServerLoginOne, $mockSqlServerLoginThree) } It 'Should return the state as absent when the members in the role are wrong' { @@ -478,7 +534,7 @@ try $testParameters = $mockDefaultParameters $testParameters += @{ ServerRoleName = $mockSqlServerRole - MembersToInclude = $mockSqlServerLoginTree + MembersToInclude = $mockSqlServerLoginThree } It 'Should return the state as absent when the members in the role are missing' { @@ -539,11 +595,43 @@ try } } + Context 'When evaluating role membership, case sensitivity should not be used. (Issue #1153)' { + It 'Should return Present when the MemberToInclude is a member of the role.' { + $testParameters = $mockDefaultParameters.Clone() + $testParameters += @{ + ServerRoleName = $mockSqlServerRole + MembersToInclude = $mockSqlServerLoginOne.ToUpper() + } + + $result = Get-TargetResource @testParameters + + $result.Ensure | Should -Be 'Present' + $result.Members | Should -Contain $mockSqlServerLoginOne + + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + } + + It 'Should return Absent when the MembersToExclude is a member of the role.' { + $testParameters = $mockDefaultParameters.Clone() + $testParameters += @{ + ServerRoleName = $mockSqlServerRole + MembersToExclude = $mockSqlServerLoginOne.ToUpper() + } + + $result = Get-TargetResource @testParameters + + $result.Ensure | Should -Be 'Absent' + $result.Members | Should -Contain $mockSqlServerLoginOne + + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + } + } + Assert-VerifiableMock } Describe "MSFT_SqlServerRole\Test-TargetResource" -Tag 'Test' { - BeforeEach { + BeforeAll { Mock -CommandName Connect-SQL -MockWith $mockConnectSQL -Verifiable } @@ -598,7 +686,7 @@ try $testParameters += @{ Ensure = 'Present' ServerRoleName = $mockSqlServerRole - Members = @($mockSqlServerLoginTree, $mockSqlServerLoginFour) + Members = @($mockSqlServerLoginThree, $mockSqlServerLoginFour) } $result = Test-TargetResource @testParameters @@ -615,7 +703,7 @@ try Ensure = 'Present' ServerRoleName = $mockSqlServerRole Members = $mockEnumMemberNames - MembersToInclude = $mockSqlServerLoginTree + MembersToInclude = $mockSqlServerLoginThree } $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull @@ -648,7 +736,7 @@ try $testParameters += @{ Ensure = 'Present' ServerRoleName = $mockSqlServerRole - MembersToInclude = $mockSqlServerLoginTree + MembersToInclude = $mockSqlServerLoginThree } $result = Test-TargetResource @testParameters @@ -682,7 +770,7 @@ try $testParameters += @{ Ensure = 'Present' ServerRoleName = $mockSqlServerRole - MembersToExclude = $mockSqlServerLoginTree + MembersToExclude = $mockSqlServerLoginThree } $result = Test-TargetResource @testParameters @@ -712,11 +800,14 @@ try } Describe "MSFT_SqlServerRole\Set-TargetResource" -Tag 'Set' { - BeforeEach { + BeforeAll { Mock -CommandName Connect-SQL -MockWith $mockConnectSQL -Verifiable Mock -CommandName New-Object -MockWith $mockNewObjectServerRole -ParameterFilter { $TypeName -eq 'Microsoft.SqlServer.Management.Smo.ServerRole' } + Mock -CommandName Test-SqlSecurityPrincipal -MockWith { + return ($mockSecurityPrincipals -contains $SecurityPrincipal) + } } Context 'When the system is not in the desired state and ensure is set to Absent' { @@ -808,7 +899,7 @@ try Ensure = 'Present' ServerRoleName = $mockSqlServerRole Members = $mockEnumMemberNames - MembersToInclude = $mockSqlServerLoginTree + MembersToInclude = $mockSqlServerLoginThree } $errorMessage = $script:localizedData.MembersToIncludeAndExcludeParamMustBeNull @@ -839,13 +930,12 @@ try Context 'When parameter MembersToInclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { It 'Should not thrown when calling the AddMember method' { - $mockExpectedMemberToAdd = $mockSqlServerLoginTree - $mockSqlServerLoginToAdd = $mockSqlServerLoginTree + $mockExpectedMemberToAdd = $mockSqlServerLoginThree $testParameters = $mockDefaultParameters $testParameters += @{ Ensure = 'Present' ServerRoleName = $mockSqlServerRole - MembersToInclude = $mockSqlServerLoginTree + MembersToInclude = $mockSqlServerLoginThree } { Set-TargetResource @testParameters } | Should -Not -Throw @@ -857,17 +947,16 @@ try Context 'When parameter MembersToInclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { It 'Should throw the correct error when calling the AddMember method' { $mockInvalidOperationForAddMemberMethod = $true - $mockExpectedMemberToAdd = $mockSqlServerLoginTree - $mockSqlServerLoginToAdd = $mockSqlServerLoginTree + $mockExpectedMemberToAdd = $mockSqlServerLoginThree $testParameters = $mockDefaultParameters $testParameters += @{ Ensure = 'Present' ServerRoleName = $mockSqlServerRole - MembersToInclude = $mockSqlServerLoginTree + MembersToInclude = $mockSqlServerLoginThree } $errorMessage = $script:localizedData.AddMemberServerRoleSetError ` - -f $mockServerName, $mockInstanceName, $mockSqlServerRole, $mockSqlServerLoginTree + -f $mockServerName, $mockInstanceName, $mockSqlServerRole, $mockSqlServerLoginThree { Set-TargetResource @testParameters } | Should -Throw $errorMessage @@ -877,8 +966,7 @@ try Context 'When parameter MembersToInclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { It 'Should throw the correct error when login does not exist' { - $mockExpectedMemberToAdd = $mockSqlServerLoginTree - $mockSqlServerLoginToAdd = $mockSqlServerLoginTree + $mockExpectedMemberToAdd = $mockSqlServerLoginThree $testParameters = $mockDefaultParameters $testParameters += @{ Ensure = 'Present' @@ -886,8 +974,12 @@ try MembersToInclude = 'KingJulian' } - $errorMessage = $script:localizedData.LoginNotFound ` - -f 'KingJulian', $mockServerName, $mockInstanceName + $errorMessage = $script:localizedData.AddMemberServerRoleSetError -f ( + $mockServerName, + $mockInstanceName, + $mockSqlServerRole, + 'KingJulian' + ) { Set-TargetResource @testParameters } | Should -Throw $errorMessage @@ -897,8 +989,8 @@ try Context 'When parameter MembersToExclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { It 'Should not throw when calling the DropMember method' { - $mockExpectedMemberToAdd = $mockSqlServerLoginTwo - $mockSqlServerLoginToAdd = $mockSqlServerLoginTwo + $mockExpectedMemberToDrop = $mockSqlServerLoginTwo + $testParameters = $mockDefaultParameters $testParameters += @{ Ensure = 'Present' @@ -916,7 +1008,6 @@ try It 'Should throw the correct error when calling the DropMember method' { $mockInvalidOperationForDropMemberMethod = $true $mockExpectedMemberToDrop = $mockSqlServerLoginTwo - $mockSqlServerLoginToDrop = $mockSqlServerLoginTwo $testParameters = $mockDefaultParameters $testParameters += @{ Ensure = 'Present' @@ -936,8 +1027,7 @@ try Context 'When parameter MembersToExclude is assigned a value, parameter Members is not assigned a value, and ensure is set to Present' { It 'Should throw the correct error when login does not exist' { $mockEnumMemberNames = @('KingJulian', $mockSqlServerLoginOne, $mockSqlServerLoginTwo) - $mockExpectedMemberToAdd = $mockSqlServerLoginTree - $mockSqlServerLoginToAdd = $mockSqlServerLoginTree + $mockExpectedMemberToAdd = $mockSqlServerLoginThree $testParameters = $mockDefaultParameters $testParameters += @{ Ensure = 'Present' @@ -945,8 +1035,12 @@ try MembersToExclude = 'KingJulian' } - $errorMessage = $script:localizedData.LoginNotFound ` - -f 'KingJulian', $mockServerName, $mockInstanceName + $errorMessage = $script:localizedData.DropMemberServerRoleSetError -f ( + $mockServerName, + $mockInstanceName, + $mockSqlServerRole, + 'KingJulian' + ) { Set-TargetResource @testParameters } | Should -Throw $errorMessage @@ -956,17 +1050,21 @@ try Context 'When parameter Members is assigned a value and ensure is set to Present' { It 'Should throw the correct error when login does not exist' { - $mockExpectedMemberToAdd = $mockSqlServerLoginTree - $mockSqlServerLoginToAdd = $mockSqlServerLoginTree + $mockExpectedMemberToDrop = $mockSqlServerLoginTwo + $testParameters = $mockDefaultParameters $testParameters += @{ Ensure = 'Present' ServerRoleName = $mockSqlServerRole - Members = @('KingJulian', $mockSqlServerLoginOne, $mockSqlServerLoginTree) + Members = @('KingJulian', $mockSqlServerLoginOne, $mockSqlServerLoginThree) } - $errorMessage = $script:localizedData.LoginNotFound ` - -f 'KingJulian', $mockServerName, $mockInstanceName + $errorMessage = $script:localizedData.AddMemberServerRoleSetError -f ( + $mockServerName, + $mockInstanceName, + $mockSqlServerRole, + 'KingJulian' + ) { Set-TargetResource @testParameters } | Should -Throw $errorMessage @@ -976,15 +1074,13 @@ try Context 'When Members parameter is set and ensure parameter is set to Present' { It 'Should not throw when calling both the AddMember and DropMember methods' { - $mockExpectedMemberToAdd = $mockSqlServerLoginTree - $mockSqlServerLoginToAdd = $mockSqlServerLoginTree + $mockExpectedMemberToAdd = $mockSqlServerLoginThree $mockExpectedMemberToDrop = $mockSqlServerLoginTwo - $mockSqlServerLoginToDrop = $mockSqlServerLoginTwo $testParameters = $mockDefaultParameters $testParameters += @{ Ensure = 'Present' ServerRoleName = $mockSqlServerRole - Members = @($mockSqlServerLoginOne, $mockSqlServerLoginTree) + Members = @($mockSqlServerLoginOne, $mockSqlServerLoginThree) } { Set-TargetResource @testParameters } | Should -Not -Throw @@ -993,12 +1089,220 @@ try } } + Context 'When nesting role membership' { + Context 'When defining an explicit list of members.' { + It 'Should not throw when the member is a Role' { + $mockExpectedMemberToAdd = $mockSqlServerChildRole + $testParameters = $mockDefaultParameters.Clone() + + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + Members = @($mockSqlServerLoginOne, $mockSqlServerLoginTwo, $mockSqlServerChildRole) + } + + { Set-TargetResource @testParameters } | Should -Not -Throw + + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + } + } + + Context 'When specifying a list of security principals to include in the Role.' { + It 'Should not throw when a member to inculde is a Role.' { + $mockExpectedMemberToAdd = $mockSqlServerChildRole + $testParameters = $mockDefaultParameters.Clone() + + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + MembersToInclude = @($mockSqlServerChildRole) + } + + { Set-TargetResource @testParameters } | Should -Not -Throw + + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + } + } + + Context 'When specifying a list of security principals to remove from the Role.' { + It 'Should not throw when the member to exclude is a Role.' { + $mockExpectedMemberToDrop = $mockSqlServerChildRole + $testParameters = $mockDefaultParameters.Clone() + + $testParameters += @{ + Ensure = 'Present' + ServerRoleName = $mockSqlServerRole + MembersToExclude = @($mockSqlServerChildRole) + } + + { Set-TargetResource @testParameters } | Should -Not -Throw + + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + } + } + } + + Context 'When evaluating role membership, case sensitivity should not be used. (Issue #1153)' { + Context 'When specifying explicit role members.' { + It 'Should not attempt to remove an explicit member from the role.' { + $mockExpectedMemberToDrop = $mockSqlServerLoginTwo + + $testParameters = $mockDefaultParameters.Clone() + $testParameters += @{ + ServerRoleName = $mockSqlServerRole + Ensure = 'Present' + Members = $mockSqlServerLoginOne.ToUpper() + } + + { Set-TargetResource @testParameters } | Should -Not -Throw + + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + } + + It 'Should not attempt to add an explicit member that already exists in the role.' { + $mockExpectedMemberToAdd = '' + + $testParameters = $mockDefaultParameters.Clone() + $testParameters += @{ + ServerRoleName = $mockSqlServerRole + Ensure = 'Present' + Members = @($mockSqlServerLoginOne.ToUpper(), $mockSqlServerLoginTwo) + } + + { Set-TargetResource @testParameters } | Should -Not -Throw + + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + } + } + + Context 'When specifying mandatory role membership.' { + It 'Should not attempt to add a member that already exists in the role.' { + $mockExpectedMemberToAdd = '' + + $testParameters = $mockDefaultParameters.Clone() + $testParameters += @{ + ServerRoleName = $mockSqlServerRole + Ensure = 'Present' + MembersToInclude = @($mockSqlServerLoginOne.ToUpper()) + } + + { Set-TargetResource @testParameters } | Should -Not -Throw + + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + } + + It 'Should attempt to remove a member that is to be excluded.' { + $mockExpectedMemberToDrop = $mockSqlServerLoginOne + + $testParameters = $mockDefaultParameters.Clone() + $testParameters += @{ + ServerRoleName = $mockSqlServerRole + Ensure = 'Present' + MembersToExclude = @($mockSqlServerLoginOne.ToUpper()) + } + + { Set-TargetResource @testParameters } | Should -Not -Throw + + Assert-MockCalled -CommandName Connect-SQL -Exactly -Times 1 -Scope It + } + } + } + Assert-VerifiableMock } + + Describe 'MSFT_SqlServerRole\Test-SqlSecurityPrincipal' -Tag 'Helper' { + BeforeAll { + Mock -CommandName Connect-SQL -MockWith $mockConnectSQL -Verifiable + + $mockPrincipalsAsArrays = $true + $testSqlServerObject = Connect-SQL -ServerName $mockServerName -InstanceName $mockInstanceName + } + + Context 'When the security principal does not exist.' { + It 'Should throw the correct exception' { + $testSecurityPrincipal = 'Nabrond' + + $testParameters = @{ + SqlServerObject = $testSqlServerObject + SecurityPrincipal = $testSecurityPrincipal + } + + $testErrorMessage = $script:localizedData.SecurityPrincipalNotFound -f ( + $testSecurityPrincipal, + "$mockServerName\$mockInstanceName" + ) + + { Test-SqlSecurityPrincipal @testParameters } | Should -Throw -ExpectedMessage $testErrorMessage + } + + It 'Should return false when ErrorAction is set to SilentlyContinue' { + $testSecurityPrincipal = 'Nabrond' + + $testParameters = @{ + SqlServerObject = $testSqlServerObject + SecurityPrincipal = $testSecurityPrincipal + } + + <# + Pester will still see the error on the stack regardless of the value used for ErrorAction + Wrap this call in a try/catch to swallow the exception and capture the return value. + #> + try + { + $result = Test-SqlSecurityPrincipal @testParameters -ErrorAction SilentlyContinue + } + catch + { + continue; + } + + $result | Should -Be $false + } + } + + Context 'When the security principal exists.' { + It 'Should return true when the principal is a Login.' { + + $testParameters = @{ + SqlServerObject = $testSqlServerObject + SecurityPrincipal = $mockSqlServerLoginOne + } + + Test-SqlSecurityPrincipal @testParameters | Should -Be $true + } + + It 'Should return true when the principal is a Login and case does not match.' { + $testParameters = @{ + SqlServerObject = $testSqlServerObject + SecurityPrincipal = $mockSqlServerLoginOne.ToUpper() + } + + Test-SqlSecurityPrincipal @testParameters | Should -Be $true + } + + It 'Should return true when the principal is a Role.' { + $testParameters = @{ + SqlServerObject = $testSqlServerObject + SecurityPrincipal = $mockSqlServerRole + } + + Test-SqlSecurityPrincipal @testParameters | Should -Be $true + } + + It 'Should return true when the principal is a Role and case does not match.' { + $testParameters = @{ + SqlServerObject = $testSqlServerObject + SecurityPrincipal = $mockSqlServerRole.ToUpper() + } + + Test-SqlSecurityPrincipal @testParameters | Should -Be $true + } + } + } } } finally { Invoke-TestCleanup } -