emqx/emqx-auth-mysql

Having column name different from "password" causes error "ok"

mvmn opened this issue · 9 comments

mvmn commented

In my DB the column name for password is, say, my_column_name_for_password.
Configuring auth query like this:

auth.mysql.auth_query = select my_column_name_for_password from my_table where my_username_column = '%u' limit 1

results in this "wonderful" error in logs:

`2017-05-19 11:55:46.687 [error] <0.413.0>@emqttd_protocol:process:202 Client(paho89953546151409@127.0.0.1:63668): Username 'user here' login failed for {{case_clause,{ok,[<<"my_column_name_for_password">>],[[<<"password here">>]]}},[{emq_auth_mysql,check,3,[{file,"d:/emqtt/v2.1.2/emq-relx/deps/emq_auth_mysql/src/emq_auth_mysql.erl"},{line,42}]},{emqttd_access_control,auth,3,[{file,"d:/emqtt/v2.1.2/emq-relx/deps/emqttd/src/emqttd_access_control.erl"},{line,60}]},{emqttd_protocol,authenticate,2,[{file,"d:/emqtt/v2.1.2/emq-relx/deps/emqttd/src/emqttd_protocol.erl"},{line,491}]},{emqttd_protocol,process,2,[{file,"d:/emqtt/v2.1.2/emq-relx/deps/emqttd/src/emqttd_protocol.erl"},{line,182}]},{emqttd_client,received,2,[{file,"d:/emqtt/v2.1.2/emq-relx/deps/emqttd/src/emqttd_client.erl"},{line,311}]},{gen_server2,handle_msg,2,[{file,"d:/emqtt/v2.1.2/emq-relx/deps/emqttd/src/gen_server2.erl"},{line,1046}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,247}]}]}

It took me a while to figure out that I must change query to this:

auth.mysql.auth_query = select my_column_name_for_password as password from my_table where my_username_column = '%u' limit 1

Could you at least mention that in documentation? Thanks!

auth.mysql.auth_query = select my_column_name_for_password as password from my_table where my_username_column = '%u' limit 1
mvmn commented

That's what I wrote.

It took me a while to figure out that I must change query to this:
auth.mysql.auth_query = select my_column_name_for_password as password from my_table where my_username_column = '%u' limit 1

Could you at least mention that in documentation? Thanks!

mvmn commented

Dude, you need to update documentation for that.
Simply closing the issue as "invalid"? Not cool, dude, not cool at all.

@mvmn Is it a common sense to use 'select as'?

mvmn commented

No.

The documentation mentions nothing about field name "password" being mandatory, so there doesn't seem to be any reason to add "as password" there.

I'm not sure about Erlang, but in other programming languages it's very easy to just take column by index, so if query is supposed to return one string column - it's (column's) name can be ignored by code, and it's value can be just taken by index (first column of resulting row).

However, since in your code it is not the case, and column name is used instead - it should be mentioned in documentation that that particular column name is mandatory.

Also a clearer error message would be nice.

Although if you think of it, the fix to take column by index (take first column of row) would be even better. Just ignore the column name - why bother enforcing particular name? The query should return only one column anyway.

Hi @mvmn, Thanks for your suggestion. Please help review the latest README that we updated just now. Would you like to submit a PR for this issue if the documentation is still not enough?

mvmn commented

Sure.

I see you've added your_password_field as password to README, but then reverted it:
ad55f8d

It was fine that way I suppose, not sure why the revert was made.

mvmn commented

Made a PR
#58

@mvmn The PR #58 has been merged.