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

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)"

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
+                        }
+                        $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)"
+
+                        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)"
+
+                        Continue
+                    }
+
+                    # user got requested permissions
+                    $permissionRead.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
-                    }
-                    $result.changed = $true
-                    $result.actions += "Revoke-SmbShareAccess -Force -Name $name -AccountName $($permission.AccountName)"
-
-                    Continue
+        }
+    } ElseIf ($rule_action -eq "add") {
+        ForEach($permission in $permissions) {
+            If ($permission.AccessControlType -eq "Deny") {
+                If ($permissionDeny.Contains($permission.AccountName)) {
+                    $permissionDeny.Remove($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
-                    }
-                    $result.changed = $true
-                    $result.actions += "Revoke-SmbShareAccess -Force -Name $name -AccountName $($permission.AccountName)"
-
-                    Continue
+            } 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
-                $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)"
-
-                    Continue
-                }
-
-                # user got requested permissions
-                $permissionRead.Remove($permission.AccountName) | Out-Null
             }
         }
     }

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'''

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}}"