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

@@ -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),
+            maxlife=dict(type="str", aliases=["krbmaxpwdlife"], default=None),
-            minlife=dict(type="int", aliases=["krbminpwdlife"], default=None),
+            minlife=dict(type="str", aliases=["krbminpwdlife"], default=None),
-            history=dict(type="int", aliases=["krbpwdhistorylength"],
+            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),
+            priority=dict(type="str", aliases=["cospriority"], default=None),
-            maxfail=dict(type="int", aliases=["krbpwdmaxfailure"],
+            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")

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", ">=")

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