diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index fa4b068a1..ba813d2c8 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/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(L) when is_list(L) -> L; -ensure_array(M) when is_map(M) -> [M]. +ensure_array(undefined, _) -> undefined; +ensure_array(L, _) when is_list(L) -> L; +ensure_array(M, _) -> [M]. -spec qos() -> typerefl:type(). qos() -> diff --git a/apps/emqx_authn/src/emqx_authn_schema.erl b/apps/emqx_authn/src/emqx_authn_schema.erl index 9c4c7d89a..ce90aebe9 100644 --- a/apps/emqx_authn/src/emqx_authn_schema.erl +++ b/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(Value, Providers, #{}); -select_union_member(_Value, _) -> - throw(#{hint => "missing 'mechanism' field"}). - -select_union_member(Value, [], ReasonsMap) when ReasonsMap =:= #{} -> +select_union_member(#{<<"mechanism">> := _} = Value, Providers0) -> BackendVal = maps:get(<<"backend">>, Value, undefined), MechanismVal = maps:get(<<"mechanism">>, Value), - throw(#{ - backend => BackendVal, - mechanism => MechanismVal, - hint => "unknown_mechanism_or_backend" - }); -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 - {ok, Type} -> - [Type]; - {error, nomatch} -> - %% obvious mismatch, do not complain - %% e.g. when 'backend' is "http", but the value is "redis", - %% then there is no need to record the error like - %% "'http' is exepcted but got 'redis'" - select_union_member(Value, Providers, ReasonsMap); - {error, Reason} -> - %% more interesting error message - %% 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. - %% hence the error message is like: - %% #{emqx_auth_http => "'http' auth backend must have get|post as 'method'"} - select_union_member(Value, Providers, ReasonsMap#{Module => Reason}) - end. - -do_select_union_member(Mechanism, Backend, Module, Value) -> - BackendVal = maps:get(<<"backend">>, Value, undefined), - 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 -> - case Backend =:= BackendVal of - true -> - try_select_union_member(Module, Value); - false -> - %% 'backend' not matching - {error, nomatch} - end; - false -> - %% 'mechanism' not matching - {error, nomatch} - end. + BackendFilterFn = fun + ({{_Mec, Backend}, _Mod}) -> + BackendVal =:= atom_to_binary(Backend); + (_) -> + BackendVal =:= undefined + end, + MechanismFilterFn = fun + ({{Mechanism, _Backend}, _Mod}) -> + MechanismVal =:= atom_to_binary(Mechanism); + ({Mechanism, _Mod}) -> + MechanismVal =:= atom_to_binary(Mechanism) + end, + case lists:filter(BackendFilterFn, Providers0) of + [] -> + throw(#{reason => "unknown_backend", backend => BackendVal}); + Providers1 -> + case lists:filter(MechanismFilterFn, Providers1) of + [] -> + throw(#{ + reason => "unsupported_mechanism", + mechanism => MechanismVal, + backend => BackendVal + }); + [{_, Module}] -> + try_select_union_member(Module, Value) + end + end; +select_union_member(Value, _Providers) when is_map(Value) -> + throw(#{reason => "missing_mechanism_field"}); +select_union_member(Value, _Providers) -> + throw(#{reason => "not_a_struct", value => Value}). 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) + %% 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 + 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(), - {ok, Type} + Module:refs() end. %% authn is a core functionality however implemented outside of emqx app diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_http.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_http.erl index ecd084c08..0c0ec3b06 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_http.erl +++ b/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). diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl index e32786b04..d96f9073e 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl +++ b/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). diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_mongodb.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_mongodb.erl index 3820005af..11048528c 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_mongodb.erl +++ b/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). diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl index cafcf1830..419899bbd 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_redis.erl +++ b/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). diff --git a/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl b/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl index 740e7387a..599eae92e 100644 --- a/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl +++ b/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(), diff --git a/apps/emqx_authn/test/emqx_authn_schema_SUITE.erl b/apps/emqx_authn/test/emqx_authn_schema_SUITE.erl new file mode 100644 index 000000000..7e67584ac --- /dev/null +++ b/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.