From d2e1aeeb6781cb79791cee5141e916aabb26673c Mon Sep 17 00:00:00 2001 From: Marius Rieder Date: Tue, 12 Nov 2019 23:42:44 +0100 Subject: [PATCH] win_domain_user: Make Identification of the user to work with more robust (#61594) * Ensure we work on only one user. After the initial get/create use the GUID of the found/created user to ensure we will not start to work with a different user. If we create a user or modify it's attributes an he is not identified with the name parameter afterwards this module fails in rather unpredictable ways. This addressed #45298 * Use splatting create_args for creating user. This prepars this for adding more optional create arguments without nesting of condictions. * Set the UserPrincipalName and SamAccountName on create. Set the UserPrincipalName and SamAccountName on the create operation if upn is given to ensure the User is created with a contollable SamAccountName. * Rename $username to $name. $username is missleading as its not the SamAccountName. * Add a identity parameter to win_domain_user This gives the user full controll over how the user is identified in the AD. * Add version_added information for new parameter and fix yaml syntax. * Added changelog fragment --- .../fragments/win_domain_user-identity.yaml | 2 + .../modules/windows/win_domain_user.ps1 | 88 ++++++++++--------- .../modules/windows/win_domain_user.py | 8 ++ 3 files changed, 58 insertions(+), 40 deletions(-) create mode 100644 changelogs/fragments/win_domain_user-identity.yaml diff --git a/changelogs/fragments/win_domain_user-identity.yaml b/changelogs/fragments/win_domain_user-identity.yaml new file mode 100644 index 00000000000..f5e2335af60 --- /dev/null +++ b/changelogs/fragments/win_domain_user-identity.yaml @@ -0,0 +1,2 @@ +minor_changes: +- win_domain_user - Added the ``identity`` module option to explicitly set the identity of the user when searching for it - https://github.com/ansible/ansible/issues/45298 diff --git a/lib/ansible/modules/windows/win_domain_user.ps1 b/lib/ansible/modules/windows/win_domain_user.ps1 index 1bc8ceeed87..78d92077b77 100644 --- a/lib/ansible/modules/windows/win_domain_user.ps1 +++ b/lib/ansible/modules/windows/win_domain_user.ps1 @@ -78,7 +78,8 @@ $domain_password = Get-AnsibleParam -obj $params -name "domain_password" -type " $domain_server = Get-AnsibleParam -obj $params -name "domain_server" -type "str" # User account parameters -$username = Get-AnsibleParam -obj $params -name "name" -type "str" -failifempty $true +$name = Get-AnsibleParam -obj $params -name "name" -type "str" -failifempty $true +$identity = Get-AnsibleParam -obj $params -name "identity" -type "str" -default $name $description = Get-AnsibleParam -obj $params -name "description" -type "str" $password = Get-AnsibleParam -obj $params -name "password" -type "str" $password_expired = Get-AnsibleParam -obj $params -name "password_expired" -type "bool" @@ -125,10 +126,12 @@ if ($null -ne $domain_server) { } try { - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + $user_obj = Get-ADUser -Identity $identity -Properties * @extra_args + $user_guid = $user_obj.ObjectGUID } catch [Microsoft.ActiveDirectory.Management.ADIdentityNotFoundException] { $user_obj = $null + $user_guid = $null } If ($state -eq 'present') { @@ -137,19 +140,24 @@ If ($state -eq 'present') { # If the account does not exist, create it If (-not $user_obj) { + $create_args = @{} + $create_args.Name = $name If ($null -ne $path){ - New-ADUser -Name $username -Path $path -WhatIf:$check_mode @extra_args + $create_args.Path = $path } - Else { - New-ADUser -Name $username -WhatIf:$check_mode @extra_args + If ($null -ne $upn){ + $create_args.UserPrincipalName = $upn + $create_args.SamAccountName = $upn.Split('@')[0] } + $user_obj = New-ADUser @create_args -WhatIf:$check_mode -PassThru @extra_args + $user_guid = $user_obj.ObjectGUID $new_user = $true - $result.created = $true + $result.created = $true $result.changed = $true If ($check_mode) { Exit-Json $result } - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args } If ($password) { @@ -166,8 +174,8 @@ If ($state -eq 'present') { } If ($set_new_credentials) { $secure_password = ConvertTo-SecureString $password -AsPlainText -Force - Set-ADAccountPassword -Identity $username -Reset:$true -Confirm:$false -NewPassword $secure_password -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Set-ADAccountPassword -Identity $user_guid -Reset:$true -Confirm:$false -NewPassword $secure_password -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.password_updated = $true $result.changed = $true } @@ -175,40 +183,40 @@ If ($state -eq 'present') { # Configure password policies If (($null -ne $password_never_expires) -and ($password_never_expires -ne $user_obj.PasswordNeverExpires)) { - Set-ADUser -Identity $username -PasswordNeverExpires $password_never_expires -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Set-ADUser -Identity $user_guid -PasswordNeverExpires $password_never_expires -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.changed = $true } If (($null -ne $password_expired) -and ($password_expired -ne $user_obj.PasswordExpired)) { - Set-ADUser -Identity $username -ChangePasswordAtLogon $password_expired -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Set-ADUser -Identity $user_guid -ChangePasswordAtLogon $password_expired -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.changed = $true } If (($null -ne $user_cannot_change_password) -and ($user_cannot_change_password -ne $user_obj.CannotChangePassword)) { - Set-ADUser -Identity $username -CannotChangePassword $user_cannot_change_password -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Set-ADUser -Identity $user_guid -CannotChangePassword $user_cannot_change_password -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.changed = $true } # Assign other account settings If (($null -ne $upn) -and ($upn -ne $user_obj.UserPrincipalName)) { - Set-ADUser -Identity $username -UserPrincipalName $upn -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Set-ADUser -Identity $user_guid -UserPrincipalName $upn -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.changed = $true } If (($null -ne $description) -and ($description -ne $user_obj.Description)) { - Set-ADUser -Identity $username -description $description -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Set-ADUser -Identity $user_guid -description $description -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.changed = $true } If ($enabled -ne $user_obj.Enabled) { - Set-ADUser -Identity $username -Enabled $enabled -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Set-ADUser -Identity $user_guid -Enabled $enabled -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.changed = $true } If ((-not $account_locked) -and ($user_obj.LockedOut -eq $true)) { - Unlock-ADAccount -Identity $username -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Unlock-ADAccount -Identity $user_guid -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.changed = $true } @@ -221,9 +229,9 @@ If ($state -eq 'present') { If ($value -ne $user_obj.$key) { $set_args = $extra_args.Clone() $set_args.$key = $value - Set-ADUser -Identity $username -WhatIf:$check_mode @set_args + Set-ADUser -Identity $user_guid -WhatIf:$check_mode @set_args $result.changed = $true - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args } } @@ -261,7 +269,7 @@ If ($state -eq 'present') { try { $user_obj = $user_obj | Set-ADUser -WhatIf:$check_mode -PassThru @set_args } catch { - Fail-Json $result "failed to change user $($username): $($_.Exception.Message)" + Fail-Json $result "failed to change user $($name): $($_.Exception.Message)" } $result.changed = $true } @@ -277,7 +285,7 @@ If ($state -eq 'present') { } $assigned_groups = @() - Foreach ($group in (Get-ADPrincipalGroupMembership -Identity $username @extra_args)) { + Foreach ($group in (Get-ADPrincipalGroupMembership -Identity $user_guid @extra_args)) { $assigned_groups += $group.DistinguishedName } @@ -285,8 +293,8 @@ If ($state -eq 'present') { "add" { Foreach ($group in $groups) { If (-not ($assigned_groups -Contains $group)) { - Add-ADGroupMember -Identity $group -Members $username -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Add-ADGroupMember -Identity $group -Members $user_guid -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.changed = $true } } @@ -294,8 +302,8 @@ If ($state -eq 'present') { "remove" { Foreach ($group in $groups) { If ($assigned_groups -Contains $group) { - Remove-ADGroupMember -Identity $group -Members $username -Confirm:$false -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Remove-ADGroupMember -Identity $group -Members $user_guid -Confirm:$false -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.changed = $true } } @@ -303,15 +311,15 @@ If ($state -eq 'present') { "replace" { Foreach ($group in $assigned_groups) { If (($group -ne $user_obj.PrimaryGroup) -and -not ($groups -Contains $group)) { - Remove-ADGroupMember -Identity $group -Members $username -Confirm:$false -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Remove-ADGroupMember -Identity $group -Members $user_guid -Confirm:$false -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.changed = $true } } Foreach ($group in $groups) { If (-not ($assigned_groups -Contains $group)) { - Add-ADGroupMember -Identity $group -Members $username -WhatIf:$check_mode @extra_args - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + Add-ADGroupMember -Identity $group -Members $user_guid -WhatIf:$check_mode @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.changed = $true } } @@ -331,7 +339,7 @@ If ($state -eq 'present') { } If ($user_obj) { - $user_obj = Get-ADUser -Identity $username -Properties * @extra_args + $user_obj = Get-ADUser -Identity $user_guid -Properties * @extra_args $result.name = $user_obj.Name $result.firstname = $user_obj.GivenName $result.surname = $user_obj.Surname @@ -352,16 +360,16 @@ If ($user_obj) { $result.sid = [string]$user_obj.SID $result.upn = $user_obj.UserPrincipalName $user_groups = @() - Foreach ($group in (Get-ADPrincipalGroupMembership $username @extra_args)) { + Foreach ($group in (Get-ADPrincipalGroupMembership $user_guid @extra_args)) { $user_groups += $group.name } $result.groups = $user_groups - $result.msg = "User '$username' is present" + $result.msg = "User '$name' is present" $result.state = "present" } Else { - $result.name = $username - $result.msg = "User '$username' is absent" + $result.name = $name + $result.msg = "User '$name' is absent" $result.state = "absent" } diff --git a/lib/ansible/modules/windows/win_domain_user.py b/lib/ansible/modules/windows/win_domain_user.py index ea01d64f07d..1880ece176a 100644 --- a/lib/ansible/modules/windows/win_domain_user.py +++ b/lib/ansible/modules/windows/win_domain_user.py @@ -23,6 +23,14 @@ options: - Name of the user to create, remove or modify. type: str required: true + identity: + description: + - Identity parameter used to find the User in the Active Directory. + - This value can be in the forms C(Distinguished Name), C(objectGUID), + C(objectSid) or C(sAMAccountName). + - Default to C(name) if not set. + type: str + version_added: '2.10' state: description: - When C(present), creates or updates the user account.