fix: schema check error reason pattern

This commit is contained in:
Zaiming (Stone) Shi 2023-01-12 12:58:28 +01:00
parent 65319567cc
commit 4f91bf415c
7 changed files with 78 additions and 40 deletions

View File

@ -2363,7 +2363,12 @@ authentication(Which) ->
Module -> Module ->
Module:root_type() Module:root_type()
end, end,
hoconsc:mk(Type, #{desc => Desc}). hoconsc:mk(Type, #{desc => Desc, converter => fun ensure_array/1}).
%% 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].
-spec qos() -> typerefl:type(). -spec qos() -> typerefl:type().
qos() -> qos() ->

View File

@ -1232,15 +1232,10 @@ serialize_error({unknown_authn_type, Type}) ->
code => <<"BAD_REQUEST">>, code => <<"BAD_REQUEST">>,
message => binfmt("Unknown type '~p'", [Type]) message => binfmt("Unknown type '~p'", [Type])
}}; }};
serialize_error({bad_authenticator_config, Reason}) ->
{400, #{
code => <<"BAD_REQUEST">>,
message => binfmt("Bad authenticator config ~p", [Reason])
}};
serialize_error(Reason) -> serialize_error(Reason) ->
{400, #{ {400, #{
code => <<"BAD_REQUEST">>, code => <<"BAD_REQUEST">>,
message => binfmt("~p", [Reason]) message => binfmt("~0p", [Reason])
}}. }}.
parse_position(<<"front">>) -> parse_position(<<"front">>) ->

View File

@ -64,7 +64,7 @@
]). ]).
namespace() -> "authn-hash". namespace() -> "authn-hash".
roots() -> [pbkdf2, bcrypt, bcrypt_rw, other_algorithms]. roots() -> [pbkdf2, bcrypt, bcrypt_rw, simple].
fields(bcrypt_rw) -> fields(bcrypt_rw) ->
fields(bcrypt) ++ fields(bcrypt) ++
@ -96,7 +96,7 @@ fields(pbkdf2) ->
)}, )},
{dk_length, fun dk_length/1} {dk_length, fun dk_length/1}
]; ];
fields(other_algorithms) -> fields(simple) ->
[ [
{name, {name,
sc( sc(
@ -112,8 +112,8 @@ desc(bcrypt) ->
"Settings for bcrypt password hashing algorithm."; "Settings for bcrypt password hashing algorithm.";
desc(pbkdf2) -> desc(pbkdf2) ->
"Settings for PBKDF2 password hashing algorithm."; "Settings for PBKDF2 password hashing algorithm.";
desc(other_algorithms) -> desc(simple) ->
"Settings for other password hashing algorithms."; "Settings for simple algorithms.";
desc(_) -> desc(_) ->
undefined. undefined.
@ -231,17 +231,31 @@ check_password(#{name := Other, salt_position := SaltPosition}, Salt, PasswordHa
%%------------------------------------------------------------------------------ %%------------------------------------------------------------------------------
rw_refs() -> rw_refs() ->
[ All = [
hoconsc:ref(?MODULE, bcrypt_rw), hoconsc:ref(?MODULE, bcrypt_rw),
hoconsc:ref(?MODULE, pbkdf2), hoconsc:ref(?MODULE, pbkdf2),
hoconsc:ref(?MODULE, other_algorithms) hoconsc:ref(?MODULE, simple)
]. ],
fun
(all_union_members) -> All;
({value, #{<<"name">> := <<"bcrypt">>}}) -> [hoconsc:ref(?MODULE, bcrypt_rw)];
({value, #{<<"name">> := <<"pbkdf2">>}}) -> [hoconsc:ref(?MODULE, pbkdf2)];
({value, #{<<"name">> := _}}) -> [hoconsc:ref(?MODULE, simple)];
({value, _}) -> throw(#{reason => "algorithm_name_missing"})
end.
ro_refs() -> ro_refs() ->
[ All = [
hoconsc:ref(?MODULE, bcrypt), hoconsc:ref(?MODULE, bcrypt),
hoconsc:ref(?MODULE, pbkdf2), hoconsc:ref(?MODULE, pbkdf2),
hoconsc:ref(?MODULE, other_algorithms) hoconsc:ref(?MODULE, simple)
]. ],
fun
(all_union_members) -> All;
({value, #{<<"name">> := <<"bcrypt">>}}) -> [hoconsc:ref(?MODULE, bcrypt)];
({value, #{<<"name">> := <<"pbkdf2">>}}) -> [hoconsc:ref(?MODULE, pbkdf2)];
({value, #{<<"name">> := _}}) -> [hoconsc:ref(?MODULE, simple)];
({value, _}) -> throw(#{reason => "algorithm_name_missing"})
end.
sc(Type, Meta) -> hoconsc:mk(Type, Meta). sc(Type, Meta) -> hoconsc:mk(Type, Meta).

View File

@ -171,7 +171,14 @@ refs() ->
refs(#{<<"mechanism">> := <<"jwt">>} = V) -> refs(#{<<"mechanism">> := <<"jwt">>} = V) ->
UseJWKS = maps:get(<<"use_jwks">>, V, undefined), UseJWKS = maps:get(<<"use_jwks">>, V, undefined),
select_ref(UseJWKS, V). select_ref(boolean(UseJWKS), V).
%% this field is technically a boolean type,
%% but union member selection is done before type casting (by typrefl),
%% so we have to allow strings too
boolean(<<"true">>) -> true;
boolean(<<"false">>) -> false;
boolean(Other) -> Other.
select_ref(true, _) -> select_ref(true, _) ->
{ok, hoconsc:ref(?MODULE, 'jwks')}; {ok, hoconsc:ref(?MODULE, 'jwks')};
@ -180,7 +187,7 @@ select_ref(false, #{<<"public_key">> := _}) ->
select_ref(false, _) -> select_ref(false, _) ->
{ok, hoconsc:ref(?MODULE, 'hmac-based')}; {ok, hoconsc:ref(?MODULE, 'hmac-based')};
select_ref(_, _) -> select_ref(_, _) ->
{error, "use_jwks must be set"}. {error, "use_jwks must be set to true or false"}.
create(_AuthenticatorID, Config) -> create(_AuthenticatorID, Config) ->
create(Config). create(Config).

View File

@ -52,6 +52,7 @@ end_per_testcase(_Case, Config) ->
-define(CONF(Conf), #{?CONF_NS_BINARY => Conf}). -define(CONF(Conf), #{?CONF_NS_BINARY => Conf}).
t_check_schema(_Config) -> t_check_schema(_Config) ->
Check = fun(C) -> emqx_config:check_config(emqx_schema, ?CONF(C)) end,
ConfigOk = #{ ConfigOk = #{
<<"mechanism">> => <<"password_based">>, <<"mechanism">> => <<"password_based">>,
<<"backend">> => <<"built_in_database">>, <<"backend">> => <<"built_in_database">>,
@ -61,8 +62,7 @@ t_check_schema(_Config) ->
<<"salt_rounds">> => <<"6">> <<"salt_rounds">> => <<"6">>
} }
}, },
_ = Check(ConfigOk),
hocon_tconf:check_plain(emqx_authn_mnesia, ?CONF(ConfigOk)),
ConfigNotOk = #{ ConfigNotOk = #{
<<"mechanism">> => <<"password_based">>, <<"mechanism">> => <<"password_based">>,
@ -72,11 +72,31 @@ t_check_schema(_Config) ->
<<"name">> => <<"md6">> <<"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)
),
?assertException( ConfigMissingAlgoName = #{
throw, <<"mechanism">> => <<"password_based">>,
{emqx_authn_mnesia, _}, <<"backend">> => <<"built_in_database">>,
hocon_tconf:check_plain(emqx_authn_mnesia, ?CONF(ConfigNotOk)) <<"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(_) -> t_create(_) ->

View File

@ -105,17 +105,12 @@ t_update_with_invalid_config(_Config) ->
AuthConfig = raw_pgsql_auth_config(), AuthConfig = raw_pgsql_auth_config(),
BadConfig = maps:without([<<"server">>], AuthConfig), BadConfig = maps:without([<<"server">>], AuthConfig),
?assertMatch( ?assertMatch(
{error, {error, #{
{bad_authenticator_config, #{
reason :=
{emqx_authn_pgsql, [
#{
kind := validation_error, kind := validation_error,
path := "authentication.server", matched_type := "authn-postgresql:authentication",
path := "authentication.1.server",
reason := required_field reason := required_field
} }},
]}
}}},
emqx:update_config( emqx:update_config(
?PATH, ?PATH,
{create_authenticator, ?GLOBAL, BadConfig} {create_authenticator, ?GLOBAL, BadConfig}

View File

@ -160,10 +160,12 @@ t_create_invalid_config(_Config) ->
Config0 = raw_redis_auth_config(), Config0 = raw_redis_auth_config(),
Config = maps:without([<<"server">>], Config0), Config = maps:without([<<"server">>], Config0),
?assertMatch( ?assertMatch(
{error, {error, #{
{bad_authenticator_config, #{ kind := validation_error,
reason := {emqx_authn_redis, [#{kind := validation_error}]} matched_type := "authn-redis:standalone",
}}}, path := "authentication.1.server",
reason := required_field
}},
emqx:update_config(?PATH, {create_authenticator, ?GLOBAL, Config}) emqx:update_config(?PATH, {create_authenticator, ?GLOBAL, Config})
), ),
?assertMatch([], emqx_config:get_raw([authentication])), ?assertMatch([], emqx_config:get_raw([authentication])),