[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>

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)

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

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']

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'