diff --git a/changelogs/fragments/user-expires.yaml b/changelogs/fragments/user-expires.yaml new file mode 100644 index 0000000000..3e661f9018 --- /dev/null +++ b/changelogs/fragments/user-expires.yaml @@ -0,0 +1,2 @@ +bugfixes: + - user - only change the expiration time when necessary (https://github.com/ansible/ansible/issues/13235) diff --git a/lib/ansible/modules/system/user.py b/lib/ansible/modules/system/user.py index d6d62877b6..2dd68c7937 100644 --- a/lib/ansible/modules/system/user.py +++ b/lib/ansible/modules/system/user.py @@ -270,6 +270,7 @@ class User(object): platform = 'Generic' distribution = None SHADOWFILE = '/etc/shadow' + SHADOWFILE_EXPIRE_INDEX = 7 DATE_FORMAT = '%Y-%m-%d' def __new__(cls, *args, **kwargs): @@ -532,8 +533,16 @@ class User(object): cmd.append(self.shell) if self.expires: - cmd.append('-e') - cmd.append(time.strftime(self.DATE_FORMAT, self.expires)) + current_expires = self.user_password()[1] + + # Convert days since Epoch to seconds since Epoch as struct_time + total_seconds = int(current_expires) * 86400 + current_expires = time.gmtime(total_seconds) + + # Compare year, month, and day only + if current_expires[:3] != self.expires[:3]: + cmd.append('-e') + cmd.append(time.strftime(self.DATE_FORMAT, self.expires)) if self.password_lock: cmd.append('-L') @@ -616,25 +625,30 @@ class User(object): return False info = self.get_pwd_info() if len(info[1]) == 1 or len(info[1]) == 0: - info[1] = self.user_password() + info[1] = self.user_password()[0] return info def user_password(self): passwd = '' + expires = '' if HAVE_SPWD: try: passwd = spwd.getspnam(self.name)[1] + expires = spwd.getspnam(self.name)[7] + return passwd, expires except KeyError: - return passwd + return passwd, expires + if not self.user_exists(): - return passwd + return passwd, expires elif self.SHADOWFILE: # Read shadow file for user's encrypted password string if os.path.exists(self.SHADOWFILE) and os.access(self.SHADOWFILE, os.R_OK): for line in open(self.SHADOWFILE).readlines(): if line.startswith('%s:' % self.name): passwd = line.split(':')[1] - return passwd + expires = line.split(':')[self.SHADOWFILE_EXPIRE_INDEX] + return passwd, expires def get_ssh_key_path(self): info = self.user_info() @@ -767,6 +781,8 @@ class FreeBsdUser(User): platform = 'FreeBSD' distribution = None SHADOWFILE = '/etc/master.passwd' + SHADOWFILE_EXPIRE_INDEX = 6 + DATE_FORMAT = '%d-%b-%Y' def remove_user(self): cmd = [ @@ -830,9 +846,8 @@ class FreeBsdUser(User): cmd.append(self.login_class) if self.expires: - days = (time.mktime(self.expires) - time.time()) // 86400 cmd.append('-e') - cmd.append(str(int(days))) + cmd.append(time.strftime(self.DATE_FORMAT, self.expires)) # system cannot be handled currently - should we error if its requested? # create the user @@ -932,8 +947,12 @@ class FreeBsdUser(User): cmd.append(','.join(new_groups)) if self.expires: - cmd.append('-e') - cmd.append(str(int(time.mktime(self.expires)))) + current_expires = time.gmtime(int(self.user_password()[1])) + + # Compare year, month, and day only + if current_expires[:3] != self.expires[:3]: + cmd.append('-e') + cmd.append(time.strftime(self.DATE_FORMAT, self.expires)) # modify the user if cmd will do anything if cmd_len != len(cmd): diff --git a/test/integration/targets/user/aliases b/test/integration/targets/user/aliases index 4485d76162..8e7d715f9c 100644 --- a/test/integration/targets/user/aliases +++ b/test/integration/targets/user/aliases @@ -1 +1,2 @@ +destructive posix/ci/group1 diff --git a/test/integration/targets/user/tasks/main.yml b/test/integration/targets/user/tasks/main.yml index 60e1d06f06..c013a1d5b1 100644 --- a/test/integration/targets/user/tasks/main.yml +++ b/test/integration/targets/user/tasks/main.yml @@ -20,142 +20,229 @@ shell: python -c 'import jinja2; print(jinja2.__version__)' register: jinja2_version delegate_to: localhost -- debug: var=jinja2_version + changed_when: no + +- debug: + msg: "Jinja version: {{ jinja2_version.stdout }}, Python version: {{ ansible_python_version }}" + -## ## user add -## -# + - name: remove the test user user: - name: ansibulluser - state: absent + name: ansibulluser + state: absent - name: try to create a user user: - name: ansibulluser - state: present + name: ansibulluser + state: present register: user_test0 -- debug: var=user_test0 + +- debug: + var: user_test0 + verbosity: 2 - name: make a list of users - script: userlist.sh "{{ ansible_distribution }}" + script: userlist.sh {{ ansible_distribution }} register: user_names -- debug: var=user_names + +- debug: + var: user_names + verbosity: 2 - name: validate results for testcase 0 assert: - that: - - 'user_test0.changed is defined' - - 'user_test0.changed' - - '"ansibulluser" in user_names.stdout_lines' + that: + - user_test0 is changed + - '"ansibulluser" in user_names.stdout_lines' + -## ## user check -## - name: run existing user check tests user: - name: "{{ user_names.stdout_lines|random }}" - state: present - create_home: no + name: "{{ user_names.stdout_lines | random }}" + state: present + create_home: no with_sequence: start=1 end=5 register: user_test1 -- debug: var=user_test1 + +- debug: + var: user_test1 + verbosity: 2 - name: validate results for testcase 1 assert: - that: - - 'user_test1.results is defined' - - 'user_test1.results|length == 5' + that: + - user_test1.results is defined + - user_test1.results | length == 5 - name: validate changed results for testcase 1 (jinja >= 2.6) assert: - that: - - "user_test1.results|map(attribute='changed')|unique|list == [False]" - - "user_test1.results|map(attribute='state')|unique|list == ['present']" - when: "jinja2_version.stdout is version('2.6', '>=')" + that: + - user_test1.results | map(attribute='changed') | unique | list == [False] + - user_test1.results | map(attribute='state') | unique | list == ['present'] + when: jinja2_version.stdout is version('2.6', '>=') -- name: validate changed results for testcase 1 (jinja >= 2.6) +- name: validate changed results for testcase 1 (jinja < 2.6) assert: - that: - - "not user_test1.results[0]['changed']" - - "not user_test1.results[1]['changed']" - - "not user_test1.results[2]['changed']" - - "not user_test1.results[3]['changed']" - - "not user_test1.results[4]['changed']" - - "user_test1.results[0]['state'] == 'present'" - - "user_test1.results[1]['state'] == 'present'" - - "user_test1.results[2]['state'] == 'present'" - - "user_test1.results[3]['state'] == 'present'" - - "user_test1.results[4]['state'] == 'present'" - when: "jinja2_version.stdout is version('2.6', '<')" + that: + - "user_test1.results[0] is not changed" + - "user_test1.results[1] is not changed" + - "user_test1.results[2] is not changed" + - "user_test1.results[3] is not changed" + - "user_test1.results[4] is not changed" + - "user_test1.results[0]['state'] == 'present'" + - "user_test1.results[1]['state'] == 'present'" + - "user_test1.results[2]['state'] == 'present'" + - "user_test1.results[3]['state'] == 'present'" + - "user_test1.results[4]['state'] == 'present'" + when: jinja2_version.stdout is version('2.6', '<') + -## ## user remove -## - + - name: try to delete the user user: - name: ansibulluser - state: absent - force: true + name: ansibulluser + state: absent + force: true register: user_test2 + - name: make a new list of users - script: userlist.sh "{{ ansible_distribution }}" + script: userlist.sh {{ ansible_distribution }} register: user_names2 -- debug: var=user_names2 + +- debug: + var: user_names2 + verbosity: 2 + - name: validate results for testcase 2 assert: - that: - - '"ansibulluser" not in user_names2.stdout_lines' + that: + - '"ansibulluser" not in user_names2.stdout_lines' - block: - - name: create non-system user on OSX to test the shell is set to /bin/bash + - name: create non-system user on macOS to test the shell is set to /bin/bash user: - name: osxuser - register: osxuser_output + name: macosuser + register: macosuser_output - name: validate the shell is set to /bin/bash assert: that: - - 'osxuser_output.shell == "/bin/bash"' + - 'macosuser_output.shell == "/bin/bash"' - name: cleanup user: - name: osxuser + name: macosuser state: absent - - name: create system user on OSX to test the shell is set to /usr/bin/false + - name: create system user on macos to test the shell is set to /usr/bin/false user: - name: osxuser + name: macosuser system: yes - register: osxuser_output + register: macosuser_output - name: validate the shell is set to /usr/bin/false assert: that: - - 'osxuser_output.shell == "/usr/bin/false"' + - 'macosuser_output.shell == "/usr/bin/false"' - name: cleanup user: - name: osxuser + name: macosuser state: absent - - name: create non-system user on OSX and set the shell to /bin/sh + - name: create non-system user on macos and set the shell to /bin/sh user: - name: osxuser + name: macosuser shell: /bin/sh - register: osxuser_output + register: macosuser_output - name: validate the shell is set to /bin/sh assert: that: - - 'osxuser_output.shell == "/bin/sh"' + - 'macosuser_output.shell == "/bin/sh"' - name: cleanup user: - name: osxuser + name: macosuser state: absent when: ansible_distribution == "MacOSX" + + +## user expires +# Date is March 3, 2050 +- name: Create user with expiration + user: + name: ansibulluser + state: present + expires: 2529881062 + register: user_test_expires1 + +- name: Create user with expiration again to ensure no change is made + user: + name: ansibulluser + state: present + expires: 2529881062 + register: user_test_expires2 + +- name: Ensure that account with expiration was created and did not change on subsequent run + assert: + that: + - user_test_expires1 is changed + - user_test_expires2 is not changed + +- name: Verify expiration date for Linux + block: + - name: LINUX | Get expiration date for ansibulluser + getent: + database: shadow + key: ansibulluser + + - name: LINUX | Ensure proper expiration date was set + assert: + that: + - getent_shadow['ansibulluser'][6] == '29281' + when: ansible_os_family in ['RedHat', 'Debian', 'Suse'] + + +- name: Verify expiration date for BSD + block: + - name: BSD | Get expiration date for ansibulluser + shell: 'grep ansibulluser /etc/master.passwd | cut -d: -f 7' + changed_when: no + register: bsd_account_expiration + + - name: BSD | Ensure proper expiration date was set + assert: + that: + - bsd_account_expiration.stdout == '2529878400' + when: ansible_os_family == 'FreeBSD' + +- name: Change timezone + timezone: + name: America/Denver + register: original_timezone + +- 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: ansibulluser + state: present + expires: 2529881062 + register: user_test_different_tz + + - name: Ensure that no change was reported + assert: + that: + - user_test_different_tz is not changed + + always: + - name: Restore original timezone - {{ original_timezone.diff.before.name }} + timezone: + name: "{{ original_timezone.diff.before.name }}"