[stable-2.7] make password locking in user module idempotent (#43671)

* Simplify logic and add FreeBSD & NetBSD

* Remove incorrect flag for lock and unlock on FreeBSD

* Add tests and changelog

Co-authored-by: Chris Gadd <gaddman@email.com>
(cherry picked from commit f75a84e382)

Co-authored-by: Christopher Gadd <gaddman@email.com>

4 files changed, 128 insertions, 9 deletions

diff --git a/changelogs/fragments/user-password_lock-change-fix.yaml b/changelogs/fragments/user-password_lock-change-fix.yaml
new file mode 100644
index 0000000000..0d2f8622bb
--- /dev/null
+++ b/changelogs/fragments/user-password_lock-change-fix.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+  - user - do not report changes every time when setting password_lock (https://github.com/ansible/ansible/issues/43670)
diff --git a/lib/ansible/modules/system/user.py b/lib/ansible/modules/system/user.py
index 3c100729a7..39773537ae 100644
--- a/lib/ansible/modules/system/user.py
+++ b/lib/ansible/modules/system/user.py
@@ -194,7 +194,7 @@ options:
             - Lock the password (usermod -L, pw lock, usermod -C).
               BUT implementation differs on different platforms, this option does not always mean the user cannot login via other methods.
               This option does not disable the user, only lock the password. Do not change the password in the same task.
-              Currently supported on Linux, FreeBSD, DragonFlyBSD, NetBSD.
+              Currently supported on Linux, FreeBSD, DragonFlyBSD, NetBSD, OpenBSD.
         type: bool
         version_added: "2.6"
     local:
@@ -718,9 +718,11 @@ class User(object):
                     cmd.append('-e')
                     cmd.append(time.strftime(self.DATE_FORMAT, self.expires))
 
-        if self.password_lock:
+        # Lock if no password or unlocked, unlock only if locked
+        if self.password_lock and not info[1].startswith('!'):
             cmd.append('-L')
-        elif self.password_lock is not None:
+        elif self.password_lock is False and info[1].startswith('!'):
+            # usermod will refuse to unlock a user with no password, module shows 'changed' regardless
             cmd.append('-U')
 
         if self.update_password == 'always' and self.password is not None and info[1] != self.password:
@@ -1214,22 +1216,20 @@ class FreeBsdUser(User):
             return self.execute_command(cmd)
 
         # we have to lock/unlock the password in a distinct command
-        if self.password_lock:
+        if self.password_lock and not info[1].startswith('*LOCKED*'):
             cmd = [
                 self.module.get_bin_path('pw', True),
                 'lock',
-                '-n',
                 self.name
             ]
             if self.uid is not None and info[2] != int(self.uid):
                 cmd.append('-u')
                 cmd.append(self.uid)
             return self.execute_command(cmd)
-        elif self.password_lock is not None:
+        elif self.password_lock is False and info[1].startswith('*LOCKED*'):
             cmd = [
                 self.module.get_bin_path('pw', True),
                 'unlock',
-                '-n',
                 self.name
             ]
             if self.uid is not None and info[2] != int(self.uid):
@@ -1402,6 +1402,11 @@ class OpenBSDUser(User):
                 cmd.append('-L')
                 cmd.append(self.login_class)
 
+        if self.password_lock and not info[1].startswith('*'):
+            cmd.append('-Z')
+        elif self.password_lock is False and info[1].startswith('*'):
+            cmd.append('-U')
+
         if self.update_password == 'always' and self.password is not None \
                 and self.password != '*' and info[1] != self.password:
             cmd.append('-p')
@@ -1562,9 +1567,9 @@ class NetBSDUser(User):
             cmd.append('-p')
             cmd.append(self.password)
 
-        if self.password_lock:
+        if self.password_lock and not info[1].startswith('*LOCKED*'):
             cmd.append('-C yes')
-        elif self.password_lock is not None:
+        elif self.password_lock is False and info[1].startswith('*LOCKED*'):
             cmd.append('-C no')
 
         # skip if no changes to be made
diff --git a/test/integration/targets/user/tasks/main.yml b/test/integration/targets/user/tasks/main.yml
index 212c0d69d5..c9c4fb0dd6 100644
--- a/test/integration/targets/user/tasks/main.yml
+++ b/test/integration/targets/user/tasks/main.yml
@@ -628,3 +628,111 @@
   file:
     path: "{{ output_dir }}/test_id_rsa"
     state: absent
+  when: ansible_os_family == 'FreeBSD'
+
+
+## password lock
+- block:
+    - name: Set password for ansibulluser
+      user:
+        name: ansibulluser
+        password: "$6$rounds=656000$TT4O7jz2M57npccl$33LF6FcUMSW11qrESXL1HX0BS.bsiT6aenFLLiVpsQh6hDtI9pJh5iY7x8J7ePkN4fP8hmElidHXaeD51pbGS."
+
+    - name: Lock account
+      user:
+        name: ansibulluser
+        password_lock: yes
+      register: password_lock_1
+
+    - name: Lock account again
+      user:
+        name: ansibulluser
+        password_lock: yes
+      register: password_lock_2
+
+    - name: Unlock account
+      user:
+        name: ansibulluser
+        password_lock: no
+      register: password_lock_3
+
+    - name: Unlock account again
+      user:
+        name: ansibulluser
+        password_lock: no
+      register: password_lock_4
+
+    - name: Ensure task reported changes appropriately
+      assert:
+        msg: The password_lock tasks did not make changes appropriately
+        that:
+          - password_lock_1 is changed
+          - password_lock_2 is not changed
+          - password_lock_3 is changed
+          - password_lock_4 is not changed
+
+    - name: Lock account
+      user:
+        name: ansibulluser
+        password_lock: yes
+
+    - name: Verify account lock for BSD
+      block:
+        - name: BSD | Get account status
+          shell: "{{ status_command[ansible_facts['system']] }}"
+          register: account_status_locked
+
+        - name: Unlock account
+          user:
+            name: ansibulluser
+            password_lock: no
+
+        - name: BSD | Get account status
+          shell: "{{ status_command[ansible_facts['system']] }}"
+          register: account_status_unlocked
+
+        - name: FreeBSD | Ensure account is locked
+          assert:
+            that:
+              - "'LOCKED' in account_status_locked.stdout"
+              - "'LOCKED' not in account_status_unlocked.stdout"
+          when: ansible_facts['system'] == 'FreeBSD'
+
+      when: ansible_facts['system'] in ['FreeBSD', 'OpenBSD']
+
+    - name: Verify account lock for Linux
+      block:
+        - name: LINUX | Get account status
+          getent:
+            database: shadow
+            key: ansibulluser
+
+        - name: LINUX | Ensure account is locked
+          assert:
+            that:
+              - getent_shadow['ansibulluser'][0].startswith('!')
+
+        - name: Unlock account
+          user:
+            name: ansibulluser
+            password_lock: no
+
+        - name: LINUX | Get account status
+          getent:
+            database: shadow
+            key: ansibulluser
+
+        - name: LINUX | Ensure account is unlocked
+          assert:
+            that:
+              - not getent_shadow['ansibulluser'][0].startswith('!')
+
+      when: ansible_facts['system'] == 'Linux'
+
+  always:
+    - name: Unlock account
+      user:
+        name: ansibulluser
+        password_lock: no
+
+  when: ansible_facts['system'] in ['FreeBSD', 'OpenBSD', 'Linux']
diff --git a/test/integration/targets/user/vars/main.yml b/test/integration/targets/user/vars/main.yml
index f5d3b3f332..603fcc59a0 100644
--- a/test/integration/targets/user/vars/main.yml
+++ b/test/integration/targets/user/vars/main.yml
@@ -3,3 +3,7 @@ user_home_prefix:
   FreeBSD: '/home'
   SunOS: '/home'
   Darwin: '/Users'
+
+status_command:
+  OpenBSD: "grep ansibulluser /etc/master.passwd | cut -d ':' -f 2"
+  FreeBSD: 'pw user show ansibulluser'