From 2aaabc77c46432df380b1364af415f22e9a58398 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Fri, 9 Oct 2020 15:34:35 -0300 Subject: [PATCH 1/2] Add FreeIPA version check to module_utils.ansible_freeipa_module. Some attribute values are only accepted for specific FreeIPA versions, for example `self` for permission's `bindtype`. Although there are options to check for command and parameter availability, there is no check for verifying if a value should be accepted. This patch add a function to evaluate the target FreeIPA host version, by comparing a giver version to the current installed one. The version evaluation uses Python packaging's version comparision, which is compatible with PEP 440, if available. If not available, it falls back to a string split, that will work for the most common cases, but might fail for versions including strings with `rc` or `dev`, for example. --- .../module_utils/ansible_freeipa_module.py | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 43f96eb6..03cfe1f1 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -23,6 +23,7 @@ import sys +import operator import os import uuid import tempfile @@ -30,6 +31,25 @@ import shutil import gssapi from datetime import datetime from pprint import pformat + +try: + from packaging import version +except ImportError: + # If `packaging` not found, split version string for creating version + # object. Although it is not PEP 440 compliant, it will work for stable + # FreeIPA releases. + import re + + class version: + @staticmethod + def parse(version_str): + """ + Split a version string A.B.C, into a tuple. + + This will not work for `rc`, `dev` or similar version string. + """ + return tuple(re.split("[-_\.]", version_str)) # noqa: W605 + from ipalib import api from ipalib import errors as ipalib_errors # noqa from ipalib.config import Env @@ -41,6 +61,7 @@ except ImportError: from ipapython.ipautil import kinit_password, kinit_keytab from ipapython.ipautil import run from ipapython.dn import DN +from ipapython.version import VERSION from ipaplatform.paths import paths from ipalib.krb_utils import get_credentials_if_valid from ansible.module_utils.basic import AnsibleModule @@ -187,6 +208,26 @@ def api_check_param(command, name): return name in api.Command[command].params +def api_check_ipa_version(oper, requested_version): + """ + Compare the installed IPA version against a requested version. + + The valid operators are: <, <=, >, >=, ==, != + """ + oper_map = { + "<": operator.lt, + "<=": operator.le, + ">": operator.gt, + ">=": operator.ge, + "==": operator.eq, + "!=": operator.ne, + } + operation = oper_map.get(oper) + if not(operation): + raise NotImplementedError("Invalid operator: %s" % oper) + return operation(version.parse(VERSION), version.parse(requested_version)) + + def execute_api_command(module, principal, password, command, name, args): """ Execute an API command. From b6cf3e5f518235d685fc360c7bfe073d6fb8759c Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Fri, 13 Nov 2020 15:26:36 -0300 Subject: [PATCH 2/2] ipapermission: add version check for bind type 'self' FreeIPA 4.8.7 has introduced bind type 'self' as a valid value, and this PR adds checks so the module fails early if the value is used with an unsupported version. Tests and documentation have been updated to reflect the changes. --- README-permission.md | 2 +- plugins/modules/ipapermission.py | 7 ++++- tests/permission/test_permission.yml | 45 +++++++++++++++++++++------- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/README-permission.md b/README-permission.md index 5ab2af24..8cb14c38 100644 --- a/README-permission.md +++ b/README-permission.md @@ -141,7 +141,7 @@ Variable | Description | Required `name` \| `cn` | The permission name string. | yes `right` \| `ipapermright` | Rights to grant. It can be a list of one or more of `read`, `search`, `compare`, `write`, `add`, `delete`, and `all` default: `all` | no `attrs` | All attributes to which the permission applies | no -`bindtype` \| `ipapermbindruletype` | Bind rule type. It can be one of `permission`, `all`, `self`, or `anonymous` defaults to `permission` for new permissions.| no +`bindtype` \| `ipapermbindruletype` | Bind rule type. It can be one of `permission`, `all`, `self`, or `anonymous` defaults to `permission` for new permissions. Bind rule type `self` can only be used on IPA versions 4.8.7 or up.| no `subtree` \| `ipapermlocation` | Subtree to apply permissions to | no `filter` \| `extratargetfilter` | Extra target filter | no `rawfilter` \| `ipapermtargetfilter` | All target filters | no diff --git a/plugins/modules/ipapermission.py b/plugins/modules/ipapermission.py index 3f91af51..73825320 100644 --- a/plugins/modules/ipapermission.py +++ b/plugins/modules/ipapermission.py @@ -152,7 +152,8 @@ RETURN = """ from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.ansible_freeipa_module import \ temp_kinit, temp_kdestroy, valid_creds, api_connect, api_command, \ - compare_args_ipa, module_params_get, gen_add_del_lists + compare_args_ipa, module_params_get, gen_add_del_lists, \ + api_check_ipa_version import six if six.PY3: @@ -336,6 +337,10 @@ def main(): msg="Argument '%s' can not be used with action " "'%s' and state '%s'" % (x, action, state)) + if bindtype == "self" and api_check_ipa_version("<", "4.8.7"): + ansible_module.fail_json( + msg="Bindtype 'self' is not supported by your IPA version.") + # Init changed = False diff --git a/tests/permission/test_permission.yml b/tests/permission/test_permission.yml index eea112b4..08373abb 100644 --- a/tests/permission/test_permission.yml +++ b/tests/permission/test_permission.yml @@ -4,15 +4,19 @@ become: true tasks: + - include_tasks: ../env_freeipa_facts.yml # CLEANUP TEST ITEMS - name: Ensure permission perm-test-1 is absent ipapermission: ipaadmin_password: SomeADMINpassword - name: perm-test-1 + name: + - perm-test-1 + - perm-test-bindtype-test + - perm-test-renamed state: absent - + # TESTS - name: Ensure permission perm-test-1 is present @@ -38,7 +42,7 @@ ipaadmin_password: SomeADMINpassword name: perm-test-1 privilege: "User Administrators" - action: member + action: member register: result failed_when: not result.changed or result.failed @@ -89,7 +93,7 @@ state: absent register: result failed_when: result.changed or result.failed - + - name: Ensure permission perm-test-renamed is present ipapermission: ipaadmin_password: SomeADMINpassword @@ -99,16 +103,35 @@ register: result failed_when: result.changed or result.failed + - name: Ensure permission with bindtype 'self' is present, if IPA version >= 4.8.7 + ipapermission: + ipaadmin_password: SomeADMINpassword + name: perm-test-bindtype-test + bindtype: self + object_type: host + right: all + when: ipa_version is version('4.8.7', '>=') + register: result + failed_when: not result.changed or result.failed + + - name: Fail to set permission perm-test-renamed bindtype to 'self', if IPA version < 4.8.7 + ipapermission: + ipaadmin_password: SomeADMINpassword + name: perm-test-bindtype-test + bindtype: self + object_type: host + right: all + when: ipa_version is version('4.8.7', '<') + register: result + failed_when: not result.failed or "Bindtype 'self' is not supported by your IPA version." not in result.msg + # CLEANUP TEST ITEMS - name: Ensure permission perm-test-1 is absent ipapermission: ipaadmin_password: SomeADMINpassword - name: perm-test-1 - state: absent - - - name: Ensure permission perm-test-renamed is absent - ipapermission: - ipaadmin_password: SomeADMINpassword - name: perm-test-renamed + name: + - perm-test-1 + - perm-test-bindtype-test + - perm-test-renamed state: absent