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).
dsccommunity · Jan 12, 2020 · 620e9b4 · 620e9b4
1 parent 911ce6a
commit 620e9b4
Show file tree

Hide file tree

Showing 6 changed files with 704 additions and 177 deletions.
diff --git 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
@@ -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,35 +490,29 @@ function Add-SqlDscServerRoleMember
         [Parameter(Mandatory = $true)]
         [ValidateNotNullOrEmpty()]
         [System.String]
-        $LoginName,
+        $SecurityPrincipal,
 
         [Parameter(Mandatory = $true)]
         [ValidateNotNullOrEmpty()]
         [System.String]
         $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,38 +544,77 @@ function Remove-SqlDscServerRoleMember
         [Parameter(Mandatory = $true)]
         [ValidateNotNullOrEmpty()]
         [System.String]
-        $LoginName,
+        $SecurityPrincipal,
 
         [Parameter(Mandatory = $true)]
         [ValidateNotNullOrEmpty()]
         [System.String]
         $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
@@ -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
@@ -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