fix(dashbaord): fix test cases to support RBAC

apps/emqx_dashboard/include/emqx_dashboard.hrl

@@ -15,10 +15,20 @@
 %%--------------------------------------------------------------------
 -define(ADMIN, emqx_admin).
 
+%% TODO:
+%% The predefined roles of the preliminary RBAC implementation,
+%% these may be removed when developing the full RBAC feature.
+%% In full RBAC feature, the role may be customised created and deleted,
+%% a predefined configuration would replace these macros.
 -define(ROLE_VIEWER, <<"viewer">>).
--define(ROLE_DEFAULT, ?ROLE_VIEWER).
 -define(ROLE_SUPERUSER, <<"superuser">>).
 
+-if(?EMQX_RELEASE_EDITION == ee).
+-define(ROLE_DEFAULT, ?ROLE_VIEWER).
+-else.
+-define(ROLE_DEFAULT, ?ROLE_SUPERUSER).
+-endif.
+
 -record(?ADMIN, {
     username :: binary(),
     pwdhash :: binary(),

apps/emqx_dashboard/src/emqx_dashboard.erl

@@ -213,7 +213,8 @@ authorize(Req) ->
                 {error, not_found} ->
                     {401, 'BAD_TOKEN', <<"Get a token by POST /login">>};
                 {error, unauthorized_role} ->
-                    {401, 'UNAUTHORIZED_ROLE', <<"Unauthorized Role">>}
+                    {403, 'UNAUTHORIZED_ROLE',
+                        <<"You don't have permission to access this resource">>}
             end;
         _ ->
             return_unauthorized(

apps/emqx_dashboard/src/emqx_dashboard_admin.erl

@@ -196,7 +196,12 @@ force_add_user(Username, Password, Role, Desc) ->
 add_user_(Username, Password, Role, Desc) ->
     case mnesia:wread({?ADMIN, Username}) of
         [] ->
-            Admin = #?ADMIN{username = Username, pwdhash = hash(Password), description = Desc},
+            Admin = #?ADMIN{
+                username = Username,
+                pwdhash = hash(Password),
+                role = Role,
+                description = Desc
+            },
             mnesia:write(Admin),
             #{username => Username, role => Role, description => Desc};
         [_] ->
@@ -305,12 +310,14 @@ all_users() ->
         fun(
             #?ADMIN{
                 username = Username,
-                description = Desc
+                description = Desc,
+                role = Role
             }
         ) ->
             #{
                 username => Username,
-                description => Desc
+                description => Desc,
+                role => ensure_role(Role)
             }
         end,
         ets:tab2list(?ADMIN)
@@ -374,12 +381,21 @@ add_default_user(Username, Password) ->
         _ -> {ok, default_user_exists}
     end.
 
+%% ensure the `role` is correct when it directly read from the table
+%% this value in old data is `undefined`
+ensure_role(undefined) ->
+    ?ROLE_SUPERUSER;
+ensure_role(Role) when is_binary(Role) ->
+    Role.
+
 -if(?EMQX_RELEASE_EDITION == ee).
 legal_role(Role) ->
     emqx_dashboard_rbac:legal_role(Role).
 
 -else.
 
+-dialyzer({no_match, [add_user/4, update_user/3]}).
+
 legal_role(_) ->
     ok.
 

apps/emqx_dashboard/src/emqx_dashboard_api.erl

@@ -206,7 +206,7 @@ field(old_pwd) ->
 field(new_pwd) ->
     {new_pwd, mk(binary(), #{desc => ?DESC(new_pwd)})};
 field(role) ->
-    {role, mk(binary(), #{desc => ?DESC(role)})}.
+    {role, mk(binary(), #{desc => ?DESC(role), example => ?ROLE_DEFAULT})}.
 
 %% -------------------------------------------------------------------------------------------------
 %% API
@@ -369,7 +369,9 @@ field_filter(role) ->
 field_filter(_) ->
     true.
 
+filter_result(Result) when is_list(Result) ->
+    lists:map(fun filter_result/1, Result);
 filter_result(Result) ->
-    maps:without([Role], Result).
+    maps:without([role], Result).
 
 -endif.

apps/emqx_dashboard/src/emqx_dashboard_token.erl

@@ -104,7 +104,7 @@ mnesia(boot) ->
 
 %%--------------------------------------------------------------------
 %% jwt apply
-do_sign(#?ADMIN{username = Username, extra = Extra}, Password) ->
+do_sign(#?ADMIN{username = Username} = User, Password) ->
     ExpTime = jwt_expiration_time(),
     Salt = salt(),
     JWK = jwk(Username, Password, Salt),
@@ -117,7 +117,8 @@ do_sign(#?ADMIN{username = Username, extra = Extra}, Password) ->
     },
     Signed = jose_jwt:sign(JWK, JWS, JWT),
     {_, Token} = jose_jws:compact(Signed),
-    JWTRec = format(Token, Username, ExpTime, Extra),
+    Role = role(User),
+    JWTRec = format(Token, Username, Role, ExpTime),
     _ = mria:transaction(?DASHBOARD_SHARD, fun mnesia:write/1, [JWTRec]),
     {ok, Token}.
 
@@ -191,12 +192,12 @@ jwt_expiration_time() ->
 token_ttl() ->
     emqx_conf:get([dashboard, token_expired_time], ?EXPTIME).
 
-format(Token, Username, ExpTime, Extra) ->
+format(Token, Username, Role, ExpTime) ->
     #?ADMIN_JWT{
         token = Token,
         username = Username,
         exptime = ExpTime,
-        extra = #{role => role(Extra)}
+        extra = #{role => Role}
     }.
 
 %%--------------------------------------------------------------------
@@ -254,11 +255,12 @@ role(Data) ->
 -else.
 
 -dialyzer({nowarn_function, [check_rbac/2]}).
+-dialyzer({no_match, [do_verify/2]}).
 
 check_rbac(_Req, _Extra) ->
     true.
 
 role(_) ->
-    undefined.
+    ?ROLE_DEFAULT.
 
 -endif.

apps/emqx_dashboard/test/emqx_dashboard_SUITE.erl

@@ -67,7 +67,9 @@ end_per_suite(_Config) ->
 
 t_overview(_) ->
     mnesia:clear_table(?ADMIN),
-    emqx_dashboard_admin:add_user(<<"admin">>, <<"public_www1">>, <<"simple_description">>),
+    emqx_dashboard_admin:add_user(
+        <<"admin">>, <<"public_www1">>, ?ROLE_SUPERUSER, <<"simple_description">>
+    ),
     Headers = auth_header_(<<"admin">>, <<"public_www1">>),
     [
         {ok, _} = request_dashboard(get, api_path([Overview]), Headers)
@@ -77,8 +79,12 @@ t_overview(_) ->
 t_admins_add_delete(_) ->
     mnesia:clear_table(?ADMIN),
     Desc = <<"simple description">>,
-    {ok, _} = emqx_dashboard_admin:add_user(<<"username">>, <<"password_0">>, Desc),
-    {ok, _} = emqx_dashboard_admin:add_user(<<"username1">>, <<"password1">>, Desc),
+    {ok, _} = emqx_dashboard_admin:add_user(
+        <<"username">>, <<"password_0">>, ?ROLE_SUPERUSER, Desc
+    ),
+    {ok, _} = emqx_dashboard_admin:add_user(
+        <<"username1">>, <<"password1">>, ?ROLE_SUPERUSER, Desc
+    ),
     Admins = emqx_dashboard_admin:all_users(),
     ?assertEqual(2, length(Admins)),
     {ok, _} = emqx_dashboard_admin:remove_user(<<"username1">>),
@@ -95,7 +101,7 @@ t_admins_add_delete(_) ->
 t_admin_delete_self_failed(_) ->
     mnesia:clear_table(?ADMIN),
     Desc = <<"simple description">>,
-    _ = emqx_dashboard_admin:add_user(<<"username1">>, <<"password_1">>, Desc),
+    _ = emqx_dashboard_admin:add_user(<<"username1">>, <<"password_1">>, ?ROLE_SUPERUSER, Desc),
     Admins = emqx_dashboard_admin:all_users(),
     ?assertEqual(1, length(Admins)),
     Header = auth_header_(<<"username1">>, <<"password_1">>),
@@ -109,7 +115,7 @@ t_rest_api(_Config) ->
     mnesia:clear_table(?ADMIN),
     Desc = <<"administrator">>,
     Password = <<"public_www1">>,
-    emqx_dashboard_admin:add_user(<<"admin">>, Password, Desc),
+    emqx_dashboard_admin:add_user(<<"admin">>, Password, ?ROLE_SUPERUSER, Desc),
     {ok, 200, Res0} = http_get(["users"]),
     ?assertEqual(
         [
@@ -120,12 +126,22 @@ t_rest_api(_Config) ->
         ],
         get_http_data(Res0)
     ),
-    {ok, 200, _} = http_put(["users", "admin"], #{<<"description">> => <<"a_new_description">>}),
-    {ok, 200, _} = http_post(["users"], #{
-        <<"username">> => <<"usera">>,
-        <<"password">> => <<"passwd_01234">>,
-        <<"description">> => Desc
-    }),
+    {ok, 200, _} = http_put(
+        ["users", "admin"],
+        filter_req(#{
+            <<"role">> => ?ROLE_SUPERUSER,
+            <<"description">> => <<"a_new_description">>
+        })
+    ),
+    {ok, 200, _} = http_post(
+        ["users"],
+        filter_req(#{
+            <<"username">> => <<"usera">>,
+            <<"password">> => <<"passwd_01234">>,
+            <<"role">> => ?ROLE_SUPERUSER,
+            <<"description">> => Desc
+        })
+    ),
     {ok, 204, _} = http_delete(["users", "usera"]),
     {ok, 404, _} = http_delete(["users", "usera"]),
     {ok, 204, _} = http_post(
@@ -136,7 +152,7 @@ t_rest_api(_Config) ->
         }
     ),
     mnesia:clear_table(?ADMIN),
-    emqx_dashboard_admin:add_user(<<"admin">>, Password, <<"administrator">>),
+    emqx_dashboard_admin:add_user(<<"admin">>, Password, ?ROLE_SUPERUSER, <<"administrator">>),
     ok.
 
 t_swagger_json(_Config) ->
@@ -180,7 +196,7 @@ t_cli(_Config) ->
 t_lookup_by_username_jwt(_Config) ->
     User = bin(["user-", integer_to_list(random_num())]),
     Pwd = bin("t_password" ++ integer_to_list(random_num())),
-    emqx_dashboard_token:sign(User, Pwd),
+    emqx_dashboard_token:sign(#?ADMIN{username = User}, Pwd),
     ?assertMatch(
         [#?ADMIN_JWT{username = User}],
         emqx_dashboard_token:lookup_by_username(User)
@@ -194,7 +210,7 @@ t_lookup_by_username_jwt(_Config) ->
 t_clean_expired_jwt(_Config) ->
     User = bin(["user-", integer_to_list(random_num())]),
     Pwd = bin("t_password" ++ integer_to_list(random_num())),
-    emqx_dashboard_token:sign(User, Pwd),
+    emqx_dashboard_token:sign(#?ADMIN{username = User}, Pwd),
     [#?ADMIN_JWT{username = User, exptime = ExpTime}] =
         emqx_dashboard_token:lookup_by_username(User),
     ok = emqx_dashboard_token:clean_expired_jwt(_Now1 = ExpTime),
@@ -261,3 +277,14 @@ api_path(Parts) ->
 json(Data) ->
     {ok, Jsx} = emqx_utils_json:safe_decode(Data, [return_maps]),
     Jsx.
+
+-if(?EMQX_RELEASE_EDITION == ee).
+filter_req(Req) ->
+    Req.
+
+-else.
+
+filter_req(Req) ->
+    maps:without([role, <<"role">>], Req).
+
+-endif.

apps/emqx_dashboard/test/emqx_dashboard_admin_SUITE.erl

@@ -42,8 +42,8 @@ t_check_user(_) ->
     BadPassword = <<"public_bad">>,
     EmptyUsername = <<>>,
     EmptyPassword = <<>>,
-    {ok, _} = emqx_dashboard_admin:add_user(Username, Password, <<"desc">>),
-    ok = emqx_dashboard_admin:check(Username, Password),
+    {ok, _} = emqx_dashboard_admin:add_user(Username, Password, ?ROLE_SUPERUSER, <<"desc">>),
+    {ok, _} = emqx_dashboard_admin:check(Username, Password),
     {error, <<"password_error">>} = emqx_dashboard_admin:check(Username, BadPassword),
     {error, <<"username_not_found">>} = emqx_dashboard_admin:check(BadUsername, Password),
     {error, <<"username_not_found">>} = emqx_dashboard_admin:check(BadUsername, BadPassword),
@@ -61,19 +61,23 @@ t_add_user(_) ->
     BadAddUser = <<"***add_user_bad">>,
 
     %% add success. not return password
-    {ok, NewUser} = emqx_dashboard_admin:add_user(AddUser, AddPassword, AddDescription),
+    {ok, NewUser} = emqx_dashboard_admin:add_user(
+        AddUser, AddPassword, ?ROLE_SUPERUSER, AddDescription
+    ),
     AddUser = maps:get(username, NewUser),
     AddDescription = maps:get(description, NewUser),
     false = maps:is_key(password, NewUser),
 
     %% add again
     {error, <<"username_already_exist">>} =
-        emqx_dashboard_admin:add_user(AddUser, AddPassword, AddDescription),
+        emqx_dashboard_admin:add_user(AddUser, AddPassword, ?ROLE_SUPERUSER, AddDescription),
 
     %% add bad username
     BadNameError =
         <<"Bad Username. Only upper and lower case letters, numbers and underscores are supported">>,
-    {error, BadNameError} = emqx_dashboard_admin:add_user(BadAddUser, AddPassword, AddDescription),
+    {error, BadNameError} = emqx_dashboard_admin:add_user(
+        BadAddUser, AddPassword, ?ROLE_SUPERUSER, AddDescription
+    ),
     ok.
 
 t_lookup_user(_) ->
@@ -84,7 +88,9 @@ t_lookup_user(_) ->
     BadLookupUser = <<"***lookup_user_bad">>,
 
     {ok, _} =
-        emqx_dashboard_admin:add_user(LookupUser, LookupPassword, LookupDescription),
+        emqx_dashboard_admin:add_user(
+            LookupUser, LookupPassword, ?ROLE_SUPERUSER, LookupDescription
+        ),
     %% lookup success. not return password
     [#emqx_admin{username = LookupUser, description = LookupDescription}] =
         emqx_dashboard_admin:lookup_user(LookupUser),
@@ -95,7 +101,7 @@ t_lookup_user(_) ->
 t_all_users(_) ->
     Username = <<"admin_all">>,
     Password = <<"public_2">>,
-    {ok, _} = emqx_dashboard_admin:add_user(Username, Password, <<"desc">>),
+    {ok, _} = emqx_dashboard_admin:add_user(Username, Password, ?ROLE_SUPERUSER, <<"desc">>),
     All = emqx_dashboard_admin:all_users(),
     ?assert(erlang:length(All) >= 1),
     ok.
@@ -108,7 +114,9 @@ t_delete_user(_) ->
     DeleteBadUser = <<"delete_user_bad">>,
 
     {ok, _NewUser} =
-        emqx_dashboard_admin:add_user(DeleteUser, DeletePassword, DeleteDescription),
+        emqx_dashboard_admin:add_user(
+            DeleteUser, DeletePassword, ?ROLE_SUPERUSER, DeleteDescription
+        ),
     {ok, ok} = emqx_dashboard_admin:remove_user(DeleteUser),
     %% remove again
     {error, <<"username_not_found">>} = emqx_dashboard_admin:remove_user(DeleteUser),
@@ -124,13 +132,17 @@ t_update_user(_) ->
 
     BadUpdateUser = <<"update_user_bad">>,
 
-    {ok, _} = emqx_dashboard_admin:add_user(UpdateUser, UpdatePassword, UpdateDescription),
+    {ok, _} = emqx_dashboard_admin:add_user(
+        UpdateUser, UpdatePassword, ?ROLE_SUPERUSER, UpdateDescription
+    ),
     {ok, NewUserInfo} =
-        emqx_dashboard_admin:update_user(UpdateUser, NewDesc),
+        emqx_dashboard_admin:update_user(UpdateUser, ?ROLE_SUPERUSER, NewDesc),
     UpdateUser = maps:get(username, NewUserInfo),
     NewDesc = maps:get(description, NewUserInfo),
 
-    {error, <<"username_not_found">>} = emqx_dashboard_admin:update_user(BadUpdateUser, NewDesc),
+    {error, <<"username_not_found">>} = emqx_dashboard_admin:update_user(
+        BadUpdateUser, ?ROLE_SUPERUSER, NewDesc
+    ),
     ok.
 
 t_change_password(_) ->
@@ -143,7 +155,7 @@ t_change_password(_) ->
 
     BadChangeUser = <<"change_user_bad">>,
 
-    {ok, _} = emqx_dashboard_admin:add_user(User, OldPassword, Description),
+    {ok, _} = emqx_dashboard_admin:add_user(User, OldPassword, ?ROLE_SUPERUSER, Description),
 
     {ok, ok} = emqx_dashboard_admin:change_password(User, OldPassword, NewPassword),
     %% change pwd again
@@ -161,17 +173,18 @@ t_clean_token(_) ->
     Username = <<"admin_token">>,
     Password = <<"public_www1">>,
     NewPassword = <<"public_www2">>,
-    {ok, _} = emqx_dashboard_admin:add_user(Username, Password, <<"desc">>),
+    {ok, _} = emqx_dashboard_admin:add_user(Username, Password, ?ROLE_SUPERUSER, <<"desc">>),
     {ok, Token} = emqx_dashboard_admin:sign_token(Username, Password),
-    ok = emqx_dashboard_admin:verify_token(Token),
+    FakeReq = #{method => <<"get">>},
+    ok = emqx_dashboard_admin:verify_token(FakeReq, Token),
     %% change password
     {ok, _} = emqx_dashboard_admin:change_password(Username, Password, NewPassword),
     timer:sleep(5),
-    {error, not_found} = emqx_dashboard_admin:verify_token(Token),
+    {error, not_found} = emqx_dashboard_admin:verify_token(FakeReq, Token),
     %% remove user
     {ok, Token2} = emqx_dashboard_admin:sign_token(Username, NewPassword),
-    ok = emqx_dashboard_admin:verify_token(Token2),
+    ok = emqx_dashboard_admin:verify_token(FakeReq, Token2),
     {ok, _} = emqx_dashboard_admin:remove_user(Username),
     timer:sleep(5),
-    {error, not_found} = emqx_dashboard_admin:verify_token(Token2),
+    {error, not_found} = emqx_dashboard_admin:verify_token(FakeReq, Token2),
     ok.

apps/emqx_dashboard_rbac/src/emqx_dashboard_rbac.erl

@@ -16,10 +16,12 @@ check_rbac(Req, Extra) ->
     Role = role(Extra),
     check_rbac_with_method(Role, Method).
 
+%% For compatibility
 role(#?ADMIN{role = undefined}) ->
     ?ROLE_SUPERUSER;
 role(#?ADMIN{role = Role}) ->
     Role;
+%% For compatibility
 role([]) ->
     ?ROLE_SUPERUSER;
 role(#{role := Role}) ->
@@ -35,7 +37,7 @@ legal_role(Role) ->
 %% ===================================================================
 check_rbac_with_method(?ROLE_SUPERUSER, _) ->
     true;
-check_rbac_with_method(?ROLE_VIEWER, <<"get">>) ->
+check_rbac_with_method(?ROLE_VIEWER, <<"GET">>) ->
     true;
 check_rbac_with_method(_, _) ->
     false.

apps/emqx_machine/priv/reboot_lists.eterm

@@ -114,7 +114,8 @@
             emqx_node_rebalance,
             emqx_ft,
             emqx_ldap,
-            emqx_gcp_device
+            emqx_gcp_device,
+            emqx_dashboard_rbac
         ],
     %% must always be of type `load'
     ce_business_apps =>

apps/emqx_management/test/emqx_mgmt_data_backup_SUITE.erl

@@ -22,6 +22,7 @@
 -include_lib("common_test/include/ct.hrl").
 -include_lib("snabbkaffe/include/snabbkaffe.hrl").
 
+-define(ROLE_SUPERUSER, <<"superuser">>).
 -define(BOOTSTRAP_BACKUP, "emqx-export-test-bootstrap-ce.tar.gz").
 
 all() ->
@@ -297,7 +298,7 @@ t_import_on_cluster(Config) ->
 t_verify_imported_mnesia_tab_on_cluster(Config) ->
     UsersToExport = users(<<"user_to_export_">>),
     UsersBeforeImport = users(<<"user_before_import_">>),
-    [{ok, _} = emqx_dashboard_admin:add_user(U, U, U) || U <- UsersToExport],
+    [{ok, _} = emqx_dashboard_admin:add_user(U, U, ?ROLE_SUPERUSER, U) || U <- UsersToExport],
     {ok, #{filename := FileName}} = emqx_mgmt_data_backup:export(),
     {ok, Cwd} = file:get_cwd(),
     AbsFilePath = filename:join(Cwd, FileName),
@@ -305,7 +306,7 @@ t_verify_imported_mnesia_tab_on_cluster(Config) ->
     [CoreNode1, CoreNode2, ReplicantNode] = ?config(cluster, Config),
 
     [
-        {ok, _} = rpc:call(CoreNode1, emqx_dashboard_admin, add_user, [U, U, U])
+        {ok, _} = rpc:call(CoreNode1, emqx_dashboard_admin, add_user, [U, U, ?ROLE_SUPERUSER, U])
      || U <- UsersBeforeImport
     ],
 

mix.exs

@@ -226,7 +226,8 @@ defmodule EMQXUmbrella.MixProject do
       :emqx_bridge_kinesis,
       :emqx_bridge_azure_event_hub,
       :emqx_ldap,
-      :emqx_gcp_device
+      :emqx_gcp_device,
+      :emqx_dashboard_rbac
     ])
   end