Skip to content
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

Plugin based authentication support #331

Merged
merged 40 commits into from
Jun 29, 2016
Merged

Plugin based authentication support #331

merged 40 commits into from
Jun 29, 2016

Conversation

sidorares
Copy link
Owner

Packets documentation:

http://dev.mysql.com/doc/internals/en/connection-phase-packets.html#packet-Protocol::AuthSwitchRequest

Protocol examples:

http://dev.mysql.com/doc/internals/en/auth-method-switch.html

related node-mysql discussion: mysqljs/mysql#1396

/cc @twocode

API:

authSwitchHandler connection option. Function, accepts data as parameter and callback to pass (err, authSwitchResponseData).Note that it can be invoked multiple times if server respond with AuthMoreData. At first invocation data.pluginName contains name of auth plugin, data.pluginData - buffer with data. Next invocations contain only data as data.pluginData.

  var conn = mysql.createConnection({
    user: 'test_user',
    password: 'test',
    database: 'test_database',
    port: port,
    authSwitchHandler: function(data, cb) {
      cb(null, response) // response: string or buffer
    }
  });

@sidorares sidorares merged commit 4652f98 into master Jun 29, 2016
@sidorares sidorares deleted the auth-switch branch June 29, 2016 07:18
@twocode
Copy link

twocode commented Jul 6, 2016

Hey @sidorares

This is fantastic. I have noticed that mysql2 can now adapt mysql authentication-switch method. Bravo!

But it seems to be not correct when the authentication switch request is "mysql_native_password", could you help to verify?

Authentication switch request (plugin method: mysql_native_password, salt):

image

Authentication switch response (0-length payload):

image

@sidorares
Copy link
Owner Author

Yeah, looks strange

Could you connect client with debug: true and post debug log?

The client supposed to send response to mysql_native_password here:

connection.writePacket(new Packets.AuthSwitchResponse(authToken).toPacket());

could you also log authToken from l133? If it's empty for some reason the resulting packet would be empty

@twocode
Copy link

twocode commented Jul 7, 2016

Hey @sidorares

I digged a little and it turned out that in line:

authToken = auth41.calculateToken(this.password, authPluginData1, authPluginData2);

this.password is undefined, thus underneath it will return a new Buffer(0)

If I hard-code a valid password string here, it will work correct.

Thanks

@sidorares
Copy link
Owner Author

Thanks! I'll have a look, should be easy to fix

@twocode
Copy link

twocode commented Jul 22, 2016

@sidorares Hey man, any updates? :p

Do we need to post an issue to track this?

Best

@sidorares
Copy link
Owner Author

@twocode hi! The fix is actually very simple, just did not have time to implement

The problem is following: When you have ChangeUser request the same AuthSwitch is performed as with initial ClientHandshake command. I'm mixing in this bit of functionality here:

ChangeUser.prototype.handshakeResult = ClientHandshake.prototype.handshakeResult;
but I think ChangeUser.prototype.handshakeResult expects password/username fields differently

If you able to debug that would be great, otherwise wait few more days (or weeks, too busy :( )

@sidorares
Copy link
Owner Author

yes, issue with detailed documentation and debug log would be helpful as well ( just to confirm that it's ChangeUser command etc )

@sidorares
Copy link
Owner Author

Hi @twocode , could you try head of https://github.com/sidorares/node-mysql2/tree/auth-switch2 branch?

@twocode
Copy link

twocode commented Jul 28, 2016

@sidorares Cool man, let me give a try. Will keep you updated!

@twocode
Copy link

twocode commented Aug 1, 2016

@sidorares Hey dude, this fix has been verified. Cheers!

@sidorares
Copy link
Owner Author

sidorares commented Aug 1, 2016

hi @twocode as I was not able to replicate exactly your behaviour, just for the record - what's your server version?

@twocode
Copy link

twocode commented Aug 1, 2016

@sidorares I was using 5.6.26.

By the way, are we having this fix in rc8? Will we have this in node-MySQL as well?

@sidorares
Copy link
Owner Author

Yes, I'll try to release another rc today and hope to have final 1.0 this week

I'm not sure about node-MySQL timeline and priorities. If everyone is happy about AuthSwitch api I might try to implement them there but afaik there are more important features on todo list there

@twocode
Copy link

twocode commented Aug 8, 2016

Hey @sidorares , I understand. No worries. Thanks so much:)

@sidorares
Copy link
Owner Author

@twocode the fix for your issue was published with rc-9

@twocode
Copy link

twocode commented Aug 10, 2016

@sidorares Gotcha!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants