From a7170da851e2a3951311a3c78bcac5346b5edacd Mon Sep 17 00:00:00 2001 From: Ruediger Pluem <53253255+rpluem-vf@users.noreply.github.com> Date: Fri, 2 Oct 2020 21:38:27 +0200 Subject: [PATCH] user - allow local users with an expiry date to be created (#72022) The luseradd / lusermod commands do not support the -e option. Set the expiry time in this case via lchage after the user was created / modified. Fixes: #71942 In Python3 math.floor returns an integer whereas Python2 returns a float. Hence always convert the result of math.floor to an int to ensure that lexpires is an integer. Move local expires tests in a separate file and import the tasks to the main.yml to keep main.yml smaller. --- .../fragments/fix_ansible_issue_71942.yaml | 5 + lib/ansible/modules/user.py | 50 ++- .../targets/user/tasks/expires_local.yml | 333 ++++++++++++++++++ test/integration/targets/user/tasks/main.yml | 3 + 4 files changed, 384 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/fix_ansible_issue_71942.yaml create mode 100644 test/integration/targets/user/tasks/expires_local.yml diff --git a/changelogs/fragments/fix_ansible_issue_71942.yaml b/changelogs/fragments/fix_ansible_issue_71942.yaml new file mode 100644 index 00000000000..0f14d9b8525 --- /dev/null +++ b/changelogs/fragments/fix_ansible_issue_71942.yaml @@ -0,0 +1,5 @@ +bugfixes: + - > + user - Local users with an expiry date cannot be created as the ``luseradd`` / + ``lusermod`` commands do not support the ``-e`` option. Set the expiry time in + this case via ``lchage`` after the user was created / modified. (https://github.com/ansible/ansible/issues/71942) diff --git a/lib/ansible/modules/user.py b/lib/ansible/modules/user.py index 9a53cd6aeaa..16db33752d8 100644 --- a/lib/ansible/modules/user.py +++ b/lib/ansible/modules/user.py @@ -413,6 +413,7 @@ import shutil import socket import subprocess import time +import math from ansible.module_utils import distro from ansible.module_utils._text import to_bytes, to_native, to_text @@ -581,6 +582,7 @@ class User(object): if self.local: command_name = 'luseradd' lgroupmod_cmd = self.module.get_bin_path('lgroupmod', True) + lchage_cmd = self.module.get_bin_path('lchage', True) else: command_name = 'useradd' @@ -648,7 +650,7 @@ class User(object): cmd.append('-s') cmd.append(self.shell) - if self.expires is not None: + if self.expires is not None and not self.local: cmd.append('-e') if self.expires < time.gmtime(0): cmd.append('') @@ -674,7 +676,22 @@ class User(object): cmd.append(self.name) (rc, err, out) = self.execute_command(cmd) - if not self.local or rc != 0 or self.groups is None or len(self.groups) == 0: + if not self.local or rc != 0: + return (rc, err, out) + + if self.expires is not None: + if self.expires < time.gmtime(0): + lexpires = -1 + else: + # Convert seconds since Epoch to days since Epoch + lexpires = int(math.floor(self.module.params['expires'])) // 86400 + (rc, _err, _out) = self.execute_command([lchage_cmd, '-E', to_native(lexpires), self.name]) + out += _out + err += _err + if rc != 0: + return (rc, out, err) + + if self.groups is None or len(self.groups) == 0: return (rc, err, out) for add_group in groups: @@ -719,6 +736,8 @@ class User(object): lgroupmod_cmd = self.module.get_bin_path('lgroupmod', True) lgroupmod_add = set() lgroupmod_del = set() + lchage_cmd = self.module.get_bin_path('lchage', True) + lexpires = None else: command_name = 'usermod' @@ -801,16 +820,23 @@ class User(object): if self.expires < time.gmtime(0): if current_expires >= 0: - cmd.append('-e') - cmd.append('') + if self.local: + lexpires = -1 + else: + cmd.append('-e') + cmd.append('') else: # Convert days since Epoch to seconds since Epoch as struct_time current_expire_date = time.gmtime(current_expires * 86400) # Current expires is negative or we compare year, month, and day only if current_expires < 0 or current_expire_date[:3] != self.expires[:3]: - cmd.append('-e') - cmd.append(time.strftime(self.DATE_FORMAT, self.expires)) + if self.local: + # Convert seconds since Epoch to days since Epoch + lexpires = int(math.floor(self.module.params['expires'])) // 86400 + else: + cmd.append('-e') + cmd.append(time.strftime(self.DATE_FORMAT, self.expires)) # Lock if no password or unlocked, unlock only if locked if self.password_lock and not info[1].startswith('!'): @@ -830,7 +856,17 @@ class User(object): cmd.append(self.name) (rc, err, out) = self.execute_command(cmd) - if not self.local or not (rc is None or rc == 0) or (len(lgroupmod_add) == 0 and len(lgroupmod_del) == 0): + if not self.local or not (rc is None or rc == 0): + return (rc, err, out) + + if lexpires is not None: + (rc, _err, _out) = self.execute_command([lchage_cmd, '-E', to_native(lexpires), self.name]) + out += _out + err += _err + if rc != 0: + return (rc, out, err) + + if len(lgroupmod_add) == 0 and len(lgroupmod_del) == 0: return (rc, err, out) for add_group in lgroupmod_add: diff --git a/test/integration/targets/user/tasks/expires_local.yml b/test/integration/targets/user/tasks/expires_local.yml new file mode 100644 index 00000000000..e66203530c5 --- /dev/null +++ b/test/integration/targets/user/tasks/expires_local.yml @@ -0,0 +1,333 @@ +--- +## local user expires +# Date is March 3, 2050 + +- name: Remove local_ansibulluser + user: + name: local_ansibulluser + state: absent + remove: yes + local: yes + tags: + - user_test_local_mode + +- name: Set user expiration + user: + name: local_ansibulluser + state: present + local: yes + expires: 2529881062 + register: user_test_local_expires1 + tags: + - timezone + - user_test_local_mode + +- name: Set user expiration again to ensure no change is made + user: + name: local_ansibulluser + state: present + local: yes + expires: 2529881062 + register: user_test_local_expires2 + tags: + - timezone + - user_test_local_mode + +- name: Ensure that account with expiration was created and did not change on subsequent run + assert: + that: + - user_test_local_expires1 is changed + - user_test_local_expires2 is not changed + tags: + - user_test_local_mode + +- name: Verify expiration date for Linux + block: + - name: LINUX | Get expiration date for local_ansibulluser + getent: + database: shadow + key: local_ansibulluser + tags: + - user_test_local_mode + + - name: LINUX | Ensure proper expiration date was set + assert: + that: + - getent_shadow['local_ansibulluser'][6] == '29281' + tags: + - user_test_local_mode + when: ansible_facts.os_family in ['RedHat', 'Debian', 'Suse'] + +- name: Change timezone + timezone: + name: America/Denver + register: original_timezone + tags: + - timezone + - user_test_local_mode + +- name: Change system timezone to make sure expiration comparison works properly + block: + - name: Create user with expiration again to ensure no change is made in a new timezone + user: + name: local_ansibulluser + state: present + local: yes + expires: 2529881062 + register: user_test_local_different_tz + tags: + - timezone + - user_test_local_mode + + - name: Ensure that no change was reported + assert: + that: + - user_test_local_different_tz is not changed + tags: + - timezone + - user_test_local_mode + + always: + - name: Restore original timezone - {{ original_timezone.diff.before.name }} + timezone: + name: "{{ original_timezone.diff.before.name }}" + when: original_timezone.diff.before.name != "n/a" + tags: + - timezone + - user_test_local_mode + + - name: Restore original timezone when n/a + file: + path: /etc/sysconfig/clock + state: absent + when: + - original_timezone.diff.before.name == "n/a" + - "'/etc/sysconfig/clock' in original_timezone.msg" + tags: + - timezone + - user_test_local_mode + + +- name: Unexpire user + user: + name: local_ansibulluser + state: present + local: yes + expires: -1 + register: user_test_local_expires3 + tags: + - user_test_local_mode + +- name: Verify un expiration date for Linux + block: + - name: LINUX | Get expiration date for local_ansibulluser + getent: + database: shadow + key: local_ansibulluser + tags: + - user_test_local_mode + + - name: LINUX | Ensure proper expiration date was set + assert: + msg: "expiry is supposed to be empty or -1, not {{ getent_shadow['local_ansibulluser'][6] }}" + that: + - not getent_shadow['local_ansibulluser'][6] or getent_shadow['local_ansibulluser'][6] | int < 0 + tags: + - user_test_local_mode + when: ansible_facts.os_family in ['RedHat', 'Debian', 'Suse'] + +- name: Verify un expiration date for Linux/BSD + block: + - name: Unexpire user again to check for change + user: + name: local_ansibulluser + state: present + local: yes + expires: -1 + register: user_test_local_expires4 + tags: + - user_test_local_mode + + - name: Ensure first expiration reported a change and second did not + assert: + msg: The second run of the expiration removal task reported a change when it should not + that: + - user_test_local_expires3 is changed + - user_test_local_expires4 is not changed + tags: + - user_test_local_mode + when: ansible_facts.os_family in ['RedHat', 'Debian', 'Suse', 'FreeBSD'] + +# Test setting no expiration when creating a new account +# https://github.com/ansible/ansible/issues/44155 +- name: Remove local_ansibulluser + user: + name: local_ansibulluser + state: absent + remove: yes + local: yes + tags: + - user_test_local_mode + +- name: Create user account without expiration + user: + name: local_ansibulluser + state: present + local: yes + expires: -1 + register: user_test_local_create_no_expires_1 + tags: + - user_test_local_mode + +- name: Create user account without expiration again + user: + name: local_ansibulluser + state: present + local: yes + expires: -1 + register: user_test_local_create_no_expires_2 + tags: + - user_test_local_mode + +- name: Ensure changes were made appropriately + assert: + msg: Setting 'expires='-1 resulted in incorrect changes + that: + - user_test_local_create_no_expires_1 is changed + - user_test_local_create_no_expires_2 is not changed + tags: + - user_test_local_mode + +- name: Verify un expiration date for Linux + block: + - name: LINUX | Get expiration date for local_ansibulluser + getent: + database: shadow + key: local_ansibulluser + tags: + - user_test_local_mode + + - name: LINUX | Ensure proper expiration date was set + assert: + msg: "expiry is supposed to be empty or -1, not {{ getent_shadow['local_ansibulluser'][6] }}" + that: + - not getent_shadow['local_ansibulluser'][6] or getent_shadow['local_ansibulluser'][6] | int < 0 + tags: + - user_test_local_mode + when: ansible_facts.os_family in ['RedHat', 'Debian', 'Suse'] + +# Test setting epoch 0 expiration when creating a new account, then removing the expiry +# https://github.com/ansible/ansible/issues/47114 +- name: Remove local_ansibulluser + user: + name: local_ansibulluser + state: absent + remove: yes + local: yes + tags: + - user_test_local_mode + +- name: Create user account with epoch 0 expiration + user: + name: local_ansibulluser + state: present + local: yes + expires: 0 + register: user_test_local_expires_create0_1 + tags: + - user_test_local_mode + +- name: Create user account with epoch 0 expiration again + user: + name: local_ansibulluser + state: present + local: yes + expires: 0 + register: user_test_local_expires_create0_2 + tags: + - user_test_local_mode + +- name: Change the user account to remove the expiry time + user: + name: local_ansibulluser + expires: -1 + local: yes + register: user_test_local_remove_expires_1 + tags: + - user_test_local_mode + +- name: Change the user account to remove the expiry time again + user: + name: local_ansibulluser + expires: -1 + local: yes + register: user_test_local_remove_expires_2 + tags: + - user_test_local_mode + + +- name: Verify un expiration date for Linux + block: + - name: LINUX | Ensure changes were made appropriately + assert: + msg: Creating an account with 'expries=0' then removing that expriation with 'expires=-1' resulted in incorrect changes + that: + - user_test_local_expires_create0_1 is changed + - user_test_local_expires_create0_2 is not changed + - user_test_local_remove_expires_1 is changed + - user_test_local_remove_expires_2 is not changed + tags: + - user_test_local_mode + + - name: LINUX | Get expiration date for local_ansibulluser + getent: + database: shadow + key: local_ansibulluser + tags: + - user_test_local_mode + + - name: LINUX | Ensure proper expiration date was set + assert: + msg: "expiry is supposed to be empty or -1, not {{ getent_shadow['local_ansibulluser'][6] }}" + that: + - not getent_shadow['local_ansibulluser'][6] or getent_shadow['local_ansibulluser'][6] | int < 0 + tags: + - user_test_local_mode + when: ansible_facts.os_family in ['RedHat', 'Debian', 'Suse'] + +# Test expiration with a very large negative number. This should have the same +# result as setting -1. +- name: Set expiration date using very long negative number + user: + name: local_ansibulluser + state: present + local: yes + expires: -2529881062 + register: user_test_local_expires5 + tags: + - user_test_local_mode + +- name: Ensure no change was made + assert: + that: + - user_test_local_expires5 is not changed + tags: + - user_test_local_mode + +- name: Verify un expiration date for Linux + block: + - name: LINUX | Get expiration date for local_ansibulluser + getent: + database: shadow + key: local_ansibulluser + tags: + - user_test_local_mode + + - name: LINUX | Ensure proper expiration date was set + assert: + msg: "expiry is supposed to be empty or -1, not {{ getent_shadow['local_ansibulluser'][6] }}" + that: + - not getent_shadow['local_ansibulluser'][6] or getent_shadow['local_ansibulluser'][6] | int < 0 + tags: + - user_test_local_mode + when: ansible_facts.os_family in ['RedHat', 'Debian', 'Suse'] diff --git a/test/integration/targets/user/tasks/main.yml b/test/integration/targets/user/tasks/main.yml index 5b83166e698..15a5e28307f 100644 --- a/test/integration/targets/user/tasks/main.yml +++ b/test/integration/targets/user/tasks/main.yml @@ -1135,3 +1135,6 @@ when: ansible_facts.system in ['Linux'] tags: - user_test_local_mode + +- name: Test expires for local users + import_tasks: expires_local.yml