From 728a1cf4dc6363e2959eb8b85f38c20199c0bf2e Mon Sep 17 00:00:00 2001 From: Wayne Teutenberg Date: Wed, 3 Jul 2019 03:00:53 +1200 Subject: [PATCH] SqlServerSecureConnection: Fix issue #1350 (#1376) - Changes to SqlServerSecureConnection - Forced $Thumbprint to lowercase to fix issue #1350 --- CHANGELOG.md | 4 ++- .../MSFT_SqlServerSecureConnection.psm1 | 12 +++++++ .../SqlServerDsc.Common.psm1 | 17 ++++++---- .../MSFT_SqlServerSecureConnection.Tests.ps1 | 33 +++++++++++++++---- 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 446e47e43..5d8f9a437 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,7 +86,9 @@ - Fix issue where calling Get would return an error because the database name list may have been returned as a string instead of as a string array ([issue #1368](https://github.com/PowerShell/SqlServerDsc/issues/1368)). - +- Changes to SqlServerSecureConnection + - Forced $Thumbprint to lowercase to fix issue#1350 + ## 12.5.0.0 - Changes to SqlServerSecureConnection diff --git a/DSCResources/MSFT_SqlServerSecureConnection/MSFT_SqlServerSecureConnection.psm1 b/DSCResources/MSFT_SqlServerSecureConnection/MSFT_SqlServerSecureConnection.psm1 index 287f1e92a..5a881a95c 100644 --- a/DSCResources/MSFT_SqlServerSecureConnection/MSFT_SqlServerSecureConnection.psm1 +++ b/DSCResources/MSFT_SqlServerSecureConnection/MSFT_SqlServerSecureConnection.psm1 @@ -68,6 +68,12 @@ function Get-TargetResource if ($Ensure -eq 'Present') { + # Configuration manager requires thumbprint to be lowercase or it won't display the configured certificate. + if (-not [string]::IsNullOrEmpty($Thumbprint)) + { + $Thumbprint = $Thumbprint.ToLower() + } + $ensureValue = 'Present' $certificateSettings = Test-CertificatePermission -Thumbprint $Thumbprint -ServiceAccount $ServiceAccount if ($encryptionSettings.Certificate -ine $Thumbprint) @@ -199,6 +205,12 @@ function Set-TargetResource $ServiceAccount ) + # Configuration manager requires thumbprint to be lowercase or it won't display the configured certificate. + if (-not [string]::IsNullOrEmpty($Thumbprint)) + { + $Thumbprint = $Thumbprint.ToLower() + } + $parameters = @{ InstanceName = $InstanceName Thumbprint = $Thumbprint diff --git a/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 b/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 index f780af7b6..95aaf2c92 100644 --- a/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 +++ b/Modules/SqlServerDsc.Common/SqlServerDsc.Common.psm1 @@ -1465,15 +1465,18 @@ function Restart-SqlService { # This call, if it fails, will take between ~9-10 seconds to return. $testConnectionServerObject = Connect-SQL -ServerName $SQLServer -InstanceName $SQLInstanceName -ErrorAction SilentlyContinue - if ($testConnectionServerObject -and $testConnectionServerObject.Status -ne 'Online') - { - # Waiting 2 seconds to not hammer the SQL Server instance. - Start-Sleep -Seconds 2 - } - else + + # Make sure we have an SMO object to test Status + if ($testConnectionServerObject) { - break + if ($testConnectionServerObject.Status -eq 'Online') + { + break + } } + + # Waiting 2 seconds to not hammer the SQL Server instance. + Start-Sleep -Seconds 2 } until ($connectTimer.Elapsed.Seconds -ge $Timeout) $connectTimer.Stop() diff --git a/Tests/Unit/MSFT_SqlServerSecureConnection.Tests.ps1 b/Tests/Unit/MSFT_SqlServerSecureConnection.Tests.ps1 index bf1d90e51..16d901879 100644 --- a/Tests/Unit/MSFT_SqlServerSecureConnection.Tests.ps1 +++ b/Tests/Unit/MSFT_SqlServerSecureConnection.Tests.ps1 @@ -65,13 +65,12 @@ try [void] AddAccessRule([System.Security.AccessControl.FileSystemAccessRule] $object) { - } } class MockedGetItem { - [string] $Thumbprint = '12345678' + [string] $Thumbprint = '1a11ab1ab1a11111a1111ab111111ab11abcdefa' [string] $PSPath = 'PathToItem' [string] $Path = 'PathToItem' [MockedAccessControl]$ACL = [MockedAccessControl]::new() @@ -89,7 +88,7 @@ try $mockNamedInstanceName = 'INSTANCE' $mockDefaultInstanceName = 'MSSQLSERVER' - $mockThumbprint = '123456789' + $mockThumbprint = '2A11AB1AB1A11111A1111AB111111AB11ABCDEFB' $mockServiceAccount = 'SqlSvc' Describe 'SqlServerSecureConnection\Get-TargetResource' -Tag 'Get' { @@ -117,14 +116,14 @@ try It 'Should return the the state of present' { $resultGetTargetResource = Get-TargetResource @defaultParameters $resultGetTargetResource.InstanceName | Should -Be $mockNamedInstanceName - $resultGetTargetResource.Thumbprint | Should -Be $mockThumbprint + $resultGetTargetResource.Thumbprint | Should -BeExactly $mockThumbprint $resultGetTargetResource.ServiceAccount | Should -Be $mockServiceAccount $resultGetTargetResource.ForceEncryption | Should -Be $true $resultGetTargetResource.Ensure | Should -Be 'Present' Assert-MockCalled -CommandName Get-EncryptedConnectionSetting -Exactly -Times 1 -Scope It + Assert-MockCalled -CommandName Test-CertificatePermission -Exactly -Times 1 -Scope It -ParameterFilter { $Thumbprint -ceq $mockThumbprint.ToLower() } } - } Context 'When the system is not in the desired state and Ensure is Present' { @@ -302,11 +301,32 @@ try Context 'When the system is not in the desired state' { + Context 'When Thumbprint contain upper-case' { + BeforeAll { + Mock -CommandName Get-TargetResource -MockWith { + return @{ + InstanceName = $mockNamedInstanceName + Thumbprint = $mockThumbprint.ToUpper() + } + } -Verifiable + } + + It 'Should configure with lower-case' { + { Set-TargetResource @defaultParameters } | Should -Not -Throw + + Assert-MockCalled -CommandName Set-EncryptedConnectionSetting -Exactly -Times 1 -Scope It -ParameterFilter { $Thumbprint -ceq $mockThumbprint.ToLower() } + + Assert-MockCalled -CommandName Set-CertificatePermission -Exactly -Times 1 -Scope It -ParameterFilter { $Thumbprint -ceq $mockThumbprint.ToLower() } + + Assert-MockCalled -CommandName Restart-SqlService -Exactly -Times 1 -Scope It + } + } + Context 'When only certificate permissions are set' { Mock -CommandName Get-TargetResource -MockWith { return @{ InstanceName = $mockNamedInstanceName - Thumbprint = '987654321' + Thumbprint = $mockThumbprint ServiceAccount = $mockServiceAccount ForceEncryption = $false Ensure = 'Present' @@ -336,6 +356,7 @@ try } } Mock -CommandName Test-CertificatePermission -MockWith { return $false } + It 'Should configure only certificate permissions' { { Set-TargetResource @defaultParameters } | Should -Not -Throw