Merge pull request #12306 from thalesmg/fix-password-deobfuscate-m-20240111

fix(bridge_api): correctly deobfuscate secrets during dry run

apps/emqx_bridge/src/emqx_bridge_api.erl

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

apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl

@@ -19,6 +19,7 @@
 -compile(export_all).
 
 -import(emqx_mgmt_api_test_util, [uri/1]).
+-import(emqx_common_test_helpers, [on_exit/1]).
 
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("common_test/include/ct.hrl").
@@ -882,7 +883,7 @@ start_stop_inconsistent_bridge(Type, Config) ->
         )
     end),
 
-    emqx_common_test_helpers:on_exit(fun() ->
+    on_exit(fun() ->
         erpc:call(Node, fun() ->
             meck:unload([emqx_bridge_resource])
         end)
@@ -999,6 +1000,8 @@ t_reset_bridges(Config) ->
     {ok, 200, []} = request_json(get, uri(["bridges"]), Config).
 
 t_with_redact_update(Config) ->
+    ok = snabbkaffe:start_trace(),
+    on_exit(fun() -> ok = snabbkaffe:stop() end),
     Name = <<"redact_update">>,
     Type = <<"mqtt">>,
     Password = <<"123456">>,
@@ -1027,6 +1030,28 @@ t_with_redact_update(Config) ->
         Password,
         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.
 
 t_bridges_probe(Config) ->
@@ -1149,7 +1174,7 @@ t_bridges_probe(Config) ->
         Config
     ),
 
-    emqx_common_test_helpers:on_exit(fun() ->
+    on_exit(fun() ->
         delete_user_auth(Chain, AuthenticatorID, User, Config)
     end),
 

apps/emqx_utils/src/emqx_utils.erl

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

apps/emqx_utils/test/emqx_utils_tests.erl

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

changes/ce/fix-12306.en.md

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