Fix column uppercasing (#569)

* 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 <laurent.indermuehle@epfl.ch>
Co-authored-by: Andrew Klychkov <aklychko@redhat.com>

CONTRIBUTORS

@@ -141,6 +141,7 @@ kalaisubbiah
 kenichi-ogawa-1988
 kkeane
 klingac
+kmarse
 koleo
 kotso
 kuntalFreshBooks

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).

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,6 +663,11 @@ def privileges_unpack(priv, mode, ensure_usage=True):
         pieces[0] = object_type + '.'.join(dbpriv)
 
         if '(' in pieces[1]:
+            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))

plugins/modules/mysql_role.py

@@ -121,6 +121,16 @@ options:
     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 @@ seealso:
 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))
 

plugins/modules/mysql_user.py

@@ -156,6 +156,16 @@ options:
     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 @@ author:
 - 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):

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

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

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

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

tests/unit/plugins/module_utils/test_mysql_user.py

@@ -9,7 +9,8 @@ from ansible_collections.community.mysql.plugins.module_utils.user import (
     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