win_acl_inheritance - fix glob like paths (#53829)

changelogs/fragments/win_acl_inheritance-paths.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+- win_acl_inheritance - Fix issues when using paths with glob like characters, e.g. ``[``, ``]``

lib/ansible/modules/windows/win_acl_inheritance.ps1

@@ -16,12 +16,12 @@ $path = Get-AnsibleParam -obj $params "path" -type "path" -failifempty $true
 $state = Get-AnsibleParam -obj $params "state" -type "str" -default "absent" -validateSet "present","absent" -resultobj $result
 $reorganize = Get-AnsibleParam -obj $params "reorganize" -type "bool" -default $false -resultobj $result
 
-If (-Not (Test-Path -Path $path)) {
+If (-Not (Test-Path -LiteralPath $path)) {
     Fail-Json $result "$path file or directory does not exist on the host"
 }
  
 Try {
-    $objACL = Get-ACL -Path $path
+    $objACL = Get-ACL -LiteralPath $path
     # AreAccessRulesProtected - $false if inheritance is set ,$true if inheritance is not set
     $inheritanceDisabled = $objACL.AreAccessRulesProtected
 
@@ -31,9 +31,9 @@ Try {
 
         If ($reorganize) {
             # it wont work without intermediate save, state would be the same
-            Set-ACL -Path $path -AclObject $objACL -WhatIf:$check_mode
+            Set-ACL -LiteralPath $path -AclObject $objACL -WhatIf:$check_mode
             $result.changed = $true
-            $objACL = Get-ACL -Path $path
+            $objACL = Get-ACL -LiteralPath $path
 
             # convert explicit ACE to inherited ACE
             ForEach($inheritedRule in $objACL.Access) {
@@ -53,11 +53,11 @@ Try {
             }
         }
 
-        Set-ACL -Path $path -AclObject $objACL -WhatIf:$check_mode
+        Set-ACL -LiteralPath $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
+        Set-ACL -LiteralPath $path -AclObject $objACL -WhatIf:$check_mode
         $result.changed = $true
     }
 } Catch {

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

@@ -1 +1 @@
-test_win_acl_inheritance_path: C:\ansible\win_acl_inheritance
+test_win_acl_inheritance_path: C:\ansible\win_acl_inheritance .ÅÑŚÌβŁÈ [$!@^&test(;)]

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

@@ -13,29 +13,21 @@ $result = @{
     changed = $false
 }
 
-$acl = Get-Acl -Path $path
+$acl = Get-Acl -LiteralPath $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 '\\','/'
+    $user = $_.IdentityReference.Translate([System.Security.Principal.SecurityIdentifier]).Value
     if ($user_details.ContainsKey($user)) {
         $details = $user_details.$user
     } else {
         $details = @{
             isinherited = $false
-            isnotinherited = $false
         }
     }
-
-    if ($_.IsInherited) {
-        $details.isinherited = $true
-    } else {
-        $details.isnotinherited = $true
-    }
-
+    $details.isinherited = $_.IsInherited
     $user_details.$user = $details
 }
 

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

@@ -1,24 +1,65 @@
 ---
- # Test setup
-- name: remove test folder for baseline
-  win_file:
-    path: '{{test_win_acl_inheritance_path}}'
-    state: absent
-
+# Test setup
+# Use single task to save in CI runtime
 - name: create test folders
-  win_file:
-    path: '{{test_win_acl_inheritance_path}}\folder'
-    state: directory
+  win_shell: |
+    $ErrorActionPreference = 'Stop'
 
-- name: create test files
-  win_copy:
-    dest: '{{test_win_acl_inheritance_path}}\folder\file.txt'
-    content: a
+    $tmp_dir = '{{ test_win_acl_inheritance_path }}'
+    if (Test-Path -LiteralPath $tmp_dir) {
+        Remove-Item -LiteralPath $tmp_dir -Force -Recurse
+    }
+    New-Item -Path $tmp_dir -ItemType Directory > $null
+
+    Add-Type -AssemblyName System.DirectoryServices.AccountManagement
+    $current_sid = ([System.DirectoryServices.AccountManagement.UserPrincipal]::Current).Sid
+    $system_sid = New-Object -TypeName System.Security.Principal.SecurityIdentifier -ArgumentList @([System.Security.Principal.WellKnownSidType]::LocalSystemSid, $null)
+    $everyone_sid = New-Object -TypeName System.Security.Principal.SecurityIdentifier -ArgumentList @([System.Security.Principal.WellKnownSidType]::WorldSid, $null)
+
+    $sd = New-Object -TypeName System.Security.AccessControl.DirectorySecurity
+    $sd.SetAccessRuleProtection($true, $false)
+    $sd.AddAccessRule(
+        (New-Object -TypeName System.Security.AccessControl.FileSystemAccessRule -ArgumentList @(
+            $system_sid,
+            [System.Security.AccessControl.FileSystemRights]::FullControl,
+            [System.Security.AccessControl.InheritanceFlags]"ContainerInherit, ObjectInherit",
+            [System.Security.AccessControl.PropagationFlags]::None,
+            [System.Security.AccessControl.AccessControlType]::Allow
+        ))
+    )
+    $sd.AddAccessRule(
+        (New-Object -TypeName System.Security.AccessControl.FileSystemAccessRule -ArgumentList @(
+            $current_sid,
+            [System.Security.AccessControl.FileSystemRights]::FullControl,
+            [System.Security.AccessControl.InheritanceFlags]"ContainerInherit, ObjectInherit",
+            [System.Security.AccessControl.PropagationFlags]::None,
+            [System.Security.AccessControl.AccessControlType]::Allow
+        ))
+    )
+    $sd.AddAccessRule(
+        (New-Object -TypeName System.Security.AccessControl.FileSystemAccessRule -ArgumentList @(
+            $everyone_sid,
+            [System.Security.AccessControl.FileSystemRights]::Read,
+            [System.Security.AccessControl.InheritanceFlags]"ContainerInherit, ObjectInherit",
+            [System.Security.AccessControl.PropagationFlags]::None,
+            [System.Security.AccessControl.AccessControlType]::Allow
+        ))
+    )
+
+    Set-Acl -LiteralPath $tmp_dir -AclObject $sd
+
+    New-Item -Path "$tmp_dir\folder" -ItemType Directory > $null
+    Set-Content -LiteralPath "$tmp_dir\folder\file.txt" -Value 'a'
+
+    $system_sid.Value
+    $current_sid.Value
+    $everyone_sid.Value
+  register: test_sids  # register the output SID values used for comparison tests below
 
 # Run tests
 - name: remove inheritance check
   win_acl_inheritance:
-    path: '{{test_win_acl_inheritance_path}}\folder'
+    path: '{{ test_win_acl_inheritance_path }}\folder'
     reorganize: True
     state: absent
   register: remove_check
@@ -26,7 +67,7 @@
 
 - name: get actual remove inheritance check
   test_get_acl:
-    path: '{{test_win_acl_inheritance_path}}\folder'
+    path: '{{ test_win_acl_inheritance_path }}\folder'
   register: actual_remove_check
 
 - name: assert remove inheritance check
@@ -34,17 +75,20 @@
     that:
     - remove_check is changed
     - actual_remove_check.inherited == True
+    - actual_remove_check.user_details[test_sids.stdout_lines[0]].isinherited == True
+    - actual_remove_check.user_details[test_sids.stdout_lines[1]].isinherited == True
+    - actual_remove_check.user_details[test_sids.stdout_lines[2]].isinherited == True
 
 - name: remove inheritance
   win_acl_inheritance:
-    path: '{{test_win_acl_inheritance_path}}\folder'
+    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'
+    path: '{{ test_win_acl_inheritance_path }}\folder'
   register: actual_remove
 
 - name: assert remove inheritance
@@ -52,44 +96,25 @@
     that:
     - remove is 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
+    - actual_remove.user_details[test_sids.stdout_lines[0]].isinherited == False
+    - actual_remove.user_details[test_sids.stdout_lines[1]].isinherited == False
+    - actual_remove.user_details[test_sids.stdout_lines[2]].isinherited == False
 
 - name: remove inheritance again
   win_acl_inheritance:
-    path: '{{test_win_acl_inheritance_path}}\folder'
+    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:
     - remove_again is not 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'
+    path: '{{ test_win_acl_inheritance_path }}\folder'
     reorganize: True
     state: present
   register: add_check
@@ -97,7 +122,7 @@
 
 - name: get actual add inheritance check
   test_get_acl:
-    path: '{{test_win_acl_inheritance_path}}\folder'
+    path: '{{ test_win_acl_inheritance_path }}\folder'
   register: actual_add_check
 
 - name: assert add inheritance check
@@ -105,25 +130,20 @@
     that:
     - add_check is 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
+    - actual_add_check.user_details[test_sids.stdout_lines[0]].isinherited == False
+    - actual_add_check.user_details[test_sids.stdout_lines[1]].isinherited == False
+    - actual_add_check.user_details[test_sids.stdout_lines[2]].isinherited == False
 
 - name: add inheritance
   win_acl_inheritance:
-    path: '{{test_win_acl_inheritance_path}}\folder'
+    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'
+    path: '{{ test_win_acl_inheritance_path }}\folder'
   register: actual_add
 
 - name: assert add inheritance
@@ -131,43 +151,24 @@
     that:
     - add is 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
+    - actual_add.user_details[test_sids.stdout_lines[0]].isinherited == True
+    - actual_add.user_details[test_sids.stdout_lines[1]].isinherited == True
+    - actual_add.user_details[test_sids.stdout_lines[2]].isinherited == True
 
 - name: add inheritance again
   win_acl_inheritance:
-    path: '{{test_win_acl_inheritance_path}}\folder'
+    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:
     - add_again is not 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}}'
+    path: '{{ test_win_acl_inheritance_path }}'
     state: absent