From 410682a01dcdde95e1e69959331017c5b5926bee Mon Sep 17 00:00:00 2001
From: Rafael Guterres Jeffman <rjeffman@redhat.com>
Date: Wed, 4 Jan 2023 18:58:27 -0300
Subject: [PATCH] pwpolicy: Allow clearing policy values.
All values for pwpolicy can be cleared with an empty string in IPA CLI,
and this behavior was missing in ansible-freeipa.
As of today, there is an issue in FreeIPA that does not allow clearing
'minlength' policy. The is is tracked by the FreeIPA project through
https://pagure.io/freeipa/issue/9297
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2150334
---
plugins/modules/ipapwpolicy.py | 99 +++++++++----
tests/pwpolicy/test_pwpolicy.yml | 112 ++++++++++++++-
.../test_pwpolicy_invalid_data_type.yml | 134 ++++++++++++++++++
3 files changed, 313 insertions(+), 32 deletions(-)
create mode 100644 tests/pwpolicy/test_pwpolicy_invalid_data_type.yml
diff --git a/plugins/modules/ipapwpolicy.py b/plugins/modules/ipapwpolicy.py
index 4398c4e8..2dfc726b 100644
--- a/plugins/modules/ipapwpolicy.py
+++ b/plugins/modules/ipapwpolicy.py
@@ -46,82 +46,82 @@ options:
aliases: ["cn"]
maxlife:
description: Maximum password lifetime (in days)
- type: int
+ type: str
required: false
aliases: ["krbmaxpwdlife"]
minlife:
description: Minimum password lifetime (in hours)
- type: int
+ type: str
required: false
aliases: ["krbminpwdlife"]
history:
description: Password history size
- type: int
+ type: str
required: false
aliases: ["krbpwdhistorylength"]
minclasses:
description: Minimum number of character classes
- type: int
+ type: str
required: false
aliases: ["krbpwdmindiffchars"]
minlength:
description: Minimum length of password
- type: int
+ type: str
required: false
aliases: ["krbpwdminlength"]
priority:
description: Priority of the policy (higher number means lower priority)
- type: int
+ type: str
required: false
aliases: ["cospriority"]
maxfail:
description: Consecutive failures before lockout
- type: int
+ type: str
required: false
aliases: ["krbpwdmaxfailure"]
failinterval:
description: Period after which failure count will be reset (seconds)
- type: int
+ type: str
required: false
aliases: ["krbpwdfailurecountinterval"]
lockouttime:
description: Period for which lockout is enforced (seconds)
- type: int
+ type: str
required: false
aliases: ["krbpwdlockoutduration"]
maxrepeat:
description: >
Maximum number of same consecutive characters.
Requires IPA 4.9+
- type: int
+ type: str
required: false
aliases: ["ipapwdmaxrepeat"]
maxsequence:
description: >
The maximum length of monotonic character sequences (abcd).
Requires IPA 4.9+
- type: int
+ type: str
required: false
aliases: ["ipapwdmaxsequence"]
dictcheck:
description: >
Check if the password is a dictionary word.
Requires IPA 4.9+
- type: bool
+ type: str
required: false
aliases: ["ipapwdictcheck"]
usercheck:
description: >
Check if the password contains the username.
Requires IPA 4.9+
- type: bool
+ type: str
required: false
aliases: ["ipapwdusercheck"]
gracelimit:
description: >
Number of LDAP authentications allowed after expiration.
Requires IPA 4.10.1+
- type: int
+ type: str
required: false
aliases: ["passwordgracelimit"]
state:
@@ -242,31 +242,31 @@ def main():
default=None, required=False),
# present
- maxlife=dict(type="int", aliases=["krbmaxpwdlife"], default=None),
- minlife=dict(type="int", aliases=["krbminpwdlife"], default=None),
- history=dict(type="int", aliases=["krbpwdhistorylength"],
+ maxlife=dict(type="str", aliases=["krbmaxpwdlife"], default=None),
+ minlife=dict(type="str", aliases=["krbminpwdlife"], default=None),
+ history=dict(type="str", aliases=["krbpwdhistorylength"],
default=None),
- minclasses=dict(type="int", aliases=["krbpwdmindiffchars"],
+ minclasses=dict(type="str", aliases=["krbpwdmindiffchars"],
default=None),
- minlength=dict(type="int", aliases=["krbpwdminlength"],
+ minlength=dict(type="str", aliases=["krbpwdminlength"],
default=None),
- priority=dict(type="int", aliases=["cospriority"], default=None),
- maxfail=dict(type="int", aliases=["krbpwdmaxfailure"],
+ priority=dict(type="str", aliases=["cospriority"], default=None),
+ maxfail=dict(type="str", aliases=["krbpwdmaxfailure"],
default=None),
- failinterval=dict(type="int",
+ failinterval=dict(type="str",
aliases=["krbpwdfailurecountinterval"],
default=None),
- lockouttime=dict(type="int", aliases=["krbpwdlockoutduration"],
+ lockouttime=dict(type="str", aliases=["krbpwdlockoutduration"],
default=None),
- maxrepeat=dict(type="int", aliases=["ipapwdmaxrepeat"],
+ maxrepeat=dict(type="str", aliases=["ipapwdmaxrepeat"],
default=None),
- maxsequence=dict(type="int", aliases=["ipapwdmaxsequence"],
+ maxsequence=dict(type="str", aliases=["ipapwdmaxsequence"],
default=None),
- dictcheck=dict(type="bool", aliases=["ipapwdictcheck"],
+ dictcheck=dict(type="str", aliases=["ipapwdictcheck"],
default=None),
- usercheck=dict(type="bool", aliases=["ipapwusercheck"],
+ usercheck=dict(type="str", aliases=["ipapwusercheck"],
default=None),
- gracelimit=dict(type="int", aliases=["passwordgracelimit"],
+ gracelimit=dict(type="str", aliases=["passwordgracelimit"],
default=None),
# state
state=dict(type="str", default="present",
@@ -325,7 +325,48 @@ def main():
ansible_module.params_fail_used_invalid(invalid, state)
- if gracelimit is not None:
+ # Ensure parameter values are valid and have proper type.
+ def int_or_empty_param(value, param):
+ if value is not None and value != "":
+ try:
+ value = int(value)
+ except ValueError:
+ ansible_module.fail_json(
+ msg="Invalid value '%s' for argument '%s'" % (value, param)
+ )
+ return value
+
+ maxlife = int_or_empty_param(maxlife, "maxlife")
+ minlife = int_or_empty_param(minlife, "minlife")
+ history = int_or_empty_param(history, "history")
+ minclasses = int_or_empty_param(minclasses, "minclasses")
+ minlength = int_or_empty_param(minlength, "minlength")
+ priority = int_or_empty_param(priority, "priority")
+ maxfail = int_or_empty_param(maxfail, "maxfail")
+ failinterval = int_or_empty_param(failinterval, "failinterval")
+ lockouttime = int_or_empty_param(lockouttime, "lockouttime")
+ maxrepeat = int_or_empty_param(maxrepeat, "maxrepeat")
+ maxsequence = int_or_empty_param(maxsequence, "maxsequence")
+ gracelimit = int_or_empty_param(gracelimit, "gracelimit")
+
+ def bool_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:
+ 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)
+ )
+
+ dictcheck = bool_param(dictcheck, "dictcheck")
+ usercheck = bool_param(usercheck, "usercheck")
+
+ # Ensure gracelimit has proper limit.
+ if gracelimit:
if gracelimit < -1:
ansible_module.fail_json(
msg="'gracelimit' must be no less than -1")
diff --git a/tests/pwpolicy/test_pwpolicy.yml b/tests/pwpolicy/test_pwpolicy.yml
index c4a480cd..fc02914b 100644
--- a/tests/pwpolicy/test_pwpolicy.yml
+++ b/tests/pwpolicy/test_pwpolicy.yml
@@ -121,7 +121,75 @@
register: result
failed_when: result.changed or result.failed
+ - name: Ensure presence of pwpolicies for group ops
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ minlife: 7
+ maxlife: 49
+ history: 5
+ priority: 1
+ lockouttime: 300
+ minlength: 8
+ minclasses: 5
+ maxfail: 3
+ failinterval: 5
+
+ - name: Ensure policies are cleared
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ minlife: ""
+ maxlife: ""
+ history: ""
+ # priority: ""
+ lockouttime: ""
+ minclasses: ""
+ maxfail: ""
+ failinterval: ""
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Ensure policies are cleared, again
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ minlife: ""
+ maxlife: ""
+ history: ""
+ # priority: ""
+ lockouttime: ""
+ minclasses: ""
+ maxfail: ""
+ failinterval: ""
+ register: result
+ failed_when: result.changed or result.failed
+
+ - name: Ensure minlength is not cleared due to FreeIPA issue
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ minlength: ""
+ register: result
+ failed_when: result.changed or (result.failed and "int() argument must be a string, a bytes-like object" not in result.msg)
+ when: ipa_version is version("4.9", ">=")
+
+ - name: Ensure minlength is not cleared due to FreeIPA issue
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ minlength: ""
+ register: result
+ failed_when: not result.changed or result.failed
+ when: ipa_version is version("4.7", "<")
+
- name: Execute tests if ipa_version >= 4.9.0
+ when: ipa_version is version("4.9", ">=")
block:
- name: Ensure maxrepeat of 2 for global_policy
ipapwpolicy:
@@ -171,6 +239,13 @@
register: result
failed_when: not result.changed or result.failed
+ - name: Ensure usercheck and dictcheck have known values
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ dictcheck: false
+ usercheck: false
+
- name: Ensure dictcheck is set for global_policy
ipapwpolicy:
ipaadmin_password: SomeADMINpassword
@@ -219,9 +294,26 @@
register: result
failed_when: not result.changed or result.failed
- when: ipa_version is version("4.9", ">=")
+ - name: Ensure usercheck and dictcheck are cleared for global_policy
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ dictcheck: ""
+ usercheck: ""
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Ensure usercheck and dictcheck are cleared for global_policy, again
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ dictcheck: ""
+ usercheck: ""
+ register: result
+ failed_when: result.changed or result.failed
- name: Execute tests if ipa_version >= 4.9.10
+ when: ipa_version is version("4.9.10", ">=")
block:
- name: Ensure grace limit is set to 10 for global_policy
ipapwpolicy:
@@ -255,6 +347,22 @@
register: result
failed_when: not result.changed or result.failed
+ - name: Ensure grace limit is cleared for global_policy
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ gracelimit: ""
+ register: result
+ failed_when: not result.changed or result.failed
+
+ - name: Ensure grace limit is cleared for global_policy, again
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ gracelimit: ""
+ register: result
+ failed_when: result.changed or result.failed
+
- name: Ensure grace limit is not set to -2 for global_policy
ipapwpolicy:
ipaadmin_password: SomeADMINpassword
@@ -262,5 +370,3 @@
gracelimit: -2
register: result
failed_when: not result.failed and "must be at least -1" not in result.msg
-
- when: ipa_version is version("4.9.10", ">=")
diff --git a/tests/pwpolicy/test_pwpolicy_invalid_data_type.yml b/tests/pwpolicy/test_pwpolicy_invalid_data_type.yml
new file mode 100644
index 00000000..8a1aaed7
--- /dev/null
+++ b/tests/pwpolicy/test_pwpolicy_invalid_data_type.yml
@@ -0,0 +1,134 @@
+---
+- name: Test pwpolicy invalid data types
+ hosts: "{{ ipa_test_host | default('ipaserver') }}"
+ become: true
+ gather_facts: false
+
+ tasks:
+ - name: Setup FreeIPA test facts.
+ ansible.builtin.import_tasks: ../env_freeipa_facts.yml
+
+ - name: Ensure presence of group ops
+ ipagroup:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ state: present
+
+ - name: Ensure invalid values raise proper error for argument minlife
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ minlife: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'minlife'" not in result.msg)
+
+ - name: Ensure invalid values raise proper error for argument maxlife
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ maxlife: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'maxlife'" not in result.msg)
+
+ - name: Ensure invalid values raise proper error for argument history
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ history: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'history'" not in result.msg)
+
+ - name: Ensure invalid values raise proper error for argument priority
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ priority: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'priority'" not in result.msg)
+
+ - name: Ensure invalid values raise proper error for argument lockouttime
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ lockouttime: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'lockouttime'" not in result.msg)
+
+ - name: Ensure invalid values raise proper error for argument minlength
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ minlength: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'minlength'" not in result.msg)
+
+ - name: Ensure invalid values raise proper error for argument minclasses
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ minclasses: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'minclasses'" not in result.msg)
+
+ - name: Ensure invalid values raise proper error for argument maxfail
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ maxfail: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'maxfail'" not in result.msg)
+
+ - name: Ensure invalid values raise proper error for argument failinterval
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ failinterval: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'failinterval'" not in result.msg)
+
+ - name: Ensure invalid values for dictcheck raise proper error.
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ dictcheck: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'dictcheck" not in result.msg)
+ when: ipa_version is version("4.9", ">=")
+
+ - name: Ensure invalid values for usercheck raise proper error.
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ usercheck: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'usercheck'" not in result.msg)
+ when: ipa_version is version("4.9", ">=")
+
+ - name: Ensure invalid values for gracelimit raise proper error.
+ ipapwpolicy:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ gracelimit: "error"
+ register: result
+ failed_when: result.changed or (result.failed and "Invalid value 'error' for argument 'gracelimit'" not in result.msg)
+ when: ipa_version is version("4.9.10", ">=")
+
+ - name: Ensure absence of group ops
+ ipagroup:
+ ipaadmin_password: SomeADMINpassword
+ ipaapi_context: "{{ ipa_context | default(omit) }}"
+ name: ops
+ state: absent
--
GitLab