Zabbix_user should not require password when ALL groups are set to LD…

…AP (#240) * zabbix_user should not ask for password with AD user groups * added improved logic and requirement that all groups must be LDAP in zabbix_user * added changelog fragment * fix wrong self referrence in zabbix_user * use old school formatting to allign with rest of the module * Fix fail_json calls
ansible-collections · Oct 18, 2020 · 77a0246 · 77a0246
1 parent 159c465
commit 77a0246
Show file tree

Hide file tree

Showing 3 changed files with 144 additions and 31 deletions.
diff --git a/changelogs/fragments/240-zabbix-user-nopass-ldap.yaml b/changelogs/fragments/240-zabbix-user-nopass-ldap.yaml
@@ -0,0 +1,3 @@
+minor_changes:
+- zabbix_user - no longer requires ``usrgrps`` when ``state=absent`` (see `#240 <https://github.com/ansible-collections/community.zabbix/issues/232>`_).
+- zabbix_user - ``passwd`` no longer required when ALL groups in ``usrgrps` use LDAP as ``gui_access`` (see `#240 <https://github.com/ansible-collections/community.zabbix/issues/232>`_).
diff --git a/plugins/modules/zabbix_user.py b/plugins/modules/zabbix_user.py
@@ -38,17 +38,21 @@
     usrgrps:
         description:
             - User groups to add the user to.
-        required: true
+            - Required when I(state=present).
+        required: false
         type: list
         elements: str
     passwd:
         description:
             - User's password.
-        required: true
+            - Required unless all of the I(usrgrps) are set to use LDAP as frontend access.
+            - Always required for Zabbix versions lower than 4.0.
+        required: false
         type: str
     override_passwd:
         description:
-            - Override password.
+            - Override password for the user.
+            - Password will not be updated on subsequent runs without setting this value to yes.
         default: no
         type: bool
     lang:
@@ -204,7 +208,7 @@
 '''
 
 EXAMPLES = r'''
-- name: create of zabbix user.
+- name: create a new zabbix user.
   community.zabbix.zabbix_user:
     server_url: "http://zabbix.example.com/zabbix/"
     login_user: Admin
@@ -238,7 +242,7 @@
     type: Zabbix super admin
     state: present
 
-- name: delete of zabbix user.
+- name: delete existing zabbix user.
   community.zabbix.zabbix_user:
     server_url: "http://zabbix.example.com/zabbix/"
     login_user: admin
@@ -271,15 +275,25 @@
 
 
 class User(ZabbixBase):
-    def get_usergroupid_by_user_group_name(self, usrgrps):
-        user_group_ids = []
-        for user_group_name in usrgrps:
-            user_group = self._zapi.usergroup.get({'output': 'extend', 'filter': {'name': user_group_name}})
-            if user_group:
-                user_group_ids.append({'usrgrpid': user_group[0]['usrgrpid']})
-            else:
-                self._module.fail_json(msg="User group not found: %s" % user_group_name)
-        return user_group_ids
+    def get_usergroups_by_name(self, usrgrps):
+        params = {
+            'output': ['usrgrpid', 'name', 'gui_access'],
+            'filter': {
+                'name': usrgrps
+            }
+        }
+        res = self._zapi.usergroup.get(params)
+        if res:
+            ids = [{'usrgrpid': g['usrgrpid']} for g in res]
+            # User can be created password-less only when all groups are LDAP
+            # Verify there are no groups with different access methods
+            not_ldap = bool([g for g in res if g['gui_access'] != '2'])
+            not_found_groups = set(usrgrps) - set([g['name'] for g in res])
+            if not_found_groups:
+                self._module.fail_json(msg='User groups not found: %s' % not_found_groups)
+            return ids, not_ldap
+        else:
+            self._module.fail_json(msg='No user groups found')
 
     def check_user_exist(self, alias):
         zbx_user = self._zapi.user.get({'output': 'extend', 'filter': {'alias': alias},
@@ -386,7 +400,7 @@ def user_parameter_difference_check(self, zbx_user, alias, name, surname, user_g
         return user_parameter_difference_check_result, diff_params
 
     def add_user(self, alias, name, surname, user_group_ids, passwd, lang, theme, autologin, autologout, refresh,
-                 rows_per_page, url, user_medias, user_type):
+                 rows_per_page, url, user_medias, user_type, not_ldap):
 
         user_medias = self.convert_user_medias_parameter_types(user_medias)
 
@@ -397,7 +411,6 @@ def add_user(self, alias, name, surname, user_group_ids, passwd, lang, theme, au
             'name': name,
             'surname': surname,
             'usrgrps': user_group_ids,
-            'passwd': passwd,
             'lang': lang,
             'theme': theme,
             'autologin': autologin,
@@ -409,6 +422,9 @@ def add_user(self, alias, name, surname, user_group_ids, passwd, lang, theme, au
             'type': user_type
         }
 
+        if LooseVersion(self._zbx_api_version) < LooseVersion('4.0') or not_ldap:
+            request_data['passwd'] = passwd
+
         diff_params = {}
         if not self._module.check_mode:
             try:
@@ -497,8 +513,8 @@ def main():
         alias=dict(type='str', required=True),
         name=dict(type='str', default=''),
         surname=dict(type='str', default=''),
-        usrgrps=dict(type='list', required=True),
-        passwd=dict(type='str', required=True, no_log=True),
+        usrgrps=dict(type='list'),
+        passwd=dict(type='str', required=False, no_log=True),
         override_passwd=dict(type='bool', required=False, default=False),
         lang=dict(type='str', default='en_GB', choices=['en_GB', 'en_US', 'zh_CN', 'cs_CZ', 'fr_FR',
                                                         'he_IL', 'it_IT', 'ko_KR', 'ja_JP', 'nb_NO',
@@ -535,6 +551,9 @@ def main():
     ))
     module = AnsibleModule(
         argument_spec=argument_spec,
+        required_if=[
+            ['state', 'present', ['usrgrps']]
+        ],
         supports_check_mode=True
     )
 
@@ -572,7 +591,11 @@ def main():
     user_ids = {}
     zbx_user = user.check_user_exist(alias)
     if state == 'present':
-        user_group_ids = user.get_usergroupid_by_user_group_name(usrgrps)
+        user_group_ids, not_ldap = user.get_usergroups_by_name(usrgrps)
+        if LooseVersion(user._zbx_api_version) < LooseVersion('4.0') or not_ldap:
+            if passwd is None:
+                module.fail_json(msg='User password is required. One or more groups are not LDAP based.')
+
         if zbx_user:
             diff_check_result, diff_params = user.user_parameter_difference_check(zbx_user, alias, name, surname,
                                                                                   user_group_ids, passwd, lang, theme,
@@ -589,7 +612,7 @@ def main():
             diff_check_result = True
             user_ids, diff_params = user.add_user(alias, name, surname, user_group_ids, passwd, lang, theme, autologin,
                                                   autologout, refresh, rows_per_page, after_login_url, user_medias,
-                                                  user_type)
+                                                  user_type, not_ldap)
 
     if state == 'absent':
         if zbx_user:

diff --git a/tests/integration/targets/test_zabbix_user/tasks/main.yml b/tests/integration/targets/test_zabbix_user/tasks/main.yml
@@ -746,7 +746,7 @@
       - not update_lang_parameter_existing_user_result.changed is sameas true
 
 # Parameter delete test from here from existing user
-- name: test - Delete user medias(SNS) for existing user with check_mode and diff
+- name: test - Delete user medias(SMS) for existing user with check_mode and diff
   zabbix_user:
     server_url: "{{ zabbix_server_url }}"
     login_user: "{{ zabbix_login_user }}"
@@ -785,7 +785,7 @@
     that:
       - delete_user_medias_existing_user_result.changed is sameas true
 
-- name: test - Delete user medias(SNS) for existing user
+- name: test - Delete user medias(SMS) for existing user
   zabbix_user:
     server_url: "{{ zabbix_server_url }}"
     login_user: "{{ zabbix_login_user }}"
@@ -864,9 +864,6 @@
     login_user: "{{ zabbix_login_user }}"
     login_password: "{{ zabbix_login_password }}"
     alias: example1
-    usrgrps:
-      - Zabbix administrators
-    passwd: update_password
     state: absent
   check_mode: yes
   diff: yes
@@ -882,9 +879,6 @@
     login_user: "{{ zabbix_login_user }}"
     login_password: "{{ zabbix_login_password }}"
     alias: example1
-    usrgrps:
-      - Zabbix administrators
-    passwd: update_password
     state: absent
   register: delete_existing_user_result
 
@@ -898,12 +892,105 @@
     login_user: "{{ zabbix_login_user }}"
     login_password: "{{ zabbix_login_password }}"
     alias: example1
-    usrgrps:
-      - Zabbix administrators
-    passwd: update_password
     state: absent
   register: delete_existing_user_result
 
 - assert:
     that:
       - not delete_existing_user_result.changed is sameas true
+
+- when: zabbix_version is version('4.0', '<')
+  block:
+  - name: test - Create new password-less user without LDAP group (fail, <4.0)
+    zabbix_user:
+      server_url: "{{ zabbix_server_url }}"
+      login_user: "{{ zabbix_login_user }}"
+      login_password: "{{ zabbix_login_password }}"
+      alias: example2alias
+      name: example2
+      surname: testldap
+      usrgrps:
+        - Guests
+    register: create_zabbix_user_ldap_fail
+    ignore_errors: True
+
+  - assert:
+      that:
+        - create_zabbix_user_ldap_fail.failed is sameas True
+
+- when: zabbix_version is version('4.0', '>=')
+  block:
+  - name: test prepare - Create LDAP user group
+    zabbix_usergroup:
+      server_url: "{{ zabbix_server_url }}"
+      login_user: "{{ zabbix_login_user }}"
+      login_password: "{{ zabbix_login_password }}"
+      name: testLDAPgrp
+      gui_access: LDAP
+    register: zbxuser_create_ldap_group
+
+  - assert:
+      that:
+        - zbxuser_create_ldap_group.changed is sameas True
+
+  - name: test - Create new password-less user without LDAP group (fail)
+    zabbix_user:
+      server_url: "{{ zabbix_server_url }}"
+      login_user: "{{ zabbix_login_user }}"
+      login_password: "{{ zabbix_login_password }}"
+      alias: example2alias
+      name: example2
+      surname: testldap
+      usrgrps:
+        - Guests
+    register: create_zabbix_user_ldap_fail
+    ignore_errors: True
+
+  - assert:
+      that:
+        - create_zabbix_user_ldap_fail.failed is sameas True
+
+  - name: test - Create new password-less user as member in LDAP group
+    zabbix_user:
+      server_url: "{{ zabbix_server_url }}"
+      login_user: "{{ zabbix_login_user }}"
+      login_password: "{{ zabbix_login_password }}"
+      alias: example2alias
+      name: example2
+      surname: testldap
+      usrgrps:
+        - testLDAPgrp
+    register: create_zabbix_user_ldap_result
+
+  - assert:
+      that:
+        - create_zabbix_user_ldap_result.changed is sameas True
+
+  - name: test - Create new password-less user as member in LDAP group (again)
+    zabbix_user:
+      server_url: "{{ zabbix_server_url }}"
+      login_user: "{{ zabbix_login_user }}"
+      login_password: "{{ zabbix_login_password }}"
+      alias: example2alias
+      name: example2
+      surname: testldap
+      usrgrps:
+        - testLDAPgrp
+    register: create_zabbix_user_ldap_result
+
+  - assert:
+      that:
+        - create_zabbix_user_ldap_result.changed is sameas False
+
+  - name: test - Delete existing user
+    zabbix_user:
+      server_url: "{{ zabbix_server_url }}"
+      login_user: "{{ zabbix_login_user }}"
+      login_password: "{{ zabbix_login_password }}"
+      alias: example2alias
+      state: absent
+    register: delete_existing_user_result
+
+  - assert:
+      that:
+        - delete_existing_user_result.changed is sameas true