Skip to content

Commit

Permalink
Fix column uppercasing (#569)
Browse files Browse the repository at this point in the history
* Add integrations tests for column case sensitive name

* add a warning when column_case_sensitive in not set

* add announce default will change in in 4.0.0

* fix tests for engine that don't wrap column in backticks

* add filter because only MySQL 5.7 is case sensitive for users privs

* add kmarse and myself to the authors

* add kmarse to the contributors list

---------

Co-authored-by: Laurent Indermühle <[email protected]>
Co-authored-by: Andrew Klychkov <[email protected]>
  • Loading branch information
3 people authored Oct 6, 2023
1 parent 8c2b6b0 commit 033b4c7
Show file tree
Hide file tree
Showing 10 changed files with 389 additions and 8 deletions.
1 change: 1 addition & 0 deletions CONTRIBUTORS
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ kalaisubbiah
kenichi-ogawa-1988
kkeane
klingac
kmarse
koleo
kotso
kuntalFreshBooks
Expand Down
21 changes: 21 additions & 0 deletions changelogs/569_fix_column_uppercasing.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
minor_changes:

- mysql_user - add ``column_case_sensitive`` option to prevent field names
from being uppercased
(https://github.com/ansible-collections/community.mysql/pull/569).
- mysql_role - add ``column_case_sensitive`` option to prevent field names
from being uppercased
(https://github.com/ansible-collections/community.mysql/pull/569).

major_changes:
- mysql_user - the ``column_case_sensitive`` argument's default value will be
changed to ``true`` in community.mysql 4.0.0. If your playbook expected the
column to be automatically uppercased for your users privileges, you should
set this to false explicitly
(https://github.com/ansible-collections/community.mysql/issues/577).
- mysql_role - the ``column_case_sensitive`` argument's default value will be
changed to ``true`` in community.mysql 4.0.0. If your playbook expected the
column to be automatically uppercased for your roles privileges, you should
set this to false explicitly
(https://github.com/ansible-collections/community.mysql/issues/578).
13 changes: 9 additions & 4 deletions plugins/module_utils/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ def sort_column_order(statement):
return '%s(%s)' % (priv_name, ', '.join(columns))


def privileges_unpack(priv, mode, ensure_usage=True):
def privileges_unpack(priv, mode, column_case_sensitive, 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
Expand Down Expand Up @@ -663,9 +663,14 @@ def privileges_unpack(priv, mode, ensure_usage=True):
pieces[0] = object_type + '.'.join(dbpriv)

if '(' in pieces[1]:
output[pieces[0]] = re.split(r',\s*(?=[^)]*(?:\(|$))', pieces[1].upper())
for i in output[pieces[0]]:
privs.append(re.sub(r'\s*\(.*\)', '', i))
if column_case_sensitive is True:
output[pieces[0]] = re.split(r',\s*(?=[^)]*(?:\(|$))', pieces[1])
for i in output[pieces[0]]:
privs.append(re.sub(r'\s*\(.*\)', '', i))
else:
output[pieces[0]] = re.split(r',\s*(?=[^)]*(?:\(|$))', pieces[1].upper())
for i in output[pieces[0]]:
privs.append(re.sub(r'\s*\(.*\)', '', i))
else:
output[pieces[0]] = pieces[1].upper().split(',')
privs = output[pieces[0]]
Expand Down
25 changes: 23 additions & 2 deletions plugins/modules/mysql_role.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,16 @@
type: bool
default: true
column_case_sensitive:
description:
- The default is C(false).
- When C(true), the module will not uppercase the field in the privileges.
- When C(false), the field names will be upper-cased. This was the default before this
feature was introduced but since MySQL/MariaDB is case sensitive you should set this
to C(true) in most cases.
type: bool
version_added: '3.8.0'
notes:
- Pay attention that the module runs C(SET DEFAULT ROLE ALL TO)
all the I(members) passed by default when the state has changed.
Expand All @@ -139,6 +149,8 @@
author:
- Andrew Klychkov (@Andersson007)
- Felix Hamme (@betanummeric)
- kmarse (@kmarse)
- Laurent Indermühle (@laurent-indermuehle)
extends_documentation_fragment:
- community.mysql.mysql
Expand Down Expand Up @@ -957,7 +969,8 @@ def main():
detach_members=dict(type='bool', default=False),
check_implicit_admin=dict(type='bool', default=False),
set_default_role_all=dict(type='bool', default=True),
members_must_exist=dict(type='bool', default=True)
members_must_exist=dict(type='bool', default=True),
column_case_sensitive=dict(type='bool', default=None), # TODO 4.0.0 add default=True
)
module = AnsibleModule(
argument_spec=argument_spec,
Expand Down Expand Up @@ -992,6 +1005,7 @@ def main():
db = ''
set_default_role_all = module.params['set_default_role_all']
members_must_exist = module.params['members_must_exist']
column_case_sensitive = module.params['column_case_sensitive']

if priv and not isinstance(priv, (str, dict)):
msg = ('The "priv" parameter must be str or dict '
Expand All @@ -1004,6 +1018,13 @@ def main():
if mysql_driver is None:
module.fail_json(msg=mysql_driver_fail_msg)

# TODO Release 4.0.0 : Remove this test and variable assignation
if column_case_sensitive is None:
column_case_sensitive = False
module.warn("Option column_case_sensitive is not provided. "
"The default is now false, so the column's name will be uppercased. "
"The default will be changed to true in community.mysql 4.0.0.")

cursor = None
try:
if check_implicit_admin:
Expand Down Expand Up @@ -1041,7 +1062,7 @@ def main():
module.fail_json(msg=to_native(e))

try:
priv = privileges_unpack(priv, mode, ensure_usage=not subtract_privs)
priv = privileges_unpack(priv, mode, column_case_sensitive, ensure_usage=not subtract_privs)
except Exception as e:
module.fail_json(msg='Invalid privileges string: %s' % to_native(e))

Expand Down
25 changes: 24 additions & 1 deletion plugins/modules/mysql_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,16 @@
type: dict
version_added: '3.6.0'
column_case_sensitive:
description:
- The default is C(false).
- When C(true), the module will not uppercase the field names in the privileges.
- When C(false), the field names will be upper-cased. This is the default
- This feature was introduced because MySQL 8 and above uses case sensitive
fields names in privileges.
type: bool
version_added: '3.8.0'
notes:
- "MySQL server installs with default I(login_user) of C(root) and no password.
To secure this user as part of an idempotent playbook, you must create at least two tasks:
Expand All @@ -181,6 +191,9 @@
- Jonathan Mainguy (@Jmainguy)
- Benjamin Malynovytch (@bmalynovytch)
- Lukasz Tomaszkiewicz (@tomaszkiewicz)
- kmarse (@kmarse)
- Laurent Indermühle (@laurent-indermuehle)
extends_documentation_fragment:
- community.mysql.mysql
'''
Expand Down Expand Up @@ -401,6 +414,7 @@ def main():
resource_limits=dict(type='dict'),
force_context=dict(type='bool', default=False),
session_vars=dict(type='dict'),
column_case_sensitive=dict(type='bool', default=None), # TODO 4.0.0 add default=True
)
module = AnsibleModule(
argument_spec=argument_spec,
Expand Down Expand Up @@ -436,6 +450,7 @@ def main():
plugin_auth_string = module.params["plugin_auth_string"]
resource_limits = module.params["resource_limits"]
session_vars = module.params["session_vars"]
column_case_sensitive = module.params["column_case_sensitive"]

if priv and not isinstance(priv, (str, dict)):
module.fail_json(msg="priv parameter must be str or dict but %s was passed" % type(priv))
Expand All @@ -462,6 +477,13 @@ def main():
module.fail_json(msg="unable to connect to database, check login_user and login_password are correct or %s has the credentials. "
"Exception message: %s" % (config_file, to_native(e)))

# TODO Release 4.0.0 : Remove this test and variable assignation
if column_case_sensitive is None:
column_case_sensitive = False
module.warn("Option column_case_sensitive is not provided. "
"The default is now false, so the column's name will be uppercased. "
"The default will be changed to true in community.mysql 4.0.0.")

if not sql_log_bin:
cursor.execute("SET SQL_LOG_BIN=0;")

Expand All @@ -475,7 +497,8 @@ def main():
mode = get_mode(cursor)
except Exception as e:
module.fail_json(msg=to_native(e))
priv = privileges_unpack(priv, mode, ensure_usage=not subtract_privs)

priv = privileges_unpack(priv, mode, column_case_sensitive, ensure_usage=not subtract_privs)
password_changed = False
if state == "present":
if user_exists(cursor, user, host, host_all):
Expand Down
4 changes: 4 additions & 0 deletions tests/integration/targets/test_mysql_role/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,7 @@
- include_tasks: test_priv_subtract.yml
vars:
enable_check_mode: yes

- name: Test column case sensitive
ansible.builtin.import_tasks:
file: test_column_case_sensitive.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
---

- vars:
mysql_parameters: &mysql_params
login_user: '{{ mysql_user }}'
login_password: '{{ mysql_password }}'
login_host: '{{ mysql_host }}'
login_port: '{{ mysql_primary_port }}'

block:

# ========================= Prepare =======================================
# We use query to prevent our module of changing the case
- name: Mysql_role Column case sensitive | Create a test table
community.mysql.mysql_query:
<<: *mysql_params
query:
- CREATE DATABASE mysql_role_column_case
- >-
CREATE TABLE mysql_role_column_case.t1
(a int, B int, cC int, Dd int)
- >-
INSERT INTO mysql_role_column_case.t1
(a, B, cC, Dd) VALUES (1,2,3,4)
- name: Mysql_role Column case sensitive | Create users
community.mysql.mysql_user:
<<: *mysql_params
name: column_case_sensitive
host: '%'
password: 'msandbox'

# ================= Reproduce failure =====================================

- name: Mysql_role Column case sensitive | Create role
community.mysql.mysql_role:
<<: *mysql_params
name: 'role_column_case_sensitive'
state: present
members:
- 'column_case_sensitive@%'
priv:
'mysql_role_column_case.t1': 'SELECT(a, B, cC, Dd)'

- name: Mysql_role Column case sensitive | Assert role privileges are all caps
community.mysql.mysql_query:
<<: *mysql_params
query:
- SHOW GRANTS FOR role_column_case_sensitive
register: column_case_insensitive_grants
failed_when:
# Column order may vary, thus test each separately
- >-
column_case_insensitive_grants.query_result[0][1]
is not search("A", ignorecase=false)
or column_case_insensitive_grants.query_result[0][1]
is not search("B", ignorecase=false)
or column_case_insensitive_grants.query_result[0][1]
is not search("CC", ignorecase=false)
or column_case_insensitive_grants.query_result[0][1]
is not search("DD", ignorecase=false)
- name: Mysql_role Column case sensitive | Assert 1 column is accessible on MySQL
community.mysql.mysql_query:
<<: *mysql_params
login_user: column_case_sensitive
query:
- DESC mysql_role_column_case.t1
register: assert_1_col_accessible
failed_when:
- assert_1_col_accessible.rowcount[0] | int != 1
when:
- db_engine == 'mysql'

- name: Mysql_role Column case sensitive | Assert 4 column are accessible on MariaDB
community.mysql.mysql_query:
<<: *mysql_params
login_user: column_case_sensitive
query:
- SET ROLE role_column_case_sensitive
- DESC mysql_role_column_case.t1
register: assert_4_col_accessible
failed_when:
- assert_4_col_accessible.rowcount[1] | int != 4
when:
- db_engine == 'mariadb'

# ====================== Test the fix =====================================

- name: Mysql_role Column case sensitive | Recreate role with case sensitive
community.mysql.mysql_role:
<<: *mysql_params
name: 'role_column_case_sensitive'
state: present
members:
- 'column_case_sensitive@%'
priv:
'mysql_role_column_case.t1': 'SELECT(a, B, cC, Dd)'
column_case_sensitive: true

- name: Mysql_role Column case sensitive | Assert role privileges are case sensitive
community.mysql.mysql_query:
<<: *mysql_params
query:
- SHOW GRANTS FOR role_column_case_sensitive
register: column_case_sensitive_grants
failed_when:
# Column order may vary, thus test each separately
- >-
column_case_sensitive_grants.query_result[0][1]
is not search("a", ignorecase=false)
or column_case_sensitive_grants.query_result[0][1]
is not search("B", ignorecase=false)
or column_case_sensitive_grants.query_result[0][1]
is not search("cC", ignorecase=false)
or column_case_sensitive_grants.query_result[0][1]
is not search("Dd", ignorecase=false)
- name: Mysql_role Column case sensitive | Assert 4 columns are accessible
community.mysql.mysql_query:
<<: *mysql_params
login_user: column_case_sensitive
query:
- SET ROLE role_column_case_sensitive
- DESC mysql_role_column_case.t1
register: assert_4_col_accessible
failed_when:
- assert_4_col_accessible.rowcount[1] | int != 4

# ========================= Teardown ======================================

- name: Mysql_role Column case sensitive | Delete test users
community.mysql.mysql_user:
<<: *mysql_params
name: column_case_sensitive
host_all: true
state: absent

- name: Mysql_role Column case sensitive | Delete role
community.mysql.mysql_role:
<<: *mysql_params
name: 'role_column_case_sensitive'
state: absent

- name: Mysql_role Column case sensitive | Delete test database
community.mysql.mysql_db:
<<: *mysql_params
name: mysql_role_column_case
state: absent
4 changes: 4 additions & 0 deletions tests/integration/targets/test_mysql_user/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -286,3 +286,7 @@
- include_tasks: test_user_grants_with_roles_applied.yml

- include_tasks: test_revoke_only_grant.yml

- name: Mysql_user - test column case sensitive
ansible.builtin.import_tasks:
file: test_column_case_sensitive.yml
Loading

0 comments on commit 033b4c7

Please sign in to comment.