refactor: authn schema union selector

apps/emqx/src/emqx_schema.erl

@@ -2363,12 +2363,12 @@ authentication(Which) ->
             Module ->
                 Module:root_type()
         end,
-    hoconsc:mk(Type, #{desc => Desc, converter => fun ensure_array/1}).
+    hoconsc:mk(Type, #{desc => Desc, converter => fun ensure_array/2}).
 
 %% the older version schema allows individual element (instead of a chain) in config
-ensure_array(undefined) -> undefined;
+ensure_array(undefined, _) -> undefined;
-ensure_array(L) when is_list(L) -> L;
+ensure_array(L, _) when is_list(L) -> L;
-ensure_array(M) when is_map(M) -> [M].
+ensure_array(M, _) -> [M].
 
 -spec qos() -> typerefl:type().
 qos() ->

apps/emqx_authn/src/emqx_authn_schema.erl

@@ -69,80 +69,53 @@ union_member_selector(Providers) ->
         ({value, Value}) -> select_union_member(Value, Providers)
     end.
 
-select_union_member(#{<<"mechanism">> := _} = Value, Providers) ->
+select_union_member(#{<<"mechanism">> := _} = Value, Providers0) ->
-    select_union_member(Value, Providers, #{});
-select_union_member(_Value, _) ->
-    throw(#{hint => "missing 'mechanism' field"}).
-
-select_union_member(Value, [], ReasonsMap) when ReasonsMap =:= #{} ->
     BackendVal = maps:get(<<"backend">>, Value, undefined),
     MechanismVal = maps:get(<<"mechanism">>, Value),
-    throw(#{
+    BackendFilterFn = fun
-        backend => BackendVal,
+        ({{_Mec, Backend}, _Mod}) ->
-        mechanism => MechanismVal,
+            BackendVal =:= atom_to_binary(Backend);
-        hint => "unknown_mechanism_or_backend"
+        (_) ->
-    });
+            BackendVal =:= undefined
-select_union_member(_Value, [], ReasonsMap) ->
-    throw(ReasonsMap);
-select_union_member(Value, [Provider | Providers], ReasonsMap) ->
-    {Mechanism, Backend, Module} =
-        case Provider of
-            {{M, B}, Mod} -> {atom_to_binary(M), atom_to_binary(B), Mod};
-            {M, Mod} when is_atom(M) -> {atom_to_binary(M), undefined, Mod}
     end,
-    case do_select_union_member(Mechanism, Backend, Module, Value) of
+    MechanismFilterFn = fun
-        {ok, Type} ->
+        ({{Mechanism, _Backend}, _Mod}) ->
-            [Type];
+            MechanismVal =:= atom_to_binary(Mechanism);
-        {error, nomatch} ->
+        ({Mechanism, _Mod}) ->
-            %% obvious mismatch, do not complain
+            MechanismVal =:= atom_to_binary(Mechanism)
-            %% e.g. when 'backend' is "http", but the value is "redis",
+    end,
-            %% then there is no need to record the error like
+    case lists:filter(BackendFilterFn, Providers0) of
-            %% "'http' is exepcted but got 'redis'"
+        [] ->
-            select_union_member(Value, Providers, ReasonsMap);
+            throw(#{reason => "unknown_backend", backend => BackendVal});
-        {error, Reason} ->
+        Providers1 ->
-            %% more interesting error message
+            case lists:filter(MechanismFilterFn, Providers1) of
-            %% e.g. when 'backend' is "http", but there is no "method" field
+                [] ->
-            %% found so there is no way to tell if it's the 'get' type or 'post' type.
+                    throw(#{
-            %% hence the error message is like:
+                        reason => "unsupported_mechanism",
-            %% #{emqx_auth_http => "'http' auth backend must have get|post as 'method'"}
+                        mechanism => MechanismVal,
-            select_union_member(Value, Providers, ReasonsMap#{Module => Reason})
+                        backend => BackendVal
-    end.
+                    });
-
+                [{_, Module}] ->
-do_select_union_member(Mechanism, Backend, Module, Value) ->
+                    try_select_union_member(Module, Value)
-    BackendVal = maps:get(<<"backend">>, Value, undefined),
+            end
-    MechanismVal = maps:get(<<"mechanism">>, Value),
-    case MechanismVal =:= Mechanism of
-        true when Backend =:= undefined ->
-            case BackendVal =:= undefined of
-                true ->
-                    %% e.g. jwt has no 'backend'
-                    try_select_union_member(Module, Value);
-                false ->
-                    {error, "unexpected 'backend' for " ++ binary_to_list(Mechanism)}
     end;
-        true ->
+select_union_member(Value, _Providers) when is_map(Value) ->
-            case Backend =:= BackendVal of
+    throw(#{reason => "missing_mechanism_field"});
-                true ->
+select_union_member(Value, _Providers) ->
-                    try_select_union_member(Module, Value);
+    throw(#{reason => "not_a_struct", value => Value}).
-                false ->
-                    %% 'backend' not matching
-                    {error, nomatch}
-            end;
-        false ->
-            %% 'mechanism' not matching
-            {error, nomatch}
-    end.
 
 try_select_union_member(Module, Value) ->
-    try
     %% some modules have refs/1 exported to help selectin the sub-types
     %% emqx_authn_http, emqx_authn_jwt, emqx_authn_mongodb and emqx_authn_redis
-        Module:refs(Value)
+    try Module:refs(Value) of
+        {ok, Type} ->
+            [Type];
+        {error, Reason} ->
+            throw(Reason)
     catch
         error:undef ->
             %% otherwise expect only one member from this module
-            [Type] = Module:refs(),
+            Module:refs()
-            {ok, Type}
     end.
 
 %% authn is a core functionality however implemented outside of emqx app

apps/emqx_authn/src/simple_authn/emqx_authn_http.erl

@@ -165,7 +165,10 @@ refs(#{<<"method">> := <<"get">>}) ->
 refs(#{<<"method">> := <<"post">>}) ->
     {ok, hoconsc:ref(?MODULE, post)};
 refs(_) ->
-    {error, "'http' auth backend must have get|post as 'method'"}.
+    {error, #{
+        field_name => method,
+        expected => "get | post"
+    }}.
 
 create(_AuthenticatorID, Config) ->
     create(Config).

apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl

@@ -187,7 +187,10 @@ select_ref(false, #{<<"public_key">> := _}) ->
 select_ref(false, _) ->
     {ok, hoconsc:ref(?MODULE, 'hmac-based')};
 select_ref(_, _) ->
-    {error, "use_jwks must be set to true or false"}.
+    {error, #{
+        field_name => use_jwks,
+        expected => "true | false"
+    }}.
 
 create(_AuthenticatorID, Config) ->
     create(Config).

apps/emqx_authn/src/simple_authn/emqx_authn_mongodb.erl

@@ -138,7 +138,10 @@ refs(#{<<"mongo_type">> := <<"rs">>}) ->
 refs(#{<<"mongo_type">> := <<"sharded">>}) ->
     {ok, hoconsc:ref(?MODULE, 'sharded-cluster')};
 refs(_) ->
-    {error, "unknown 'mongo_type'"}.
+    {error, #{
+        field_name => mongo_type,
+        expected => "single | rs | sharded"
+    }}.
 
 create(_AuthenticatorID, Config) ->
     create(Config).

apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl

@@ -105,7 +105,10 @@ refs(#{<<"redis_type">> := <<"cluster">>}) ->
 refs(#{<<"redis_type">> := <<"sentinel">>}) ->
     {ok, hoconsc:ref(?MODULE, sentinel)};
 refs(_) ->
-    {error, "unknown 'redis_type'"}.
+    {error, #{
+        field_name => redis_type,
+        expected => "single | cluster | sentinel"
+    }}.
 
 create(_AuthenticatorID, Config) ->
     create(Config).

apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl

@@ -49,56 +49,6 @@ end_per_testcase(_Case, Config) ->
 %% Tests
 %%------------------------------------------------------------------------------
 
--define(CONF(Conf), #{?CONF_NS_BINARY => Conf}).
-
-t_check_schema(_Config) ->
-    Check = fun(C) -> emqx_config:check_config(emqx_schema, ?CONF(C)) end,
-    ConfigOk = #{
-        <<"mechanism">> => <<"password_based">>,
-        <<"backend">> => <<"built_in_database">>,
-        <<"user_id_type">> => <<"username">>,
-        <<"password_hash_algorithm">> => #{
-            <<"name">> => <<"bcrypt">>,
-            <<"salt_rounds">> => <<"6">>
-        }
-    },
-    _ = Check(ConfigOk),
-
-    ConfigNotOk = #{
-        <<"mechanism">> => <<"password_based">>,
-        <<"backend">> => <<"built_in_database">>,
-        <<"user_id_type">> => <<"username">>,
-        <<"password_hash_algorithm">> => #{
-            <<"name">> => <<"md6">>
-        }
-    },
-    ?assertThrow(
-        #{
-            path := "authentication.1.password_hash_algorithm.name",
-            matched_type := "authn-builtin_db:authentication/authn-hash:simple",
-            reason := unable_to_convert_to_enum_symbol
-        },
-        Check(ConfigNotOk)
-    ),
-
-    ConfigMissingAlgoName = #{
-        <<"mechanism">> => <<"password_based">>,
-        <<"backend">> => <<"built_in_database">>,
-        <<"user_id_type">> => <<"username">>,
-        <<"password_hash_algorithm">> => #{
-            <<"foo">> => <<"bar">>
-        }
-    },
-
-    ?assertThrow(
-        #{
-            path := "authentication.1.password_hash_algorithm",
-            reason := "algorithm_name_missing",
-            matched_type := "authn-builtin_db:authentication"
-        },
-        Check(ConfigMissingAlgoName)
-    ).
-
 t_create(_) ->
     Config0 = config(),
 

apps/emqx_authn/test/emqx_authn_schema_SUITE.erl

@@ -0,0 +1,190 @@
+-module(emqx_authn_schema_SUITE).
+
+-compile(export_all).
+-compile(nowarn_export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+-include("emqx_authn.hrl").
+
+all() ->
+    emqx_common_test_helpers:all(?MODULE).
+
+init_per_suite(Config) ->
+    _ = application:load(emqx_conf),
+    emqx_common_test_helpers:start_apps([emqx_authn]),
+    Config.
+
+end_per_suite(_) ->
+    emqx_common_test_helpers:stop_apps([emqx_authn]),
+    ok.
+
+init_per_testcase(_Case, Config) ->
+    {ok, _} = emqx_cluster_rpc:start_link(node(), emqx_cluster_rpc, 1000),
+    mria:clear_table(emqx_authn_mnesia),
+    Config.
+
+end_per_testcase(_Case, Config) ->
+    Config.
+
+-define(CONF(Conf), #{?CONF_NS_BINARY => Conf}).
+
+t_check_schema(_Config) ->
+    Check = fun(C) -> emqx_config:check_config(emqx_schema, ?CONF(C)) end,
+    ConfigOk = #{
+        <<"mechanism">> => <<"password_based">>,
+        <<"backend">> => <<"built_in_database">>,
+        <<"user_id_type">> => <<"username">>,
+        <<"password_hash_algorithm">> => #{
+            <<"name">> => <<"bcrypt">>,
+            <<"salt_rounds">> => <<"6">>
+        }
+    },
+    _ = Check(ConfigOk),
+
+    ConfigNotOk = #{
+        <<"mechanism">> => <<"password_based">>,
+        <<"backend">> => <<"built_in_database">>,
+        <<"user_id_type">> => <<"username">>,
+        <<"password_hash_algorithm">> => #{
+            <<"name">> => <<"md6">>
+        }
+    },
+    ?assertThrow(
+        #{
+            path := "authentication.1.password_hash_algorithm.name",
+            matched_type := "authn-builtin_db:authentication/authn-hash:simple",
+            reason := unable_to_convert_to_enum_symbol
+        },
+        Check(ConfigNotOk)
+    ),
+
+    ConfigMissingAlgoName = #{
+        <<"mechanism">> => <<"password_based">>,
+        <<"backend">> => <<"built_in_database">>,
+        <<"user_id_type">> => <<"username">>,
+        <<"password_hash_algorithm">> => #{
+            <<"foo">> => <<"bar">>
+        }
+    },
+
+    ?assertThrow(
+        #{
+            path := "authentication.1.password_hash_algorithm",
+            reason := "algorithm_name_missing",
+            matched_type := "authn-builtin_db:authentication"
+        },
+        Check(ConfigMissingAlgoName)
+    ).
+
+t_union_member_selector(_) ->
+    ?assertMatch(#{authentication := undefined}, check(undefined)),
+    C1 = #{<<"backend">> => <<"built_in_database">>},
+    ?assertThrow(
+        #{
+            path := "authentication.1",
+            reason := "missing_mechanism_field"
+        },
+        check(C1)
+    ),
+    C2 = <<"foobar">>,
+    ?assertThrow(
+        #{
+            path := "authentication.1",
+            reason := "not_a_struct",
+            value := <<"foobar">>
+        },
+        check(C2)
+    ),
+    Base = #{
+        <<"user_id_type">> => <<"username">>,
+        <<"password_hash_algorithm">> => #{
+            <<"name">> => <<"plain">>
+        }
+    },
+    BadBackend = Base#{<<"mechanism">> => <<"password_based">>, <<"backend">> => <<"bar">>},
+    ?assertThrow(
+        #{
+            reason := "unknown_backend",
+            backend := <<"bar">>
+        },
+        check(BadBackend)
+    ),
+    BadMechanism = Base#{<<"mechanism">> => <<"foo">>, <<"backend">> => <<"built_in_database">>},
+    ?assertThrow(
+        #{
+            reason := "unsupported_mechanism",
+            mechanism := <<"foo">>,
+            backend := <<"built_in_database">>
+        },
+        check(BadMechanism)
+    ),
+    BadCombination = Base#{<<"mechanism">> => <<"scram">>, <<"backend">> => <<"http">>},
+    ?assertThrow(
+        #{
+            reason := "unsupported_mechanism",
+            mechanism := <<"scram">>,
+            backend := <<"http">>
+        },
+        check(BadCombination)
+    ),
+    ok.
+
+t_http_auth_selector(_) ->
+    C1 = #{
+        <<"mechanism">> => <<"password_based">>,
+        <<"backend">> => <<"http">>
+    },
+    ?assertThrow(
+        #{
+            field_name := method,
+            expected := "get | post"
+        },
+        check(C1)
+    ),
+    ok.
+
+t_mongo_auth_selector(_) ->
+    C1 = #{
+        <<"mechanism">> => <<"password_based">>,
+        <<"backend">> => <<"mongodb">>
+    },
+    ?assertThrow(
+        #{
+            field_name := mongo_type,
+            expected := "single | rs | sharded"
+        },
+        check(C1)
+    ),
+    ok.
+
+t_redis_auth_selector(_) ->
+    C1 = #{
+        <<"mechanism">> => <<"password_based">>,
+        <<"backend">> => <<"redis">>
+    },
+    ?assertThrow(
+        #{
+            field_name := redis_type,
+            expected := "single | cluster | sentinel"
+        },
+        check(C1)
+    ),
+    ok.
+
+t_redis_jwt_selector(_) ->
+    C1 = #{
+        <<"mechanism">> => <<"jwt">>
+    },
+    ?assertThrow(
+        #{
+            field_name := use_jwks,
+            expected := "true | false"
+        },
+        check(C1)
+    ),
+    ok.
+
+check(C) ->
+    {_Mappings, Checked} = emqx_config:check_config(emqx_schema, ?CONF(C)),
+    Checked.