wbrawner/ansible
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Only change expiration date if it is different (#38885)

Browse source 
* Only change expiration date if it is different

Modify user_info() method to also return the password expiration.
Compare current and desired expiration times and only change if they are different.

* Improve formatting on user tests

* Add integration test for expiration

* Add changelog fragment

* Improve integration test

Skip macOS and use getent module for validating expiration date.

* Fix expiration change for FreeBSD

* Don't use datetime since the total_seconds method isn't available on CentOS 6

* Use better name for expiration index field

Use separate tasks for verifying expiration date on BSD

* Use calendar.timegm() rather than time.mktime()

calendar.timegm() is the inverse of time.gmtime() and returns a timestamp in UTC not localtime
Add tests that change the system timezone away from UTC

* Mark tests as destructive and use test for change status

* Fix account expiration for FreeBSD

Use DATE_FORMAT when setting expiration date on FreeBSD. Previously the argument passed to -e was an integer of days since epoch when the account will expire which was inserted directly into master.passwd. This value is interpreted as seconds since epoch by the system, meaning the account expiration was actually set to a few hours past epoch.

Greatly simply comparing desired  and current expiration time by using the first three values of the struct_time tuple rather than doing a whole bunch of manipulations of the seconds since epoch.
This commit is contained in:
Sam Doran 2018-05-01 11:19:02 -04:00 committed by Brian Coca
parent b9f7f582d1
commit 5a6bdef76b
4 changed files with 185 additions and 76 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
changelogs/fragments/user-expires.yaml Normal file
View file

@ -0,0 +1,2 @@
bugfixes:
- user - only change the expiration time when necessary (https://github.com/ansible/ansible/issues/13235)

39
lib/ansible/modules/system/user.py
View file

@ -270,6 +270,7 @@ class User(object):
platform = 'Generic' platform = 'Generic'
distribution = None distribution = None
SHADOWFILE = '/etc/shadow' SHADOWFILE = '/etc/shadow'
SHADOWFILE_EXPIRE_INDEX = 7
DATE_FORMAT = '%Y-%m-%d' DATE_FORMAT = '%Y-%m-%d'
def __new__(cls, *args, **kwargs): def __new__(cls, *args, **kwargs):
@ -532,8 +533,16 @@ class User(object):
cmd.append(self.shell) cmd.append(self.shell)
if self.expires: if self.expires:
cmd.append('-e') current_expires = self.user_password()[1]
cmd.append(time.strftime(self.DATE_FORMAT, self.expires))
# 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: if self.password_lock:
cmd.append('-L') cmd.append('-L')
@ -616,25 +625,30 @@ class User(object):
return False return False
info = self.get_pwd_info() info = self.get_pwd_info()
if len(info[1]) == 1 or len(info[1]) == 0: if len(info[1]) == 1 or len(info[1]) == 0:
info[1] = self.user_password() info[1] = self.user_password()[0]
return info return info
def user_password(self): def user_password(self):
passwd = '' passwd = ''
expires = ''
if HAVE_SPWD: if HAVE_SPWD:
try: try:
passwd = spwd.getspnam(self.name)[1] passwd = spwd.getspnam(self.name)[1]
expires = spwd.getspnam(self.name)[7]
return passwd, expires
except KeyError: except KeyError:
return passwd return passwd, expires
if not self.user_exists(): if not self.user_exists():
return passwd return passwd, expires
elif self.SHADOWFILE: elif self.SHADOWFILE:
# Read shadow file for user's encrypted password string # Read shadow file for user's encrypted password string
if os.path.exists(self.SHADOWFILE) and os.access(self.SHADOWFILE, os.R_OK): if os.path.exists(self.SHADOWFILE) and os.access(self.SHADOWFILE, os.R_OK):
for line in open(self.SHADOWFILE).readlines(): for line in open(self.SHADOWFILE).readlines():
if line.startswith('%s:' % self.name): if line.startswith('%s:' % self.name):
passwd = line.split(':')[1] passwd = line.split(':')[1]
return passwd expires = line.split(':')[self.SHADOWFILE_EXPIRE_INDEX]
return passwd, expires
def get_ssh_key_path(self): def get_ssh_key_path(self):
info = self.user_info() info = self.user_info()
@ -767,6 +781,8 @@ class FreeBsdUser(User):
platform = 'FreeBSD' platform = 'FreeBSD'
distribution = None distribution = None
SHADOWFILE = '/etc/master.passwd' SHADOWFILE = '/etc/master.passwd'
SHADOWFILE_EXPIRE_INDEX = 6
DATE_FORMAT = '%d-%b-%Y'
def remove_user(self): def remove_user(self):
cmd = [ cmd = [
@ -830,9 +846,8 @@ class FreeBsdUser(User):
cmd.append(self.login_class) cmd.append(self.login_class)
if self.expires: if self.expires:
days = (time.mktime(self.expires) - time.time()) // 86400
cmd.append('-e') 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? # system cannot be handled currently - should we error if its requested?
# create the user # create the user
@ -932,8 +947,12 @@ class FreeBsdUser(User):
cmd.append(','.join(new_groups)) cmd.append(','.join(new_groups))
if self.expires: if self.expires:
cmd.append('-e') current_expires = time.gmtime(int(self.user_password()[1]))
cmd.append(str(int(time.mktime(self.expires))))
# 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 # modify the user if cmd will do anything
if cmd_len != len(cmd): if cmd_len != len(cmd):

1
test/integration/targets/user/aliases
View file

@ -1 +1,2 @@
destructive
posix/ci/group1 posix/ci/group1

219
test/integration/targets/user/tasks/main.yml
View file

@ -20,142 +20,229 @@
shell: python -c 'import jinja2; print(jinja2.__version__)' shell: python -c 'import jinja2; print(jinja2.__version__)'
register: jinja2_version register: jinja2_version
delegate_to: localhost delegate_to: localhost
- debug: var=jinja2_version changed_when: no
- debug:
msg: "Jinja version: {{ jinja2_version.stdout }}, Python version: {{ ansible_python_version }}"
##
## user add ## user add
##
#
- name: remove the test user - name: remove the test user
user: user:
name: ansibulluser name: ansibulluser
state: absent state: absent
- name: try to create a user - name: try to create a user
user: user:
name: ansibulluser name: ansibulluser
state: present state: present
register: user_test0 register: user_test0
- debug: var=user_test0
- debug:
var: user_test0
verbosity: 2
- name: make a list of users - name: make a list of users
script: userlist.sh "{{ ansible_distribution }}" script: userlist.sh {{ ansible_distribution }}
register: user_names register: user_names
- debug: var=user_names
- debug:
var: user_names
verbosity: 2
- name: validate results for testcase 0 - name: validate results for testcase 0
assert: assert:
that: that:
- 'user_test0.changed is defined' - user_test0 is changed
- 'user_test0.changed' - '"ansibulluser" in user_names.stdout_lines'
- '"ansibulluser" in user_names.stdout_lines'
##
## user check ## user check
##
- name: run existing user check tests - name: run existing user check tests
user: user:
name: "{{ user_names.stdout_lines|random }}" name: "{{ user_names.stdout_lines | random }}"
state: present state: present
create_home: no create_home: no
with_sequence: start=1 end=5 with_sequence: start=1 end=5
register: user_test1 register: user_test1
- debug: var=user_test1
- debug:
var: user_test1
verbosity: 2
- name: validate results for testcase 1 - name: validate results for testcase 1
assert: assert:
that: that:
- 'user_test1.results is defined' - user_test1.results is defined
- 'user_test1.results|length == 5' - user_test1.results | length == 5
- name: validate changed results for testcase 1 (jinja >= 2.6) - name: validate changed results for testcase 1 (jinja >= 2.6)
assert: assert:
that: that:
- "user_test1.results|map(attribute='changed')|unique|list == [False]" - user_test1.results | map(attribute='changed') | unique | list == [False]
- "user_test1.results|map(attribute='state')|unique|list == ['present']" - user_test1.results | map(attribute='state') | unique | list == ['present']
when: "jinja2_version.stdout is version('2.6', '>=')" 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: assert:
that: that:
- "not user_test1.results[0]['changed']" - "user_test1.results[0] is not changed"
- "not user_test1.results[1]['changed']" - "user_test1.results[1] is not changed"
- "not user_test1.results[2]['changed']" - "user_test1.results[2] is not changed"
- "not user_test1.results[3]['changed']" - "user_test1.results[3] is not changed"
- "not user_test1.results[4]['changed']" - "user_test1.results[4] is not changed"
- "user_test1.results[0]['state'] == 'present'" - "user_test1.results[0]['state'] == 'present'"
- "user_test1.results[1]['state'] == 'present'" - "user_test1.results[1]['state'] == 'present'"
- "user_test1.results[2]['state'] == 'present'" - "user_test1.results[2]['state'] == 'present'"
- "user_test1.results[3]['state'] == 'present'" - "user_test1.results[3]['state'] == 'present'"
- "user_test1.results[4]['state'] == 'present'" - "user_test1.results[4]['state'] == 'present'"
when: "jinja2_version.stdout is version('2.6', '<')" when: jinja2_version.stdout is version('2.6', '<')
##
## user remove ## user remove
##
- name: try to delete the user - name: try to delete the user
user: user:
name: ansibulluser name: ansibulluser
state: absent state: absent
force: true force: true
register: user_test2 register: user_test2
- name: make a new list of users - name: make a new list of users
script: userlist.sh "{{ ansible_distribution }}" script: userlist.sh {{ ansible_distribution }}
register: user_names2 register: user_names2
- debug: var=user_names2
- debug:
var: user_names2
verbosity: 2
- name: validate results for testcase 2 - name: validate results for testcase 2
assert: assert:
that: that:
- '"ansibulluser" not in user_names2.stdout_lines' - '"ansibulluser" not in user_names2.stdout_lines'
- block: - 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: user:
name: osxuser name: macosuser
register: osxuser_output register: macosuser_output
- name: validate the shell is set to /bin/bash - name: validate the shell is set to /bin/bash
assert: assert:
that: that:
- 'osxuser_output.shell == "/bin/bash"' - 'macosuser_output.shell == "/bin/bash"'
- name: cleanup - name: cleanup
user: user:
name: osxuser name: macosuser
state: absent 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: user:
name: osxuser name: macosuser
system: yes system: yes
register: osxuser_output register: macosuser_output
- name: validate the shell is set to /usr/bin/false - name: validate the shell is set to /usr/bin/false
assert: assert:
that: that:
- 'osxuser_output.shell == "/usr/bin/false"' - 'macosuser_output.shell == "/usr/bin/false"'
- name: cleanup - name: cleanup
user: user:
name: osxuser name: macosuser
state: absent 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: user:
name: osxuser name: macosuser
shell: /bin/sh shell: /bin/sh
register: osxuser_output register: macosuser_output
- name: validate the shell is set to /bin/sh - name: validate the shell is set to /bin/sh
assert: assert:
that: that:
- 'osxuser_output.shell == "/bin/sh"' - 'macosuser_output.shell == "/bin/sh"'
- name: cleanup - name: cleanup
user: user:
name: osxuser name: macosuser
state: absent state: absent
when: ansible_distribution == "MacOSX" 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 }}"