diff --git a/changelogs/fragments/243-get-rid-of-privs-comparison.yml b/changelogs/fragments/243-get-rid-of-privs-comparison.yml new file mode 100644 index 00000000..6c298329 --- /dev/null +++ b/changelogs/fragments/243-get-rid-of-privs-comparison.yml @@ -0,0 +1,2 @@ +breaking_changes: + - mysql_user - validate privileges using database engine directly (https://github.com/ansible-collections/community.mysql/issues/234 https://github.com/ansible-collections/community.mysql/pull/243). Do not validate privileges in this module anymore. diff --git a/plugins/module_utils/user.py b/plugins/module_utils/user.py index 0532de94..fc9c984b 100644 --- a/plugins/module_utils/user.py +++ b/plugins/module_utils/user.py @@ -19,49 +19,6 @@ ) -EXTRA_PRIVS = ['ALL', 'ALL PRIVILEGES', 'GRANT', 'REQUIRESSL'] - -# This list is kept for backwards compatibility after release 2.3.0, -# see https://github.com/ansible-collections/community.mysql/issues/232 for details -VALID_PRIVS = [ - 'CREATE', 'DROP', 'GRANT', 'GRANT OPTION', - 'LOCK TABLES', 'REFERENCES', 'EVENT', 'ALTER', - 'DELETE', 'INDEX', 'INSERT', 'SELECT', 'UPDATE', - 'CREATE TEMPORARY TABLES', 'TRIGGER', 'CREATE VIEW', - 'SHOW VIEW', 'ALTER ROUTINE', 'CREATE ROUTINE', - 'EXECUTE', 'FILE', 'CREATE TABLESPACE', 'CREATE USER', - 'PROCESS', 'PROXY', 'RELOAD', 'REPLICATION CLIENT', - 'REPLICATION SLAVE', 'SHOW DATABASES', 'SHUTDOWN', - 'SUPER', 'ALL', 'ALL PRIVILEGES', 'USAGE', - 'REQUIRESSL', # Deprecated, to be removed in version 3.0.0 - 'CREATE ROLE', 'DROP ROLE', 'APPLICATION_PASSWORD_ADMIN', - 'AUDIT_ADMIN', 'BACKUP_ADMIN', 'BINLOG_ADMIN', - 'BINLOG_ENCRYPTION_ADMIN', 'CLONE_ADMIN', 'CONNECTION_ADMIN', - 'ENCRYPTION_KEY_ADMIN', 'FIREWALL_ADMIN', 'FIREWALL_USER', - 'GROUP_REPLICATION_ADMIN', 'INNODB_REDO_LOG_ARCHIVE', - 'NDB_STORED_USER', 'PERSIST_RO_VARIABLES_ADMIN', - 'REPLICATION_APPLIER', 'REPLICATION_SLAVE_ADMIN', - 'RESOURCE_GROUP_ADMIN', 'RESOURCE_GROUP_USER', - 'ROLE_ADMIN', 'SESSION_VARIABLES_ADMIN', 'SET_USER_ID', - 'SYSTEM_USER', 'SYSTEM_VARIABLES_ADMIN', 'SYSTEM_USER', - 'TABLE_ENCRYPTION_ADMIN', 'VERSION_TOKEN_ADMIN', - 'XA_RECOVER_ADMIN', 'LOAD FROM S3', 'SELECT INTO S3', - 'INVOKE LAMBDA', - 'ALTER ROUTINE', - 'BINLOG ADMIN', - 'BINLOG MONITOR', - 'BINLOG REPLAY', - 'CONNECTION ADMIN', - 'READ_ONLY ADMIN', - 'REPLICATION MASTER ADMIN', - 'REPLICATION SLAVE ADMIN', - 'SET USER', - 'SHOW_ROUTINE', - 'SLAVE MONITOR', - 'REPLICA MONITOR', -] - - class InvalidPrivsError(Exception): pass @@ -147,14 +104,6 @@ def get_tls_requires(cursor, user, host): return requires or None -def get_valid_privs(cursor): - cursor.execute("SHOW PRIVILEGES") - show_privs = [priv[0].upper() for priv in cursor.fetchall()] - # See the comment above VALID_PRIVS declaration - all_privs = show_privs + EXTRA_PRIVS + VALID_PRIVS - return frozenset(all_privs) - - def get_grants(cursor, user, host): cursor.execute("SHOW GRANTS FOR %s@%s", (user, host)) grants_line = list(filter(lambda x: "ON *.*" in x[0], cursor.fetchall()))[0] @@ -597,7 +546,7 @@ def sort_column_order(statement): return '%s(%s)' % (priv_name, ', '.join(columns)) -def privileges_unpack(priv, mode, valid_privs): +def privileges_unpack(priv, mode): """ 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 @@ -643,10 +592,6 @@ def privileges_unpack(priv, mode, valid_privs): # Handle cases when there's privs like GRANT SELECT (colA, ...) in privs. output[pieces[0]] = normalize_col_grants(output[pieces[0]]) - new_privs = frozenset(privs) - if not new_privs.issubset(valid_privs): - raise InvalidPrivsError('Invalid privileges specified: %s' % new_privs.difference(valid_privs)) - if '*.*' not in output: output['*.*'] = ['USAGE'] @@ -699,7 +644,10 @@ def privileges_grant(cursor, user, host, db_table, priv, tls_requires, maria_rol if 'GRANT' in priv: query.append("WITH GRANT OPTION") query = ' '.join(query) - cursor.execute(query, params) + try: + cursor.execute(query, params) + except (mysql_driver.ProgrammingError, mysql_driver.OperationalError, mysql_driver.InternalError) as e: + raise InvalidPrivsError("Error granting privileges, invalid priv string: %s" % priv_string) def convert_priv_dict_to_str(priv): diff --git a/plugins/modules/mysql_role.py b/plugins/modules/mysql_role.py index 47b0b581..80d01445 100644 --- a/plugins/modules/mysql_role.py +++ b/plugins/modules/mysql_role.py @@ -250,7 +250,6 @@ get_mode, user_mod, privileges_grant, - get_valid_privs, privileges_unpack, ) from ansible.module_utils._text import to_native @@ -1014,8 +1013,7 @@ def main(): module.fail_json(msg=to_native(e)) try: - valid_privs = get_valid_privs(cursor) - priv = privileges_unpack(priv, mode, valid_privs) + priv = privileges_unpack(priv, mode) except Exception as e: module.fail_json(msg='Invalid privileges string: %s' % to_native(e)) diff --git a/plugins/modules/mysql_user.py b/plugins/modules/mysql_user.py index 9c043cca..1eb67685 100644 --- a/plugins/modules/mysql_user.py +++ b/plugins/modules/mysql_user.py @@ -318,7 +318,6 @@ handle_requiressl_in_priv_string, InvalidPrivsError, limit_resources, - get_valid_privs, privileges_unpack, sanitize_requires, user_add, @@ -421,11 +420,7 @@ def main(): mode = get_mode(cursor) except Exception as e: module.fail_json(msg=to_native(e)) - try: - valid_privs = get_valid_privs(cursor) - priv = privileges_unpack(priv, mode, valid_privs) - except Exception as e: - module.fail_json(msg="invalid privileges string: %s" % to_native(e)) + priv = privileges_unpack(priv, mode) if state == "present": if user_exists(cursor, user, host, host_all): diff --git a/tests/integration/targets/test_mysql_user/tasks/test_priv_append.yml b/tests/integration/targets/test_mysql_user/tasks/test_priv_append.yml index 7dc15ca0..cd101474 100644 --- a/tests/integration/targets/test_mysql_user/tasks/test_priv_append.yml +++ b/tests/integration/targets/test_mysql_user/tasks/test_priv_append.yml @@ -96,6 +96,25 @@ - "'GRANT SELECT, DELETE ON `data2`.*' in result.stdout" when: enable_check_mode == 'yes' + - name: Try to append invalid privileges + mysql_user: + <<: *mysql_params + name: '{{ user_name_4 }}' + password: '{{ user_password_4 }}' + priv: 'data1.*:INVALID/data2.*:SELECT' + append_privs: yes + state: present + check_mode: '{{ enable_check_mode }}' + register: result + ignore_errors: true + + - name: Assert that there wasn't a change in privileges if check_mode is set to 'no' + assert: + that: + - result is failed + - "'Error granting privileges' in result.msg" + when: enable_check_mode == 'no' + ########## # Clean up - name: Drop test databases diff --git a/tests/integration/targets/test_mysql_user/tasks/test_privs.yml b/tests/integration/targets/test_mysql_user/tasks/test_privs.yml index 4ed75d17..27beb776 100644 --- a/tests/integration/targets/test_mysql_user/tasks/test_privs.yml +++ b/tests/integration/targets/test_mysql_user/tasks/test_privs.yml @@ -178,6 +178,23 @@ that: - "result.changed == false" + # ============================================================ + - name: update user with invalid privileges + mysql_user: + <<: *mysql_params + name: '{{ user_name_2 }}' + password: '{{ user_password_2 }}' + priv: '*.*:INVALID' + state: present + register: result + ignore_errors: yes + + - name: Assert that priv did not change + assert: + that: + - result is failed + - "'Error granting privileges' in result.msg" + - name: remove username mysql_user: <<: *mysql_params