Merge pull request #9696 from zhongwencool/dashboard-password-validate-v44

fix: update dashboard password validator

changes/v4.4.14-en.md

@@ -1,11 +1,11 @@
 # v4.4.14
 
 ## Enhancements
-- Add a password complexity requirement when adding or modifying Dashboard users via the API. Now passwords must contain at least 2 of alphabetic, numeric and special characters, and must be 8 to 64 characters long [#1696](https://github.com/emqx/emqx-enterprise/pull/1696).
+- Add a password complexity requirement when adding or modifying Dashboard users via the API. Now passwords must contain at least 2 of alphabetic, numeric and special characters, and must be 8 to 64 characters long [#9696](https://github.com/emqx/emqx/pull/9696).
 
 ## Bug fixes
 
-- Fix dashboard password validator is too simple. Now dashboard password must contain at least two different kind of characters from groups of letters, numbers and special characters. [#1696](https://github.com/emqx/emqx-enterprise/pull/1696).
+- Fix dashboard password validator is too simple. Now dashboard password must contain at least two different kind of characters from groups of letters, numbers and special characters. [#9696](https://github.com/emqx/emqx/pull/9696).
-- Fix the problem that adding or importing Dashboard users via the API fails to add complex passwords due to incorrect checksum of the passwords [#1696](https://github.com/emqx/emqx-enterprise/pull/1696).
+- Fix the problem that adding or importing Dashboard users via the API fails to add complex passwords due to incorrect checksum of the passwords [#9692](https://github.com/emqx/emqx/pull/9692).
 
-- Fix load bootstrap_app_file's apps is not sync when reboot. [#1697](https://github.com/emqx/emqx-enterprise/pull/1697).
+- Fix load bootstrap_app_file's apps is not sync when reboot. [#9692](https://github.com/emqx/emqx/pull/9692).

changes/v4.4.14-zh.md

@@ -2,10 +2,10 @@
 
 ## 增强
 
-- 通过 API 添加、修改 Dashboard 用户时，增加对密码复杂度的要求。现在密码必须包含字母、数字以及特殊字符中的至少 2 种，并且长度范围必须是 8~64 个字符 [#1696](https://github.com/emqx/emqx-enterprise/pull/1696)。
+- 通过 API 添加、修改 Dashboard 用户时，增加对密码复杂度的要求。现在密码必须包含字母、数字以及特殊字符中的至少 2 种，并且长度范围必须是 8~64 个字符 [#9696](https://github.com/emqx/emqx/pull/9696)。
 
 ## 修复
 
-- 修复通过 API 添加或者导入 Dashboard 用户时，对密码进行了错误的校验，导致复杂密码添加失败的问题 [#1696](https://github.com/emqx/emqx-enterprise/pull/1696)。
+- 修复通过 API 添加或者导入 Dashboard 用户时，对密码进行了错误的校验，导致复杂密码添加失败的问题 [#9696](https://github.com/emqx/emqx/pull/9696)。
 
-- 修复 boostrap_apps_file 文件更新后没有同步更新至数据库导致文件中新加 apps 未生效 [#1697](https://github.com/emqx/emqx-enterprise/pull/1697)。
+- 修复 boostrap_apps_file 文件更新后未同步至数据库，导致变更的 apps 无法生效 [#9692](https://github.com/emqx/emqx/pull/9692)。

lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl

@@ -23,6 +23,7 @@
 -include("emqx_dashboard.hrl").
 -include_lib("emqx/include/logger.hrl").
 -define(DEFAULT_PASSWORD, <<"public">>).
+-define(INVALID_PASSWORD_MSG, <<"The password must contain at least two different kind of characters from groups of letters, numbers, and special characters. For example, if password is composed from letters, it must contain at least one number or a special character.">>).
 
 -boot_mnesia({mnesia, [boot]}).
 -copy_mnesia({mnesia, [copy]}).
@@ -41,8 +42,10 @@
         , lookup_user/1
         , change_password/2
         , change_password/3
+        , force_change_password/2
         , all_users/0
         , check/2
+        , hash/1
         ]).
 
 %% gen_server Function Exports
@@ -79,7 +82,7 @@ start_link() ->
 
 -spec(add_user(binary(), binary(), binary()) -> ok | {error, any()}).
 add_user(Username, Password, Tags) when is_binary(Username), is_binary(Password) ->
-    case {emqx_misc:is_sane_id(Username), emqx_misc:is_sane_id(Password, 2, 32)} of
+    case {emqx_misc:is_sane_id(Username), is_valid_pwd(Password)} of
         {ok, ok} ->
             Admin = #mqtt_admin{username = Username, password = hash(Password), tags = Tags},
             return(mnesia:transaction(fun add_user_/1, [Admin]));
@@ -88,17 +91,12 @@ add_user(Username, Password, Tags) when is_binary(Username), is_binary(Password)
     end.
 
 force_add_user(Username, Password, Tags) ->
-    case {emqx_misc:is_sane_id(Username), emqx_misc:is_sane_id(Password, 2, 32)} of
+    AddFun = fun() ->
-        {ok, ok} ->
+        mnesia:write(#mqtt_admin{username = Username, password = Password, tags = Tags})
-            AddFun = fun() ->
+             end,
-                mnesia:write(#mqtt_admin{username = Username, password = Password, tags = Tags})
+    case mnesia:transaction(AddFun) of
-                     end,
+        {atomic, ok} -> ok;
-            case mnesia:transaction(AddFun) of
+        {aborted, Reason} -> {error, Reason}
-                {atomic, ok} -> ok;
-                {aborted, Reason} -> {error, Reason}
-            end;
-        {{error, Reason}, _} -> {error, Reason};
-        {_, {error, Reason}} -> {error, Reason}
     end.
 
 %% @private
@@ -138,6 +136,12 @@ change_password(Username, OldPasswd, NewPasswd) when is_binary(Username) ->
     end.
 
 change_password(Username, Password) when is_binary(Username), is_binary(Password) ->
+    case is_valid_pwd(Password) of
+        ok -> change_password_hash(Username, hash(Password));
+        {error, Error} -> {error, Error}
+    end.
+
+force_change_password(Username, Password) when is_binary(Username), is_binary(Password) ->
     change_password_hash(Username, hash(Password)).
 
 change_password_hash(Username, PasswordHash) ->
@@ -145,6 +149,37 @@ change_password_hash(Username, PasswordHash) ->
                         User#mqtt_admin{password = PasswordHash}
                 end).
 
+-define(LOW_LETTER_CHARS, "abcdefghijklmnopqrstuvwxyz").
+-define(UPPER_LETTER_CHARS, "ABCDEFGHIJKLMNOPQRSTUVWXYZ").
+-define(LETTER, ?LOW_LETTER_CHARS ++ ?UPPER_LETTER_CHARS).
+-define(NUMBER, "0123456789").
+-define(SPECIAL_CHARS, "!@#$%^&*()_+-=[]{}\"|;':,./<>?`~ ").
+
+is_valid_pwd(Password) when is_binary(Password) ->
+    is_valid_pwd(binary_to_list(Password));
+is_valid_pwd(Password) ->
+    Len = erlang:length(Password),
+    case Len >= 8 andalso Len =< 64 of
+        true ->
+            Letter = contain(Password, ?LETTER),
+            Number = contain(Password, ?NUMBER),
+            Special = contain(Password, ?SPECIAL_CHARS),
+            OK = lists:filter(fun(C) -> C end, [Letter, Number, Special]),
+            case length(OK) >= 2 of
+                true ->
+                   %% regex-any-ascii-character
+                    case re:run(Password, "[^\\x00-\\x7F]+", [unicode, {capture, none}]) of
+                        match -> {error, <<"only ascii characters are allowed in the password">>};
+                        nomatch -> ok
+                    end;
+                _ -> {error, ?INVALID_PASSWORD_MSG}
+            end;
+        false ->
+            {error, <<"The password length: 8-64">>}
+    end.
+
+contain(Xs, Spec) -> lists:any(fun(X) -> lists:member(X, Spec) end, Xs).
+
 update_pwd(Username, Fun) ->
     Trans = fun() ->
                     User =
@@ -317,3 +352,36 @@ maybe_warn_default_pwd() ->
         false ->
             ok
     end.
+
+-ifdef(TEST).
+-include_lib("eunit/include/eunit.hrl").
+
+password_test() ->
+    ?assertEqual({error, <<"The password length: 8-64">>}, is_valid_pwd(<<"123">>)),
+    MaxPassword = iolist_to_binary([lists:duplicate(63, "x"), "1"]),
+    ?assertEqual(ok, is_valid_pwd(MaxPassword)),
+    TooLongPassword = lists:duplicate(65, "y"),
+    ?assertEqual({error, <<"The password length: 8-64">>}, is_valid_pwd(TooLongPassword)),
+
+    ?assertEqual({error, ?INVALID_PASSWORD_MSG}, is_valid_pwd(<<"12345678">>)),
+    ?assertEqual({error, ?INVALID_PASSWORD_MSG}, is_valid_pwd(?LETTER)),
+    ?assertEqual({error, ?INVALID_PASSWORD_MSG}, is_valid_pwd(?NUMBER)),
+    ?assertEqual({error, ?INVALID_PASSWORD_MSG}, is_valid_pwd(?SPECIAL_CHARS)),
+    ?assertEqual({error, ?INVALID_PASSWORD_MSG}, is_valid_pwd(<<"映映映映无天在请"/utf8>>)),
+    ?assertEqual({error, <<"only ascii characters are allowed in the password">>}, is_valid_pwd(<<"️test_for_non_ascii1中"/utf8>>)),
+    ?assertEqual({error, <<"only ascii characters are allowed in the password">>}, is_valid_pwd(<<"云☁️test_for_unicode"/utf8>>)),
+
+    ?assertEqual(ok, is_valid_pwd(?LOW_LETTER_CHARS ++ ?NUMBER)),
+    ?assertEqual(ok, is_valid_pwd(?UPPER_LETTER_CHARS ++ ?NUMBER)),
+    ?assertEqual(ok, is_valid_pwd(?LOW_LETTER_CHARS ++ ?SPECIAL_CHARS)),
+    ?assertEqual(ok, is_valid_pwd(?UPPER_LETTER_CHARS ++ ?SPECIAL_CHARS)),
+    ?assertEqual(ok, is_valid_pwd(?SPECIAL_CHARS ++ ?NUMBER)),
+
+    ?assertEqual(ok, is_valid_pwd(<<"abckldiekflkdf12">>)),
+    ?assertEqual(ok, is_valid_pwd(<<"abckldiekflkdf w">>)),
+    ?assertEqual(ok, is_valid_pwd(<<"# abckldiekflkdf w">>)),
+    ?assertEqual(ok, is_valid_pwd(<<"# 12344858">>)),
+    ?assertEqual(ok, is_valid_pwd(<<"# %12344858">>)),
+    ok.
+
+-endif.

lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl

@@ -71,7 +71,7 @@ init_per_testcase(Case, Config) ->
 
 end_per_testcase(Case, Config) ->
     %% revert to default password
-    emqx_dashboard_admin:change_password(<<"admin">>, <<"public">>),
+    emqx_dashboard_admin:force_change_password(<<"admin">>, <<"public">>),
     ?MODULE:Case({'end', Config}).
 
 t_overview({init, Config}) -> Config;
@@ -85,35 +85,36 @@ t_admins_add_delete(_) ->
     ?assertEqual({error,<<"0 < Length =< 256">>},
         emqx_dashboard_admin:add_user(<<"">>, <<"password">>, <<"tag1">>)),
 
-    ?assertEqual({error,<<"2 < Length =< 32">>},
+    ?assertEqual({error,<<"The password length: 8-64">>},
         emqx_dashboard_admin:add_user(<<"badusername">>, <<"">>, <<"tag1">>)),
-    ?assertEqual({error,<<"2 < Length =< 32">>},
+    ?assertEqual({error,<<"The password length: 8-64">>},
         emqx_dashboard_admin:add_user(<<"badusername">>, <<"p">>, <<"tag1">>)),
-    P33 = iolist_to_binary(lists:duplicate(33, <<"p">>)),
+    P65 = iolist_to_binary(lists:duplicate(65, <<"p">>)),
-    ?assertEqual({error,<<"2 < Length =< 32">>},
+    ?assertEqual({error,<<"The password length: 8-64">>},
-        emqx_dashboard_admin:add_user(<<"badusername">>, P33, <<"tag1">>)),
+        emqx_dashboard_admin:add_user(<<"badusername">>, P65, <<"tag1">>)),
-    P32 = iolist_to_binary(lists:duplicate(32, <<"p">>)),
+    P64 = iolist_to_binary([<<"1">> | lists:duplicate(63, <<"p">>)]),
-    ?assertEqual(ok, emqx_dashboard_admin:add_user(<<"goodusername">>, P32, <<"tag1">>)),
+    ?assertEqual(ok, emqx_dashboard_admin:add_user(<<"goodusername">>, P64, <<"tag1">>)),
     ok = emqx_dashboard_admin:remove_user(<<"goodusername">>),
 
-    ok = emqx_dashboard_admin:add_user(<<"username">>, <<"password">>, <<"tag">>),
+    ok = emqx_dashboard_admin:add_user(<<"username1">>, <<"password1">>, <<"tag">>),
-    ok = emqx_dashboard_admin:add_user(<<"username1">>, <<"password1">>, <<"tag1">>),
+    ok = emqx_dashboard_admin:add_user(<<"username2">>, <<"password2">>, <<"tag1">>),
     ok = emqx_dashboard_admin:add_user(<<"1username1">>, <<"password1">>, <<"tag1">>),
     {error, _} = emqx_dashboard_admin:add_user(<<"u/sername1">>, <<"password1">>, <<"tag1">>),
     {error, _} = emqx_dashboard_admin:add_user(<<"/username1">>, <<"password1">>, <<"tag1">>),
     Admins = emqx_dashboard_admin:all_users(),
     ?assertEqual(4, length(Admins)),
-    ok = emqx_dashboard_admin:remove_user(<<"username1">>),
+    ok = emqx_dashboard_admin:remove_user(<<"username2">>),
     ok = emqx_dashboard_admin:remove_user(<<"1username1">>),
     Users = emqx_dashboard_admin:all_users(),
     ?assertEqual(2, length(Users)),
-    ok = emqx_dashboard_admin:change_password(<<"username">>, <<"password">>, <<"pwd">>),
+    {error, _} = emqx_dashboard_admin:change_password(<<"username1">>, <<"password1">>, <<"password">>),
+    ok = emqx_dashboard_admin:change_password(<<"username1">>, <<"password1">>, <<"password+">>),
     timer:sleep(10),
-    ?assert(request_dashboard(get, api_path("brokers"), auth_header_("username", "pwd"))),
+    ?assert(request_dashboard(get, api_path("brokers"), auth_header_("username1", "password+"))),
 
-    ok = emqx_dashboard_admin:remove_user(<<"username">>),
+    ok = emqx_dashboard_admin:remove_user(<<"username1">>),
     ?assertNotEqual(true, request_dashboard(get, api_path("brokers"),
-        auth_header_("username", "pwd"))).
+        auth_header_("username1", "password+"))).
 
 t_admins_persist_default_password({init, Config}) -> Config;
 t_admins_persist_default_password({'end', _Config}) -> ok;
@@ -212,16 +213,19 @@ t_rest_api(_Config) ->
     ?assert(lists:member(#{<<"username">> => <<"admin">>, <<"tags">> => <<"administrator">>},
         Users)),
 
+    {ok, ErrorRes} = http_put("change_pwd/admin", [{<<"old_pwd">>, <<"public">>}, {<<"new_pwd">>, <<"simplepwd">>}]),
+    ?assertMatch(#{<<"message">> := _}, json(ErrorRes)),
+
     AssertSuccess = fun({ok, Res}) ->
                         ?assertEqual(#{<<"code">> => 0}, json(Res))
                     end,
     [AssertSuccess(R)
      || R <- [ http_put("users/admin", #{<<"tags">> => <<"a_new_tag">>})
-             , http_post("users", #{<<"username">> => <<"usera">>, <<"password">> => <<"passwd">>})
+             , http_post("users", #{<<"username">> => <<"username1">>, <<"password">> => <<"passwd+123">>})
-             , http_post("auth", #{<<"username">> => <<"usera">>, <<"password">> => <<"passwd">>})
+             , http_post("auth", #{<<"username">> => <<"username1">>, <<"password">> => <<"passwd+123">>})
-             , http_delete("users/usera")
+             , http_delete("users/username1")
-             , http_put("change_pwd/admin", #{<<"old_pwd">> => <<"public">>, <<"new_pwd">> => <<"newpwd">>})
+             , http_put("change_pwd/admin", #{<<"old_pwd">> => <<"public">>, <<"new_pwd">> => <<"newpwd+123">>})
-             , http_post("auth", #{<<"username">> => <<"admin">>, <<"password">> => <<"newpwd">>})
+             , http_post("auth", #{<<"username">> => <<"admin">>, <<"password">> => <<"newpwd+123">>})
              ]],
     ok.
 
@@ -236,18 +240,18 @@ t_cli({init, Config}) -> Config;
 t_cli({'end', _Config}) -> ok;
 t_cli(_Config) ->
     [mnesia:dirty_delete({mqtt_admin, Admin}) ||  Admin <- mnesia:dirty_all_keys(mqtt_admin)],
-    emqx_dashboard_cli:admins(["add", "username", "password"]),
+    emqx_dashboard_cli:admins(["add", "username", "password1"]),
     [{mqtt_admin, <<"username">>, <<Salt:4/binary, Hash/binary>>, _}] =
         emqx_dashboard_admin:lookup_user(<<"username">>),
-    ?assertEqual(Hash, erlang:md5(<<Salt/binary, <<"password">>/binary>>)),
+    ?assertEqual(Hash, erlang:md5(<<Salt/binary, <<"password1">>/binary>>)),
-    emqx_dashboard_cli:admins(["passwd", "username", "newpassword"]),
+    emqx_dashboard_cli:admins(["passwd", "username", "newpassword1"]),
     [{mqtt_admin, <<"username">>, <<Salt1:4/binary, Hash1/binary>>, _}] =
         emqx_dashboard_admin:lookup_user(<<"username">>),
-    ?assertEqual(Hash1, erlang:md5(<<Salt1/binary, <<"newpassword">>/binary>>)),
+    ?assertEqual(Hash1, erlang:md5(<<Salt1/binary, <<"newpassword1">>/binary>>)),
     emqx_dashboard_cli:admins(["del", "username"]),
     [] = emqx_dashboard_admin:lookup_user(<<"username">>),
-    emqx_dashboard_cli:admins(["add", "admin1", "pass1"]),
+    emqx_dashboard_cli:admins(["add", "admin1", "password+1"]),
-    emqx_dashboard_cli:admins(["add", "admin2", "passw2"]),
+    emqx_dashboard_cli:admins(["add", "admin2", "password+2"]),
     AdminList = emqx_dashboard_admin:all_users(),
     ?assertEqual(2, length(AdminList)).
 

scripts/get-dashboard.sh

@@ -13,7 +13,7 @@ case "${PKG_VSN}" in
         ;;
     4.4*)
         # keep the above 4.3 untouched, otherwise conflicts!
-        EMQX_CE_DASHBOARD_VERSION='v4.4.7'
+        EMQX_CE_DASHBOARD_VERSION='v4.4.8'
         EMQX_EE_DASHBOARD_VERSION='v4.4.18'
         ;;
     *)