Skip to content

Commit da50dc9

Browse files
johljujohlju
committed
Add Refresh parameter to Get-SqlDscDatabasePermission and Test-SqlDscIsDatabasePrincipal for improved principal visibility
1 parent 6d8645e commit da50dc9

File tree

6 files changed

+221
-3
lines changed

6 files changed

+221
-3
lines changed

‎CHANGELOG.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
8080
correctly in real environments
8181
[issue #2214](https://github.com/dsccommunity/SqlServerDsc/issues/2214).
8282

83+
### Changed
84+
85+
- `Test-SqlDscIsDatabasePrincipal` and `Get-SqlDscDatabasePermission`
86+
- Added `Refresh` parameter to refresh SMO collections before checking
87+
database principals, addressing issues with custom database roles created
88+
via T-SQL that aren't immediately visible to SMO. The refresh logic is
89+
optimized to only refresh collections that will be used based on exclude
90+
parameters, improving performance on databases with large numbers of principals
91+
([issue #2221](https://github.com/dsccommunity/SqlServerDsc/issues/2221)).
92+
8393
### Fixed
8494

8595
- `Add-SqlDscTraceFlag` and `Remove-SqlDscTraceFlag`

‎source/Public/Get-SqlDscDatabasePermission.ps1‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@
1515
Specifies the name of the database principal for which the permissions are
1616
returned.
1717
18+
.PARAMETER Refresh
19+
Specifies that the database's principal collections (Users, Roles, and
20+
ApplicationRoles) should be refreshed before testing if the principal exists.
21+
This is helpful when principals could have been modified outside of the
22+
**ServerObject**, for example through T-SQL. But on databases with a large
23+
amount of principals it might be better to make sure the **ServerObject**
24+
is recent enough.
25+
1826
.OUTPUTS
1927
[Microsoft.SqlServer.Management.Smo.DatabasePermissionInfo[]]
2028
@@ -24,6 +32,13 @@
2432
2533
Get the permissions for the principal 'MyPrincipal'.
2634
35+
.EXAMPLE
36+
$serverInstance = Connect-SqlDscDatabaseEngine
37+
Get-SqlDscDatabasePermission -ServerObject $serverInstance -DatabaseName 'MyDatabase' -Name 'MyPrincipal' -Refresh
38+
39+
Get the permissions for the principal 'MyPrincipal'. The database's principal
40+
collections are refreshed before testing if the principal exists.
41+
2742
.NOTES
2843
This command excludes fixed roles like _db_datareader_ by default, and will
2944
always return `$null` if a fixed role is specified as **Name**.
@@ -52,7 +67,11 @@ function Get-SqlDscDatabasePermission
5267

5368
[Parameter(Mandatory = $true)]
5469
[System.String]
55-
$Name
70+
$Name,
71+
72+
[Parameter()]
73+
[System.Management.Automation.SwitchParameter]
74+
$Refresh
5675
)
5776

5877
# cSpell: ignore GSDDP
@@ -76,6 +95,11 @@ function Get-SqlDscDatabasePermission
7695
ExcludeFixedRoles = $true
7796
}
7897

98+
if ($Refresh.IsPresent)
99+
{
100+
$testSqlDscIsDatabasePrincipalParameters.Refresh = $true
101+
}
102+
79103
$isDatabasePrincipal = Test-SqlDscIsDatabasePrincipal @testSqlDscIsDatabasePrincipalParameters
80104

81105
if ($isDatabasePrincipal)

‎source/Public/Test-SqlDscIsDatabasePrincipal.ps1‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,16 @@
2828
Specifies that fixed application roles should not be evaluated for the
2929
specified name.
3030
31+
.PARAMETER Refresh
32+
Specifies that the database's principal collections (Users, Roles, and
33+
ApplicationRoles) should be refreshed before testing if the principal exists.
34+
This is helpful when principals could have been modified outside of the
35+
**ServerObject**, for example through T-SQL. But on databases with a large
36+
amount of principals it might be better to make sure the **ServerObject**
37+
is recent enough. When exclude parameters are specified (e.g., **ExcludeUsers**,
38+
**ExcludeRoles**, **ExcludeApplicationRoles**), only the collections that will
39+
be used are refreshed to improve performance.
40+
3141
.OUTPUTS
3242
[System.Boolean]
3343
@@ -60,6 +70,13 @@
6070
Test-SqlDscIsDatabasePrincipal -ServerObject $serverInstance -DatabaseName 'MyDatabase' -Name 'MyPrincipal' -ExcludeApplicationRoles
6171
6272
Returns $true if the principal exist in the database and is not a application role, if not $false is returned.
73+
74+
.EXAMPLE
75+
$serverInstance = Connect-SqlDscDatabaseEngine
76+
Test-SqlDscIsDatabasePrincipal -ServerObject $serverInstance -DatabaseName 'MyDatabase' -Name 'MyPrincipal' -Refresh
77+
78+
Returns $true if the principal exist in the database, if not $false is returned.
79+
The database's principal collections are refreshed before testing.
6380
#>
6481
function Test-SqlDscIsDatabasePrincipal
6582
{
@@ -94,15 +111,45 @@ function Test-SqlDscIsDatabasePrincipal
94111

95112
[Parameter()]
96113
[System.Management.Automation.SwitchParameter]
97-
$ExcludeApplicationRoles
114+
$ExcludeApplicationRoles,
115+
116+
[Parameter()]
117+
[System.Management.Automation.SwitchParameter]
118+
$Refresh
98119
)
99120

100121
process
101122
{
102123
$principalExist = $false
103124

125+
if ($Refresh.IsPresent)
126+
{
127+
# Refresh the server's databases collection to ensure we have current data
128+
$ServerObject.Databases.Refresh()
129+
}
130+
104131
$sqlDatabaseObject = $ServerObject.Databases[$DatabaseName]
105132

133+
if ($Refresh.IsPresent -and $sqlDatabaseObject)
134+
{
135+
# Refresh the database object's collections to ensure we have current data
136+
# Only refresh collections that will be used based on exclude parameters
137+
if (-not $ExcludeRoles.IsPresent)
138+
{
139+
$sqlDatabaseObject.Roles.Refresh()
140+
}
141+
142+
if (-not $ExcludeUsers.IsPresent)
143+
{
144+
$sqlDatabaseObject.Users.Refresh()
145+
}
146+
147+
if (-not $ExcludeApplicationRoles.IsPresent)
148+
{
149+
$sqlDatabaseObject.ApplicationRoles.Refresh()
150+
}
151+
}
152+
106153
if (-not $sqlDatabaseObject)
107154
{
108155
$PSCmdlet.ThrowTerminatingError(

‎tests/Integration/Commands/Get-SqlDscDatabasePermission.Integration.Tests.ps1‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ Describe 'Get-SqlDscDatabasePermission' -Tag @('Integration_SQL2017', 'Integrati
184184
Invoke-SqlDscQuery -ServerObject $script:serverObject -DatabaseName $script:testDatabaseName -Query $grantRolePermissionSql -Force -ErrorAction 'Stop'
185185

186186
# Test getting permissions for the custom role
187-
$result = Get-SqlDscDatabasePermission -ServerObject $script:serverObject -DatabaseName $script:testDatabaseName -Name $customRoleName
187+
$result = Get-SqlDscDatabasePermission -ServerObject $script:serverObject -DatabaseName $script:testDatabaseName -Name $customRoleName -Refresh
188188

189189
$result | Should -Not -BeNullOrEmpty
190190
$result | Should -BeOfType [Microsoft.SqlServer.Management.Smo.DatabasePermissionInfo]

‎tests/Unit/Public/Get-SqlDscDatabasePermission.Tests.ps1‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,5 +225,17 @@ Describe 'Get-SqlDscDatabasePermission' -Tag 'Public' {
225225
$mockResult[1].PermissionType.Update | Should -BeTrue
226226
}
227227
}
228+
229+
Context 'When using the Refresh parameter' {
230+
It 'Should pass the Refresh parameter to Test-SqlDscIsDatabasePrincipal' {
231+
Mock -CommandName Test-SqlDscIsDatabasePrincipal -MockWith {
232+
return $true
233+
} -ParameterFilter { $Refresh -eq $true }
234+
235+
$mockResult = Get-SqlDscDatabasePermission -ServerObject $mockServerObject -DatabaseName 'AdventureWorks' -Name 'Zebes\SamusAran' -Refresh -ErrorAction 'Stop'
236+
237+
Should -Invoke -CommandName Test-SqlDscIsDatabasePrincipal -ParameterFilter { $Refresh -eq $true } -Exactly -Times 1 -Scope It
238+
}
239+
}
228240
}
229241
}

‎tests/Unit/Public/Test-SqlDscIsDatabasePrincipal.Tests.ps1‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,131 @@ Describe 'Test-SqlDscIsDatabasePrincipal' -Tag 'Public' {
169169
$result | Should -BeFalse
170170
}
171171
}
172+
173+
Context 'When using the Refresh parameter' {
174+
BeforeAll {
175+
$script:refreshCalled = $false
176+
177+
$mockServerObjectWithRefresh = New-Object -TypeName 'Microsoft.SqlServer.Management.Smo.Server' |
178+
Add-Member -MemberType 'ScriptProperty' -Name 'Databases' -Value {
179+
return @{
180+
'AdventureWorks' = New-Object -TypeName Object |
181+
Add-Member -MemberType 'NoteProperty' -Name Name -Value 'AdventureWorks' -PassThru |
182+
Add-Member -MemberType 'ScriptProperty' -Name 'Users' -Value {
183+
return @{
184+
'Zebes\SamusAran' = New-Object -TypeName Object |
185+
Add-Member -MemberType 'NoteProperty' -Name 'Name' -Value 'Zebes\SamusAran' -PassThru -Force
186+
} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
187+
$script:refreshCalled = $true
188+
} -PassThru -Force
189+
} -PassThru |
190+
Add-Member -MemberType 'ScriptProperty' -Name 'ApplicationRoles' -Value {
191+
return @{} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
192+
# Mock refresh for ApplicationRoles
193+
} -PassThru -Force
194+
} -PassThru |
195+
Add-Member -MemberType 'ScriptProperty' -Name 'Roles' -Value {
196+
return @{} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
197+
# Mock refresh for Roles
198+
} -PassThru -Force
199+
} -PassThru -Force
200+
} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
201+
# Mock refresh for Databases collection
202+
$script:refreshCalled = $true
203+
} -PassThru -Force
204+
} -PassThru -Force
205+
}
206+
207+
It 'Should call Refresh on the database collections when Refresh parameter is specified' {
208+
$result = Test-SqlDscIsDatabasePrincipal -ServerObject $mockServerObjectWithRefresh -DatabaseName 'AdventureWorks' -Name 'Zebes\SamusAran' -Refresh
209+
210+
$result | Should -BeTrue
211+
$script:refreshCalled | Should -BeTrue
212+
}
213+
214+
It 'Should only refresh Users collection when other types are excluded' {
215+
$script:usersRefreshed = $false
216+
$script:rolesRefreshed = $false
217+
$script:appRolesRefreshed = $false
218+
219+
$mockServerObjectOptimized = New-Object -TypeName 'Microsoft.SqlServer.Management.Smo.Server' |
220+
Add-Member -MemberType 'ScriptProperty' -Name 'Databases' -Value {
221+
return @{
222+
'AdventureWorks' = New-Object -TypeName Object |
223+
Add-Member -MemberType 'NoteProperty' -Name Name -Value 'AdventureWorks' -PassThru |
224+
Add-Member -MemberType 'ScriptProperty' -Name 'Users' -Value {
225+
return @{
226+
'Zebes\SamusAran' = New-Object -TypeName Object |
227+
Add-Member -MemberType 'NoteProperty' -Name 'Name' -Value 'Zebes\SamusAran' -PassThru -Force
228+
} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
229+
$script:usersRefreshed = $true
230+
} -PassThru -Force
231+
} -PassThru |
232+
Add-Member -MemberType 'ScriptProperty' -Name 'ApplicationRoles' -Value {
233+
return @{} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
234+
$script:appRolesRefreshed = $true
235+
} -PassThru -Force
236+
} -PassThru |
237+
Add-Member -MemberType 'ScriptProperty' -Name 'Roles' -Value {
238+
return @{} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
239+
$script:rolesRefreshed = $true
240+
} -PassThru -Force
241+
} -PassThru -Force
242+
} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
243+
# Mock refresh for Databases collection
244+
} -PassThru -Force
245+
} -PassThru -Force
246+
247+
$result = Test-SqlDscIsDatabasePrincipal -ServerObject $mockServerObjectOptimized -DatabaseName 'AdventureWorks' -Name 'Zebes\SamusAran' -Refresh -ExcludeRoles -ExcludeApplicationRoles
248+
249+
$result | Should -BeTrue
250+
$script:usersRefreshed | Should -BeTrue
251+
$script:rolesRefreshed | Should -BeFalse
252+
$script:appRolesRefreshed | Should -BeFalse
253+
}
254+
255+
It 'Should only refresh Roles collection when Users and ApplicationRoles are excluded' {
256+
$script:usersRefreshed = $false
257+
$script:rolesRefreshed = $false
258+
$script:appRolesRefreshed = $false
259+
260+
$mockServerObjectOptimized = New-Object -TypeName 'Microsoft.SqlServer.Management.Smo.Server' |
261+
Add-Member -MemberType 'ScriptProperty' -Name 'Databases' -Value {
262+
return @{
263+
'AdventureWorks' = New-Object -TypeName Object |
264+
Add-Member -MemberType 'NoteProperty' -Name Name -Value 'AdventureWorks' -PassThru |
265+
Add-Member -MemberType 'ScriptProperty' -Name 'Users' -Value {
266+
return @{} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
267+
$script:usersRefreshed = $true
268+
} -PassThru -Force
269+
} -PassThru |
270+
Add-Member -MemberType 'ScriptProperty' -Name 'ApplicationRoles' -Value {
271+
return @{} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
272+
$script:appRolesRefreshed = $true
273+
} -PassThru -Force
274+
} -PassThru |
275+
Add-Member -MemberType 'ScriptProperty' -Name 'Roles' -Value {
276+
return @{
277+
'TestRole' = New-Object -TypeName Object |
278+
Add-Member -MemberType 'NoteProperty' -Name 'Name' -Value 'TestRole' -PassThru |
279+
Add-Member -MemberType 'NoteProperty' -Name 'IsFixedRole' -Value $false -PassThru -Force
280+
} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
281+
$script:rolesRefreshed = $true
282+
} -PassThru -Force
283+
} -PassThru -Force
284+
} | Add-Member -MemberType 'ScriptMethod' -Name 'Refresh' -Value {
285+
# Mock refresh for Databases collection
286+
} -PassThru -Force
287+
} -PassThru -Force
288+
289+
$result = Test-SqlDscIsDatabasePrincipal -ServerObject $mockServerObjectOptimized -DatabaseName 'AdventureWorks' -Name 'TestRole' -Refresh -ExcludeUsers -ExcludeApplicationRoles
290+
291+
$result | Should -BeTrue
292+
$script:usersRefreshed | Should -BeFalse
293+
$script:rolesRefreshed | Should -BeTrue
294+
$script:appRolesRefreshed | Should -BeFalse
295+
}
296+
}
172297
}
173298

174299
Context 'When the specified principal is a application role' {

0 commit comments

Comments
 (0)