SqlServerSecureConnection: Fix issue #1350 (#1376)

- Changes to SqlServerSecureConnection - Forced $Thumbprint to lowercase to fix issue #1350
dsccommunity · Jul 2, 2019 · 728a1cf · 728a1cf
1 parent 943c878
commit 728a1cf
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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