diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 3acc8f33d..36e8a08bb 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -141,6 +141,7 @@ kalaisubbiah kenichi-ogawa-1988 kkeane klingac +kmarse koleo kotso kuntalFreshBooks diff --git a/changelogs/569_fix_column_uppercasing.yml b/changelogs/569_fix_column_uppercasing.yml new file mode 100644 index 000000000..781304ef3 --- /dev/null +++ b/changelogs/569_fix_column_uppercasing.yml @@ -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). diff --git a/plugins/module_utils/user.py b/plugins/module_utils/user.py index a63ad89b5..e1d80ab27 100644 --- a/plugins/module_utils/user.py +++ b/plugins/module_utils/user.py @@ -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 @@ -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]] diff --git a/plugins/modules/mysql_role.py b/plugins/modules/mysql_role.py index 7d672d788..e8920933b 100644 --- a/plugins/modules/mysql_role.py +++ b/plugins/modules/mysql_role.py @@ -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. @@ -139,6 +149,8 @@ author: - Andrew Klychkov (@Andersson007) - Felix Hamme (@betanummeric) + - kmarse (@kmarse) + - Laurent Indermühle (@laurent-indermuehle) extends_documentation_fragment: - community.mysql.mysql @@ -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, @@ -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 ' @@ -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: @@ -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)) diff --git a/plugins/modules/mysql_user.py b/plugins/modules/mysql_user.py index 38e51246e..3e914e6a4 100644 --- a/plugins/modules/mysql_user.py +++ b/plugins/modules/mysql_user.py @@ -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: @@ -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 ''' @@ -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, @@ -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)) @@ -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;") @@ -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): diff --git a/tests/integration/targets/test_mysql_role/tasks/main.yml b/tests/integration/targets/test_mysql_role/tasks/main.yml index b517fc053..44e3308e2 100644 --- a/tests/integration/targets/test_mysql_role/tasks/main.yml +++ b/tests/integration/targets/test_mysql_role/tasks/main.yml @@ -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 diff --git a/tests/integration/targets/test_mysql_role/tasks/test_column_case_sensitive.yml b/tests/integration/targets/test_mysql_role/tasks/test_column_case_sensitive.yml new file mode 100644 index 000000000..74849e066 --- /dev/null +++ b/tests/integration/targets/test_mysql_role/tasks/test_column_case_sensitive.yml @@ -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 diff --git a/tests/integration/targets/test_mysql_user/tasks/main.yml b/tests/integration/targets/test_mysql_user/tasks/main.yml index dc5c9d3cd..481680548 100644 --- a/tests/integration/targets/test_mysql_user/tasks/main.yml +++ b/tests/integration/targets/test_mysql_user/tasks/main.yml @@ -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 diff --git a/tests/integration/targets/test_mysql_user/tasks/test_column_case_sensitive.yml b/tests/integration/targets/test_mysql_user/tasks/test_column_case_sensitive.yml new file mode 100644 index 000000000..68e95aa4e --- /dev/null +++ b/tests/integration/targets/test_mysql_user/tasks/test_column_case_sensitive.yml @@ -0,0 +1,134 @@ +--- + +- 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_user Column case sensitive | Create a test table + community.mysql.mysql_query: + <<: *mysql_params + query: + - CREATE DATABASE mysql_user_column_case + - >- + CREATE TABLE mysql_user_column_case.t1 + (a int, B int, cC int, Dd int) + - >- + INSERT INTO mysql_user_column_case.t1 + (a, B, cC, Dd) VALUES (1,2,3,4) + + # ================= Reproduce failure ===================================== + + - name: Mysql_user Column case sensitive | Create test user + community.mysql.mysql_user: + <<: *mysql_params + name: column_case_sensitive + host: '%' + password: 'msandbox' + priv: + 'mysql_user_column_case.t1': 'SELECT(a, B, cC, Dd)' + + - name: Mysql_user Column case sensitive | Assert user privileges are all caps + community.mysql.mysql_query: + <<: *mysql_params + query: + - SHOW GRANTS FOR 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_user Column case sensitive | Assert 1 column is accessible on MySQL 5.7 + community.mysql.mysql_query: + <<: *mysql_params + login_user: column_case_sensitive + query: + - DESC mysql_user_column_case.t1 + register: assert_1_col_accessible + failed_when: + - assert_1_col_accessible.rowcount[0] | int != 1 + when: + - db_engine == 'mysql' and db_version is version('5.7', '<=') + + - name: Mysql_user Column case sensitive | Assert 4 column are accessible on MariaDB and MySQL 8+ + community.mysql.mysql_query: + <<: *mysql_params + login_user: column_case_sensitive + query: + - DESC mysql_user_column_case.t1 + register: assert_4_col_accessible + failed_when: + - assert_4_col_accessible.rowcount[0] | int != 4 + when: + - >- + db_engine == 'mariadb' + or (db_engine == 'mysql' and db_version is version('8.0', '>=')) + + # ======================== Test fix ====================================== + + - name: Mysql_user Column case sensitive | Create users with case sensitive + community.mysql.mysql_user: + <<: *mysql_params + name: column_case_sensitive + host: '%' + password: 'msandbox' + priv: + 'mysql_user_column_case.t1': 'SELECT(a, B, cC, Dd)' + column_case_sensitive: true + + - name: Mysql_user Column case sensitive | Assert user privileges are case sensitive + community.mysql.mysql_query: + <<: *mysql_params + query: + - SHOW GRANTS FOR 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_user Column case sensitive | Assert 4 columns are accessible + community.mysql.mysql_query: + <<: *mysql_params + login_user: column_case_sensitive + query: + - DESC mysql_user_column_case.t1 + register: assert_4_col_accessible + failed_when: + - assert_4_col_accessible.rowcount[0] | int != 4 + + # ========================= Teardown ====================================== + + - name: Mysql_user Column case sensitive | Delete test users + community.mysql.mysql_user: + <<: *mysql_params + name: column_case_sensitive + host_all: true + state: absent + + - name: Mysql_user Column case sensitive | Delete test database + community.mysql.mysql_db: + <<: *mysql_params + name: mysql_user_column_case + state: absent diff --git a/tests/unit/plugins/module_utils/test_mysql_user.py b/tests/unit/plugins/module_utils/test_mysql_user.py index 46b3b8eb6..bb1ec2446 100644 --- a/tests/unit/plugins/module_utils/test_mysql_user.py +++ b/tests/unit/plugins/module_utils/test_mysql_user.py @@ -9,7 +9,8 @@ handle_grant_on_col, has_grant_on_col, normalize_col_grants, - sort_column_order + sort_column_order, + privileges_unpack, ) @@ -92,3 +93,21 @@ def test_handle_grant_on_col(privileges, start, end, output): def test_normalize_col_grants(input_, expected): """Tests normalize_col_grants function.""" assert normalize_col_grants(input_) == expected + + +@pytest.mark.parametrize( + 'priv,expected,mode,column_case_sensitive,ensure_usage', + [ + ('mydb.*:SELECT', {'"mydb".*': ['SELECT']}, 'ANSI', False, False), + ('mydb.*:SELECT', {'`mydb`.*': ['SELECT']}, 'NOTANSI', False, False), + ('mydb.*:SELECT', {'"mydb".*': ['SELECT'], '*.*': ['USAGE']}, 'ANSI', False, True), + ('mydb.*:SELECT', {'`mydb`.*': ['SELECT'], '*.*': ['USAGE']}, 'NOTANSI', False, True), + ('mydb.*:SELECT (a)', {'`mydb`.*': ['SELECT (A)']}, 'NOTANSI', False, False), + ('mydb.*:UPDATE (b, a)', {'`mydb`.*': ['UPDATE (a, b)']}, 'NOTANSI', True, False), + ('mydb.*:SELECT (b, a, c)', {'`mydb`.*': ['SELECT (A, B, C)']}, 'NOTANSI', False, False), + ('mydb.*:SELECT (b, a, c)', {'`mydb`.*': ['SELECT (a, b, c)']}, 'NOTANSI', True, False), + ] +) +def test_privileges_unpack(priv, mode, column_case_sensitive, ensure_usage, expected): + """Tests privileges_unpack function.""" + assert privileges_unpack(priv, mode, column_case_sensitive, ensure_usage) == expected