Merge pull request #12100 from thalesmg/fix-password-action-probe-m-20231205

fix(bridges/actions api): correctly deobfuscate passwords when probing

apps/emqx_bridge/src/emqx_bridge_v2_api.erl

@@ -551,8 +551,8 @@ schema("/action_types") ->
 '/action_types'(get, _Request) ->
     ?OK(emqx_bridge_v2_schema:types()).
 
-maybe_deobfuscate_bridge_probe(#{<<"type">> := BridgeType, <<"name">> := BridgeName} = Params) ->
-    case emqx_bridge:lookup(BridgeType, BridgeName) of
+maybe_deobfuscate_bridge_probe(#{<<"type">> := ActionType, <<"name">> := BridgeName} = Params) ->
+    case emqx_bridge_v2:lookup(ActionType, BridgeName) of
         {ok, #{raw_config := RawConf}} ->
             %% TODO check if RawConf optained above is compatible with the commented out code below
             %% RawConf = emqx:get_raw_config([bridges, BridgeType, BridgeName], #{}),

apps/emqx_bridge/src/schema/emqx_bridge_schema.erl

@@ -35,6 +35,9 @@
     metrics_fields/0
 ]).
 
+%% for testing only
+-export([enterprise_api_schemas/1]).
+
 %%======================================================================================
 %% Hocon Schema Definitions
 
@@ -57,7 +60,7 @@ api_schema(Method) ->
             {<<"mqtt">>, emqx_bridge_mqtt_schema}
         ]
     ],
-    EE = enterprise_api_schemas(Method),
+    EE = ?MODULE:enterprise_api_schemas(Method),
     hoconsc:union(bridge_api_union(Broker ++ EE)).
 
 bridge_api_union(Refs) ->

apps/emqx_bridge/test/emqx_bridge_v1_compatibility_layer_SUITE.erl

@@ -21,6 +21,7 @@
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("common_test/include/ct.hrl").
 -include_lib("typerefl/include/types.hrl").
+-include_lib("emqx/include/asserts.hrl").
 
 -import(emqx_common_test_helpers, [on_exit/1]).
 
@@ -108,6 +109,14 @@ setup_mocks() ->
         fun(Method) -> [{bridge_type_bin(), hoconsc:ref(?MODULE, "api_" ++ Method)}] end
     ),
 
+    catch meck:new(emqx_bridge_schema, MeckOpts),
+    meck:expect(
+        emqx_bridge_schema,
+        enterprise_api_schemas,
+        1,
+        fun(Method) -> [{bridge_type_bin(), hoconsc:ref(?MODULE, "api_" ++ Method)}] end
+    ),
+
     ok.
 
 con_mod() ->
@@ -142,7 +151,9 @@ con_schema() ->
 fields("connector") ->
     [
         {enable, hoconsc:mk(any(), #{})},
+        {password, emqx_schema_secret:mk(#{required => false})},
         {resource_opts, hoconsc:mk(map(), #{})},
+        {on_start_fun, hoconsc:mk(binary(), #{})},
         {ssl, hoconsc:ref(ssl)}
     ];
 fields("api_post") ->
@@ -458,6 +469,29 @@ enable_rule_http(RuleId) ->
     Params = #{<<"enable">> => true},
     update_rule_http(RuleId, Params).
 
+probe_bridge_http_api_v1(Opts) ->
+    Name = maps:get(name, Opts),
+    Overrides = maps:get(overrides, Opts, #{}),
+    BridgeConfig0 = emqx_utils_maps:deep_merge(bridge_config(), Overrides),
+    BridgeConfig = maps:without([<<"connector">>], BridgeConfig0),
+    Params = BridgeConfig#{<<"type">> => bridge_type_bin(), <<"name">> => Name},
+    Path = emqx_mgmt_api_test_util:api_path(["bridges_probe"]),
+    ct:pal("probe bridge (http v1) (~p):\n  ~p", [#{name => Name}, Params]),
+    Res = request(post, Path, Params),
+    ct:pal("probe bridge (http v1) (~p) result:\n  ~p", [#{name => Name}, Res]),
+    Res.
+
+probe_action_http_api_v2(Opts) ->
+    Name = maps:get(name, Opts),
+    Overrides = maps:get(overrides, Opts, #{}),
+    BridgeConfig = emqx_utils_maps:deep_merge(bridge_config(), Overrides),
+    Params = BridgeConfig#{<<"type">> => bridge_type_bin(), <<"name">> => Name},
+    Path = emqx_mgmt_api_test_util:api_path(["actions_probe"]),
+    ct:pal("probe action (http v2) (~p):\n  ~p", [#{name => Name}, Params]),
+    Res = request(post, Path, Params),
+    ct:pal("probe action (http v2) (~p) result:\n  ~p", [#{name => Name}, Res]),
+    Res.
+
 %%------------------------------------------------------------------------------
 %% Test cases
 %%------------------------------------------------------------------------------
@@ -825,3 +859,63 @@ t_create_with_bad_name(_Config) ->
             }
         }}} = create_bridge_http_api_v1(Opts),
     ok.
+
+t_obfuscated_secrets_probe(_Config) ->
+    Name = <<"bridgev2">>,
+    Me = self(),
+    ets:new(emqx_bridge_v2_SUITE:fun_table_name(), [named_table, public]),
+    OnStartFun = emqx_bridge_v2_SUITE:wrap_fun(fun(Conf) ->
+        Me ! {on_start, Conf},
+        {ok, Conf}
+    end),
+    OriginalPassword = <<"supersecret">>,
+    Overrides = #{<<"password">> => OriginalPassword, <<"on_start_fun">> => OnStartFun},
+    %% Using the real password, like when creating the bridge for the first time.
+    ?assertMatch(
+        {ok, {{_, 204, _}, _, _}},
+        probe_bridge_http_api_v1(#{name => Name, overrides => Overrides})
+    ),
+
+    %% Check that we still can probe created bridges that use passwords.
+    ?assertMatch(
+        {ok, {{_, 201, _}, _, #{}}},
+        create_bridge_http_api_v1(#{name => Name, overrides => Overrides})
+    ),
+    %% Password is obfuscated
+    ?assertMatch(
+        {ok, {{_, 200, _}, _, #{<<"password">> := <<"******">>}}},
+        get_bridge_http_api_v1(Name)
+    ),
+    %% still using the password
+    ?assertMatch(
+        {ok, {{_, 204, _}, _, _}},
+        probe_bridge_http_api_v1(#{name => Name, overrides => Overrides})
+    ),
+    %% now with obfuscated password (loading the UI again)
+    ?assertMatch(
+        {ok, {{_, 204, _}, _, _}},
+        probe_bridge_http_api_v1(#{
+            name => Name,
+            overrides => Overrides#{<<"password">> => <<"******">>}
+        })
+    ),
+    ?assertMatch(
+        {ok, {{_, 204, _}, _, _}},
+        probe_action_http_api_v2(#{
+            name => Name,
+            overrides => Overrides#{<<"password">> => <<"******">>}
+        })
+    ),
+
+    %% We have to check that the connector was started with real passwords during dry runs
+    StartConfs = [Conf || {on_start, Conf} <- ?drainMailbox()],
+    Passwords = lists:map(fun(#{password := P}) -> P end, StartConfs),
+    ?assert(lists:all(fun is_function/1, Passwords), #{passwords => Passwords}),
+    UnwrappedPasswords = [F() || F <- Passwords],
+    ?assertEqual(
+        [OriginalPassword],
+        lists:usort(UnwrappedPasswords),
+        #{passwords => UnwrappedPasswords}
+    ),
+
+    ok.

apps/emqx_utils/src/emqx_utils.erl

@@ -759,6 +759,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, _V, _Fun) ->
     false.
 

apps/emqx_utils/test/emqx_utils_tests.erl

@@ -0,0 +1,28 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+-module(emqx_utils_tests).
+
+-include_lib("eunit/include/eunit.hrl").
+
+is_redacted_test_() ->
+    [
+        ?_assertNot(emqx_utils:is_redacted(password, <<"secretpass">>)),
+        ?_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">>)))
+    ].