Merge pull request #8286 from DDDHuang/fix_rm_dashboard_user

fix: dashboard users api, cannot delete self
2022-07-01 09:42:46 +08:00 · 2022-07-01 09:42:46 +08:00 · c64f589ef5
parent 3df86508bc f502e09f5f
commit c64f589ef5
4 changed files with 87 additions and 38 deletions
--- a/apps/emqx_dashboard/src/emqx_dashboard.appup.src
+++ b/apps/emqx_dashboard/src/emqx_dashboard.appup.src
@ -1,7 +1,13 @@
 %% -*- mode: erlang -*-
 %% Unless you know what you are doing, DO NOT edit manually!!
 {VSN,
-  [{"5.0.0",[{load_module,emqx_dashboard_api,brutal_purge,soft_purge,[]}]},
+  [{"5.0.0", [
+    {load_module,emqx_dashboard_api,brutal_purge,soft_purge,[]},
+    {load_module,emqx_dashboard_token,brutal_purge,soft_purge,[]}
+   ]},
   {<<".*">>,[]}],
-  [{"5.0.0",[{load_module,emqx_dashboard_api,brutal_purge,soft_purge,[]}]},
+  [{"5.0.0", [
+    {load_module,emqx_dashboard_api,brutal_purge,soft_purge,[]},
+    {load_module,emqx_dashboard_token,brutal_purge,soft_purge,[]}
+   ]},
   {<<".*">>,[]}]}.
--- a/apps/emqx_dashboard/src/emqx_dashboard_api.erl
+++ b/apps/emqx_dashboard/src/emqx_dashboard_api.erl
@ -272,22 +272,59 @@ user(put, #{bindings := #{username := Username}, body := Params}) ->
        {error, Reason} ->
            {404, ?USER_NOT_FOUND, Reason}
    end;
-user(delete, #{bindings := #{username := Username}}) ->
+user(delete, #{bindings := #{username := Username}, headers := Headers}) ->
    case Username == emqx_dashboard_admin:default_username() of
        true ->
            ?SLOG(info, #{msg => "Dashboard delete admin user failed", username => Username}),
            Message = list_to_binary(io_lib:format("Cannot delete user ~p", [Username])),
            {400, ?NOT_ALLOWED, Message};
        false ->
-            case emqx_dashboard_admin:remove_user(Username) of
-                {error, Reason} ->
-                    {404, ?USER_NOT_FOUND, Reason};
-                {ok, _} ->
-                    ?SLOG(info, #{msg => "Dashboard delete admin user", username => Username}),
-                    {204}
+            case is_self_auth(Username, Headers) of
+                true ->
+                    {400, ?NOT_ALLOWED, <<"Cannot delete self">>};
+                false ->
+                    case emqx_dashboard_admin:remove_user(Username) of
+                        {error, Reason} ->
+                            {404, ?USER_NOT_FOUND, Reason};
+                        {ok, _} ->
+                            ?SLOG(info, #{
+                                msg => "Dashboard delete admin user", username => Username
+                            }),
+                            {204}
+                    end
            end
    end.

+is_self_auth(Username, #{<<"authorization">> := Token}) ->
+    is_self_auth(Username, Token);
+is_self_auth(Username, #{<<"Authorization">> := Token}) ->
+    is_self_auth(Username, Token);
+is_self_auth(Username, <<"basic ", Token/binary>>) ->
+    is_self_auth_basic(Username, Token);
+is_self_auth(Username, <<"Basic ", Token/binary>>) ->
+    is_self_auth_basic(Username, Token);
+is_self_auth(Username, <<"bearer ", Token/binary>>) ->
+    is_self_auth_token(Username, Token);
+is_self_auth(Username, <<"Bearer ", Token/binary>>) ->
+    is_self_auth_token(Username, Token).
+
+is_self_auth_basic(Username, Token) ->
+    UP = base64:decode(Token),
+    case binary:match(UP, Username) of
+        {0, N} ->
+            binary:part(UP, {N, 1}) == <<":">>;
+        _ ->
+            false
+    end.
+
+is_self_auth_token(Username, Token) ->
+    case emqx_dashboard_token:owner(Token) of
+        {ok, Owner} ->
+            Owner == Username;
+        {error, _NotFound} ->
+            false
+    end.
+
 change_pwd(put, #{bindings := #{username := Username}, body := Params}) ->
    LogMeta = #{msg => "Dashboard change password", username => Username},
    OldPwd = maps:get(<<"old_pwd">>, Params),
--- a/apps/emqx_dashboard/src/emqx_dashboard_token.erl
+++ b/apps/emqx_dashboard/src/emqx_dashboard_token.erl
@ -22,6 +22,7 @@
    sign/2,
    verify/1,
    lookup/1,
+    owner/1,
    destroy/1,
    destroy_by_username/1
 ]).
@ -161,6 +162,14 @@ lookup_by_username(Username) ->
    {atomic, List} = mria:ro_transaction(?DASHBOARD_SHARD, Fun),
    List.

+-spec owner(Token :: binary()) -> {ok, Username :: binary()} | {error, not_found}.
+owner(Token) ->
+    Fun = fun() -> mnesia:read(?TAB, Token) end,
+    case mria:ro_transaction(?DASHBOARD_SHARD, Fun) of
+        {atomic, [#?ADMIN_JWT{username = Username}]} -> {ok, Username};
+        {atomic, []} -> {error, not_found}
+    end.
+
 jwk(Username, Password, Salt) ->
    Key = crypto:hash(md5, <<Salt/binary, Username/binary, Password/binary>>),
    #{
--- a/apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl
+++ b/apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl
@ -44,23 +44,17 @@
 -define(APP_MANAGEMENT, emqx_management).

 -define(OVERVIEWS, [
-    'alarms/activated',
-    'alarms/deactivated',
-    banned,
-    brokers,
-    stats,
-    metrics,
-    listeners,
-    clients,
-    subscriptions,
-    routes,
-    plugins
+    "alarms",
+    "banned",
+    "stats",
+    "metrics",
+    "listeners",
+    "clients",
+    "subscriptions"
 ]).

 all() ->
-    %% TODO: V5 API
-    %% emqx_common_test_helpers:all(?MODULE).
-    [t_cli, t_lookup_by_username_jwt, t_clean_expired_jwt, t_rest_api].
+    emqx_common_test_helpers:all(?MODULE).

 end_suite() ->
    end_suite([]).
@ -98,37 +92,40 @@ t_overview(_) ->
    mnesia:clear_table(?ADMIN),
    emqx_dashboard_admin:add_user(<<"admin">>, <<"public">>, <<"simple_description">>),
    [
-        ?assert(
-            request_dashboard(
-                get,
-                api_path(erlang:atom_to_list(Overview)),
-                auth_header_()
-            )
-        )
+        {ok, _} = request_dashboard(get, api_path([Overview]), auth_header_())
     || 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(<<"username1">>, <<"password1">>, Desc),
+    {ok, _} = emqx_dashboard_admin:add_user(<<"username">>, <<"password">>, Desc),
+    {ok, _} = emqx_dashboard_admin:add_user(<<"username1">>, <<"password1">>, Desc),
    Admins = emqx_dashboard_admin:all_users(),
    ?assertEqual(2, length(Admins)),
-    ok = emqx_dashboard_admin:remove_user(<<"username1">>),
+    {ok, _} = emqx_dashboard_admin:remove_user(<<"username1">>),
    Users = emqx_dashboard_admin:all_users(),
    ?assertEqual(1, length(Users)),
-    ok = emqx_dashboard_admin:change_password(
+    {ok, _} = emqx_dashboard_admin:change_password(
        <<"username">>,
        <<"password">>,
        <<"pwd">>
    ),
    timer:sleep(10),
-    Header = auth_header_(<<"username">>, <<"pwd">>),
-    ?assert(request_dashboard(get, api_path("brokers"), Header)),
+    {ok, _} = emqx_dashboard_admin:remove_user(<<"username">>).

-    ok = emqx_dashboard_admin:remove_user(<<"username">>),
-    ?assertNotEqual(true, request_dashboard(get, api_path("brokers"), Header)).
+t_admin_delete_self_failed(_) ->
+    mnesia:clear_table(?ADMIN),
+    Desc = <<"simple description">>,
+    _ = emqx_dashboard_admin:add_user(<<"username1">>, <<"password">>, Desc),
+    Admins = emqx_dashboard_admin:all_users(),
+    ?assertEqual(1, length(Admins)),
+    Header = auth_header_(<<"username1">>, <<"password">>),
+    {error, {_, 400, _}} = request_dashboard(delete, api_path(["users", "username1"]), Header),
+    Token = erlang:iolist_to_binary(["Basic ", base64:encode("username1:password")]),
+    Header2 = {"Authorization", Token},
+    {error, {_, 400, _}} = request_dashboard(delete, api_path(["users", "username1"]), Header2),
+    mnesia:clear_table(?ADMIN).

 t_rest_api(_Config) ->
    mnesia:clear_table(?ADMIN),