Browse Source

fix(authn): throw exception on invalid config from the API

Zaiming (Stone) Shi 3 years ago
parent
commit
83e66da2aa

+ 2 - 24
apps/emqx/src/emqx_config.erl

@@ -414,32 +414,10 @@ check_config(SchemaMod, RawConf, Opts0) ->
         do_check_config(SchemaMod, RawConf, Opts0)
     catch
         throw:Errors:Stacktrace ->
-            throw(emqx_hocon:compact_errors(Errors, Stacktrace))
+            {error, Reason} = emqx_hocon:compact_errors(Errors, Stacktrace),
+            erlang:raise(throw, Reason, Stacktrace)
     end.
 
-%% HOCON tries to be very informative about all the detailed errors
-%% it's maybe too much when reporting to the user
--spec compact_errors(any(), any()) -> no_return().
-compact_errors(Schema, [Error0 | More]) when is_map(Error0) ->
-    Error1 =
-        case length(More) of
-            0 ->
-                Error0;
-            N ->
-                Error0#{unshown_errors => N}
-        end,
-    Error =
-        case is_atom(Schema) of
-            true ->
-                Error1#{schema_module => Schema};
-            false ->
-                Error1
-        end,
-    throw(Error);
-compact_errors(Schema, Errors) ->
-    %% unexpected, we need the stacktrace reported, hence error
-    error({Schema, Errors}).
-
 do_check_config(SchemaMod, RawConf, Opts0) ->
     Opts1 = #{
         return_plain => true,

+ 7 - 3
apps/emqx/src/emqx_hocon.erl

@@ -20,6 +20,7 @@
 -export([
     format_path/1,
     check/2,
+    check/3,
     compact_errors/2,
     format_error/1,
     format_error/2,
@@ -37,20 +38,23 @@ format_path([Name | Rest]) -> [iol(Name), "." | format_path(Rest)].
 %% Always return plain map with atom keys.
 -spec check(module(), hocon:config() | iodata()) ->
     {ok, hocon:config()} | {error, any()}.
-check(SchemaModule, Conf) when is_map(Conf) ->
+check(SchemaModule, Conf) ->
     %% TODO: remove required
     %% fields should state required or not in their schema
     Opts = #{atom_key => true, required => false},
+    check(SchemaModule, Conf, Opts).
+
+check(SchemaModule, Conf, Opts) when is_map(Conf) ->
     try
         {ok, hocon_tconf:check_plain(SchemaModule, Conf, Opts)}
     catch
         throw:Errors:Stacktrace ->
             compact_errors(Errors, Stacktrace)
     end;
-check(SchemaModule, HoconText) ->
+check(SchemaModule, HoconText, Opts) ->
     case hocon:binary(HoconText, #{format => map}) of
         {ok, MapConfig} ->
-            check(SchemaModule, MapConfig);
+            check(SchemaModule, MapConfig, Opts);
         {error, Reason} ->
             {error, Reason}
     end.

+ 1 - 1
apps/emqx_authn/src/emqx_authn.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_authn, [
     {description, "EMQX Authentication"},
-    {vsn, "0.1.12"},
+    {vsn, "0.1.13"},
     {modules, []},
     {registered, [emqx_authn_sup, emqx_authn_registry]},
     {applications, [kernel, stdlib, emqx_resource, emqx_connector, ehttpc, epgsql, mysql, jose]},

+ 10 - 5
apps/emqx_authn/src/emqx_authn.erl

@@ -69,11 +69,7 @@ do_check_config(#{<<"mechanism">> := Mec0} = Config, Opts) ->
         false ->
             throw(#{error => unknown_authn_provider, which => Key});
         {_, ProviderModule} ->
-            emqx_hocon:check(
-                ProviderModule,
-                #{?CONF_NS_BINARY => Config},
-                Opts#{atom_key => true}
-            )
+            do_check_config_maybe_throw(ProviderModule, Config, Opts)
     end;
 do_check_config(Config, Opts) when is_map(Config) ->
     throw(#{
@@ -82,6 +78,15 @@ do_check_config(Config, Opts) when is_map(Config) ->
         reason => "mechanism_field_required"
     }).
 
+do_check_config_maybe_throw(ProviderModule, Config0, Opts) ->
+    Config = #{?CONF_NS_BINARY => Config0},
+    case emqx_hocon:check(ProviderModule, Config, Opts#{atom_key => true}) of
+        {ok, Checked} ->
+            Checked;
+        {error, Reason} ->
+            throw(Reason)
+    end.
+
 %% 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.

+ 11 - 20
apps/emqx_authn/src/emqx_authn_app.erl

@@ -53,26 +53,17 @@ stop(_State) ->
 %%------------------------------------------------------------------------------
 
 initialize() ->
-    try
-        ok = ?AUTHN:register_providers(emqx_authn:providers()),
-
-        lists:foreach(
-            fun({ChainName, RawAuthConfigs}) ->
-                AuthConfig = emqx_authn:check_configs(RawAuthConfigs),
-                ?AUTHN:initialize_authentication(
-                    ChainName,
-                    AuthConfig
-                )
-            end,
-            chain_configs()
-        )
-    of
-        ok -> ok
-    catch
-        throw:Reason ->
-            ?SLOG(error, #{msg => "failed_to_initialize_authentication", reason => Reason}),
-            {error, {failed_to_initialize_authentication, Reason}}
-    end.
+    ok = ?AUTHN:register_providers(emqx_authn:providers()),
+    lists:foreach(
+        fun({ChainName, RawAuthConfigs}) ->
+            AuthConfig = emqx_authn:check_configs(RawAuthConfigs),
+            ?AUTHN:initialize_authentication(
+                ChainName,
+                AuthConfig
+            )
+        end,
+        chain_configs()
+    ).
 
 deinitialize() ->
     ok = ?AUTHN:deregister_providers(provider_types()),

+ 1 - 1
apps/emqx_conf/src/emqx_conf.app.src

@@ -1,6 +1,6 @@
 {application, emqx_conf, [
     {description, "EMQX configuration management"},
-    {vsn, "0.1.11"},
+    {vsn, "0.1.12"},
     {registered, []},
     {mod, {emqx_conf_app, []}},
     {applications, [kernel, stdlib]},