mysql_user, mysql_role: add argument subtract_privs to revoke privileges explicitly

changelogs/fragments/333-mysql_user-mysql_role-add-subtract_privileges-argument.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - "mysql_user and mysql_role: Add the argument ``subtract_privs`` (boolean, default false, mutually exclusive with ``append_privs``). If set, the privileges given in ``priv`` are revoked and existing privileges are kept. (https://github.com/ansible-collections/community.mysql/pull/333)"

plugins/module_utils/user.py

@@ -169,7 +169,7 @@ def is_hash(password):
 
 def user_mod(cursor, user, host, host_all, password, encrypted,
              plugin, plugin_hash_string, plugin_auth_string, new_priv,
-             append_privs, tls_requires, module, role=False, maria_role=False):
+             append_privs, subtract_privs, tls_requires, module, role=False, maria_role=False):
     changed = False
     msg = "User unchanged"
     grant_option = False
@@ -288,47 +288,58 @@ def user_mod(cursor, user, host, host_all, password, encrypted,
 
             # If the user has privileges on a db.table that doesn't appear at all in
             # the new specification, then revoke all privileges on it.
-            for db_table, priv in iteritems(curr_priv):
-                # If the user has the GRANT OPTION on a db.table, revoke it first.
-                if "GRANT" in priv:
-                    grant_option = True
-                if db_table not in new_priv:
-                    if user != "root" and "PROXY" not in priv and not append_privs:
-                        msg = "Privileges updated"
-                        if module.check_mode:
-                            return (True, msg)
-                        privileges_revoke(cursor, user, host, db_table, priv, grant_option, maria_role)
-                        changed = True
+            if not append_privs and not subtract_privs:
+                for db_table, priv in iteritems(curr_priv):
+                    # If the user has the GRANT OPTION on a db.table, revoke it first.
+                    if "GRANT" in priv:
+                        grant_option = True
+                    if db_table not in new_priv:
+                        if user != "root" and "PROXY" not in priv:
+                            msg = "Privileges updated"
+                            if module.check_mode:
+                                return (True, msg)
+                            privileges_revoke(cursor, user, host, db_table, priv, grant_option, maria_role)
+                            changed = True
 
             # If the user doesn't currently have any privileges on a db.table, then
             # we can perform a straight grant operation.
-            for db_table, priv in iteritems(new_priv):
-                if db_table not in curr_priv:
-                    msg = "New privileges granted"
-                    if module.check_mode:
-                        return (True, msg)
-                    privileges_grant(cursor, user, host, db_table, priv, tls_requires, maria_role)
-                    changed = True
+            if not subtract_privs:
+                for db_table, priv in iteritems(new_priv):
+                    if db_table not in curr_priv:
+                        msg = "New privileges granted"
+                        if module.check_mode:
+                            return (True, msg)
+                        privileges_grant(cursor, user, host, db_table, priv, tls_requires, maria_role)
+                        changed = True
 
             # If the db.table specification exists in both the user's current privileges
             # and in the new privileges, then we need to see if there's a difference.
             db_table_intersect = set(new_priv.keys()) & set(curr_priv.keys())
             for db_table in db_table_intersect:
 
-                # If appending privileges, only the set difference between new privileges and current privileges matter.
-                # The symmetric difference isn't relevant for append because existing privileges will not be revoked.
+                grant_privs = []
+                revoke_privs = []
                 if append_privs:
-                    priv_diff = set(new_priv[db_table]) - set(curr_priv[db_table])
+                    # When appending privileges, only missing privileges need to be granted. Nothing is revoked.
+                    grant_privs = list(set(new_priv[db_table]) - set(curr_priv[db_table]))
+                elif subtract_privs:
+                    # When subtracting privileges, revoke only the intersection of requested and current privileges.
+                    # No privileges are granted.
+                    revoke_privs = list(set(new_priv[db_table]) & set(curr_priv[db_table]))
                 else:
-                    priv_diff = set(new_priv[db_table]) ^ set(curr_priv[db_table])
+                    # When replacing (neither append_privs nor subtract_privs), grant all missing privileges
+                    # 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]))
 
-                if len(priv_diff) > 0:
+                if len(grant_privs) + len(revoke_privs) > 0:
                     msg = "Privileges updated"
                     if module.check_mode:
                         return (True, msg)
-                    if not append_privs:
-                        privileges_revoke(cursor, user, host, db_table, curr_priv[db_table], grant_option, maria_role)
-                    privileges_grant(cursor, user, host, db_table, new_priv[db_table], tls_requires, maria_role)
+                    if len(revoke_privs) > 0:
+                        privileges_revoke(cursor, user, host, db_table, revoke_privs, grant_option, maria_role)
+                    if len(grant_privs) > 0:
+                        privileges_grant(cursor, user, host, db_table, grant_privs, tls_requires, maria_role)
                     changed = True
 
         if role:
@@ -549,7 +560,7 @@ def sort_column_order(statement):
     return '%s(%s)' % (priv_name, ', '.join(columns))
 
 
-def privileges_unpack(priv, mode):
+def privileges_unpack(priv, mode, ensure_usage=True):
     """ Take a privileges string, typically passed as a parameter, and unserialize
     it into a dictionary, the same format as privileges_get() above. We have this
     custom format to avoid using YAML/JSON strings inside YAML playbooks. Example
@@ -595,7 +606,7 @@ def privileges_unpack(priv, mode):
         # Handle cases when there's privs like GRANT SELECT (colA, ...) in privs.
         output[pieces[0]] = normalize_col_grants(output[pieces[0]])
 
-    if '*.*' not in output:
+    if ensure_usage and '*.*' not in output:
         output['*.*'] = ['USAGE']
 
     return output

plugins/modules/mysql_role.py

@@ -51,7 +51,15 @@
   append_privs:
     description:
       - Append the privileges defined by the I(priv) option to the existing ones
-        for this role instead of overwriting them.
+        for this role instead of overwriting them. Mutually exclusive with I(subtract_privs).
+    type: bool
+    default: no
+
+  subtract_privs:
+    description:
+      - Revoke the privileges defined by the I(priv) option and keep other existing privileges.
+        If set, invalid privileges in I(priv) are ignored.
+        Mutually exclusive with I(append_privs).
     type: bool
     default: no
 
@@ -233,6 +241,14 @@
     name: business
     members:
     - marketing
+
+- name: Ensure the role foo does not have the DELETE privilege
+  community.mysql.mysql_role:
+    state: present
+    name: foo
+    subtract_privs: yes
+    priv:
+      'db1.*': DELETE
 '''
 
 RETURN = '''#'''
@@ -821,9 +837,9 @@ def __remove_member(self, user, check_mode=False):
         return True
 
     def update(self, users, privs, check_mode=False,
-               append_privs=False, append_members=False,
-               detach_members=False, admin=False,
-               set_default_role_all=True):
+               append_privs=False, subtract_privs=False,
+               append_members=False, detach_members=False,
+               admin=False, set_default_role_all=True):
         """Update a role.
 
         Update a role if needed.
@@ -837,6 +853,8 @@ def update(self, users, privs, check_mode=False,
             check_mode (bool): If True, just checks and does nothing.
             append_privs (bool): If True, adds new privileges passed through privs
                 not touching current privileges.
+            subtract_privs (bool): If True, revoke the privileges passed through privs
+                not touching other existing privileges.
             append_members (bool): If True, adds new members passed through users
                 not touching current members.
             detach_members (bool): If True, removes members passed through users from a role.
@@ -861,7 +879,7 @@ def update(self, users, privs, check_mode=False,
         if privs:
             changed, msg = user_mod(self.cursor, self.name, self.host,
                                     None, None, None, None, None, None,
-                                    privs, append_privs, None,
+                                    privs, append_privs, subtract_privs, None,
                                     self.module, role=True, maria_role=self.is_mariadb)
 
         if admin:
@@ -931,6 +949,7 @@ def main():
         admin=dict(type='str'),
         priv=dict(type='raw'),
         append_privs=dict(type='bool', default=False),
+        subtract_privs=dict(type='bool', default=False),
         members=dict(type='list', elements='str'),
         append_members=dict(type='bool', default=False),
         detach_members=dict(type='bool', default=False),
@@ -945,6 +964,7 @@ def main():
             ('admin', 'members'),
             ('admin', 'append_members'),
             ('admin', 'detach_members'),
+            ('append_privs', 'subtract_privs'),
         ),
     )
 
@@ -958,6 +978,7 @@ def main():
     connect_timeout = module.params['connect_timeout']
     config_file = module.params['config_file']
     append_privs = module.params['append_privs']
+    subtract_privs = module.boolean(module.params['subtract_privs'])
     members = module.params['members']
     append_members = module.params['append_members']
     detach_members = module.params['detach_members']
@@ -1014,7 +1035,7 @@ def main():
             module.fail_json(msg=to_native(e))
 
         try:
-            priv = privileges_unpack(priv, mode)
+            priv = privileges_unpack(priv, mode, ensure_usage=not subtract_privs)
         except Exception as e:
             module.fail_json(msg='Invalid privileges string: %s' % to_native(e))
 
@@ -1043,11 +1064,13 @@ def main():
     try:
         if state == 'present':
             if not role.exists:
+                if subtract_privs:
+                    priv = None  # avoid granting unwanted privileges
                 changed = role.add(members, priv, module.check_mode, admin,
                                    set_default_role_all)
 
             else:
-                changed = role.update(members, priv, module.check_mode, append_privs,
+                changed = role.update(members, priv, module.check_mode, append_privs, subtract_privs,
                                       append_members, detach_members, admin,
                                       set_default_role_all)
 

plugins/modules/mysql_user.py

@@ -63,7 +63,14 @@
   append_privs:
     description:
       - Append the privileges defined by priv to the existing ones for this
-        user instead of overwriting existing ones.
+        user instead of overwriting existing ones. Mutually exclusive with I(subtract_privs).
+    type: bool
+    default: no
+  subtract_privs:
+    description:
+      - Revoke the privileges defined by the I(priv) option and keep other existing privileges.
+        If set, invalid privileges in I(priv) are ignored.
+        Mutually exclusive with I(append_privs).
     type: bool
     default: no
   tls_requires:
@@ -306,6 +313,13 @@
       MAX_QUERIES_PER_HOUR: 10
       MAX_CONNECTIONS_PER_HOUR: 5
 
+- name: Ensure bob does not have the DELETE privilege
+  community.mysql.mysql_user:
+    name: bob
+    subtract_privs: yes
+    priv:
+      'db1.*': DELETE
+
 # Example .my.cnf file for setting the root password
 # [client]
 # user=root
@@ -352,6 +366,7 @@ def main():
         priv=dict(type='raw'),
         tls_requires=dict(type='dict'),
         append_privs=dict(type='bool', default=False),
+        subtract_privs=dict(type='bool', default=False),
         check_implicit_admin=dict(type='bool', default=False),
         update_password=dict(type='str', default='always', choices=['always', 'on_create'], no_log=False),
         sql_log_bin=dict(type='bool', default=True),
@@ -364,6 +379,7 @@ def main():
     module = AnsibleModule(
         argument_spec=argument_spec,
         supports_check_mode=True,
+        mutually_exclusive=(('append_privs', 'subtract_privs'),)
     )
     login_user = module.params["login_user"]
     login_password = module.params["login_password"]
@@ -379,6 +395,7 @@ def main():
     connect_timeout = module.params["connect_timeout"]
     config_file = module.params["config_file"]
     append_privs = module.boolean(module.params["append_privs"])
+    subtract_privs = module.boolean(module.params['subtract_privs'])
     update_password = module.params['update_password']
     ssl_cert = module.params["client_cert"]
     ssl_key = module.params["client_key"]
@@ -427,26 +444,28 @@ def main():
             mode = get_mode(cursor)
         except Exception as e:
             module.fail_json(msg=to_native(e))
-        priv = privileges_unpack(priv, mode)
+        priv = privileges_unpack(priv, mode, ensure_usage=not subtract_privs)
 
     if state == "present":
         if user_exists(cursor, user, host, host_all):
             try:
                 if update_password == "always":
                     changed, msg = user_mod(cursor, user, host, host_all, password, encrypted,
                                             plugin, plugin_hash_string, plugin_auth_string,
-                                            priv, append_privs, tls_requires, module)
+                                            priv, append_privs, subtract_privs, tls_requires, module)
                 else:
                     changed, msg = user_mod(cursor, user, host, host_all, None, encrypted,
                                             plugin, plugin_hash_string, plugin_auth_string,
-                                            priv, append_privs, tls_requires, module)
+                                            priv, append_privs, subtract_privs, tls_requires, module)
 
             except (SQLParseError, InvalidPrivsError, mysql_driver.Error) as e:
                 module.fail_json(msg=to_native(e))
         else:
             if host_all:
                 module.fail_json(msg="host_all parameter cannot be used when adding a user")
             try:
+                if subtract_privs:
+                    priv = None  # avoid granting unwanted privileges
                 changed = user_add(cursor, user, host, host_all, password, encrypted,
                                    plugin, plugin_hash_string, plugin_auth_string,
                                    priv, tls_requires, module.check_mode)

tests/integration/targets/test_mysql_role/defaults/main.yml

@@ -14,3 +14,4 @@ nonexistent: user3
 
 role0: role0
 role1: role1
+role2: role2

tests/integration/targets/test_mysql_role/tasks/main.yml

@@ -3,5 +3,15 @@
 # and should not be used as examples of how to write Ansible roles #
 ####################################################################
 
+- name: alias mysql command to include default options
+  set_fact:
+    mysql_command: "mysql -u{{ mysql_user }} -p{{ mysql_password }} -P{{ mysql_primary_port }} --protocol=tcp"
+
+
 # mysql_role module initial CI tests
 - import_tasks: mysql_role_initial.yml
+
+# Test that subtract_privs will only revoke the grants given by priv
+# (https://github.com/ansible-collections/community.mysql/issues/331)
+- include: test_priv_subtract.yml enable_check_mode=no
+- include: test_priv_subtract.yml enable_check_mode=yes