Browse Source

feat: returns user imported details

JianBo He 1 year atrás
parent
commit
df227f4a8c

+ 9 - 1
apps/emqx_auth/src/emqx_authn/emqx_authn_chains.erl

@@ -151,6 +151,14 @@ end).
     atom() => term()
 }.
 
+-type import_users_result() :: #{
+    total := non_neg_integer(),
+    success := non_neg_integer(),
+    override := non_neg_integer(),
+    skipped := non_neg_integer(),
+    failed := non_neg_integer()
+}.
+
 -export_type([authenticator/0, config/0, state/0, extra/0, user_info/0]).
 
 %%------------------------------------------------------------------------------
@@ -317,7 +325,7 @@ reorder_authenticator(ChainName, AuthenticatorIDs) ->
     authenticator_id(),
     {plain | hash, prepared_user_list | binary(), binary()}
 ) ->
-    ok | {error, term()}.
+    {ok, import_users_result()} | {error, term()}.
 import_users(ChainName, AuthenticatorID, Filename) ->
     call({import_users, ChainName, AuthenticatorID, Filename}).
 

+ 16 - 5
apps/emqx_auth/src/emqx_authn/emqx_authn_user_import_api.erl

@@ -21,6 +21,7 @@
 -include("emqx_authn.hrl").
 -include_lib("emqx/include/logger.hrl").
 -include_lib("hocon/include/hoconsc.hrl").
+-include_lib("typerefl/include/types.hrl").
 
 -import(emqx_dashboard_swagger, [error_codes/2]).
 
@@ -34,7 +35,8 @@
 -export([
     api_spec/0,
     paths/0,
-    schema/1
+    schema/1,
+    import_result_schema/0
 ]).
 
 -export([
@@ -61,7 +63,7 @@ schema("/authentication/:id/import_users") ->
             parameters => [emqx_authn_api:param_auth_id(), param_password_type()],
             'requestBody' => request_body_schema(),
             responses => #{
-                204 => <<"Users imported">>,
+                200 => import_result_schema(),
                 400 => error_codes([?BAD_REQUEST], <<"Bad Request">>),
                 404 => error_codes([?NOT_FOUND], <<"Not Found">>)
             }
@@ -81,7 +83,7 @@ schema("/listeners/:listener_id/authentication/:id/import_users") ->
             ],
             'requestBody' => request_body_schema(),
             responses => #{
-                204 => <<"Users imported">>,
+                200 => import_result_schema(),
                 400 => error_codes([?BAD_REQUEST], <<"Bad Request">>),
                 404 => error_codes([?NOT_FOUND], <<"Not Found">>)
             }
@@ -115,6 +117,15 @@ request_body_schema() ->
         description => <<"Import body">>
     }.
 
+import_result_schema() ->
+    [
+        {total, hoconsc:mk(integer(), #{description => ?DESC(import_result_total)})},
+        {success, hoconsc:mk(integer(), #{description => ?DESC(import_result_success)})},
+        {override, hoconsc:mk(integer(), #{description => ?DESC(import_result_override)})},
+        {skipped, hoconsc:mk(integer(), #{description => ?DESC(import_result_skipped)})},
+        {failed, hoconsc:mk(integer(), #{description => ?DESC(import_result_failed)})}
+    ].
+
 authenticator_import_users(
     post,
     Req = #{
@@ -142,7 +153,7 @@ authenticator_import_users(
                 end
         end,
     case Result of
-        ok -> {204};
+        {ok, Result1} -> {200, Result1};
         {error, Reason} -> emqx_authn_api:serialize_error(Reason)
     end.
 
@@ -165,7 +176,7 @@ listener_authenticator_import_users(
                         ChainName, AuthenticatorID, {PasswordType, FileName, FileData}
                     )
                 of
-                    ok -> {204};
+                    {ok, Result} -> {200, Result};
                     {error, Reason} -> emqx_authn_api:serialize_error(Reason)
                 end
             end

+ 1 - 1
apps/emqx_auth_mnesia/src/emqx_auth_mnesia.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_auth_mnesia, [
     {description, "EMQX Buitl-in Database Authentication and Authorization"},
-    {vsn, "0.2.0"},
+    {vsn, "0.2.1"},
     {registered, []},
     {mod, {emqx_auth_mnesia_app, []}},
     {applications, [

+ 39 - 44
apps/emqx_auth_mnesia/src/emqx_authn_mnesia.erl

@@ -179,23 +179,12 @@ import_users({PasswordType, Filename, FileData}, State, Opts) ->
     try parse_import_users(Filename, FileData, Convertor) of
         {_NewUsersCnt, Users} ->
             case do_import_users(Users, Opts#{filename => Filename}) of
-                ok ->
-                    ok;
+                {ok, Result} ->
+                    {ok, Result};
                 %% Do not log empty user entries.
                 %% The default etc/auth-built-in-db.csv file contains an empty user entry.
                 {error, empty_users} ->
-                    {error, empty_users};
-                {error, Reason} ->
-                    ?SLOG(
-                        warning,
-                        #{
-                            msg => "import_authn_users_failed",
-                            reason => Reason,
-                            type => PasswordType,
-                            filename => Filename
-                        }
-                    ),
-                    {error, Reason}
+                    {error, empty_users}
             end
     catch
         error:Reason:Stk ->
@@ -215,16 +204,19 @@ import_users({PasswordType, Filename, FileData}, State, Opts) ->
 do_import_users([], _Opts) ->
     {error, empty_users};
 do_import_users(Users, Opts) ->
-    trans(
-        fun() ->
-            lists:foreach(
-                fun(User) ->
-                    insert_user(User, Opts)
-                end,
-                Users
-            )
-        end
-    ).
+    Fun = fun() ->
+        lists:foldl(
+            fun(User, Acc) ->
+                Return = insert_user(User, Opts),
+                N = maps:get(Return, Acc, 0),
+                maps:put(Return, N + 1, Acc)
+            end,
+            #{success => 0, skipped => 0, override => 0, failed => 0},
+            Users
+        )
+    end,
+    Res = trans(Fun),
+    {ok, Res#{total => length(Users)}}.
 
 add_user(
     UserInfo,
@@ -241,7 +233,7 @@ do_add_user(
 ) ->
     case mnesia:read(?TAB, DBUserID, write) of
         [] ->
-            insert_user(UserInfoRecord),
+            ok = insert_user(UserInfoRecord),
             {ok, #{user_id => UserID, is_superuser => IsSuperuser}};
         [_] ->
             {error, already_exist}
@@ -281,7 +273,7 @@ do_update_user(
             {error, not_found};
         [#user_info{} = UserInfoRecord] ->
             NUserInfoRecord = update_user_record(UserInfoRecord, FieldsToUpdate),
-            insert_user(NUserInfoRecord),
+            ok = insert_user(NUserInfoRecord),
             {ok, #{user_id => UserID, is_superuser => NUserInfoRecord#user_info.is_superuser}}
     end.
 
@@ -332,6 +324,7 @@ run_fuzzy_filter(
 %% Internal functions
 %%------------------------------------------------------------------------------
 
+-spec insert_user(map(), map()) -> success | skipped | override | failed.
 insert_user(User, Opts) ->
     #{
         <<"user_group">> := UserGroup,
@@ -345,26 +338,28 @@ insert_user(User, Opts) ->
         user_info_record(UserGroup, UserID, PasswordHash, Salt, IsSuperuser),
     case mnesia:read(?TAB, DBUserID, write) of
         [] ->
-            insert_user(UserInfoRecord);
+            ok = insert_user(UserInfoRecord),
+            success;
         [UserInfoRecord] ->
-            ok;
+            skipped;
         [_] ->
-            Msg =
-                case maps:get(override, Opts, false) of
-                    true ->
-                        insert_user(UserInfoRecord),
-                        "override_an_exists_userid_into_authentication_database_ok";
-                    false ->
-                        "import_an_exists_userid_into_authentication_database_failed"
-                end,
-            ?SLOG(warning, #{
-                msg => Msg,
-                user_id => UserID,
-                group_id => UserGroup,
-                bootstrap_file => maps:get(filename, Opts),
-                suggestion =>
-                    "If you've altered it differently, delete the user_id from the bootstrap file."
-            })
+            LogF = fun(Msg) ->
+                ?SLOG(warning, #{
+                    msg => Msg,
+                    user_id => UserID,
+                    group_id => UserGroup,
+                    bootstrap_file => maps:get(filename, Opts)
+                })
+            end,
+            case maps:get(override, Opts, false) of
+                true ->
+                    ok = insert_user(UserInfoRecord),
+                    LogF("override_an_exists_userid_into_authentication_database_ok"),
+                    override;
+                false ->
+                    LogF("import_an_exists_userid_into_authentication_database_failed"),
+                    failed
+            end
     end.
 
 insert_user(#user_info{} = UserInfoRecord) ->

+ 11 - 5
apps/emqx_auth_mnesia/test/emqx_authn_api_mnesia_SUITE.erl

@@ -330,22 +330,28 @@ test_authenticator_import_users(PathPrefix) ->
     CSVFileName = filename:join([Dir, <<"data/user-credentials.csv">>]),
 
     {ok, JSONData} = file:read_file(JSONFileName),
-    {ok, 204, _} = multipart_formdata_request(ImportUri, [], [
+    {ok, 200, Result1} = multipart_formdata_request(ImportUri, [], [
         {filename, "user-credentials.json", JSONData}
     ]),
+    ?assertMatch(
+        #{<<"total">> := 2, <<"success">> := 2}, emqx_utils_json:decode(Result1, [return_maps])
+    ),
 
     {ok, CSVData} = file:read_file(CSVFileName),
-    {ok, 204, _} = multipart_formdata_request(ImportUri, [], [
+    {ok, 200, Result2} = multipart_formdata_request(ImportUri, [], [
         {filename, "user-credentials.csv", CSVData}
     ]),
+    ?assertMatch(
+        #{<<"total">> := 2, <<"success">> := 2}, emqx_utils_json:decode(Result2, [return_maps])
+    ),
 
     %% test application/json
-    {ok, 204, _} = request(post, ImportUri ++ "?type=hash", emqx_utils_json:decode(JSONData)),
+    {ok, 200, _} = request(post, ImportUri ++ "?type=hash", emqx_utils_json:decode(JSONData)),
     {ok, JSONData1} = file:read_file(filename:join([Dir, <<"data/user-credentials-plain.json">>])),
-    {ok, 204, _} = request(post, ImportUri ++ "?type=plain", emqx_utils_json:decode(JSONData1)),
+    {ok, 200, _} = request(post, ImportUri ++ "?type=plain", emqx_utils_json:decode(JSONData1)),
 
     %% test application/json; charset=utf-8
-    {ok, 204, _} = request_with_charset(post, ImportUri ++ "?type=plain", JSONData1),
+    {ok, 200, _} = request_with_charset(post, ImportUri ++ "?type=plain", JSONData1),
     ok.
 
 %%------------------------------------------------------------------------------

+ 20 - 37
apps/emqx_gateway/src/emqx_gateway_api_authn_user_import.erl

@@ -67,28 +67,12 @@ import_users(post, #{
     bindings := #{name := Name0},
     body := Body
 }) ->
-    with_authn(Name0, fun(
-        _GwName,
-        #{
-            id := AuthId,
-            chain_name := ChainName
-        }
-    ) ->
-        case maps:get(<<"filename">>, Body, undefined) of
-            undefined ->
-                emqx_authn_api:serialize_error({missing_parameter, filename});
-            File ->
-                [{FileName, FileData}] = maps:to_list(maps:without([type], File)),
-                case
-                    emqx_authn_chains:import_users(
-                        ChainName, AuthId, {hash, FileName, FileData}
-                    )
-                of
-                    ok -> {204};
-                    {error, Reason} -> emqx_authn_api:serialize_error(Reason)
-                end
+    with_authn(
+        Name0,
+        fun(_GwName, #{id := AuthId, chain_name := ChainName}) ->
+            do_import_users(ChainName, AuthId, Body)
         end
-    end).
+    ).
 
 import_listener_users(post, #{
     bindings := #{name := Name0, id := Id},
@@ -98,23 +82,22 @@ import_listener_users(post, #{
         Name0,
         Id,
         fun(_GwName, #{id := AuthId, chain_name := ChainName}) ->
-            case maps:get(<<"filename">>, Body, undefined) of
-                undefined ->
-                    emqx_authn_api:serialize_error({missing_parameter, filename});
-                File ->
-                    [{FileName, FileData}] = maps:to_list(maps:without([type], File)),
-                    case
-                        emqx_authn_chains:import_users(
-                            ChainName, AuthId, {hash, FileName, FileData}
-                        )
-                    of
-                        ok -> {204};
-                        {error, Reason} -> emqx_authn_api:serialize_error(Reason)
-                    end
-            end
+            do_import_users(ChainName, AuthId, Body)
         end
     ).
 
+do_import_users(ChainName, AuthId, HttpBody) ->
+    case maps:get(<<"filename">>, HttpBody, undefined) of
+        undefined ->
+            emqx_authn_api:serialize_error({missing_parameter, filename});
+        File ->
+            [{FileName, FileData}] = maps:to_list(maps:without([type], File)),
+            case emqx_authn_chains:import_users(ChainName, AuthId, {hash, FileName, FileData}) of
+                {ok, Result} -> {200, Result};
+                {error, Reason} -> emqx_authn_api:serialize_error(Reason)
+            end
+    end.
+
 %%--------------------------------------------------------------------
 %% Swagger defines
 %%--------------------------------------------------------------------
@@ -130,7 +113,7 @@ schema("/gateways/:name/authentication/import_users") ->
                 parameters => params_gateway_name_in_path(),
                 'requestBody' => emqx_dashboard_swagger:file_schema(filename),
                 responses =>
-                    ?STANDARD_RESP(#{204 => <<"Imported">>})
+                    ?STANDARD_RESP(#{200 => emqx_authn_user_import_api:import_result_schema()})
             }
     };
 schema("/gateways/:name/listeners/:id/authentication/import_users") ->
@@ -145,7 +128,7 @@ schema("/gateways/:name/listeners/:id/authentication/import_users") ->
                     params_listener_id_in_path(),
                 'requestBody' => emqx_dashboard_swagger:file_schema(filename),
                 responses =>
-                    ?STANDARD_RESP(#{204 => <<"Imported">>})
+                    ?STANDARD_RESP(#{200 => emqx_authn_user_import_api:import_result_schema()})
             }
     }.
 

+ 36 - 12
apps/emqx_gateway/test/emqx_gateway_api_SUITE.erl

@@ -379,15 +379,27 @@ t_authn_data_mgmt(_) ->
     Dir = code:lib_dir(emqx_auth, test),
     JSONFileName = filename:join([Dir, <<"data/user-credentials.json">>]),
     {ok, JSONData} = file:read_file(JSONFileName),
-    {ok, 204, _} = emqx_dashboard_api_test_helpers:multipart_formdata_request(ImportUri, [], [
-        {filename, "user-credentials.json", JSONData}
-    ]),
+    {ok, 200, ImportedResults} = emqx_dashboard_api_test_helpers:multipart_formdata_request(
+        ImportUri, [], [
+            {filename, "user-credentials.json", JSONData}
+        ]
+    ),
+    ?assertMatch(
+        #{<<"total">> := 2, <<"success">> := 2},
+        emqx_utils_json:decode(ImportedResults, [return_maps])
+    ),
 
     CSVFileName = filename:join([Dir, <<"data/user-credentials.csv">>]),
     {ok, CSVData} = file:read_file(CSVFileName),
-    {ok, 204, _} = emqx_dashboard_api_test_helpers:multipart_formdata_request(ImportUri, [], [
-        {filename, "user-credentials.csv", CSVData}
-    ]),
+    {ok, 200, ImportedResults2} = emqx_dashboard_api_test_helpers:multipart_formdata_request(
+        ImportUri, [], [
+            {filename, "user-credentials.csv", CSVData}
+        ]
+    ),
+    ?assertMatch(
+        #{<<"total">> := 2, <<"success">> := 2},
+        emqx_utils_json:decode(ImportedResults2, [return_maps])
+    ),
 
     {204, _} = request(delete, "/gateways/stomp/authentication"),
     {204, _} = request(get, "/gateways/stomp/authentication"),
@@ -584,15 +596,27 @@ t_listeners_authn_data_mgmt(_) ->
     Dir = code:lib_dir(emqx_auth, test),
     JSONFileName = filename:join([Dir, <<"data/user-credentials.json">>]),
     {ok, JSONData} = file:read_file(JSONFileName),
-    {ok, 204, _} = emqx_dashboard_api_test_helpers:multipart_formdata_request(ImportUri, [], [
-        {filename, "user-credentials.json", JSONData}
-    ]),
+    {ok, 200, ImportedResults} = emqx_dashboard_api_test_helpers:multipart_formdata_request(
+        ImportUri, [], [
+            {filename, "user-credentials.json", JSONData}
+        ]
+    ),
+    ?assertMatch(
+        #{<<"total">> := 2, <<"success">> := 2},
+        emqx_utils_json:decode(ImportedResults, [return_maps])
+    ),
 
     CSVFileName = filename:join([Dir, <<"data/user-credentials.csv">>]),
     {ok, CSVData} = file:read_file(CSVFileName),
-    {ok, 204, _} = emqx_dashboard_api_test_helpers:multipart_formdata_request(ImportUri, [], [
-        {filename, "user-credentials.csv", CSVData}
-    ]),
+    {ok, 200, ImportedResults2} = emqx_dashboard_api_test_helpers:multipart_formdata_request(
+        ImportUri, [], [
+            {filename, "user-credentials.csv", CSVData}
+        ]
+    ),
+    ?assertMatch(
+        #{<<"total">> := 2, <<"success">> := 2},
+        emqx_utils_json:decode(ImportedResults2, [return_maps])
+    ),
 
     ok.
 

+ 25 - 0
rel/i18n/emqx_authn_user_import_api.hocon

@@ -10,4 +10,29 @@ listeners_listener_id_authentication_id_import_users_post.desc:
 listeners_listener_id_authentication_id_import_users_post.label:
 """Import users into authenticator in listener"""
 
+import_result_total.desc:
+"""Total number of users contained in the file."""
+import_result_total.label:
+"""Total"""
+
+import_result_succ.desc:
+"""Total number of users successfully imported with new records."""
+import_result_succ.label:
+"""Success"""
+
+import_result_override.desc:
+"""Total number of users successfully imported as overrides."""
+import_result_override.label:
+"""Override"""
+
+import_result_skipped.desc:
+"""Total number of users skipped because imported users are identical to existing users."""
+import_result_skipped.label:
+"""Skipped"""
+
+import_result_failed.desc:
+"""Total number of users whose import failed."""
+import_result_failed.label:
+"""Failed"""
+
 }