Merge pull request #11691 from lafirest/fix/sso_ssl

fix(sso): support for SSL update && ensure update is atomic

apps/emqx/src/emqx_logger_jsonfmt.erl

@@ -224,10 +224,6 @@ best_effort_json_obj(Map, Config) ->
             do_format_msg("~p", [Map], Config)
     end.
 
-json([], _) ->
-    "";
-json(<<"">>, _) ->
-    "\"\"";
 json(A, _) when is_atom(A) -> atom_to_binary(A, utf8);
 json(I, _) when is_integer(I) -> I;
 json(F, _) when is_float(F) -> F;
@@ -239,12 +235,18 @@ json(B, Config) when is_binary(B) ->
 json(M, Config) when is_list(M), is_tuple(hd(M)), tuple_size(hd(M)) =:= 2 ->
     best_effort_json_obj(M, Config);
 json(L, Config) when is_list(L) ->
-    try unicode:characters_to_binary(L, utf8) of
-        B when is_binary(B) -> B;
-        _ -> [json(I, Config) || I <- L]
-    catch
-        _:_ ->
-            [json(I, Config) || I <- L]
+    case lists:all(fun erlang:is_binary/1, L) of
+        true ->
+            %% string array
+            L;
+        false ->
+            try unicode:characters_to_binary(L, utf8) of
+                B when is_binary(B) -> B;
+                _ -> [json(I, Config) || I <- L]
+            catch
+                _:_ ->
+                    [json(I, Config) || I <- L]
+            end
     end;
 json(Map, Config) when is_map(Map) ->
     best_effort_json_obj(Map, Config);
@@ -462,4 +464,15 @@ chars_limit_applied_on_format_result_test() ->
     ?assertEqual(Limit, size(LongStr1)),
     ok.
 
+string_array_test() ->
+    Array = #{<<"arr">> => [<<"a">>, <<"b">>]},
+    Encoded = emqx_utils_json:encode(json(Array, config())),
+    ?assertEqual(Array, emqx_utils_json:decode(Encoded)).
+
+iolist_test() ->
+    Iolist = #{iolist => ["a", ["b"]]},
+    Concat = #{<<"iolist">> => <<"ab">>},
+    Encoded = emqx_utils_json:encode(json(Iolist, config())),
+    ?assertEqual(Concat, emqx_utils_json:decode(Encoded)).
+
 -endif.

apps/emqx/src/emqx_tls_certfile_gc.erl

@@ -271,9 +271,12 @@ find_config_references(Root) ->
 is_file_reference(Stack) ->
     lists:any(
         fun(KP) -> lists:prefix(lists:reverse(KP), Stack) end,
-        emqx_tls_lib:ssl_file_conf_keypaths()
+        conf_keypaths()
     ).
 
+conf_keypaths() ->
+    emqx_tls_lib:ssl_file_conf_keypaths().
+
 mk_fileref(AbsPath) ->
     case emqx_utils_fs:read_info(AbsPath) of
         {ok, Info} ->

apps/emqx/src/emqx_tls_lib.erl

@@ -50,11 +50,17 @@
 -define(IS_FALSE(Val), ((Val =:= false) orelse (Val =:= <<"false">>))).
 
 -define(SSL_FILE_OPT_PATHS, [
+    %% common ssl options
     [<<"keyfile">>],
     [<<"certfile">>],
     [<<"cacertfile">>],
-    [<<"ocsp">>, <<"issuer_pem">>]
+    %% OCSP
+    [<<"ocsp">>, <<"issuer_pem">>],
+    %% SSO
+    [<<"sp_public_key">>],
+    [<<"sp_private_key">>]
 ]).
+
 -define(SSL_FILE_OPT_PATHS_A, [
     [keyfile],
     [certfile],

apps/emqx_dashboard_sso/src/emqx_dashboard_sso.erl

@@ -13,7 +13,8 @@
     create/2,
     update/3,
     destroy/2,
-    login/3
+    login/3,
+    convert_certs/3
 ]).
 
 -export([types/0, modules/0, provider/1, backends/0, format/1]).
@@ -26,7 +27,9 @@
     backend => atom(),
     atom() => term()
 }.
--type state() :: #{atom() => term()}.
+
+%% Note: if a backend has a resource, it must be stored in the state and named resource_id
+-type state() :: #{resource_id => binary(), atom() => term()}.
 -type raw_config() :: #{binary() => term()}.
 -type config() :: parsed_config() | raw_config().
 -type hocon_ref() :: ?R_REF(Module :: atom(), Name :: atom() | binary()).
@@ -43,6 +46,11 @@
     | {redirect, tuple()}
     | {error, Reason :: term()}.
 
+-callback convert_certs(
+    Dir :: file:filename_all(),
+    config()
+) -> config().
+
 %%------------------------------------------------------------------------------
 %% Callback Interface
 %%------------------------------------------------------------------------------
@@ -66,6 +74,9 @@ destroy(Mod, State) ->
 login(Mod, Req, State) ->
     Mod:login(Req, State).
 
+convert_certs(Mod, Dir, Config) ->
+    Mod:convert_certs(Dir, Config).
+
 %%------------------------------------------------------------------------------
 %% API
 %%------------------------------------------------------------------------------

apps/emqx_dashboard_sso/src/emqx_dashboard_sso_api.erl

@@ -133,24 +133,28 @@ schema("/sso/:backend") ->
     }.
 
 fields(backend_status) ->
-    emqx_dashboard_sso_schema:common_backend_schema(emqx_dashboard_sso:types()).
+    emqx_dashboard_sso_schema:common_backend_schema(emqx_dashboard_sso:types()) ++
+        [
+            {running,
+                mk(
+                    boolean(), #{
+                        desc => ?DESC(running)
+                    }
+                )},
+            {last_error,
+                mk(
+                    binary(), #{
+                        desc => ?DESC(last_error)
+                    }
+                )}
+        ].
 
 %%--------------------------------------------------------------------
 %% API
 %%--------------------------------------------------------------------
 
 running(get, _Request) ->
-    SSO = emqx:get_config(?MOD_KEY_PATH, #{}),
-    {200,
-        lists:filtermap(
-            fun
-                (#{backend := Backend, enable := true}) ->
-                    {true, Backend};
-                (_) ->
-                    false
-            end,
-            maps:values(SSO)
-        )}.
+    {200, emqx_dashboard_sso_manager:running()}.
 
 login(post, #{bindings := #{backend := Backend}, body := Body} = Request) ->
     case emqx_dashboard_sso_manager:lookup_state(Backend) of
@@ -185,8 +189,12 @@ sso(get, _Request) ->
     SSO = emqx:get_config(?MOD_KEY_PATH, #{}),
     {200,
         lists:map(
-            fun(Backend) ->
-                maps:with([backend, enable], Backend)
+            fun(#{backend := Backend, enable := Enable}) ->
+                Status = emqx_dashboard_sso_manager:get_backend_status(Backend, Enable),
+                Status#{
+                    backend => Backend,
+                    enable => Enable
+                }
             end,
             maps:values(SSO)
         )}.
@@ -263,8 +271,6 @@ handle_backend_update_result(ok, _) ->
     204;
 handle_backend_update_result({error, not_exists}, _) ->
     {404, #{code => ?BACKEND_NOT_FOUND, message => <<"Backend not found">>}};
-handle_backend_update_result({error, already_exists}, _) ->
-    {400, #{code => ?BAD_REQUEST, message => <<"Backend already exists">>}};
 handle_backend_update_result({error, failed_to_load_metadata}, _) ->
     {400, #{code => ?BAD_REQUEST, message => <<"Failed to load metadata">>}};
 handle_backend_update_result({error, Reason}, _) when is_binary(Reason) ->

apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl

@@ -22,7 +22,8 @@
     login/2,
     create/1,
     update/2,
-    destroy/1
+    destroy/1,
+    convert_certs/2
 ]).
 
 %%------------------------------------------------------------------------------
@@ -163,3 +164,21 @@ ensure_user_exists(Username) ->
                     Error
             end
     end.
+
+convert_certs(Dir, Conf) ->
+    case
+        emqx_tls_lib:ensure_ssl_files(
+            Dir, maps:get(<<"ssl">>, Conf, undefined)
+        )
+    of
+        {ok, SSL} ->
+            new_ssl_source(Conf, SSL);
+        {error, Reason} ->
+            ?SLOG(error, Reason#{msg => "bad_ssl_config"}),
+            throw({bad_ssl_config, Reason})
+    end.
+
+new_ssl_source(Source, undefined) ->
+    Source;
+new_ssl_source(Source, SSL) ->
+    Source#{<<"ssl">> => SSL}.

apps/emqx_dashboard_sso/src/emqx_dashboard_sso_manager.erl

@@ -25,10 +25,10 @@
 -export([
     running/0,
     lookup_state/1,
+    get_backend_status/2,
     make_resource_id/1,
     create_resource/3,
-    update_resource/3,
-    call/1
+    update_resource/3
 ]).
 
 -export([
@@ -39,20 +39,21 @@
     propagated_post_config_update/5
 ]).
 
--import(emqx_dashboard_sso, [provider/1]).
+-import(emqx_dashboard_sso, [provider/1, format/1]).
 
 -define(MOD_TAB, emqx_dashboard_sso).
 -define(MOD_KEY_PATH, [dashboard, sso]).
--define(CALL_TIMEOUT, timer:seconds(10)).
 -define(MOD_KEY_PATH(Sub), [dashboard, sso, Sub]).
 -define(RESOURCE_GROUP, <<"emqx_dashboard_sso">>).
+-define(NO_ERROR, <<>>).
 -define(DEFAULT_RESOURCE_OPTS, #{
     start_after_created => false
 }).
 
 -record(?MOD_TAB, {
     backend :: atom(),
-    state :: map()
+    state :: undefined | map(),
+    last_error = ?NO_ERROR :: term()
 }).
 
 %%------------------------------------------------------------------------------
@@ -62,17 +63,44 @@ start_link() ->
     gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
 
 running() ->
-    maps:fold(
+    SSO = emqx:get_config(?MOD_KEY_PATH, #{}),
+    lists:filtermap(
         fun
-            (Type, #{enable := true}, Acc) ->
-                [Type | Acc];
-            (_Type, _Cfg, Acc) ->
-                Acc
+            (#{backend := Backend, enable := true}) ->
+                case lookup(Backend) of
+                    undefined ->
+                        false;
+                    #?MOD_TAB{last_error = ?NO_ERROR} ->
+                        {true, Backend};
+                    _ ->
+                        false
+                end;
+            (_) ->
+                false
         end,
-        [],
-        emqx:get_config(?MOD_KEY_PATH)
+        maps:values(SSO)
     ).
 
+get_backend_status(Backend, false) ->
+    #{
+        backend => Backend,
+        enable => false,
+        running => false,
+        last_error => ?NO_ERROR
+    };
+get_backend_status(Backend, _) ->
+    case lookup(Backend) of
+        undefined ->
+            #{
+                backend => Backend,
+                enable => true,
+                running => false,
+                last_error => <<"Resource not found">>
+            };
+        Data ->
+            maps:merge(#{backend => Backend, enable => true}, do_get_backend_status(Data))
+    end.
+
 update(Backend, Config) ->
     update_config(Backend, {?FUNCTION_NAME, Backend, Config}).
 delete(Backend) ->
@@ -98,7 +126,7 @@ create_resource(ResourceId, Module, Config) ->
         Config,
         ?DEFAULT_RESOURCE_OPTS
     ),
-    start_resource_if_enabled(ResourceId, Result, Config, fun clean_when_start_failed/1).
+    start_resource_if_enabled(ResourceId, Result, Config).
 
 update_resource(ResourceId, Module, Config) ->
     Result = emqx_resource:recreate_local(
@@ -106,14 +134,6 @@ update_resource(ResourceId, Module, Config) ->
     ),
     start_resource_if_enabled(ResourceId, Result, Config).
 
-call(Req) ->
-    try
-        gen_server:call(?MODULE, Req, ?CALL_TIMEOUT)
-    catch
-        exit:{timeout, _} ->
-            {error, <<"Update backend failed: timeout">>}
-    end.
-
 %%------------------------------------------------------------------------------
 %% gen_server callbacks
 %%------------------------------------------------------------------------------
@@ -133,9 +153,6 @@ init([]) ->
     start_backend_services(),
     {ok, #{}}.
 
-handle_call({update_config, Req, NewConf}, _From, State) ->
-    Result = on_config_update(Req, NewConf),
-    {reply, Result, State};
 handle_call(_Request, _From, State) ->
     Reply = ok,
     {reply, Reply, State}.
@@ -170,12 +187,14 @@ start_backend_services() ->
                         msg => "start_sso_backend_successfully",
                         backend => Backend
                     }),
-                    ets:insert(?MOD_TAB, #?MOD_TAB{backend = Backend, state = State});
+                    update_state(Backend, State);
                 {error, Reason} ->
+                    SafeReason = emqx_utils:redact(Reason),
+                    update_last_error(Backend, SafeReason),
                     ?SLOG(error, #{
                         msg => "start_sso_backend_failed",
                         backend => Backend,
-                        reason => Reason
+                        reason => SafeReason
                     })
             end
         end,
@@ -183,39 +202,38 @@ start_backend_services() ->
     ).
 
 update_config(Backend, UpdateReq) ->
+    %% we always make sure the valid configuration will update successfully,
+    %% ignore the runtime error during its update
     case emqx_conf:update(?MOD_KEY_PATH(Backend), UpdateReq, #{override_to => cluster}) of
-        {ok, UpdateResult} ->
-            #{post_config_update := #{?MODULE := Result}} = UpdateResult,
-            ?SLOG(info, #{
-                msg => "update_sso_successfully",
-                backend => Backend,
-                result => Result
-            }),
-            Result;
-        {error, Reason} ->
+        {ok, _UpdateResult} ->
+            case lookup(Backend) of
+                undefined ->
+                    ok;
+                #?MOD_TAB{state = State, last_error = ?NO_ERROR} ->
+                    {ok, State};
+                Data ->
+                    {error, Data#?MOD_TAB.last_error}
+            end;
+        {error, Reason} = Error ->
+            SafeReason = emqx_utils:redact(Reason),
             ?SLOG(error, #{
                 msg => "update_sso_failed",
                 backend => Backend,
-                reason => Reason
+                reason => SafeReason
             }),
-            {error,
-                case Reason of
-                    {_Stage, _Mod, Reason2} ->
-                        Reason2;
-                    _ ->
-                        Reason
-                end}
+            Error
     end.
 
 pre_config_update(_, {update, _Backend, Config}, _OldConf) ->
-    {ok, Config};
+    {ok, maybe_write_certs(Config)};
 pre_config_update(_, {delete, _Backend}, undefined) ->
     throw(not_exists);
 pre_config_update(_, {delete, _Backend}, _OldConf) ->
     {ok, null}.
 
 post_config_update(_, UpdateReq, NewConf, _OldConf, _AppEnvs) ->
-    call({update_config, UpdateReq, NewConf}).
+    _ = on_config_update(UpdateReq, NewConf),
+    ok.
 
 propagated_post_config_update(
     ?MOD_KEY_PATH(BackendBin) = Path, _UpdateReq, undefined, OldConf, AppEnvs
@@ -241,26 +259,30 @@ on_config_update({update, Backend, _RawConfig}, Config) ->
     case lookup(Backend) of
         undefined ->
             on_backend_updated(
+                Backend,
                 emqx_dashboard_sso:create(Provider, Config),
                 fun(State) ->
-                    ets:insert(?MOD_TAB, #?MOD_TAB{backend = Backend, state = State})
+                    update_state(Backend, State)
                 end
             );
         Data ->
+            update_last_error(Backend, ?NO_ERROR),
             on_backend_updated(
+                Backend,
                 emqx_dashboard_sso:update(Provider, Config, Data#?MOD_TAB.state),
                 fun(State) ->
-                    ets:insert(?MOD_TAB, Data#?MOD_TAB{state = State})
+                    update_state(Backend, State)
                 end
             )
     end;
 on_config_update({delete, Backend}, _NewConf) ->
     case lookup(Backend) of
         undefined ->
-            {error, not_exists};
+            on_backend_updated(Backend, {error, not_exists}, undefined);
         Data ->
             Provider = provider(Backend),
             on_backend_updated(
+                Backend,
                 emqx_dashboard_sso:destroy(Provider, Data#?MOD_TAB.state),
                 fun() ->
                     ets:delete(?MOD_TAB, Backend)
@@ -276,15 +298,12 @@ lookup(Backend) ->
             undefined
     end.
 
-start_resource_if_enabled(ResourceId, Result, Config) ->
-    start_resource_if_enabled(ResourceId, Result, Config, undefined).
-
-start_resource_if_enabled(
-    ResourceId, {ok, _} = Result, #{enable := true}, CleanWhenStartFailed
-) ->
+%% to avoid resource leakage the resource start will never affect the update result,
+%% so the resource_id will always be recorded
+start_resource_if_enabled(ResourceId, {ok, _} = Result, #{enable := true, backend := Backend}) ->
     case emqx_resource:start(ResourceId) of
         ok ->
-            Result;
+            ok;
         {error, Reason} ->
             SafeReason = emqx_utils:redact(Reason),
             ?SLOG(error, #{
@@ -292,31 +311,21 @@ start_resource_if_enabled(
                 resource_id => ResourceId,
                 reason => SafeReason
             }),
-            erlang:is_function(CleanWhenStartFailed) andalso
-                CleanWhenStartFailed(ResourceId),
-            {error, emqx_dashboard_sso:format(["Start backend failed, Reason: ", SafeReason])}
-    end;
-start_resource_if_enabled(_ResourceId, Result, _Config, _) ->
+            update_last_error(Backend, SafeReason),
+            ok
+    end,
+    Result;
+start_resource_if_enabled(_ResourceId, Result, _Config) ->
     Result.
 
-%% ensure the backend creation is atomic, clean the corresponding resource when necessary,
-%% when creating a backend fails, nothing will be inserted into the SSO table,
-%% thus the resources created by backend will leakage.
-%% Although we can treat start failure as successful,
-%% and insert the resource data into the SSO table,
-%% it may be strange for users: it succeeds, but can't be used.
-clean_when_start_failed(ResourceId) ->
-    _ = emqx_resource:remove_local(ResourceId),
-    ok.
-
-%% this first level `ok` is for emqx_config_handler, and the second level is for the caller
-on_backend_updated({ok, State} = Ok, Fun) ->
+on_backend_updated(_Backend, {ok, State} = Ok, Fun) ->
     Fun(State),
-    {ok, Ok};
-on_backend_updated(ok, Fun) ->
+    Ok;
+on_backend_updated(_Backend, ok, Fun) ->
     Fun(),
-    {ok, ok};
-on_backend_updated(Error, _) ->
+    ok;
+on_backend_updated(Backend, {error, Reason} = Error, _) ->
+    update_last_error(Backend, Reason),
     Error.
 
 bin(A) when is_atom(A) -> atom_to_binary(A, utf8);
@@ -331,3 +340,50 @@ add_handler() ->
 
 remove_handler() ->
     ok = emqx_conf:remove_handler(?MOD_KEY_PATH('?')).
+
+maybe_write_certs(#{<<"backend">> := Backend} = Conf) ->
+    Dir = certs_path(Backend),
+    Provider = provider(Backend),
+    emqx_dashboard_sso:convert_certs(Provider, Dir, Conf).
+
+certs_path(Backend) ->
+    filename:join(["sso", Backend]).
+
+update_state(Backend, State) ->
+    Data = ensure_backend_data(Backend),
+    ets:insert(?MOD_TAB, Data#?MOD_TAB{state = State}).
+
+update_last_error(Backend, LastError) ->
+    Data = ensure_backend_data(Backend),
+    ets:insert(?MOD_TAB, Data#?MOD_TAB{last_error = LastError}).
+
+ensure_backend_data(Backend) ->
+    case ets:lookup(?MOD_TAB, Backend) of
+        [Data] ->
+            Data;
+        [] ->
+            #?MOD_TAB{backend = Backend}
+    end.
+
+do_get_backend_status(#?MOD_TAB{state = #{resource_id := ResourceId}}) ->
+    case emqx_resource_manager:lookup(ResourceId) of
+        {ok, _Group, #{status := connected}} ->
+            #{running => true, last_error => ?NO_ERROR};
+        {ok, _Group, #{status := Status}} ->
+            #{
+                running => false,
+                last_error => format([<<"Resource not valid, status: ">>, Status])
+            };
+        {error, not_found} ->
+            #{
+                running => false,
+                last_error => <<"Resource not found">>
+            }
+    end;
+do_get_backend_status(#?MOD_TAB{last_error = ?NO_ERROR}) ->
+    #{running => true, last_error => ?NO_ERROR};
+do_get_backend_status(#?MOD_TAB{last_error = LastError}) ->
+    #{
+        running => false,
+        last_error => format([LastError])
+    }.

apps/emqx_dashboard_sso/src/emqx_dashboard_sso_saml.erl

@@ -22,7 +22,8 @@
 -export([
     create/1,
     update/2,
-    destroy/1
+    destroy/1,
+    convert_certs/2
 ]).
 
 -export([login/2, callback/2]).
@@ -104,7 +105,7 @@ create(#{enable := false} = _Config) ->
     {ok, undefined};
 create(#{sp_sign_request := true} = Config) ->
     try
-        do_create(ensure_cert_and_key(Config))
+        do_create(Config)
     catch
         Kind:Error ->
             Msg = failed_to_ensure_cert_and_key,
@@ -152,10 +153,31 @@ callback(_Req = #{body := Body}, #{sp := SP, dashboard_addr := DashboardAddr} =
             {error, iolist_to_binary(Reason)}
     end.
 
+convert_certs(
+    Dir,
+    #{<<"sp_sign_request">> := true, <<"sp_public_key">> := Cert, <<"sp_private_key">> := Key} =
+        Conf
+) ->
+    case
+        emqx_tls_lib:ensure_ssl_files(
+            Dir, #{enable => ture, certfile => Cert, keyfile => Key}, #{}
+        )
+    of
+        {ok, #{certfile := CertPath, keyfile := KeyPath}} ->
+            Conf#{<<"sp_public_key">> => bin(CertPath), <<"sp_private_key">> => bin(KeyPath)};
+        {error, Reason} ->
+            ?SLOG(error, #{msg => "failed_to_save_sp_sign_keys", reason => Reason}),
+            throw("Failed to save sp signing key(s)")
+    end;
+convert_certs(_Dir, Conf) ->
+    Conf.
+
 %%------------------------------------------------------------------------------
 %% Internal functions
 %%------------------------------------------------------------------------------
 
+bin(X) -> iolist_to_binary(X).
+
 do_create(
     #{
         dashboard_addr := DashboardAddr,
@@ -224,18 +246,6 @@ gen_redirect_response(DashboardAddr, Username) ->
 %% Helpers functions
 %%------------------------------------------------------------------------------
 
-ensure_cert_and_key(#{sp_public_key := Cert, sp_private_key := Key} = Config) ->
-    case
-        emqx_tls_lib:ensure_ssl_files(
-            ?DIR, #{enable => ture, certfile => Cert, keyfile => Key}, #{}
-        )
-    of
-        {ok, #{certfile := CertPath, keyfile := KeyPath} = _NSSL} ->
-            Config#{sp_public_key => CertPath, sp_private_key => KeyPath};
-        {error, #{which_options := KeyPath}} ->
-            error({missing_key, lists:flatten(KeyPath)})
-    end.
-
 %% TODO: unify with emqx_dashboard_sso_manager:ensure_user_exists/1
 ensure_user_exists(Username) ->
     case emqx_dashboard_admin:lookup_user(saml, Username) of

apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl

@@ -24,13 +24,14 @@
 
 all() ->
     [
-        t_create_atomicly,
+        t_bad_create,
         t_create,
         t_update,
         t_get,
         t_login_with_bad,
         t_first_login,
         t_next_login,
+        t_bad_update,
         t_delete
     ].
 
@@ -60,7 +61,7 @@ end_per_testcase(Case, _) ->
     end,
     ok.
 
-t_create_atomicly(_) ->
+t_bad_create(_) ->
     Path = uri(["sso", "ldap"]),
     ?assertMatch(
         {ok, 400, _},
@@ -74,12 +75,18 @@ t_create_atomicly(_) ->
             })
         )
     ),
-    ?assertEqual(undefined, emqx:get_config(?MOD_KEY_PATH, undefined)),
-    ?assertEqual([], ets:tab2list(?MOD_TAB)),
+    ?assertMatch(#{backend := ldap}, emqx:get_config(?MOD_KEY_PATH, undefined)),
+    check_running([]),
+    ?assertMatch(
+        [#{backend := <<"ldap">>, enable := true, running := false, last_error := _}], get_sso()
+    ),
+
+    emqx_dashboard_sso_manager:delete(ldap),
+
     ?retry(
-        _Interval = 1000,
-        _NAttempts = 5,
-        ?assertEqual([], emqx_resource_manager:list_group(?RESOURCE_GROUP))
+        _Interval = 500,
+        _NAttempts = 10,
+        ?assertMatch([], emqx_resource_manager:list_group(?RESOURCE_GROUP))
     ),
     ok.
 
@@ -158,6 +165,28 @@ t_next_login(_) ->
     ?assertMatch(#{license := _, token := _}, decode_json(Result)),
     ok.
 
+t_bad_update(_) ->
+    Path = uri(["sso", "ldap"]),
+    ?assertMatch(
+        {ok, 400, _},
+        request(
+            put,
+            Path,
+            ldap_config(#{
+                <<"username">> => <<"invalid">>,
+                <<"enable">> => true,
+                <<"request_timeout">> => <<"1s">>
+            })
+        )
+    ),
+    ?assertMatch(#{backend := ldap}, emqx:get_config(?MOD_KEY_PATH, undefined)),
+    check_running([]),
+    ?assertMatch(
+        [#{backend := <<"ldap">>, enable := true, running := false, last_error := _}], get_sso()
+    ),
+
+    ok.
+
 t_delete(_) ->
     Path = uri(["sso", "ldap"]),
     ?assertMatch({ok, 204, _}, request(delete, Path)),

apps/emqx_ldap/src/emqx_ldap.erl

@@ -74,7 +74,7 @@ fields(config) ->
         {request_timeout,
             ?HOCON(emqx_schema:timeout_duration_ms(), #{
                 desc => ?DESC(request_timeout),
-                default => <<"5s">>
+                default => <<"10s">>
             })},
         {ssl,
             ?HOCON(?R_REF(?MODULE, ssl), #{

apps/emqx_machine/src/emqx_restricted_shell.erl

@@ -104,7 +104,7 @@ max_heap_size_warning(MF, Args) ->
                 msg => "shell_process_exceed_max_heap_size",
                 current_heap_size => HeapSize,
                 function => MF,
-                args => Args,
+                args => pp_args(Args),
                 max_heap_size => ?MAX_HEAP_SIZE
             })
     end.
@@ -115,21 +115,30 @@ log(IsAllow, MF, Args) ->
     ?AUDIT(warning, "from_remote_console", #{
         time => logger:timestamp(),
         function => MF,
-        args => Args,
+        args => pp_args(Args),
         permission => IsAllow
     }),
     to_console(IsAllow, MF, Args).
 
 to_console(prohibited, MF, Args) ->
     warning("DANGEROUS FUNCTION: FORBIDDEN IN SHELL!!!!!", []),
-    ?SLOG(error, #{msg => "execute_function_in_shell_prohibited", function => MF, args => Args});
+    ?SLOG(error, #{
+        msg => "execute_function_in_shell_prohibited",
+        function => MF,
+        args => pp_args(Args)
+    });
 to_console(exempted, MF, Args) ->
     limit_warning(MF, Args),
     ?SLOG(error, #{
-        msg => "execute_dangerous_function_in_shell_exempted", function => MF, args => Args
+        msg => "execute_dangerous_function_in_shell_exempted",
+        function => MF,
+        args => pp_args(Args)
     });
 to_console(ok, MF, Args) ->
     limit_warning(MF, Args).
 
 warning(Format, Args) ->
     io:format(?RED_BG ++ Format ++ ?RESET ++ "~n", Args).
+
+pp_args(Args) ->
+    iolist_to_binary(io_lib:format("~0p", [Args])).

apps/emqx_utils/src/emqx_utils.erl

@@ -645,6 +645,7 @@ try_to_existing_atom(Convert, Data, Encoding) ->
         _:Reason -> {error, Reason}
     end.
 
+%% NOTE: keep alphabetical order
 is_sensitive_key(aws_secret_access_key) -> true;
 is_sensitive_key("aws_secret_access_key") -> true;
 is_sensitive_key(<<"aws_secret_access_key">>) -> true;
@@ -663,6 +664,8 @@ is_sensitive_key(<<"secret_key">>) -> true;
 is_sensitive_key(security_token) -> true;
 is_sensitive_key("security_token") -> true;
 is_sensitive_key(<<"security_token">>) -> true;
+is_sensitive_key(sp_private_key) -> true;
+is_sensitive_key(<<"sp_private_key">>) -> true;
 is_sensitive_key(token) -> true;
 is_sensitive_key("token") -> true;
 is_sensitive_key(<<"token">>) -> true;

rel/i18n/emqx_dashboard_sso_api.hocon

@@ -51,4 +51,16 @@ backend_name.desc:
 backend_name.label:
 """Backend Name"""
 
+running.desc:
+"""Is the backend running"""
+
+running.label:
+"""Running"""
+
+last_error.desc:
+"""Last error of this backend"""
+
+last_error.label:
+"""Last Error"""
+
 }