Skip to content

Commit 4c74df4

Browse files
Shawn Sesnagaelcolas
Shawn Sesna
authored and
gaelcolas
committed
Improve module quality & tests (#20)
Improved Code quality, cosmetic, & Added the ability to have all Access variables present, but allow for null or empty.
1 parent c15564c commit 4c74df4

File tree

3 files changed

+559
-85
lines changed

3 files changed

+559
-85
lines changed

‎DscResources/MSFT_xSmbShare/MSFT_xSmbShare.psm1

+163-78
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ function Get-TargetResource
2121
if ($smbShare -ne $null)
2222
{
2323
$smbShareAccess = Get-SmbShareAccess -Name $Name
24-
$smbShareAccess | % {
24+
$smbShareAccess | ForEach-Object {
2525
$access = $_;
2626
if ($access.AccessRight -eq 'Change' -and $access.AccessControlType -eq 'Allow')
2727
{
@@ -30,7 +30,7 @@ function Get-TargetResource
3030
elseif ($access.AccessRight -eq 'Read' -and $access.AccessControlType -eq 'Allow')
3131
{
3232
$readAccess += $access.AccountName
33-
}
33+
}
3434
elseif ($access.AccessRight -eq 'Full' -and $access.AccessControlType -eq 'Allow')
3535
{
3636
$fullAccess += $access.AccountName
@@ -44,24 +44,24 @@ function Get-TargetResource
4444
else
4545
{
4646
Write-Verbose "Share with name $Name does not exist"
47-
}
47+
}
4848

4949
$returnValue = @{
50-
Name = $smbShare.Name
51-
Path = $smbShare.Path
52-
Description = $smbShare.Description
53-
ConcurrentUserLimit = $smbShare.ConcurrentUserLimit
54-
EncryptData = $smbShare.EncryptData
55-
FolderEnumerationMode = $smbShare.FolderEnumerationMode
56-
ShareState = $smbShare.ShareState
57-
ShareType = $smbShare.ShareType
58-
ShadowCopy = $smbShare.ShadowCopy
59-
Special = $smbShare.Special
60-
ChangeAccess = $changeAccess
61-
ReadAccess = $readAccess
62-
FullAccess = $fullAccess
63-
NoAccess = $noAccess
64-
Ensure = if($smbShare) {"Present"} else {"Absent"}
50+
Name = $smbShare.Name
51+
Path = $smbShare.Path
52+
Description = $smbShare.Description
53+
ConcurrentUserLimit = $smbShare.ConcurrentUserLimit
54+
EncryptData = $smbShare.EncryptData
55+
FolderEnumerationMode = $smbShare.FolderEnumerationMode
56+
ShareState = $smbShare.ShareState
57+
ShareType = $smbShare.ShareType
58+
ShadowCopy = $smbShare.ShadowCopy
59+
Special = $smbShare.Special
60+
ChangeAccess = $changeAccess
61+
ReadAccess = $readAccess
62+
FullAccess = $fullAccess
63+
NoAccess = $noAccess
64+
Ensure = if($smbShare) {"Present"} else {"Absent"}
6565
}
6666

6767
$returnValue
@@ -71,7 +71,7 @@ function Set-AccessPermission
7171
{
7272
[CmdletBinding()]
7373
Param
74-
(
74+
(
7575
$ShareName,
7676

7777
[string[]]
@@ -94,11 +94,52 @@ function Set-AccessPermission
9494
}
9595
}
9696

97+
Function Set-BoundParameters
98+
{
99+
# Define parameters
100+
Param
101+
(
102+
$BoundParameters
103+
)
104+
105+
# Check for null access before passing to New-SmbShare
106+
if (($BoundParameters.ContainsKey("ChangeAccess")) -and ([string]::IsNullOrEmpty($BoundParameters["ChangeAccess"])))
107+
{
108+
Write-Verbose "Parameter ChangeAccess is null or empty, removing from collection."
109+
# Remove the parameter
110+
$BoundParameters.Remove("ChangeAccess")
111+
}
112+
113+
if (($BoundParameters.ContainsKey("ReadAccess")) -and ([string]::IsNullOrEmpty($BoundParameters["ReadAccess"])))
114+
{
115+
Write-Verbose "Paramater ReadAccess is null or empty, removing from collection."
116+
# Remove the parameter
117+
$BoundParameters.Remove("ReadAccess")
118+
}
119+
120+
if (($BoundParameters.ContainsKey("FullAccess")) -and ([string]::IsNullOrEmpty($BoundParameters["FullAccess"])))
121+
{
122+
Write-Verbose "Parameter FullAccess is null or empty, removing from collection."
123+
# Remove the parameter
124+
$BoundParameters.Remove("FullAccess")
125+
}
126+
127+
if (($BoundParameters.ContainsKey("NoAccess")) -and ([string]::IsNullOrEmpty($BoundParameters["NoAccess"])))
128+
{
129+
Write-Verbose "Parameter NoAccess is null or empty, removing from collection."
130+
# Remove the parameter
131+
$BoundParameters.Remove("NoAccess")
132+
}
133+
134+
# Return the parameter collection
135+
return $BoundParameters
136+
}
137+
97138
function Remove-AccessPermission
98139
{
99140
[CmdletBinding()]
100141
Param
101-
(
142+
(
102143
$ShareName,
103144

104145
[string[]]
@@ -114,7 +155,8 @@ function Remove-AccessPermission
114155
if ($AccessPermission -eq "Change" -or $AccessPermission -eq "Read" -or $AccessPermission -eq "Full")
115156
{
116157
Revoke-SmbShareAccess -Name $Name -AccountName $UserName -Force
117-
}
158+
159+
}
118160
else
119161
{
120162
UnBlock-SmbShareAccess -Name $Name -AccountName $userName -Force
@@ -164,7 +206,7 @@ function Set-TargetResource
164206
$Ensure = 'Present'
165207
)
166208

167-
$psboundparameters.Remove("Debug")
209+
$PSBoundParameters.Remove("Debug")
168210

169211
$shareExists = $false
170212
$smbShare = Get-SmbShare -Name $Name -ErrorAction SilentlyContinue
@@ -177,94 +219,115 @@ function Set-TargetResource
177219
{
178220
if ($shareExists -eq $false)
179221
{
180-
$psboundparameters.Remove("Ensure")
222+
$PSBoundParameters.Remove("Ensure")
181223
Write-Verbose "Creating share $Name to ensure it is Present"
182-
New-SmbShare @psboundparameters
224+
225+
# Alter bound parameters
226+
$newShareParameters = Set-BoundParameters -BoundParameters $PSBoundParameters
227+
228+
# Pass the parameter collection to New-SmbShare
229+
New-SmbShare @newShareParameters
183230
}
184231
else
185232
{
186233
# Need to call either Set-SmbShare or *ShareAccess cmdlets
187-
if ($psboundparameters.ContainsKey("ChangeAccess"))
234+
if ($PSBoundParameters.ContainsKey("ChangeAccess"))
188235
{
189-
$changeAccessValue = $psboundparameters["ChangeAccess"]
190-
$psboundparameters.Remove("ChangeAccess")
236+
$changeAccessValue = $PSBoundParameters["ChangeAccess"]
237+
$PSBoundParameters.Remove("ChangeAccess")
191238
}
192-
if ($psboundparameters.ContainsKey("ReadAccess"))
239+
if ($PSBoundParameters.ContainsKey("ReadAccess"))
193240
{
194-
$readAccessValue = $psboundparameters["ReadAccess"]
195-
$psboundparameters.Remove("ReadAccess")
241+
$readAccessValue = $PSBoundParameters["ReadAccess"]
242+
$PSBoundParameters.Remove("ReadAccess")
196243
}
197-
if ($psboundparameters.ContainsKey("FullAccess"))
244+
if ($PSBoundParameters.ContainsKey("FullAccess"))
198245
{
199-
$fullAccessValue = $psboundparameters["FullAccess"]
200-
$psboundparameters.Remove("FullAccess")
246+
$fullAccessValue = $PSBoundParameters["FullAccess"]
247+
$PSBoundParameters.Remove("FullAccess")
201248
}
202-
if ($psboundparameters.ContainsKey("NoAccess"))
249+
if ($PSBoundParameters.ContainsKey("NoAccess"))
203250
{
204-
$noAccessValue = $psboundparameters["NoAccess"]
205-
$psboundparameters.Remove("NoAccess")
251+
$noAccessValue = $PSBoundParameters["NoAccess"]
252+
$PSBoundParameters.Remove("NoAccess")
206253
}
207-
254+
208255
# Use Set-SmbShare for performing operations other than changing access
209-
$psboundparameters.Remove("Ensure")
210-
$psboundparameters.Remove("Path")
256+
$PSBoundParameters.Remove("Ensure")
257+
$PSBoundParameters.Remove("Path")
211258
Set-SmbShare @PSBoundParameters -Force
212-
259+
213260
# Use *SmbShareAccess cmdlets to change access
214-
$smbshareAccessValues = Get-SmbShareAccess -Name $Name
261+
$smbShareAccessValues = Get-SmbShareAccess -Name $Name
262+
263+
# Remove Change permissions
264+
$smbShareAccessValues | Where-Object {$_.AccessControlType -eq 'Allow' -and $_.AccessRight -eq 'Change'} `
265+
| ForEach-Object {
266+
Remove-AccessPermission -ShareName $Name -UserName $_.AccountName -AccessPermission Change
267+
}
268+
215269
if ($ChangeAccess -ne $null)
216270
{
217-
# Blow off whatever is in there and replace it with this list
218-
$smbshareAccessValues | ? {$_.AccessControlType -eq 'Allow' -and $_.AccessRight -eq 'Change'} `
219-
| % {
220-
Remove-AccessPermission -ShareName $Name -UserName $_.AccountName -AccessPermission Change
221-
}
222-
223-
$changeAccessValue | % {
271+
# Add change permissions
272+
$changeAccessValue | ForEach-Object {
224273
Set-AccessPermission -ShareName $Name -AccessPermission "Change" -Username $_
225274
}
226275
}
227-
$smbshareAccessValues = Get-SmbShareAccess -Name $Name
276+
277+
$smbShareAccessValues = Get-SmbShareAccess -Name $Name
278+
279+
# Remove read access
280+
$smbShareAccessValues | Where-Object {$_.AccessControlType -eq 'Allow' -and $_.AccessRight -eq 'Read'} `
281+
| ForEach-Object {
282+
Remove-AccessPermission -ShareName $Name -UserName $_.AccountName -AccessPermission Read
283+
}
284+
228285
if ($ReadAccess -ne $null)
229286
{
230-
# Blow off whatever is in there and replace it with this list
231-
$smbshareAccessValues | ? {$_.AccessControlType -eq 'Allow' -and $_.AccessRight -eq 'Read'} `
232-
| % {
233-
Remove-AccessPermission -ShareName $Name -UserName $_.AccountName -AccessPermission Read
234-
}
235-
236-
$readAccessValue | % {
237-
Set-AccessPermission -ShareName $Name -AccessPermission "Read" -Username $_
287+
# Add read access
288+
$readAccessValue | ForEach-Object {
289+
Set-AccessPermission -ShareName $Name -AccessPermission "Read" -Username $_
238290
}
239291
}
240-
$smbshareAccessValues = Get-SmbShareAccess -Name $Name
292+
293+
294+
$smbShareAccessValues = Get-SmbShareAccess -Name $Name
295+
296+
# Remove full access
297+
$smbShareAccessValues | Where-Object {$_.AccessControlType -eq 'Allow' -and $_.AccessRight -eq 'Full'} `
298+
| ForEach-Object {
299+
Remove-AccessPermission -ShareName $Name -UserName $_.AccountName -AccessPermission Full
300+
}
301+
302+
241303
if ($FullAccess -ne $null)
242304
{
243-
# Blow off whatever is in there and replace it with this list
244-
$smbshareAccessValues | ? {$_.AccessControlType -eq 'Allow' -and $_.AccessRight -eq 'Full'} `
245-
| % {
246-
Remove-AccessPermission -ShareName $Name -UserName $_.AccountName -AccessPermission Full
247-
}
248-
249-
$fullAccessValue | % {
250-
Set-AccessPermission -ShareName $Name -AccessPermission "Full" -Username $_
305+
306+
# Add full access
307+
$fullAccessValue | ForEach-Object {
308+
Set-AccessPermission -ShareName $Name -AccessPermission "Full" -Username $_
251309
}
252310
}
253-
$smbshareAccessValues = Get-SmbShareAccess -Name $Name
311+
312+
$smbShareAccessValues = Get-SmbShareAccess -Name $Name
313+
314+
# Remove explicit deny
315+
$smbShareAccessValues | Where-Object {$_.AccessControlType -eq 'Deny'} `
316+
| ForEach-Object {
317+
Remove-AccessPermission -ShareName $Name -UserName $_.AccountName -AccessPermission No
318+
}
319+
320+
254321
if ($NoAccess -ne $null)
255322
{
256-
# Blow off whatever is in there and replace it with this list
257-
$smbshareAccessValues | ? {$_.AccessControlType -eq 'Deny'} `
258-
| % {
259-
Remove-AccessPermission -ShareName $Name -UserName $_.AccountName -AccessPermission No
260-
}
261-
$noAccessValue | % {
323+
# Add explicit deny
324+
$noAccessValue | ForEach-Object {
262325
Set-AccessPermission -ShareName $Name -AccessPermission "No" -Username $_
263326
}
264327
}
265328
}
266329
}
267-
else
330+
else
268331
{
269332
Write-Verbose "Removing share $Name to ensure it is Absent"
270333
Remove-SmbShare -name $Name -Force
@@ -314,8 +377,13 @@ function Test-TargetResource
314377
[System.String]
315378
$Ensure = 'Present'
316379
)
380+
381+
# Alter the bound parameters, removing anything that is null or emtpy
382+
$alteredBoundParameters = Set-BoundParameters -boundparameters $PSBoundParameters
383+
317384
$testResult = $false;
318385
$share = Get-TargetResource -Name $Name -Path $Path -ErrorAction SilentlyContinue -ErrorVariable ev
386+
$differences = @()
319387
if ($Ensure -ne "Absent")
320388
{
321389
if ($share.Ensure -eq "Absent")
@@ -325,9 +393,27 @@ function Test-TargetResource
325393
elseif ($share.Ensure -eq "Present")
326394
{
327395
$Params = 'Name', 'Path', 'Description', 'ChangeAccess', 'ConcurrentUserLimit', 'EncryptData', 'FolderEnumerationMode', 'FullAccess', 'NoAccess', 'ReadAccess', 'Ensure'
328-
if ($PSBoundParameters.Keys.Where({$_ -in $Params}) | ForEach-Object {Compare-Object -ReferenceObject $PSBoundParameters.$_ -DifferenceObject $share.$_})
329-
{
330-
$testResult = $false
396+
397+
# Get all matching parameters from alteredBoundParameters that are in Params
398+
$matchingParameters = $alteredBoundParameters.Keys.Where({($_ -in $Params)})
399+
400+
if ($null -ne $matchingParameters)
401+
{
402+
foreach ($matchingParameter in $matchingParameters)
403+
{
404+
$differences += Compare-Object -ReferenceObject $alteredBoundParameters[$matchingParameter] -DifferenceObject $share.$matchingParameter #; $differences
405+
}
406+
407+
# Check to see if there is anything in $differences
408+
if (($null -ne $differences) -and ($differences.Length -gt 0))
409+
{
410+
$differences | ForEach-Object {Write-Verbose -Message "$_"}
411+
$testResult = $false
412+
}
413+
else
414+
{
415+
$testResult = $true
416+
}
331417
}
332418
else
333419
{
@@ -351,4 +437,3 @@ function Test-TargetResource
351437
}
352438

353439
Export-ModuleMember -Function *-TargetResource
354-

0 commit comments

Comments
 (0)