Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of privs comparison #243

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/243-get-rid-of-privs-comparison.yml
Original file line number Diff line number Diff line change
@@ -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.
62 changes: 5 additions & 57 deletions plugins/module_utils/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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']

Expand Down Expand Up @@ -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):
Expand Down
4 changes: 1 addition & 3 deletions plugins/modules/mysql_role.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@
get_mode,
user_mod,
privileges_grant,
get_valid_privs,
privileges_unpack,
)
from ansible.module_utils._text import to_native
Expand Down Expand Up @@ -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))

Expand Down
7 changes: 1 addition & 6 deletions plugins/modules/mysql_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,6 @@
handle_requiressl_in_priv_string,
InvalidPrivsError,
limit_resources,
get_valid_privs,
privileges_unpack,
sanitize_requires,
user_add,
Expand Down Expand Up @@ -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):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions tests/integration/targets/test_mysql_user/tasks/test_privs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down