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.

Sign up for GitHub

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

xADUser: Fix CN/Path Concurrent Change and Empty String Property on Creation Exceptions #412

Merged
merged 9 commits into from
Jul 5, 2019
Merged

xADUser: Fix CN/Path Concurrent Change and Empty String Property on Creation Exceptions #412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b58b470
Empty String fix
X-Guardian Jul 2, 2019
6631f07
Relocate Move/Rename user code to after Set-ADUser
X-Guardian Jul 2, 2019
3b04ae1
Identity fix
X-Guardian Jul 2, 2019
521d192
Update CHANGELOG
X-Guardian Jul 2, 2019
8d3e607
review updates
X-Guardian Jul 3, 2019
aea4a0f
Merge branch 'dev' into xADUser-Empty-String-Fix
X-Guardian Jul 3, 2019
2c39286
Add Compare-Object comments
X-Guardian Jul 3, 2019
45f5458
Update MSFT_xADUser.psm1
X-Guardian Jul 5, 2019
5ea4735
Merge branch 'dev' into xADUser-Empty-String-Fix
X-Guardian Jul 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
- Fixed the GUID in Example 3-AddComputerAccountSpecificPath_Config. ([issue #410](https://github.com/PowerShell/xActiveDirectory/pull/410))
- Changes to xADOrganizationalUnit
- Catch exception when the path property specifies a non-existing path ([issue #408](https://github.com/PowerShell/xActiveDirectory/pull/408))
- Changes to xADUser
- Fixes exception when creating a user with an empty string property ([issue #407](https://github.com/PowerShell/xActiveDirectory/pull/407)).
- Fixes exception when updating `CommonName` and `Path` concurrently ([issue #402](https://github.com/PowerShell/xActiveDirectory/pull/402)).

## 3.0.0.0

Expand Down
65 changes: 46 additions & 19 deletions DSCResources/MSFT_xADUser/MSFT_xADUser.psm1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1063,7 +1063,10 @@ function Test-TargetResource
# This check is required to be able to explicitly remove values with an empty string, if required
if (([System.String]::IsNullOrEmpty($PSBoundParameters.$parameter)) -and ([System.String]::IsNullOrEmpty($targetResource.$parameter)))
{
# Both values are null/empty and therefore we are compliant
<#
Both values are null/empty and therefore we are compliant
Must catch this scenario separately, as Compare-Object can't compare Null objects
#>
}
elseif (($null -ne $PSBoundParameters.$parameter -and $null -eq $targetResource.$parameter) -or
($null -eq $PSBoundParameters.$parameter -and $null -ne $targetResource.$parameter) -or
Expand Down Expand Up @@ -1488,6 +1491,8 @@ function Set-TargetResource
$setADUserParams = Get-ADCommonParameters @PSBoundParameters
$replaceUserProperties = @{ }
$clearUserProperties = @()
$moveUserRequired = $false
$renameUserRequired = $false

foreach ($parameter in $PSBoundParameters.Keys)
{
Expand All @@ -1498,27 +1503,13 @@ function Set-TargetResource
$adProperty = $adPropertyMap | Where-Object -FilterScript { $_.Parameter -eq $parameter }
if ($parameter -eq 'Path' -and ($PSBoundParameters.Path -ne $targetResource.Path))
{
# Cannot move users by updating the DistinguishedName property
$adCommonParameters = Get-ADCommonParameters @PSBoundParameters

# Using the SamAccountName for identity with Move-ADObject does not work, use the DN instead
$adCommonParameters['Identity'] = $targetResource.DistinguishedName

Write-Verbose -Message ($script:localizedData.MovingADUser -f $targetResource.Path, $PSBoundParameters.Path)

Move-ADObject @adCommonParameters -TargetPath $PSBoundParameters.Path
# Move user after any property changes
$moveUserRequired = $true
}
elseif ($parameter -eq 'CommonName' -and ($PSBoundParameters.CommonName -ne $targetResource.CommonName))
{
# Cannot rename users by updating the CN property directly
$adCommonParameters = Get-ADCommonParameters @PSBoundParameters

# Using the SamAccountName for identity with Rename-ADObject does not work, use the DN instead
$adCommonParameters['Identity'] = $targetResource.DistinguishedName

Write-Verbose -Message ($script:localizedData.RenamingADUser -f $targetResource.CommonName, $PSBoundParameters.CommonName)

Rename-ADObject @adCommonParameters -NewName $PSBoundParameters.CommonName
# Rename user after any property changes
$renameUserRequired = $true
}
elseif ($parameter -eq 'Password' -and $PasswordNeverResets -eq $false)
{
Expand Down Expand Up @@ -1550,6 +1541,13 @@ function Set-TargetResource
#>
Write-Verbose -Message ($script:localizedData.UpdatingADUserProperty -f $parameter, $PSBoundParameters.$parameter)
}
elseif (([System.String]::IsNullOrEmpty($PSBoundParameters.$parameter)) -and ([System.String]::IsNullOrEmpty($targetResource.$parameter)))
{
<#
Both values are null/empty and therefore we are compliant
Must catch this scenario separately, as Compare-Object can't compare Null objects
#>
}
# Use Compare-Object to allow comparison of string and array parameters
elseif (($null -ne $PSBoundParameters.$parameter -and $null -eq $targetResource.$parameter) -or
($null -eq $PSBoundParameters.$parameter -and $null -ne $targetResource.$parameter) -or
Expand Down Expand Up @@ -1622,6 +1620,35 @@ function Set-TargetResource
Write-Verbose -Message ($script:localizedData.UpdatingADUser -f $UserName)

[ref] $null = Set-ADUser @setADUserParams -Enabled $Enabled

if ($moveUserRequired)
{
# Cannot move users by updating the DistinguishedName property
$moveAdObjectParameters = Get-ADCommonParameters @PSBoundParameters

# Using the SamAccountName for identity with Move-ADObject does not work, use the DN instead
$moveAdObjectParameters['Identity'] = $targetResource.DistinguishedName

Write-Verbose -Message ($script:localizedData.MovingADUser -f $targetResource.Path, $PSBoundParameters.Path)

Move-ADObject @moveAdObjectParameters -TargetPath $PSBoundParameters.Path

# Set new target resource DN in case a rename is also required
$targetResource.DistinguishedName = "cn=$($targetResource.CommonName),$($PSBoundParameters.Path)"
}

if ($renameUserRequired)
{
# Cannot rename users by updating the CN property directly
$renameAdObjectParameters = Get-ADCommonParameters @PSBoundParameters

# Using the SamAccountName for identity with Rename-ADObject does not work, use the DN instead
$renameAdObjectParameters['Identity'] = $targetResource.DistinguishedName

Write-Verbose -Message ($script:localizedData.RenamingADUser -f $targetResource.CommonName, $PSBoundParameters.CommonName)

Rename-ADObject @renameAdObjectParameters -NewName $PSBoundParameters.CommonName
}
}
elseif (($Ensure -eq 'Absent') -and ($targetResource.Ensure -eq 'Present'))
{
Expand Down