From 1e595493d92952d168af6cf1bef58d41abcb6a8f Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Thu, 14 Mar 2019 22:16:53 -0400 Subject: [PATCH] User module - Check local database when local is specified in the task (#51088) The output of pw.getpwnam() does not distinbuish between local and remote accounts. It will return a result if an account exists locally or in the directory. When local is set to True in the task parameters, look through the local password database explicitly. * Ensure luseradd is present for tests * Add docs and warnings about local mode --- changelogs/user-read-passwd-when-local.yaml | 2 + lib/ansible/modules/system/user.py | 39 +++++++-- test/integration/targets/user/tasks/main.yml | 84 +++++++++++++++++++- 3 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 changelogs/user-read-passwd-when-local.yaml diff --git a/changelogs/user-read-passwd-when-local.yaml b/changelogs/user-read-passwd-when-local.yaml new file mode 100644 index 0000000000..829cf7ac6a --- /dev/null +++ b/changelogs/user-read-passwd-when-local.yaml @@ -0,0 +1,2 @@ +bugfixes: + - user - fix a bug when checking if a local user account exists on a system using directory authentication (https://github.com/ansible/ansible/issues/50947, https://github.com/ansible/ansible/issues/38206) diff --git a/lib/ansible/modules/system/user.py b/lib/ansible/modules/system/user.py index d54ad2827c..7ed5764632 100644 --- a/lib/ansible/modules/system/user.py +++ b/lib/ansible/modules/system/user.py @@ -206,7 +206,9 @@ options: - Forces the use of "local" command alternatives on platforms that implement it. - This is useful in environments that use centralized authentification when you want to manipulate the local users (i.e. it uses C(luseradd) instead of C(useradd)). - - This requires that these commands exist on the targeted host, otherwise it will be a fatal error. + - This will check C(/etc/passwd) for an existing account before invoking commands. If the local account database + exists somewhere other than C(/etc/passwd), this setting will not work properly. + - This requires that the above commands as well as C(/etc/passwd) must exist on the target host, otherwise it will be a fatal error. type: bool default: no version_added: "2.4" @@ -446,6 +448,7 @@ class User(object): platform = 'Generic' distribution = None + PASSWORDFILE = '/etc/passwd' SHADOWFILE = '/etc/shadow' SHADOWFILE_EXPIRE_INDEX = 7 LOGIN_DEFS = '/etc/login.defs' @@ -840,11 +843,35 @@ class User(object): return groups def user_exists(self): - try: - if pwd.getpwnam(self.name): - return True - except KeyError: - return False + # The pwd module does not distinguish between local and directory accounts. + # It's output cannot be used to determine whether or not an account exists locally. + # It returns True if the account exists locally or in the directory, so instead + # look in the local PASSWORD file for an existing account. + if self.local: + if not os.path.exists(self.PASSWORDFILE): + self.module.fail_json(msg="'local: true' specified but unable to find local account file {0} to parse.".format(self.PASSWORDFILE)) + + exists = False + name_test = '{0}:'.format(self.name) + with open(self.PASSWORDFILE, 'rb') as f: + reversed_lines = f.readlines()[::-1] + for line in reversed_lines: + if line.startswith(to_bytes(name_test)): + exists = True + break + + self.module.warn( + "'local: true' specified and user was not found in {file}. " + "The local user account may already exist if the local account database exists somewhere other than {file}.".format(file=self.PASSWORDFILE)) + + return exists + + else: + try: + if pwd.getpwnam(self.name): + return True + except KeyError: + return False def get_pwd_info(self): if not self.user_exists(): diff --git a/test/integration/targets/user/tasks/main.yml b/test/integration/targets/user/tasks/main.yml index bc268e2d51..3923c0d79c 100644 --- a/test/integration/targets/user/tasks/main.yml +++ b/test/integration/targets/user/tasks/main.yml @@ -255,7 +255,7 @@ mode = oct(0o777 & ~umask) print(str(mode).replace('o', '')) args: - executable: python + executable: "{{ ansible_facts.python.executable }}" register: user_login_defs_umask - name: validate that user home dir is created @@ -775,3 +775,85 @@ password_lock: no when: ansible_facts['system'] in ['FreeBSD', 'OpenBSD', 'Linux'] + + + ## Check local mode + # Even if we don't have a system that is bound to a directory, it's useful + # to run with local: true to exercise the code path that reads through the local + # user database file. + # https://github.com/ansible/ansible/issues/50947 + +- name: Create /etc/gshadow + file: + path: /etc/gshadow + state: touch + when: ansible_facts.os_family == 'Suse' + tags: + - user_test_local_mode + +- name: Create /etc/libuser.conf + file: + path: /etc/libuser.conf + state: touch + when: + - ansible_facts.distribution == 'Ubuntu' + - ansible_facts.distribution_major_version is version_compare('16', '==') + tags: + - user_test_local_mode + +- name: Ensure luseradd is present + action: "{{ ansible_facts.pkg_mgr }}" + args: + name: libuser + state: present + when: ansible_facts.system in ['Linux'] + tags: + - user_test_local_mode + +- name: Create local_ansibulluser + user: + name: local_ansibulluser + state: present + local: yes + register: local_user_test_1 + tags: + - user_test_local_mode + +- name: Create local_ansibulluser again + user: + name: local_ansibulluser + state: present + local: yes + register: local_user_test_2 + tags: + - user_test_local_mode + +- name: Remove local_ansibulluser + user: + name: local_ansibulluser + state: absent + remove: yes + local: yes + register: local_user_test_3 + tags: + - user_test_local_mode + +- name: Remove local_ansibulluser again + user: + name: local_ansibulluser + state: absent + remove: yes + local: yes + register: local_user_test_4 + tags: + - user_test_local_mode + +- name: Ensure local user accounts were created + assert: + that: + - local_user_test_1 is changed + - local_user_test_2 is not changed + - local_user_test_3 is changed + - local_user_test_4 is not changed + tags: + - user_test_local_mode