diff --git a/apps/emqx/test/emqx_common_test_helpers.erl b/apps/emqx/test/emqx_common_test_helpers.erl index e4f50f2a1..954151efa 100644 --- a/apps/emqx/test/emqx_common_test_helpers.erl +++ b/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]). diff --git a/apps/emqx_dashboard/src/emqx_dashboard_admin.erl b/apps/emqx_dashboard/src/emqx_dashboard_admin.erl index e36c2628b..aaa43d621 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_admin.erl +++ b/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 - true -> - return( - mria:transaction(?DASHBOARD_SHARD, fun add_user_/3, [Username, Password, Desc]) - ); - false -> + case {legal_username(Username), legal_password(Password)} of + {ok, ok} -> do_add_user(Username, Password, Desc); + {{error, Reason}, _} -> {error, Reason}; + {_, {error, Reason}} -> {error, Reason} + end. + +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 $_ -legal_username(<<>>) -> false; -legal_username(UserName) -> nomatch /= re:run(UserName, "^[_a-zA-Z0-9]*$"). +-define(LOW_LETTER_CHARS, "abcdefghijklmnopqrstuvwxyz"). +-define(UPPER_LETTER_CHARS, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"). +-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. diff --git a/apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl b/apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl index 23d1b40c1..8190b7c54 100644 --- a/apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl +++ b/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">>, - <<"pwd">> + <<"password_0">>, + <<"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">>, - <<"new_pwd">> => <<"newpwd">> + <<"old_pwd">> => Password, + <<"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 = <>}] = emqx_dashboard_admin:lookup_user(<<"username">>), - ?assertEqual(Hash, crypto:hash(sha256, <>/binary>>)), - emqx_dashboard_cli:admins(["passwd", "username", "newpassword"]), + ?assertEqual(Hash, crypto:hash(sha256, <>/binary>>)), + emqx_dashboard_cli:admins(["passwd", "username", "new_password"]), [#?ADMIN{username = <<"username">>, pwdhash = <>}] = emqx_dashboard_admin:lookup_user(<<"username">>), - ?assertEqual(Hash1, crypto:hash(sha256, <>/binary>>)), + ?assertEqual(Hash1, crypto:hash(sha256, <>/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", "admin2", "passw2"]), + emqx_dashboard_cli:admins(["add", "admin1", "pass_lkdfkd1"]), + 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]}, diff --git a/apps/emqx_dashboard/test/emqx_dashboard_admin_SUITE.erl b/apps/emqx_dashboard/test/emqx_dashboard_admin_SUITE.erl index fefc492cc..9ae5d4418 100644 --- a/apps/emqx_dashboard/test/emqx_dashboard_admin_SUITE.erl +++ b/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">>, - NewPassword = <<"public1">>, + Password = <<"public_www1">>, + 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), diff --git a/apps/emqx_dashboard/test/emqx_dashboard_monitor_SUITE.erl b/apps/emqx_dashboard/test/emqx_dashboard_monitor_SUITE.erl index 7d4980320..74c6d9cc1 100644 --- a/apps/emqx_dashboard/test/emqx_dashboard_monitor_SUITE.erl +++ b/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. diff --git a/changes/v5.0.15/feat-9774.en.md b/changes/v5.0.15/feat-9774.en.md new file mode 100644 index 000000000..722c4db6b --- /dev/null +++ b/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. diff --git a/changes/v5.0.15/feat-9774.zh.md b/changes/v5.0.15/feat-9774.zh.md new file mode 100644 index 000000000..21bfddfaf --- /dev/null +++ b/changes/v5.0.15/feat-9774.zh.md @@ -0,0 +1,2 @@ +通过 API 添加、修改 Dashboard 用户时,增加对密码复杂度的要求。 +现在密码必须包含字母、数字以及特殊字符中的至少 2 种,并且长度范围必须是 8~64 个字符。 diff --git a/changes/v5.0.15/fix-9726-en.md b/changes/v5.0.15/fix-9726.en.md similarity index 100% rename from changes/v5.0.15/fix-9726-en.md rename to changes/v5.0.15/fix-9726.en.md diff --git a/changes/v5.0.15/fix-9726-zh.md b/changes/v5.0.15/fix-9726.zh.md similarity index 100% rename from changes/v5.0.15/fix-9726-zh.md rename to changes/v5.0.15/fix-9726.zh.md diff --git a/changes/v5.0.15/fix-9735-en.md b/changes/v5.0.15/fix-9735.en.md similarity index 100% rename from changes/v5.0.15/fix-9735-en.md rename to changes/v5.0.15/fix-9735.en.md diff --git a/changes/v5.0.15/fix-9735-zh.md b/changes/v5.0.15/fix-9735.zh.md similarity index 100% rename from changes/v5.0.15/fix-9735-zh.md rename to changes/v5.0.15/fix-9735.zh.md diff --git a/changes/v5.0.15/fix-9748-en.md b/changes/v5.0.15/fix-9748.en.md similarity index 100% rename from changes/v5.0.15/fix-9748-en.md rename to changes/v5.0.15/fix-9748.en.md diff --git a/changes/v5.0.15/fix-9748-zh.md b/changes/v5.0.15/fix-9748.zh.md similarity index 100% rename from changes/v5.0.15/fix-9748-zh.md rename to changes/v5.0.15/fix-9748.zh.md diff --git a/changes/v5.0.15/fix-9749-en.md b/changes/v5.0.15/fix-9749.en.md similarity index 100% rename from changes/v5.0.15/fix-9749-en.md rename to changes/v5.0.15/fix-9749.en.md diff --git a/changes/v5.0.15/fix-9749-zh.md b/changes/v5.0.15/fix-9749.zh.md similarity index 100% rename from changes/v5.0.15/fix-9749-zh.md rename to changes/v5.0.15/fix-9749.zh.md