Merge pull request #12948 from thalesmg/fix-http-bridge-header-update…

…-obfuscate-r57-20240429

fix(http connector): deobfuscate sensitive headers

apps/emqx_bridge/test/emqx_bridge_v2_testlib.erl

@@ -173,6 +173,11 @@ source_hookpoint(Config) ->
     BridgeId = emqx_bridge_resource:bridge_id(Type, Name),
     emqx_bridge_v2:source_hookpoint(BridgeId).
 
+action_hookpoint(Config) ->
+    #{kind := action, type := Type, name := Name} = get_common_values(Config),
+    BridgeId = emqx_bridge_resource:bridge_id(Type, Name),
+    emqx_bridge_resource:bridge_hookpoint(BridgeId).
+
 add_source_hookpoint(Config) ->
     Hookpoint = source_hookpoint(Config),
     ok = emqx_hooks:add(Hookpoint, {?MODULE, source_hookpoint_callback, [self()]}, 1000),
@@ -378,6 +383,14 @@ start_connector_api(ConnectorName, ConnectorType) ->
     ct:pal("connector update (http) result:\n  ~p", [Res]),
     Res.
 
+get_connector_api(ConnectorType, ConnectorName) ->
+    ConnectorId = emqx_connector_resource:connector_id(ConnectorType, ConnectorName),
+    Path = emqx_mgmt_api_test_util:api_path(["connectors", ConnectorId]),
+    ct:pal("get connector ~s (http)", [ConnectorId]),
+    Res = request(get, Path, _Params = []),
+    ct:pal("get connector (http) result:\n  ~p", [Res]),
+    Res.
+
 create_action_api(Config) ->
     create_action_api(Config, _Overrides = #{}).
 

apps/emqx_bridge_http/test/emqx_bridge_http_v2_SUITE.erl

@@ -69,10 +69,21 @@ end_per_suite(Config) ->
 suite() ->
     [{timetrap, {seconds, 60}}].
 
+init_per_testcase(t_update_with_sensitive_data, Config) ->
+    HTTPPath = <<"/foo/bar">>,
+    ServerSSLOpts = false,
+    {ok, {HTTPPort, _Pid}} = emqx_bridge_http_connector_test_server:start_link(
+        _Port = random, HTTPPath, ServerSSLOpts
+    ),
+    ok = emqx_bridge_http_connector_test_server:set_handler(success_handler()),
+    [{path, HTTPPath}, {http_server, #{port => HTTPPort, path => HTTPPath}} | Config];
 init_per_testcase(_TestCase, Config) ->
     Server = start_http_server(#{response_delay_ms => 0}),
     [{http_server, Server} | Config].
 
+end_per_testcase(t_update_with_sensitive_data, Config) ->
+    ok = emqx_bridge_http_connector_test_server:stop(),
+    end_per_testcase(common, proplists:delete(http_server, Config));
 end_per_testcase(_TestCase, Config) ->
     case ?config(http_server, Config) of
         undefined -> ok;
@@ -112,6 +123,69 @@ t_compose_connector_url_and_action_path(Config) ->
     ),
     ok.
 
+%% Checks that we can successfully update a connector containing sensitive headers and
+%% they won't be clobbered by the update.
+t_update_with_sensitive_data(Config) ->
+    ?check_trace(
+        begin
+            ConnectorCfg0 = make_connector_config(Config),
+            AuthHeader = <<"Bearer some_token">>,
+            ConnectorCfg1 = emqx_utils_maps:deep_merge(
+                ConnectorCfg0,
+                #{<<"headers">> => #{<<"authorization">> => AuthHeader}}
+            ),
+            ActionCfg = make_action_config(Config),
+            CreateConfig = [
+                {bridge_kind, action},
+                {action_type, ?BRIDGE_TYPE},
+                {action_name, ?BRIDGE_NAME},
+                {action_config, ActionCfg},
+                {connector_type, ?BRIDGE_TYPE},
+                {connector_name, ?CONNECTOR_NAME},
+                {connector_config, ConnectorCfg1}
+            ],
+            {ok, {{_, 201, _}, _, #{<<"headers">> := #{<<"authorization">> := Obfuscated}}}} =
+                emqx_bridge_v2_testlib:create_connector_api(CreateConfig),
+            {ok, _} =
+                emqx_bridge_v2_testlib:create_kind_api(CreateConfig),
+            BridgeId = emqx_bridge_resource:bridge_id(?BRIDGE_TYPE, ?BRIDGE_NAME),
+            {ok, _} = emqx_bridge_v2_testlib:create_rule_api(
+                #{
+                    sql => <<"select * from \"t/http\" ">>,
+                    actions => [BridgeId]
+                }
+            ),
+            emqx:publish(emqx_message:make(<<"t/http">>, <<"1">>)),
+            ?assertReceive({http, #{<<"authorization">> := AuthHeader}, _}),
+
+            %% Now update the connector and see if the header stays deobfuscated.  We send the old
+            %% auth header as an obfuscated value to simulate the behavior of the frontend.
+            ConnectorCfg2 = emqx_utils_maps:deep_merge(
+                ConnectorCfg1,
+                #{
+                    <<"headers">> => #{
+                        <<"authorization">> => Obfuscated,
+                        <<"other_header">> => <<"new">>
+                    }
+                }
+            ),
+            {ok, _} = emqx_bridge_v2_testlib:update_connector_api(
+                ?CONNECTOR_NAME,
+                ?BRIDGE_TYPE,
+                ConnectorCfg2
+            ),
+
+            emqx:publish(emqx_message:make(<<"t/http">>, <<"2">>)),
+            %% Should not be obfuscated.
+            ?assertReceive({http, #{<<"authorization">> := AuthHeader}, _}, 2_000),
+
+            ok
+        end,
+        []
+    ),
+
+    ok.
+
 %%--------------------------------------------------------------------
 %% helpers
 %%--------------------------------------------------------------------
@@ -123,7 +197,10 @@ make_connector_config(Config) ->
         <<"url">> => iolist_to_binary(io_lib:format("http://localhost:~p", [Port])),
         <<"headers">> => #{},
         <<"pool_type">> => <<"hash">>,
-        <<"pool_size">> => 1
+        <<"pool_size">> => 1,
+        <<"resource_opts">> => #{
+            <<"health_check_interval">> => <<"100ms">>
+        }
     }.
 
 make_action_config(Config) ->
@@ -136,5 +213,22 @@ make_action_config(Config) ->
             <<"method">> => <<"post">>,
             <<"headers">> => #{},
             <<"body">> => <<"${.}">>
+        },
+        <<"resource_opts">> => #{
+            <<"health_check_interval">> => <<"100ms">>
         }
     }.
+
+success_handler() ->
+    TestPid = self(),
+    fun(Req0, State) ->
+        {ok, Body, Req} = cowboy_req:read_body(Req0),
+        TestPid ! {http, cowboy_req:headers(Req), Body},
+        Rep = cowboy_req:reply(
+            200,
+            #{<<"content-type">> => <<"application/json">>},
+            <<"{}">>,
+            Req
+        ),
+        {ok, Rep, State}
+    end.

apps/emqx_utils/src/emqx_utils_redact.erl

@@ -152,19 +152,24 @@ redact_v(_V) ->
     ?REDACT_VAL.
 
 deobfuscate(NewConf, OldConf) ->
+    deobfuscate(NewConf, OldConf, fun(_) -> false end).
+
+deobfuscate(NewConf, OldConf, IsSensitiveFun) ->
     maps:fold(
         fun(K, V, Acc) ->
             case maps:find(K, OldConf) of
                 error ->
-                    case is_redacted(K, V) of
+                    case is_redacted(K, V, IsSensitiveFun) of
                         %% don't put redacted value into new config
                         true -> Acc;
                         false -> Acc#{K => V}
                     end;
+                {ok, OldV} when is_map(V), is_map(OldV), ?IS_KEY_HEADERS(K) ->
+                    Acc#{K => deobfuscate(V, OldV, fun check_is_sensitive_header/1)};
                 {ok, OldV} when is_map(V), is_map(OldV) ->
-                    Acc#{K => deobfuscate(V, OldV)};
+                    Acc#{K => deobfuscate(V, OldV, IsSensitiveFun)};
                 {ok, OldV} ->
-                    case is_redacted(K, V) of
+                    case is_redacted(K, V, IsSensitiveFun) of
                         true ->
                             Acc#{K => OldV};
                         _ ->
@@ -280,6 +285,11 @@ deobfuscate_test() ->
     %% Don't have password before and should allow put non-redact-val into new config
     NewConf3 = #{foo => <<"bar3">>, password => <<"123456">>},
     ?assertEqual(NewConf3, deobfuscate(NewConf3, #{foo => <<"bar">>})),
+
+    HeaderConf1 = #{<<"headers">> => #{<<"Authorization">> => <<"Bearer token">>}},
+    HeaderConf1Obs = #{<<"headers">> => #{<<"Authorization">> => ?REDACT_VAL}},
+    ?assertEqual(HeaderConf1, deobfuscate(HeaderConf1Obs, HeaderConf1)),
+
     ok.
 
 redact_header_test_() ->

changes/ce/fix-12948.en.md

@@ -0,0 +1 @@
+Fixed an issue where sensitive HTTP header values like `Authorization` would be substituted by `******` after updating a connector.