refactor(dashboard): rename 'password' field to 'pwdhash' (#5990)

* refactor(dashboard): rename 'password' field to 'pwdhash'

rename as it is not plaintext password stored in db

* refactor(emqx_dashboard): rename records

* test(emqx_dashboard_token): add test case to cover match specs
This commit is contained in:
Zaiming (Stone) Shi 2021-10-26 14:41:33 +02:00 committed by GitHub
parent 4a07d5e1f3
commit 4dbe3ccf71
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 124 additions and 71 deletions

View File

@ -14,20 +14,24 @@
%% limitations under the License.
%%--------------------------------------------------------------------
-record(mqtt_admin, {
-record(emqx_admin, {
username :: binary(),
password :: binary(),
pwdhash :: binary(),
tags :: list() | binary(),
role = undefined :: atom()
role = undefined :: atom(),
extra = [] :: term() %% not used so far, for future extension
}).
-record(mqtt_admin_jwt, {
-define(ADMIN, emqx_admin).
-record(emqx_admin_jwt, {
token :: binary(),
username :: binary(),
exptime :: integer()
exptime :: integer(),
extra = [] :: term() %% not used so far, fur future extension
}).
-type(mqtt_admin() :: #mqtt_admin{}).
-define(ADMIN_JWT, emqx_admin_jwt).
-define(EMPTY_KEY(Key), ((Key == undefined) orelse (Key == <<>>))).

View File

@ -25,7 +25,6 @@
%% Mnesia bootstrap
-export([mnesia/1]).
%% mqtt_admin api
-export([ add_user/3
, force_add_user/3
, remove_user/1
@ -44,17 +43,19 @@
-export([add_default_user/0]).
-type emqx_admin() :: #?ADMIN{}.
%%--------------------------------------------------------------------
%% Mnesia bootstrap
%%--------------------------------------------------------------------
mnesia(boot) ->
ok = mria:create_table(mqtt_admin, [
ok = mria:create_table(?ADMIN, [
{type, set},
{rlog_shard, ?DASHBOARD_SHARD},
{storage, disc_copies},
{record_name, mqtt_admin},
{attributes, record_info(fields, mqtt_admin)},
{record_name, ?ADMIN},
{attributes, record_info(fields, ?ADMIN)},
{storage_properties, [{ets, [{read_concurrency, true},
{write_concurrency, true}]}]}]).
@ -64,13 +65,14 @@ mnesia(boot) ->
-spec(add_user(binary(), binary(), binary()) -> ok | {error, any()}).
add_user(Username, Password, Tags) when is_binary(Username), is_binary(Password) ->
Admin = #mqtt_admin{username = Username, password = hash(Password), tags = Tags},
Admin = #?ADMIN{username = Username, pwdhash = hash(Password), tags = Tags},
return(mria:transaction(?DASHBOARD_SHARD, fun add_user_/1, [Admin])).
%% black-magic: force overwrite a user
force_add_user(Username, Password, Tags) ->
AddFun = fun() ->
mnesia:write(#mqtt_admin{username = Username,
password = Password,
mnesia:write(#?ADMIN{username = Username,
pwdhash = hash(Password),
tags = Tags})
end,
case mria:transaction(?DASHBOARD_SHARD, AddFun) of
@ -79,8 +81,8 @@ force_add_user(Username, Password, Tags) ->
end.
%% @private
add_user_(Admin = #mqtt_admin{username = Username}) ->
case mnesia:wread({mqtt_admin, Username}) of
add_user_(Admin = #?ADMIN{username = Username}) ->
case mnesia:wread({?ADMIN, Username}) of
[] -> mnesia:write(Admin);
[_] -> mnesia:abort(<<"Username Already Exist">>)
end.
@ -93,7 +95,7 @@ remove_user(Username) when is_binary(Username) ->
mnesia:abort(<<"Username Not Found">>);
_ -> ok
end,
mnesia:delete({mqtt_admin, Username})
mnesia:delete({?ADMIN, Username})
end,
return(mria:transaction(?DASHBOARD_SHARD, Trans)).
@ -103,9 +105,9 @@ update_user(Username, Tags) when is_binary(Username) ->
%% @private
update_user_(Username, Tags) ->
case mnesia:wread({mqtt_admin, Username}) of
case mnesia:wread({?ADMIN, Username}) of
[] -> mnesia:abort(<<"Username Not Found">>);
[Admin] -> mnesia:write(Admin#mqtt_admin{tags = Tags})
[Admin] -> mnesia:write(Admin#?ADMIN{tags = Tags})
end.
change_password(Username, OldPasswd, NewPasswd) when is_binary(Username) ->
@ -119,7 +121,7 @@ change_password(Username, Password) when is_binary(Username), is_binary(Password
change_password_hash(Username, PasswordHash) ->
update_pwd(Username, fun(User) ->
User#mqtt_admin{password = PasswordHash}
User#?ADMIN{pwdhash = PasswordHash}
end).
update_pwd(Username, Fun) ->
@ -135,14 +137,23 @@ update_pwd(Username, Fun) ->
return(mria:transaction(?DASHBOARD_SHARD, Trans)).
-spec(lookup_user(binary()) -> [mqtt_admin()]).
-spec(lookup_user(binary()) -> [emqx_admin()]).
lookup_user(Username) when is_binary(Username) ->
Fun = fun() -> mnesia:read(mqtt_admin, Username) end,
Fun = fun() -> mnesia:read(?ADMIN, Username) end,
{atomic, User} = mria:ro_transaction(?DASHBOARD_SHARD, Fun),
User.
-spec(all_users() -> [#mqtt_admin{}]).
all_users() -> ets:tab2list(mqtt_admin).
-spec(all_users() -> [map()]).
all_users() ->
lists:map(fun(#?ADMIN{username = Username,
tags = Tags
}) ->
#{username => Username,
%% named tag but not tags, for unknown reason
%% TODO: fix this comment
tag => Tags
}
end, ets:tab2list(?ADMIN)).
return({atomic, _}) ->
ok;
@ -150,18 +161,18 @@ return({aborted, Reason}) ->
{error, Reason}.
check(undefined, _) ->
{error, <<"Username undefined">>};
{error, <<"username_not_provided">>};
check(_, undefined) ->
{error, <<"Password undefined">>};
{error, <<"password_not_provided">>};
check(Username, Password) ->
case lookup_user(Username) of
[#mqtt_admin{password = <<Salt:4/binary, Hash/binary>>}] ->
[#?ADMIN{pwdhash = <<Salt:4/binary, Hash/binary>>}] ->
case Hash =:= md5_hash(Salt, Password) of
true -> ok;
false -> {error, <<"PASSWORD_ERROR">>}
false -> {error, <<"BAD_USERNAME_OR_PASSWORD">>}
end;
[] ->
{error, <<"USERNAME_ERROR">>}
{error, <<"BAD_USERNAME_OR_PASSWORD">>}
end.
%%--------------------------------------------------------------------
@ -179,7 +190,7 @@ verify_token(Token) ->
destroy_token_by_username(Username, Token) ->
case emqx_dashboard_token:lookup(Token) of
{ok, #mqtt_admin_jwt{username = Username}} ->
{ok, #?ADMIN_JWT{username = Username}} ->
emqx_dashboard_token:destroy(Token);
_ ->
{error, not_found}

View File

@ -188,7 +188,7 @@ logout(_, #{body := #{<<"username">> := Username},
end.
users(get, _Request) ->
{200, [row(User) || User <- emqx_dashboard_admin:all_users()]};
{200, emqx_dashboard_admin:all_users()};
users(post, #{body := Params}) ->
Tag = maps:get(<<"tag">>, Params),
@ -231,6 +231,3 @@ change_pwd(put, #{bindings := #{username := Username}, body := Params}) ->
{error, Reason} ->
{400, #{code => <<"CHANGE_PWD_FAIL">>, message => Reason}}
end.
row(#mqtt_admin{username = Username, tags = Tag}) ->
#{username => Username, tag => Tag}.

View File

@ -57,6 +57,10 @@ default_username(_) -> undefined.
default_password(type) -> string();
default_password(default) -> "public";
default_password(nullable) -> false;
default_password(sensitive) -> true;
default_password(desc) -> """
The initial default password for dashboard 'admin' user.
For safty, it should be changed as soon as possible.""";
default_password(_) -> undefined.
sc(Type, Meta) -> hoconsc:mk(Type, Meta).

View File

@ -18,8 +18,6 @@
-include("emqx_dashboard.hrl").
-define(TAB, mqtt_admin_jwt).
-export([ sign/2
, verify/1
, lookup/1
@ -31,9 +29,12 @@
-export([mnesia/1]).
-define(EXPTIME, 60 * 60 * 1000).
-ifdef(TEST).
-export([lookup_by_username/1, clean_expired_jwt/1]).
-endif.
-define(CLEAN_JWT_INTERVAL, 60 * 60 * 1000).
-define(TAB, ?ADMIN_JWT).
-define(EXPTIME, 60 * 60 * 1000).
%%--------------------------------------------------------------------
%% gen server part
@ -60,13 +61,12 @@ sign(Username, Password) ->
verify(Token) ->
do_verify(Token).
-spec(destroy(KeyOrKeys :: list() | binary() | #mqtt_admin_jwt{}) -> ok).
-spec(destroy(KeyOrKeys :: list() | binary() | #?ADMIN_JWT{}) -> ok).
destroy([]) ->
ok;
destroy(JWTorTokenList) when is_list(JWTorTokenList)->
[destroy(JWTorToken) || JWTorToken <- JWTorTokenList],
ok;
destroy(#mqtt_admin_jwt{token = Token}) ->
lists:foreach(fun destroy/1, JWTorTokenList);
destroy(#?ADMIN_JWT{token = Token}) ->
destroy(Token);
destroy(Token) when is_binary(Token)->
do_destroy(Token).
@ -80,8 +80,8 @@ mnesia(boot) ->
{type, set},
{rlog_shard, ?DASHBOARD_SHARD},
{storage, disc_copies},
{record_name, mqtt_admin_jwt},
{attributes, record_info(fields, mqtt_admin_jwt)},
{record_name, ?ADMIN_JWT},
{attributes, record_info(fields, ?ADMIN_JWT)},
{storage_properties, [{ets, [{read_concurrency, true},
{write_concurrency, true}]}]}]).
@ -106,10 +106,10 @@ do_sign(Username, Password) ->
do_verify(Token)->
case lookup(Token) of
{ok, JWT = #mqtt_admin_jwt{exptime = ExpTime}} ->
{ok, JWT = #?ADMIN_JWT{exptime = ExpTime}} ->
case ExpTime > erlang:system_time(millisecond) of
true ->
NewJWT = JWT#mqtt_admin_jwt{exptime = jwt_expiration_time()},
NewJWT = JWT#?ADMIN_JWT{exptime = jwt_expiration_time()},
{atomic, Res} = mria:transaction(?DASHBOARD_SHARD, fun mnesia:write/1, [NewJWT]),
Res;
_ ->
@ -129,7 +129,7 @@ do_destroy_by_username(Username) ->
%%--------------------------------------------------------------------
%% jwt internal util function
-spec(lookup(Token :: binary()) -> {ok, #mqtt_admin_jwt{}} | {error, not_found}).
-spec(lookup(Token :: binary()) -> {ok, #?ADMIN_JWT{}} | {error, not_found}).
lookup(Token) ->
Fun = fun() -> mnesia:read(?TAB, Token) end,
case mria:ro_transaction(?DASHBOARD_SHARD, Fun) of
@ -137,13 +137,13 @@ lookup(Token) ->
{atomic, []} -> {error, not_found}
end.
-dialyzer({nowarn_function, lookup_by_username/1}).
lookup_by_username(Username) ->
Spec = [{{mqtt_admin_jwt, '_', Username, '_'}, [], ['$_']}],
Spec = [{#?ADMIN_JWT{username = Username, _ = '_'}, [], ['$_']}],
Fun = fun() -> mnesia:select(?TAB, Spec) end,
{atomic, List} = mria:ro_transaction(?DASHBOARD_SHARD, Fun),
List.
jwk(Username, Password, Salt) ->
Key = erlang:md5(<<Salt/binary, Username/binary, Password/binary>>),
#{
@ -152,8 +152,10 @@ jwk(Username, Password, Salt) ->
}.
jwt_expiration_time() ->
ExpTime = emqx_conf:get([emqx_dashboard, token_expired_time], ?EXPTIME),
erlang:system_time(millisecond) + ExpTime.
erlang:system_time(millisecond) + token_ttl().
token_ttl() ->
emqx_conf:get([emqx_dashboard, token_expired_time], ?EXPTIME).
salt() ->
_ = emqx_misc:rand_seed(),
@ -161,7 +163,7 @@ salt() ->
<<Salt:32>>.
format(Token, Username, ExpTime) ->
#mqtt_admin_jwt{
#?ADMIN_JWT{
token = Token,
username = Username,
exptime = ExpTime
@ -189,10 +191,7 @@ handle_cast(_Request, State) ->
handle_info(clean_jwt, State) ->
timer_clean(self()),
Now = erlang:system_time(millisecond),
Spec = [{{mqtt_admin_jwt, '_', '_', '$1'}, [{'<', '$1', Now}], ['$_']}],
{atomic, JWTList} = mria:ro_transaction(?DASHBOARD_SHARD,
fun() -> mnesia:select(?TAB, Spec) end),
destroy(JWTList),
ok = clean_expired_jwt(Now),
{noreply, State};
handle_info(_Info, State) ->
{noreply, State}.
@ -204,4 +203,12 @@ code_change(_OldVsn, State, _Extra) ->
{ok, State}.
timer_clean(Pid) ->
erlang:send_after(?CLEAN_JWT_INTERVAL, Pid, clean_jwt).
erlang:send_after(token_ttl(), Pid, clean_jwt).
-dialyzer({nowarn_function, clean_expired_jwt/1}).
clean_expired_jwt(Now) ->
Spec = [{#?ADMIN_JWT{exptime = '$1', token = '$2', _ = '_'},
[{'<', '$1', Now}], ['$2']}],
{atomic, JWTList} = mria:ro_transaction(?DASHBOARD_SHARD,
fun() -> mnesia:select(?TAB, Spec) end),
ok = destroy(JWTList).

View File

@ -26,8 +26,8 @@
]).
-include_lib("eunit/include/eunit.hrl").
-include_lib("emqx/include/emqx.hrl").
-include("emqx_dashboard.hrl").
-define(CONTENT_TYPE, "application/x-www-form-urlencoded").
@ -41,8 +41,8 @@
all() ->
%% TODO: V5 API
%% emqx_common_test_helpers:all(?MODULE).
[].
% emqx_common_test_helpers:all(?MODULE).
[t_cli, t_lookup_by_username_jwt, t_clean_expired_jwt].
init_per_suite(Config) ->
emqx_common_test_helpers:start_apps([emqx_management, emqx_dashboard],fun set_special_configs/1),
@ -60,12 +60,12 @@ set_special_configs(_) ->
ok.
t_overview(_) ->
mnesia:clear_table(mqtt_admin),
mnesia:clear_table(?ADMIN),
emqx_dashboard_admin:add_user(<<"admin">>, <<"public">>, <<"tag">>),
[?assert(request_dashboard(get, api_path(erlang:atom_to_list(Overview)), auth_header_()))|| Overview <- ?OVERVIEWS].
t_admins_add_delete(_) ->
mnesia:clear_table(mqtt_admin),
mnesia:clear_table(?ADMIN),
ok = emqx_dashboard_admin:add_user(<<"username">>, <<"password">>, <<"tag">>),
ok = emqx_dashboard_admin:add_user(<<"username1">>, <<"password1">>, <<"tag1">>),
Admins = emqx_dashboard_admin:all_users(),
@ -81,7 +81,7 @@ t_admins_add_delete(_) ->
?assertNotEqual(true, request_dashboard(get, api_path("brokers"), auth_header_("username", "pwd"))).
t_rest_api(_Config) ->
mnesia:clear_table(mqtt_admin),
mnesia:clear_table(?ADMIN),
emqx_dashboard_admin:add_user(<<"admin">>, <<"public">>, <<"administrator">>),
{ok, Res0} = http_get("users"),
@ -102,13 +102,13 @@ t_rest_api(_Config) ->
ok.
t_cli(_Config) ->
[mria:dirty_delete(mqtt_admin, Admin) || Admin <- mnesia:dirty_all_keys(mqtt_admin)],
[mria:dirty_delete(?ADMIN, Admin) || Admin <- mnesia:dirty_all_keys(?ADMIN)],
emqx_dashboard_cli:admins(["add", "username", "password"]),
[{mqtt_admin, <<"username">>, <<Salt:4/binary, Hash/binary>>, _}] =
[#?ADMIN{ username = <<"username">>, pwdhash = <<Salt:4/binary, Hash/binary>>}] =
emqx_dashboard_admin:lookup_user(<<"username">>),
?assertEqual(Hash, erlang:md5(<<Salt/binary, <<"password">>/binary>>)),
emqx_dashboard_cli:admins(["passwd", "username", "newpassword"]),
[{mqtt_admin, <<"username">>, <<Salt1:4/binary, Hash1/binary>>, _}] =
[#?ADMIN{username = <<"username">>, pwdhash = <<Salt1:4/binary, Hash1/binary>>}] =
emqx_dashboard_admin:lookup_user(<<"username">>),
?assertEqual(Hash1, erlang:md5(<<Salt1/binary, <<"newpassword">>/binary>>)),
emqx_dashboard_cli:admins(["del", "username"]),
@ -118,10 +118,40 @@ t_cli(_Config) ->
AdminList = emqx_dashboard_admin:all_users(),
?assertEqual(2, length(AdminList)).
t_lookup_by_username_jwt(_Config) ->
User = bin(["user-", integer_to_list(random_num())]),
Pwd = bin(integer_to_list(random_num())),
emqx_dashboard_token:sign(User, Pwd),
?assertMatch([#?ADMIN_JWT{username = User}],
emqx_dashboard_token:lookup_by_username(User)),
ok = emqx_dashboard_token:destroy_by_username(User),
%% issue a gen_server call to sync the async destroy gen_server cast
ok = gen_server:call(emqx_dashboard_token, dummy, infinity),
?assertMatch([], emqx_dashboard_token:lookup_by_username(User)),
ok.
t_clean_expired_jwt(_Config) ->
User = bin(["user-", integer_to_list(random_num())]),
Pwd = bin(integer_to_list(random_num())),
emqx_dashboard_token:sign(User, Pwd),
[#?ADMIN_JWT{username = User, exptime = ExpTime}] =
emqx_dashboard_token:lookup_by_username(User),
ok = emqx_dashboard_token:clean_expired_jwt(_Now1 = ExpTime),
?assertMatch([#?ADMIN_JWT{username = User}],
emqx_dashboard_token:lookup_by_username(User)),
ok = emqx_dashboard_token:clean_expired_jwt(_Now2 = ExpTime + 1),
?assertMatch([], emqx_dashboard_token:lookup_by_username(User)),
ok.
%%------------------------------------------------------------------------------
%% Internal functions
%%------------------------------------------------------------------------------
bin(X) -> iolist_to_binary(X).
random_num() ->
erlang:system_time(nanosecond).
http_get(Path) ->
request_api(get, api_path(Path), auth_header_()).

View File

@ -7,7 +7,7 @@ cd -P -- "$(dirname -- "$0")/.."
find_app() {
local appdir="$1"
find "${appdir}" -mindepth 1 -maxdepth 1 -type d | grep -vE "emqx_dashboard"
find "${appdir}" -mindepth 1 -maxdepth 1 -type d
}
find_app 'apps'