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

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

Merged
merged 9 commits into from
Jul 5, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
- Added a requirement to README stating "Group Managed Service Accounts need at least one Windows Server 2012 Domain Controller" ([issue #399](https://github.com/PowerShell/xActiveDirectory/pull/399)).
- Changes to xADComputer
- Fixed the GUID in Example 3-AddComputerAccountSpecificPath_Config. ([issue #410](https://github.com/PowerShell/xActiveDirectory/pull/410))
- 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
59 changes: 41 additions & 18 deletions DSCResources/MSFT_xADUser/MSFT_xADUser.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,7 @@ function Test-TargetResource
if (([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
}
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 +1489,8 @@ function Set-TargetResource
$setADUserParams = Get-ADCommonParameters @PSBoundParameters
$replaceUserProperties = @{ }
$clearUserProperties = @()
$moveUserRequired = $false
$renameUserRequired = $false

foreach ($parameter in $PSBoundParameters.Keys)
{
Expand All @@ -1498,27 +1501,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 +1539,11 @@ 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 +1616,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