ipaconfig: Do not require enable_sid for add_sids or netbios_name

Current behavior of ipaconfig mimics FreeIPA CLI and requires that
'enable_sid' is set to True every time add_sids or netbios_name are
used. It is sufficient that SID generation is enabled to use add_sids
and netbios_name, but the IPA API requires 'enable_sid' so that the
operations are executed.

This patch allows ansible-freeipa plugin ipaconfig to run 'add_sids' or
set 'netbios_name without requiring 'enable_sid' to be set on the
playbook.

If SID generation is enabled, 'add_sids' and 'netbios_name' can be used
without 'enable_sid: yes'. If SID generation is not enabled, an error
message will be raised if 'enable_sid: yes' is not used.

README-config.md

@@ -149,8 +149,8 @@ Variable | Description | Required
 `domain_resolution_order` \| `ipadomainresolutionorder` | Set list of domains used for short name qualification | no
 `ca_renewal_master_server` \| `ipacarenewalmasterserver`| Renewal master for IPA certificate authority. | no
 `enable_sid` | New users and groups automatically get a SID assigned. Cannot be deactivated once activated. Requires IPA 4.9.8+. (bool) | no
-`netbios_name` | NetBIOS name of the IPA domain. Requires IPA 4.9.8+ and 'enable_sid: yes'. | no
-`add_sids` | Add SIDs for existing users and groups. Requires IPA 4.9.8+ and 'enable_sid: yes'. (bool) | no
+`netbios_name` | NetBIOS name of the IPA domain. Requires IPA 4.9.8+ and SID generation to be activated. | no
+`add_sids` | Add SIDs for existing users and groups. Requires IPA 4.9.8+ and SID generation to be activated. (bool) | no
 
 
 Return Values

plugins/modules/ipaconfig.py

@@ -180,14 +180,14 @@ options:
         type: bool
     netbios_name:
         description: >
-          NetBIOS name of the IPA domain.
-          Requires IPA 4.9.8+ and 'enable_sid: yes'.
+          NetBIOS name of the IPA domain. Requires IPA 4.9.8+
+          and SID generation to be activated.
         required: false
         type: str
     add_sids:
         description: >
-          Add SIDs for existing users and groups.
-          Requires IPA 4.9.8+ and 'enable_sid: yes'.
+          Add SIDs for existing users and groups. Requires IPA 4.9.8+
+          and SID generation to be activated.
         required: false
         type: bool
 '''
@@ -362,7 +362,7 @@ def get_netbios_name(module):
 
 
 def is_enable_sid(module):
-    """When 'enable-sid' is true admin user and admins group have SID set."""
+    """When 'enable_sid' is true admin user and admins group have SID set."""
     _result = module.ipa_command("user_show", "admin", {"all": True})
     sid = _result["result"].get("ipantsecurityidentifier", [""])
     if not sid[0].endswith("-500"):
@@ -517,7 +517,7 @@ def main():
     changed = False
     exit_args = {}
 
-    # Connect to IPA API (enable-sid requires context == 'client')
+    # Connect to IPA API (enable_sid requires context == 'client')
     with ansible_module.ipa_connect(context="client"):
         has_enable_sid = ansible_module.ipa_command_param_exists(
             "config_mod", "enable_sid")
@@ -532,20 +532,8 @@ def main():
                 ansible_module.fail_json(msg="SID cannot be disabled.")
 
             netbios_name = params.get("netbios_name")
-            if netbios_name:
-                netbios_name = netbios_name.upper()
             add_sids = params.get("add_sids")
-            required_sid = any([netbios_name, add_sids])
-            if required_sid and not enable_sid:
-                ansible_module.fail_json(
-                    msg="'enable-sid: yes' required for 'netbios_name' "
-                        "and 'add-sids'."
-                )
-            if enable_sid:
-                if not has_enable_sid:
-                    ansible_module.fail_json(
-                        msg="This version of IPA does not support enable-sid."
-                    )
+            if has_enable_sid:
                 if (
                     netbios_name
                     and netbios_name == get_netbios_name(ansible_module)
@@ -554,12 +542,27 @@ def main():
                     netbios_name = None
                 if not add_sids and "add_sids" in params:
                     del params["add_sids"]
-                if (
-                    not any([netbios_name, add_sids])
-                    and sid_is_enabled
-                ):
-                    del params["enable_sid"]
+                if any([netbios_name, add_sids]):
+                    if sid_is_enabled:
+                        params["enable_sid"] = True
+                    else:
+                        if not enable_sid:
+                            ansible_module.fail_json(
+                                msg="SID generation must be enabled for "
+                                    "'netbios_name' and 'add_sids'. Use "
+                                    "'enable_sid: yes'."
+                            )
+                else:
+                    if sid_is_enabled and "enable_sid" in params:
+                        del params["enable_sid"]
 
+            else:
+                if any([enable_sid, netbios_name, add_sids is not None]):
+                    ansible_module.fail_json(
+                        msg="This version of IPA does not support enable_sid, "
+                            "add_sids or netbios_name setting through the "
+                            "config module"
+                    )
             params = {
                 k: v for k, v in params.items()
                 if k not in result or result[k] != v

tests/config/test_config_sid.yml

@@ -19,6 +19,32 @@
 
   # TESTS
   - block:
+    - name: Check if SID is enabled.
+      ipaconfig:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        enable_sid: yes
+      check_mode: yes
+      register: sid_disabled
+
+    - name: Ensure netbios_name can't be changed without SID enabled.
+      ipaconfig:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        netbios_name: IPATESTPLAY
+      register: result
+      failed_when: not result.failed and "SID generation must be enabled" in result.msg
+      when: sid_disabled.changed
+
+    - name: Ensure SIDs can't be changed without SID enabled.
+      ipaconfig:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        add_sids: yes
+      register: result
+      failed_when: not result.failed and "SID generation must be enabled" in result.msg
+      when: sid_disabled.changed
+
     - name: Ensure SID is enabled.
       ipaconfig:
         ipaadmin_password: SomeADMINpassword
@@ -56,18 +82,36 @@
       ipaconfig:
         ipaadmin_password: SomeADMINpassword
         ipaapi_context: "{{ ipa_context | default(omit) }}"
-        enable_sid: yes
         netbios_name: IPATESTPLAY
       register: result
       failed_when: result.failed or result.changed
 
+    - name: Ensure netbios_name cannot be set with lowercase characters
+      ipaconfig:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        netbios_name: IPATESTplay
+      register: result
+      failed_when:
+        (not result.failed
+         and "Up to 15 characters and only uppercase ASCII letters, digits and dashes are allowed" not in result.message)
+
+    - name: Ensure netbios_name cannot be set different lowercase characters
+      ipaconfig:
+        ipaadmin_password: SomeADMINpassword
+        ipaapi_context: "{{ ipa_context | default(omit) }}"
+        netbios_name: otherPLAY
+      register: result
+      failed_when:
+        (not result.failed
+         and "Up to 15 characters and only uppercase ASCII letters, digits and dashes are allowed" not in result.message)
+
     # add_sids is not idempotent as it always tries to generate the missing
     # SIDs for users and groups.
     - name: Add SIDs to users and groups.
       ipaconfig:
         ipaadmin_password: SomeADMINpassword
         ipaapi_context: "{{ ipa_context | default(omit) }}"
-        enable_sid: yes
         add_sids: yes
 
     # only run tests if version supports enable-sid