xADUser: Fix CN/Path Concurrent Change and Empty String Property on C…

…reation Exceptions (dsccommunity#412) - Changes to xADUser - Fixes exception when creating a user with an empty string property (issue dsccommunity#407). - Fixes exception when updating `CommonName` and `Path` concurrently (issue dsccommunity#402).
johlju · Jul 5, 2019 · f81e3b7 · f81e3b7
1 parent 46c6cf2
commit f81e3b7
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 19 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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
 

diff --git a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1
@@ -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
@@ -1488,6 +1491,8 @@ function Set-TargetResource
         $setADUserParams = Get-ADCommonParameters @PSBoundParameters
         $replaceUserProperties = @{ }
         $clearUserProperties = @()
+        $moveUserRequired = $false
+        $renameUserRequired = $false
 
         foreach ($parameter in $PSBoundParameters.Keys)
         {
@@ -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)
                 {
@@ -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
@@ -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'))
     {