Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SqlAGDatabase: Fix impersonate permission test #1295

Merged
merged 32 commits into from
Mar 5, 2019
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
a560d1e
Framework
codykonior Feb 19, 2019
f695c43
Doco
codykonior Feb 19, 2019
501b43c
Add extra permissions descriptions to help text and verbose output
codykonior Feb 19, 2019
3b90064
Make mocks more realistic by including database owners and connections
codykonior Feb 19, 2019
81f3aa7
Add tests
codykonior Feb 19, 2019
4c6848f
Make tests pass and ensure code coverage
codykonior Feb 19, 2019
d9456a3
Add code coverage test for New-TerminatingError
codykonior Feb 19, 2019
4754adf
Update changelog
codykonior Feb 19, 2019
0c0f614
Add permissions details to error message
codykonior Feb 19, 2019
4e23a8b
Merge branch 'dev' of https://github.com/codykonior/SqlServerDsc into…
codykonior Feb 26, 2019
1575055
Allow log restore to work with EXECUTE AS
codykonior Feb 27, 2019
63f9e2e
Move changlog section to unreleased
codykonior Feb 27, 2019
43ab2ba
Fix this test text
codykonior Feb 27, 2019
6f55899
Fix mocked ConnectionContext TypeName. Remove Restore-SqlDatabase mock
codykonior Feb 27, 2019
3102c38
Rename ConnectionContext class to ServerConnection
codykonior Mar 2, 2019
5a62f72
.
codykonior Mar 2, 2019
011cf10
.
codykonior Mar 2, 2019
2f71d59
Spelling
codykonior Mar 2, 2019
998d5dc
Change PSDscRunAsCredential to PsDscRunAsCredential
codykonior Mar 3, 2019
9b49767
Add internal help for helper function
codykonior Mar 3, 2019
074c6a3
Remove extra line of code
codykonior Mar 3, 2019
afc7f17
Add more verbose output
codykonior Mar 3, 2019
8789567
Revert test for Test-ImpersonatePermissions so it only checks
codykonior Mar 3, 2019
cac591f
Phrasing
codykonior Mar 3, 2019
6a87852
.
codykonior Mar 3, 2019
0a21dba
Add control login as equivalent to impersonate login
codykonior Mar 3, 2019
809e380
Merge branch 'dev' into cody-konior/matchdatabaseowner
codykonior Mar 3, 2019
ea96077
Add tests to "Testing Test-LoginEffectivePermissions" section
codykonior Mar 3, 2019
fc30c32
Add tests for Test-ImpersonatePermissions
codykonior Mar 3, 2019
1aabbd0
Fix duplicate test
codykonior Mar 5, 2019
35a908e
Rename test for clarity
codykonior Mar 5, 2019
3f60d7c
Update help text to match order of terms.
codykonior Mar 5, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@
- Changed the logic of 'Build the argument string to be passed to setup' to
not quote the value if root directory is specified
([issue #1254](https://github.com/PowerShell/SqlServerDsc/issues/1254)).
- Changes to SqlAGDatabase
- 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
the output even if the "missing localized message" localized message is
also missing.

## 12.3.0.0

Expand Down
88 changes: 58 additions & 30 deletions DSCResources/MSFT_SqlAGDatabase/MSFT_SqlAGDatabase.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -138,11 +138,12 @@ function Get-TargetResource
.PARAMETER MatchDatabaseOwner
If set to $true, this ensures the database owner of the database on the primary replica is the
owner of the database on all secondary replicas. This requires the database owner is available
as a login on all replicas and that the PSDscRunAsCredential has impersonate permissions.
as a login on all replicas and that the PsDscRunAsCredential has impersonate login, impersonate
any login, or control server permissions.

If set to $false, the owner of the database will be the PSDscRunAsCredential.
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.
Expand Down Expand Up @@ -232,27 +233,32 @@ function Set-TargetResource
# Get only the secondary replicas. Some tests do not need to be performed on the primary replica
$secondaryReplicas = $availabilityGroup.AvailabilityReplicas | Where-Object -FilterScript { $_.Role -ne 'Primary' }

# Ensure the appropriate permissions are in place on all the replicas
if ( $MatchDatabaseOwner )
foreach ( $databaseToAddToAvailabilityGroup in $databasesToAddToAvailabilityGroup )
{
$impersonatePermissionsStatus = @{}
$databaseObject = $primaryServerObject.Databases[$databaseToAddToAvailabilityGroup]

foreach ( $availabilityGroupReplica in $secondaryReplicas )
# Ensure the appropriate permissions are in place on all the replicas
if ( $MatchDatabaseOwner )
{
$currentAvailabilityGroupReplicaServerObject = Connect-SQL -ServerName $availabilityGroupReplica.Name
$impersonatePermissionsStatus.Add(
$availabilityGroupReplica.Name,
( Test-ImpersonatePermissions -ServerObject $currentAvailabilityGroupReplicaServerObject )
)
}
$impersonatePermissionsStatus = @{}

if ( $impersonatePermissionsStatus.Values -contains $false )
{
$impersonatePermissionsMissingParameters = @(
[System.Security.Principal.WindowsIdentity]::GetCurrent().Name,
( ( $impersonatePermissionsStatus.GetEnumerator() | Where-Object -FilterScript { -not $_.Value } | Select-Object -ExpandProperty Key ) -join ', ' )
)
throw ($script:localizedData.ImpersonatePermissionsMissing -f $impersonatePermissionsMissingParameters )
foreach ( $availabilityGroupReplica in $secondaryReplicas )
{
$currentAvailabilityGroupReplicaServerObject = Connect-SQL -ServerName $availabilityGroupReplica.Name
$impersonatePermissionsStatus.Add(
$availabilityGroupReplica.Name,
( Test-ImpersonatePermissions -ServerObject $currentAvailabilityGroupReplicaServerObject -Securable $databaseObject.Owner )
)
}

if ( $impersonatePermissionsStatus.Values -contains $false )
{
$impersonatePermissionsMissingParameters = @(
[System.Security.Principal.WindowsIdentity]::GetCurrent().Name,
( ( $impersonatePermissionsStatus.GetEnumerator() | Where-Object -FilterScript { -not $_.Value } | Select-Object -ExpandProperty Key ) -join ', ' )
)
throw ($script:localizedData.ImpersonatePermissionsMissing -f $impersonatePermissionsMissingParameters )
}
}
}

Expand Down Expand Up @@ -481,15 +487,36 @@ function Set-TargetResource
$restoreDatabaseQueryStringBuilder.Append($databaseFullBackupFile) | Out-Null
$restoreDatabaseQueryStringBuilder.AppendLine('''') | Out-Null
$restoreDatabaseQueryStringBuilder.Append('WITH NORECOVERY') | Out-Null
if ( $MatchDatabaseOwner )
{
$restoreDatabaseQueryStringBuilder.AppendLine() | Out-Null
$restoreDatabaseQueryStringBuilder.Append('REVERT') | Out-Null
}
$restoreDatabaseQueryString = $restoreDatabaseQueryStringBuilder.ToString()

# Build the parameters to restore the transaction log
$restoreSqlDatabaseLogParameters = @{
Database = $databaseToAddToAvailabilityGroup
BackupFile = $databaseLogBackupFile
RestoreAction = 'Log'
NoRecovery = $true
# Need to restore the database with a query in order to impersonate the correct login
$restoreLogQueryStringBuilder = New-Object -TypeName System.Text.StringBuilder

if ( $MatchDatabaseOwner )
{
$restoreLogQueryStringBuilder.Append('EXECUTE AS LOGIN = ''') | Out-Null
$restoreLogQueryStringBuilder.Append($databaseObject.Owner) | Out-Null
$restoreLogQueryStringBuilder.AppendLine('''') | Out-Null
}

$restoreLogQueryStringBuilder.Append('RESTORE DATABASE [') | Out-Null
$restoreLogQueryStringBuilder.Append($databaseToAddToAvailabilityGroup) | Out-Null
$restoreLogQueryStringBuilder.AppendLine(']') | Out-Null
$restoreLogQueryStringBuilder.Append('FROM DISK = ''') | Out-Null
$restoreLogQueryStringBuilder.Append($databaseLogBackupFile) | Out-Null
$restoreLogQueryStringBuilder.AppendLine('''') | Out-Null
$restoreLogQueryStringBuilder.Append('WITH NORECOVERY') | Out-Null
if ( $MatchDatabaseOwner )
{
$restoreLogQueryStringBuilder.AppendLine() | Out-Null
$restoreLogQueryStringBuilder.Append('REVERT') | Out-Null
}
$restoreLogQueryString = $restoreLogQueryStringBuilder.ToString()

try
{
Expand All @@ -502,7 +529,7 @@ function Set-TargetResource

# Restore the database
Invoke-Query -SQLServer $currentAvailabilityGroupReplicaServerObject.NetName -SQLInstanceName $currentAvailabilityGroupReplicaServerObject.ServiceName -Database master -Query $restoreDatabaseQueryString
Restore-SqlDatabase -InputObject $currentAvailabilityGroupReplicaServerObject @restoreSqlDatabaseLogParameters
Invoke-Query -SQLServer $currentAvailabilityGroupReplicaServerObject.NetName -SQLInstanceName $currentAvailabilityGroupReplicaServerObject.ServiceName -Database master -Query $restoreLogQueryString

# Add the database to the Availability Group
Add-SqlAvailabilityDatabase -InputObject $currentReplicaAvailabilityGroupObject -Database $databaseToAddToAvailabilityGroup
Expand Down Expand Up @@ -604,11 +631,12 @@ function Set-TargetResource
.PARAMETER MatchDatabaseOwner
If set to $true, this ensures the database owner of the database on the primary replica is the
owner of the database on all secondary replicas. This requires the database owner is available
as a login on all replicas and that the PSDscRunAsCredential has impersonate permissions.
as a login on all replicas and that the PsDscRunAsCredential has impersonate login, impersonate
any login, or control server permissions.

If set to $false, the owner of the database will be the PSDscRunAsCredential.
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 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;
};
Original file line number Diff line number Diff line change
Expand Up @@ -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, 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,12 @@ PARAMETER Force
PARAMETER MatchDatabaseOwner
If set to $true, this ensures the database owner of the database on the primary replica is the
owner of the database on all secondary replicas. This requires the database owner is available
as a login on all replicas and that the PSDscRunAsCredential has impersonate permissions.
as a login on all replicas and that the PsDscRunAsCredential has impersonate login, impersonate
any login, or control server permissions.

If set to $false, the owner of the database will be the PSDscRunAsCredential.
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.
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,11 @@ group.
Availability Group. This parameter is ignored when 'Ensure' is 'Absent'.
* **`[Boolean]` MatchDatabaseOwner** _(Write)_: If set to $true, this ensures the
database owner of the database on the primary replica is the owner of the database
on all secondary replicas. This requires the database owner is available as a
login on all replicas and that the PsDscRunAsCredential has impersonate permissions.
on all secondary replicas. This requires the database owner is available
as a login on all replicas and that the PsDscRunAsCredential has impersonate 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 '$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.
Expand Down
Loading