From 7137a422dfc76d09b5fed4d34f53eeb8522ea583 Mon Sep 17 00:00:00 2001
From: "Zaiming (Stone) Shi" <zmstone@gmail.com>
Date: Wed, 14 Dec 2022 11:04:10 +0100
Subject: [PATCH] refactor(emqx_authn): make error messages more readable

---
 apps/emqx/src/emqx_config.erl      |  1 +
 apps/emqx_authn/src/emqx_authn.erl | 44 ++++++++++++++++--------------
 2 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/apps/emqx/src/emqx_config.erl b/apps/emqx/src/emqx_config.erl
index 1f9a46c03..16d9f31a7 100644
--- a/apps/emqx/src/emqx_config.erl
+++ b/apps/emqx/src/emqx_config.erl
@@ -402,6 +402,7 @@ merge_envs(SchemaMod, RawConf) ->
         required => false,
         format => map,
         apply_override_envs => true,
+        remove_env_meta => true,
         check_lazy => true
     },
     hocon_tconf:merge_env_overrides(SchemaMod, RawConf, all, Opts).
diff --git a/apps/emqx_authn/src/emqx_authn.erl b/apps/emqx_authn/src/emqx_authn.erl
index ac0ee0eb0..c7f9d6fdf 100644
--- a/apps/emqx_authn/src/emqx_authn.erl
+++ b/apps/emqx_authn/src/emqx_authn.erl
@@ -39,12 +39,15 @@ providers() ->
         {{scram, built_in_database}, emqx_enhanced_authn_scram_mnesia}
     ].
 
-check_configs(C) when is_map(C) ->
-    check_configs([C]);
-check_configs([]) ->
+check_configs(CM) when is_map(CM) ->
+    check_configs([CM]);
+check_configs(CL) ->
+    check_configs(CL, 1).
+
+check_configs([], _Nth) ->
     [];
-check_configs([Config | Configs]) ->
-    [check_config(Config) | check_configs(Configs)].
+check_configs([Config | Configs], Nth) ->
+    [check_config(Config, #{id_for_log => Nth}) | check_configs(Configs, Nth + 1)].
 
 check_config(Config) ->
     check_config(Config, #{}).
@@ -55,15 +58,16 @@ check_config(Config, Opts) ->
         #{?CONF_NS_BINARY := WithDefaults} -> WithDefaults
     end.
 
-do_check_config(#{<<"mechanism">> := Mec} = Config, Opts) ->
+do_check_config(#{<<"mechanism">> := Mec0} = Config, Opts) ->
+    Mec = atom(Mec0, #{error => unknown_mechanism}),
     Key =
         case maps:get(<<"backend">>, Config, false) of
-            false -> atom(Mec);
-            Backend -> {atom(Mec), atom(Backend)}
+            false -> Mec;
+            Backend -> {Mec, atom(Backend, #{error => unknown_backend})}
         end,
     case lists:keyfind(Key, 1, providers()) of
         false ->
-            throw({unknown_handler, Key});
+            throw(#{error => unknown_authn_provider, which => Key});
         {_, ProviderModule} ->
             hocon_tconf:check_plain(
                 ProviderModule,
@@ -71,22 +75,22 @@ do_check_config(#{<<"mechanism">> := Mec} = Config, Opts) ->
                 Opts#{atom_key => true}
             )
     end;
-do_check_config(Config, _Opts) when is_map(Config) ->
-    throw({invalid_config, "mechanism_field_required", Config});
-do_check_config(RawConf, Opts) ->
-    %% authentication conf is lazy type, when it comes from ENV, it is a string
-    %% EMQX_AUTHENTICATION__1="{mechanism=\"password_based\"...}"
-    case hocon:binary(RawConf, Opts) of
-        {ok, Conf} -> do_check_config(Conf, Opts);
-        {error, Reason} -> throw({invalid_config, Reason})
-    end.
+do_check_config(Config, Opts) when is_map(Config) ->
+    throw(#{
+        error => invalid_config,
+        which => maps:get(id_for_log, Opts, unknown),
+        reason => "mechanism_field_required"
+    }).
 
-atom(Bin) ->
+%% The atoms have to be loaded already,
+%% which might be an issue for plugins which are loaded after node boot
+%% but they should really manage their own configs in that case.
+atom(Bin, ErrorContext) ->
     try
         binary_to_existing_atom(Bin, utf8)
     catch
         _:_ ->
-            throw({unknown_auth_provider, Bin})
+            throw(ErrorContext#{value => Bin})
     end.
 
 -spec get_enabled_authns() ->