From 083396e133219b82b9f7a92af129fd1c54662770 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 11 Apr 2023 15:18:11 -0300 Subject: [PATCH 1/3] module_utils: Export Ansible's 'boolean' parsing function. Export Ansible's 'boolean' parsing function so it can be used to verify if a string can be handled as a truthy value, allowing module parameters to use strings instead of bools, as strings can be cleared by using empty strings. --- plugins/module_utils/ansible_freeipa_module.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/module_utils/ansible_freeipa_module.py b/plugins/module_utils/ansible_freeipa_module.py index 6bae7cc3..0af0d63e 100644 --- a/plugins/module_utils/ansible_freeipa_module.py +++ b/plugins/module_utils/ansible_freeipa_module.py @@ -30,7 +30,7 @@ __all__ = ["gssapi", "netaddr", "api", "ipalib_errors", "Env", "kinit_password", "kinit_keytab", "run", "DN", "VERSION", "paths", "tasks", "get_credentials_if_valid", "Encoding", "DNSName", "getargspec", "certificate_loader", - "write_certificate_list"] + "write_certificate_list", "boolean"] import os # ansible-freeipa requires locale to be C, IPA requires utf-8. @@ -49,6 +49,7 @@ from ansible.module_utils._text import to_text from ansible.module_utils.common.text.converters import jsonify from ansible.module_utils import six from ansible.module_utils.common._collections_compat import Mapping +from ansible.module_utils.parsing.convert_bool import boolean # Import getargspec from inspect or provide own getargspec for # Python 2 compatibility with Python 3.11+. From 694c717829236a5f43881ca93db85eb0c4933e47 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 11 Apr 2023 15:27:05 -0300 Subject: [PATCH 2/3] ipapwpolicy: Modify handling of usercheck and dictcheck Modified handling of boolean values by using Ansible's 'boolean()' check function so that a string can be used and either a bool value is accepted or an empty string. As the error message was changed to use the same Ansible message, tests were also updated. --- plugins/modules/ipapwpolicy.py | 17 ++++++----------- .../test_pwpolicy_invalid_data_type.yml | 4 ++-- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/plugins/modules/ipapwpolicy.py b/plugins/modules/ipapwpolicy.py index f7563553..4766a243 100644 --- a/plugins/modules/ipapwpolicy.py +++ b/plugins/modules/ipapwpolicy.py @@ -151,7 +151,7 @@ RETURN = """ """ from ansible.module_utils.ansible_freeipa_module import \ - IPAAnsibleModule, compare_args_ipa + IPAAnsibleModule, compare_args_ipa, boolean def find_pwpolicy(module, name): @@ -359,17 +359,12 @@ def main(): gracelimit = int_or_empty_param(gracelimit, "gracelimit") def bool_or_empty_param(value, param): # pylint: disable=R1710 - # As of Ansible 2.14, values True, False, Yes an No, with variable - # capitalization are accepted by Ansible. - if not value: + if value is None or value == "": return value - if value in ["TRUE", "True", "true", "YES", "Yes", "yes"]: - return True - if value in ["FALSE", "False", "false", "NO", "No", "no"]: - return False - ansible_module.fail_json( - msg="Invalid value '%s' for argument '%s'." % (value, param) - ) + try: + return boolean(value) + except TypeError as terr: + ansible_module.fail_json(msg="Param '%s': %s" % (param, str(terr))) dictcheck = bool_or_empty_param(dictcheck, "dictcheck") usercheck = bool_or_empty_param(usercheck, "usercheck") diff --git a/tests/pwpolicy/test_pwpolicy_invalid_data_type.yml b/tests/pwpolicy/test_pwpolicy_invalid_data_type.yml index 8a1aaed7..4c97622b 100644 --- a/tests/pwpolicy/test_pwpolicy_invalid_data_type.yml +++ b/tests/pwpolicy/test_pwpolicy_invalid_data_type.yml @@ -103,7 +103,7 @@ name: ops dictcheck: "error" register: result - failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'dictcheck" not in result.msg) + failed_when: result.changed or (result.failed and "is not a valid boolean" not in result.msg) when: ipa_version is version("4.9", ">=") - name: Ensure invalid values for usercheck raise proper error. @@ -113,7 +113,7 @@ name: ops usercheck: "error" register: result - failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'usercheck'" not in result.msg) + failed_when: result.changed or (result.failed and "is not a valid boolean" not in result.msg) when: ipa_version is version("4.9", ">=") - name: Ensure invalid values for gracelimit raise proper error. From 7b2701b985235796e7a8d2f94ba9d258a723cac2 Mon Sep 17 00:00:00 2001 From: Rafael Guterres Jeffman Date: Tue, 11 Apr 2023 15:22:30 -0300 Subject: [PATCH 3/3] ipapwpolicy: Updated module documentation. Most of ipapwpolicy parameters can be set to an empty string ("") so that the policy is not applied to pwpolicy. This was not refelected on the documentation. This change adds 'or ""' to all the fields that can be disabled by setting it to an empty string. Also, `data types were reviewed and fixed. --- README-pwpolicy.md | 28 ++++++++++++++-------------- plugins/modules/ipapwpolicy.py | 30 ++++++++++++++++-------------- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/README-pwpolicy.md b/README-pwpolicy.md index 7fd05164..8d158459 100644 --- a/README-pwpolicy.md +++ b/README-pwpolicy.md @@ -128,20 +128,20 @@ Variable | Description | Required `ipaapi_context` | The context in which the module will execute. Executing in a server context is preferred. If not provided context will be determined by the execution environment. Valid values are `server` and `client`. | no `ipaapi_ldap_cache` | Use LDAP cache for IPA connection. The bool setting defaults to yes. (bool) | no `name` \| `cn` | The list of pwpolicy name strings. If name is not given, `global_policy` will be used automatically. | no -`maxlife` \| `krbmaxpwdlife` | Maximum password lifetime in days. (int) | no -`minlife` \| `krbminpwdlife` | Minimum password lifetime in hours. (int) | no -`history` \| `krbpwdhistorylength` | Password history size. (int) | no -`minclasses` \| `krbpwdmindiffchars` | Minimum number of character classes. (int) | no -`minlength` \| `krbpwdminlength` | Minimum length of password. (int) | no -`priority` \| `cospriority` | Priority of the policy, higher number means lower priority. (int) | no -`maxfail` \| `krbpwdmaxfailure` | Consecutive failures before lockout. (int) | no -`failinterval` \| `krbpwdfailurecountinterval` | Period after which failure count will be reset in seconds. (int) | no -`lockouttime` \| `krbpwdlockoutduration` | Period for which lockout is enforced in seconds. (int) | no -`maxrepeat` \| `ipapwdmaxrepeat` | Maximum number of same consecutive characters. Requires IPA 4.9+ (int) | no -`maxsequence` \| `ipapwdmaxsequence` | The maximum length of monotonic character sequences (abcd). Requires IPA 4.9+ (int) | no -`dictcheck` \| `ipapwdictcheck` | Check if the password is a dictionary word. Requires IPA 4.9+ (int) | no -`usercheck` \| `ipapwdusercheck` | Check if the password contains the username. Requires IPA 4.9+ (int) | no -`gracelimit` \| `passwordgracelimit` | Number of LDAP authentications allowed after expiration. Requires IPA 4.9.10 (int) | no +`maxlife` \| `krbmaxpwdlife` | Maximum password lifetime in days. (int or "") | no +`minlife` \| `krbminpwdlife` | Minimum password lifetime in hours. (int or "") | no +`history` \| `krbpwdhistorylength` | Password history size. (int or "") | no +`minclasses` \| `krbpwdmindiffchars` | Minimum number of character classes. (int or "") | no +`minlength` \| `krbpwdminlength` | Minimum length of password. (int or "") | no +`priority` \| `cospriority` | Priority of the policy, higher number means lower priority. (int or "") | no +`maxfail` \| `krbpwdmaxfailure` | Consecutive failures before lockout. (int or "") | no +`failinterval` \| `krbpwdfailurecountinterval` | Period after which failure count will be reset in seconds. (int or "") | no +`lockouttime` \| `krbpwdlockoutduration` | Period for which lockout is enforced in seconds. (int or "") | no +`maxrepeat` \| `ipapwdmaxrepeat` | Maximum number of same consecutive characters. Requires IPA 4.9+ (int or "") | no +`maxsequence` \| `ipapwdmaxsequence` | The maximum length of monotonic character sequences (abcd). Requires IPA 4.9+ (int or "") | no +`dictcheck` \| `ipapwdictcheck` | Check if the password is a dictionary word. Requires IPA 4.9+. (bool or "") | no +`usercheck` \| `ipapwdusercheck` | Check if the password contains the username. Requires IPA 4.9+. (bool or "") | no +`gracelimit` \| `passwordgracelimit` | Number of LDAP authentications allowed after expiration. Requires IPA 4.9.10 (int or "") | no `state` | The state to ensure. It can be one of `present` or `absent`, default: `present`. | yes diff --git a/plugins/modules/ipapwpolicy.py b/plugins/modules/ipapwpolicy.py index 4766a243..a71a88dd 100644 --- a/plugins/modules/ipapwpolicy.py +++ b/plugins/modules/ipapwpolicy.py @@ -45,82 +45,84 @@ options: required: false aliases: ["cn"] maxlife: - description: Maximum password lifetime (in days) + description: Maximum password lifetime (in days). (int or "") type: str required: false aliases: ["krbmaxpwdlife"] minlife: - description: Minimum password lifetime (in hours) + description: Minimum password lifetime (in hours). (int or "") type: str required: false aliases: ["krbminpwdlife"] history: - description: Password history size + description: Password history size. (int or "") type: str required: false aliases: ["krbpwdhistorylength"] minclasses: - description: Minimum number of character classes + description: Minimum number of character classes. (int or "") type: str required: false aliases: ["krbpwdmindiffchars"] minlength: - description: Minimum length of password + description: Minimum length of password. (int or "") type: str required: false aliases: ["krbpwdminlength"] priority: - description: Priority of the policy (higher number means lower priority) + description: > + Priority of the policy (higher number means lower priority). (int or "") type: str required: false aliases: ["cospriority"] maxfail: - description: Consecutive failures before lockout + description: Consecutive failures before lockout. (int or "") type: str required: false aliases: ["krbpwdmaxfailure"] failinterval: - description: Period after which failure count will be reset (seconds) + description: > + Period after which failure count will be reset (seconds). (int or "") type: str required: false aliases: ["krbpwdfailurecountinterval"] lockouttime: - description: Period for which lockout is enforced (seconds) + description: Period for which lockout is enforced (seconds). (int or "") type: str required: false aliases: ["krbpwdlockoutduration"] maxrepeat: description: > Maximum number of same consecutive characters. - Requires IPA 4.9+ + Requires IPA 4.9+. (int or "") type: str required: false aliases: ["ipapwdmaxrepeat"] maxsequence: description: > The maximum length of monotonic character sequences (abcd). - Requires IPA 4.9+ + Requires IPA 4.9+. (int or "") type: str required: false aliases: ["ipapwdmaxsequence"] dictcheck: description: > Check if the password is a dictionary word. - Requires IPA 4.9+ + Requires IPA 4.9+. (bool or "") type: str required: false aliases: ["ipapwdictcheck"] usercheck: description: > Check if the password contains the username. - Requires IPA 4.9+ + Requires IPA 4.9+. (bool or "") type: str required: false aliases: ["ipapwdusercheck"] gracelimit: description: > Number of LDAP authentications allowed after expiration. - Requires IPA 4.10.1+ + Requires IPA 4.10.1+. (int or "") type: str required: false aliases: ["passwordgracelimit"]