Repository: saltstack/salt
Pull request: Reworking the mysql modules so that passwordless does not use unix_socket plugin
### What does this PR do?
Reworking the mysql module and state so that passwordless does not try to use unix_socket until unix_socket is set to True.  Adding tests to test passwordless and unix_socket functionality.

### What issues does this PR fix or reference?
Fixes: #59337 

### Merge requirements satisfied?
**[NOTICE] Bug fixes or features added to Salt require tests.**
<!-- Please review the [test documentation](https://docs.saltstack.com/en/master/topics/tutorials/writing_tests.html) for details on how to implement tests into Salt's test suite. -->
- [ ] Docs
- [x] Changelog - https://docs.saltstack.com/en/master/topics/development/changelog.html
- [x] Tests written/updated

### Commits signed with GPG?
Yes/No

Please review [Salt's Contributing Guide](https://docs.saltstack.com/en/master/topics/development/contributing.html) for best practices.

See GitHub's [page on GPG signing](https://help.github.com/articles/signing-commits-using-gpg/) for more information about signing commits with GPG.


Problem statement:
[BUG] allow_passwordless in mysql_user.present force the use of unix_socket
**Description**
allow_passwordless in mysql_user.present force the use of unix_socket even if the unix_socket set to false

**Setup**
```
phpmyadmin-user:
  mysql_user.present:
    - name: phpmyadmin
    - host: 'localhost'
    - allow_passwordless: true
    - unix_socket: false
```

**Steps to Reproduce the behavior**
```
[INFO    ] Executing state mysql_user.present for [phpmyadmin]
[DEBUG   ] Doing query: SELECT VERSION()
[DEBUG   ] Doing query: SELECT column_name from information_schema.COLUMNS WHERE table_schema=%(schema)s and table_name=%(table)s and column_name=%(column)s args: {'schema': 'mysql', 'table': 'user', 'column': 'Password'} 
[DEBUG   ] Doing query: SELECT plugin FROM mysql.user WHERE User=%(user)s and Host=%(host)s args: {'user': 'phpmyadmin', 'host': 'localhost'} 
[DEBUG   ] ()
[DEBUG   ] Doing query: SELECT User,Host FROM mysql.user WHERE User = %(user)s AND Host = %(host)s AND Password = '' args: {'user': 'phpmyadmin', 'host': 'localhost'} 
[DEBUG   ] Doing query: SELECT plugin FROM mysql.user WHERE User=%(user)s and Host=%(host)s args: {'user': 'phpmyadmin', 'host': 'localhost'} 
[DEBUG   ] ()
[DEBUG   ] Doing query: SELECT User,Host FROM mysql.user WHERE User = %(user)s AND Host = %(host)s args: {'user': 'phpmyadmin', 'host': 'localhost'} 
[DEBUG   ] Doing query: SELECT plugin FROM mysql.user WHERE User=%(user)s and Host=%(host)s args: {'user': 'phpmyadmin', 'host': 'localhost'} 
[DEBUG   ] ()
[DEBUG   ] Doing query: SELECT User,Host FROM mysql.user WHERE User = %(user)s AND Host = %(host)s args: {'user': 'phpmyadmin', 'host': 'localhost'} 
[DEBUG   ] Doing query: SELECT PLUGIN_STATUS FROM INFORMATION_SCHEMA.PLUGINS WHERE PLUGIN_NAME = %(name)s args: {'name': 'unix_socket'} 
[ERROR   ] The unix_socket plugin is not enabled.
[ERROR   ] Failed to create user phpmyadmin@localhost (The unix_socket plugin is not enabled.)
```

**Expected behavior**
The user has been added.

**Versions Report**
<details><summary>salt --versions-report</summary>
(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.) 

```
Salt Version:
          Salt: 3002.2
 
Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.4.2
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.11.1
       libgit2: Not Installed
      M2Crypto: 0.35.2
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: 1.3.12
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: Not Installed
        pygit2: Not Installed
        Python: 3.6.8 (default, Nov 16 2020, 16:55:22)
  python-gnupg: Not Installed
        PyYAML: 3.13
         PyZMQ: 17.0.0
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.1.4
 
System Versions:
          dist: centos 7 Core
        locale: UTF-8
       machine: x86_64
       release: 3.10.0-1160.11.1.el7.x86_64
        system: Linux
       version: CentOS Linux 7 Core
```
</details>

**Additional context**
No problems in salt version 2019.2.8

Hints from issue discussion:
It looks like it is requiring the unix socket to be enabled, as the default is likely assuming you are trying to use this over the network, but if you are using it locally it wouldn't work as the docs indicate:
```
unix_socket
        If ``True`` and allow_passwordless is ``True``, the unix_socket auth
        plugin will be used.
```
It likely isn't accounting for your use case here to be used locally or something like that. It may be a MySQL issue that we cannot change it Salt, likely it was a change to MySQL.

But we can make the docs more clear since the config option is available.
@sagetherage 

I'm not sure how it is for more recent versions of Mysql/Mariadb (I'm running ancient stuff here) but in my situation this is a limitation enforced by salt. If I rip out all the unix_socket auth plugin references from the module everything works as before. 

I've quickly checked the mariadb docs and they don't mention any authentication plugin requirement for passwordless accounts, for either sockets or network. 

To me this is a regression, probably introduced here https://github.com/saltstack/salt/commit/e871a3ffd12e5499027cf66b9b7e1510036c7231 when the module was reworked.

I see two problems, we currently can't have:
- Passwordless localhost without unix_socket plugin
- Passwordless network accounts
@Emrvb you're correct, updated and @garethgreenaway is working on a fix for the next major release `Silicon`

Commit message: Merge branch 'master' into 59337_mysql_user_passwordless

Patch to review:
diff --git a/changelog/59337.fixed b/changelog/59337.fixed
new file mode 100644
index 000000000000..210359c8741f
--- /dev/null
+++ b/changelog/59337.fixed
@@ -0,0 +1 @@
+Reworking the mysql module and state so that passwordless does not try to use unix_socket until unix_socket is set to True. 
diff --git a/salt/modules/mysql.py b/salt/modules/mysql.py
index 7559c20cc0e7..68d3c67c836e 100644
--- a/salt/modules/mysql.py
+++ b/salt/modules/mysql.py
@@ -1585,21 +1585,7 @@ def _mysql_user_create(
     args = {}
     args["user"] = user
     args["host"] = host
-    if password is not None:
-        if salt.utils.versions.version_cmp(server_version, compare_version) >= 0:
-            args["auth_plugin"] = auth_plugin
-            qry += " IDENTIFIED WITH %(auth_plugin)s BY %(password)s"
-        else:
-            qry += " IDENTIFIED BY %(password)s"
-        args["password"] = str(password)
-    elif password_hash is not None:
-        if salt.utils.versions.version_cmp(server_version, compare_version) >= 0:
-            args["auth_plugin"] = auth_plugin
-            qry += " IDENTIFIED WITH %(auth_plugin)s AS %(password)s"
-        else:
-            qry += " IDENTIFIED BY PASSWORD %(password)s"
-        args["password"] = password_hash
-    elif salt.utils.data.is_true(allow_passwordless):
+    if unix_socket:
         if not plugin_status("auth_socket", **connection_args):
             err = "The auth_socket plugin is not enabled."
             log.error(err)
@@ -1612,12 +1598,33 @@ def _mysql_user_create(
                 else:
                     log.error("Auth via unix_socket can be set only for host=localhost")
     else:
-        log.error(
-            "password or password_hash must be specified, unless "
-            "allow_passwordless=True"
-        )
-        qry = False
-
+        if not salt.utils.data.is_true(allow_passwordless):
+            if password is not None:
+                if (
+                    salt.utils.versions.version_cmp(server_version, compare_version)
+                    >= 0
+                ):
+                    args["auth_plugin"] = auth_plugin
+                    qry += " IDENTIFIED WITH %(auth_plugin)s BY %(password)s"
+                else:
+                    qry += " IDENTIFIED BY %(password)s"
+                args["password"] = str(password)
+            elif password_hash is not None:
+                if (
+                    salt.utils.versions.version_cmp(server_version, compare_version)
+                    >= 0
+                ):
+                    args["auth_plugin"] = auth_plugin
+                    qry += " IDENTIFIED WITH %(auth_plugin)s AS %(password)s"
+                else:
+                    qry += " IDENTIFIED BY PASSWORD %(password)s"
+                args["password"] = password_hash
+            else:
+                log.error(
+                    "password or password_hash must be specified, unless "
+                    "allow_passwordless=True"
+                )
+                qry = False
     return qry, args
 
 
@@ -1637,13 +1644,7 @@ def _mariadb_user_create(
     args = {}
     args["user"] = user
     args["host"] = host
-    if password is not None:
-        qry += " IDENTIFIED BY %(password)s"
-        args["password"] = str(password)
-    elif password_hash is not None:
-        qry += " IDENTIFIED BY PASSWORD %(password)s"
-        args["password"] = password_hash
-    elif salt.utils.data.is_true(allow_passwordless):
+    if unix_socket:
         if not plugin_status("unix_socket", **connection_args):
             err = "The unix_socket plugin is not enabled."
             log.error(err)
@@ -1656,12 +1657,19 @@ def _mariadb_user_create(
                 else:
                     log.error("Auth via unix_socket can be set only for host=localhost")
     else:
-        log.error(
-            "password or password_hash must be specified, unless "
-            "allow_passwordless=True"
-        )
-        qry = False
-
+        if not salt.utils.data.is_true(allow_passwordless):
+            if password is not None:
+                qry += " IDENTIFIED BY %(password)s"
+                args["password"] = str(password)
+            elif password_hash is not None:
+                qry += " IDENTIFIED BY PASSWORD %(password)s"
+                args["password"] = password_hash
+            else:
+                log.error(
+                    "password or password_hash must be specified, unless "
+                    "allow_passwordless=True"
+                )
+                qry = False
     return qry, args
 
 
diff --git a/salt/states/mysql_user.py b/salt/states/mysql_user.py
index 265ef67e6aa1..61f2caa936a7 100644
--- a/salt/states/mysql_user.py
+++ b/salt/states/mysql_user.py
@@ -124,7 +124,7 @@ def present(
 
     # check if user exists with the same password (or passwordless login)
     if passwordless:
-        if not salt.utils.data.is_true(allow_passwordless):
+        if not salt.utils.data.is_true(allow_passwordless) and not unix_socket:
             ret["comment"] = (
                 "Either password or password_hash must be "
                 "specified, unless allow_passwordless is True"
@@ -140,7 +140,8 @@ def present(
                 password_column=password_column,
                 **connection_args
             ):
-                ret["comment"] += " with passwordless login"
+                if allow_passwordless:
+                    ret["comment"] += " with passwordless login"
                 return ret
             else:
                 err = _get_mysql_error()
@@ -228,11 +229,13 @@ def present(
         if __opts__["test"]:
             ret["comment"] = "User {}@{} is set to be added".format(name, host)
             ret["result"] = None
-            if passwordless:
+            if allow_passwordless:
                 ret["comment"] += " with passwordless login"
                 if not salt.utils.data.is_true(allow_passwordless):
                     ret["comment"] += ", but allow_passwordless != True"
                     ret["result"] = False
+            if unix_socket:
+                ret["comment"] += " using unix_socket"
             return ret
 
         if __salt__["mysql.user_create"](
@@ -247,8 +250,10 @@ def present(
             **connection_args
         ):
             ret["comment"] = "The user {}@{} has been added".format(name, host)
-            if passwordless:
+            if allow_passwordless:
                 ret["comment"] += " with passwordless login"
+            if unix_socket:
+                ret["comment"] += " using unix_socket"
             ret["changes"][name] = "Present"
         else:
             ret["comment"] = "Failed to create user {}@{}".format(name, host)
diff --git a/tests/pytests/integration/states/test_mysql.py b/tests/pytests/integration/states/test_mysql.py
index 603fdeb38de9..2ad4ac4cd8b6 100644
--- a/tests/pytests/integration/states/test_mysql.py
+++ b/tests/pytests/integration/states/test_mysql.py
@@ -168,3 +168,93 @@ def test_user_present_absent(salt_cli_wrapper):
 
     assert "comment" in state
     assert state["comment"] == "User george@localhost has been removed"
+
+
+def test_user_present_absent_passwordless(salt_cli_wrapper):
+
+    ret = salt_cli_wrapper(
+        "state.single",
+        "mysql_user.present",
+        name="george",
+        host="localhost",
+        allow_passwordless=True,
+    )
+    state = ret.json["mysql_user_|-george_|-george_|-present"]
+    assert ret.exitcode == 0, ret
+
+    assert "changes" in state
+    assert state["changes"] == {"george": "Present"}
+
+    assert "comment" in state
+    log.debug("=== comment %s ===", state["comment"])
+    assert (
+        state["comment"]
+        == "The user george@localhost has been added with passwordless login"
+    )
+
+    ret = salt_cli_wrapper(
+        "state.single", "mysql_user.absent", name="george", host="localhost",
+    )
+    state = ret.json["mysql_user_|-george_|-george_|-absent"]
+    assert ret.exitcode == 0, ret
+
+    assert "changes" in state
+    assert state["changes"] == {"george": "Absent"}
+
+    assert "comment" in state
+    assert state["comment"] == "User george@localhost has been removed"
+
+
+def test_user_present_absent_unixsocket(salt_cli_wrapper, mysql_container):
+
+    # The auth_socket plugin on MariaDB is unavailable
+    # on versions 10.1 - 10.3
+    if "mariadb" in mysql_container.mysql_name:
+        if mysql_container.mysql_version in ("10.1", "10.2", "10.3"):
+            pytest.skip(
+                "The auth_socket plugin is unavaiable "
+                "for the {}:{} docker image.".format(
+                    mysql_container.mysql_name, mysql_container.mysql_version
+                )
+            )
+
+    # enable the auth_socket plugin on MySQL
+    # already enabled on MariaDB > 10.3
+    if "mariadb" not in mysql_container.mysql_name:
+        ret = salt_cli_wrapper("mysql.plugin_add", "auth_socket")
+        assert ret.json
+
+    ret = salt_cli_wrapper(
+        "state.single",
+        "mysql_user.present",
+        name="george",
+        host="localhost",
+        unix_socket=True,
+        allow_passwordless=False,
+    )
+    state = ret.json["mysql_user_|-george_|-george_|-present"]
+    assert ret.exitcode == 0, ret
+
+    assert "changes" in state
+    assert state["changes"] == {"george": "Present"}
+
+    assert "comment" in state
+    assert (
+        state["comment"] == "The user george@localhost has been added using unix_socket"
+    )
+
+    ret = salt_cli_wrapper(
+        "state.single", "mysql_user.absent", name="george", host="localhost",
+    )
+    state = ret.json["mysql_user_|-george_|-george_|-absent"]
+    assert ret.exitcode == 0, ret
+
+    assert "changes" in state
+    assert state["changes"] == {"george": "Absent"}
+
+    assert "comment" in state
+    assert state["comment"] == "User george@localhost has been removed"
+
+    if "mariadb" not in mysql_container.mysql_name:
+        ret = salt_cli_wrapper("mysql.plugin_remove", "auth_socket")
+        assert ret.json
diff --git a/tests/pytests/unit/states/mysql/test_user.py b/tests/pytests/unit/states/mysql/test_user.py
index a6c4ea427e32..709796d8f695 100644
--- a/tests/pytests/unit/states/mysql/test_user.py
+++ b/tests/pytests/unit/states/mysql/test_user.py
@@ -58,7 +58,7 @@ def test_present():
         with patch.object(salt.utils.data, "is_true", mock_t):
             comt = "User frank@localhost is already present with passwordless login"
             ret.update({"comment": comt, "result": True})
-            assert mysql_user.present(name) == ret
+            assert mysql_user.present(name, allow_passwordless=True) == ret
 
             with patch.object(mysql_user, "_get_mysql_error", mock_str):
                 ret.update({"comment": "salt", "result": False})


Code review comments:
diff_hunk: @@ -1612,12 +1598,33 @@ def _mysql_user_create(
                 else:
                     log.error("Auth via unix_socket can be set only for host=localhost")
path: salt/modules/mysql.py
text: @user1:
**question**: is this else necessary? Or, could it just be `elif host == "localhost"`? Also... what happens if unix_socket, but it's not `is_true`? Or is that even a possibility?

@author:
**praise** Good catch.  That secondary check on whether unix_socket is True isn't necessary and was a hold over from a previous approach.