From 79a4a041e46fe6259534a354d3e4529622303dbf Mon Sep 17 00:00:00 2001
From: Thales Macedo Garitezi <thalesmg@gmail.com>
Date: Thu, 11 Jan 2024 14:26:38 -0300
Subject: [PATCH] fix(bridge_api): correctly deobfuscate secrets during dry run

Fixes https://emqx.atlassian.net/browse/EMQX-11733
---
 apps/emqx_bridge/src/emqx_bridge_api.erl      |  2 ++
 .../test/emqx_bridge_api_SUITE.erl            | 29 +++++++++++++++++--
 apps/emqx_utils/src/emqx_utils.erl            |  6 ++--
 apps/emqx_utils/test/emqx_utils_tests.erl     |  6 ++--
 changes/ce/fix-12306.en.md                    |  1 +
 5 files changed, 37 insertions(+), 7 deletions(-)
 create mode 100644 changes/ce/fix-12306.en.md

diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl
index d0b2e28fe..e1cd03ac2 100644
--- a/apps/emqx_bridge/src/emqx_bridge_api.erl
+++ b/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;
diff --git a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl b/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl
index 0ac1b2e3c..82efc77d2 100644
--- a/apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl
+++ b/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),
 
diff --git a/apps/emqx_utils/src/emqx_utils.erl b/apps/emqx_utils/src/emqx_utils.erl
index 510a7b27c..8d7c622a4 100644
--- a/apps/emqx_utils/src/emqx_utils.erl
+++ b/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.
 
diff --git a/apps/emqx_utils/test/emqx_utils_tests.erl b/apps/emqx_utils/test/emqx_utils_tests.erl
index b72ce169e..1db0efa6a 100644
--- a/apps/emqx_utils/test/emqx_utils_tests.erl
+++ b/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(<<"******">>)))
     ].
diff --git a/changes/ce/fix-12306.en.md b/changes/ce/fix-12306.en.md
new file mode 100644
index 000000000..2973019f5
--- /dev/null
+++ b/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.