From dc32842573e647ab0df07b0e13eb2069d0a77b7e Mon Sep 17 00:00:00 2001
From: jamessemai <james@semai.com>
Date: Fri, 13 Jul 2018 06:08:14 +0200
Subject: [PATCH] win_security_policy: Allow setting a value to empty (#42051)

* win_security_policy: allow removing values (resolves #40869)

* Removing warning

* Adding test for remove policy setting

* Fixing string comparison

* Make idempotent

* Adding idempotency and diff test

* added changelog fragment
---
 .../win_security_policy-empty-value.yaml      |  2 +
 .../modules/windows/win_security_policy.ps1   |  4 ++
 .../win_security_policy/tasks/tests.yml       | 53 +++++++++++++++++++
 3 files changed, 59 insertions(+)
 create mode 100644 changelogs/fragments/win_security_policy-empty-value.yaml

diff --git a/changelogs/fragments/win_security_policy-empty-value.yaml b/changelogs/fragments/win_security_policy-empty-value.yaml
new file mode 100644
index 0000000000..eaef3a38ca
--- /dev/null
+++ b/changelogs/fragments/win_security_policy-empty-value.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+- win_security_policy - allows an empty string to reset a policy value https://github.com/ansible/ansible/issues/40869
diff --git a/lib/ansible/modules/windows/win_security_policy.ps1 b/lib/ansible/modules/windows/win_security_policy.ps1
index 2381d0e4b9..44d89ca0ce 100644
--- a/lib/ansible/modules/windows/win_security_policy.ps1
+++ b/lib/ansible/modules/windows/win_security_policy.ps1
@@ -169,6 +169,8 @@ if ($secedit_ini.$section.ContainsKey($key)) {
         $secedit_ini.$section.$key = $value
         $will_change = $true
     }
+} elseif ([string]$value -eq "") {
+      # Value is requested to be removed, and has already been removed, do nothing
 } else {
     if ($diff_mode) {
         $result.diff.prepared = @"
@@ -194,6 +196,8 @@ if ($will_change -eq $true) {
             if ($new_value -cne $value) {
                 Fail-Json $result "Failed to change the value for key '$key' in section '$section', the value is still $new_value"
             }
+        } elseif ([string]$value -eq "") {
+            # Value was empty, so OK if no longer in the result
         } else {
             Fail-Json $result "The key '$key' in section '$section' is not a valid key, cannot set this value"
         }
diff --git a/test/integration/targets/win_security_policy/tasks/tests.yml b/test/integration/targets/win_security_policy/tasks/tests.yml
index 1afa534269..724b6010a3 100644
--- a/test/integration/targets/win_security_policy/tasks/tests.yml
+++ b/test/integration/targets/win_security_policy/tasks/tests.yml
@@ -131,3 +131,56 @@
     that:
     - change_existing_string_again is not changed
     - change_existing_string_again.value == "New Guest"
+
+- name: add policy setting
+  win_security_policy:
+    section: Privilege Rights
+    # following key is empty by default
+    key: SeCreateTokenPrivilege
+    # add Guests
+    value: '*S-1-5-32-546'
+    
+- name: get actual policy setting
+  test_win_security_policy:
+    section: Privilege Rights
+    key: SeCreateTokenPrivilege
+  register: add_policy_setting_actual
+  
+- name: assert add policy setting
+  assert:
+    that:
+    - add_policy_setting_actual.value == '*S-1-5-32-546'
+  
+- name: remove policy setting
+  win_security_policy:
+    section: Privilege Rights
+    key: SeCreateTokenPrivilege
+    value: ''
+  diff: yes
+  register: remove_policy_setting
+
+- name: get actual policy setting
+  test_win_security_policy:
+    section: Privilege Rights
+    key: SeCreateTokenPrivilege
+  register: remove_policy_setting_actual
+
+- name: assert remove policy setting
+  assert:
+    that:
+    - remove_policy_setting is changed
+    - remove_policy_setting.diff.prepared == "[Privilege Rights]\n-SeCreateTokenPrivilege = *S-1-5-32-546\n+SeCreateTokenPrivilege = "
+    - remove_policy_setting_actual.value is none
+    
+- name: remove policy setting again
+  win_security_policy:
+    section: Privilege Rights
+    key: SeCreateTokenPrivilege
+    value: ''
+  register: remove_policy_setting_again
+
+- name: assert remove policy setting again
+  assert:
+    that:
+    - remove_policy_setting_again is not changed
+    - remove_policy_setting_again.value == ''