diff --git a/CHANGELOG.md b/CHANGELOG.md index b2a04d91f..0c71fb459 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -132,6 +132,10 @@ - Fix unit tests that didn't mock some of the calls. It no longer fail when a SQL Server installation is not present on the node running the unit test ([issue #983](https://github.com/PowerShell/SqlServerDsc/issues/983)). +- 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.0.0.0 diff --git a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 index 6660f8fec..3dcd96501 100644 --- a/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 +++ b/DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1 @@ -232,33 +232,33 @@ 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 intended secondary replicas for each database + 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 -SQLServer $availabilityGroupReplica.Name + $impersonatePermissionsStatus.Add( + $availabilityGroupReplica.Name, + ( Test-ImpersonatePermissions -ServerObject $currentAvailabilityGroupReplicaServerObject -Securable $databaseObject.Owner ) + ) + } - foreach ( $databaseToAddToAvailabilityGroup in $databasesToAddToAvailabilityGroup ) - { - $databaseObject = $primaryServerObject.Databases[$databaseToAddToAvailabilityGroup] + 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 ) + } + } <# Verify the prerequisites prior to joining the database to the availability group @@ -608,7 +608,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/SqlServerDscHelper.psm1 b/SqlServerDscHelper.psm1 index 059a2d35f..bd5ca6bf8 100644 --- a/SqlServerDscHelper.psm1 +++ b/SqlServerDscHelper.psm1 @@ -1079,7 +1079,14 @@ function Test-LoginEffectivePermissions [Parameter(Mandatory = $true)] [System.String[]] - $Permissions + $Permissions, + + [ValidateSet('APPLICATION ROLE', 'ASSEMBLY', 'ASYMMETRIC KEY', 'CERTIFICATE', 'CONTRACT', 'DATABASE', 'ENDPOINT', 'FULLTEXT CATALOG', + 'LOGIN', 'MESSAGE TYPE', 'OBJECT', 'REMOTE SERVICE BINDING', 'ROLE', 'ROUTE', 'SCHEMA', 'SERVER', 'SERVICE', 'SYMMETRIC KEY', + 'TYPE', 'USER', 'XML SCHEMA COLLECTION')] + [System.String] $SecurableClass = 'SERVER', + + [System.String] $Securable ) # Assume the permissions are not present @@ -1092,13 +1099,21 @@ function Test-LoginEffectivePermissions WithResults = $true } - $queryToGetEffectivePermissionsForLogin = " - EXECUTE AS LOGIN = '$LoginName' - SELECT DISTINCT permission_name - FROM fn_my_permissions(null,'SERVER') - REVERT - " - + if ($null -eq $Securable) { + $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('$Securable', '$SecurableClass') + REVERT + " + } Write-Verbose -Message ($script:localizedData.GetEffectivePermissionForLogin -f $LoginName, $sqlInstanceName) -Verbose $loginEffectivePermissionsResult = Invoke-Query @invokeQueryParameters -Query $queryToGetEffectivePermissionsForLogin @@ -1233,6 +1248,10 @@ function Get-PrimaryReplicaServerObject .PARAMETER ServerObject The server object on which to perform the test. + + .PARAMETER Securable + If set then impersonate permission on tihs specific securable (login) is also checked. + #> function Test-ImpersonatePermissions { @@ -1240,18 +1259,47 @@ function Test-ImpersonatePermissions ( [Parameter(Mandatory = $true)] [Microsoft.SqlServer.Management.Smo.Server] - $ServerObject + $ServerObject, + + [System.String] $Securable ) + $impersonatePermissionsPresent = $false + + # Only exists on this is SQL 2014 or newer $testLoginEffectivePermissionsParams = @{ SQLServer = $ServerObject.ComputerNamePhysicalNetBIOS SQLInstanceName = $ServerObject.ServiceName LoginName = $ServerObject.ConnectionContext.TrueLogin Permissions = @('IMPERSONATE ANY LOGIN') } - $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($Securable) ) { + # Check for login-specific impersonation permissions + $testLoginEffectivePermissionsParams = @{ + SQLServer = $ServerObject.ComputerNamePhysicalNetBIOS + SQLInstanceName = $ServerObject.ServiceName + LoginName = $ServerObject.ConnectionContext.TrueLogin + Permissions = @('IMPERSONATE') + SecurableClass = 'LOGIN' + Securable = $Securable + } + $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 ) diff --git a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 index 43f35edb3..78cca14cf 100644 --- a/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 +++ b/Tests/Unit/MSFT_SqlAGDatabase.Tests.ps1 @@ -83,11 +83,13 @@ try #region mock names - $mockServerObjectDomainInstanceName = 'Server1' - $mockPrimaryServerObjectDomainInstanceName = 'Server2' - $mockAvailabilityGroupObjectName = 'AvailabilityGroup1' - $mockAvailabilityGroupWithoutDatabasesObjectName = 'AvailabilityGroupWithoutDatabases' - $mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServerName = 'AvailabilityGroup2' + $mockTrueLogin = 'Login1' + $mockDatabaseOwner = 'DatabaseOwner1' + $mockServerObjectDomainInstanceName = 'Server1' + $mockPrimaryServerObjectDomainInstanceName = 'Server2' + $mockAvailabilityGroupObjectName = 'AvailabilityGroup1' + $mockAvailabilityGroupWithoutDatabasesObjectName = 'AvailabilityGroupWithoutDatabases' + $mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServerName = 'AvailabilityGroup2' #endregion mock names @@ -295,9 +297,31 @@ try Files = @{ FileName = ( [IO.Path]::Combine( $mockDataFilePathIncorrect, "$($mockPresentDatabaseName).mdf" ) ) } + $newDatabaseObject.Owner = $mockDatabaseOwner + + # Add the database object to the database collection + $mockDatabaseObjects.Add($newDatabaseObject) } - $newDatabaseObject.LogFiles = @{ - FileName = ( [IO.Path]::Combine( $mockLogFilePathIncorrect, "$($mockPresentDatabaseName).ldf" ) ) + $mockDatabaseObjects.Add($mockMasterDatabaseObject1) + + $mockDatabaseObjectsWithIncorrectFileNames = New-Object -TypeName Microsoft.SqlServer.Management.Smo.DatabaseCollection + foreach ( $mockPresentDatabaseName in $mockPresentDatabaseNames ) + { + $newDatabaseObject = New-Object -TypeName Microsoft.SqlServer.Management.Smo.Database + $newDatabaseObject.Name = $mockPresentDatabaseName + $newDatabaseObject.FileGroups = @{ + Name = 'PRIMARY' + Files = @{ + FileName = ( [IO.Path]::Combine( $mockDataFilePathIncorrect, "$($mockPresentDatabaseName).mdf" ) ) + } + } + $newDatabaseObject.LogFiles = @{ + FileName = ( [IO.Path]::Combine( $mockLogFilePathIncorrect, "$($mockPresentDatabaseName).ldf" ) ) + } + $newDatabaseObject.Owner = $mockDatabaseOwner + + # Add the database object to the database collection + $mockDatabaseObjectsWithIncorrectFileNames.Add($newDatabaseObject) } # Add the database object to the database collection @@ -308,33 +332,39 @@ try #region Server mocks - $mockBadServerObject = New-Object -TypeName Object - - $mockServerObject = New-Object -TypeName Microsoft.SqlServer.Management.Smo.Server - $mockServerObject.AvailabilityGroups = New-Object -TypeName Microsoft.SqlServer.Management.Smo.AvailabilityGroupCollection - $mockServerObject.AvailabilityGroups.Add($mockAvailabilityGroupObject.Clone()) - $mockServerObject.AvailabilityGroups.Add($mockAvailabilityGroupWithoutDatabasesObject.Clone()) - $mockServerObject.AvailabilityGroups.Add($mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServer.Clone()) - $mockServerObject.Databases = $mockDatabaseObjects - $mockServerObject.DomainInstanceName = $mockServerObjectDomainInstanceName - $mockServerObject.NetName = $mockServerObjectDomainInstanceName - $mockServerObject.ServiceName = 'MSSQLSERVER' - $mockServerObject.AvailabilityGroups[$mockAvailabilityGroupObject.Name].LocalReplicaRole = 'Primary' - $mockServerObject.AvailabilityGroups[$mockAvailabilityGroupWithoutDatabasesObject.Name].LocalReplicaRole = 'Primary' - $mockServerObject.AvailabilityGroups[$mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServer.Name].LocalReplicaRole = 'Secondary' - - $mockServer2Object = New-Object -TypeName Microsoft.SqlServer.Management.Smo.Server - $mockServer2Object.AvailabilityGroups = New-Object -TypeName Microsoft.SqlServer.Management.Smo.AvailabilityGroupCollection - $mockServer2Object.AvailabilityGroups.Add($mockAvailabilityGroupObject.Clone()) - $mockServer2Object.AvailabilityGroups.Add($mockAvailabilityGroupWithoutDatabasesObject.Clone()) - $mockServer2Object.AvailabilityGroups.Add($mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServer.Clone()) - $mockServer2Object.Databases = $mockDatabaseObjects - $mockServer2Object.DomainInstanceName = $mockPrimaryServerObjectDomainInstanceName - $mockServer2Object.NetName = $mockPrimaryServerObjectDomainInstanceName - $mockServer2Object.ServiceName = 'MSSQLSERVER' - $mockServer2Object.AvailabilityGroups[$mockAvailabilityGroupObject.Name].LocalReplicaRole = 'Secondary' - $mockServer2Object.AvailabilityGroups[$mockAvailabilityGroupWithoutDatabasesObject.Name].LocalReplicaRole = 'Secondary' - $mockServer2Object.AvailabilityGroups[$mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServer.Name].LocalReplicaRole = 'Primary' + $mockBadServerObject = New-Object -TypeName Object + + $mockServerObject = New-Object -TypeName Microsoft.SqlServer.Management.Smo.Server + $mockServerObject.AvailabilityGroups = New-Object -TypeName Microsoft.SqlServer.Management.Smo.AvailabilityGroupCollection + $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 + $mockServerObject.ServiceName = 'MSSQLSERVER' + $mockServerObject.AvailabilityGroups[$mockAvailabilityGroupObject.Name].LocalReplicaRole = 'Primary' + $mockServerObject.AvailabilityGroups[$mockAvailabilityGroupWithoutDatabasesObject.Name].LocalReplicaRole = 'Primary' + $mockServerObject.AvailabilityGroups[$mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServer.Name].LocalReplicaRole = 'Secondary' + + $mockServer2Object = New-Object -TypeName Microsoft.SqlServer.Management.Smo.Server + $mockServer2Object.AvailabilityGroups = New-Object -TypeName Microsoft.SqlServer.Management.Smo.AvailabilityGroupCollection + $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 + $mockServer2Object.ServiceName = 'MSSQLSERVER' + $mockServer2Object.AvailabilityGroups[$mockAvailabilityGroupObject.Name].LocalReplicaRole = 'Secondary' + $mockServer2Object.AvailabilityGroups[$mockAvailabilityGroupWithoutDatabasesObject.Name].LocalReplicaRole = 'Secondary' + $mockServer2Object.AvailabilityGroups[$mockAvailabilityGroupObjectWithPrimaryReplicaOnAnotherServer.Name].LocalReplicaRole = 'Primary' #endregion Server mocks diff --git a/Tests/Unit/SqlServerDSCHelper.Tests.ps1 b/Tests/Unit/SqlServerDSCHelper.Tests.ps1 index 31aaafb39..a3798c471 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 any login permission is present'{ + $mockInvokeQueryImpersonatePermissionsSet = $mockImpersonateAnyLoginPermissionsPresent.Clone() - It 'Should return true when the impersonate permissions are present for the login'{ Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $true + # It's called once because it short-circuits with the IMPERSONATE ANY LOGIN persmission is present + Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 1 -Scope It + } + + It 'Should return true when the control server permission is present'{ + $mockInvokeQueryImpersonatePermissionsSet = $mockControlServerPermissionsPresent.Clone() - Assert-MockCalled -CommandName Test-LoginEffectivePermissions -Scope It -Times 1 -Exactly + Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $true + # It's called twice, once for IMPERSONATE ANY LOGIN and again for CONTROL SERVER + Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 2 -Scope It + } + + It 'Should return true when the impersonate login permission is present'{ + $mockInvokeQueryImpersonatePermissionsSet = $mockImpersonateLoginPermissionsPresent.Clone() + + Test-ImpersonatePermissions -ServerObject $mockServerObject -Securable 'DatabaseOwner1' | Should -Be $true + # It's called three times, IMPERSONATE ANY, CONTROL SERVER, and then a specific IMPERSONATE LOGIN + Assert-MockCalled -CommandName Invoke-Query -Exactly -Times 3 -Scope It } } 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 + $mockInvokeQueryImpersonatePermissionsSet = $mockPermissionsMissing.Clone() - Assert-MockCalled -CommandName Test-LoginEffectivePermissions -Scope It -Times 1 -Exactly + Test-ImpersonatePermissions -ServerObject $mockServerObject | Should -Be $false } } }