Merge pull request #9774 from zhongwencool/dashboard-password-validate-v50

feat: add dashboard password validation

apps/emqx/test/emqx_common_test_helpers.erl

@@ -175,6 +175,7 @@ start_apps(Apps, SpecAppConfig) when is_function(SpecAppConfig) ->
     %% Because, minirest, ekka etc.. application will scan these modules
     lists:foreach(fun load/1, [emqx | Apps]),
     ok = start_ekka(),
+    mnesia:clear_table(emqx_admin),
     ok = emqx_ratelimiter_SUITE:load_conf(),
     lists:foreach(fun(App) -> start_app(App, SpecAppConfig) end, [emqx | Apps]).
 

apps/emqx_dashboard/src/emqx_dashboard_admin.erl

@@ -92,21 +92,79 @@ add_default_user() ->
 add_user(Username, Password, Desc) when
     is_binary(Username), is_binary(Password)
 ->
-    case legal_username(Username) of
+    case {legal_username(Username), legal_password(Password)} of
-        true ->
+        {ok, ok} -> do_add_user(Username, Password, Desc);
-            return(
+        {{error, Reason}, _} -> {error, Reason};
-                mria:transaction(?DASHBOARD_SHARD, fun add_user_/3, [Username, Password, Desc])
+        {_, {error, Reason}} -> {error, Reason}
-            );
+    end.
-        false ->
+
+do_add_user(Username, Password, Desc) ->
+    Res = mria:transaction(?DASHBOARD_SHARD, fun add_user_/3, [Username, Password, Desc]),
+    return(Res).
+
+%% 0-9 or A-Z or a-z or $_
+legal_username(<<>>) ->
+    {error, <<"Username cannot be empty">>};
+legal_username(UserName) ->
+    case re:run(UserName, "^[_a-zA-Z0-9]*$", [{capture, none}]) of
+        nomatch ->
             {error, <<
                 "Bad Username."
                 " Only upper and lower case letters, numbers and underscores are supported"
-            >>}
+            >>};
+        match ->
+            ok
     end.
 
-%% 0 - 9 or A -Z or a - z or $_
+-define(LOW_LETTER_CHARS, "abcdefghijklmnopqrstuvwxyz").
-legal_username(<<>>) -> false;
+-define(UPPER_LETTER_CHARS, "ABCDEFGHIJKLMNOPQRSTUVWXYZ").
-legal_username(UserName) -> nomatch /= re:run(UserName, "^[_a-zA-Z0-9]*$").
+-define(LETTER, ?LOW_LETTER_CHARS ++ ?UPPER_LETTER_CHARS).
+-define(NUMBER, "0123456789").
+-define(SPECIAL_CHARS, "!@#$%^&*()_+-=[]{}\"|;':,./<>?`~ ").
+-define(INVALID_PASSWORD_MSG, <<
+    "Bad password. "
+    "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."
+>>).
+-define(BAD_PASSWORD_LEN, <<"The range of password length is 8~64">>).
+
+legal_password(Password) when is_binary(Password) ->
+    legal_password(binary_to_list(Password));
+legal_password(Password) when is_list(Password) ->
+    legal_password(Password, erlang:length(Password)).
+
+legal_password(Password, Len) when Len >= 8 andalso Len =< 64 ->
+    case is_mixed_password(Password) of
+        true -> ascii_character_validate(Password);
+        false -> {error, ?INVALID_PASSWORD_MSG}
+    end;
+legal_password(_Password, _Len) ->
+    {error, ?BAD_PASSWORD_LEN}.
+
+%% The password must contain at least two different kind of characters
+%% from groups of letters, numbers, and special characters.
+is_mixed_password(Password) -> is_mixed_password(Password, [?NUMBER, ?LETTER, ?SPECIAL_CHARS], 0).
+
+is_mixed_password(_Password, _Chars, 2) ->
+    true;
+is_mixed_password(_Password, [], _Count) ->
+    false;
+is_mixed_password(Password, [Chars | Rest], Count) ->
+    NewCount =
+        case contain(Password, Chars) of
+            true -> Count + 1;
+            false -> Count
+        end,
+    is_mixed_password(Password, Rest, NewCount).
+
+%% regex-non-ascii-character, such as Chinese, Japanese, Korean, etc.
+ascii_character_validate(Password) ->
+    case re:run(Password, "[^\\x00-\\x7F]+", [unicode, {capture, none}]) of
+        match -> {error, <<"Only ascii characters are allowed in the password">>};
+        nomatch -> ok
+    end.
+
+contain(Xs, Spec) -> lists:any(fun(X) -> lists:member(X, Spec) end, Xs).
 
 %% black-magic: force overwrite a user
 force_add_user(Username, Password, Desc) ->
@@ -188,7 +246,10 @@ change_password(Username, OldPasswd, NewPasswd) when is_binary(Username) ->
     end.
 
 change_password(Username, Password) when is_binary(Username), is_binary(Password) ->
-    change_password_hash(Username, hash(Password)).
+    case legal_password(Password) of
+        ok -> change_password_hash(Username, hash(Password));
+        Error -> Error
+    end.
 
 change_password_hash(Username, PasswordHash) ->
     ChangePWD =
@@ -292,6 +353,45 @@ add_default_user(Username, Password) when ?EMPTY_KEY(Username) orelse ?EMPTY_KEY
     {ok, empty};
 add_default_user(Username, Password) ->
     case lookup_user(Username) of
-        [] -> add_user(Username, Password, <<"administrator">>);
+        [] -> do_add_user(Username, Password, <<"administrator">>);
         _ -> {ok, default_user_exists}
     end.
+
+-ifdef(TEST).
+-include_lib("eunit/include/eunit.hrl").
+
+legal_password_test() ->
+    ?assertEqual({error, ?BAD_PASSWORD_LEN}, legal_password(<<"123">>)),
+    MaxPassword = iolist_to_binary([lists:duplicate(63, "x"), "1"]),
+    ?assertEqual(ok, legal_password(MaxPassword)),
+    TooLongPassword = lists:duplicate(65, "y"),
+    ?assertEqual({error, ?BAD_PASSWORD_LEN}, legal_password(TooLongPassword)),
+
+    ?assertEqual({error, ?INVALID_PASSWORD_MSG}, legal_password(<<"12345678">>)),
+    ?assertEqual({error, ?INVALID_PASSWORD_MSG}, legal_password(?LETTER)),
+    ?assertEqual({error, ?INVALID_PASSWORD_MSG}, legal_password(?NUMBER)),
+    ?assertEqual({error, ?INVALID_PASSWORD_MSG}, legal_password(?SPECIAL_CHARS)),
+    ?assertEqual({error, ?INVALID_PASSWORD_MSG}, legal_password(<<"映映映映无天在请"/utf8>>)),
+    ?assertEqual(
+        {error, <<"Only ascii characters are allowed in the password">>},
+        legal_password(<<"️test_for_non_ascii1中"/utf8>>)
+    ),
+    ?assertEqual(
+        {error, <<"Only ascii characters are allowed in the password">>},
+        legal_password(<<"云☁️test_for_unicode"/utf8>>)
+    ),
+
+    ?assertEqual(ok, legal_password(?LOW_LETTER_CHARS ++ ?NUMBER)),
+    ?assertEqual(ok, legal_password(?UPPER_LETTER_CHARS ++ ?NUMBER)),
+    ?assertEqual(ok, legal_password(?LOW_LETTER_CHARS ++ ?SPECIAL_CHARS)),
+    ?assertEqual(ok, legal_password(?UPPER_LETTER_CHARS ++ ?SPECIAL_CHARS)),
+    ?assertEqual(ok, legal_password(?SPECIAL_CHARS ++ ?NUMBER)),
+
+    ?assertEqual(ok, legal_password(<<"abckldiekflkdf12">>)),
+    ?assertEqual(ok, legal_password(<<"abckldiekflkdf w">>)),
+    ?assertEqual(ok, legal_password(<<"# abckldiekflkdf w">>)),
+    ?assertEqual(ok, legal_password(<<"# 12344858">>)),
+    ?assertEqual(ok, legal_password(<<"# %12344858">>)),
+    ok.
+
+-endif.

apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl

@@ -61,6 +61,7 @@ end_suite() ->
 
 end_suite(Apps) ->
     application:unload(emqx_management),
+    mnesia:clear_table(?ADMIN),
     emqx_common_test_helpers:stop_apps(Apps ++ [emqx_dashboard]).
 
 init_per_suite(Config) ->
@@ -82,16 +83,17 @@ set_special_configs(_) ->
 
 t_overview(_) ->
     mnesia:clear_table(?ADMIN),
-    emqx_dashboard_admin:add_user(<<"admin">>, <<"public">>, <<"simple_description">>),
+    emqx_dashboard_admin:add_user(<<"admin">>, <<"public_www1">>, <<"simple_description">>),
+    Headers = auth_header_(<<"admin">>, <<"public_www1">>),
     [
-        {ok, _} = request_dashboard(get, api_path([Overview]), auth_header_())
+        {ok, _} = request_dashboard(get, api_path([Overview]), Headers)
      || Overview <- ?OVERVIEWS
     ].
 
 t_admins_add_delete(_) ->
     mnesia:clear_table(?ADMIN),
     Desc = <<"simple description">>,
-    {ok, _} = emqx_dashboard_admin:add_user(<<"username">>, <<"password">>, Desc),
+    {ok, _} = emqx_dashboard_admin:add_user(<<"username">>, <<"password_0">>, Desc),
     {ok, _} = emqx_dashboard_admin:add_user(<<"username1">>, <<"password1">>, Desc),
     Admins = emqx_dashboard_admin:all_users(),
     ?assertEqual(2, length(Admins)),
@@ -100,8 +102,8 @@ t_admins_add_delete(_) ->
     ?assertEqual(1, length(Users)),
     {ok, _} = emqx_dashboard_admin:change_password(
         <<"username">>,
-        <<"password">>,
+        <<"password_0">>,
-        <<"pwd">>
+        <<"new_pwd_1234">>
     ),
     timer:sleep(10),
     {ok, _} = emqx_dashboard_admin:remove_user(<<"username">>).
@@ -109,12 +111,12 @@ t_admins_add_delete(_) ->
 t_admin_delete_self_failed(_) ->
     mnesia:clear_table(?ADMIN),
     Desc = <<"simple description">>,
-    _ = emqx_dashboard_admin:add_user(<<"username1">>, <<"password">>, Desc),
+    _ = emqx_dashboard_admin:add_user(<<"username1">>, <<"password_1">>, Desc),
     Admins = emqx_dashboard_admin:all_users(),
     ?assertEqual(1, length(Admins)),
-    Header = auth_header_(<<"username1">>, <<"password">>),
+    Header = auth_header_(<<"username1">>, <<"password_1">>),
     {error, {_, 400, _}} = request_dashboard(delete, api_path(["users", "username1"]), Header),
-    Token = ["Basic ", base64:encode("username1:password")],
+    Token = ["Basic ", base64:encode("username1:password_1")],
     Header2 = {"Authorization", Token},
     {error, {_, 401, _}} = request_dashboard(delete, api_path(["users", "username1"]), Header2),
     mnesia:clear_table(?ADMIN).
@@ -122,7 +124,8 @@ t_admin_delete_self_failed(_) ->
 t_rest_api(_Config) ->
     mnesia:clear_table(?ADMIN),
     Desc = <<"administrator">>,
-    emqx_dashboard_admin:add_user(<<"admin">>, <<"public">>, Desc),
+    Password = <<"public_www1">>,
+    emqx_dashboard_admin:add_user(<<"admin">>, Password, Desc),
     {ok, 200, Res0} = http_get(["users"]),
     ?assertEqual(
         [
@@ -136,7 +139,7 @@ t_rest_api(_Config) ->
     {ok, 200, _} = http_put(["users", "admin"], #{<<"description">> => <<"a_new_description">>}),
     {ok, 200, _} = http_post(["users"], #{
         <<"username">> => <<"usera">>,
-        <<"password">> => <<"passwd">>,
+        <<"password">> => <<"passwd_01234">>,
         <<"description">> => Desc
     }),
     {ok, 204, _} = http_delete(["users", "usera"]),
@@ -144,34 +147,34 @@ t_rest_api(_Config) ->
     {ok, 204, _} = http_post(
         ["users", "admin", "change_pwd"],
         #{
-            <<"old_pwd">> => <<"public">>,
+            <<"old_pwd">> => Password,
-            <<"new_pwd">> => <<"newpwd">>
+            <<"new_pwd">> => <<"newpwd_lkdfki1">>
         }
     ),
     mnesia:clear_table(?ADMIN),
-    emqx_dashboard_admin:add_user(<<"admin">>, <<"public">>, <<"administrator">>),
+    emqx_dashboard_admin:add_user(<<"admin">>, Password, <<"administrator">>),
     ok.
 
 t_cli(_Config) ->
     [mria:dirty_delete(?ADMIN, Admin) || Admin <- mnesia:dirty_all_keys(?ADMIN)],
-    emqx_dashboard_cli:admins(["add", "username", "password"]),
+    emqx_dashboard_cli:admins(["add", "username", "password_ww2"]),
     [#?ADMIN{username = <<"username">>, pwdhash = <<Salt:4/binary, Hash/binary>>}] =
         emqx_dashboard_admin:lookup_user(<<"username">>),
-    ?assertEqual(Hash, crypto:hash(sha256, <<Salt/binary, <<"password">>/binary>>)),
+    ?assertEqual(Hash, crypto:hash(sha256, <<Salt/binary, <<"password_ww2">>/binary>>)),
-    emqx_dashboard_cli:admins(["passwd", "username", "newpassword"]),
+    emqx_dashboard_cli:admins(["passwd", "username", "new_password"]),
     [#?ADMIN{username = <<"username">>, pwdhash = <<Salt1:4/binary, Hash1/binary>>}] =
         emqx_dashboard_admin:lookup_user(<<"username">>),
-    ?assertEqual(Hash1, crypto:hash(sha256, <<Salt1/binary, <<"newpassword">>/binary>>)),
+    ?assertEqual(Hash1, crypto:hash(sha256, <<Salt1/binary, <<"new_password">>/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", "pass_lkdfkd1"]),
-    emqx_dashboard_cli:admins(["add", "admin2", "passw2"]),
+    emqx_dashboard_cli:admins(["add", "admin2", "w_pass_lkdfkd2"]),
     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())),
+    Pwd = bin("t_password" ++ integer_to_list(random_num())),
     emqx_dashboard_token:sign(User, Pwd),
     ?assertMatch(
         [#?ADMIN_JWT{username = User}],
@@ -185,7 +188,7 @@ t_lookup_by_username_jwt(_Config) ->
 
 t_clean_expired_jwt(_Config) ->
     User = bin(["user-", integer_to_list(random_num())]),
-    Pwd = bin(integer_to_list(random_num())),
+    Pwd = bin("t_password" ++ integer_to_list(random_num())),
     emqx_dashboard_token:sign(User, Pwd),
     [#?ADMIN_JWT{username = User, exptime = ExpTime}] =
         emqx_dashboard_token:lookup_by_username(User),
@@ -208,16 +211,16 @@ random_num() ->
     erlang:system_time(nanosecond).
 
 http_get(Parts) ->
-    request_api(get, api_path(Parts), auth_header_()).
+    request_api(get, api_path(Parts), auth_header_(<<"admin">>, <<"public_www1">>)).
 
 http_delete(Parts) ->
-    request_api(delete, api_path(Parts), auth_header_()).
+    request_api(delete, api_path(Parts), auth_header_(<<"admin">>, <<"public_www1">>)).
 
 http_post(Parts, Body) ->
-    request_api(post, api_path(Parts), [], auth_header_(), Body).
+    request_api(post, api_path(Parts), [], auth_header_(<<"admin">>, <<"public_www1">>), Body).
 
 http_put(Parts, Body) ->
-    request_api(put, api_path(Parts), [], auth_header_(), Body).
+    request_api(put, api_path(Parts), [], auth_header_(<<"admin">>, <<"public_www1">>), Body).
 
 request_dashboard(Method, Url, Auth) ->
     Request = {Url, [Auth]},

apps/emqx_dashboard/test/emqx_dashboard_admin_SUITE.erl

@@ -51,7 +51,7 @@ end_suite() ->
 
 t_check_user(_) ->
     Username = <<"admin1">>,
-    Password = <<"public">>,
+    Password = <<"public_1">>,
     BadUsername = <<"admin_bad">>,
     BadPassword = <<"public_bad">>,
     EmptyUsername = <<>>,
@@ -108,7 +108,7 @@ t_lookup_user(_) ->
 
 t_all_users(_) ->
     Username = <<"admin_all">>,
-    Password = <<"public">>,
+    Password = <<"public_2">>,
     {ok, _} = emqx_dashboard_admin:add_user(Username, Password, <<"desc">>),
     All = emqx_dashboard_admin:all_users(),
     ?assert(erlang:length(All) >= 1),
@@ -153,6 +153,7 @@ t_change_password(_) ->
     Description = <<"change_description">>,
 
     NewPassword = <<"new_password">>,
+    NewBadPassword = <<"public">>,
 
     BadChangeUser = <<"change_user_bad">>,
 
@@ -163,14 +164,17 @@ t_change_password(_) ->
     {error, <<"password_error">>} =
         emqx_dashboard_admin:change_password(User, OldPassword, NewPassword),
 
+    {error, <<"The range of password length is 8~64">>} =
+        emqx_dashboard_admin:change_password(User, NewPassword, NewBadPassword),
+
     {error, <<"username_not_found">>} =
         emqx_dashboard_admin:change_password(BadChangeUser, OldPassword, NewPassword),
     ok.
 
 t_clean_token(_) ->
     Username = <<"admin_token">>,
-    Password = <<"public">>,
+    Password = <<"public_www1">>,
-    NewPassword = <<"public1">>,
+    NewPassword = <<"public_www2">>,
     {ok, _} = emqx_dashboard_admin:add_user(Username, Password, <<"desc">>),
     {ok, Token} = emqx_dashboard_admin:sign_token(Username, Password),
     ok = emqx_dashboard_admin:verify_token(Token),

apps/emqx_dashboard/test/emqx_dashboard_monitor_SUITE.erl

@@ -33,6 +33,7 @@ all() ->
     emqx_common_test_helpers:all(?MODULE).
 
 init_per_suite(Config) ->
+    application:load(emqx_dashboard),
     mria:start(),
     emqx_common_test_helpers:start_apps([emqx_dashboard], fun set_special_configs/1),
     Config.

changes/v5.0.15/feat-9774.en.md

@@ -0,0 +1,3 @@
+Add a password complexity requirement when adding or modifying Dashboard users via the API.
+Now password must contain at least 2 of alphabetic, numeric and special characters,
+and must be 8 to 64 characters long.

changes/v5.0.15/feat-9774.zh.md

@@ -0,0 +1,2 @@
+通过 API 添加、修改 Dashboard 用户时，增加对密码复杂度的要求。
+现在密码必须包含字母、数字以及特殊字符中的至少 2 种，并且长度范围必须是 8~64 个字符。