fix(bridge_api): correctly deobfuscate secrets during dry run

Fixes https://emqx.atlassian.net/browse/EMQX-11733
This commit is contained in:
Thales Macedo Garitezi 2024-01-11 14:26:38 -03:00
parent c4ea79db1e
commit 79a4a041e4
5 changed files with 37 additions and 7 deletions

View File

@ -22,6 +22,7 @@
-include_lib("emqx/include/logger.hrl"). -include_lib("emqx/include/logger.hrl").
-include_lib("emqx_utils/include/emqx_utils_api.hrl"). -include_lib("emqx_utils/include/emqx_utils_api.hrl").
-include_lib("emqx_bridge/include/emqx_bridge.hrl"). -include_lib("emqx_bridge/include/emqx_bridge.hrl").
-include_lib("snabbkaffe/include/snabbkaffe.hrl").
-import(hoconsc, [mk/2, array/1, enum/1]). -import(hoconsc, [mk/2, array/1, enum/1]).
@ -564,6 +565,7 @@ schema("/bridges_probe") ->
{ok, #{body := #{<<"type">> := BridgeType} = Params}} -> {ok, #{body := #{<<"type">> := BridgeType} = Params}} ->
Params1 = maybe_deobfuscate_bridge_probe(Params), Params1 = maybe_deobfuscate_bridge_probe(Params),
Params2 = maps:remove(<<"type">>, Params1), Params2 = maps:remove(<<"type">>, Params1),
?tp(bridge_v1_api_dry_run, #{params => Params2}),
case emqx_bridge_resource:create_dry_run(BridgeType, Params2) of case emqx_bridge_resource:create_dry_run(BridgeType, Params2) of
ok -> ok ->
?NO_CONTENT; ?NO_CONTENT;

View File

@ -19,6 +19,7 @@
-compile(export_all). -compile(export_all).
-import(emqx_mgmt_api_test_util, [uri/1]). -import(emqx_mgmt_api_test_util, [uri/1]).
-import(emqx_common_test_helpers, [on_exit/1]).
-include_lib("eunit/include/eunit.hrl"). -include_lib("eunit/include/eunit.hrl").
-include_lib("common_test/include/ct.hrl"). -include_lib("common_test/include/ct.hrl").
@ -882,7 +883,7 @@ start_stop_inconsistent_bridge(Type, Config) ->
) )
end), end),
emqx_common_test_helpers:on_exit(fun() -> on_exit(fun() ->
erpc:call(Node, fun() -> erpc:call(Node, fun() ->
meck:unload([emqx_bridge_resource]) meck:unload([emqx_bridge_resource])
end) end)
@ -999,6 +1000,8 @@ t_reset_bridges(Config) ->
{ok, 200, []} = request_json(get, uri(["bridges"]), Config). {ok, 200, []} = request_json(get, uri(["bridges"]), Config).
t_with_redact_update(Config) -> t_with_redact_update(Config) ->
ok = snabbkaffe:start_trace(),
on_exit(fun() -> ok = snabbkaffe:stop() end),
Name = <<"redact_update">>, Name = <<"redact_update">>,
Type = <<"mqtt">>, Type = <<"mqtt">>,
Password = <<"123456">>, Password = <<"123456">>,
@ -1027,6 +1030,28 @@ t_with_redact_update(Config) ->
Password, Password,
get_raw_config([bridges, Type, Name, password], Config) get_raw_config([bridges, Type, Name, password], Config)
), ),
%% probe with new password; should not be considered redacted
{_, {ok, #{params := UsedParams}}} =
?wait_async_action(
request(
post,
uri(["bridges_probe"]),
Template#{<<"password">> := <<"newpassword">>},
Config
),
#{?snk_kind := bridge_v1_api_dry_run},
1_000
),
UsedPassword0 = maps:get(<<"password">>, UsedParams),
%% the password field schema makes
%% `emqx_dashboard_swagger:filter_check_request_and_translate_body' wrap the password.
%% hack: this fails with `badfun' in CI only, due to cover compile, if not evaluated
%% in the original node...
PrimaryNode = ?config(node, Config),
erpc:call(PrimaryNode, fun() -> ?assertEqual(<<"newpassword">>, UsedPassword0()) end),
ok = snabbkaffe:stop(),
ok. ok.
t_bridges_probe(Config) -> t_bridges_probe(Config) ->
@ -1149,7 +1174,7 @@ t_bridges_probe(Config) ->
Config Config
), ),
emqx_common_test_helpers:on_exit(fun() -> on_exit(fun() ->
delete_user_auth(Chain, AuthenticatorID, User, Config) delete_user_auth(Chain, AuthenticatorID, User, Config)
end), end),

View File

@ -793,9 +793,9 @@ do_is_redacted(K, ?REDACT_VAL, Fun) ->
Fun(K); Fun(K);
do_is_redacted(K, <<?REDACT_VAL>>, Fun) -> do_is_redacted(K, <<?REDACT_VAL>>, Fun) ->
Fun(K); Fun(K);
do_is_redacted(_K, V, _Fun) when is_function(V, 0) -> do_is_redacted(K, WrappedFun, Fun) when is_function(WrappedFun, 0) ->
%% already wrapped by `emqx_secret' or other module %% wrapped by `emqx_secret' or other module
true; do_is_redacted(K, WrappedFun(), Fun);
do_is_redacted(_K, _V, _Fun) -> do_is_redacted(_K, _V, _Fun) ->
false. false.

View File

@ -23,6 +23,8 @@ is_redacted_test_() ->
?_assertNot(emqx_utils:is_redacted(password, <<>>)), ?_assertNot(emqx_utils:is_redacted(password, <<>>)),
?_assertNot(emqx_utils:is_redacted(password, undefined)), ?_assertNot(emqx_utils:is_redacted(password, undefined)),
?_assert(emqx_utils:is_redacted(password, <<"******">>)), ?_assert(emqx_utils:is_redacted(password, <<"******">>)),
?_assert(emqx_utils:is_redacted(password, fun() -> <<"secretpass">> end)), ?_assertNot(emqx_utils:is_redacted(password, fun() -> <<"secretpass">> end)),
?_assert(emqx_utils:is_redacted(password, emqx_secret:wrap(<<"secretpass">>))) ?_assertNot(emqx_utils:is_redacted(password, emqx_secret:wrap(<<"secretpass">>))),
?_assert(emqx_utils:is_redacted(password, fun() -> <<"******">> end)),
?_assert(emqx_utils:is_redacted(password, emqx_secret:wrap(<<"******">>)))
]. ].

View File

@ -0,0 +1 @@
Fixed an issue that prevented probing a bridge from working after changing the password in the HTTP API request.