fix: deobfuscate ft's secret keys in api

apps/emqx_bridge/src/emqx_bridge.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_bridge, [
     {description, "EMQX bridges"},
-    {vsn, "0.1.31"},
+    {vsn, "0.1.32"},
     {registered, [emqx_bridge_sup]},
     {mod, {emqx_bridge_app, []}},
     {applications, [

apps/emqx_bridge/src/emqx_bridge_api.erl

@@ -495,7 +495,7 @@ schema("/bridges_probe") ->
         case emqx_bridge:lookup(BridgeType, BridgeName) of
             {ok, #{raw_config := RawConf}} ->
                 %% TODO will the maybe_upgrade step done by emqx_bridge:lookup cause any problems
-                Conf = deobfuscate(Conf1, RawConf),
+                Conf = emqx_utils:deobfuscate(Conf1, RawConf),
                 update_bridge(BridgeType, BridgeName, Conf);
             {error, not_found} ->
                 ?BRIDGE_NOT_FOUND(BridgeType, BridgeName);
@@ -587,9 +587,9 @@ maybe_deobfuscate_bridge_probe(#{<<"type">> := BridgeType0, <<"name">> := Bridge
     BridgeType = upgrade_type(BridgeType0),
     case emqx_bridge:lookup(BridgeType, BridgeName) of
         {ok, #{raw_config := RawConf}} ->
-            %% TODO check if RawConf optained above is compatible with the commented out code below
+            %% TODO check if RawConf obtained above is compatible with the commented out code below
             %% RawConf = emqx:get_raw_config([bridges, BridgeType, BridgeName], #{}),
-            deobfuscate(Params, RawConf);
+            emqx_utils:deobfuscate(Params, RawConf);
         _ ->
             %% A bridge may be probed before it's created, so not finding it here is fine
             Params
@@ -1123,27 +1123,6 @@ supported_versions(_Call) -> [1, 2, 3, 4, 5].
 redact(Term) ->
     emqx_utils:redact(Term).
 
-deobfuscate(NewConf, OldConf) ->
-    maps:fold(
-        fun(K, V, Acc) ->
-            case maps:find(K, OldConf) of
-                error ->
-                    Acc#{K => V};
-                {ok, OldV} when is_map(V), is_map(OldV) ->
-                    Acc#{K => deobfuscate(V, OldV)};
-                {ok, OldV} ->
-                    case emqx_utils:is_redacted(K, V) of
-                        true ->
-                            Acc#{K => OldV};
-                        _ ->
-                            Acc#{K => V}
-                    end
-            end
-        end,
-        #{},
-        NewConf
-    ).
-
 map_to_json(M0) ->
     %% When dealing with Hocon validation errors, `value' might contain non-serializable
     %% values (e.g.: user_lookup_fun), so we try again without that key if serialization

apps/emqx_bridge/src/emqx_bridge_v2_api.erl

@@ -423,7 +423,7 @@ schema("/action_types") ->
         case emqx_bridge_v2:lookup(BridgeType, BridgeName) of
             {ok, _} ->
                 RawConf = emqx:get_raw_config([bridges, BridgeType, BridgeName], #{}),
-                Conf = deobfuscate(Conf1, RawConf),
+                Conf = emqx_utils:deobfuscate(Conf1, RawConf),
                 update_bridge(BridgeType, BridgeName, Conf);
             {error, not_found} ->
                 ?BRIDGE_NOT_FOUND(BridgeType, BridgeName)
@@ -554,9 +554,9 @@ schema("/action_types") ->
 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
+            %% TODO check if RawConf obtained above is compatible with the commented out code below
             %% RawConf = emqx:get_raw_config([bridges, BridgeType, BridgeName], #{}),
-            deobfuscate(Params, RawConf);
+            emqx_utils:deobfuscate(Params, RawConf);
         _ ->
             %% A bridge may be probed before it's created, so not finding it here is fine
             Params
@@ -586,27 +586,6 @@ is_ok(ResL) ->
         ErrL -> hd(ErrL)
     end.
 
-deobfuscate(NewConf, OldConf) ->
-    maps:fold(
-        fun(K, V, Acc) ->
-            case maps:find(K, OldConf) of
-                error ->
-                    Acc#{K => V};
-                {ok, OldV} when is_map(V), is_map(OldV) ->
-                    Acc#{K => deobfuscate(V, OldV)};
-                {ok, OldV} ->
-                    case emqx_utils:is_redacted(K, V) of
-                        true ->
-                            Acc#{K => OldV};
-                        _ ->
-                            Acc#{K => V}
-                    end
-            end
-        end,
-        #{},
-        NewConf
-    ).
-
 %% bridge helpers
 lookup_from_all_nodes(BridgeType, BridgeName, SuccCode) ->
     Nodes = mria:running_nodes(),

apps/emqx_connector/src/emqx_connector.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_connector, [
     {description, "EMQX Data Integration Connectors"},
-    {vsn, "0.1.35"},
+    {vsn, "0.1.36"},
     {registered, []},
     {mod, {emqx_connector_app, []}},
     {applications, [

apps/emqx_connector/src/emqx_connector_api.erl

@@ -348,7 +348,7 @@ schema("/connectors_probe") ->
         case emqx_connector:lookup(ConnectorType, ConnectorName) of
             {ok, _} ->
                 RawConf = emqx:get_raw_config([connectors, ConnectorType, ConnectorName], #{}),
-                Conf = deobfuscate(Conf1, RawConf),
+                Conf = emqx_utils:deobfuscate(Conf1, RawConf),
                 update_connector(ConnectorType, ConnectorName, Conf);
             {error, not_found} ->
                 ?CONNECTOR_NOT_FOUND(ConnectorType, ConnectorName)
@@ -409,7 +409,7 @@ maybe_deobfuscate_connector_probe(
     case emqx_connector:lookup(ConnectorType, ConnectorName) of
         {ok, _} ->
             RawConf = emqx:get_raw_config([connectors, ConnectorType, ConnectorName], #{}),
-            deobfuscate(Params, RawConf);
+            emqx_utils:deobfuscate(Params, RawConf);
         _ ->
             %% A connector may be probed before it's created, so not finding it here is fine
             Params
@@ -780,27 +780,6 @@ maybe_unwrap(RpcMulticallResult) ->
 redact(Term) ->
     emqx_utils:redact(Term).
 
-deobfuscate(NewConf, OldConf) ->
-    maps:fold(
-        fun(K, V, Acc) ->
-            case maps:find(K, OldConf) of
-                error ->
-                    Acc#{K => V};
-                {ok, OldV} when is_map(V), is_map(OldV) ->
-                    Acc#{K => deobfuscate(V, OldV)};
-                {ok, OldV} ->
-                    case emqx_utils:is_redacted(K, V) of
-                        true ->
-                            Acc#{K => OldV};
-                        _ ->
-                            Acc#{K => V}
-                    end
-            end
-        end,
-        #{},
-        NewConf
-    ).
-
 map_to_json(M0) ->
     %% When dealing with Hocon validation errors, `value' might contain non-serializable
     %% values (e.g.: user_lookup_fun), so we try again without that key if serialization

apps/emqx_ft/src/emqx_ft.app.src

@@ -1,6 +1,6 @@
 {application, emqx_ft, [
     {description, "EMQX file transfer over MQTT"},
-    {vsn, "0.1.10"},
+    {vsn, "0.1.11"},
     {registered, []},
     {mod, {emqx_ft_app, []}},
     {applications, [

apps/emqx_ft/src/emqx_ft_api.erl

@@ -178,7 +178,9 @@ check_ft_enabled(Params, _Meta) ->
 '/file_transfer'(get, _Meta) ->
     {200, format_config(emqx_ft_conf:get())};
 '/file_transfer'(put, #{body := ConfigIn}) ->
-    case emqx_ft_conf:update(ConfigIn) of
+    OldConf = emqx_ft_conf:get_raw(),
+    UpdateConf = emqx_utils:deobfuscate(ConfigIn, OldConf),
+    case emqx_ft_conf:update(UpdateConf) of
         {ok, #{config := Config}} ->
             {200, format_config(Config)};
         {error, Error = #{kind := validation_error}} ->
@@ -199,7 +201,11 @@ format_page(#{items := Files}) ->
 
 format_config(Config) ->
     Schema = emqx_hocon:make_schema(emqx_ft_schema:fields(file_transfer)),
-    hocon_tconf:make_serializable(Schema, emqx_utils_maps:binary_key_map(Config), #{}).
+    hocon_tconf:make_serializable(
+        Schema,
+        emqx_utils_maps:binary_key_map(Config),
+        #{obfuscate_sensitive_values => true}
+    ).
 
 format_validation_error(Error) ->
     emqx_logger_jsonfmt:best_effort_json(Error).

apps/emqx_ft/src/emqx_ft_conf.erl

@@ -37,7 +37,8 @@
     load/0,
     unload/0,
     get/0,
-    update/1
+    update/1,
+    get_raw/0
 ]).
 
 %% callbacks for emqx_config_handler
@@ -112,6 +113,9 @@ unload() ->
 get() ->
     emqx_config:get([file_transfer]).
 
+get_raw() ->
+    emqx:get_raw_config([file_transfer], #{}).
+
 -spec update(emqx_config:config()) -> {ok, emqx_config:update_result()} | {error, term()}.
 update(Config) ->
     emqx_conf:update([file_transfer], Config, #{override_to => cluster}).

apps/emqx_ft/test/emqx_ft_api_SUITE.erl

@@ -22,6 +22,8 @@
 -include_lib("common_test/include/ct.hrl").
 -include_lib("stdlib/include/assert.hrl").
 
+-define(SECRET_ACCESS_KEY, <<"fake_secret_access_key">>).
+
 -import(emqx_dashboard_api_test_helpers, [host/0, uri/1]).
 
 all() -> emqx_common_test_helpers:all(?MODULE).
@@ -335,6 +337,7 @@ t_configure(Config) ->
         <<"host">> => <<"localhost">>,
         <<"port">> => 9000,
         <<"bucket">> => <<"emqx">>,
+        <<"secret_access_key">> => ?SECRET_ACCESS_KEY,
         <<"transport_options">> => #{
             <<"ssl">> => #{
                 <<"enable">> => true,
@@ -343,8 +346,24 @@ t_configure(Config) ->
             }
         }
     },
+    {ok, 200, GetConfigJson} =
+        request_json(
+            put,
+            uri(["file_transfer"]),
+            #{
+                <<"enable">> => true,
+                <<"storage">> => #{
+                    <<"local">> => #{
+                        <<"exporter">> => #{
+                            <<"s3">> => S3Exporter
+                        }
+                    }
+                }
+            },
+            Config
+        ),
     ?assertMatch(
-        {ok, 200, #{
+        #{
             <<"enable">> := true,
             <<"storage">> := #{
                 <<"local">> := #{
@@ -356,27 +375,14 @@ t_configure(Config) ->
                                     <<"certfile">> := <<"/", _CertFilepath/bytes>>,
                                     <<"keyfile">> := <<"/", _KeyFilepath/bytes>>
                                 }
-                            }
+                            },
+                            <<"secret_access_key">> := <<"******">>
                         }
                     }
                 }
             }
-        }},
-        request_json(
-            put,
-            uri(["file_transfer"]),
-            #{
-                <<"enable">> => true,
-                <<"storage">> => #{
-                    <<"local">> => #{
-                        <<"exporter">> => #{
-                            <<"s3">> => S3Exporter
-                        }
-                    }
-                }
-            },
-            Config
-        )
+        },
+        GetConfigJson
     ),
     ?assertMatch(
         {ok, 400, _},
@@ -405,23 +411,47 @@ t_configure(Config) ->
         request_json(
             put,
             uri(["file_transfer"]),
-            #{
-                <<"enable">> => true,
-                <<"storage">> => #{
-                    <<"local">> => #{
-                        <<"exporter">> => #{
-                            <<"s3">> => emqx_utils_maps:deep_put(
-                                [<<"transport_options">>, <<"ssl">>, <<"enable">>],
-                                S3Exporter,
-                                false
-                            )
+            emqx_utils_maps:deep_merge(
+                GetConfigJson,
+                #{
+                    <<"enable">> => true,
+                    <<"storage">> => #{
+                        <<"local">> => #{
+                            <<"exporter">> => #{
+                                <<"s3">> => emqx_utils_maps:deep_put(
+                                    [<<"transport_options">>, <<"ssl">>, <<"enable">>],
+                                    S3Exporter,
+                                    false
+                                )
+                            }
                         }
                     }
                 }
-            },
+            ),
             Config
         )
     ),
+    %% put secret as ******, check the secret is unchanged
+    ?assertMatch(
+        #{
+            <<"storage">> :=
+                #{
+                    <<"local">> :=
+                        #{
+                            <<"enable">> := true,
+                            <<"exporter">> :=
+                                #{
+                                    <<"s3">> :=
+                                        #{
+                                            <<"enable">> := true,
+                                            <<"secret_access_key">> := ?SECRET_ACCESS_KEY
+                                        }
+                                }
+                        }
+                }
+        },
+        get_ft_config(Config)
+    ),
     ok.
 
 %%--------------------------------------------------------------------
@@ -500,3 +530,7 @@ reset_ft_config(Config, Enable) ->
         },
     {ok, _} = rpc:call(Node, emqx_ft_conf, update, [LocalConfig]),
     ok.
+
+get_ft_config(Config) ->
+    [Node | _] = test_nodes(Config),
+    rpc:call(Node, emqx_ft_conf, get_raw, []).

apps/emqx_utils/src/emqx_utils.erl

@@ -78,6 +78,7 @@
 ]).
 
 -export([clamp/3, redact/1, redact/2, is_redacted/2, is_redacted/3]).
+-export([deobfuscate/2]).
 
 -export_type([
     readable_error_msg/1
@@ -754,6 +755,27 @@ redact_v([{str, Bin}]) when is_binary(Bin) ->
 redact_v(_V) ->
     ?REDACT_VAL.
 
+deobfuscate(NewConf, OldConf) ->
+    maps:fold(
+        fun(K, V, Acc) ->
+            case maps:find(K, OldConf) of
+                error ->
+                    Acc#{K => V};
+                {ok, OldV} when is_map(V), is_map(OldV) ->
+                    Acc#{K => deobfuscate(V, OldV)};
+                {ok, OldV} ->
+                    case is_redacted(K, V) of
+                        true ->
+                            Acc#{K => OldV};
+                        _ ->
+                            Acc#{K => V}
+                    end
+            end
+        end,
+        #{},
+        NewConf
+    ).
+
 is_redacted(K, V) ->
     do_is_redacted(K, V, fun is_sensitive_key/1).