SqlDatabaseUser: Pass parameter ServerName to Assert-SqlLogin (#1681)

- SqlDatabaseUser - Added parameter `ServerName` to the call of `Assert-SqlLogin`. `@PSBoundParameters` doesn't capture the default value of `ServerName` when it is not explicitly set by the caller (issue #1647).
dsccommunity · Feb 1, 2021 · f7676b0 · f7676b0
1 parent db3c76f
commit f7676b0
Show file tree

Hide file tree

Showing 3 changed files with 141 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -71,6 +71,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Added/corrected `InstallSharedDir`, property output when using SQL Server 2019.
 - SqlTraceFlag
   - Fixed Assembly not loaded error ([issue #1680](https://github.com/dsccommunity/SqlServerDsc/issues/1680)).
+- SqlDatabaseUser
+  - Added parameter `ServerName` to the call of `Assert-SqlLogin`.
+    `@PSBoundParameters` doesn't capture the default value of `ServerName`
+    when it is not explicitly set by the caller ([issue #1647](https://github.com/dsccommunity/SqlServerDsc/issues/1647)).
 
 ## [15.0.1] - 2021-01-09
 

diff --git a/source/DSCResources/DSC_SqlDatabaseUser/DSC_SqlDatabaseUser.psm1 b/source/DSCResources/DSC_SqlDatabaseUser/DSC_SqlDatabaseUser.psm1
@@ -145,7 +145,7 @@ function Get-TargetResource
 #>
 function Set-TargetResource
 {
-    [Diagnostics.CodeAnalysis.SuppressMessageAttribute('SqlServerDsc.AnalyzerRules\Measure-CommandsNeededToLoadSMO', '', Justification='The command Connect-Sql is called when Get-TargetResource is called')]
+    [Diagnostics.CodeAnalysis.SuppressMessageAttribute('SqlServerDsc.AnalyzerRules\Measure-CommandsNeededToLoadSMO', '', Justification = 'The command Connect-Sql is called when Get-TargetResource is called')]
     [CmdletBinding()]
     param
     (
@@ -243,8 +243,14 @@ function Set-TargetResource
                                 $script:localizedData.ChangingLoginName -f $Name, $LoginName
                             )
 
+                            $assertSqlLoginParameters = @{
+                                ServerName   = $ServerName
+                                InstanceName = $InstanceName
+                                LoginName    = $LoginName
+                            }
+
                             # Assert that the login exist.
-                            Assert-SqlLogin @PSBoundParameters
+                            Assert-SqlLogin @assertSqlLoginParameters
 
                             try
                             {
@@ -359,8 +365,14 @@ function Set-TargetResource
             {
                 'Login'
                 {
+                    $assertSqlLoginParameters = @{
+                        ServerName   = $ServerName
+                        InstanceName = $InstanceName
+                        LoginName    = $LoginName
+                    }
+
                     # Assert that the login exist.
-                    Assert-SqlLogin @PSBoundParameters
+                    Assert-SqlLogin @assertSqlLoginParameters
 
                     Invoke-Query @invokeQueryParameters -Query (
                         'CREATE USER [{0}] FOR LOGIN [{1}];' -f $Name, $LoginName
@@ -444,7 +456,7 @@ function Set-TargetResource
 #>
 function Test-TargetResource
 {
-    [Diagnostics.CodeAnalysis.SuppressMessageAttribute('SqlServerDsc.AnalyzerRules\Measure-CommandsNeededToLoadSMO', '', Justification='The command Connect-Sql is called when Get-TargetResource is called')]
+    [Diagnostics.CodeAnalysis.SuppressMessageAttribute('SqlServerDsc.AnalyzerRules\Measure-CommandsNeededToLoadSMO', '', Justification = 'The command Connect-Sql is called when Get-TargetResource is called')]
     [CmdletBinding()]
     [OutputType([System.Boolean])]
     param
@@ -735,11 +747,7 @@ function Assert-SqlLogin
 
         [Parameter(Mandatory = $true)]
         [System.String]
-        $LoginName,
-
-        # Catch all other splatted parameters from $PSBoundParameters
-        [Parameter(ValueFromRemainingArguments = $true)]
-        $RemainingArguments
+        $LoginName
     )
 
     $sqlServerObject = Connect-SQL -ServerName $ServerName -InstanceName $InstanceName

diff --git a/tests/Unit/DSC_SqlDatabaseUser.Tests.ps1 b/tests/Unit/DSC_SqlDatabaseUser.Tests.ps1
@@ -523,19 +523,60 @@ try
                     }
 
                     Context 'When creating a database user with a login' {
-                        BeforeAll {
-                            $setTargetResourceParameters = $mockDefaultParameters.Clone()
-                            $setTargetResourceParameters['LoginName'] = $mockLoginName
-                            $setTargetResourceParameters['UserType'] = 'Login'
+                        Context 'When calling an using the default ServerName' {
+                            BeforeAll {
+                                $setTargetResourceParameters = $mockDefaultParameters.Clone()
+                                $setTargetResourceParameters['LoginName'] = $mockLoginName
+                                $setTargetResourceParameters['UserType'] = 'Login'
+
+                                <#
+                                    Make sure to use the default value for ServerName.
+                                    Regression test for issue #1647.
+                                #>
+                                $setTargetResourceParameters.Remove('ServerName')
+                            }
+
+                            It 'Should not throw and should call the correct mocks' {
+                                { Set-TargetResource @setTargetResourceParameters } | Should -Not -Throw
+
+                                Assert-MockCalled -CommandName Get-TargetResource -Exactly -Times 1 -Scope It
+                                Assert-MockCalled -CommandName Invoke-Query -ParameterFilter {
+                                    $Query -eq ('CREATE USER [{0}] FOR LOGIN [{1}];' -f $mockName, $mockLoginName)
+                                } -Exactly -Times 1 -Scope It
+
+                                Assert-MockCalled -CommandName Assert-SqlLogin -ParameterFilter {
+                                    $ServerName -eq (Get-ComputerName)
+                                } -Exactly -Times 1 -Scope It
+                            }
                         }
 
-                        It 'Should not throw and should call the correct mocks' {
-                            { Set-TargetResource @setTargetResourceParameters } | Should -Not -Throw
+                        Context 'When calling with an explicit ServerName' {
+                            BeforeAll {
+                                $expectedServerName = 'host.company.local'
 
-                            Assert-MockCalled -CommandName Get-TargetResource -Exactly -Times 1 -Scope It
-                            Assert-MockCalled -CommandName Invoke-Query -ParameterFilter {
-                                $Query -eq ('CREATE USER [{0}] FOR LOGIN [{1}];' -f $mockName, $mockLoginName)
-                            } -Exactly -Times 1 -Scope It
+                                $setTargetResourceParameters = $mockDefaultParameters.Clone()
+                                $setTargetResourceParameters['LoginName'] = $mockLoginName
+                                $setTargetResourceParameters['UserType'] = 'Login'
+
+                                <#
+                                    Set an explicit value for ServerName.
+                                    Regression test for issue #1647.
+                                #>
+                                $setTargetResourceParameters['ServerName'] = $expectedServerName
+                            }
+
+                            It 'Should not throw and should call the correct mocks' {
+                                { Set-TargetResource @setTargetResourceParameters } | Should -Not -Throw
+
+                                Assert-MockCalled -CommandName Get-TargetResource -Exactly -Times 1 -Scope It
+                                Assert-MockCalled -CommandName Invoke-Query -ParameterFilter {
+                                    $Query -eq ('CREATE USER [{0}] FOR LOGIN [{1}];' -f $mockName, $mockLoginName)
+                                } -Exactly -Times 1 -Scope It
+
+                                Assert-MockCalled -CommandName Assert-SqlLogin -ParameterFilter {
+                                    $ServerName -eq $expectedServerName
+                                } -Exactly -Times 1 -Scope It
+                            }
                         }
                     }
 
@@ -628,23 +669,70 @@ try
                                     LoginType          = 'WindowsUser'
                                 }
                             }
+                        }
 
-                            $setTargetResourceParameters = $mockDefaultParameters.Clone()
-                            $setTargetResourceParameters['LoginName'] = 'OtherLogin1'
-                            $setTargetResourceParameters['UserType'] = 'Login'
+                        Context 'When calling an using the default ServerName' {
+                            BeforeAll {
+                                $setTargetResourceParameters = $mockDefaultParameters.Clone()
+                                $setTargetResourceParameters['LoginName'] = 'OtherLogin1'
+                                $setTargetResourceParameters['UserType'] = 'Login'
+
+                                <#
+                                    Make sure to use the default value for ServerName.
+                                    Regression test for issue #1647.
+                                #>
+                                $setTargetResourceParameters.Remove('ServerName')
+                            }
+
+                            It 'Should not throw and should call the correct mocks' {
+                                { Set-TargetResource @setTargetResourceParameters } | Should -Not -Throw
+
+                                Assert-MockCalled -CommandName Get-TargetResource -Exactly -Times 1 -Scope It
+                                Assert-MockCalled -CommandName Invoke-Query -ParameterFilter {
+                                    $Query -eq ('ALTER USER [{0}] WITH NAME = [{1}], LOGIN = [{2}];' -f $mockName, $mockName, 'OtherLogin1')
+                                } -Exactly -Times 1 -Scope It
+
+                                Assert-MockCalled -CommandName Assert-SqlLogin -ParameterFilter {
+                                    $ServerName -eq (Get-ComputerName)
+                                } -Exactly -Times 1 -Scope It
+                            }
                         }
 
-                        It 'Should not throw and should call the correct mocks' {
-                            { Set-TargetResource @setTargetResourceParameters } | Should -Not -Throw
+                        Context 'When calling with an explicit ServerName' {
+                            BeforeAll {
+                                $expectedServerName = 'host.company.local'
 
-                            Assert-MockCalled -CommandName Get-TargetResource -Exactly -Times 1 -Scope It
-                            Assert-MockCalled -CommandName Invoke-Query -ParameterFilter {
-                                $Query -eq ('ALTER USER [{0}] WITH NAME = [{1}], LOGIN = [{2}];' -f $mockName, $mockName, 'OtherLogin1')
-                            } -Exactly -Times 1 -Scope It
+                                $setTargetResourceParameters = $mockDefaultParameters.Clone()
+                                $setTargetResourceParameters['LoginName'] = 'OtherLogin1'
+                                $setTargetResourceParameters['UserType'] = 'Login'
+
+                                <#
+                                    Set an explicit value for ServerName.
+                                    Regression test for issue #1647.
+                                #>
+                                $setTargetResourceParameters['ServerName'] = $expectedServerName
+                            }
+
+                            It 'Should not throw and should call the correct mocks' {
+                                { Set-TargetResource @setTargetResourceParameters } | Should -Not -Throw
+
+                                Assert-MockCalled -CommandName Get-TargetResource -Exactly -Times 1 -Scope It
+                                Assert-MockCalled -CommandName Invoke-Query -ParameterFilter {
+                                    $Query -eq ('ALTER USER [{0}] WITH NAME = [{1}], LOGIN = [{2}];' -f $mockName, $mockName, 'OtherLogin1')
+                                } -Exactly -Times 1 -Scope It
+
+                                Assert-MockCalled -CommandName Assert-SqlLogin -ParameterFilter {
+                                    $ServerName -eq $expectedServerName
+                                } -Exactly -Times 1 -Scope It
+                            }
                         }
 
                         Context 'When trying to alter the login name but Invoke-Query fails' {
                             BeforeAll {
+                                $setTargetResourceParameters = $mockDefaultParameters.Clone()
+                                $setTargetResourceParameters['LoginName'] = 'OtherLogin1'
+                                $setTargetResourceParameters['UserType'] = 'Login'
+
                                 Mock -CommandName Invoke-Query -MockWith {
                                     throw
                                 }
@@ -657,7 +745,7 @@ try
 
                                 Assert-MockCalled -CommandName Get-TargetResource -Exactly -Times 1 -Scope It
                                 Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 1 -Scope It
-                            }
+                           }
                         }
                     }
 
@@ -959,8 +1047,12 @@ try
 
             Context 'When the SQL login exist' {
                 BeforeAll {
-                    $assertSqlLoginParameters = $mockDefaultParameters.Clone()
-                    $assertSqlLoginParameters['LoginName'] = $mockLoginName
+                    $assertSqlLoginParameters = @{
+                        InstanceName = $mockInstanceName
+                        ServerName   = $mockServerName
+                        LoginName    = $mockLoginName
+                        Verbose      = $true
+                    }
                 }
 
                 It 'Should not throw any error' {
@@ -970,8 +1062,12 @@ try
 
             Context 'When the SQL login does not exist' {
                 BeforeAll {
-                    $assertSqlLoginParameters = $mockDefaultParameters.Clone()
-                    $assertSqlLoginParameters['LoginName'] = 'AnyValue'
+                    $assertSqlLoginParameters = @{
+                        InstanceName = $mockInstanceName
+                        ServerName   = $mockServerName
+                        LoginName    = 'AnyValue'
+                        Verbose      = $true
+                    }
                 }
 
                 It 'Should throw the correct error' {