-
-
Notifications
You must be signed in to change notification settings - Fork 638
Add mysql clear password auth #1552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add mysql clear password auth #1552
Conversation
There is a fix for failing tests in b96364e - either cherry-pick into your branch or wait for me to merge fix into master and rebase on top |
…en server versions
Bumps [minimist](https://github.com/substack/minimist) from 1.2.5 to 1.2.6. - [Release notes](https://github.com/substack/minimist/releases) - [Commits](https://github.com/substack/minimist/compare/1.2.5...1.2.6) --- updated-dependencies: - dependency-name: minimist dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]>
added tests and fixes for sidorares#1546
…arn/minimist-1.2.6 Bump minimist from 1.2.5 to 1.2.6
hey @silverbullettruck2001 do you know what's the easiest way to test this? results in |
@sidorares can you try this instead
|
@sidorares did you make any progress with getting the tests for this working? |
@silverbullettruck2001 your docker command stoped with the error |
@sidorares I did some digging and some testing. What I found out was this. Based on the mysql documentation there is no server side plugin that needs to be loaded for this to work. It's simply a way to invoke the client side connection so that it sends the password in clear text. It's up to the server side plugin (whichever one you pick) to be able to consume the password in clear text. The environment variable that you are setting only specifies that all client connections will use clear text. So I used the below command to start my container:
Then you just need to specify the
Afterwards the connection is established (by sending the password in clear text) you can create a user like you would any other user. You do not have to specify the Here's the documentation I found on this (in paragraph 3): |
@sidorares have you considered using test containers for these types of tests? - https://github.com/testcontainers/testcontainers-node I use them in Beekeeper Studio to do integration tests - defining the whole thing in code, runs great on github actions. Here's the MySQL example from my codebase: I know right now you use Docker and have an action per-database type, not sure if this would simplify that work, but it does make it easy to do stuff like looping through several variations of a database with different flags and running the same tests. Like for PSQL I run several versions with a simple loop |
interesting @rathboma, I never tried that. How does it work under the hood and what is the difference with "docker run mysql" like we have in ci workflow?
|
@silverbullettruck2001 I'll need to investigate what mysql cli is doing when In order to test above flow I need to add this functionality ( default plugin to be used initially ) |
@sidorares mostly it allows for easier scripting of containers as part of the javascript, and provides an easy api for doing stuff like checking if the container is alive before running tests. In development it can also keep containers around for a little while automatically so repeated runs of tests are faster. You already have it set-up with CI, so certainly not urgent, but worth considering if you ever do a big rewrite! |
@sidorares is there anything I can do to help with this? I am limited but would be willing to discuss on Slack or discord or whatever to keep this moving forward so I can make sure this gets incorporated so I can continue using this tool. |
@sidorares Just following up on this. Is there any way to finalize this? |
Notes for me on how to configure mysql server to use cleartext auth plugin:
INSTALL PLUGIN cleartext_plugin_server SONAME 'auth_test_plugin.so'
CREATE USER uplain@localhost IDENTIFIED WITH 'cleartext_plugin_server' AS 'cleartext_test' @silverbullettruck2001 @rathboma I still struggle to connect after that point. Did you have a chance to connect to a real ( or mock ) mysql server using cleartext plugin? What are your steps? For some reason mysql2 client ( |
@sidorares In my specific use case, I am connecting to an AWS RDS instance that uses MySQL and IAM authentication. I provided links below that show how to set this up on AWS, if that helps. Also, the way I have interpreted the |
Sure. have a look at my last comment in #1403 |
@tkchk could you provide some steps on how you configured your mysql database and user to verify that it authenticated correctly? |
@silverbullettruck2001 So, the setup is a bit tricky.
This instruction may be incomplete though. |
I'm very tempted to merge this PR without any integration or unit tests but might give it another try over the weekend
|
@sidorares I think the risk of doing this is relatively low. I support you merging this. |
its more about accidentally breaking this feature by some other future change if its not covered by tests |
@sidorares any further thoughts on how to get this merged? |
@silverbullettruck2001 merged. I'll track e2e test via real docker server or mock server separately I'll try to setup https://github.com/googleapis/release-please here some time later today and hopefully will have releases more often |
Adding upon @rathboma's changes to implement
mysql_clear_password
auth plugin