Merge pull request #13854 from thalesmg/20240923-r58-abs-deobfuscate-account-key

fix(azure blob storage connector): handle obfuscated account key

apps/emqx_bridge/test/emqx_bridge_v2_testlib.erl

@@ -1285,6 +1285,56 @@ t_aggreg_upload_restart_corrupted(TCConfig, Opts) ->
     ),
     ok.
 
+%% Simulates a sequence of requests from the frontend and checks that secrets are
+%% deobfuscated correctly for a connector.  The sequence is simply:
+%%
+%%   1) Create a connector.
+%%   2) Update the connector with the response config.
+%%
+%% This assumes that the response from (1) is already obfuscated.  That is, this doesn't
+%% check that secret fields are correctly marked as such.
+t_deobfuscate_connector(Config) ->
+    ?check_trace(
+        begin
+            #{
+                connector_type := ConnectorType,
+                connector_name := ConnectorName
+            } = get_common_values(Config),
+            OriginalConnectorConfig = get_value(connector_config, Config),
+            {201, Response} = simplify_result(create_connector_api(Config)),
+            %% Sanity check
+            ?assertEqual(
+                OriginalConnectorConfig,
+                emqx_config:get_raw([<<"connectors">>, bin(ConnectorType), bin(ConnectorName)])
+            ),
+            ConnectorConfig = maps:without(
+                [
+                    <<"name">>,
+                    <<"actions">>,
+                    <<"sources">>,
+                    <<"node_status">>,
+                    <<"status">>,
+                    <<"type">>
+                ],
+                Response
+            ),
+            ?assertMatch(
+                {200, _},
+                simplify_result(
+                    update_connector_api(ConnectorName, ConnectorType, ConnectorConfig)
+                )
+            ),
+            %% Even if the request is accepted, shouldn't clobber secrets
+            ?assertEqual(
+                OriginalConnectorConfig,
+                emqx_config:get_raw([<<"connectors">>, bin(ConnectorType), bin(ConnectorName)])
+            ),
+            ok
+        end,
+        []
+    ),
+    ok.
+
 snk_timetrap() ->
     {CTTimetrap, _} = ct:get_timetrap_info(),
     #{timetrap => max(0, CTTimetrap - 1_000)}.
@@ -1302,3 +1352,5 @@ proplist_update(Proplist, K, Fn) ->
     {K, OldV} = lists:keyfind(K, 1, Proplist),
     NewV = Fn(OldV),
     lists:keystore(K, 1, Proplist, {K, NewV}).
+
+bin(X) -> emqx_utils_conv:bin(X).

apps/emqx_bridge_azure_blob_storage/src/emqx_bridge_azure_blob_storage_connector.erl

@@ -4,6 +4,8 @@
 
 -module(emqx_bridge_azure_blob_storage_connector).
 
+-feature(maybe_expr, enable).
+
 -behaviour(emqx_resource).
 -behaviour(emqx_connector_aggreg_delivery).
 -behaviour(emqx_template).
@@ -160,17 +162,20 @@ on_start(_ConnResId, ConnConfig) ->
         account_name := AccountName,
         account_key := AccountKey
     } = ConnConfig,
-    Endpoint = maps:get(endpoint, ConnConfig, undefined),
-    {ok, DriverState} = erlazure:new(#{
-        account => AccountName,
-        key => AccountKey,
-        endpoint => Endpoint
-    }),
-    State = #{
-        driver_state => DriverState,
-        installed_actions => #{}
-    },
-    {ok, State}.
+    maybe
+        ok ?= emqx_bridge_azure_blob_storage_connector_schema:validate_account_key(AccountKey),
+        Endpoint = maps:get(endpoint, ConnConfig, undefined),
+        {ok, DriverState} = erlazure:new(#{
+            account => AccountName,
+            key => AccountKey,
+            endpoint => Endpoint
+        }),
+        State = #{
+            driver_state => DriverState,
+            installed_actions => #{}
+        },
+        {ok, State}
+    end.
 
 -spec on_stop(connector_resource_id(), connector_state()) -> ok.
 on_stop(_ConnResId, _ConnState) ->

apps/emqx_bridge_azure_blob_storage/src/emqx_bridge_azure_blob_storage_connector_schema.erl

@@ -23,6 +23,9 @@
     connector_examples/1
 ]).
 
+%% Internal exports
+-export([validate_account_key/1]).
+
 %% API
 -export([]).
 
@@ -138,16 +141,31 @@ connector_example(put) ->
 %%------------------------------------------------------------------------------
 
 %%------------------------------------------------------------------------------
-%% Internal fns
+%% Internal exports
 %%------------------------------------------------------------------------------
 
-mk(Type, Meta) -> hoconsc:mk(Type, Meta).
-
-account_key_validator(Val) ->
+validate_account_key(Val) ->
     try
         _ = base64:decode(emqx_secret:unwrap(Val)),
         ok
     catch
         _:_ ->
-            {error, <<"bad account key">>}
+            {error, <<"bad account key; must be a valid base64 encoded value">>}
+    end.
+
+%%------------------------------------------------------------------------------
+%% Internal fns
+%%------------------------------------------------------------------------------
+
+mk(Type, Meta) -> hoconsc:mk(Type, Meta).
+
+account_key_validator(Val) ->
+    case emqx_secret:unwrap(Val) of
+        <<"******">> ->
+            %% The frontend sends obfuscated values when updating a connector...  So we
+            %% cannot distinguish an obfuscated value from an user explicitly setting this
+            %% field to this value.
+            ok;
+        Key ->
+            validate_account_key(Key)
     end.

apps/emqx_bridge_azure_blob_storage/test/emqx_bridge_azure_blob_storage_SUITE.erl

@@ -683,7 +683,7 @@ t_bad_account_key(Config) ->
                 {400, #{
                     <<"message">> := #{
                         <<"kind">> := <<"validation_error">>,
-                        <<"reason">> := <<"bad account key">>
+                        <<"reason">> := <<"bad account key", _/binary>>
                     }
                 }},
                 emqx_bridge_v2_testlib:simplify_result(
@@ -697,7 +697,7 @@ t_bad_account_key(Config) ->
                 {400, #{
                     <<"message">> := #{
                         <<"kind">> := <<"validation_error">>,
-                        <<"reason">> := <<"bad account key">>
+                        <<"reason">> := <<"bad account key", _/binary>>
                     }
                 }},
                 emqx_bridge_v2_testlib:simplify_result(
@@ -734,3 +734,29 @@ t_bad_account_name(Config) ->
         []
     ),
     ok.
+
+t_deobfuscate_connector(Config) ->
+    emqx_bridge_v2_testlib:?FUNCTION_NAME(Config).
+
+%% Checks that we verify at runtime that the provided account key is a valid base64 string.
+t_create_connector_with_obfuscated_key(Config0) ->
+    ?check_trace(
+        begin
+            RedactedValue = <<"******">>,
+            Config = emqx_bridge_v2_testlib:proplist_update(Config0, connector_config, fun(Old) ->
+                Old#{<<"account_key">> := RedactedValue}
+            end),
+            ?assertMatch(
+                {201, #{
+                    <<"status">> := <<"disconnected">>,
+                    <<"status_reason">> := <<"bad account key", _/binary>>
+                }},
+                emqx_bridge_v2_testlib:simplify_result(
+                    emqx_bridge_v2_testlib:create_connector_api(Config)
+                )
+            ),
+            ok
+        end,
+        []
+    ),
+    ok.