From 6d0116823821197b329decab8221c55745d90adb Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Tue, 20 Jun 2017 02:47:35 +1000 Subject: [PATCH] win_acl_inheritance: Added tests and formatting improvements (#25382) --- .../modules/windows/win_acl_inheritance.ps1 | 48 +++-- .../targets/win_acl_inheritance/aliases | 1 + .../win_acl_inheritance/defaults/main.yml | 1 + .../library/test_get_acl.ps1 | 44 +++++ .../win_acl_inheritance/tasks/main.yml | 173 ++++++++++++++++++ 5 files changed, 240 insertions(+), 27 deletions(-) create mode 100644 test/integration/targets/win_acl_inheritance/aliases create mode 100644 test/integration/targets/win_acl_inheritance/defaults/main.yml create mode 100644 test/integration/targets/win_acl_inheritance/library/test_get_acl.ps1 create mode 100644 test/integration/targets/win_acl_inheritance/tasks/main.yml diff --git a/lib/ansible/modules/windows/win_acl_inheritance.ps1 b/lib/ansible/modules/windows/win_acl_inheritance.ps1 index 78b495ba18..cfc8b3bc0b 100644 --- a/lib/ansible/modules/windows/win_acl_inheritance.ps1 +++ b/lib/ansible/modules/windows/win_acl_inheritance.ps1 @@ -19,38 +19,39 @@ # WANT_JSON # POWERSHELL_COMMON - -$params = Parse-Args $args; +$params = Parse-Args $args -supports_check_mode $true +$check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -default $false $result = @{ changed = $false } -$path = Get-Attr $params "path" -failifempty $true -$state = Get-Attr $params "state" "absent" -validateSet "present","absent" -resultobj $result -$reorganize = Get-Attr $params "reorganize" "no" -validateSet "no","yes" -resultobj $result -$reorganize = $reorganize | ConvertTo-Bool +$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)) { Fail-Json $result "$path file or directory does not exist on the host" } Try { - $objACL = Get-ACL $path - $inheritanceEnabled = !$objACL.AreAccessRulesProtected + $objACL = Get-ACL -Path $path + # 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 - Set-ACL $path $objACL - $objACL = Get-ACL $path + # it wont work without intermediate save, state would be the same + Set-ACL -Path $path -AclObject $objACL -WhatIf:$check_mode + $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) { - If ($reorganize) { - $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" +} Catch { + Fail-Json $result "an error occurred when attempting to disable inheritance: $($_.Exception.Message)" } Exit-Json $result diff --git a/test/integration/targets/win_acl_inheritance/aliases b/test/integration/targets/win_acl_inheritance/aliases new file mode 100644 index 0000000000..e1e7fedbc8 --- /dev/null +++ b/test/integration/targets/win_acl_inheritance/aliases @@ -0,0 +1 @@ +windows/ci.group1 diff --git a/test/integration/targets/win_acl_inheritance/defaults/main.yml b/test/integration/targets/win_acl_inheritance/defaults/main.yml new file mode 100644 index 0000000000..325bfbe81d --- /dev/null +++ b/test/integration/targets/win_acl_inheritance/defaults/main.yml @@ -0,0 +1 @@ +test_win_acl_inheritance_path: C:\ansible\win_acl_inheritance diff --git a/test/integration/targets/win_acl_inheritance/library/test_get_acl.ps1 b/test/integration/targets/win_acl_inheritance/library/test_get_acl.ps1 new file mode 100644 index 0000000000..6c73f96c0a --- /dev/null +++ b/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 diff --git a/test/integration/targets/win_acl_inheritance/tasks/main.yml b/test/integration/targets/win_acl_inheritance/tasks/main.yml new file mode 100644 index 0000000000..5bf0f51d74 --- /dev/null +++ b/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