From 8e972143092fd331a7e22247059af64a2edafeb8 Mon Sep 17 00:00:00 2001
From: Nicolas Payart <npayart@gmail.com>
Date: Sat, 20 Jun 2020 10:45:41 +0200
Subject: [PATCH 1/4] Add option update_password (always, on_create) to
 tower_user module

---
 awx_collection/plugins/modules/tower_user.py | 11 ++++++++++-
 awx_collection/test/awx/test_user.py         | 13 +++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/awx_collection/plugins/modules/tower_user.py b/awx_collection/plugins/modules/tower_user.py
index 4b648356e4..68b2f65a48 100644
--- a/awx_collection/plugins/modules/tower_user.py
+++ b/awx_collection/plugins/modules/tower_user.py
@@ -55,6 +55,13 @@ options:
       description:
         - Write-only field used to change the password.
       type: str
+    update_password:
+      description:
+        - C(always) will update passwords if they differ.
+        - C(on_create) will only set the password for newly created users.
+      type: str
+      choices: [ always, on_create ]
+      default: always
     state:
       description:
         - Desired state of the resource.
@@ -115,6 +122,7 @@ def main():
         is_superuser=dict(type='bool', default=False, aliases=['superuser']),
         is_system_auditor=dict(type='bool', default=False, aliases=['auditor']),
         password=dict(no_log=True),
+        update_password=dict(choices=['always', 'on_create'], default='always', no_log=False),
         state=dict(choices=['present', 'absent'], default='present'),
     )
 
@@ -129,6 +137,7 @@ def main():
     is_superuser = module.params.get('is_superuser')
     is_system_auditor = module.params.get('is_system_auditor')
     password = module.params.get('password')
+    update_password = module.params.get('update_password')
     state = module.params.get('state')
 
     # Attempt to look up the related items the user specified (these will fail the module if not found)
@@ -154,7 +163,7 @@ def main():
         new_fields['is_superuser'] = is_superuser
     if is_system_auditor:
         new_fields['is_system_auditor'] = is_system_auditor
-    if password:
+    if password and (not existing_item or update_password == 'always'):
         new_fields['password'] = password
 
     # If the state was present and we can let the module build or update the existing item, this will return on its own
diff --git a/awx_collection/test/awx/test_user.py b/awx_collection/test/awx/test_user.py
index db705bd5be..5babf6efc0 100644
--- a/awx_collection/test/awx/test_user.py
+++ b/awx_collection/test/awx/test_user.py
@@ -44,3 +44,16 @@ def test_password_no_op_warning(run_module, admin_user, mock_auth_stuff, silence
     silence_warning.assert_called_once_with(
         "The field password of user {0} has encrypted data and "
         "may inaccurately report task is changed.".format(result['id']))
+
+
+@pytest.mark.django_db
+def test_update_password_on_create(run_module, admin_user, mock_auth_stuff):
+    for i in range(2):
+        result = run_module('tower_user', dict(
+            username='Bob',
+            password='pass4word',
+            update_password='on_create'
+        ), admin_user)
+        assert not result.get('failed', False), result.get('msg', result)
+
+    assert not result.get('changed')

From 362d6a3204e9a696db544b14d6b62eb13f2e4437 Mon Sep 17 00:00:00 2001
From: Alan Rominger <arominge@redhat.com>
Date: Fri, 4 Sep 2020 11:48:56 -0400
Subject: [PATCH 2/4] Add new option update_secrets to allow lazy or strict
 updating

---
 .../plugins/module_utils/tower_api.py         | 35 ++++++++++++++++---
 .../plugins/module_utils/tower_module.py      |  1 -
 .../plugins/modules/tower_credential.py       |  7 ++++
 awx_collection/plugins/modules/tower_user.py  | 16 ++++-----
 awx_collection/test/awx/test_completeness.py  |  2 +-
 awx_collection/test/awx/test_credential.py    | 12 +++++--
 awx_collection/test/awx/test_user.py          |  2 +-
 7 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/awx_collection/plugins/module_utils/tower_api.py b/awx_collection/plugins/module_utils/tower_api.py
index 24a14e5713..b23596d8fe 100644
--- a/awx_collection/plugins/module_utils/tower_api.py
+++ b/awx_collection/plugins/module_utils/tower_api.py
@@ -28,6 +28,7 @@ class TowerAPIModule(TowerModule):
         'workflow_job_template_nodes': 'identifier',
         'instances': 'hostname'
     }
+    ENCRYPTED_STRING = "$encrypted$"
 
     def __init__(self, argument_spec, direct_params=None, error_callback=None, warn_callback=None, **kwargs):
         kwargs['supports_check_mode'] = True
@@ -36,6 +37,11 @@ class TowerAPIModule(TowerModule):
                                              error_callback=error_callback, warn_callback=warn_callback, **kwargs)
         self.session = Request(cookies=CookieJar(), validate_certs=self.verify_ssl)
 
+        if 'update_secrets' in self.params:
+            self.update_secrets = self.params.pop('update_secrets')
+        else:
+            self.update_secrets = True
+
     @staticmethod
     def param_to_endpoint(name):
         exceptions = {
@@ -478,6 +484,25 @@ class TowerAPIModule(TowerModule):
             return True
         return False
 
+    @staticmethod
+    def fields_could_be_same(old_field, new_field):
+        """Treating $encrypted$ as a wild card,
+        return False if the two values are KNOWN to be different
+        return True if the two values are the same, or could potentially be the same,
+        depending on the unknown $encrypted$ value or sub-values
+        """
+        if isinstance(old_field, dict) and isinstance(new_field, dict):
+            if set(old_field.keys()) != set(new_field.keys()):
+                return False
+            for key in new_field.keys():
+                if not TowerAPIModule.fields_could_be_same(old_field[key], new_field[key]):
+                    return False
+            return True  # all sub-fields are either equal or could be equal
+        else:
+            if old_field == TowerAPIModule.ENCRYPTED_STRING:
+                return True
+            return bool(new_field == old_field)
+
     def objects_could_be_different(self, old, new, field_set=None, warning=False):
         if field_set is None:
             field_set = set(fd for fd in new.keys() if fd not in ('modified', 'related', 'summary_fields'))
@@ -485,11 +510,13 @@ class TowerAPIModule(TowerModule):
             new_field = new.get(field, None)
             old_field = old.get(field, None)
             if old_field != new_field:
-                return True  # Something doesn't match
+                if self.update_secrets or (not self.fields_could_be_same(old_field, new_field)):
+                    return True  # Something doesn't match, or something might not match
             elif self.has_encrypted_values(new_field) or field not in new:
-                # case of 'field not in new' - user password write-only field that API will not display
-                self._encrypted_changed_warning(field, old, warning=warning)
-                return True
+                if self.update_secrets or (not self.fields_could_be_same(old_field, new_field)):
+                    # case of 'field not in new' - user password write-only field that API will not display
+                    self._encrypted_changed_warning(field, old, warning=warning)
+                    return True
         return False
 
     def update_if_needed(self, existing_item, new_item, on_update=None, associations=None):
diff --git a/awx_collection/plugins/module_utils/tower_module.py b/awx_collection/plugins/module_utils/tower_module.py
index 215e2db3c8..470ea80de3 100644
--- a/awx_collection/plugins/module_utils/tower_module.py
+++ b/awx_collection/plugins/module_utils/tower_module.py
@@ -52,7 +52,6 @@ class TowerModule(AnsibleModule):
     oauth_token_id = None
     authenticated = False
     config_name = 'tower_cli.cfg'
-    ENCRYPTED_STRING = "$encrypted$"
     version_checked = False
     error_callback = None
     warn_callback = None
diff --git a/awx_collection/plugins/modules/tower_credential.py b/awx_collection/plugins/modules/tower_credential.py
index f244ae812e..c8b3f89afa 100644
--- a/awx_collection/plugins/modules/tower_credential.py
+++ b/awx_collection/plugins/modules/tower_credential.py
@@ -52,6 +52,12 @@ options:
           Refer to the Ansible Tower documentation for example syntax.
         - Any fields in this dict will take prescedence over any fields mentioned below (i.e. host, username, etc)
       type: dict
+    update_secrets:
+      description:
+        - C(true) will always update encrypted values.
+        - C(false) will only updated encrypted values if a change is absolutely known to be needed.
+      type: bool
+      default: true
     user:
       description:
         - User that should own this credential.
@@ -308,6 +314,7 @@ def main():
         organization=dict(),
         credential_type=dict(),
         inputs=dict(type='dict', no_log=True),
+        update_secrets=dict(type='bool', default=True, no_log=False),
         user=dict(),
         team=dict(),
         # These are for backwards compatability
diff --git a/awx_collection/plugins/modules/tower_user.py b/awx_collection/plugins/modules/tower_user.py
index 68b2f65a48..8250a8176a 100644
--- a/awx_collection/plugins/modules/tower_user.py
+++ b/awx_collection/plugins/modules/tower_user.py
@@ -55,13 +55,12 @@ options:
       description:
         - Write-only field used to change the password.
       type: str
-    update_password:
+    update_secrets:
       description:
-        - C(always) will update passwords if they differ.
-        - C(on_create) will only set the password for newly created users.
-      type: str
-      choices: [ always, on_create ]
-      default: always
+        - C(true) will always password if user specifies password and API gives $encrypted$ for password.
+        - C(false) will only set the password if other values change too.
+      type: bool
+      default: true
     state:
       description:
         - Desired state of the resource.
@@ -122,7 +121,7 @@ def main():
         is_superuser=dict(type='bool', default=False, aliases=['superuser']),
         is_system_auditor=dict(type='bool', default=False, aliases=['auditor']),
         password=dict(no_log=True),
-        update_password=dict(choices=['always', 'on_create'], default='always', no_log=False),
+        update_secrets=dict(type='bool', default=True, no_log=False),
         state=dict(choices=['present', 'absent'], default='present'),
     )
 
@@ -137,7 +136,6 @@ def main():
     is_superuser = module.params.get('is_superuser')
     is_system_auditor = module.params.get('is_system_auditor')
     password = module.params.get('password')
-    update_password = module.params.get('update_password')
     state = module.params.get('state')
 
     # Attempt to look up the related items the user specified (these will fail the module if not found)
@@ -163,7 +161,7 @@ def main():
         new_fields['is_superuser'] = is_superuser
     if is_system_auditor:
         new_fields['is_system_auditor'] = is_system_auditor
-    if password and (not existing_item or update_password == 'always'):
+    if password:
         new_fields['password'] = password
 
     # If the state was present and we can let the module build or update the existing item, this will return on its own
diff --git a/awx_collection/test/awx/test_completeness.py b/awx_collection/test/awx/test_completeness.py
index ab8c1547f8..5dc232e2ed 100644
--- a/awx_collection/test/awx/test_completeness.py
+++ b/awx_collection/test/awx/test_completeness.py
@@ -30,7 +30,7 @@ no_endpoint_for_module = [
 
 # Global module parameters we can ignore
 ignore_parameters = [
-    'state', 'new_name',
+    'state', 'new_name', 'update_secrets'
 ]
 
 # Some modules take additional parameters that do not appear in the API
diff --git a/awx_collection/test/awx/test_credential.py b/awx_collection/test/awx/test_credential.py
index 713006a386..7c0070d9e0 100644
--- a/awx_collection/test/awx/test_credential.py
+++ b/awx_collection/test/awx/test_credential.py
@@ -154,7 +154,8 @@ def test_make_use_of_custom_credential_type(run_module, organization, admin_user
 
 
 @pytest.mark.django_db
-def test_secret_field_write_twice(run_module, organization, admin_user, cred_type):
+@pytest.mark.parametrize('update_secrets', [True, False])
+def test_secret_field_write_twice(run_module, organization, admin_user, cred_type, update_secrets):
     val1 = '7rEZK38DJl58A7RxA6EC7lLvUHbBQ1'
     result = run_module('tower_credential', dict(
         name='Galaxy Token for Steve',
@@ -172,9 +173,14 @@ def test_secret_field_write_twice(run_module, organization, admin_user, cred_typ
         name='Galaxy Token for Steve',
         organization=organization.name,
         credential_type=cred_type.name,
-        inputs={'token': val2}
+        inputs={'token': val2},
+        update_secrets=update_secrets
     ), admin_user)
     assert not result.get('failed', False), result.get('msg', result)
 
     Credential.objects.get(id=result['id']).inputs['token'] == val2
-    assert result.get('changed'), result
+    print(result)
+    if update_secrets:
+        assert result.get('changed'), result
+    else:
+        assert result.get('changed') is False, result
diff --git a/awx_collection/test/awx/test_user.py b/awx_collection/test/awx/test_user.py
index 5babf6efc0..6c49f04121 100644
--- a/awx_collection/test/awx/test_user.py
+++ b/awx_collection/test/awx/test_user.py
@@ -52,7 +52,7 @@ def test_update_password_on_create(run_module, admin_user, mock_auth_stuff):
         result = run_module('tower_user', dict(
             username='Bob',
             password='pass4word',
-            update_password='on_create'
+            update_secrets=False
         ), admin_user)
         assert not result.get('failed', False), result.get('msg', result)
 

From 2f1a9a28eafacc5aaaeb0a8816b8b1772e23dd04 Mon Sep 17 00:00:00 2001
From: Alan Rominger <arominge@redhat.com>
Date: Fri, 4 Sep 2020 12:22:03 -0400
Subject: [PATCH 3/4] streamline credential test

---
 awx_collection/test/awx/test_credential.py | 32 ++++++++--------------
 1 file changed, 12 insertions(+), 20 deletions(-)

diff --git a/awx_collection/test/awx/test_credential.py b/awx_collection/test/awx/test_credential.py
index 7c0070d9e0..0ab7017158 100644
--- a/awx_collection/test/awx/test_credential.py
+++ b/awx_collection/test/awx/test_credential.py
@@ -157,30 +157,22 @@ def test_make_use_of_custom_credential_type(run_module, organization, admin_user
 @pytest.mark.parametrize('update_secrets', [True, False])
 def test_secret_field_write_twice(run_module, organization, admin_user, cred_type, update_secrets):
     val1 = '7rEZK38DJl58A7RxA6EC7lLvUHbBQ1'
-    result = run_module('tower_credential', dict(
-        name='Galaxy Token for Steve',
-        organization=organization.name,
-        credential_type=cred_type.name,
-        inputs={'token': val1}
-    ), admin_user)
-    assert not result.get('failed', False), result.get('msg', result)
-
-    Credential.objects.get(id=result['id']).inputs['token'] == val1
-
     val2 = '7rEZ238DJl5837rxA6xxxlLvUHbBQ1'
+    for val in (val1, val2):
+        result = run_module('tower_credential', dict(
+            name='Galaxy Token for Steve',
+            organization=organization.name,
+            credential_type=cred_type.name,
+            inputs={'token': val},
+            update_secrets=update_secrets
+        ), admin_user)
+        assert not result.get('failed', False), result.get('msg', result)
 
-    result = run_module('tower_credential', dict(
-        name='Galaxy Token for Steve',
-        organization=organization.name,
-        credential_type=cred_type.name,
-        inputs={'token': val2},
-        update_secrets=update_secrets
-    ), admin_user)
-    assert not result.get('failed', False), result.get('msg', result)
+        if update_secrets:
+            assert Credential.objects.get(id=result['id']).get_input('token') == val
 
-    Credential.objects.get(id=result['id']).inputs['token'] == val2
-    print(result)
     if update_secrets:
         assert result.get('changed'), result
     else:
         assert result.get('changed') is False, result
+        assert Credential.objects.get(id=result['id']).get_input('token') == val1

From 3aec1a115d1d9bee853ad5ef8af42f6c09b0b7d8 Mon Sep 17 00:00:00 2001
From: Alan Rominger <arominge@redhat.com>
Date: Tue, 8 Sep 2020 12:52:50 -0400
Subject: [PATCH 4/4] fix wording

---
 awx_collection/plugins/module_utils/tower_api.py | 1 -
 awx_collection/plugins/modules/tower_user.py     | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/awx_collection/plugins/module_utils/tower_api.py b/awx_collection/plugins/module_utils/tower_api.py
index b23596d8fe..d41c32b772 100644
--- a/awx_collection/plugins/module_utils/tower_api.py
+++ b/awx_collection/plugins/module_utils/tower_api.py
@@ -22,7 +22,6 @@ class TowerAPIModule(TowerModule):
         'tower': 'Red Hat Ansible Tower',
     }
     session = None
-    cookie_jar = CookieJar()
     IDENTITY_FIELDS = {
         'users': 'username',
         'workflow_job_template_nodes': 'identifier',
diff --git a/awx_collection/plugins/modules/tower_user.py b/awx_collection/plugins/modules/tower_user.py
index 8250a8176a..8f32c42666 100644
--- a/awx_collection/plugins/modules/tower_user.py
+++ b/awx_collection/plugins/modules/tower_user.py
@@ -57,7 +57,7 @@ options:
       type: str
     update_secrets:
       description:
-        - C(true) will always password if user specifies password and API gives $encrypted$ for password.
+        - C(true) will always change password if user specifies password, even if API gives $encrypted$ for password.
         - C(false) will only set the password if other values change too.
       type: bool
       default: true