Repository: ansible/ansible
Pull request: win_acl: Fix problems with special IDRef in different languages than english
##### SUMMARY
Fixes #56757

##### ISSUE TYPE

- Bugfix Pull Request

##### COMPONENT NAME
win_acl.ps1

##### ADDITIONAL INFORMATION

Problem statement:
win_acl: User Name to SID Conversion Issue 
<!--- Verify first that your issue is not already reported on GitHub -->
<!--- Also test if the latest release and devel branch are affected too -->
<!--- Complete *all* sections as described, this form is processed automatically -->

##### SUMMARY
When trying to set the permission using win_acl, it fails if there is the following permission.
- APPLICATION PACKAGE AUTHORITY\制限されたすべてのアプリケーション パッケージ
( A.k.a ALL RESTRICTED APPLICATION PACKAGES)

The permission used for the value of $specialIdRefs between lines 161 and 170 does not seem to hit when compared with the value of $idRefShotValue, since the value that can be obtained depending on the language actually changes.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
- win_acl.ps1

##### ANSIBLE VERSION
```
ansible 2.8.0
  config file = /usr/local/ansible/Designer/ansible.cfg
  configured module search path = ['/home/user/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.6/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.6.6 (default, Aug 13 2018, 18:24:23) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]
```

##### CONFIGURATION
<!--- Paste verbatim output from "ansible-config dump --only-changed" between quotes -->
```DEFAULT_FORKS(/usr/local/ansible/Designer/ansible.cfg) = 30
DEFAULT_LOG_PATH(/usr/local/ansible/Designer/ansible.cfg) = /usr/local/ansible/Designer/log/ansible.log
RETRY_FILES_ENABLED(/usr/local/ansible/Designer/ansible.cfg) = False
```

##### OS / ENVIRONMENT
CentOS Linux 3.10.0-693.21.1.el7.x86_64 

Target Machine:　
　Windows 10 Pro 1809(Microsoft Windows [Version 10.0.17763.379])
```$PSVersionTable

Name                           Value
----                           -----
PSVersion                      5.1.17763.316
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.17763.316
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
```

##### STEPS TO REPRODUCE
<!--- Describe exactly how to reproduce the problem, using a minimal test-case -->

1. Create Directory 
    `md C:\Program Files\test`

2. Check Permission.
     Run cmd.
```> icacls "C:\Program Files\test"
C:\Program Files\test NT SERVICE\TrustedInstaller:(I)(F)
                      NT SERVICE\TrustedInstaller:(I)(CI)(IO)(F)
                      NT AUTHORITY\SYSTEM:(I)(F)
                      NT AUTHORITY\SYSTEM:(I)(OI)(CI)(IO)(F)
                      BUILTIN\Administrators:(I)(F)
                      BUILTIN\Administrators:(I)(OI)(CI)(IO)(F)
                      BUILTIN\Users:(I)(RX)
                      BUILTIN\Users:(I)(OI)(CI)(IO)(GR,GE)
                      CREATOR OWNER:(I)(OI)(CI)(IO)(F)
                      APPLICATION PACKAGE AUTHORITY\ALL APPLICATION PACKAGES:(I)(RX)
                      APPLICATION PACKAGE AUTHORITY\ALL APPLICATION PACKAGES:(I)(OI)(CI)(IO)(GR,GE)
                      APPLICATION PACKAGE AUTHORITY\制限されたすべてのアプリケーション パッケージ:(I)(RX)
                      APPLICATION PACKAGE AUTHORITY\制限されたすべてのアプリケーション パッケージ:(I)(OI)(CI)(IO)(GR,GE)
```
3. Execute playbook .

Sample Yaml.
```- hosts: all
  tasks: 
    - name: Add ACL test
      win_acl:
        path: 'c:\Program Files\test'
        user: SOMEDOM\testuser
        rights: 'Modify'
        type: 'allow'
        state: 'present'
        inherit: 'ContainerInherit,ObjectInherit'
        propagation: 'None'

```

##### EXPECTED RESULTS

Permissions changed

##### ACTUAL RESULTS
```ansible-playbook testperm.yml -i hosts/production -l testclient

PLAY [all] ************************************************************************************************************************************************************************************************************************************************************

TASK [Gathering Facts] ************************************************************************************************************************************************************************************************************************************************
ok: [testclient]

TASK [Add ACL test] ***************************************************************************************************************************************************************************************************************************************************
fatal: [testclient]: FAILED! => {"changed": false, "msg": "an error occurred when attempting to present Modify permission(s) on c:\\Program Files\\test for SOMEDOM\\testuser - Exception calling \"Translate\" with \"1\" argument(s): \"Some or all identity references could not be translated.\""}

PLAY RECAP ************************************************************************************************************************************************************************************************************************************************************
testclient                     : ok=1    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0
```

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/modules/windows/win_acl.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/windows/win_acl.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @dagwieers @if-meaton @jborean93 @jhawkesworth @nitzmahone @schwartzmx @trondhindenes
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
https://github.com/ansible/ansible/blob/486370a744f8edad498eea943227365836455bae/lib/ansible/modules/windows/win_acl.ps1#L166
Checks the `USER` part of `DOM\USER` and not the `DOM` so maybe that has to do with it
The presence of 'DOM' does not matter.
It occurs for both users belonging to the local administrator group and users belonging to the domain administrator group.

https://github.com/ansible/ansible/blob/486370a744f8edad498eea943227365836455bae/lib/ansible/modules/windows/win_acl.ps1#L164-L173
I have not confirmed what happens in other languages ​​because I confirmed only in Japanese, but if 'ALL RESTRICTED APPLICATION PACKAGES' is in Japanese (制限されたすべてのアプリケーション パッケージ) this problem is will occur.


If it is in Japanese (including in other languages), $specialIdRefs does not hit $idRefShortValue, so the conversion fails with the following code and ends with an error
https://github.com/ansible/ansible/blob/486370a744f8edad498eea943227365836455bae/lib/ansible/modules/windows/win_acl.ps1#L172

I think that if we change it to check for APPLICATION PACKAGE AUTHORITY before the '\' it might work

@geoside - can you confirm `APPLICATION PACKAGE AUTHORITY` is not being translated?

Commit message: Changelog added

Patch to review:
diff --git a/changelogs/fragments/Make-Win_acl-Ignore-language-diff-in-IDRefs.yml b/changelogs/fragments/Make-Win_acl-Ignore-language-diff-in-IDRefs.yml
new file mode 100644
index 00000000000000..af3cd622f2f064
--- /dev/null
+++ b/changelogs/fragments/Make-Win_acl-Ignore-language-diff-in-IDRefs.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - "win_acl - Change special id ref recognitionto avoid language diff (https://github.com/ansible/ansible/issues/56757)"
diff --git a/lib/ansible/modules/windows/win_acl.ps1 b/lib/ansible/modules/windows/win_acl.ps1
index af55f2ae21f264..4c209c4910da68 100644
--- a/lib/ansible/modules/windows/win_acl.ps1
+++ b/lib/ansible/modules/windows/win_acl.ps1
@@ -161,11 +161,12 @@ Try {
     # Workaround to handle special use case 'APPLICATION PACKAGE AUTHORITY\ALL APPLICATION PACKAGES' and
     # 'APPLICATION PACKAGE AUTHORITY\ALL RESTRICTED APPLICATION PACKAGES'- can't translate fully qualified name (win32 API bug/oddity)
     # 'ALL APPLICATION PACKAGES' exists only on Win2k12 and Win2k16 and 'ALL RESTRICTED APPLICATION PACKAGES' exists only in Win2k16
-    $specialIdRefs = "ALL APPLICATION PACKAGES","ALL RESTRICTED APPLICATION PACKAGES"
+    $specialIdRefPrefixes = "APPLICATION PACKAGE AUTHORITY"
     ForEach($rule in $objACL.Access){
+        $idRefShortPrefix = ($rule.IdentityReference.Value).split('\')[0]
         $idRefShortValue = ($rule.IdentityReference.Value).split('\')[-1]
 
-        if ( $idRefShortValue -in $specialIdRefs ) {
+        if ( $idRefShortPrefix -in $specialIdRefPrefixes ) {
             $ruleIdentity = (New-Object Security.Principal.NTAccount $idRefShortValue).Translate([Security.Principal.SecurityIdentifier])
         }
         else {


Code review comments:
diff_hunk: @@ -161,11 +161,12 @@ Try {
     # Workaround to handle special use case 'APPLICATION PACKAGE AUTHORITY\ALL APPLICATION PACKAGES' and
     # 'APPLICATION PACKAGE AUTHORITY\ALL RESTRICTED APPLICATION PACKAGES'- can't translate fully qualified name (win32 API bug/oddity)
     # 'ALL APPLICATION PACKAGES' exists only on Win2k12 and Win2k16 and 'ALL RESTRICTED APPLICATION PACKAGES' exists only in Win2k16
-    $specialIdRefs = "ALL APPLICATION PACKAGES","ALL RESTRICTED APPLICATION PACKAGES"
+    $specialIdRefPrefixes = "APPLICATION PACKAGE AUTHORITY"
     ForEach($rule in $objACL.Access){
+        $idRefShortPrefix = ($rule.IdentityReference.Value).split('\')[0]
         $idRefShortValue = ($rule.IdentityReference.Value).split('\')[-1]
 
-        if ( $idRefShortValue -in $specialIdRefs ) {
+        if ( $idRefShortPrefix -in $specialIdRefPrefixes ) {
path: lib/ansible/modules/windows/win_acl.ps1
text: @user1:
This is probably a really tiny nitpick, but would 
`if ( $idRefShortPrefix -eq $specialIdRefPrefixes ) { `
be clearer here, since $specialIdRefPrefixes is just a string not an Array?
I think -in and -eq behave the same in this case but I think -eq would make it clearer what the intention is.

@author:
I prefer changing `specialIdRefPrefixes` to be an array or future additions