SqlAGDatabase: Fix impersonate permission test

CHANGELOG.md

@@ -30,6 +30,14 @@
   - Added a new helper function `Get-InstalledSharedFeatures` to move out
     some of the code from the `Get-TargetResource` to make unit testing
     easier and faster.
+- Changes to SqlAGDatabase
+  - Fix MatchDatabaseOwner to check for CONTROL SERVER and IMPERSONATE LOGIN
+    permission in addition to IMPERSONATE ANY LOGIN.
+  - Update and fix MatchDatabaseOwner help text.
+- Changes to xSQLServerHelper
+  - New-TerminatingError error text for a missing localized message now matches
+    the output even if the "missing localized message" localized message is
+    also missing.
 
 ## 12.3.0.0
 

DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1

@@ -138,11 +138,12 @@ function Get-TargetResource
     .PARAMETER MatchDatabaseOwner
         If set to $true, this ensures the database owner of the database on the primary replica is the
         owner of the database on all secondary replicas. This requires the database owner is available
-        as a login on all replicas and that the PSDscRunAsCredential has impersonate permissions.
+        as a login on all replicas and that the PSDscRunAsCredential has impersonate login, impersonate
+        any login, or control server permissions.
 
         If set to $false, the owner of the database will be the PSDscRunAsCredential.
 
-        The default is '$true'.
+        The default is '$false'.
 
     .PARAMETER ProcessOnlyOnActiveNode
         Specifies that the resource will only determine if a change is needed if the target node is the active host of the SQL Server Instance.
@@ -232,27 +233,32 @@ function Set-TargetResource
         # Get only the secondary replicas. Some tests do not need to be performed on the primary replica
         $secondaryReplicas = $availabilityGroup.AvailabilityReplicas | Where-Object -FilterScript { $_.Role -ne 'Primary' }
 
-        # Ensure the appropriate permissions are in place on all the replicas
-        if ( $MatchDatabaseOwner )
+        foreach ( $databaseToAddToAvailabilityGroup in $databasesToAddToAvailabilityGroup )
         {
-            $impersonatePermissionsStatus = @{}
+            $databaseObject = $primaryServerObject.Databases[$databaseToAddToAvailabilityGroup]
 
-            foreach ( $availabilityGroupReplica in $secondaryReplicas )
+            # Ensure the appropriate permissions are in place on all the replicas
+            if ( $MatchDatabaseOwner )
             {
-                $currentAvailabilityGroupReplicaServerObject = Connect-SQL -ServerName $availabilityGroupReplica.Name
-                $impersonatePermissionsStatus.Add(
-                    $availabilityGroupReplica.Name,
-                    ( Test-ImpersonatePermissions -ServerObject $currentAvailabilityGroupReplicaServerObject )
-                )
-            }
+                $impersonatePermissionsStatus = @{}
 
-            if ( $impersonatePermissionsStatus.Values -contains $false )
-            {
-                $impersonatePermissionsMissingParameters = @(
-                    [System.Security.Principal.WindowsIdentity]::GetCurrent().Name,
-                    ( ( $impersonatePermissionsStatus.GetEnumerator() | Where-Object -FilterScript { -not $_.Value } | Select-Object -ExpandProperty Key ) -join ', ' )
-                )
-                throw ($script:localizedData.ImpersonatePermissionsMissing -f $impersonatePermissionsMissingParameters )
+                foreach ( $availabilityGroupReplica in $secondaryReplicas )
+                {
+                    $currentAvailabilityGroupReplicaServerObject = Connect-SQL -ServerName $availabilityGroupReplica.Name
+                    $impersonatePermissionsStatus.Add(
+                        $availabilityGroupReplica.Name,
+                        ( Test-ImpersonatePermissions -ServerObject $currentAvailabilityGroupReplicaServerObject -Securable $databaseObject.Owner )
+                    )
+                }
+
+                if ( $impersonatePermissionsStatus.Values -contains $false )
+                {
+                    $impersonatePermissionsMissingParameters = @(
+                        [System.Security.Principal.WindowsIdentity]::GetCurrent().Name,
+                        ( ( $impersonatePermissionsStatus.GetEnumerator() | Where-Object -FilterScript { -not $_.Value } | Select-Object -ExpandProperty Key ) -join ', ' )
+                    )
+                    throw ($script:localizedData.ImpersonatePermissionsMissing -f $impersonatePermissionsMissingParameters )
+                }
             }
         }
 
@@ -481,15 +487,36 @@ function Set-TargetResource
                 $restoreDatabaseQueryStringBuilder.Append($databaseFullBackupFile) | Out-Null
                 $restoreDatabaseQueryStringBuilder.AppendLine('''') | Out-Null
                 $restoreDatabaseQueryStringBuilder.Append('WITH NORECOVERY') | Out-Null
+                if ( $MatchDatabaseOwner )
+                {
+                    $restoreDatabaseQueryStringBuilder.AppendLine() | Out-Null
+                    $restoreDatabaseQueryStringBuilder.Append('REVERT') | Out-Null
+                }
                 $restoreDatabaseQueryString = $restoreDatabaseQueryStringBuilder.ToString()
 
-                # Build the parameters to restore the transaction log
-                $restoreSqlDatabaseLogParameters = @{
-                    Database      = $databaseToAddToAvailabilityGroup
-                    BackupFile    = $databaseLogBackupFile
-                    RestoreAction = 'Log'
-                    NoRecovery    = $true
+                # Need to restore the database with a query in order to impersonate the correct login
+                $restoreLogQueryStringBuilder = New-Object -TypeName System.Text.StringBuilder
+
+                if ( $MatchDatabaseOwner )
+                {
+                    $restoreLogQueryStringBuilder.Append('EXECUTE AS LOGIN = ''') | Out-Null
+                    $restoreLogQueryStringBuilder.Append($databaseObject.Owner) | Out-Null
+                    $restoreLogQueryStringBuilder.AppendLine('''') | Out-Null
+                }
+
+                $restoreLogQueryStringBuilder.Append('RESTORE DATABASE [') | Out-Null
+                $restoreLogQueryStringBuilder.Append($databaseToAddToAvailabilityGroup) | Out-Null
+                $restoreLogQueryStringBuilder.AppendLine(']') | Out-Null
+                $restoreLogQueryStringBuilder.Append('FROM DISK = ''') | Out-Null
+                $restoreLogQueryStringBuilder.Append($databaseLogBackupFile) | Out-Null
+                $restoreLogQueryStringBuilder.AppendLine('''') | Out-Null
+                $restoreLogQueryStringBuilder.Append('WITH NORECOVERY') | Out-Null
+                if ( $MatchDatabaseOwner )
+                {
+                    $restoreLogQueryStringBuilder.AppendLine() | Out-Null
+                    $restoreLogQueryStringBuilder.Append('REVERT') | Out-Null
                 }
+                $restoreLogQueryString = $restoreLogQueryStringBuilder.ToString()
 
                 try
                 {
@@ -502,7 +529,7 @@ function Set-TargetResource
 
                         # Restore the database
                         Invoke-Query -SQLServer $currentAvailabilityGroupReplicaServerObject.NetName -SQLInstanceName $currentAvailabilityGroupReplicaServerObject.ServiceName -Database master -Query $restoreDatabaseQueryString
-                        Restore-SqlDatabase -InputObject $currentAvailabilityGroupReplicaServerObject @restoreSqlDatabaseLogParameters
+                        Invoke-Query -SQLServer $currentAvailabilityGroupReplicaServerObject.NetName -SQLInstanceName $currentAvailabilityGroupReplicaServerObject.ServiceName -Database master -Query $restoreLogQueryString
 
                         # Add the database to the Availability Group
                         Add-SqlAvailabilityDatabase -InputObject $currentReplicaAvailabilityGroupObject -Database $databaseToAddToAvailabilityGroup
@@ -604,11 +631,12 @@ function Set-TargetResource
     .PARAMETER MatchDatabaseOwner
         If set to $true, this ensures the database owner of the database on the primary replica is the
         owner of the database on all secondary replicas. This requires the database owner is available
-        as a login on all replicas and that the PSDscRunAsCredential has impersonate permissions.
+        as a login on all replicas and that the PSDscRunAsCredential has impersonate login, impersonate
+        any login, or control server permissions.
 
         If set to $false, the owner of the database will be the PSDscRunAsCredential.
 
-        The default is '$true'.
+        The default is '$false'.
 
     .PARAMETER ProcessOnlyOnActiveNode
         Specifies that the resource will only determine if a change is needed if the target node is the active host of the SQL Server Instance.

DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof

@@ -8,7 +8,7 @@ class MSFT_SqlAGDatabase : OMI_BaseResource
     [Required, Description("The path used to seed the availability group replicas. This should be a path that is accessible by all of the replicas")] String BackupPath;
     [Write, Description("Specifies the membership of the database(s) in the availability group. The options are: Present: The defined database(s) are added to the availability group. All other databases that may be a member of the availability group are ignored. Absent: The defined database(s) are removed from the availability group. All other databases that may be a member of the availability group are ignored. The default is 'Present'."), ValueMap{"Present","Absent"}, Values{"Present","Absent"}] String Ensure;
     [Write, Description("When used with 'Ensure = 'Present'' it ensures the specified database(s) are the only databases that are a member of the specified Availability Group. This parameter is ignored when 'Ensure' is 'Absent'.")] Boolean Force;
-    [Write, Description("If set to $true, this ensures the database owner of the database on the primary replica is the owner of the database on all secondary replicas. This requires the database owner is available as a login on all replicas and that the PsDscRunAsCredential has impersonate permissions. If set to $false, the owner of the database will be the PsDscRunAsCredential. The default is '$true'")] Boolean MatchDatabaseOwner;
+    [Write, Description("If set to $true, this ensures the database owner of the database on the primary replica is the owner of the database on all secondary replicas. This requires the database owner is available as a login on all replicas and that the PSDscRunAsCredential has impersonate login, impersonate any login, or control server permissions. If set to $false, the owner of the database will be the PsDscRunAsCredential. The default is '$false'")] Boolean MatchDatabaseOwner;
     [Write, Description("Specifies that the resource will only determine if a change is needed if the target node is the active host of the SQL Server Instance.")] Boolean ProcessOnlyOnActiveNode;
     [Read, Description("Determines if the current node is actively hosting the SQL Server instance.")] Boolean IsActiveNode;
 };

DSCResources/MSFT_SqlAGDatabase/en-US/MSFT_SqlAGDatabase.strings.psd1

@@ -7,7 +7,7 @@ ConvertFrom-StringData @'
     DatabaseShouldBeMember = The following databases should be a member of the availability group '{0}': {1}.
     DatabaseShouldNotBeMember = The following databases should not be a member of the availability group '{0}': {1}.
     DatabasesNotFound = The following databases were not found in the instance: {0}.
-    ImpersonatePermissionsMissing = The login '{0}' is missing impersonate permissions in the instances '{1}'.
+    ImpersonatePermissionsMissing = The login '{0}' is missing impersonate any login, control server, or impersonate permissions in the instances '{1}'.
     NotActiveNode = The node '{0}' is not actively hosting the instance '{1}'. Exiting the test.
     ParameterNotOfType = The parameter '{0}' is not of the type '{1}'.
     ParameterNullOrEmpty = The parameter '{0}' is NULL or empty.

DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt

@@ -40,11 +40,12 @@ PARAMETER Force
 PARAMETER MatchDatabaseOwner
     If set to $true, this ensures the database owner of the database on the primary replica is the
     owner of the database on all secondary replicas. This requires the database owner is available
-    as a login on all replicas and that the PSDscRunAsCredential has impersonate permissions.
+    as a login on all replicas and that the PSDscRunAsCredential has impersonate login, impersonate
+    any login, or control server permissions.
 
     If set to $false, the owner of the database will be the PSDscRunAsCredential.
 
-    The default is '$true'.
+    The default is '$false'.
 
 .PARAMETER ProcessOnlyOnActiveNode
         Specifies that the resource will only determine if a change is needed if the target node is the active host of the SQL Server Instance.

README.md

@@ -286,10 +286,11 @@ group.
   Availability Group. This parameter is ignored when 'Ensure' is 'Absent'.
 * **`[Boolean]` MatchDatabaseOwner** _(Write)_: If set to $true, this ensures the
   database owner of the database on the primary replica is the owner of the database
-  on all secondary replicas. This requires the database owner is available as a
-  login on all replicas and that the PsDscRunAsCredential has impersonate permissions.
+  on all secondary replicas. This requires the database owner is available
+  as a login on all replicas and that the PSDscRunAsCredential has impersonate login,
+  impersonate any login, or control server permissions.
   If set to $false, the owner of the database will be the PsDscRunAsCredential.
-  The default is '$true'.
+  The default is '$false'.
 * **`[Boolean]` ProcessOnlyOnActiveNode** _(Write)_: Specifies that the resource
   will only determine if a change is needed if the target node is the active
   host of the SQL Server Instance.

SqlServerDscHelper.psm1

@@ -289,7 +289,7 @@ function New-TerminatingError
 
         if (!$errorMessage)
         {
-            $errorMessage = ("No Localization key found for key: {0}" -f $ErrorType)
+            $errorMessage = ("No Localization key found for ErrorType: '{0}'." -f $ErrorType)
         }
     }
 
@@ -1079,7 +1079,20 @@ function Test-LoginEffectivePermissions
 
         [Parameter(Mandatory = $true)]
         [System.String[]]
-        $Permissions
+        $Permissions,
+
+        # Other types are not used here and so not implemented:
+        # 'APPLICATION ROLE', 'ASSEMBLY', 'ASYMMETRIC KEY', 'CERTIFICATE', 'CONTRACT', 'DATABASE', 'ENDPOINT', 'FULLTEXT CATALOG',
+        # 'MESSAGE TYPE', 'OBJECT', 'REMOTE SERVICE BINDING', 'ROLE', 'ROUTE', 'SCHEMA', 'SERVICE', 'SYMMETRIC KEY',
+        # 'TYPE', 'USER', 'XML SCHEMA COLLECTION'
+        [ValidateSet('SERVER', 'LOGIN')]
+        [Parameter()]
+        [System.String]
+        $SecurableClass = 'SERVER',
+
+        [Parameter()]
+        [System.String]
+        $SecurableName
     )
 
     # Assume the permissions are not present
@@ -1092,12 +1105,24 @@ function Test-LoginEffectivePermissions
         WithResults     = $true
     }
 
-    $queryToGetEffectivePermissionsForLogin = "
-        EXECUTE AS LOGIN = '$LoginName'
-        SELECT DISTINCT permission_name
-        FROM fn_my_permissions(null,'SERVER')
-        REVERT
-    "
+    if ( [System.String]::IsNullOrEmpty($SecurableName) )
+    {
+        $queryToGetEffectivePermissionsForLogin = "
+            EXECUTE AS LOGIN = '$LoginName'
+            SELECT DISTINCT permission_name
+            FROM fn_my_permissions(null,'$SecurableClass')
+            REVERT
+        "
+    }
+    else
+    {
+        $queryToGetEffectivePermissionsForLogin = "
+            EXECUTE AS LOGIN = '$LoginName'
+            SELECT DISTINCT permission_name
+            FROM fn_my_permissions('$SecurableName','$SecurableClass')
+            REVERT
+        "
+    }
 
     Write-Verbose -Message ($script:localizedData.GetEffectivePermissionForLogin -f $LoginName, $sqlInstanceName) -Verbose
 
@@ -1233,16 +1258,27 @@ function Get-PrimaryReplicaServerObject
 
     .PARAMETER ServerObject
         The server object on which to perform the test.
+
+    .PARAMETER SecurableName
+        If set then impersonate permission on this specific securable (e.g. login) is also checked.
+
 #>
 function Test-ImpersonatePermissions
 {
     param
     (
         [Parameter(Mandatory = $true)]
         [Microsoft.SqlServer.Management.Smo.Server]
-        $ServerObject
+        $ServerObject,
+
+        [Parameter()]
+        [System.String]
+        $SecurableName
     )
 
+    $impersonatePermissionsPresent = $false
+
+    # The impersonate any login permission only exists in SQL 2014 and above
     $testLoginEffectivePermissionsParams = @{
         SQLServer       = $ServerObject.ComputerNamePhysicalNetBIOS
         SQLInstanceName = $ServerObject.ServiceName
@@ -1251,10 +1287,51 @@ function Test-ImpersonatePermissions
     }
 
     $impersonatePermissionsPresent = Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams
+    if ($impersonatePermissionsPresent)
+    {
+        return $impersonatePermissionsPresent
+    }
+    else
+    {
+        New-VerboseMessage -Message ( 'The login "{0}" does not have impersonate any login permissions on the instance "{1}\{2}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName )
+    }
 
-    if ( -not $impersonatePermissionsPresent )
+    # Check for sysadmin / control server permission which allows impersonation
+    $testLoginEffectivePermissionsParams = @{
+        SQLServer       = $ServerObject.ComputerNamePhysicalNetBIOS
+        SQLInstanceName = $ServerObject.ServiceName
+        LoginName       = $ServerObject.ConnectionContext.TrueLogin
+        Permissions     = @('CONTROL SERVER')
+    }
+    $impersonatePermissionsPresent = Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams
+    if ($impersonatePermissionsPresent)
+    {
+        return $impersonatePermissionsPresent
+    }
+    else
     {
-        New-VerboseMessage -Message ( 'The login "{0}" does not have impersonate permissions on the instance "{1}\{2}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName )
+        New-VerboseMessage -Message ( 'The login "{0}" does not have control server permissions on the instance "{1}\{2}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName )
+    }
+
+    if ( -not [System.String]::IsNullOrEmpty($SecurableName) ) {
+        # Check for login-specific impersonation permissions
+        $testLoginEffectivePermissionsParams = @{
+            SQLServer       = $ServerObject.ComputerNamePhysicalNetBIOS
+            SQLInstanceName = $ServerObject.ServiceName
+            LoginName       = $ServerObject.ConnectionContext.TrueLogin
+            Permissions     = @('IMPERSONATE')
+            SecurableClass  = 'LOGIN'
+            SecurableName   = $SecurableName
+        }
+        $impersonatePermissionsPresent = Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams
+        if ($impersonatePermissionsPresent)
+        {
+            return $impersonatePermissionsPresent
+        }
+        else
+        {
+            New-VerboseMessage -Message ( 'The login "{0}" does not have impersonate permissions on the instance "{1}\{2}" for the login "{3}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName, $SecurableName )
+        }
     }
 
     return $impersonatePermissionsPresent
@@ -1572,7 +1649,7 @@ function Get-ServiceAccount
 function Find-ExceptionByNumber
 {
     # Define parameters
-    param 
+    param
     (
         [Parameter(Mandatory = $true)]
         [System.Exception]