Deprecate REQUIRESSL privilege (#132)

* Deprecate REQUIRESSL privilege

* Add missing whitespace

* Fix according to PR review

* Fix conditional check

* Fix privilege string parsing

* Add unit tests for the new function

* Add integration tests

* Fix parentheses indentation

* Cover alternative error message

* Fix privileges

* Limit verification of access denied to pymysql connector

* Fix REQUIRE SSL verification tests

changelogs/fragments/101-drop-requiressl-support.yml

@@ -0,0 +1,4 @@
+minor_changes:
+  - mysql_user - deprecate the ``REQUIRESSL`` privilege (https://github.com/ansible-collections/community.mysql/issues/101).
+major_changes:
+  - mysql_user - the ``REQUIRESSL`` is an alias for the ``ssl`` key in the ``tls_requires`` option in ``community.mysql`` 2.0.0 and support will be dropped altogether in ``community.mysql`` 3.0.0 (https://github.com/ansible-collections/community.mysql/issues/121).

plugins/modules/mysql_user.py

@@ -189,7 +189,7 @@ EXAMPLES = r'''
       'db2.*': 'ALL,GRANT'
 
 # Note that REQUIRESSL is a special privilege that should only apply to *.* by itself.
-# Setting this privilege in this manner is supported for backwards compatibility only.
+# Setting this privilege in this manner is deprecated.
 # Use 'tls_requires' instead.
 - name: Modify user to require SSL connections
   community.mysql.mysql_user:
@@ -316,7 +316,8 @@ VALID_PRIVS = frozenset(('CREATE', 'DROP', 'GRANT', 'GRANT OPTION',
                          'EXECUTE', 'FILE', 'CREATE TABLESPACE', 'CREATE USER',
                          'PROCESS', 'PROXY', 'RELOAD', 'REPLICATION CLIENT',
                          'REPLICATION SLAVE', 'SHOW DATABASES', 'SHUTDOWN',
-                         'SUPER', 'ALL', 'ALL PRIVILEGES', 'USAGE', 'REQUIRESSL',
+                         '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',
@@ -745,8 +746,6 @@ def privileges_get(cursor, user, host):
 
         if "WITH GRANT OPTION" in res.group(7):
             privileges.append('GRANT')
-        if 'REQUIRE SSL' in res.group(7):
-            privileges.append('REQUIRESSL')
         db = res.group(2)
         output.setdefault(db, []).extend(privileges)
     return output
@@ -919,11 +918,6 @@ def privileges_unpack(priv, mode):
     if '*.*' not in output:
         output['*.*'] = ['USAGE']
 
-    # if we are only specifying something like REQUIRESSL and/or GRANT (=WITH GRANT OPTION) in *.*
-    # we still need to add USAGE as a privilege to avoid syntax errors
-    if 'REQUIRESSL' in priv and not set(output['*.*']).difference(set(['GRANT', 'REQUIRESSL'])):
-        output['*.*'].append('USAGE')
-
     return output
 
 
@@ -935,7 +929,7 @@ def privileges_revoke(cursor, user, host, db_table, priv, grant_option):
         query.append("FROM %s@%s")
         query = ' '.join(query)
         cursor.execute(query, (user, host))
-    priv_string = ",".join([p for p in priv if p not in ('GRANT', 'REQUIRESSL')])
+    priv_string = ",".join([p for p in priv if p not in ('GRANT', )])
     query = ["REVOKE %s ON %s" % (priv_string, db_table)]
     query.append("FROM %s@%s")
     query = ' '.join(query)
@@ -946,15 +940,13 @@ def privileges_grant(cursor, user, host, db_table, priv, tls_requires):
     # Escape '%' since mysql db.execute uses a format string and the
     # specification of db and table often use a % (SQL wildcard)
     db_table = db_table.replace('%', '%%')
-    priv_string = ",".join([p for p in priv if p not in ('GRANT', 'REQUIRESSL')])
+    priv_string = ",".join([p for p in priv if p not in ('GRANT', )])
     query = ["GRANT %s ON %s" % (priv_string, db_table)]
     query.append("TO %s@%s")
     params = (user, host)
     if tls_requires and impl.use_old_user_mgmt(cursor):
         query, params = mogrify_requires(" ".join(query), params, tls_requires)
         query = [query]
-    if 'REQUIRESSL' in priv and not tls_requires:
-        query.append("REQUIRE SSL")
     if 'GRANT' in priv:
         query.append("WITH GRANT OPTION")
     query = ' '.join(query)
@@ -975,6 +967,27 @@ def convert_priv_dict_to_str(priv):
     return '/'.join(priv_list)
 
 
+def handle_requiressl_in_priv_string(module, priv, tls_requires):
+    module.deprecate('The "REQUIRESSL" privilege is deprecated, use the "tls_requires" option instead.',
+                     version='3.0.0', collection_name='community.mysql')
+    priv_groups = re.search(r"(.*?)(\*\.\*:)([^/]*)(.*)", priv)
+    if priv_groups.group(3) == "REQUIRESSL":
+        priv = priv_groups.group(1) + priv_groups.group(4) or None
+    else:
+        inner_priv_groups = re.search(r"(.*?),?REQUIRESSL,?(.*)", priv_groups.group(3))
+        priv = '{0}{1}{2}{3}'.format(
+            priv_groups.group(1),
+            priv_groups.group(2),
+            ','.join(filter(None, (inner_priv_groups.group(1), inner_priv_groups.group(2)))),
+            priv_groups.group(4)
+        )
+    if not tls_requires:
+        tls_requires = "SSL"
+    else:
+        module.warn('Ignoring "REQUIRESSL" privilege as "tls_requires" is defined and it takes precedence.')
+    return priv, tls_requires
+
+
 # Alter user is supported since MySQL 5.6 and MariaDB 10.2.0
 def server_supports_alter_user(cursor):
     """Check if the server supports ALTER USER statement or doesn't.
@@ -1167,6 +1180,9 @@ def main():
     if priv and isinstance(priv, dict):
         priv = convert_priv_dict_to_str(priv)
 
+    if priv and "REQUIRESSL" in priv:
+        priv, tls_requires = handle_requiressl_in_priv_string(module, priv, tls_requires)
+
     if mysql_driver is None:
         module.fail_json(msg=mysql_driver_fail_msg)
 

tests/integration/targets/test_mysql_user/tasks/issue-121.yml

@@ -0,0 +1,115 @@
+---
+- vars:
+    mysql_parameters: &mysql_params
+      login_user: '{{ mysql_user }}'
+      login_password: '{{ mysql_password }}'
+      login_host: 127.0.0.1
+      login_port: '{{ mysql_primary_port }}'
+
+  block:
+
+    # ============================================================
+    - shell: pip show pymysql | awk '/Version/ {print $2}'
+      register: pymysql_version
+
+    - name: get server certificate
+      copy:
+        content: "{{ lookup('pipe', \"openssl s_client -starttls mysql -connect localhost:3307 -showcerts 2>/dev/null </dev/null |  sed -ne '/-BEGIN CERTIFICATE-/,/-END CERTIFICATE-/p'\") }}"
+        dest: /tmp/cert.pem
+      delegate_to: localhost
+
+    - name: get server version
+      mysql_info:
+        <<: *mysql_params
+        filter: version
+      register: db_version
+
+    - set_fact:
+        old_user_mgmt: "{{ db_version.version.major <= 5 and db_version.version.minor <= 6 or db_version.version.major == 10 and db_version.version.minor < 2 | bool }}"
+
+    - name: Drop mysql user if exists
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ item }}'
+        state: absent
+      ignore_errors: yes
+      with_items:
+        - "{{ user_name_1 }}"
+        - "{{ user_name_2 }}"
+
+    - name: create user with REQUIRESSL privilege
+      mysql_user:
+        <<: *mysql_params
+        name: "{{ user_name_1 }}"
+        password: "{{ user_password_1 }}"
+        priv: '*.*:SELECT,CREATE USER,REQUIRESSL,GRANT'
+
+    - name: verify REQUIRESSL is assigned to the user
+      mysql_query:
+        <<: *mysql_params
+        query: "SHOW {{ what }} '{{ user_name_1}}'@'localhost'"
+      register: result
+      vars:
+        what: "{{ 'GRANTS FOR' if old_user_mgmt else 'CREATE USER' }}"
+
+    - assert:
+        that:
+          - result is succeeded and 'REQUIRE SSL' in (result.query_result | string)
+
+    - name: create user with equivalent ssl requirement in tls_requires (expect unchanged)
+      mysql_user:
+        <<: *mysql_params
+        name: "{{ user_name_1 }}"
+        password: "{{ user_password_1 }}"
+        priv: '*.*:SELECT,CREATE USER,GRANT'
+        tls_requires:
+          SSL:
+      register: result
+
+    - assert:
+        that:
+          - result is not changed
+
+    - name: create the same user again, with REQUIRESSL privilege once more
+      mysql_user:
+        <<: *mysql_params
+        name: "{{ user_name_1 }}"
+        password: "{{ user_password_1 }}"
+        priv: '*.*:SELECT,CREATE USER,REQUIRESSL,GRANT'
+      register: result
+
+    - assert:
+        that:
+          - result is not changed
+
+    - name: create user with both REQUIRESSL privilege and an incompatible tls_requires option
+      mysql_user:
+        <<: *mysql_params
+        name: "{{ user_name_1 }}"
+        password: "{{ user_password_1 }}"
+        priv: '*.*:SELECT,CREATE USER,REQUIRESSL,GRANT'
+        tls_requires:
+          X509:
+
+    - name: create same user again without REQUIRESSL privilege
+      mysql_user:
+        <<: *mysql_params
+        name: "{{ user_name_1 }}"
+        password: "{{ user_password_1 }}"
+        priv: '*.*:SELECT,CREATE USER,GRANT'
+        tls_requires:
+          X509:
+      register: result
+
+    - assert:
+        that: result is not changed
+
+    - name: Drop mysql user
+      mysql_user:
+        <<: *mysql_params
+        name: '{{ item }}'
+        host: 127.0.0.1
+        state: absent
+      with_items:
+        - "{{ user_name_1 }}"
+        - "{{ user_name_2 }}"

tests/integration/targets/test_mysql_user/tasks/main.yml

@@ -37,6 +37,8 @@
 
   block:
 
+    - include: issue-121.yml
+
     - include: issue-28.yml
 
     - include: create_user.yml user_name={{user_name_1}} user_password={{ user_password_1 }}

tests/unit/plugins/modules/test_mysql_user.py

@@ -4,12 +4,17 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
 import pytest
+try:
+    from unittest.mock import MagicMock
+except ImportError:
+    from mock import MagicMock
 
 from ansible_collections.community.mysql.plugins.modules.mysql_user import (
     handle_grant_on_col,
     has_grant_on_col,
     normalize_col_grants,
     sort_column_order,
+    handle_requiressl_in_priv_string
 )
 from ..utils import dummy_cursor_class
 
@@ -74,6 +79,71 @@ def test_handle_grant_on_col(privileges, start, end, output):
     assert handle_grant_on_col(privileges, start, end) == output
 
 
+@pytest.mark.parametrize(
+    'input_tuple,output_tuple',
+    [
+        (('*.*:REQUIRESSL', None), (None, 'SSL')),
+        (('*.*:ALL,REQUIRESSL', None), ('*.*:ALL', 'SSL')),
+        (('*.*:REQUIRESSL,ALL', None), ('*.*:ALL', 'SSL')),
+        (('*.*:ALL,REQUIRESSL,GRANT', None), ('*.*:ALL,GRANT', 'SSL')),
+        (('*.*:ALL,REQUIRESSL,GRANT/a.b:USAGE', None), ('*.*:ALL,GRANT/a.b:USAGE', 'SSL')),
+        (('*.*:REQUIRESSL', 'X509'), (None, 'X509')),
+        (('*.*:ALL,REQUIRESSL', 'X509'), ('*.*:ALL', 'X509')),
+        (('*.*:REQUIRESSL,ALL', 'X509'), ('*.*:ALL', 'X509')),
+        (('*.*:ALL,REQUIRESSL,GRANT', 'X509'), ('*.*:ALL,GRANT', 'X509')),
+        (('*.*:ALL,REQUIRESSL,GRANT/a.b:USAGE', 'X509'), ('*.*:ALL,GRANT/a.b:USAGE', 'X509')),
+        (('*.*:REQUIRESSL', {
+            'subject': '/CN=alice/O=MyDom, Inc./C=US/ST=Oregon/L=Portland',
+            'cipher': 'ECDHE-ECDSA-AES256-SHA384',
+            'issuer': '/CN=org/O=MyDom, Inc./C=US/ST=Oregon/L=Portland'
+        }), (None, {
+            'subject': '/CN=alice/O=MyDom, Inc./C=US/ST=Oregon/L=Portland',
+            'cipher': 'ECDHE-ECDSA-AES256-SHA384',
+            'issuer': '/CN=org/O=MyDom, Inc./C=US/ST=Oregon/L=Portland'
+        })),
+        (('*.*:ALL,REQUIRESSL', {
+            'subject': '/CN=alice/O=MyDom, Inc./C=US/ST=Oregon/L=Portland',
+            'cipher': 'ECDHE-ECDSA-AES256-SHA384',
+            'issuer': '/CN=org/O=MyDom, Inc./C=US/ST=Oregon/L=Portland'
+        }), ('*.*:ALL', {
+            'subject': '/CN=alice/O=MyDom, Inc./C=US/ST=Oregon/L=Portland',
+            'cipher': 'ECDHE-ECDSA-AES256-SHA384',
+            'issuer': '/CN=org/O=MyDom, Inc./C=US/ST=Oregon/L=Portland'
+        })),
+        (('*.*:REQUIRESSL,ALL', {
+            'subject': '/CN=alice/O=MyDom, Inc./C=US/ST=Oregon/L=Portland',
+            'cipher': 'ECDHE-ECDSA-AES256-SHA384',
+            'issuer': '/CN=org/O=MyDom, Inc./C=US/ST=Oregon/L=Portland'
+        }), ('*.*:ALL', {
+            'subject': '/CN=alice/O=MyDom, Inc./C=US/ST=Oregon/L=Portland',
+            'cipher': 'ECDHE-ECDSA-AES256-SHA384',
+            'issuer': '/CN=org/O=MyDom, Inc./C=US/ST=Oregon/L=Portland'
+        })),
+        (('*.*:ALL,REQUIRESSL,GRANT', {
+            'subject': '/CN=alice/O=MyDom, Inc./C=US/ST=Oregon/L=Portland',
+            'cipher': 'ECDHE-ECDSA-AES256-SHA384',
+            'issuer': '/CN=org/O=MyDom, Inc./C=US/ST=Oregon/L=Portland'
+        }), ('*.*:ALL,GRANT', {
+            'subject': '/CN=alice/O=MyDom, Inc./C=US/ST=Oregon/L=Portland',
+            'cipher': 'ECDHE-ECDSA-AES256-SHA384',
+            'issuer': '/CN=org/O=MyDom, Inc./C=US/ST=Oregon/L=Portland'
+        })),
+        (('*.*:ALL,REQUIRESSL,GRANT/a.b:USAGE', {
+            'subject': '/CN=alice/O=MyDom, Inc./C=US/ST=Oregon/L=Portland',
+            'cipher': 'ECDHE-ECDSA-AES256-SHA384',
+            'issuer': '/CN=org/O=MyDom, Inc./C=US/ST=Oregon/L=Portland'
+        }), ('*.*:ALL,GRANT/a.b:USAGE', {
+            'subject': '/CN=alice/O=MyDom, Inc./C=US/ST=Oregon/L=Portland',
+            'cipher': 'ECDHE-ECDSA-AES256-SHA384',
+            'issuer': '/CN=org/O=MyDom, Inc./C=US/ST=Oregon/L=Portland'
+        }))
+    ]
+)
+def test_handle_requiressl_in_priv_string(input_tuple, output_tuple):
+    """Tests the handle_requiressl_in_priv_string funciton."""
+    assert handle_requiressl_in_priv_string(MagicMock(), *input_tuple) == output_tuple
+
+
 @pytest.mark.parametrize(
     'input_,expected',
     [