From 584824f560dd88b4f35a4632e082e5945b0495bd Mon Sep 17 00:00:00 2001 From: Shachaf92 Date: Wed, 4 Dec 2019 06:16:10 +0200 Subject: [PATCH] win_share - Implement append paramtere for access rules (#59469) * win_share - Implement append paramtere for access rules * changed fragment * add test * missing bracket * removed whitespace * Wrong number of lines * Forgot the actual new parameter in the test * community review * Change option names * version update * Update tests.yml * Add idempotence to rule_action: add --- ...ment-append-paramtere-for-access-rules.yml | 2 + lib/ansible/modules/windows/win_share.ps1 | 115 ++++++++++-------- lib/ansible/modules/windows/win_share.py | 7 ++ .../targets/win_share/tasks/tests.yml | 38 ++++++ 4 files changed, 114 insertions(+), 48 deletions(-) create mode 100644 changelogs/fragments/win_share-Implement-append-paramtere-for-access-rules.yml diff --git a/changelogs/fragments/win_share-Implement-append-paramtere-for-access-rules.yml b/changelogs/fragments/win_share-Implement-append-paramtere-for-access-rules.yml new file mode 100644 index 00000000000..260f0688b84 --- /dev/null +++ b/changelogs/fragments/win_share-Implement-append-paramtere-for-access-rules.yml @@ -0,0 +1,2 @@ +minor_changes: + - "win_share - Implement append parameter for access rules (https://github.com/ansible/ansible/issues/59237)" \ No newline at end of file diff --git a/lib/ansible/modules/windows/win_share.ps1 b/lib/ansible/modules/windows/win_share.ps1 index eadce970c07..d48f74d07e8 100644 --- a/lib/ansible/modules/windows/win_share.ps1 +++ b/lib/ansible/modules/windows/win_share.ps1 @@ -47,6 +47,7 @@ $check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "b $name = Get-AnsibleParam -obj $params -name "name" -type "str" -failifempty $true $state = Get-AnsibleParam -obj $params -name "state" -type "str" -default "present" -validateset "present","absent" +$rule_action = Get-AnsibleParam -obj $params -name "rule_action" -type "str" -default "set" -validateset "set","add" if (-not (Get-Command -Name Get-SmbShare -ErrorAction SilentlyContinue)) { Fail-Json $result "The current host does not support the -SmbShare cmdlets required by this module. Please run on Server 2012 or Windows 8 and later" @@ -151,65 +152,83 @@ If ($state -eq "absent") { # remove permissions $permissions = Get-SmbShareAccess -Name $name - ForEach ($permission in $permissions) { - If ($permission.AccessControlType -eq "Deny") { - $cim_count = 0 - foreach ($count in $permissions) { - $cim_count++ - } - # Don't remove the Deny entry for Everyone if there are no other permissions set (cim_count == 1) - if (-not ($permission.AccountName -eq 'Everyone' -and $cim_count -eq 1)) { - If (-not ($permissionDeny.Contains($permission.AccountName))) { - if (-not $check_mode) { - Unblock-SmbShareAccess -Force -Name $name -AccountName $permission.AccountName | Out-Null + if($rule_action -eq "set") { + ForEach ($permission in $permissions) { + If ($permission.AccessControlType -eq "Deny") { + $cim_count = 0 + foreach ($count in $permissions) { + $cim_count++ + } + # Don't remove the Deny entry for Everyone if there are no other permissions set (cim_count == 1) + if (-not ($permission.AccountName -eq 'Everyone' -and $cim_count -eq 1)) { + If (-not ($permissionDeny.Contains($permission.AccountName))) { + if (-not $check_mode) { + Unblock-SmbShareAccess -Force -Name $name -AccountName $permission.AccountName | Out-Null + } + $result.changed = $true + $result.actions += "Unblock-SmbShareAccess -Force -Name $name -AccountName $($permission.AccountName)" + } else { + # Remove from the deny list as it already has the permissions + $permissionDeny.remove($permission.AccountName) | Out-Null } - $result.changed = $true - $result.actions += "Unblock-SmbShareAccess -Force -Name $name -AccountName $($permission.AccountName)" - } else { - # Remove from the deny list as it already has the permissions - $permissionDeny.remove($permission.AccountName) | Out-Null } - } - } ElseIf ($permission.AccessControlType -eq "Allow") { - If ($permission.AccessRight -eq "Full") { - If (-not ($permissionFull.Contains($permission.AccountName))) { - if (-not $check_mode) { - Revoke-SmbShareAccess -Force -Name $name -AccountName $permission.AccountName | Out-Null + } ElseIf ($permission.AccessControlType -eq "Allow") { + If ($permission.AccessRight -eq "Full") { + If (-not ($permissionFull.Contains($permission.AccountName))) { + if (-not $check_mode) { + Revoke-SmbShareAccess -Force -Name $name -AccountName $permission.AccountName | Out-Null + } + $result.changed = $true + $result.actions += "Revoke-SmbShareAccess -Force -Name $name -AccountName $($permission.AccountName)" + + Continue } - $result.changed = $true - $result.actions += "Revoke-SmbShareAccess -Force -Name $name -AccountName $($permission.AccountName)" - Continue - } + # user got requested permissions + $permissionFull.remove($permission.AccountName) | Out-Null + } ElseIf ($permission.AccessRight -eq "Change") { + If (-not ($permissionChange.Contains($permission.AccountName))) { + if (-not $check_mode) { + Revoke-SmbShareAccess -Force -Name $name -AccountName $permission.AccountName | Out-Null + } + $result.changed = $true + $result.actions += "Revoke-SmbShareAccess -Force -Name $name -AccountName $($permission.AccountName)" - # user got requested permissions - $permissionFull.remove($permission.AccountName) | Out-Null - } ElseIf ($permission.AccessRight -eq "Change") { - If (-not ($permissionChange.Contains($permission.AccountName))) { - if (-not $check_mode) { - Revoke-SmbShareAccess -Force -Name $name -AccountName $permission.AccountName | Out-Null + Continue } - $result.changed = $true - $result.actions += "Revoke-SmbShareAccess -Force -Name $name -AccountName $($permission.AccountName)" - Continue - } + # user got requested permissions + $permissionChange.remove($permission.AccountName) | Out-Null + } ElseIf ($permission.AccessRight -eq "Read") { + If (-not ($permissionRead.Contains($permission.AccountName))) { + if (-not $check_mode) { + Revoke-SmbShareAccess -Force -Name $name -AccountName $permission.AccountName | Out-Null + } + $result.changed = $true + $result.actions += "Revoke-SmbShareAccess -Force -Name $name -AccountName $($permission.AccountName)" - # user got requested permissions - $permissionChange.remove($permission.AccountName) | Out-Null - } ElseIf ($permission.AccessRight -eq "Read") { - If (-not ($permissionRead.Contains($permission.AccountName))) { - if (-not $check_mode) { - Revoke-SmbShareAccess -Force -Name $name -AccountName $permission.AccountName | Out-Null + Continue } - $result.changed = $true - $result.actions += "Revoke-SmbShareAccess -Force -Name $name -AccountName $($permission.AccountName)" - Continue + # user got requested permissions + $permissionRead.Remove($permission.AccountName) | Out-Null + } + } + } + } ElseIf ($rule_action -eq "add") { + ForEach($permission in $permissions) { + If ($permission.AccessControlType -eq "Deny") { + If ($permissionDeny.Contains($permission.AccountName)) { + $permissionDeny.Remove($permission.AccountName) + } + } ElseIf ($permission.AccessControlType -eq "Allow") { + If ($permissionFull.Contains($permission.AccountName) -and $permission.AccessRight -eq "Full") { + $permissionFull.Remove($permission.AccountName) + } ElseIf ($permissionChange.Contains($permission.AccountName) -and $permission.AccessRight -eq "Change") { + $permissionChange.Remove($permission.AccountName) + } ElseIf ($permissionRead.Contains($permission.AccountName) -and $permission.AccessRight -eq "Read") { + $permissionRead.Remove($permission.AccountName) } - - # user got requested permissions - $permissionRead.Remove($permission.AccountName) | Out-Null } } } diff --git a/lib/ansible/modules/windows/win_share.py b/lib/ansible/modules/windows/win_share.py index 0a730f7879c..c0b005a5adf 100644 --- a/lib/ansible/modules/windows/win_share.py +++ b/lib/ansible/modules/windows/win_share.py @@ -75,9 +75,16 @@ options: type: bool default: no version_added: "2.4" + rule_action: + description: Whether to add or set (replace) access control entries. + type: str + choices: [ set, add ] + default: set + version_added: "2.10" author: - Hans-Joachim Kliemeck (@h0nIg) - David Baumann (@daBONDi) + - Shachaf Goldstein (@Shachaf92) ''' EXAMPLES = r''' diff --git a/test/integration/targets/win_share/tasks/tests.yml b/test/integration/targets/win_share/tasks/tests.yml index c120491b28c..543f7bae89a 100644 --- a/test/integration/targets/win_share/tasks/tests.yml +++ b/test/integration/targets/win_share/tasks/tests.yml @@ -398,6 +398,44 @@ - set_acl_actual_again.stdout_lines[2] == 'Change|Allow|BUILTIN\\Users' - set_acl_actual_again.stdout_lines[3] == 'Full|Allow|BUILTIN\\Administrators' +- name: Append ACLs on share + win_share: + name: "{{test_win_share_name}}" + state: present + path: "{{test_win_share_path}}" + change: Remote Desktop Users + rule_action: add + register: append_acl + +- name: get actual share ACLs + win_shell: foreach ($acl in Get-SmbShareAccess -Name '{{test_win_share_name}}') { Write-Host "$($acl.AccessRight)|$($acl.AccessControlType)|$($acl.AccountName)" } + register: append_acl_actual + +- name: assert Append ACLs on share + assert: + that: + - append_acl is changed + - append_acl_actual.stdout_lines|length == 5 + - append_acl_actual.stdout_lines[0] == 'Full|Deny|BUILTIN\Remote Desktop Users' + - append_acl_actual.stdout_lines[1] == 'Read|Allow|BUILTIN\\Guests' + - append_acl_actual.stdout_lines[2] == 'Change|Allow|BUILTIN\\Users' + - append_acl_actual.stdout_lines[3] == 'Full|Allow|BUILTIN\\Administrators' + - append_acl_actual.stdout_lines[4] == 'Change|Allow|BUILTIN\\Remote Desktop Users' + +- name: Append ACLs on share (idempotent) + win_share: + name: "{{test_win_share_name}}" + state: present + path: "{{test_win_share_path}}" + change: Remote Desktop Users + rule_action: add + register: append_acl_again + +- name: assert Append ACLs on share (idempotent) + assert: + that: + - not append_acl_again is changed + - name: remove share check win_share: name: "{{test_win_share_name}}"