Fix: grant revoked priv (#434)

* Fix: exclude mysql 8 from test_mysql_user's 'Assert that priv did not change' test

* Add tests to verify that GRANT permission is present after user modification

* Fix: do not revoke GRANT permission when it's already allowed and present in priv parameter

* Deduplicate tests name

Easier to debug this way

* Fix assertions named 'GRANT permission is present'

* Only revoke grant option if it exists and absence is requested

* Fix assertion comments

* Fix: Only revoke grant option if it exists and absence is requested

* Avoid pointless revocations when ALL are granted

* Assert that priv did not change on mariadb also

* Fix: sanity and unity tests

* Format long lines

* Add changelog fragment

Co-authored-by: Laurent Indermühle <laurent.indermuehle@pm.me>

Author: rsicart

changelogs/fragments/434-do-not-revoke-grant-option-always.yaml

@@ -0,0 +1,5 @@
+---
+bugfixes:
+  - mysql_user - grant option was revoked accidentally when modifying users.
+    This fix revokes grant option only when privs are setup to do that
+    (https://github.com/ansible-collections/community.mysql/issues/77#issuecomment-1209693807).

plugins/module_utils/user.py

@@ -359,9 +359,20 @@ def user_mod(cursor, user, host, host_all, password, encrypted,
                     revoke_privs = list(set(new_priv[db_table]) & set(curr_priv[db_table]))
                 else:
                     # When replacing (neither append_privs nor subtract_privs), grant all missing privileges
-                    # and revoke existing privileges that were not requested.
+                    # and revoke existing privileges that were not requested...
                     grant_privs = list(set(new_priv[db_table]) - set(curr_priv[db_table]))
                     revoke_privs = list(set(curr_priv[db_table]) - set(new_priv[db_table]))
+
+                    # ... avoiding pointless revocations when ALL are granted
+                    if 'ALL' in grant_privs or 'ALL PRIVILEGES' in grant_privs:
+                        revoke_privs = list(set(['GRANT', 'PROXY']).intersection(set(revoke_privs)))
+
+                    # Only revoke grant option if it exists and absence is requested
+                    #
+                    # For more details
+                    # https://github.com/ansible-collections/community.mysql/issues/77#issuecomment-1209693807
+                    grant_option = 'GRANT' in revoke_privs and 'GRANT' not in grant_privs
+
                 if grant_privs == ['GRANT']:
                     # USAGE grants no privileges, it is only needed because 'WITH GRANT OPTION' cannot stand alone
                     grant_privs.append('USAGE')

tests/integration/targets/test_mysql_user/tasks/test_privs.yml

@@ -164,7 +164,7 @@
         that:
           - result is changed
 
-    - name: Test idempotency (expect ok)
+    - name: Test idempotency with a long privileges list (expect ok)
       mysql_user:
         <<: *mysql_params
         name: '{{ user_name_2 }}'
@@ -173,12 +173,75 @@
         state: present
       register: result
 
-    # FIXME: on mariadb >=10.5.2 there's always a change because the REPLICATION CLIENT privilege was renamed to BINLOG MONITOR
+    # FIXME: on mysql >=8 and mariadb >=10.5.2 there's always a change because
+    # the REPLICATION CLIENT privilege was renamed to BINLOG MONITOR
+    - name: Assert that priv did not change
+      assert:
+        that:
+          - result is not changed
+      when: (install_type == 'mysql' and mysql_version is version('8', '<')) or
+            (install_type == 'mariadb' and mariadb_version is version('10.5', '<'))
+
+    - name: remove username
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ user_name_2 }}'
+        password: '{{ user_password_2 }}'
+        state: absent
+
+    # ============================================================
+    - name: grant all privileges with grant option
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ user_name_2 }}'
+        password: '{{ user_password_2 }}'
+        priv: '*.*:ALL,GRANT'
+        state: present
+      register: result
+
+    - name: Assert that priv changed
+      assert:
+        that:
+          - result is changed
+
+    - name: Collect user info by host
+      community.mysql.mysql_info:
+        <<: *mysql_params
+        filter: "users"
+      register: mysql_info_about_users
+
+    - name: Assert that 'GRANT' permission is present
+      assert:
+        that:
+          - mysql_info_about_users.users.localhost.{{ user_name_2 }}.Grant_priv == 'Y'
+
+    - name: Test idempotency (expect ok)
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ user_name_2 }}'
+        password: '{{ user_password_2 }}'
+        priv: '*.*:ALL,GRANT'
+        state: present
+      register: result
+
+    # FIXME: on mysql >=8 there's always a change (ALL PRIVILEGES -> specific privileges)
     - name: Assert that priv did not change
       assert:
         that:
           - result is not changed
-      when: install_type == 'mysql' or (install_type == 'mariadb' and mariadb_version is version('10.2', '=='))
+      when: (install_type == 'mysql' and mysql_version is version('8', '<')) or
+            (install_type == 'mariadb')
+
+    - name: Collect user info by host
+      community.mysql.mysql_info:
+        <<: *mysql_params
+        filter: "users"
+      register: mysql_info_about_users
+
+    - name: Assert that 'GRANT' permission is present
+      assert:
+        that:
+          - mysql_info_about_users.users.localhost.{{ user_name_2 }}.Grant_priv == 'Y'
 
     # ============================================================
     - name: update user with invalid privileges