From 264bc930efc764b9e232230f7435d5e65c81e411 Mon Sep 17 00:00:00 2001 From: Christopher Gadd Date: Fri, 9 Nov 2018 15:29:38 +1300 Subject: [PATCH] [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 (cherry picked from commit f75a84e382) Co-authored-by: Christopher Gadd --- .../user-password_lock-change-fix.yaml | 2 + lib/ansible/modules/system/user.py | 23 ++-- test/integration/targets/user/tasks/main.yml | 108 ++++++++++++++++++ test/integration/targets/user/vars/main.yml | 4 + 4 files changed, 128 insertions(+), 9 deletions(-) create mode 100644 changelogs/fragments/user-password_lock-change-fix.yaml 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 00000000000..0d2f8622bb5 --- /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 3c100729a70..39773537ae1 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 212c0d69d51..c9c4fb0dd65 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 f5d3b3f332b..603fcc59a0d 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'