From a560d1e220403a301e06447fdca399080e0cc2b7 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 19 Feb 2019 20:41:14 +0800 Subject: [PATCH 01/30] Framework --- CHANGELOG.md | 4 + .../MSFT_SqlAGDatabase.psm1 | 43 +++++----- .../MSFT_SqlAGDatabase.schema.mof | 2 +- .../en-US/about_SqlAGDatabase.help.txt | 2 +- README.md | 2 +- SqlServerDscHelper.psm1 | 78 ++++++++++++++++--- 6 files changed, 100 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2a04d91f..1ee09c62c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,10 @@ - Add integration tests ([issue #744](https://github.com/PowerShell/SqlServerDsc/issues/744)). [Maxime Daniou (@mdaniou)](https://github.com/mdaniou) +- Changes to SqlAGDatabase + - Fix MatchDatabaseOwner to check for CONTROL SERVER and IMPERSONATE LOGIN + permission in addition to IMPERSONATE ANY LOGIN. + - Fix MatchDatabaseOwner help text. ## 12.2.0.0 diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 index 6660f8fec..777a89c2b 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 +++ b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 @@ -142,7 +142,7 @@ function Get-TargetResource 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 +232,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 ) + } } } @@ -608,7 +613,7 @@ function Set-TargetResource 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. diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof index a4c183253..a3792f0ab 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof +++ b/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 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; }; diff --git a/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt b/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt index 706886d20..24e6964a4 100644 --- a/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt +++ b/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt @@ -44,7 +44,7 @@ PARAMETER MatchDatabaseOwner 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. diff --git a/README.md b/README.md index 7b2254141..578f7ced2 100644 --- a/README.md +++ b/README.md @@ -289,7 +289,7 @@ group. 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'. + 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. diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index 059a2d35f..8b1f87894 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -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,23 @@ 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,6 +1257,10 @@ function Get-PrimaryReplicaServerObject .PARAMETER ServerObject The server object on which to perform the test. + + .PARAMETER SecurableName + If set then impersonate permission on tihs specific securable (e.g. login) is also checked. + #> function Test-ImpersonatePermissions { @@ -1240,9 +1268,16 @@ function Test-ImpersonatePermissions ( [Parameter(Mandatory = $true)] [Microsoft.SqlServer.Management.Smo.Server] - $ServerObject + $ServerObject, + + [Parameter()] + [System.String] + $SecurableName ) + $impersonatePermissionsPresent = $false + + # This permission only exists in SQL 2014 and above $testLoginEffectivePermissionsParams = @{ SQLServer = $ServerObject.ComputerNamePhysicalNetBIOS SQLInstanceName = $ServerObject.ServiceName @@ -1252,6 +1287,31 @@ function Test-ImpersonatePermissions $impersonatePermissionsPresent = Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams + 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 ( -not $impersonatePermissionsPresent -and -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 ( -not $impersonatePermissionsPresent ) { New-VerboseMessage -Message ( 'The login "{0}" does not have impersonate permissions on the instance "{1}\{2}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName ) @@ -1572,7 +1632,7 @@ function Get-ServiceAccount function Find-ExceptionByNumber { # Define parameters - param + param ( [Parameter(Mandatory = $true)] [System.Exception] From f695c43039ae3afe8a081238507565e18d059cb6 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 19 Feb 2019 20:43:38 +0800 Subject: [PATCH 02/30] Doco --- SqlServerDscHelper.psm1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index 8b1f87894..25712f1e6 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -1277,7 +1277,7 @@ function Test-ImpersonatePermissions $impersonatePermissionsPresent = $false - # This permission only exists in SQL 2014 and above + # The impersonate any login permission only exists in SQL 2014 and above $testLoginEffectivePermissionsParams = @{ SQLServer = $ServerObject.ComputerNamePhysicalNetBIOS SQLInstanceName = $ServerObject.ServiceName From 501b43caa1fadb196e89b287f991363dff88a579 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 19 Feb 2019 21:09:31 +0800 Subject: [PATCH 03/30] Add extra permissions descriptions to help text and verbose output --- CHANGELOG.md | 2 +- .../MSFT_SqlAGDatabase.psm1 | 6 ++- .../MSFT_SqlAGDatabase.schema.mof | 2 +- .../en-US/about_SqlAGDatabase.help.txt | 3 +- README.md | 5 +- SqlServerDscHelper.psm1 | 49 +++++++++++++------ 6 files changed, 44 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ee09c62c..d8cd1d6cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,7 +65,7 @@ - Changes to SqlAGDatabase - Fix MatchDatabaseOwner to check for CONTROL SERVER and IMPERSONATE LOGIN permission in addition to IMPERSONATE ANY LOGIN. - - Fix MatchDatabaseOwner help text. + - Update and fix MatchDatabaseOwner help text. ## 12.2.0.0 diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 index 777a89c2b..ff054296e 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 +++ b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 @@ -138,7 +138,8 @@ 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. @@ -609,7 +610,8 @@ 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. diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof index a3792f0ab..1c092d721 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof +++ b/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 '$false'")] 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; }; diff --git a/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt b/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt index 24e6964a4..e889672e8 100644 --- a/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt +++ b/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt @@ -40,7 +40,8 @@ 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. diff --git a/README.md b/README.md index 578f7ced2..1840e7a3e 100644 --- a/README.md +++ b/README.md @@ -286,8 +286,9 @@ 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 '$false'. * **`[Boolean]` ProcessOnlyOnActiveNode** _(Write)_: Specifies that the resource diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index 25712f1e6..cd642ce93 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -1105,7 +1105,8 @@ function Test-LoginEffectivePermissions WithResults = $true } - if ([System.String]::IsNullOrEmpty($SecurableName)) + if ( [System.String]::IsNullOrEmpty($SecurableName) ) + { $queryToGetEffectivePermissionsForLogin = " EXECUTE AS LOGIN = '$LoginName' SELECT DISTINCT permission_name @@ -1286,20 +1287,33 @@ 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) { - # 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 + return $impersonatePermissionsPresent + } + else + { + 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 $impersonatePermissionsPresent -and -not [System.String]::IsNullOrEmpty($SecurableName) ) { + if ( [System.String]::IsNullOrEmpty($SecurableName) ) { # Check for login-specific impersonation permissions $testLoginEffectivePermissionsParams = @{ SQLServer = $ServerObject.ComputerNamePhysicalNetBIOS @@ -1310,11 +1324,14 @@ function Test-ImpersonatePermissions SecurableName = $SecurableName } $impersonatePermissionsPresent = Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams - } - - if ( -not $impersonatePermissionsPresent ) - { - New-VerboseMessage -Message ( 'The login "{0}" does not have impersonate permissions on the instance "{1}\{2}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName ) + 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 From 3b9006459b169d9a3264d86baada18de5b60b6c4 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 19 Feb 2019 21:19:04 +0800 Subject: [PATCH 04/30] Make mocks more realistic by including database owners and connections --- Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 index 43f35edb3..474af3a8a 100644 --- a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 +++ b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 @@ -88,6 +88,8 @@ try $mockAvailabilityGroupObjectName = 'AvailabilityGroup1' $mockAvailabilityGroupWithoutDatabasesObjectName = 'AvailabilityGroupWithoutDatabases' $mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServerName = 'AvailabilityGroup2' + $mockTrueLogin = 'Login1' + $mockDatabaseOwner = 'DatabaseOwner1' #endregion mock names @@ -279,6 +281,7 @@ try $newDatabaseObject.LogFiles = @{ FileName = ( [IO.Path]::Combine( $mockLogFilePath, "$($mockPresentDatabaseName).ldf" ) ) } + $newDatabaseObject.Owner = $mockDatabaseOwner # Add the database object to the database collection $mockDatabaseObjects.Add($newDatabaseObject) @@ -299,6 +302,7 @@ try $newDatabaseObject.LogFiles = @{ FileName = ( [IO.Path]::Combine( $mockLogFilePathIncorrect, "$($mockPresentDatabaseName).ldf" ) ) } + $newDatabaseObject.Owner = $mockDatabaseOwner # Add the database object to the database collection $mockDatabaseObjectsWithIncorrectFileNames.Add($newDatabaseObject) @@ -315,6 +319,9 @@ try $mockServerObject.AvailabilityGroups.Add($mockAvailabilityGroupObject.Clone()) $mockServerObject.AvailabilityGroups.Add($mockAvailabilityGroupWithoutDatabasesObject.Clone()) $mockServerObject.AvailabilityGroups.Add($mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServer.Clone()) + $mockServerObject.ComputerNamePhysicalNetBIOS = $mockServerObjectDomainInstanceName + $mockServerObject.ConnectionContext = New-Object -TypeName Microsoft.SqlServer.Management.Smo.ConnectionContext + $mockServerObject.ConnectionContext.TrueLogin = $mockTrueLogin $mockServerObject.Databases = $mockDatabaseObjects $mockServerObject.DomainInstanceName = $mockServerObjectDomainInstanceName $mockServerObject.NetName = $mockServerObjectDomainInstanceName @@ -328,6 +335,9 @@ try $mockServer2Object.AvailabilityGroups.Add($mockAvailabilityGroupObject.Clone()) $mockServer2Object.AvailabilityGroups.Add($mockAvailabilityGroupWithoutDatabasesObject.Clone()) $mockServer2Object.AvailabilityGroups.Add($mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServer.Clone()) + $mockServer2Object.ComputerNamePhysicalNetBIOS = $mockPrimaryServerObjectDomainInstanceName + $mockServer2Object.ConnectionContext = New-Object -TypeName Microsoft.SqlServer.Management.Smo.ConnectionContext + $mockServer2Object.ConnectionContext.TrueLogin = $mockTrueLogin $mockServer2Object.Databases = $mockDatabaseObjects $mockServer2Object.DomainInstanceName = $mockPrimaryServerObjectDomainInstanceName $mockServer2Object.NetName = $mockPrimaryServerObjectDomainInstanceName From 81f3aa7150efbcfb5ceccb9a447e2ec23607176c Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 19 Feb 2019 21:29:15 +0800 Subject: [PATCH 05/30] Add tests --- Tests/Unit/SqlServerDSCHelper.Tests.ps1 | 61 +++++++++++++++++++++---- 1 file changed, 53 insertions(+), 8 deletions(-) diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index 31aaafb39..0ba94770d 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -1238,23 +1238,68 @@ InModuleScope $script:helperModuleName { $mockServerObject.ServiceName = 'MSSQLSERVER' $mockServerObject.ConnectionContext = $mockConnectionContextObject + $mockImpersonateAnyLoginPermissionsPresent = @( + 'IMPERSONATE ANY LOGIN' + ) + + $mockControlServerPermissionsPresent = @( + 'CONTROL SERVER' + ) + + $mockImpersonateLoginPermissionsPresent = @( + 'IMPERSONATE' + ) + + $mockPermissionsMissing = @( + ) + + $mockInvokeQueryImpersonatePermissionsSet = @() # Will be set dynamically in the check + + $mockInvokeQueryImpersonatePermissionsResult = { + return New-Object -TypeName PSObject -Property @{ + Tables = @{ + Rows = @{ + permission_name = $mockInvokeQueryImpersonatePermissionsSet + } + } + } + } + + BeforeEach { + Mock -CommandName Invoke-Query -MockWith $mockInvokeQueryImpersonatePermissionsResult -Verifiable + } + Context 'When impersonate permissions are present for the login' { Mock -CommandName Test-LoginEffectivePermissions -MockWith { $true } - It 'Should return true when the impersonate permissions are present for the login'{ + It 'Should return true when impersonate any login permissions are present for the login' { + $mockInvokeQueryImpersonatePermissionsSet = $mockImpersonateAnyLoginPermissionsPresent.Clone() Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $true + # It's called once because it short-circuits when the IMPERSONATE ANY LOGIN permission is present + Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 1 -Scope It + } - Assert-MockCalled -CommandName Test-LoginEffectivePermissions -Scope It -Times 1 -Exactly + It 'Should return true when control server login permissions are present for the login' { + $mockInvokeQueryImpersonatePermissionsSet = $mockControlServerPermissionsPresent.Clone() + Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $true + # It's called twice because IMPERSONATE ANY LOGIN is checked first then it short-circuits + Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 2 -Scope It } - } - Context 'When impersonate permissions are missing for the login' { - Mock -CommandName Test-LoginEffectivePermissions -MockWith { $false } -Verifiable + It 'Should return true when impersonate login permissions are present for the login' { + $mockInvokeQueryImpersonatePermissionsSet = $mockImpersonateLoginPermissionsPresent.Clone() + Test-ImpersonatePermissions -ServerObject $mockServerObject-Securable 'DatabaseOwner1' | Should -Be $true + # It's called three times because IMPERSONATE ANY LOGIN and CONTROL SERVER permissions have been checked first + Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 3 -Scope It + } + } - It 'Should return false when the impersonate permissions are missing for the login'{ + Context 'When impersonate any, control server, and impersonate permissions are missing for the login' { + It 'Should return false when all of the needed permissions are missing for the login'{ + $mockInvokeQueryImpersonatePermissionsSet = $mockPermissionsMissing.Clone() Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $false - - Assert-MockCalled -CommandName Test-LoginEffectivePermissions -Scope It -Times 1 -Exactly + # It's called three times because IMPERSONATE ANY LOGIN and CONTROL SERVER and IMPERSONATE permissions have been checked + Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 3 -Scope It } } } From 4c6848f8fd0e55c0e479a5cdae3111a6e106e3fe Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 19 Feb 2019 21:57:48 +0800 Subject: [PATCH 06/30] Make tests pass and ensure code coverage --- SqlServerDscHelper.psm1 | 2 +- Tests/Unit/SqlServerDSCHelper.Tests.ps1 | 27 ++++++++++++++++--------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index cd642ce93..e0d701baa 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -1313,7 +1313,7 @@ function Test-ImpersonatePermissions 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 ( [System.String]::IsNullOrEmpty($SecurableName) ) { + if ( -not [System.String]::IsNullOrEmpty($SecurableName) ) { # Check for login-specific impersonation permissions $testLoginEffectivePermissionsParams = @{ SQLServer = $ServerObject.ComputerNamePhysicalNetBIOS diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index 0ba94770d..885001db6 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -1242,7 +1242,7 @@ InModuleScope $script:helperModuleName { 'IMPERSONATE ANY LOGIN' ) - $mockControlServerPermissionsPresent = @( + $mockControlServerPermissionsPresent = @( 'CONTROL SERVER' ) @@ -1270,35 +1270,42 @@ InModuleScope $script:helperModuleName { } Context 'When impersonate permissions are present for the login' { - Mock -CommandName Test-LoginEffectivePermissions -MockWith { $true } - It 'Should return true when impersonate any login permissions are present for the login' { $mockInvokeQueryImpersonatePermissionsSet = $mockImpersonateAnyLoginPermissionsPresent.Clone() Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $true - # It's called once because it short-circuits when the IMPERSONATE ANY LOGIN permission is present + # It's called once because IMPERSONATE ANY LOGIN is found and it exits Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 1 -Scope It } It 'Should return true when control server login permissions are present for the login' { $mockInvokeQueryImpersonatePermissionsSet = $mockControlServerPermissionsPresent.Clone() Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $true - # It's called twice because IMPERSONATE ANY LOGIN is checked first then it short-circuits + # It's called twice because IMPERSONATE ANY LOGIN is not found and then CONTROL SERVER is found Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 2 -Scope It } It 'Should return true when impersonate login permissions are present for the login' { $mockInvokeQueryImpersonatePermissionsSet = $mockImpersonateLoginPermissionsPresent.Clone() - Test-ImpersonatePermissions -ServerObject $mockServerObject-Securable 'DatabaseOwner1' | Should -Be $true - # It's called three times because IMPERSONATE ANY LOGIN and CONTROL SERVER permissions have been checked first + Test-ImpersonatePermissions -ServerObject $mockServerObject -SecurableName 'DatabaseOwner1' | Should -Be $true + # It's called three times because IMPERSONATE ANY LOGIN and CONTROL SERVER were not found, but + # IMPERSONATE LOGIN was found Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 3 -Scope It } } - Context 'When impersonate any, control server, and impersonate permissions are missing for the login' { - It 'Should return false when all of the needed permissions are missing for the login'{ + Context 'When impersonate any, control server, and impersonate permissions are missing' { + It 'Should return false when all of the needed permissions are missing and no login is specified' { $mockInvokeQueryImpersonatePermissionsSet = $mockPermissionsMissing.Clone() Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $false - # It's called three times because IMPERSONATE ANY LOGIN and CONTROL SERVER and IMPERSONATE permissions have been checked + # It's called three times because IMPERSONATE ANY LOGIN and CONTROL SERVER were not found and IMPERSONATE cannot + # be checked + Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 2 -Scope It + } + + It 'Should return false when all of the needed permissions are missing and a login is specified' { + $mockInvokeQueryImpersonatePermissionsSet = $mockPermissionsMissing.Clone() + Test-ImpersonatePermissions -ServerObject $mockServerObject -SecurableName 'DatabaseOwner2' | Should -Be $false + # It's called three times because IMPERSONATE ANY LOGIN and CONTROL SERVER and IMPERSONATE were all not found Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 3 -Scope It } } From d9456a31b2f235a653318cf488244e38d47d0069 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 19 Feb 2019 22:25:17 +0800 Subject: [PATCH 07/30] Add code coverage test for New-TerminatingError --- SqlServerDscHelper.psm1 | 2 +- Tests/Unit/SqlServerDSCHelper.Tests.ps1 | 31 ++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index e0d701baa..a424f5222 100644 --- a/SqlServerDscHelper.psm1 +++ b/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) } } diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index 885001db6..5df785846 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -1755,10 +1755,29 @@ InModuleScope $script:helperModuleName { Describe 'Testing New-TerminatingError' -Tag NewWarningMessage { Context -Name 'When building a localized error message' -Fixture { It 'Should return the correct error record with the correct error message' { - $errorRecord = New-TerminatingError -ErrorType 'NoKeyFound' -FormatArgs 'Dummy error' $errorRecord.Exception.Message | Should -Be 'No Localization key found for ErrorType: ''Dummy error''.' } + + It 'Should return the correct error record with the correct error message including InnerException' { + $errorRecord = New-TerminatingError -ErrorType 'NoKeyFound' -FormatArgs 'Dummy error' -InnerException 'Dummy exception' + $errorRecord.Exception.Message | Should -Be 'No Localization key found for ErrorType: ''Dummy error''. InnerException: Dummy exception' + } + + It 'Should return the correct error record with a matching FullyQualifiedErrorId' { + $errorRecord = New-TerminatingError -ErrorType 'NoKeyFound' -FormatArgs 'Dummy error' + $errorRecord.FullyQualifiedErrorId | Should -Be 'SqlServerDSCHelper.NoKeyFound' + } + + It 'Should return the correct error record with a matching FullyQualifiedErrorId when there is no calling module' { + Mock -CommandName Get-PSCallStack -MockWith { + , [PSCustomObject] @{ + ScriptName = '' + } + } + $errorRecord = New-TerminatingError -ErrorType 'NoKeyFound' -FormatArgs 'Dummy error' + $errorRecord.FullyQualifiedErrorId | Should -Be 'NoKeyFound' + } } Context -Name 'When building a localized error message that does not exists' -Fixture { @@ -1766,6 +1785,16 @@ InModuleScope $script:helperModuleName { $errorRecord = New-TerminatingError -ErrorType 'UnknownDummyMessage' -FormatArgs 'Dummy error' $errorRecord.Exception.Message | Should -Be 'No Localization key found for ErrorType: ''UnknownDummyMessage''.' } + + It 'Should return the correct error record with the correct error message even if the NoKeyFound message is missing' { + $noKeyFound = $script:localizedData.NoKeyFound + $script:localizedData.Remove('NoKeyFound') + + $errorRecord = New-TerminatingError -ErrorType 'UnknownDummyMessage' -FormatArgs 'Dummy error' + $errorRecord.Exception.Message | Should -Be 'No Localization key found for ErrorType: ''UnknownDummyMessage''.' + + $script:localizedData.NoKeyFound = $noKeyFound + } } Assert-VerifiableMock From 4754adf9aaa3a00ccd61c3557c0fb173d9889b03 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 19 Feb 2019 22:41:52 +0800 Subject: [PATCH 08/30] Update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8cd1d6cb..56a0c9531 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,6 +66,10 @@ - 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.2.0.0 From 0c0f6145fc968e32e83a7920d798daa58f28ce68 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 19 Feb 2019 22:52:01 +0800 Subject: [PATCH 09/30] Add permissions details to error message --- .../MSFT_SqlAGDatabase/en-US/MSFT_SqlAGDatabase.strings.psd1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DSCResources/MSFT_SqlAGDatabase/en-US/MSFT_SqlAGDatabase.strings.psd1 b/DSCResources/MSFT_SqlAGDatabase/en-US/MSFT_SqlAGDatabase.strings.psd1 index 112cf9e1e..462d8769e 100644 --- a/DSCResources/MSFT_SqlAGDatabase/en-US/MSFT_SqlAGDatabase.strings.psd1 +++ b/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. From 15750554882ba723b303fa0d28a700cb5c3c98d3 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Wed, 27 Feb 2019 19:13:29 +0800 Subject: [PATCH 10/30] Allow log restore to work with EXECUTE AS --- .../MSFT_SqlAGDatabase.psm1 | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 index ff054296e..82f4c1e97 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 +++ b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 @@ -486,17 +486,30 @@ function Set-TargetResource $restoreDatabaseQueryStringBuilder.Append('FROM DISK = ''') | Out-Null $restoreDatabaseQueryStringBuilder.Append($databaseFullBackupFile) | Out-Null $restoreDatabaseQueryStringBuilder.AppendLine('''') | Out-Null - $restoreDatabaseQueryStringBuilder.Append('WITH NORECOVERY') | Out-Null + $restoreDatabaseQueryStringBuilder.AppendLine('WITH NORECOVERY') | Out-Null + $restoreDatabaseQueryStringBuilder.AppendLine('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.AppendLine('WITH NORECOVERY') | Out-Null + $restoreLogQueryStringBuilder.AppendLine('REVERT') | Out-Null + $restoreLogQueryString = $restoreLogQueryStringBuilder.ToString() + try { foreach ( $availabilityGroupReplica in $secondaryReplicas ) @@ -508,7 +521,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 From 63f9e2e27a116ea2167078011a4f9a570906e05c Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Wed, 27 Feb 2019 19:37:58 +0800 Subject: [PATCH 11/30] Move changlog section to unreleased --- CHANGELOG.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbc68d51b..6d87c1755 100644 --- a/CHANGELOG.md +++ b/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 @@ -93,14 +101,6 @@ - Add integration tests ([issue #744](https://github.com/PowerShell/SqlServerDsc/issues/744)). [Maxime Daniou (@mdaniou)](https://github.com/mdaniou) -- 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.2.0.0 From 43ab2bac0c1266bf4ff9430c5825189a9985c2a5 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Wed, 27 Feb 2019 21:08:28 +0800 Subject: [PATCH 12/30] Fix this test text --- Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 index 474af3a8a..8e04929ba 100644 --- a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 +++ b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 @@ -611,7 +611,7 @@ WITH NORECOVERY' It 'Should throw the correct error when "MatchDatabaseOwner" is $true and the current login does not have impersonate permissions' { Mock -CommandName Test-ImpersonatePermissions -MockWith { $false } -Verifiable - { Set-TargetResource @mockSetTargetResourceParameters } | Should -Throw "The login '$([System.Security.Principal.WindowsIdentity]::GetCurrent().Name)' is missing impersonate permissions in the instances 'Server2'." + { Set-TargetResource @mockSetTargetResourceParameters } | Should -Throw "The login '$([System.Security.Principal.WindowsIdentity]::GetCurrent().Name)' is missing impersonate any login, control server, or impersonate permissions in the instances 'Server2'." Assert-MockCalled -CommandName Add-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly -ParameterFilter { $InputObject.PrimaryReplicaServerName -eq 'Server1' -and $InputObject.LocalReplicaRole -eq 'Primary' } Assert-MockCalled -CommandName Add-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly -ParameterFilter { $InputObject.PrimaryReplicaServerName -eq 'Server1' -and $InputObject.LocalReplicaRole -eq 'Secondary' } From 6f558998d33160db0e9b046a8e9069db97e5838a Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Wed, 27 Feb 2019 22:31:10 +0800 Subject: [PATCH 13/30] Fix mocked ConnectionContext TypeName. Remove Restore-SqlDatabase mock and increment Invoke-Query mock call count instead. --- .../MSFT_SqlAGDatabase.psm1 | 10 ++++- Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 | 45 ++++++------------- Tests/Unit/Stubs/SMO.cs | 2 +- 3 files changed, 22 insertions(+), 35 deletions(-) diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 index 82f4c1e97..9a233af58 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 +++ b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 @@ -487,7 +487,10 @@ function Set-TargetResource $restoreDatabaseQueryStringBuilder.Append($databaseFullBackupFile) | Out-Null $restoreDatabaseQueryStringBuilder.AppendLine('''') | Out-Null $restoreDatabaseQueryStringBuilder.AppendLine('WITH NORECOVERY') | Out-Null - $restoreDatabaseQueryStringBuilder.AppendLine('REVERT') | Out-Null + if ( $MatchDatabaseOwner ) + { + $restoreDatabaseQueryStringBuilder.AppendLine('REVERT') | Out-Null + } $restoreDatabaseQueryString = $restoreDatabaseQueryStringBuilder.ToString() # Need to restore the database with a query in order to impersonate the correct login @@ -507,7 +510,10 @@ function Set-TargetResource $restoreLogQueryStringBuilder.Append($databaseLogBackupFile) | Out-Null $restoreLogQueryStringBuilder.AppendLine('''') | Out-Null $restoreLogQueryStringBuilder.AppendLine('WITH NORECOVERY') | Out-Null - $restoreLogQueryStringBuilder.AppendLine('REVERT') | Out-Null + if ( $MatchDatabaseOwner ) + { + $restoreLogQueryStringBuilder.AppendLine('REVERT') | Out-Null + } $restoreLogQueryString = $restoreLogQueryStringBuilder.ToString() try diff --git a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 index 8e04929ba..32ca58c2a 100644 --- a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 +++ b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 @@ -373,14 +373,17 @@ try $mockInvokeQueryParameterRestoreDatabase = { $Query -like 'RESTORE DATABASE * FROM DISK = * -WITH NORECOVERY' +WITH NORECOVERY +' } $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs = { $Query -like 'EXECUTE AS LOGIN = * RESTORE DATABASE * FROM DISK = * -WITH NORECOVERY' +WITH NORECOVERY +REVERT +' } #endregion Invoke Query Mock @@ -464,7 +467,6 @@ WITH NORECOVERY' Mock -CommandName Join-Path -MockWith { [IO.Path]::Combine($databaseMembershipClass.BackupPath,"$($database.Name)_Log_$(Get-Date -Format 'yyyyMMddhhmmss').trn") } -Verifiable -ParameterFilter { $ChildPath -like '*_Log_*.trn' } Mock -CommandName New-TerminatingError { $ErrorType } -Verifiable Mock -CommandName Remove-Item -Verifiable - Mock -CommandName Restore-SqlDatabase -Verifiable } BeforeEach { @@ -511,13 +513,12 @@ WITH NORECOVERY' Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter { $Query -like 'EXEC master.dbo.xp_fileexist *' } Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 0 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabase - Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs + Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Full_*.bak' } Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Log_*.trn' } Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly } @@ -540,13 +541,12 @@ WITH NORECOVERY' Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter { $Query -like 'EXEC master.dbo.xp_fileexist *' } Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 0 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabase - Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs + Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Full_*.bak' } Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Log_*.trn' } Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly } @@ -575,7 +575,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 0 -Exactly } @@ -597,14 +596,13 @@ WITH NORECOVERY' Assert-MockCalled -CommandName Get-PrimaryReplicaServerObject -Scope It -Times 0 -Exactly -ParameterFilter { $AvailabilityGroup.PrimaryReplicaServerName -eq 'Server2' } Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter { $Query -like 'EXEC master.dbo.xp_fileexist *' } - Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabase + Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabase Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 0 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Full_*.bak' } Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Log_*.trn' } Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 0 -Exactly } @@ -633,7 +631,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly } @@ -686,8 +683,7 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly + Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly $mockServerObject.Databases['DB1'].($prerequisiteCheck.Key) = $originalValue } @@ -718,7 +714,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly } @@ -756,8 +751,7 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly + Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly $mockServerObject.Databases['DB1'].($filestreamProperty.Key) = $originalValue } @@ -789,7 +783,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly $mockServerObject.Databases['DB1'].ContainmentType = $originalValue @@ -822,7 +815,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly $mockServer2Object.Databases['DB1'].FileGroups.Files.FileName = $originalValue @@ -855,7 +847,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly $mockServer2Object.Databases['DB1'].LogFiles.FileName = $originalValue @@ -888,7 +879,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly $mockServerObject.Databases['DB1'].EncryptionEnabled = $false @@ -915,13 +905,12 @@ WITH NORECOVERY' Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter { $Query -like 'EXEC master.dbo.xp_fileexist *' } Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 0 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabase - Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs + Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Full_*.bak' } Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Log_*.trn' } Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly } @@ -950,7 +939,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly } @@ -979,7 +967,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly } @@ -1008,7 +995,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly } @@ -1031,13 +1017,12 @@ WITH NORECOVERY' Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter { $Query -like 'EXEC master.dbo.xp_fileexist *' } Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 0 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabase - Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs + Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Full_*.bak' } Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Log_*.trn' } Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly } } @@ -1070,7 +1055,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 2 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 0 -Exactly } @@ -1099,7 +1083,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 2 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 0 -Exactly } @@ -1128,7 +1111,6 @@ WITH NORECOVERY' Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 2 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 0 -Exactly } } @@ -1156,13 +1138,12 @@ WITH NORECOVERY' Assert-MockCalled -CommandName Import-SQLPSModule -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter { $Query -like 'EXEC master.dbo.xp_fileexist *' } Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 0 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabase - Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs + Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 2 -Exactly -ParameterFilter $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Full_*.bak' } Assert-MockCalled -CommandName Join-Path -Scope It -Times 1 -Exactly -ParameterFilter { $ChildPath -like '*_Log_*.trn' } Assert-MockCalled -CommandName New-TerminatingError -Scope It -Times 0 -Exactly Assert-MockCalled -CommandName Remove-Item -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Remove-SqlAvailabilityDatabase -Scope It -Times 1 -Exactly - Assert-MockCalled -CommandName Restore-SqlDatabase -Scope It -Times 1 -Exactly Assert-MockCalled -CommandName Test-ImpersonatePermissions -Scope It -Times 1 -Exactly } } diff --git a/Tests/Unit/Stubs/SMO.cs b/Tests/Unit/Stubs/SMO.cs index 9d507d8e3..bd2aec66a 100644 --- a/Tests/Unit/Stubs/SMO.cs +++ b/Tests/Unit/Stubs/SMO.cs @@ -780,7 +780,7 @@ public void Create() {} } - // TypeName: Microsoft.SqlServer.Management.Common.ServerConnection + // TypeName: Microsoft.SqlServer.Management.Smo.ConnectionContext // Used by: // SqlAGDatabase public class ConnectionContext From 3102c386a56246e005b7e82abdb2b6461ae9d737 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sat, 2 Mar 2019 17:39:47 +0800 Subject: [PATCH 14/30] Rename ConnectionContext class to ServerConnection --- Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 | 4 ++-- Tests/Unit/SqlServerDSCHelper.Tests.ps1 | 2 +- Tests/Unit/Stubs/SMO.cs | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 index 32ca58c2a..bdd8b2a1c 100644 --- a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 +++ b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 @@ -320,7 +320,7 @@ try $mockServerObject.AvailabilityGroups.Add($mockAvailabilityGroupWithoutDatabasesObject.Clone()) $mockServerObject.AvailabilityGroups.Add($mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServer.Clone()) $mockServerObject.ComputerNamePhysicalNetBIOS = $mockServerObjectDomainInstanceName - $mockServerObject.ConnectionContext = New-Object -TypeName Microsoft.SqlServer.Management.Smo.ConnectionContext + $mockServerObject.ConnectionContext = New-Object -TypeName Microsoft.SqlServer.Management.Smo.ServerConnection $mockServerObject.ConnectionContext.TrueLogin = $mockTrueLogin $mockServerObject.Databases = $mockDatabaseObjects $mockServerObject.DomainInstanceName = $mockServerObjectDomainInstanceName @@ -336,7 +336,7 @@ try $mockServer2Object.AvailabilityGroups.Add($mockAvailabilityGroupWithoutDatabasesObject.Clone()) $mockServer2Object.AvailabilityGroups.Add($mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServer.Clone()) $mockServer2Object.ComputerNamePhysicalNetBIOS = $mockPrimaryServerObjectDomainInstanceName - $mockServer2Object.ConnectionContext = New-Object -TypeName Microsoft.SqlServer.Management.Smo.ConnectionContext + $mockServer2Object.ConnectionContext = New-Object -TypeName Microsoft.SqlServer.Management.Smo.ServerConnection $mockServer2Object.ConnectionContext.TrueLogin = $mockTrueLogin $mockServer2Object.Databases = $mockDatabaseObjects $mockServer2Object.DomainInstanceName = $mockPrimaryServerObjectDomainInstanceName diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index 5df785846..8b1cbe4c0 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -1230,7 +1230,7 @@ InModuleScope $script:helperModuleName { } Describe 'Testing Test-ImpersonatePermissions' { - $mockConnectionContextObject = New-Object -TypeName Microsoft.SqlServer.Management.Smo.ConnectionContext + $mockConnectionContextObject = New-Object -TypeName Microsoft.SqlServer.Management.Smo.ServerConnection $mockConnectionContextObject.TrueLogin = 'Login1' $mockServerObject = New-Object -TypeName Microsoft.SqlServer.Management.Smo.Server diff --git a/Tests/Unit/Stubs/SMO.cs b/Tests/Unit/Stubs/SMO.cs index bd2aec66a..12744e18a 100644 --- a/Tests/Unit/Stubs/SMO.cs +++ b/Tests/Unit/Stubs/SMO.cs @@ -238,7 +238,7 @@ public class Server public string MockGranteeName; public AvailabilityGroupCollection AvailabilityGroups = new AvailabilityGroupCollection(); - public ConnectionContext ConnectionContext; + public ServerConnection ConnectionContext; public string ComputerNamePhysicalNetBIOS; public DatabaseCollection Databases = new DatabaseCollection(); public string DisplayName; @@ -780,10 +780,10 @@ public void Create() {} } - // TypeName: Microsoft.SqlServer.Management.Smo.ConnectionContext + // TypeName: Microsoft.SqlServer.Management.Common.ServerConnection // Used by: // SqlAGDatabase - public class ConnectionContext + public class ServerConnection { public string TrueLogin; From 5a62f72d11c9dba42fc3bcd04e015946898a6c89 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sat, 2 Mar 2019 22:41:45 +0800 Subject: [PATCH 15/30] . --- .../MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 | 10 ++++++---- Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 | 5 ++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 index 9a233af58..5c5f30c77 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 +++ b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 @@ -486,10 +486,11 @@ function Set-TargetResource $restoreDatabaseQueryStringBuilder.Append('FROM DISK = ''') | Out-Null $restoreDatabaseQueryStringBuilder.Append($databaseFullBackupFile) | Out-Null $restoreDatabaseQueryStringBuilder.AppendLine('''') | Out-Null - $restoreDatabaseQueryStringBuilder.AppendLine('WITH NORECOVERY') | Out-Null + $restoreDatabaseQueryStringBuilder.Append('WITH NORECOVERY') | Out-Null if ( $MatchDatabaseOwner ) { - $restoreDatabaseQueryStringBuilder.AppendLine('REVERT') | Out-Null + $restoreDatabaseQueryStringBuilder.AppendLine() | Out-Null + $restoreDatabaseQueryStringBuilder.Append('REVERT') | Out-Null } $restoreDatabaseQueryString = $restoreDatabaseQueryStringBuilder.ToString() @@ -509,10 +510,11 @@ function Set-TargetResource $restoreLogQueryStringBuilder.Append('FROM DISK = ''') | Out-Null $restoreLogQueryStringBuilder.Append($databaseLogBackupFile) | Out-Null $restoreLogQueryStringBuilder.AppendLine('''') | Out-Null - $restoreLogQueryStringBuilder.AppendLine('WITH NORECOVERY') | Out-Null + $restoreLogQueryStringBuilder.Append('WITH NORECOVERY') | Out-Null if ( $MatchDatabaseOwner ) { - $restoreLogQueryStringBuilder.AppendLine('REVERT') | Out-Null + $restoreLogQueryStringBuilder.AppendLine() | Out-Null + $restoreLogQueryStringBuilder.Append('REVERT') | Out-Null } $restoreLogQueryString = $restoreLogQueryStringBuilder.ToString() diff --git a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 index bdd8b2a1c..d1701f9c1 100644 --- a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 +++ b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 @@ -381,9 +381,8 @@ WITH NORECOVERY $Query -like 'EXECUTE AS LOGIN = * RESTORE DATABASE * FROM DISK = * -WITH NORECOVERY -REVERT -' +WITH NORECOVERY* +REVERT' } #endregion Invoke Query Mock From 011cf10cf76363ba7816274250b41ee55b501c58 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sat, 2 Mar 2019 22:49:50 +0800 Subject: [PATCH 16/30] . --- Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 index d1701f9c1..7032fa81f 100644 --- a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 +++ b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 @@ -373,8 +373,7 @@ try $mockInvokeQueryParameterRestoreDatabase = { $Query -like 'RESTORE DATABASE * FROM DISK = * -WITH NORECOVERY -' +WITH NORECOVERY' } $mockInvokeQueryParameterRestoreDatabaseWithExecuteAs = { From 2f71d591e7fdefcd583333445127dd6bea28bf8f Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sat, 2 Mar 2019 23:36:21 +0800 Subject: [PATCH 17/30] Spelling --- SqlServerDscHelper.psm1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index a424f5222..d1f360066 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -1260,7 +1260,7 @@ function Get-PrimaryReplicaServerObject The server object on which to perform the test. .PARAMETER SecurableName - If set then impersonate permission on tihs specific securable (e.g. login) is also checked. + If set then impersonate permission on this specific securable (e.g. login) is also checked. #> function Test-ImpersonatePermissions From 998d5dc9eef153fbb8365da271b3f0d1638639d8 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sun, 3 Mar 2019 18:39:06 +0800 Subject: [PATCH 18/30] Change PSDscRunAsCredential to PsDscRunAsCredential --- DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 | 8 ++++---- .../MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof | 2 +- .../MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt | 4 ++-- README.md | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 index 5c5f30c77..393f75df7 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 +++ b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 @@ -138,10 +138,10 @@ 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 login, impersonate + 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. + If set to $false, the owner of the database will be the PsDscRunAsCredential. The default is '$false'. @@ -631,10 +631,10 @@ 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 login, impersonate + 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. + If set to $false, the owner of the database will be the PsDscRunAsCredential. The default is '$false'. diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof index 1c092d721..91ab18a8b 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof +++ b/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 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("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; }; diff --git a/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt b/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt index e889672e8..90497613f 100644 --- a/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt +++ b/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt @@ -40,10 +40,10 @@ 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 login, impersonate + 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. + If set to $false, the owner of the database will be the PsDscRunAsCredential. The default is '$false'. diff --git a/README.md b/README.md index 11a62402e..3dccd490c 100644 --- a/README.md +++ b/README.md @@ -287,7 +287,7 @@ group. * **`[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 login, + 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'. From 9b497678b522af29c0ccd1c7a8e6c4b5ce31b967 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sun, 3 Mar 2019 18:55:18 +0800 Subject: [PATCH 19/30] Add internal help for helper function --- SqlServerDscHelper.psm1 | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index d1f360066..371f9fc9a 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -1059,6 +1059,43 @@ function Update-AvailabilityGroupReplica } } +<# + .SYNOPSIS + Impersonates a login and determines whether a permission is present. + + .PARAMETER SQLServer + String containing the host name of the SQL Server to connect to. + + .PARAMETER SQLInstanceName + String containing the SQL Server Database Engine instance to connect to. + + .PARAMETER LoginName + String containing the login (user) which should be checked for a permission. + + .PARAMETER Permissions + This is a list that represents a SQL Server set of database permissions. + + .PARAMETER SecurableClass + String containing the class of permissions to test. It can be: + SERVER: A permission that is applicable against server objects. + LOGIN: A permission that is applicable against login objects. + + Default is 'SERVER'. + + .PARAMETER SecurableName + String containing the name of the object against which permissions exist, e.g. if SecurableClass is LOGIN this is the name of a login permissions may exist against. + + Default is $null. + + .NOTES + These SecurableClass are not yet in this module yet and so are 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' + +#> function Test-LoginEffectivePermissions { param @@ -1081,10 +1118,6 @@ function Test-LoginEffectivePermissions [System.String[]] $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] From 074c6a3728ff7d17301b9fc7d6f11cde7297c748 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sun, 3 Mar 2019 18:55:43 +0800 Subject: [PATCH 20/30] Remove extra line of code --- SqlServerDscHelper.psm1 | 2 -- 1 file changed, 2 deletions(-) diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index 371f9fc9a..7098750e0 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -1309,8 +1309,6 @@ function Test-ImpersonatePermissions $SecurableName ) - $impersonatePermissionsPresent = $false - # The impersonate any login permission only exists in SQL 2014 and above $testLoginEffectivePermissionsParams = @{ SQLServer = $ServerObject.ComputerNamePhysicalNetBIOS From afc7f17a32491cf1d5c4eebf933e06eb6392d5cf Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sun, 3 Mar 2019 18:58:05 +0800 Subject: [PATCH 21/30] Add more verbose output --- SqlServerDscHelper.psm1 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index 7098750e0..85cf22411 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -1320,6 +1320,7 @@ function Test-ImpersonatePermissions $impersonatePermissionsPresent = Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams if ($impersonatePermissionsPresent) { + New-VerboseMessage -Message ( 'The login "{0}" has impersonate any login permissions on the instance "{1}\{2}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName ) return $impersonatePermissionsPresent } else @@ -1337,6 +1338,7 @@ function Test-ImpersonatePermissions $impersonatePermissionsPresent = Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams if ($impersonatePermissionsPresent) { + New-VerboseMessage -Message ( 'The login "{0}" has control server permissions on the instance "{1}\{2}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName ) return $impersonatePermissionsPresent } else @@ -1357,6 +1359,7 @@ function Test-ImpersonatePermissions $impersonatePermissionsPresent = Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams if ($impersonatePermissionsPresent) { + New-VerboseMessage -Message ( 'The login "{0}" has impersonate permissions on the instance "{1}\{2}" for the login "{3}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName, $SecurableName ) return $impersonatePermissionsPresent } else @@ -1365,6 +1368,7 @@ function Test-ImpersonatePermissions } } + New-VerboseMessage -Message ( 'The login "{0}" does not have any impersonate permissions required on the instance "{1}\{2}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName ) return $impersonatePermissionsPresent } From 8789567975bd22b72715ab6de4af53af18e7d551 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sun, 3 Mar 2019 19:56:33 +0800 Subject: [PATCH 22/30] Revert test for Test-ImpersonatePermissions so it only checks Test-LoginEffectivePermissions is called. --- Tests/Unit/SqlServerDSCHelper.Tests.ps1 | 68 +++---------------------- 1 file changed, 8 insertions(+), 60 deletions(-) diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index 8b1cbe4c0..27359b5ab 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -1238,75 +1238,23 @@ InModuleScope $script:helperModuleName { $mockServerObject.ServiceName = 'MSSQLSERVER' $mockServerObject.ConnectionContext = $mockConnectionContextObject - $mockImpersonateAnyLoginPermissionsPresent = @( - 'IMPERSONATE ANY LOGIN' - ) - - $mockControlServerPermissionsPresent = @( - 'CONTROL SERVER' - ) - - $mockImpersonateLoginPermissionsPresent = @( - 'IMPERSONATE' - ) - - $mockPermissionsMissing = @( - ) - - $mockInvokeQueryImpersonatePermissionsSet = @() # Will be set dynamically in the check - - $mockInvokeQueryImpersonatePermissionsResult = { - return New-Object -TypeName PSObject -Property @{ - Tables = @{ - Rows = @{ - permission_name = $mockInvokeQueryImpersonatePermissionsSet - } - } - } - } - - BeforeEach { - Mock -CommandName Invoke-Query -MockWith $mockInvokeQueryImpersonatePermissionsResult -Verifiable - } - Context 'When impersonate permissions are present for the login' { - It 'Should return true when impersonate any login permissions are present for the login' { - $mockInvokeQueryImpersonatePermissionsSet = $mockImpersonateAnyLoginPermissionsPresent.Clone() - Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $true - # It's called once because IMPERSONATE ANY LOGIN is found and it exits - Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 1 -Scope It - } + Mock -CommandName Test-LoginEffectivePermissions -MockWith { $true } - It 'Should return true when control server login permissions are present for the login' { - $mockInvokeQueryImpersonatePermissionsSet = $mockControlServerPermissionsPresent.Clone() + It 'Should return true when the impersonate permissions are present for the login' { Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $true - # It's called twice because IMPERSONATE ANY LOGIN is not found and then CONTROL SERVER is found - Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 2 -Scope It - } - It 'Should return true when impersonate login permissions are present for the login' { - $mockInvokeQueryImpersonatePermissionsSet = $mockImpersonateLoginPermissionsPresent.Clone() - Test-ImpersonatePermissions -ServerObject $mockServerObject -SecurableName 'DatabaseOwner1' | Should -Be $true - # It's called three times because IMPERSONATE ANY LOGIN and CONTROL SERVER were not found, but - # IMPERSONATE LOGIN was found - Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 3 -Scope It + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -Scope It -Times 1 -Exactly } } - Context 'When impersonate any, control server, and impersonate permissions are missing' { - It 'Should return false when all of the needed permissions are missing and no login is specified' { - $mockInvokeQueryImpersonatePermissionsSet = $mockPermissionsMissing.Clone() + Context 'When impersonate permissions are missing for the login' { + Mock -CommandName Test-LoginEffectivePermissions -MockWith { $false } -Verifiable + + It 'Should return false when the impersonate permissions are missing for the login' { Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $false - # It's called three times because IMPERSONATE ANY LOGIN and CONTROL SERVER were not found and IMPERSONATE cannot - # be checked - Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 2 -Scope It - } - It 'Should return false when all of the needed permissions are missing and a login is specified' { - $mockInvokeQueryImpersonatePermissionsSet = $mockPermissionsMissing.Clone() - Test-ImpersonatePermissions -ServerObject $mockServerObject -SecurableName 'DatabaseOwner2' | Should -Be $false - # It's called three times because IMPERSONATE ANY LOGIN and CONTROL SERVER and IMPERSONATE were all not found - Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 3 -Scope It + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -Scope It -Times 1 -Exactly } } } From cac591fa87b98db3ab893c69bda23e20445a5383 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sun, 3 Mar 2019 19:58:29 +0800 Subject: [PATCH 23/30] Phrasing --- SqlServerDscHelper.psm1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index 85cf22411..9c4c022af 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -1061,7 +1061,7 @@ function Update-AvailabilityGroupReplica <# .SYNOPSIS - Impersonates a login and determines whether a permission is present. + Impersonates a login and determines whether required permissions are present. .PARAMETER SQLServer String containing the host name of the SQL Server to connect to. From 6a87852967c15f22f8b1c3809c2d531682a88873 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sun, 3 Mar 2019 21:05:12 +0800 Subject: [PATCH 24/30] . --- Tests/Unit/SqlServerDSCHelper.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index 27359b5ab..ba9029c09 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -1254,7 +1254,7 @@ InModuleScope $script:helperModuleName { It 'Should return false when the impersonate permissions are missing for the login' { Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $false - Assert-MockCalled -CommandName Test-LoginEffectivePermissions -Scope It -Times 1 -Exactly + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -Scope It -Times 2 -Exactly } } } From 0a21dba044e734beb95f5555af09176360fbe3d6 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sun, 3 Mar 2019 22:05:13 +0800 Subject: [PATCH 25/30] Add control login as equivalent to impersonate login --- CHANGELOG.md | 4 ++-- .../MSFT_SqlAGDatabase.schema.mof | 2 +- .../en-US/MSFT_SqlAGDatabase.strings.psd1 | 2 +- README.md | 4 ++-- SqlServerDscHelper.psm1 | 20 +++++++++++++++++++ Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 | 2 +- 6 files changed, 27 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d87c1755..8db0ac290 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,8 +31,8 @@ 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. + - Fix MatchDatabaseOwner to check for CONTROL SERVER, IMPERSONATE LOGIN, or + CONTROL 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 diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof index 91ab18a8b..d25a1bae3 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.schema.mof +++ b/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 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("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 any login, control server, impersonate login, or control login 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; }; diff --git a/DSCResources/MSFT_SqlAGDatabase/en-US/MSFT_SqlAGDatabase.strings.psd1 b/DSCResources/MSFT_SqlAGDatabase/en-US/MSFT_SqlAGDatabase.strings.psd1 index 462d8769e..6e08e9903 100644 --- a/DSCResources/MSFT_SqlAGDatabase/en-US/MSFT_SqlAGDatabase.strings.psd1 +++ b/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 any login, control server, or impersonate permissions in the instances '{1}'. + ImpersonatePermissionsMissing = The login '{0}' is missing impersonate any login, control server, impersonate login, or control login 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. diff --git a/README.md b/README.md index 3dccd490c..9ce9bfccb 100644 --- a/README.md +++ b/README.md @@ -287,8 +287,8 @@ group. * **`[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 login, - impersonate any login, or control server permissions. + as a login on all replicas and that the PsDscRunAsCredential has impersonate any + login, control server, impersonate login, or control login permissions. If set to $false, the owner of the database will be the PsDscRunAsCredential. The default is '$false'. * **`[Boolean]` ProcessOnlyOnActiveNode** _(Write)_: Specifies that the resource diff --git a/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index 9c4c022af..8f3f682fc 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -1366,6 +1366,26 @@ function Test-ImpersonatePermissions { 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 ) } + + # Check for login-specific control permissions + $testLoginEffectivePermissionsParams = @{ + SQLServer = $ServerObject.ComputerNamePhysicalNetBIOS + SQLInstanceName = $ServerObject.ServiceName + LoginName = $ServerObject.ConnectionContext.TrueLogin + Permissions = @('CONTROL') + SecurableClass = 'LOGIN' + SecurableName = $SecurableName + } + $impersonatePermissionsPresent = Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams + if ($impersonatePermissionsPresent) + { + New-VerboseMessage -Message ( 'The login "{0}" has control permissions on the instance "{1}\{2}" for the login "{3}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName, $SecurableName ) + return $impersonatePermissionsPresent + } + else + { + New-VerboseMessage -Message ( 'The login "{0}" does not have control permissions on the instance "{1}\{2}" for the login "{3}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName, $SecurableName ) + } } New-VerboseMessage -Message ( 'The login "{0}" does not have any impersonate permissions required on the instance "{1}\{2}".' -f $testLoginEffectivePermissionsParams.LoginName, $testLoginEffectivePermissionsParams.SQLServer, $testLoginEffectivePermissionsParams.SQLInstanceName ) diff --git a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 index 7032fa81f..788ff9f74 100644 --- a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 +++ b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 @@ -607,7 +607,7 @@ REVERT' It 'Should throw the correct error when "MatchDatabaseOwner" is $true and the current login does not have impersonate permissions' { Mock -CommandName Test-ImpersonatePermissions -MockWith { $false } -Verifiable - { Set-TargetResource @mockSetTargetResourceParameters } | Should -Throw "The login '$([System.Security.Principal.WindowsIdentity]::GetCurrent().Name)' is missing impersonate any login, control server, or impersonate permissions in the instances 'Server2'." + { Set-TargetResource @mockSetTargetResourceParameters } | Should -Throw "The login '$([System.Security.Principal.WindowsIdentity]::GetCurrent().Name)' is missing impersonate any login, control server, impersonate login, or control login permissions in the instances 'Server2'." Assert-MockCalled -CommandName Add-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly -ParameterFilter { $InputObject.PrimaryReplicaServerName -eq 'Server1' -and $InputObject.LocalReplicaRole -eq 'Primary' } Assert-MockCalled -CommandName Add-SqlAvailabilityDatabase -Scope It -Times 0 -Exactly -ParameterFilter { $InputObject.PrimaryReplicaServerName -eq 'Server1' -and $InputObject.LocalReplicaRole -eq 'Secondary' } From ea96077af970407b946fe4a57693e6461ecfda13 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sun, 3 Mar 2019 22:34:16 +0800 Subject: [PATCH 26/30] Add tests to "Testing Test-LoginEffectivePermissions" section These cover the alternate code path inside where a different Invoke-Query is issued for a LOGIN object (the new tests) as opposed to a SERVER object (the old tests). --- Tests/Unit/SqlServerDSCHelper.Tests.ps1 | 81 +++++++++++++++++++------ 1 file changed, 62 insertions(+), 19 deletions(-) diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index ba9029c09..0a130f39c 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -681,68 +681,111 @@ InModuleScope $script:helperModuleName { Describe "Testing Test-LoginEffectivePermissions" { - $mockAllPermissionsPresent = @( + $mockAllServerPermissionsPresent = @( 'Connect SQL', 'Alter Any Availability Group', 'View Server State' ) - $mockPermissionsMissing = @( + $mockServerPermissionsMissing = @( 'Connect SQL', 'View Server State' ) - $mockInvokeQueryClusterServicePermissionsSet = @() # Will be set dynamically in the check + $mockAllLoginPermissionsPresent = @( + 'View Definition', + 'Impersonate' + ) + + $mockLoginPermissionsMissing = @( + 'View Definition' + ) + + $mockInvokeQueryPermissionsSet = @() # Will be set dynamically in the check - $mockInvokeQueryClusterServicePermissionsResult = { + $mockInvokeQueryPermissionsResult = { return New-Object -TypeName PSObject -Property @{ Tables = @{ Rows = @{ - permission_name = $mockInvokeQueryClusterServicePermissionsSet + permission_name = $mockInvokeQueryPermissionsSet } } } } - $testLoginEffectivePermissionsParams = @{ + $testLoginEffectiveServerPermissionsParams = @{ SQLServer = 'Server1' SQLInstanceName = 'MSSQLSERVER' Login = 'NT SERVICE\ClusSvc' Permissions = @() } + $testLoginEffectiveLoginPermissionsParams = @{ + SQLServer = 'Server1' + SQLInstanceName = 'MSSQLSERVER' + Login = 'NT SERVICE\ClusSvc' + Permissions = @() + SecurableClass = 'LOGIN' + SecurableName = 'Login1' + } + BeforeEach { - Mock -CommandName Invoke-Query -MockWith $mockInvokeQueryClusterServicePermissionsResult -Verifiable + Mock -CommandName Invoke-Query -MockWith $mockInvokeQueryPermissionsResult -Verifiable } Context 'When all of the permissions are present' { + It 'Should return $true when the desired server permissions are present' { + $mockInvokeQueryPermissionsSet = $mockAllServerPermissionsPresent.Clone() + $testLoginEffectiveServerPermissionsParams.Permissions = $mockAllServerPermissionsPresent.Clone() - It 'Should return $true when the desired permissions are present' { - $mockInvokeQueryClusterServicePermissionsSet = $mockAllPermissionsPresent.Clone() - $testLoginEffectivePermissionsParams.Permissions = $mockAllPermissionsPresent.Clone() + Test-LoginEffectivePermissions @testLoginEffectiveServerPermissionsParams | Should -Be $true + + Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly + } - Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams | Should -Be $true + It 'Should return $true when the desired login permissions are present' { + $mockInvokeQueryPermissionsSet = $mockAllLoginPermissionsPresent.Clone() + $testLoginEffectiveLoginPermissionsParams.Permissions = $mockAllLoginPermissionsPresent.Clone() - Assert-MockCalled -CommandName Invoke-Query -Times 1 -Exactly + Test-LoginEffectivePermissions @testLoginEffectiveLoginPermissionsParams | Should -Be $true + + Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly } } Context 'When a permission is missing' { + It 'Should return $false when the desired server permissions are not present' { + $mockInvokeQueryPermissionsSet = $mockServerPermissionsMissing.Clone() + $testLoginEffectiveServerPermissionsParams.Permissions = $mockAllServerPermissionsPresent.Clone() + + Test-LoginEffectivePermissions @testLoginEffectiveServerPermissionsParams | Should -Be $false + + Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly + } + + It 'Should return $false when the specified login has no permissions assigned' { + $mockInvokeQueryPermissionsSet = @() + $testLoginEffectiveServerPermissionsParams.Permissions = $mockAllServerPermissionsPresent.Clone() + + Test-LoginEffectivePermissions @testLoginEffectiveServerPermissionsParams | Should -Be $false + + Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly + } - It 'Should return $false when the desired permissions are not present' { - $mockInvokeQueryClusterServicePermissionsSet = $mockPermissionsMissing.Clone() - $testLoginEffectivePermissionsParams.Permissions = $mockAllPermissionsPresent.Clone() + It 'Should return $false when the desired login permissions are not present' { + $mockInvokeQueryPermissionsSet = $mockLoginPermissionsMissing.Clone() + $testLoginEffectiveLoginPermissionsParams.Permissions = $mockAllLoginPermissionsPresent.Clone() - Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams | Should -Be $false + Test-LoginEffectivePermissions @testLoginEffectiveLoginPermissionsParams | Should -Be $false Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly } It 'Should return $false when the specified login has no permissions assigned' { - $mockInvokeQueryClusterServicePermissionsSet = @() - $testLoginEffectivePermissionsParams.Permissions = $mockAllPermissionsPresent.Clone() + $mockInvokeQueryPermissionsSet = @() + $testLoginEffectiveServerPermissionsParams.Permissions = $mockAllServerPermissionsPresent.Clone() - Test-LoginEffectivePermissions @testLoginEffectivePermissionsParams | Should -Be $false + Test-LoginEffectivePermissions @testLoginEffectiveServerPermissionsParams | Should -Be $false Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly } From fc30c32c5216954fb8304e3db23d3e4c24c93301 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Sun, 3 Mar 2019 23:46:27 +0800 Subject: [PATCH 27/30] Add tests for Test-ImpersonatePermissions --- Tests/Unit/SqlServerDSCHelper.Tests.ps1 | 67 ++++++++++++++++++++++--- 1 file changed, 60 insertions(+), 7 deletions(-) diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index 0a130f39c..555048eb8 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -1272,6 +1272,22 @@ InModuleScope $script:helperModuleName { } } + $mockTestLoginEffectivePermissions_ImpersonateAnyLogin_ParameterFilter = { + $Permissions -eq @('IMPERSONATE ANY LOGIN') + } + + $mockTestLoginEffectivePermissions_ControlServer_ParameterFilter = { + $Permissions -eq @('CONTROL SERVER') + } + + $mockTestLoginEffectivePermissions_ImpersonateLogin_ParameterFilter = { + $Permissions -eq @('IMPERSONATE') + } + + $mockTestLoginEffectivePermissions_ControlLogin_ParameterFilter = { + $Permissions -eq @('CONTROL') + } + Describe 'Testing Test-ImpersonatePermissions' { $mockConnectionContextObject = New-Object -TypeName Microsoft.SqlServer.Management.Smo.ServerConnection $mockConnectionContextObject.TrueLogin = 'Login1' @@ -1281,23 +1297,60 @@ InModuleScope $script:helperModuleName { $mockServerObject.ServiceName = 'MSSQLSERVER' $mockServerObject.ConnectionContext = $mockConnectionContextObject + BeforeEach { + Mock -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ImpersonateAnyLogin_ParameterFilter -MockWith { $false } -Verifiable + Mock -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ControlServer_ParameterFilter -MockWith { $false } -Verifiable + Mock -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ImpersonateLogin_ParameterFilter -MockWith { $false } -Verifiable + Mock -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ControlLogin_ParameterFilter -MockWith { $false } -Verifiable + } + Context 'When impersonate permissions are present for the login' { - Mock -CommandName Test-LoginEffectivePermissions -MockWith { $true } + It 'Should return true when the impersonate any login permissions are present for the login' { + Mock -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ImpersonateAnyLogin_ParameterFilter -MockWith { $true } -Verifiable + Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $true - It 'Should return true when the impersonate permissions are present for the login' { + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ImpersonateAnyLogin_ParameterFilter -Scope It -Times 1 -Exactly + } + + It 'Should return true when the control server permissions are present for the login' { + Mock -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ControlServer_ParameterFilter -MockWith { $true } -Verifiable Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $true - Assert-MockCalled -CommandName Test-LoginEffectivePermissions -Scope It -Times 1 -Exactly + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ControlServer_ParameterFilter -Scope It -Times 1 -Exactly + } + + It 'Should return true when the impersonate login permissions are present for the login' { + Mock -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ImpersonateLogin_ParameterFilter -MockWith { $true } -Verifiable + Test-ImpersonatePermissions -ServerObject $mockServerObject -SecurableName 'Login1' | Should -Be $true + + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ImpersonateLogin_ParameterFilter -Scope It -Times 1 -Exactly + } + + It 'Should return true when the control login permissions are present for the login' { + Mock -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ControlLogin_ParameterFilter -MockWith { $true } -Verifiable + Test-ImpersonatePermissions -ServerObject $mockServerObject -SecurableName 'Login1' | Should -Be $true + + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ControlLogin_ParameterFilter -Scope It -Times 1 -Exactly } } Context 'When impersonate permissions are missing for the login' { - Mock -CommandName Test-LoginEffectivePermissions -MockWith { $false } -Verifiable - - It 'Should return false when the impersonate permissions are missing for the login' { + It 'Should return false when the server permissions are missing for the login' { Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $false - Assert-MockCalled -CommandName Test-LoginEffectivePermissions -Scope It -Times 2 -Exactly + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ImpersonateAnyLogin_ParameterFilter -Scope It -Times 1 -Exactly + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ControlServer_ParameterFilter -Scope It -Times 1 -Exactly + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ImpersonateLogin_ParameterFilter -Scope It -Times 0 -Exactly + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ControlLogin_ParameterFilter -Scope It -Times 0 -Exactly + } + + It 'Should return false when the login permissions are missing for the login' { + Test-ImpersonatePermissions -ServerObject $mockServerObject -SecurableName 'Login1' | Should -Be $false + + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ImpersonateAnyLogin_ParameterFilter -Scope It -Times 1 -Exactly + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ControlServer_ParameterFilter -Scope It -Times 1 -Exactly + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ImpersonateLogin_ParameterFilter -Scope It -Times 1 -Exactly + Assert-MockCalled -CommandName Test-LoginEffectivePermissions -ParameterFilter $mockTestLoginEffectivePermissions_ControlLogin_ParameterFilter -Scope It -Times 1 -Exactly } } } From 1aabbd0a809c6323ed43a825bfc7216b6c9ddebb Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 5 Mar 2019 10:18:06 +0800 Subject: [PATCH 28/30] Fix duplicate test --- Tests/Unit/SqlServerDSCHelper.Tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index 555048eb8..02fc466cc 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -783,9 +783,9 @@ InModuleScope $script:helperModuleName { It 'Should return $false when the specified login has no permissions assigned' { $mockInvokeQueryPermissionsSet = @() - $testLoginEffectiveServerPermissionsParams.Permissions = $mockAllServerPermissionsPresent.Clone() + $testLoginEffectiveLoginPermissionsParams.Permissions = $mockAllLoginPermissionsPresent.Clone() - Test-LoginEffectivePermissions @testLoginEffectiveServerPermissionsParams | Should -Be $false + Test-LoginEffectivePermissions @testLoginEffectiveLoginPermissionsParams | Should -Be $false Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly } From 35a908ef19af621221fb7c14771667575e598b0b Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 5 Mar 2019 10:20:52 +0800 Subject: [PATCH 29/30] Rename test for clarity --- Tests/Unit/SqlServerDSCHelper.Tests.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index 02fc466cc..451d7e6ff 100644 --- a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 +++ b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 @@ -763,7 +763,7 @@ InModuleScope $script:helperModuleName { Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly } - It 'Should return $false when the specified login has no permissions assigned' { + It 'Should return $false when the specified login has no server permissions assigned' { $mockInvokeQueryPermissionsSet = @() $testLoginEffectiveServerPermissionsParams.Permissions = $mockAllServerPermissionsPresent.Clone() @@ -781,7 +781,7 @@ InModuleScope $script:helperModuleName { Assert-MockCalled -CommandName Invoke-Query -Scope It -Times 1 -Exactly } - It 'Should return $false when the specified login has no permissions assigned' { + It 'Should return $false when the specified login has no login permissions assigned' { $mockInvokeQueryPermissionsSet = @() $testLoginEffectiveLoginPermissionsParams.Permissions = $mockAllLoginPermissionsPresent.Clone() From 3f60d7c94f3ca47e82ccef60180d643cf0e34cd8 Mon Sep 17 00:00:00 2001 From: Cody Konior Date: Tue, 5 Mar 2019 10:28:01 +0800 Subject: [PATCH 30/30] Update help text to match order of terms. --- DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 | 8 ++++---- .../MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 index 393f75df7..58381970c 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 +++ b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 @@ -138,8 +138,8 @@ 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 login, impersonate - any login, or control server permissions. + as a login on all replicas and that the PsDscRunAsCredential has impersonate any login, control + server, impersonate login, or control login permissions. If set to $false, the owner of the database will be the PsDscRunAsCredential. @@ -631,8 +631,8 @@ 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 login, impersonate - any login, or control server permissions. + as a login on all replicas and that the PsDscRunAsCredential has impersonate any login, control + server, impersonate login, or control login permissions. If set to $false, the owner of the database will be the PsDscRunAsCredential. diff --git a/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt b/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt index 90497613f..214e0e88c 100644 --- a/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt +++ b/DSCResources/MSFT_SqlAGDatabase/en-US/about_SqlAGDatabase.help.txt @@ -40,8 +40,8 @@ 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 login, impersonate - any login, or control server permissions. + as a login on all replicas and that the PsDscRunAsCredential has impersonate any login, control + server, impersonate login, or control login permissions. If set to $false, the owner of the database will be the PsDscRunAsCredential.