From b44512cdab40bb0ea2eb27efc36d8bcacbdd38e1 Mon Sep 17 00:00:00 2001
From: zhongwencool <zhongwencool@gmail.com>
Date: Wed, 16 Mar 2022 10:20:09 +0800
Subject: [PATCH 1/7] fix: Add string legitimacy check.

---
 apps/emqx_management/src/emqx_mgmt_auth.erl   | 64 +++++++++++--------
 .../src/emqx_dashboard_admin.erl              | 26 +++++---
 .../test/emqx_dashboard_SUITE.erl             |  4 +-
 src/emqx_misc.erl                             | 45 +++++++++++++
 4 files changed, 100 insertions(+), 39 deletions(-)

diff --git a/apps/emqx_management/src/emqx_mgmt_auth.erl b/apps/emqx_management/src/emqx_mgmt_auth.erl
index c05cbf581..6793c9885 100644
--- a/apps/emqx_management/src/emqx_mgmt_auth.erl
+++ b/apps/emqx_management/src/emqx_mgmt_auth.erl
@@ -89,36 +89,45 @@ add_app(AppId, Name, Desc, Status, Expired) when is_binary(AppId) ->
       -> {ok, appsecret()}
        | {error, term()}).
 add_app(AppId, Name, Secret, Desc, Status, Expired) when is_binary(AppId) ->
-    Secret1 = generate_appsecret_if_need(Secret),
-    App = #mqtt_app{id = AppId,
-                    secret = Secret1,
-                    name = Name,
-                    desc = Desc,
-                    status = Status,
-                    expired = Expired},
-    AddFun = fun() ->
-                 case mnesia:wread({mqtt_app, AppId}) of
-                     [] -> mnesia:write(App);
-                     _  -> mnesia:abort(alread_existed)
-                 end
-             end,
-    case mnesia:transaction(AddFun) of
-        {atomic, ok} -> {ok, Secret1};
-        {aborted, Reason} -> {error, Reason}
+    case emqx_misc:valid_str(Name) of
+        ok ->
+            Secret1 = generate_appsecret_if_need(Secret),
+            App = #mqtt_app{id = AppId,
+                            secret = Secret1,
+                            name = Name,
+                            desc = Desc,
+                            status = Status,
+                            expired = Expired},
+            AddFun = fun() ->
+                case mnesia:wread({mqtt_app, AppId}) of
+                    [] -> mnesia:write(App);
+                    _  -> mnesia:abort(alread_existed)
+                end
+                     end,
+            case mnesia:transaction(AddFun) of
+                {atomic, ok} -> {ok, Secret1};
+                {aborted, Reason} -> {error, Reason}
+            end;
+        {error, Reason} -> {error, Reason}
     end.
 
 force_add_app(AppId, Name, Secret, Desc, Status, Expired) ->
-    AddFun = fun() ->
-                 mnesia:write(#mqtt_app{id = AppId,
-                                        secret = Secret,
-                                        name = Name,
-                                        desc = Desc,
-                                        status = Status,
-                                        expired = Expired})
-             end,
-    case mnesia:transaction(AddFun) of
-        {atomic, ok} -> ok;
-        {aborted, Reason} -> {error, Reason}
+    case emqx_misc:valid_str(Name) of
+        ok ->
+            AddFun = fun() ->
+                mnesia:write(#mqtt_app{
+                    id = AppId,
+                    secret = Secret,
+                    name = Name,
+                    desc = Desc,
+                    status = Status,
+                    expired = Expired})
+                     end,
+            case mnesia:transaction(AddFun) of
+                {atomic, ok} -> ok;
+                {aborted, Reason} -> {error, Reason}
+            end;
+        {error, Reason} -> {error, Reason}
     end.
 
 -spec(generate_appsecret_if_need(binary() | undefined) -> binary()).
@@ -207,4 +216,3 @@ is_authorized(AppId, AppSecret) ->
 
 is_expired(undefined) -> true;
 is_expired(Expired)   -> Expired >= erlang:system_time(second).
-
diff --git a/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl b/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl
index c9e9bee3c..7ce1f564f 100644
--- a/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl
+++ b/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl
@@ -78,18 +78,24 @@ start_link() ->
 
 -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},
-    return(mnesia:transaction(fun add_user_/1, [Admin])).
+    case emqx_misc:valid_str(Username) of
+        ok ->
+            Admin = #mqtt_admin{username = Username, password = hash(Password), tags = Tags},
+            return(mnesia:transaction(fun add_user_/1, [Admin]));
+        {error, Reason} -> {error, Reason}
+    end.
 
 force_add_user(Username, Password, Tags) ->
-    AddFun = fun() ->
-                 mnesia:write(#mqtt_admin{username = Username,
-                                          password = Password,
-                                          tags = Tags})
-             end,
-    case mnesia:transaction(AddFun) of
-        {atomic, ok} -> ok;
-        {aborted, Reason} -> {error, Reason}
+    case emqx_misc:valid_str(Username) of
+        ok ->
+            AddFun = fun() ->
+                mnesia:write(#mqtt_admin{username = Username, password = Password, tags = Tags})
+                     end,
+            case mnesia:transaction(AddFun) of
+                {atomic, ok} -> ok;
+                {aborted, Reason} -> {error, Reason}
+            end;
+        {error, Reason} -> {error, Reason}
     end.
 
 %% @private
diff --git a/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl b/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl
index ef2e747fa..3bca197f0 100644
--- a/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl
+++ b/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl
@@ -67,6 +67,9 @@ t_overview(_) ->
 t_admins_add_delete(_) ->
     ok = emqx_dashboard_admin:add_user(<<"username">>, <<"password">>, <<"tag">>),
     ok = emqx_dashboard_admin:add_user(<<"username1">>, <<"password1">>, <<"tag1">>),
+    {error, _} = 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(3, length(Admins)),
     ok = emqx_dashboard_admin:remove_user(<<"username1">>),
@@ -165,4 +168,3 @@ api_path(Path) ->
 
 json(Data) ->
     {ok, Jsx} = emqx_json:safe_decode(Data, [return_maps]), Jsx.
-
diff --git a/src/emqx_misc.erl b/src/emqx_misc.erl
index eb6a25377..8c76ff4a6 100644
--- a/src/emqx_misc.erl
+++ b/src/emqx_misc.erl
@@ -52,6 +52,26 @@
         , hexstr2bin/1
         ]).
 
+-export([ valid_str/1
+        ]).
+
+-define(VALID_STR_RE, "^[A-Za-z]+[A-Za-z0-9-_]*$").
+
+-spec valid_str(list() | binary()) -> ok | {error, Reason::binary()}.
+valid_str(Str) ->
+    StrLen = len(Str),
+    case StrLen > 0 andalso StrLen =< 256 of
+        true ->
+            case re:run(Str, ?VALID_STR_RE) of
+                nomatch -> {error, <<"required: " ?VALID_STR_RE>>};
+                _ -> ok
+            end;
+        false -> {error, <<"0 < Length =< 256">>}
+    end.
+
+len(Bin) when is_binary(Bin) -> byte_size(Bin);
+len(Str) when is_list(Str) -> length(Str).
+
 -define(OOM_FACTOR, 1.25).
 
 %% @doc Parse v4 or v6 string format address to tuple.
@@ -309,4 +329,29 @@ hexchar2int(I) when I >= $a andalso I =< $f -> I - $a + 10.
 ipv6_probe_test() ->
     ?assertEqual([{ipv6_probe, true}], ipv6_probe([])).
 
+valid_str_test() ->
+    ?assertMatch({error, _}, valid_str("")),
+    ?assertMatch({error, _}, valid_str("_")),
+    ?assertMatch({error, _}, valid_str("_aaa")),
+    ?assertMatch({error, _}, valid_str("lkad/oddl")),
+    ?assertMatch({error, _}, valid_str("lkad*oddl")),
+    ?assertMatch({error, _}, valid_str("<script>lkadoddl")),
+    ?assertMatch({error, _}, valid_str("1lkdfaldk")),
+    ?assertMatch({error, _}, valid_str("1223333434")),
+
+    ?assertMatch(ok, valid_str(<<"Abckdf_lkdfd_1222">>)),
+    ?assertMatch(ok, valid_str("Abckdf_lkdfd_1222")),
+    ?assertMatch(ok, valid_str("abckdf_lkdfd_1222")),
+    ?assertMatch(ok, valid_str("abckdflkdfd1222")),
+    ?assertMatch(ok, valid_str("abckdflkdf")),
+    ?assertMatch(ok, valid_str("a1122222")),
+
+    Ok = lists:flatten(lists:duplicate(256, "a")),
+    Bad = Ok ++ "a",
+    ?assertMatch(ok, valid_str(Ok)),
+    ?assertMatch(ok, valid_str(list_to_binary(Ok))),
+    ?assertMatch({error, _}, valid_str(Bad)),
+    ?assertMatch({error, _}, valid_str(list_to_binary(Bad))),
+    ok.
+
 -endif.

From 6b91a80712a21a6a5b7fc250d5311dedeb2b1a5b Mon Sep 17 00:00:00 2001
From: zhongwencool <zhongwencool@gmail.com>
Date: Wed, 16 Mar 2022 10:47:18 +0800
Subject: [PATCH 2/7] chore: update changelog-4.3

---
 CHANGES-4.3.md | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/CHANGES-4.3.md b/CHANGES-4.3.md
index 04447e220..cff632e46 100644
--- a/CHANGES-4.3.md
+++ b/CHANGES-4.3.md
@@ -22,7 +22,7 @@ File format:
 * CLI `emqx_ctl pem_cache clean` to force purge x509 certificate cache,
   to force an immediate reload of all certificates after the files are updated on disk.
 * Refactor the ExProto so that anonymous clients can also be displayed on the dashboard [#6983]
-* Force shutdown of processe that cannot answer takeover event [#7026]
+* Force shutdown of processes that cannot answer takeover event [#7026]
 * `topic` parameter in bridge configuration can have `${node}` substitution (just like in `clientid` parameter)
 * Add UTF-8 string validity check in `strict_mode` for MQTT packet.
   When set to true, invalid UTF-8 strings will cause the client to be disconnected. i.e. client ID, topic name. [#7261]
@@ -42,6 +42,7 @@ File format:
 * Fix false alert level log “cannot_find_plugins” caused by duplicate plugin names in `loaded_plugins` files.
 * Prompt user how to change the dashboard's initial default password when emqx start.
 * Fix errno=13 'Permission denied' Cannot create FIFO boot error in Amazon Linux 2022 (el8 package)
+* Fix user or appid created, name only allow `^[A-Za-z]+[A-Za-z0-9-_]*$`
 
 ## v4.3.12
 ### Important changes

From 0555bce47ef0384dfe4c8f4287cb483061daddd4 Mon Sep 17 00:00:00 2001
From: zhongwencool <zhongwencool@gmail.com>
Date: Wed, 16 Mar 2022 10:54:09 +0800
Subject: [PATCH 3/7] chore: add more doc for dashboard.default_user

---
 lib-ce/emqx_dashboard/etc/emqx_dashboard.conf | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lib-ce/emqx_dashboard/etc/emqx_dashboard.conf b/lib-ce/emqx_dashboard/etc/emqx_dashboard.conf
index f67b88c17..2d59264a1 100644
--- a/lib-ce/emqx_dashboard/etc/emqx_dashboard.conf
+++ b/lib-ce/emqx_dashboard/etc/emqx_dashboard.conf
@@ -3,12 +3,17 @@
 ##--------------------------------------------------------------------
 
 ## Default user's login name.
-##
+## For safety, it should be changed as soon as possible.
+## Please use the './bin/emqx_ctl admins' CLI to change it.
+## Then comment `dashboard.default_user.login/password` from here
 ## Value: String
 dashboard.default_user.login = admin
 
 ## Default user's password.
-##
+## The initial default password for 'dashboard.default_user.login'"
+## For safety, it should be changed as soon as possible.
+## Please use the './bin/emqx_ctl admins' CLI to change it.
+## Then comment `dashboard.default_user.login/password` from here
 ## Value: String
 dashboard.default_user.password = public
 
@@ -127,4 +132,3 @@ dashboard.listener.http.ipv6_v6only = false
 ##
 ## Value: on | off
 ## dashboard.listener.https.honor_cipher_order = on
-

From f70d777623231b333d7e6ef254644161f38f266d Mon Sep 17 00:00:00 2001
From: zhongwencool <zhongwencool@gmail.com>
Date: Wed, 16 Mar 2022 11:19:25 +0800
Subject: [PATCH 4/7] chore: update appup

---
 src/emqx.appup.src | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/emqx.appup.src b/src/emqx.appup.src
index 95e2f48f2..86ca424b3 100644
--- a/src/emqx.appup.src
+++ b/src/emqx.appup.src
@@ -13,10 +13,12 @@
      {load_module,emqx_app,brutal_purge,soft_purge,[]},
      {load_module,emqx_ctl,brutal_purge,soft_purge,[]},
      {load_module,emqx_cm,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_connection,brutal_purge,soft_purge,[]}]},
    {"4.3.12",
     [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
      {load_module,emqx_frame,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys_mon,brutal_purge,soft_purge,[]},
      {load_module,emqx_pmon,brutal_purge,soft_purge,[]},
      {load_module,emqx_banned,brutal_purge,soft_purge,[]},
@@ -40,6 +42,7 @@
     [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
      {load_module,emqx_pmon,brutal_purge,soft_purge,[]},
      {load_module,emqx_banned,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys,brutal_purge,soft_purge,[]},
      {load_module,emqx_cm,brutal_purge,soft_purge,[]},
      {load_module,emqx_vm_mon,brutal_purge,soft_purge,[]},
@@ -63,6 +66,7 @@
    {"4.3.10",
     [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
      {load_module,emqx_pmon,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_banned,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys,brutal_purge,soft_purge,[]},
      {load_module,emqx_cm,brutal_purge,soft_purge,[]},
@@ -87,6 +91,7 @@
    {"4.3.9",
     [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
      {load_module,emqx_pmon,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_banned,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys,brutal_purge,soft_purge,[]},
      {load_module,emqx_vm_mon,brutal_purge,soft_purge,[]},
@@ -115,6 +120,7 @@
    {"4.3.8",
     [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
      {load_module,emqx_pmon,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_banned,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys,brutal_purge,soft_purge,[]},
      {load_module,emqx_vm_mon,brutal_purge,soft_purge,[]},
@@ -410,6 +416,7 @@
      {load_module,emqx_frame,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys_mon,brutal_purge,soft_purge,[]},
      {load_module,emqx_pmon,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_banned,brutal_purge,soft_purge,[]},
      {load_module,emqx_os_mon,brutal_purge,soft_purge,[]},
      {load_module,emqx_channel,brutal_purge,soft_purge,[]},
@@ -422,6 +429,7 @@
     [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
      {load_module,emqx_frame,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys_mon,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_pmon,brutal_purge,soft_purge,[]},
      {load_module,emqx_banned,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys,brutal_purge,soft_purge,[]},
@@ -442,6 +450,7 @@
    {"4.3.11",
     [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
      {load_module,emqx_pmon,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_banned,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys,brutal_purge,soft_purge,[]},
      {load_module,emqx_cm,brutal_purge,soft_purge,[]},
@@ -465,6 +474,7 @@
    {"4.3.10",
     [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
      {load_module,emqx_pmon,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_banned,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys,brutal_purge,soft_purge,[]},
      {load_module,emqx_cm,brutal_purge,soft_purge,[]},
@@ -488,6 +498,7 @@
    {"4.3.9",
     [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
      {load_module,emqx_pmon,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_banned,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys,brutal_purge,soft_purge,[]},
      {load_module,emqx_vm_mon,brutal_purge,soft_purge,[]},
@@ -515,6 +526,7 @@
    {"4.3.8",
     [{load_module,emqx_plugins,brutal_purge,soft_purge,[]},
      {load_module,emqx_pmon,brutal_purge,soft_purge,[]},
+     {load_module,emqx_misc,brutal_purge,soft_purge,[]},
      {load_module,emqx_banned,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys,brutal_purge,soft_purge,[]},
      {load_module,emqx_vm_mon,brutal_purge,soft_purge,[]},

From bfadcebb9c026158715e013711bb119600b511d4 Mon Sep 17 00:00:00 2001
From: zhongwencool <zhongwencool@gmail.com>
Date: Wed, 16 Mar 2022 15:17:56 +0800
Subject: [PATCH 5/7] chore: remind the user to delete bad appid/username.

---
 apps/emqx_management/src/emqx_mgmt_auth.erl   | 22 +++++++--
 apps/emqx_management/src/emqx_mgmt_sup.erl    |  2 +-
 .../src/emqx_dashboard_admin.erl              | 17 ++++++-
 src/emqx_misc.erl                             | 47 ++++++++++---------
 4 files changed, 59 insertions(+), 29 deletions(-)

diff --git a/apps/emqx_management/src/emqx_mgmt_auth.erl b/apps/emqx_management/src/emqx_mgmt_auth.erl
index 6793c9885..e48132e71 100644
--- a/apps/emqx_management/src/emqx_mgmt_auth.erl
+++ b/apps/emqx_management/src/emqx_mgmt_auth.erl
@@ -16,6 +16,8 @@
 
 -module(emqx_mgmt_auth).
 
+-include_lib("emqx/include/logger.hrl").
+
 %% Mnesia Bootstrap
 -export([mnesia/1]).
 -boot_mnesia({mnesia, [boot]}).
@@ -35,6 +37,8 @@
         , list_apps/0
         ]).
 
+-export([abnormal_appid_warning/0]).
+
 %% APP Auth/ACL API
 -export([is_authorized/2]).
 
@@ -89,7 +93,7 @@ add_app(AppId, Name, Desc, Status, Expired) when is_binary(AppId) ->
       -> {ok, appsecret()}
        | {error, term()}).
 add_app(AppId, Name, Secret, Desc, Status, Expired) when is_binary(AppId) ->
-    case emqx_misc:valid_str(Name) of
+    case emqx_misc:is_sane_id(AppId) of
         ok ->
             Secret1 = generate_appsecret_if_need(Secret),
             App = #mqtt_app{id = AppId,
@@ -101,7 +105,7 @@ add_app(AppId, Name, Secret, Desc, Status, Expired) when is_binary(AppId) ->
             AddFun = fun() ->
                 case mnesia:wread({mqtt_app, AppId}) of
                     [] -> mnesia:write(App);
-                    _  -> mnesia:abort(alread_existed)
+                    _  -> mnesia:abort(already_existed)
                 end
                      end,
             case mnesia:transaction(AddFun) of
@@ -112,7 +116,7 @@ add_app(AppId, Name, Secret, Desc, Status, Expired) when is_binary(AppId) ->
     end.
 
 force_add_app(AppId, Name, Secret, Desc, Status, Expired) ->
-    case emqx_misc:valid_str(Name) of
+    case emqx_misc:is_sane_id(AppId) of
         ok ->
             AddFun = fun() ->
                 mnesia:write(#mqtt_app{
@@ -216,3 +220,15 @@ is_authorized(AppId, AppSecret) ->
 
 is_expired(undefined) -> true;
 is_expired(Expired)   -> Expired >= erlang:system_time(second).
+
+abnormal_appid_warning() ->
+    lists:foreach(fun(Id) ->
+        case emqx_misc:is_sane_id(Id) of
+            ok -> ok;
+            {error, _} ->
+                ?LOG(warning,
+                    "[app] ~ts is not a sane appid(^[A-Za-z0-9]+[A-Za-z0-9-_]*$). "
+                    "Please use `emqx_ctl mgmt delete ~ts` to delete it and create a new one.",
+                    [Id, Id])
+        end
+                  end, mnesia:dirty_all_keys(mqtt_app)).
diff --git a/apps/emqx_management/src/emqx_mgmt_sup.erl b/apps/emqx_management/src/emqx_mgmt_sup.erl
index f3f5545f2..ef3856654 100644
--- a/apps/emqx_management/src/emqx_mgmt_sup.erl
+++ b/apps/emqx_management/src/emqx_mgmt_sup.erl
@@ -23,8 +23,8 @@
 -export([init/1]).
 
 start_link() ->
+    emqx_mgmt_auth:abnormal_appid_warning(),
 	supervisor:start_link({local, ?MODULE}, ?MODULE, []).
 
 init([]) ->
 	{ok, {{one_for_one, 1, 5}, []}}.
-
diff --git a/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl b/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl
index 7ce1f564f..c9327aedf 100644
--- a/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl
+++ b/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl
@@ -78,7 +78,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:valid_str(Username) of
+    case emqx_misc:is_sane_id(Username) of
         ok ->
             Admin = #mqtt_admin{username = Username, password = hash(Password), tags = Tags},
             return(mnesia:transaction(fun add_user_/1, [Admin]));
@@ -86,7 +86,7 @@ add_user(Username, Password, Tags) when is_binary(Username), is_binary(Password)
     end.
 
 force_add_user(Username, Password, Tags) ->
-    case emqx_misc:valid_str(Username) of
+    case emqx_misc:is_sane_id(Username) of
         ok ->
             AddFun = fun() ->
                 mnesia:write(#mqtt_admin{username = Username, password = Password, tags = Tags})
@@ -188,6 +188,7 @@ check(Username, Password) ->
 init([]) ->
     %% Add default admin user
     _ = add_default_user(binenv(default_user_username), binenv(default_user_passwd)),
+    abnormal_username_warning(),
     {ok, state}.
 
 handle_call(_Req, _From, State) ->
@@ -256,3 +257,15 @@ add_default_user(Username, Password) ->
             end
     end,
     ok.
+
+abnormal_username_warning() ->
+    lists:foreach(fun(Name) ->
+        case emqx_misc:is_sane_id(Name) of
+            ok -> ok;
+            {error, _} ->
+                ?LOG(warning,
+                    "[dashboard] `~ts` is not a sane username(^[A-Za-z0-9]+[A-Za-z0-9-_]*$). "
+                    "Please use `emqx_ctl admins del ~ts` to delete it and create a new one.",
+                    [Name, Name])
+        end
+                  end, mnesia:dirty_all_keys(mqtt_admin)).
diff --git a/src/emqx_misc.erl b/src/emqx_misc.erl
index 8c76ff4a6..3c866b732 100644
--- a/src/emqx_misc.erl
+++ b/src/emqx_misc.erl
@@ -52,13 +52,13 @@
         , hexstr2bin/1
         ]).
 
--export([ valid_str/1
+-export([ is_sane_id/1
         ]).
 
--define(VALID_STR_RE, "^[A-Za-z]+[A-Za-z0-9-_]*$").
+-define(VALID_STR_RE, "^[A-Za-z0-9]+[A-Za-z0-9-_]*$").
 
--spec valid_str(list() | binary()) -> ok | {error, Reason::binary()}.
-valid_str(Str) ->
+-spec is_sane_id(list() | binary()) -> ok | {error, Reason::binary()}.
+is_sane_id(Str) ->
     StrLen = len(Str),
     case StrLen > 0 andalso StrLen =< 256 of
         true ->
@@ -329,29 +329,30 @@ hexchar2int(I) when I >= $a andalso I =< $f -> I - $a + 10.
 ipv6_probe_test() ->
     ?assertEqual([{ipv6_probe, true}], ipv6_probe([])).
 
-valid_str_test() ->
-    ?assertMatch({error, _}, valid_str("")),
-    ?assertMatch({error, _}, valid_str("_")),
-    ?assertMatch({error, _}, valid_str("_aaa")),
-    ?assertMatch({error, _}, valid_str("lkad/oddl")),
-    ?assertMatch({error, _}, valid_str("lkad*oddl")),
-    ?assertMatch({error, _}, valid_str("<script>lkadoddl")),
-    ?assertMatch({error, _}, valid_str("1lkdfaldk")),
-    ?assertMatch({error, _}, valid_str("1223333434")),
+is_sane_id_test() ->
+    ?assertMatch({error, _}, is_sane_id("")),
+    ?assertMatch({error, _}, is_sane_id("_")),
+    ?assertMatch({error, _}, is_sane_id("_aaa")),
+    ?assertMatch({error, _}, is_sane_id("lkad/oddl")),
+    ?assertMatch({error, _}, is_sane_id("lkad*oddl")),
+    ?assertMatch({error, _}, is_sane_id("script>lkadoddl")),
+    ?assertMatch({error, _}, is_sane_id("<script>lkadoddl")),
 
-    ?assertMatch(ok, valid_str(<<"Abckdf_lkdfd_1222">>)),
-    ?assertMatch(ok, valid_str("Abckdf_lkdfd_1222")),
-    ?assertMatch(ok, valid_str("abckdf_lkdfd_1222")),
-    ?assertMatch(ok, valid_str("abckdflkdfd1222")),
-    ?assertMatch(ok, valid_str("abckdflkdf")),
-    ?assertMatch(ok, valid_str("a1122222")),
+    ?assertMatch(ok, is_sane_id(<<"Abckdf_lkdfd_1222">>)),
+    ?assertMatch(ok, is_sane_id("Abckdf_lkdfd_1222")),
+    ?assertMatch(ok, is_sane_id("abckdf_lkdfd_1222")),
+    ?assertMatch(ok, is_sane_id("abckdflkdfd1222")),
+    ?assertMatch(ok, is_sane_id("abckdflkdf")),
+    ?assertMatch(ok, is_sane_id("a1122222")),
+    ?assertMatch(ok, is_sane_id("1223333434")),
+    ?assertMatch(ok, is_sane_id("1lkdfaldk")),
 
     Ok = lists:flatten(lists:duplicate(256, "a")),
     Bad = Ok ++ "a",
-    ?assertMatch(ok, valid_str(Ok)),
-    ?assertMatch(ok, valid_str(list_to_binary(Ok))),
-    ?assertMatch({error, _}, valid_str(Bad)),
-    ?assertMatch({error, _}, valid_str(list_to_binary(Bad))),
+    ?assertMatch(ok, is_sane_id(Ok)),
+    ?assertMatch(ok, is_sane_id(list_to_binary(Ok))),
+    ?assertMatch({error, _}, is_sane_id(Bad)),
+    ?assertMatch({error, _}, is_sane_id(list_to_binary(Bad))),
     ok.
 
 -endif.

From 8cf97a93af51f8d0c8c86dd6c24f2791363bfecc Mon Sep 17 00:00:00 2001
From: zhongwencool <zhongwencool@gmail.com>
Date: Wed, 16 Mar 2022 16:33:03 +0800
Subject: [PATCH 6/7] fix: urldecode appid and username.

---
 apps/emqx_management/src/emqx_mgmt_api_apps.erl    |  6 +++---
 apps/emqx_management/src/emqx_mgmt_auth.erl        | 14 --------------
 apps/emqx_management/src/emqx_mgmt_sup.erl         |  1 -
 lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl | 13 -------------
 lib-ce/emqx_dashboard/src/emqx_dashboard_api.erl   |  8 ++++----
 5 files changed, 7 insertions(+), 35 deletions(-)

diff --git a/apps/emqx_management/src/emqx_mgmt_api_apps.erl b/apps/emqx_management/src/emqx_mgmt_api_apps.erl
index cca0b41f0..abcfe1961 100644
--- a/apps/emqx_management/src/emqx_mgmt_api_apps.erl
+++ b/apps/emqx_management/src/emqx_mgmt_api_apps.erl
@@ -68,7 +68,7 @@ add_app(_Bindings, Params) ->
     end.
 
 del_app(#{appid := AppId}, _Params) ->
-    case emqx_mgmt_auth:del_app(AppId) of
+    case emqx_mgmt_auth:del_app(emqx_mgmt_util:urldecode(AppId)) of
         ok -> minirest:return();
         {error, Reason} -> minirest:return({error, Reason})
     end.
@@ -77,7 +77,7 @@ list_apps(_Bindings, _Params) ->
     minirest:return({ok, [format(Apps)|| Apps <- emqx_mgmt_auth:list_apps()]}).
 
 lookup_app(#{appid := AppId}, _Params) ->
-    case emqx_mgmt_auth:lookup_app(AppId) of
+    case emqx_mgmt_auth:lookup_app(emqx_mgmt_util:urldecode(AppId)) of
         {AppId, AppSecret, Name, Desc, Status, Expired} ->
             minirest:return({ok, #{app_id => AppId,
                           secret => AppSecret,
@@ -94,7 +94,7 @@ update_app(#{appid := AppId}, Params) ->
     Desc = proplists:get_value(<<"desc">>, Params),
     Status = proplists:get_value(<<"status">>, Params),
     Expired = proplists:get_value(<<"expired">>, Params),
-    case emqx_mgmt_auth:update_app(AppId, Name, Desc, Status, Expired) of
+    case emqx_mgmt_auth:update_app(emqx_mgmt_util:urldecode(AppId), Name, Desc, Status, Expired) of
         ok -> minirest:return();
         {error, Reason} -> minirest:return({error, Reason})
     end.
diff --git a/apps/emqx_management/src/emqx_mgmt_auth.erl b/apps/emqx_management/src/emqx_mgmt_auth.erl
index e48132e71..5f2b96dff 100644
--- a/apps/emqx_management/src/emqx_mgmt_auth.erl
+++ b/apps/emqx_management/src/emqx_mgmt_auth.erl
@@ -37,8 +37,6 @@
         , list_apps/0
         ]).
 
--export([abnormal_appid_warning/0]).
-
 %% APP Auth/ACL API
 -export([is_authorized/2]).
 
@@ -220,15 +218,3 @@ is_authorized(AppId, AppSecret) ->
 
 is_expired(undefined) -> true;
 is_expired(Expired)   -> Expired >= erlang:system_time(second).
-
-abnormal_appid_warning() ->
-    lists:foreach(fun(Id) ->
-        case emqx_misc:is_sane_id(Id) of
-            ok -> ok;
-            {error, _} ->
-                ?LOG(warning,
-                    "[app] ~ts is not a sane appid(^[A-Za-z0-9]+[A-Za-z0-9-_]*$). "
-                    "Please use `emqx_ctl mgmt delete ~ts` to delete it and create a new one.",
-                    [Id, Id])
-        end
-                  end, mnesia:dirty_all_keys(mqtt_app)).
diff --git a/apps/emqx_management/src/emqx_mgmt_sup.erl b/apps/emqx_management/src/emqx_mgmt_sup.erl
index ef3856654..8ad0bc963 100644
--- a/apps/emqx_management/src/emqx_mgmt_sup.erl
+++ b/apps/emqx_management/src/emqx_mgmt_sup.erl
@@ -23,7 +23,6 @@
 -export([init/1]).
 
 start_link() ->
-    emqx_mgmt_auth:abnormal_appid_warning(),
 	supervisor:start_link({local, ?MODULE}, ?MODULE, []).
 
 init([]) ->
diff --git a/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl b/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl
index c9327aedf..420380f88 100644
--- a/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl
+++ b/lib-ce/emqx_dashboard/src/emqx_dashboard_admin.erl
@@ -188,7 +188,6 @@ check(Username, Password) ->
 init([]) ->
     %% Add default admin user
     _ = add_default_user(binenv(default_user_username), binenv(default_user_passwd)),
-    abnormal_username_warning(),
     {ok, state}.
 
 handle_call(_Req, _From, State) ->
@@ -257,15 +256,3 @@ add_default_user(Username, Password) ->
             end
     end,
     ok.
-
-abnormal_username_warning() ->
-    lists:foreach(fun(Name) ->
-        case emqx_misc:is_sane_id(Name) of
-            ok -> ok;
-            {error, _} ->
-                ?LOG(warning,
-                    "[dashboard] `~ts` is not a sane username(^[A-Za-z0-9]+[A-Za-z0-9-_]*$). "
-                    "Please use `emqx_ctl admins del ~ts` to delete it and create a new one.",
-                    [Name, Name])
-        end
-                  end, mnesia:dirty_all_keys(mqtt_admin)).
diff --git a/lib-ce/emqx_dashboard/src/emqx_dashboard_api.erl b/lib-ce/emqx_dashboard/src/emqx_dashboard_api.erl
index e1c89efbb..4dbf2517f 100644
--- a/lib-ce/emqx_dashboard/src/emqx_dashboard_api.erl
+++ b/lib-ce/emqx_dashboard/src/emqx_dashboard_api.erl
@@ -77,9 +77,10 @@ auth(_Bindings, Params) ->
     Password = proplists:get_value(<<"password">>, Params),
     return(emqx_dashboard_admin:check(Username, Password)).
 
-change_pwd(#{username := Username}, Params) ->
+change_pwd(#{username := Username0}, Params) ->
     OldPwd = proplists:get_value(<<"old_pwd">>, Params),
     NewPwd = proplists:get_value(<<"new_pwd">>, Params),
+    Username = emqx_mgmt_util:urldecode(Username0),
     return(emqx_dashboard_admin:change_password(Username, OldPwd, NewPwd)).
 
 create(_Bindings, Params) ->
@@ -96,14 +97,13 @@ list(_Bindings, _Params) ->
 
 update(#{name := Username}, Params) ->
     Tags = proplists:get_value(<<"tags">>, Params),
-    return(emqx_dashboard_admin:update_user(Username, Tags)).
+    return(emqx_dashboard_admin:update_user(emqx_mgmt_util:urldecode(Username), Tags)).
 
 delete(#{name := <<"admin">>}, _Params) ->
     return({error, <<"Cannot delete admin">>});
 
 delete(#{name := Username}, _Params) ->
-    return(emqx_dashboard_admin:remove_user(Username)).
+    return(emqx_dashboard_admin:remove_user(emqx_mgmt_util:urldecode(Username))).
 
 row(#mqtt_admin{username = Username, tags = Tags}) ->
     #{username => Username, tags => Tags}.
-

From f0a81d593d93ae27a1ef377cd00f3cd6801525ef Mon Sep 17 00:00:00 2001
From: zhongwencool <zhongwencool@gmail.com>
Date: Wed, 16 Mar 2022 16:48:03 +0800
Subject: [PATCH 7/7] fix: dashboard ct failed

---
 .../emqx_dashboard/test/emqx_dashboard_SUITE.erl   | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl b/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl
index 3bca197f0..5099d4449 100644
--- a/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl
+++ b/lib-ce/emqx_dashboard/test/emqx_dashboard_SUITE.erl
@@ -67,12 +67,13 @@ t_overview(_) ->
 t_admins_add_delete(_) ->
     ok = emqx_dashboard_admin:add_user(<<"username">>, <<"password">>, <<"tag">>),
     ok = emqx_dashboard_admin:add_user(<<"username1">>, <<"password1">>, <<"tag1">>),
-    {error, _} = emqx_dashboard_admin:add_user(<<"1username1">>, <<"password1">>, <<"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(3, length(Admins)),
+    ?assertEqual(4, length(Admins)),
     ok = emqx_dashboard_admin:remove_user(<<"username1">>),
+    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">>),
@@ -80,13 +81,14 @@ t_admins_add_delete(_) ->
     ?assert(request_dashboard(get, api_path("brokers"), auth_header_("username", "pwd"))),
 
     ok = emqx_dashboard_admin:remove_user(<<"username">>),
-    ?assertNotEqual(true, request_dashboard(get, api_path("brokers"), auth_header_("username", "pwd"))).
+    ?assertNotEqual(true, request_dashboard(get, api_path("brokers"),
+        auth_header_("username", "pwd"))).
 
 t_rest_api(_Config) ->
     {ok, Res0} = http_get("users"),
-
-    ?assertEqual([#{<<"username">> => <<"admin">>,
-                    <<"tags">> => <<"administrator">>}], get_http_data(Res0)),
+    Users = get_http_data(Res0),
+    ?assert(lists:member(#{<<"username">> => <<"admin">>, <<"tags">> => <<"administrator">>},
+        Users)),
 
     AssertSuccess = fun({ok, Res}) ->
                         ?assertEqual(#{<<"code">> => 0}, json(Res))