From df335d91b0b84a127c8d1256d6683aa1193d415e Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Thu, 23 Aug 2018 12:29:33 -0400 Subject: [PATCH] Properly un expire account on creation (#44174) When creating a new account, check to see if the expiration parameter is negative and pass in the appropriate parameter. Since the negative integer passed into expires is converted to time.struct_time which in turn gets converted to a formatted time string when passed to the underlying command, a -1 or large negative number would result in passing a date before 1970-01-01 to the underlying command. This had the opposite effect of creating an account with no expiration account resulting in a newly created account that was already expired, or just throwing an error on certain systems. --- lib/ansible/modules/system/user.py | 10 ++- test/integration/targets/user/tasks/main.yml | 94 ++++++++++++++++++-- 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/lib/ansible/modules/system/user.py b/lib/ansible/modules/system/user.py index 0d5680c4b2..b5817d75ff 100644 --- a/lib/ansible/modules/system/user.py +++ b/lib/ansible/modules/system/user.py @@ -560,7 +560,10 @@ class User(object): if self.expires is not None: cmd.append('-e') - cmd.append(time.strftime(self.DATE_FORMAT, self.expires)) + if self.expires < time.gmtime(0): + cmd.append('') + else: + cmd.append(time.strftime(self.DATE_FORMAT, self.expires)) if self.password is not None: cmd.append('-p') @@ -1008,7 +1011,10 @@ class FreeBsdUser(User): if self.expires is not None: cmd.append('-e') - cmd.append(time.strftime(self.DATE_FORMAT, self.expires)) + if self.expires < time.gmtime(0): + cmd.append('0') + else: + cmd.append(time.strftime(self.DATE_FORMAT, self.expires)) # system cannot be handled currently - should we error if its requested? # create the user diff --git a/test/integration/targets/user/tasks/main.yml b/test/integration/targets/user/tasks/main.yml index edb9ddfe72..86595a48f4 100644 --- a/test/integration/targets/user/tasks/main.yml +++ b/test/integration/targets/user/tasks/main.yml @@ -263,14 +263,14 @@ ## user expires # Date is March 3, 2050 -- name: Create user with expiration +- name: Set user expiration user: name: ansibulluser state: present expires: 2529881062 register: user_test_expires1 -- name: Create user with expiration again to ensure no change is made +- name: Set user expiration again to ensure no change is made user: name: ansibulluser state: present @@ -351,9 +351,9 @@ - name: LINUX | Ensure proper expiration date was set assert: - msg: "expiry is supposed to be empty or -1, not {{getent_shadow['ansibulluser'][6]}}" + msg: "expiry is supposed to be empty or -1, not {{ getent_shadow['ansibulluser'][6] }}" that: - - not getent_shadow['ansibulluser'][6] or getent_shadow['ansibulluser'][6] < 0 + - not getent_shadow['ansibulluser'][6] or getent_shadow['ansibulluser'][6] | int < 0 when: ansible_os_family in ['RedHat', 'Debian', 'Suse'] - name: Verify un expiration date for linux/BSD @@ -382,7 +382,91 @@ - name: BSD | Ensure proper expiration date was set assert: - msg: "expiry is supposed to be '0', not {{bsd_account_expiration.stdout}}" + msg: "expiry is supposed to be '0', not {{ bsd_account_expiration.stdout }}" + that: + - bsd_account_expiration.stdout == '0' + when: ansible_os_family == 'FreeBSD' + +# Test setting no expiration when creating a new account +# https://github.com/ansible/ansible/issues/44155 +- name: Remove ansibulluser + user: + name: ansibulluser + state: absent + +- name: Create user account without expiration + user: + name: ansibulluser + state: present + expires: -1 + register: user_test_create_no_expires_1 + +- name: Verify un 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: + msg: "expiry is supposed to be empty or -1, not {{ getent_shadow['ansibulluser'][6] }}" + that: + - not getent_shadow['ansibulluser'][6] or getent_shadow['ansibulluser'][6] | int < 0 + when: ansible_os_family in ['RedHat', 'Debian', 'Suse'] + +- name: Verify un 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: + msg: "expiry is supposed to be '0', not {{ bsd_account_expiration.stdout }}" + that: + - bsd_account_expiration.stdout == '0' + when: ansible_os_family == 'FreeBSD' + +# 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: ansibulluser + state: present + expires: -2529881062 + register: user_test_expires5 + +- name: Ensure no change was made + assert: + that: + - user_test_expires5 is not changed + +- name: Verify un 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: + msg: "expiry is supposed to be empty or -1, not {{ getent_shadow['ansibulluser'][6] }}" + that: + - not getent_shadow['ansibulluser'][6] or getent_shadow['ansibulluser'][6] | int < 0 + when: ansible_os_family in ['RedHat', 'Debian', 'Suse'] + +- name: Verify un 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: + msg: "expiry is supposed to be '0', not {{ bsd_account_expiration.stdout }}" that: - bsd_account_expiration.stdout == '0' when: ansible_os_family == 'FreeBSD'