From b58b470fecd844ef939bdae527604e9b8d55d10b Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Tue, 2 Jul 2019 23:19:48 +0100 Subject: [PATCH 1/7] Empty String fix --- DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 index c9ab39030..01cc1094c 100644 --- a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 +++ b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 @@ -1550,6 +1550,10 @@ 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 + } # 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 From 6631f0705fbec4a9e23d6e91a1c6acd97faaad0d Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Tue, 2 Jul 2019 23:20:44 +0100 Subject: [PATCH 2/7] Relocate Move/Rename user code to after Set-ADUser --- DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 | 50 +++++++++++++-------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 index 01cc1094c..1b781e804 100644 --- a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 +++ b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 @@ -1488,6 +1488,8 @@ function Set-TargetResource $setADUserParams = Get-ADCommonParameters @PSBoundParameters $replaceUserProperties = @{ } $clearUserProperties = @() + $moveUserRequired = $false + $renameUserRequired = $false foreach ($parameter in $PSBoundParameters.Keys) { @@ -1498,27 +1500,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) { @@ -1626,6 +1614,32 @@ 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 + $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 + } + + if ($renameUserRequired) + { + # 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'] = "cn=$($targetResource.CommonName),$($PSBoundParameters.Path)" + + Write-Verbose -Message ($script:localizedData.RenamingADUser -f $targetResource.CommonName, $PSBoundParameters.CommonName) + + Rename-ADObject @adCommonParameters -NewName $PSBoundParameters.CommonName + } } elseif (($Ensure -eq 'Absent') -and ($targetResource.Ensure -eq 'Present')) { From 3b04ae1249f13dc6e987a9980e51876da7410d18 Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Tue, 2 Jul 2019 23:51:09 +0100 Subject: [PATCH 3/7] Identity fix --- DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 index 1b781e804..478a8e3c8 100644 --- a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 +++ b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 @@ -1626,6 +1626,9 @@ function Set-TargetResource Write-Verbose -Message ($script:localizedData.MovingADUser -f $targetResource.Path, $PSBoundParameters.Path) Move-ADObject @adCommonParameters -TargetPath $PSBoundParameters.Path + + # Set new target resource DN in case a rename is also required + $targetResource.DistinguishedName = "cn=$($targetResource.CommonName),$($PSBoundParameters.Path)" } if ($renameUserRequired) @@ -1634,7 +1637,7 @@ function Set-TargetResource $adCommonParameters = Get-ADCommonParameters @PSBoundParameters # Using the SamAccountName for identity with Rename-ADObject does not work, use the DN instead - $adCommonParameters['Identity'] = "cn=$($targetResource.CommonName),$($PSBoundParameters.Path)" + $adCommonParameters['Identity'] = $targetResource.DistinguishedName Write-Verbose -Message ($script:localizedData.RenamingADUser -f $targetResource.CommonName, $PSBoundParameters.CommonName) From 521d19278c05088e1af8c44f7a63bd25a4d4a342 Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Wed, 3 Jul 2019 00:04:42 +0100 Subject: [PATCH 4/7] Update CHANGELOG --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a013ebcd3..05b955516 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ - Added 'about_\.help.txt' file to all resources ([issue #404](https://github.com/PowerShell/xActiveDirectory/pull/404)). - Changes to xADManagedServiceAccount - 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 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 From 8d3e607cdab420191df33f03692d7165d5aab135 Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Wed, 3 Jul 2019 19:32:39 +0100 Subject: [PATCH 5/7] review updates --- DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 index 478a8e3c8..bcc1d705b 100644 --- a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 +++ b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 @@ -1501,7 +1501,7 @@ function Set-TargetResource if ($parameter -eq 'Path' -and ($PSBoundParameters.Path -ne $targetResource.Path)) { # Move user after any property changes - $moveUserRequired = $True + $moveUserRequired = $true } elseif ($parameter -eq 'CommonName' -and ($PSBoundParameters.CommonName -ne $targetResource.CommonName)) { @@ -1618,14 +1618,14 @@ function Set-TargetResource if ($moveUserRequired) { # Cannot move users by updating the DistinguishedName property - $adCommonParameters = Get-ADCommonParameters @PSBoundParameters + $moveAdObjectParameters = Get-ADCommonParameters @PSBoundParameters # Using the SamAccountName for identity with Move-ADObject does not work, use the DN instead - $adCommonParameters['Identity'] = $targetResource.DistinguishedName + $moveAdObjectParameters['Identity'] = $targetResource.DistinguishedName Write-Verbose -Message ($script:localizedData.MovingADUser -f $targetResource.Path, $PSBoundParameters.Path) - Move-ADObject @adCommonParameters -TargetPath $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)" @@ -1634,14 +1634,14 @@ function Set-TargetResource if ($renameUserRequired) { # Cannot rename users by updating the CN property directly - $adCommonParameters = Get-ADCommonParameters @PSBoundParameters + $renameAdObjectParameters = Get-ADCommonParameters @PSBoundParameters # Using the SamAccountName for identity with Rename-ADObject does not work, use the DN instead - $adCommonParameters['Identity'] = $targetResource.DistinguishedName + $renameAdObjectParameters['Identity'] = $targetResource.DistinguishedName Write-Verbose -Message ($script:localizedData.RenamingADUser -f $targetResource.CommonName, $PSBoundParameters.CommonName) - Rename-ADObject @adCommonParameters -NewName $PSBoundParameters.CommonName + Rename-ADObject @renameAdObjectParameters -NewName $PSBoundParameters.CommonName } } elseif (($Ensure -eq 'Absent') -and ($targetResource.Ensure -eq 'Present')) From 2c3928605e525f22133f441cf17c8929b11d90d7 Mon Sep 17 00:00:00 2001 From: Simon Heather Date: Wed, 3 Jul 2019 21:20:05 +0100 Subject: [PATCH 6/7] Add Compare-Object comments --- DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 index bcc1d705b..fc46ba290 100644 --- a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 +++ b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 @@ -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 @@ -1541,6 +1542,7 @@ function Set-TargetResource 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 From 45f5458bd01fc0fea2249172f38f25541c360635 Mon Sep 17 00:00:00 2001 From: Simon Heather <32168619+X-Guardian@users.noreply.github.com> Date: Fri, 5 Jul 2019 14:09:42 +0100 Subject: [PATCH 7/7] Update MSFT_xADUser.psm1 --- DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 index fc46ba290..04ed7b8b7 100644 --- a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 +++ b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 @@ -1063,8 +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 - # Must catch this scenario separately, as Compare-Object can't compare Null objects + <# + 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 @@ -1541,8 +1543,10 @@ function Set-TargetResource } 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 + <# + 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