From a66fcde9c4fcac37336166c8bd4da94b569d6d59 Mon Sep 17 00:00:00 2001 From: Johan Ljunggren Date: Thu, 18 Jul 2019 15:39:27 +0200 Subject: [PATCH] Changes to helper function Connect-SQL - Connect-SQL now uses parameter sets to more intuitive evaluate that the correct parameters are used in different scenarios (issue #1403). --- CHANGELOG.md | 3 + .../SqlServerDsc.Common.psm1 | 93 ++++++----- Tests/Unit/SqlServerDsc.Common.Tests.ps1 | 147 ++++++++++++------ 3 files changed, 152 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1cf52c850..157017bb81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,9 @@ - Changes to helper function Connect-SQL - When impersonating WindowsUser credential use the NetworkCredential UserName. - Added addtional verbose logging. + - Connect-SQL now uses parameter sets to more intuitive evaluate that + the correct parameters are used in different scenarios + ([issue #1403](https://github.com/PowerShell/SqlServerDsc/issues/1403)). - Changes to SqlServerSecureConnection - Forced $Thumbprint to lowercase to fix [issue #1350](https://github.com/PowerShell/SqlServerDsc/issues/1350). - Add parameter SuppressRestart with default value false. diff --git a/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 b/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 index 4f36e2b045..9b21d1a2da 100644 --- a/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 +++ b/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 @@ -920,54 +920,71 @@ function Start-SqlSetupProcess .PARAMETER SQLServer String containing the host name of the SQL Server to connect to. + Defaults to $env:COMPUTERNAME. .PARAMETER SQLInstanceName String containing the SQL Server Database Engine instance to connect to. + Defaults to 'MSSQLSERVER'. .PARAMETER SetupCredential - PSCredential object with the credentials to use to impersonate a user when connecting. - If this is not provided then the current user will be used to connect to the SQL Server Database Engine instance. + The credentials to use to impersonate a user when connecting to the + SQL Server Database Engine instance. If this parameter is left out, then + the current user will be used to connect to the SQL Server Database Engine + instance using Windows Integrated authentication. .PARAMETER LoginType Specifies which type of logon credential should be used. The valid types - are Integrated, WindowsUser, or SqlLogin. If WindowsUser or SqlLogin are - specified then the parameter SetupCredential needs to be specified as well. - If set to 'Integrated' then the credentials that the resource current are - run with will be used. + are 'WindowsUser' or 'SqlLogin'. Defaults to 'WindowsUser' If set to 'WindowsUser' then the it will impersonate using the Windows login specified in the parameter SetupCredential. If set to 'WindowsUser' then the it will impersonate using the native SQL login specified in the parameter SetupCredential. - Default value is 'Integrated'. .PARAMETER StatementTimeout - Set the query StatementTimeout in seconds. Default 600 seconds (10mins). + Set the query StatementTimeout in seconds. Default 600 seconds (10 minutes). + + .EXAMPLE + Connect-SQL + + Connects to the default instance on the local server. + + .EXAMPLE + Connect-SQL -InstanceName 'MyInstance' + + Connects to the instance 'MyInstance' on the local server. + + .EXAMPLE + Connect-SQL ServerName 'sql.company.local' -InstanceName 'MyInstance' + + Connects to the instance 'MyInstance' on the server 'sql.company.local'. #> function Connect-SQL { - [CmdletBinding()] + [CmdletBinding(DefaultParameterSetName='SqlServer')] param ( - [Parameter()] + [Parameter(ParameterSetName='SqlServer')] + [Parameter(ParameterSetName='SqlServerWithCredential')] [ValidateNotNull()] [System.String] $ServerName = $env:COMPUTERNAME, - [Parameter()] + [Parameter(ParameterSetName='SqlServer')] + [Parameter(ParameterSetName='SqlServerWithCredential')] [ValidateNotNull()] [System.String] $InstanceName = 'MSSQLSERVER', - [Parameter()] + [Parameter(ParameterSetName='SqlServerWithCredential', Mandatory = $true)] [ValidateNotNull()] [Alias('DatabaseCredential')] [System.Management.Automation.PSCredential] $SetupCredential, - [Parameter()] - [ValidateSet('Integrated', 'WindowsUser', 'SqlLogin')] + [Parameter(ParameterSetName='SqlServerWithCredential')] + [ValidateSet('WindowsUser', 'SqlLogin')] [System.String] - $LoginType = 'Integrated', + $LoginType = 'WindowsUser', [Parameter()] [ValidateNotNull()] @@ -992,7 +1009,7 @@ function Connect-SQL $sqlConnectionContext.StatementTimeout = $StatementTimeout $sqlConnectionContext.ApplicationName = 'SqlServerDsc' - if ($LoginType -eq 'Integrated') + if ($PSCmdlet.ParameterSetName -eq 'SqlServer') { <# This is only used for verbose messaging and not for the connection @@ -1006,33 +1023,25 @@ function Connect-SQL } else { - if ($SetupCredential) - { - $connectUserName = $SetupCredential.GetNetworkCredential().UserName + $connectUserName = $SetupCredential.GetNetworkCredential().UserName - Write-Verbose -Message ( - $script:localizedData.ConnectingUsingImpersonation -f $connectUsername, $LoginType - ) -Verbose - - if ($LoginType -eq 'SqlLogin') - { - $sqlConnectionContext.LoginSecure = $false - $sqlConnectionContext.Login = $connectUserName - $sqlConnectionContext.SecurePassword = $SetupCredential.Password - } + Write-Verbose -Message ( + $script:localizedData.ConnectingUsingImpersonation -f $connectUsername, $LoginType + ) -Verbose - if ($LoginType -eq 'WindowsUser') - { - $sqlConnectionContext.LoginSecure = $true - $sqlConnectionContext.ConnectAsUser = $true - $sqlConnectionContext.ConnectAsUserName = $connectUserName - $sqlConnectionContext.ConnectAsUserPassword = $SetupCredential.GetNetworkCredential().Password - } + if ($LoginType -eq 'SqlLogin') + { + $sqlConnectionContext.LoginSecure = $false + $sqlConnectionContext.Login = $connectUserName + $sqlConnectionContext.SecurePassword = $SetupCredential.Password } - else + + if ($LoginType -eq 'WindowsUser') { - $errorMessage = $script:localizedData.CredentialsNotSpecified -f $LoginType - New-InvalidArgumentException -ArgumentName 'SetupCredential' -Message $errorMessage + $sqlConnectionContext.LoginSecure = $true + $sqlConnectionContext.ConnectAsUser = $true + $sqlConnectionContext.ConnectAsUserName = $connectUserName + $sqlConnectionContext.ConnectAsUserPassword = $SetupCredential.GetNetworkCredential().Password } } @@ -1698,10 +1707,14 @@ function Invoke-Query $connectSQLParameters = @{ ServerName = $SQLServer InstanceName = $SQLInstanceName - LoginType = $LoginType StatementTimeout = $StatementTimeout } + if ($LoginType -ne 'Integrated') + { + $connectSQLParameters['LoginType'] = $LoginType + } + if ($PSBoundParameters.ContainsKey('DatabaseCredential')) { $connectSQLParameters.SetupCredential = $DatabaseCredential diff --git a/Tests/Unit/SqlServerDsc.Common.Tests.ps1 b/Tests/Unit/SqlServerDsc.Common.Tests.ps1 index 032b940cb4..ee239e1e03 100644 --- a/Tests/Unit/SqlServerDsc.Common.Tests.ps1 +++ b/Tests/Unit/SqlServerDsc.Common.Tests.ps1 @@ -1451,7 +1451,7 @@ InModuleScope 'SqlServerDsc.Common' { Mock -CommandName New-InvalidOperationException -MockWith $mockThrowLocalizedMessage -Verifiable } - $queryParams = @{ + $queryParameters = @{ SQLServer = 'Server1' SQLInstanceName = 'MSSQLSERVER' Database = 'master' @@ -1471,17 +1471,22 @@ InModuleScope 'SqlServerDsc.Common' { } It 'Should execute the query silently' { - $queryParams.Query = "EXEC sp_configure 'show advanced option', '1'" - $mockExpectedQuery = $queryParams.Query.Clone() + $queryParameters.Query = "EXEC sp_configure 'show advanced option', '1'" + $mockExpectedQuery = $queryParameters.Query.Clone() - { Invoke-Query @queryParams } | Should -Not -Throw + { Invoke-Query @queryParameters } | Should -Not -Throw + + Assert-MockCalled -CommandName Connect-SQL -ParameterFilter { + # Should not be called with a login type. + $PSBoundParameters.ContainsKey('LoginType') -eq $false + } -Scope It -Times 1 -Exactly } It 'Should throw the correct error, ExecuteNonQueryFailed, when executing the query fails' { - $queryParams.Query = 'BadQuery' + $queryParameters.Query = 'BadQuery' - { Invoke-Query @queryParams } | Should -Throw ( - $script:localizedData.ExecuteNonQueryFailed -f $queryParams.Database + { Invoke-Query @queryParameters } | Should -Throw ( + $script:localizedData.ExecuteNonQueryFailed -f $queryParameters.Database ) } @@ -1513,21 +1518,51 @@ InModuleScope 'SqlServerDsc.Common' { } } - Context 'When executing a query with results' { + Context 'When executing a query with no results using Windows impersonation' { + It 'Should execute the query silently' { + $testParameters = $queryParameters.Clone() + $testParameters.LoginType = 'WindowsUser' + $testParameters.Query = "EXEC sp_configure 'show advanced option', '1'" + $mockExpectedQuery = $testParameters.Query.Clone() + + { Invoke-Query @testParameters } | Should -Not -Throw + + Assert-MockCalled -CommandName Connect-SQL -ParameterFilter { + $LoginType -eq 'WindowsUser' + } -Scope It -Times 1 -Exactly + } + } + + Context 'when executing a query with no results using SQL impersonation' { + It 'Should execute the query silently' { + $testParameters = $queryParameters.Clone() + $testParameters.LoginType = 'SqlLogin' + $testParameters.Query = "EXEC sp_configure 'show advanced option', '1'" + $mockExpectedQuery = $testParameters.Query.Clone() + + { Invoke-Query @testParameters } | Should -Not -Throw + + Assert-MockCalled -CommandName Connect-SQL -ParameterFilter { + $LoginType -eq 'SqlLogin' + } -Scope It -Times 1 -Exactly + } + } + + Context 'when executing a query with results' { It 'Should execute the query and return a result set' { - $queryParams.Query = 'SELECT name FROM sys.databases' - $mockExpectedQuery = $queryParams.Query.Clone() + $queryParameters.Query = 'SELECT name FROM sys.databases' + $mockExpectedQuery = $queryParameters.Query.Clone() - Invoke-Query @queryParams -WithResults | Should -Not -BeNullOrEmpty + Invoke-Query @queryParameters -WithResults | Should -Not -BeNullOrEmpty Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 1 -Exactly } It 'Should throw the correct error, ExecuteQueryWithResultsFailed, when executing the query fails' { - $queryParams.Query = 'BadQuery' + $queryParameters.Query = 'BadQuery' - { Invoke-Query @queryParams -WithResults } | Should -Throw ( - $script:localizedData.ExecuteQueryWithResultsFailed -f $queryParams.Database + { Invoke-Query @queryParameters -WithResults } | Should -Throw ( + $script:localizedData.ExecuteQueryWithResultsFailed -f $queryParameters.Database ) Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 1 -Exactly @@ -1576,7 +1611,7 @@ InModuleScope 'SqlServerDsc.Common' { $queryParametersWithSMO.Query = 'BadQuery' { Invoke-Query @queryParametersWithSMO } | Should -Throw ( - $script:localizedData.ExecuteNonQueryFailed -f $queryParams.Database + $script:localizedData.ExecuteNonQueryFailed -f $queryParameters.Database ) Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 0 -Exactly @@ -1597,7 +1632,7 @@ InModuleScope 'SqlServerDsc.Common' { $queryParametersWithSMO.Query = 'BadQuery' { Invoke-Query @queryParametersWithSMO -WithResults } | Should -Throw ( - $script:localizedData.ExecuteQueryWithResultsFailed -f $queryParams.Database + $script:localizedData.ExecuteQueryWithResultsFailed -f $queryParameters.Database ) Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 0 -Exactly @@ -1620,7 +1655,7 @@ InModuleScope 'SqlServerDsc.Common' { { $mockSMOServer | Invoke-Query -Query $mockQuery -Database master -WithResults } | Should -Throw ( - $script:localizedData.ExecuteQueryWithResultsFailed -f $queryParams.Database + $script:localizedData.ExecuteQueryWithResultsFailed -f $queryParameters.Database ) Assert-MockCalled -CommandName Connect-SQL -Scope It -Times 0 -Exactly @@ -2508,27 +2543,56 @@ InModuleScope 'SqlServerDsc.Common' { } Context 'When connecting to the named instance using Windows Authentication impersonation' { - It 'Should return the correct service instance' { + BeforeAll { $mockExpectedDatabaseEngineServer = $env:COMPUTERNAME $mockExpectedDatabaseEngineInstance = $mockInstanceName + } - $testParameters = @{ - ServerName = $mockExpectedDatabaseEngineServer - InstanceName = $mockExpectedDatabaseEngineInstance - SetupCredential = $mockWinCredential - LoginType = 'WindowsUser' + Context 'When using the default login type' { + BeforeAll { + $testParameters = @{ + ServerName = $mockExpectedDatabaseEngineServer + InstanceName = $mockExpectedDatabaseEngineInstance + SetupCredential = $mockWinCredential + } } - $databaseEngineServerObject = Connect-SQL @testParameters - $databaseEngineServerObject.ConnectionContext.ServerInstance | Should -BeExactly "$mockExpectedDatabaseEngineServer\$mockExpectedDatabaseEngineInstance" - $databaseEngineServerObject.ConnectionContext.ConnectAsUser | Should -Be $true - $databaseEngineServerObject.ConnectionContext.ConnectAsUserPassword | Should -BeExactly $mockWinCredential.GetNetworkCredential().Password - $databaseEngineServerObject.ConnectionContext.ConnectAsUserName | Should -BeExactly $mockWinCredential.GetNetworkCredential().UserName - $databaseEngineServerObject.ConnectionContext.ConnectAsUser | Should -Be $true - $databaseEngineServerObject.ConnectionContext.LoginSecure | Should -Be $true + It 'Should return the correct service instance' { + $databaseEngineServerObject = Connect-SQL @testParameters + $databaseEngineServerObject.ConnectionContext.ServerInstance | Should -BeExactly "$mockExpectedDatabaseEngineServer\$mockExpectedDatabaseEngineInstance" + $databaseEngineServerObject.ConnectionContext.ConnectAsUser | Should -Be $true + $databaseEngineServerObject.ConnectionContext.ConnectAsUserPassword | Should -BeExactly $mockWinCredential.GetNetworkCredential().Password + $databaseEngineServerObject.ConnectionContext.ConnectAsUserName | Should -BeExactly $mockWinCredential.GetNetworkCredential().UserName + $databaseEngineServerObject.ConnectionContext.ConnectAsUser | Should -Be $true + $databaseEngineServerObject.ConnectionContext.LoginSecure | Should -Be $true - Assert-MockCalled -CommandName New-Object -Exactly -Times 1 -Scope It ` - -ParameterFilter $mockNewObject_MicrosoftDatabaseEngine_ParameterFilter + Assert-MockCalled -CommandName New-Object -Exactly -Times 1 -Scope It ` + -ParameterFilter $mockNewObject_MicrosoftDatabaseEngine_ParameterFilter + } + } + + Context 'When using the default login type' { + BeforeAll { + $testParameters = @{ + ServerName = $mockExpectedDatabaseEngineServer + InstanceName = $mockExpectedDatabaseEngineInstance + SetupCredential = $mockWinCredential + LoginType = 'WindowsUser' + } + } + + It 'Should return the correct service instance' { + $databaseEngineServerObject = Connect-SQL @testParameters + $databaseEngineServerObject.ConnectionContext.ServerInstance | Should -BeExactly "$mockExpectedDatabaseEngineServer\$mockExpectedDatabaseEngineInstance" + $databaseEngineServerObject.ConnectionContext.ConnectAsUser | Should -Be $true + $databaseEngineServerObject.ConnectionContext.ConnectAsUserPassword | Should -BeExactly $mockWinCredential.GetNetworkCredential().Password + $databaseEngineServerObject.ConnectionContext.ConnectAsUserName | Should -BeExactly $mockWinCredential.GetNetworkCredential().UserName + $databaseEngineServerObject.ConnectionContext.ConnectAsUser | Should -Be $true + $databaseEngineServerObject.ConnectionContext.LoginSecure | Should -Be $true + + Assert-MockCalled -CommandName New-Object -Exactly -Times 1 -Scope It ` + -ParameterFilter $mockNewObject_MicrosoftDatabaseEngine_ParameterFilter + } } } @@ -2550,25 +2614,6 @@ InModuleScope 'SqlServerDsc.Common' { } } - Context 'When the logon type is WindowsUser or SqlLogin, but not credentials were passed' { - It 'Should throw the correct error' { - $mockExpectedDatabaseEngineServer = 'TestServer' - $mockExpectedDatabaseEngineInstance = 'MSSQLSERVER' - - $connectSqlParameters = @{ - ServerName = $mockExpectedDatabaseEngineServer - LoginType = 'WindowsUser' - } - - $mockCorrectErrorMessage = $script:localizedData.CredentialsNotSpecified -f $connectSqlParameters.LoginType - - { Connect-SQL @connectSqlParameters } | Should -Throw $mockCorrectErrorMessage - - Assert-MockCalled -CommandName New-Object -Exactly -Times 1 -Scope It ` - -ParameterFilter $mockNewObject_MicrosoftDatabaseEngine_ParameterFilter - } - } - Assert-VerifiableMock }