win_acl_inheritance: Added tests and formatting improvements (#25382)

lib/ansible/modules/windows/win_acl_inheritance.ps1

@@ -19,38 +19,39 @@
 # WANT_JSON
 # POWERSHELL_COMMON
 
-
+$params = Parse-Args $args -supports_check_mode $true
-$params = Parse-Args $args;
+$check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -default $false
 
 $result = @{
     changed = $false
 }
 
-$path = Get-Attr $params "path" -failifempty $true
+$path = Get-AnsibleParam -obj $params "path" -type "path" -failifempty $true
-$state = Get-Attr $params "state" "absent" -validateSet "present","absent" -resultobj $result
+$state = Get-AnsibleParam -obj $params "state" -type "str" -default "absent" -validateSet "present","absent" -resultobj $result
-$reorganize = Get-Attr $params "reorganize" "no" -validateSet "no","yes" -resultobj $result
+$reorganize = Get-AnsibleParam -obj $params "reorganize" -type "bool" -default $false -resultobj $result
-$reorganize = $reorganize | ConvertTo-Bool
 
 If (-Not (Test-Path -Path $path)) {
     Fail-Json $result "$path file or directory does not exist on the host"
 }
  
 Try {
-    $objACL = Get-ACL $path
+    $objACL = Get-ACL -Path $path
-    $inheritanceEnabled = !$objACL.AreAccessRulesProtected
+    # AreAccessRulesProtected - $false if inheritance is set ,$true if inheritance is not set
+    $inheritanceDisabled = $objACL.AreAccessRulesProtected
 
-    If (($state -eq "present") -And !$inheritanceEnabled) {
+    If (($state -eq "present") -And $inheritanceDisabled) {
         # second parameter is ignored if first=$False
         $objACL.SetAccessRuleProtection($False, $False)
 
         If ($reorganize) {
-            # it won't work without intermediate save, state would be the same
+            # it wont work without intermediate save, state would be the same
-            Set-ACL $path $objACL
+            Set-ACL -Path $path -AclObject $objACL -WhatIf:$check_mode
-            $objACL = Get-ACL $path
+            $result.changed = $true
+            $objACL = Get-ACL -Path $path
 
             # convert explicit ACE to inherited ACE
             ForEach($inheritedRule in $objACL.Access) {
-                If (!$inheritedRule.IsInherited) {
+                If (-not $inheritedRule.IsInherited) {
                     Continue
                 }
 
@@ -66,22 +67,15 @@ Try {
             }
         }
 
-        Set-ACL $path $objACL
+        Set-ACL -Path $path -AclObject $objACL -WhatIf:$check_mode
+        $result.changed = $true
+    } Elseif (($state -eq "absent") -And (-not $inheritanceDisabled)) {
+        $objACL.SetAccessRuleProtection($True, $reorganize)
+        Set-ACL -Path $path -AclObject $objACL -WhatIf:$check_mode
         $result.changed = $true
     }
-    Elseif (($state -eq "absent") -And $inheritanceEnabled) {
+} Catch {
-        If ($reorganize) {
+    Fail-Json $result "an error occurred when attempting to disable inheritance: $($_.Exception.Message)"
-            $objACL.SetAccessRuleProtection($True, $True)
-        } Else {
-            $objACL.SetAccessRuleProtection($True, $False)
-        }
-
-        Set-ACL $path $objACL
-        $result.changed = $true
-    }
-}
-Catch {
-    Fail-Json $result "an error occurred when attempting to disable inheritance"
 }
  
 Exit-Json $result

test/integration/targets/win_acl_inheritance/aliases

@@ -0,0 +1 @@
+windows/ci.group1

test/integration/targets/win_acl_inheritance/defaults/main.yml

@@ -0,0 +1 @@
+test_win_acl_inheritance_path: C:\ansible\win_acl_inheritance

test/integration/targets/win_acl_inheritance/library/test_get_acl.ps1

@@ -0,0 +1,44 @@
+#!powershell
+
+# WANT_JSON
+# POWERSHELL_COMMON
+
+$ErrorActionPreference = 'Stop'
+Set-StrictMode -Version 2.0
+
+$params = Parse-Args $args -supports_check_mode $false
+$path = Get-AnsibleParam -obj $params "path" -type "path" -failifempty $true
+
+$result = @{
+    changed = $false
+}
+
+$acl = Get-Acl -Path $path
+
+$result.inherited = $acl.AreAccessRulesProtected -eq $false
+
+$user_details = @{}
+$acl.Access | ForEach-Object {
+    # Backslashes are the bane of my existance, convert to / to we can export to JSON
+    $user = $_.IdentityReference -replace '\\','/'
+    if ($user_details.ContainsKey($user)) {
+        $details = $user_details.$user
+    } else {
+        $details = @{
+            isinherited = $false
+            isnotinherited = $false
+        }
+    }
+
+    if ($_.IsInherited) {
+        $details.isinherited = $true
+    } else {
+        $details.isnotinherited = $true
+    }
+
+    $user_details.$user = $details
+}
+
+$result.user_details = $user_details
+
+Exit-Json $result

test/integration/targets/win_acl_inheritance/tasks/main.yml

@@ -0,0 +1,173 @@
+---
+ # Test setup
+- name: remove test folder for baseline
+  win_file:
+    path: '{{test_win_acl_inheritance_path}}'
+    state: absent
+
+- name: create test folders
+  win_file:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+    state: directory
+
+- name: create test files
+  win_copy:
+    dest: '{{test_win_acl_inheritance_path}}\folder\file.txt'
+    content: a
+
+# Run tests
+- name: remove inheritance check
+  win_acl_inheritance:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+    reorganize: True
+    state: absent
+  register: remove_check
+  check_mode: True
+
+- name: get actual remove inheritance check
+  test_get_acl:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+  register: actual_remove_check
+
+- name: assert remove inheritance check
+  assert:
+    that:
+    - remove_check|changed
+    - actual_remove_check.inherited == True
+
+- name: remove inheritance
+  win_acl_inheritance:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+    reorganize: True
+    state: absent
+  register: remove
+
+- name: get actual remove inheritance
+  test_get_acl:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+  register: actual_remove
+
+- name: assert remove inheritance
+  assert:
+    that:
+    - remove|changed
+    - actual_remove.inherited == False
+    - actual_remove.user_details['BUILTIN/Administrators'].isinherited == False
+    - actual_remove.user_details['BUILTIN/Administrators'].isnotinherited == True
+    - actual_remove.user_details['BUILTIN/Users'].isinherited == False
+    - actual_remove.user_details['BUILTIN/Users'].isnotinherited == True
+    - actual_remove.user_details['CREATOR OWNER'].isinherited == False
+    - actual_remove.user_details['CREATOR OWNER'].isnotinherited == True
+    - actual_remove.user_details['NT AUTHORITY/SYSTEM'].isinherited == False
+    - actual_remove.user_details['NT AUTHORITY/SYSTEM'].isnotinherited == True
+
+- name: remove inheritance again
+  win_acl_inheritance:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+    reorganize: True
+    state: absent
+  register: remove_again
+
+- name: get actual remove inheritance again
+  test_get_acl:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+  register: actual_remove_again
+
+- name: assert remove inheritance again
+  assert:
+    that:
+    - not remove_again|changed
+    - actual_remove_again.inherited == False
+    - actual_remove.user_details['BUILTIN/Administrators'].isinherited == False
+    - actual_remove.user_details['BUILTIN/Administrators'].isnotinherited == True
+    - actual_remove.user_details['BUILTIN/Users'].isinherited == False
+    - actual_remove.user_details['BUILTIN/Users'].isnotinherited == True
+    - actual_remove.user_details['CREATOR OWNER'].isinherited == False
+    - actual_remove.user_details['CREATOR OWNER'].isnotinherited == True
+    - actual_remove.user_details['NT AUTHORITY/SYSTEM'].isinherited == False
+    - actual_remove.user_details['NT AUTHORITY/SYSTEM'].isnotinherited == True
+
+- name: add inheritance check
+  win_acl_inheritance:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+    reorganize: True
+    state: present
+  register: add_check
+  check_mode: True
+
+- name: get actual add inheritance check
+  test_get_acl:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+  register: actual_add_check
+
+- name: assert add inheritance check
+  assert:
+    that:
+    - add_check|changed
+    - actual_add_check.inherited == False
+    - actual_add_check.user_details['BUILTIN/Administrators'].isinherited == False
+    - actual_add_check.user_details['BUILTIN/Administrators'].isnotinherited == True
+    - actual_add_check.user_details['BUILTIN/Users'].isinherited == False
+    - actual_add_check.user_details['BUILTIN/Users'].isnotinherited == True
+    - actual_add_check.user_details['CREATOR OWNER'].isinherited == False
+    - actual_add_check.user_details['CREATOR OWNER'].isnotinherited == True
+    - actual_add_check.user_details['NT AUTHORITY/SYSTEM'].isinherited == False
+    - actual_add_check.user_details['NT AUTHORITY/SYSTEM'].isnotinherited == True
+
+- name: add inheritance
+  win_acl_inheritance:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+    reorganize: True
+    state: present
+  register: add
+
+- name: get actual add inheritance
+  test_get_acl:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+  register: actual_add
+
+- name: assert add inheritance
+  assert:
+    that:
+    - add|changed
+    - actual_add.inherited == True
+    - actual_add.user_details['BUILTIN/Administrators'].isinherited == True
+    - actual_add.user_details['BUILTIN/Administrators'].isnotinherited == False
+    - actual_add.user_details['BUILTIN/Users'].isinherited == True
+    - actual_add.user_details['BUILTIN/Users'].isnotinherited == True # Bug in win_acl_inheritance, resetting inheritance doubles up entries
+    - actual_add.user_details['CREATOR OWNER'].isinherited == True
+    - actual_add.user_details['CREATOR OWNER'].isnotinherited == False
+    - actual_add.user_details['NT AUTHORITY/SYSTEM'].isinherited == True
+    - actual_add.user_details['NT AUTHORITY/SYSTEM'].isnotinherited == False
+
+- name: add inheritance again
+  win_acl_inheritance:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+    reorganize: True
+    state: present
+  register: add_again
+
+- name: get actual add inheritance again
+  test_get_acl:
+    path: '{{test_win_acl_inheritance_path}}\folder'
+  register: actual_add_again
+
+- name: assert add inheritance again
+  assert:
+    that:
+    - not add_again|changed
+    - actual_add_again.inherited == True
+    - actual_add_again.user_details['BUILTIN/Administrators'].isinherited == True
+    - actual_add_again.user_details['BUILTIN/Administrators'].isnotinherited == False
+    - actual_add_again.user_details['BUILTIN/Users'].isinherited == True
+    - actual_add_again.user_details['BUILTIN/Users'].isnotinherited == True # Bug in win_acl_inheritance, resetting inheritance doubles up entries
+    - actual_add_again.user_details['CREATOR OWNER'].isinherited == True
+    - actual_add_again.user_details['CREATOR OWNER'].isnotinherited == False
+    - actual_add_again.user_details['NT AUTHORITY/SYSTEM'].isinherited == True
+    - actual_add_again.user_details['NT AUTHORITY/SYSTEM'].isnotinherited == False
+
+# Test cleanup
+- name: remove test folder
+  win_file:
+    path: '{{test_win_acl_inheritance_path}}'
+    state: absent