mysql_user, mysql_role: add argument subtract_privs to revoke privile…

…ges explicitly (#333) * add option subtract_privs to mysql_role and mysql_user see #331 * add integration tests for subtract_privs for mysql_role and mysql_user * add changelog fragment for PR #333 * mysql_role, mysql_user: when subtract_privileges, don't grant unwanted privileges and don't revoke USAGE implicitly * fix integration tests * mysql_role, mysql_user: invalid privileges are ignored when subtract_privs is true -> document that and fix integration tests * fix mysql_role integration tests * fix mysql_role, mysql_user integration tests * formatting make the PEP8 check happy * mysql_user and mysql_role: fix granting privileges when only the GRANT OPTION needs to be added * mysql_user and mysql_role: log some updated privileges; explain integration test blind spot * mysql_user and mysql_role: don't grant too much privileges If only the grant option needs to be granted, at least one privilege needs to be granted to get valid syntax. USAGE is better for that than the existing privileges, because unwanted privileges would be re-added after revokation. * mysql_user and mysql_role: fix type error * Update changelogs/fragments/333-mysql_user-mysql_role-add-subtract_privileges-argument.yml Co-authored-by: Andrew Klychkov <[email protected]> * Update plugins/modules/mysql_role.py Co-authored-by: Andrew Klychkov <[email protected]> * Update plugins/modules/mysql_user.py Co-authored-by: Andrew Klychkov <[email protected]> Co-authored-by: Felix Hamme <[email protected]> Co-authored-by: Andrew Klychkov <[email protected]>
ansible-collections · May 9, 2022 · ba4fea6 · ba4fea6
1 parent 1dcc5ec
commit ba4fea6
Show file tree

Hide file tree

Showing 10 changed files with 460 additions and 43 deletions.
diff --git a/changelogs/fragments/333-mysql_user-mysql_role-add-subtract_privileges-argument.yml b/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)."
diff --git a/plugins/module_utils/user.py b/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,61 @@ 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])
-
-                if len(priv_diff) > 0:
-                    msg = "Privileges updated"
+                    # 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 grant_privs == ['GRANT']:
+                    # USAGE grants no privileges, it is only needed because 'WITH GRANT OPTION' cannot stand alone
+                    grant_privs.append('USAGE')
+
+                if len(grant_privs) + len(revoke_privs) > 0:
+                    msg = "Privileges updated: granted %s, revoked %s" % (grant_privs, revoke_privs)
                     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 +563,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 +609,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

diff --git a/plugins/modules/mysql_role.py b/plugins/modules/mysql_role.py
@@ -51,7 +51,16 @@
   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).
+    version_added: '3.2.0'
     type: bool
     default: no
 
@@ -233,6 +242,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 +838,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 +854,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 +880,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 +950,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 +965,7 @@ def main():
             ('admin', 'members'),
             ('admin', 'append_members'),
             ('admin', 'detach_members'),
+            ('append_privs', 'subtract_privs'),
         ),
     )
 
@@ -958,6 +979,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 +1036,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 +1065,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)
 

diff --git a/plugins/modules/mysql_user.py b/plugins/modules/mysql_user.py
@@ -63,7 +63,15 @@
   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).
+    version_added: '3.2.0'
     type: bool
     default: no
   tls_requires:
@@ -306,6 +314,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 +367,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 +380,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 +396,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 +445,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)

diff --git a/tests/integration/targets/test_mysql_role/defaults/main.yml b/tests/integration/targets/test_mysql_role/defaults/main.yml
@@ -14,3 +14,4 @@ nonexistent: user3
 
 role0: role0
 role1: role1
+role2: role2
diff --git a/tests/integration/targets/test_mysql_role/tasks/main.yml b/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
Original file line number	Diff line number	Diff line change
		@@ -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)."
Original file line number	Diff line number	Diff line change
Expand Up		@@ -14,3 +14,4 @@ nonexistent: user3

		role0: role0
		role1: role1
		role2: role2