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

apps/emqx_dashboard/include/emqx_dashboard.hrl

@@ -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 == <<>>))).
 

apps/emqx_dashboard/src/emqx_dashboard_admin.erl

@@ -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},
+                {record_name, ?ADMIN},
-                {attributes, record_info(fields, mqtt_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,
+                 mnesia:write(#?ADMIN{username = Username,
-                                          password = Password,
+                                      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}) ->
+add_user_(Admin = #?ADMIN{username = Username}) ->
-    case mnesia:wread({mqtt_admin, Username}) of
+    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{}]).
+-spec(all_users() -> [map()]).
-all_users() -> ets:tab2list(mqtt_admin).
+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}

apps/emqx_dashboard/src/emqx_dashboard_api.erl

@@ -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}.

apps/emqx_dashboard/src/emqx_dashboard_schema.erl

@@ -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).

apps/emqx_dashboard/src/emqx_dashboard_token.erl

@@ -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],
+    lists:foreach(fun destroy/1, JWTorTokenList);
-    ok;
+destroy(#?ADMIN_JWT{token = Token}) ->
-destroy(#mqtt_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},
+                {record_name, ?ADMIN_JWT},
-                {attributes, record_info(fields, mqtt_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) + token_ttl().
-    erlang:system_time(millisecond) + ExpTime.
+
+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}], ['$_']}],
+    ok = clean_expired_jwt(Now),
-    {atomic, JWTList} = mria:ro_transaction(?DASHBOARD_SHARD,
-        fun() -> mnesia:select(?TAB, Spec) end),
-    destroy(JWTList),
     {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).

apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl

@@ -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_()).
 

scripts/find-apps.sh

@@ -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'