Mark AllowNativePassword in DSN param string be true by default.

[elemount]

Description

This is the change about issue #642 (Is it possible to set the default value of "allowNativePasswords" to "true"?)
I see this AllowNativePassword param is introduced by @twocode in #494 , hi @twocode why you create this parameter and set the default value is true? Any security reason or others?
I think set the we should not have the allowNativePasswords param, so could we set it true?

Checklist

• Code compiles correctly
• Created tests which fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary
• Added myself / the copyright holder to the AUTHORS file

[twocode]

Hey @elemount , setting allowNativePasswords to false by default was basically following similar syntax as allowOldPasswords and allowClearTextPasswords. However, native authentication is the default auth method and safer than old password or clear text; it should be set to true by default. Besides your code changes made in this PR, I think we should by default allow authentication switch with native password authentication in this code:

mysql/driver.go

Line 165 in 5926162

} else if mc.cfg.AllowNativePasswords && err == ErrNativePassword {

@julienschmidt , Hi Julien, what do you think?

Thanks,
Xiangyu

[bingosummer]

@julienschmidt @methane Could you please have a review?

[julienschmidt]

Generally, this change makes sense to me.
If no one else of the team has any objections, I think it can and should be allowed by default.

[julienschmidt]

Only the effect of allowNativePasswords=false has to be documented, as true is the default value and setting it manually wouldn't make sense as it doesn't change anything.

[elemount]

Sure.

[methane]

LGTM

[julienschmidt]

Might be nit-picking, but "during auth switch process" is an internal detail. The average driver user has no idea what is meant by this. Just remove it.
And if you push a change, please also change "mysql" to "MySQL" to keep it consistent within the README

[elemount]

Hi @julienschmidt , I've updated it.

[julienschmidt]

Thanks for your first contribution!

[wayneashleyberry]

I just updated to 1.4 and started getting native password errors:

[mysql] 2018/06/04 08:50:00 driver.go:120: could not use requested auth plugin 'mysql_native_password': this user requires mysql native password authentication.
panic: could not ping database: this user requires mysql native password authentication.

When I checked the DSN I noticed that allowNativePasswords was being set to false

root:root@tcp(127.0.0.1:3306)/my_db?allowNativePasswords=false&parseTime=true&maxAllowedPacket=0&charset=utf8mb4

I had to add AllowNativePasswords: true, to my mysql.Config struct, but that seems like the opposite of what this PR added, or am I missing something?

[julienschmidt]

@wayneashleyberry are you creating a new mysql.Config without using NewConfig() (which sets defaults such as AllowNativePasswords: true)?

[wayneashleyberry]

Ah, that's exactly what I'm doing @julienschmidt. Didn't know about NewConfig, will check it out — thanks!

[julienschmidt]

When the DSN is formatted there is no difference between the zero value for AllowNativePasswords (false) and explicitly setting it to false.

What is new in v1.4.0 is that these settings are checked correctly. Before there were situations where mysql_native_password could be used even though AllowNativePasswords was set to false.

[wayneashleyberry]

Yeah, makes sense @julienschmidt — just didn't expect the change in behaviour.

I have learnt something new though, using NewConfig everywhere new :) Thanks for taking the time to explain the change 🙇