From b87b346a0a72c5173abbefdb6d8eab06b151acdf Mon Sep 17 00:00:00 2001
From: Rafael Guterres Jeffman <rjeffman@redhat.com>
Date: Wed, 27 Dec 2023 19:19:51 -0300
Subject: [PATCH] ipahbacrule: Fix handling of hbacsvcgroup in members

FreeIPA provides a default hbacsvcgroup named "Sudo", with capital 'S',
that is different from every other hbacsvcgroup, which are all
represented by lower case letters.

As data from IPA API was not modified, this causes an idempotence error
when using different capitalization with the 'hbacsvcgroup' parameter.

This patch fixes the issue by using the CaseInsensitive comparator to
create the hbacsvcgroup list.

Tests were update to make sure a regression is not included in the
future.
---
 plugins/modules/ipahbacrule.py                | 14 +++----
 .../test_hbacrule_member_case_insensitive.yml | 42 ++++++++++++++++++-
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/plugins/modules/ipahbacrule.py b/plugins/modules/ipahbacrule.py
index e81adae4..3c335267 100644
--- a/plugins/modules/ipahbacrule.py
+++ b/plugins/modules/ipahbacrule.py
@@ -188,13 +188,12 @@ def find_hbacrule(module, name):
     elif len(_result["result"]) == 1:
         res = _result["result"][0]
         # hbacsvcgroup names are converted to lower case while creation with
-        # hbacsvcgroup_add.
-        # The hbacsvcgroup for sudo is builtin with the name "Sudo" though.
-        # This breaks the lower case comparison. Therefore all
-        # memberservice_hbacsvcgroup items are converted to lower case if
-        # "Sudo" is in the list.
+        # hbacsvcgroup_add, but builtin names may have mixed case as "Sudo",
+        # breaking the lower case comparison. Therefore all
+        # memberservice_hbacsvcgroup items are converted to lower case.
+        # (See: https://pagure.io/freeipa/issue/9464).
         _member = "memberservice_hbacsvcgroup"
-        if _member in res and "Sudo" in res[_member]:
+        if _member in res:
             res[_member] = [item.lower() for item in res[_member]]
         return res
 
@@ -400,7 +399,8 @@ def main():
 
                     if hbacsvc is not None:
                         hbacsvc_add, hbacsvc_del = gen_add_del_lists(
-                            hbacsvc, res_find.get("memberservice_hbacsvc"))
+                            hbacsvc, res_find.get("memberservice_hbacsvc"),
+                        )
 
                     if hbacsvcgroup is not None:
                         hbacsvcgroup_add, hbacsvcgroup_del = gen_add_del_lists(
diff --git a/tests/hbacrule/test_hbacrule_member_case_insensitive.yml b/tests/hbacrule/test_hbacrule_member_case_insensitive.yml
index 59eaed79..83c22d6e 100644
--- a/tests/hbacrule/test_hbacrule_member_case_insensitive.yml
+++ b/tests/hbacrule/test_hbacrule_member_case_insensitive.yml
@@ -468,11 +468,51 @@
         register: result
         failed_when: result.changed or result.failed
 
+      # Specifically test 'Sudo', as FreeIPA adds a "Sudo" hbacsvcgroup instead of "sudo"
+      - name: Ensure 'sudo' works as hbacsvcgroup.
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: "test_sudo"
+          hbacsvcgroup:
+          - sudo
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Ensure 'sudo' works as hbacsvcgroup, again.
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: "test_sudo"
+          hbacsvcgroup:
+          - sudo
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure 'sudo' works as hbacsvcgroup, action member.
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: "test_sudo"
+          hbacsvcgroup:
+          - sudo
+          action: member
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Ensure 'Sudo' works as hbacsvcgroup, action member.
+        ipahbacrule:
+          ipaadmin_password: SomeADMINpassword
+          name: "test_sudo"
+          hbacsvcgroup:
+          - Sudo
+        register: result
+        failed_when: result.changed or result.failed
+
     always:
       - name: Ensure test hbacrule is absent
         ipahbacrule:
           ipaadmin_password: SomeADMINpassword
-          name: testrule
+          name:
+            - testrule
+            - test_sudo
           state: absent
 
       - name: Ensure test users are absent
-- 
GitLab