diff --git a/apps/emqx/src/emqx_schema.erl b/apps/emqx/src/emqx_schema.erl index 77b51f074..fa4b068a1 100644 --- a/apps/emqx/src/emqx_schema.erl +++ b/apps/emqx/src/emqx_schema.erl @@ -2363,7 +2363,12 @@ authentication(Which) -> Module -> Module:root_type() 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(). qos() -> diff --git a/apps/emqx_authn/src/emqx_authn_api.erl b/apps/emqx_authn/src/emqx_authn_api.erl index 452a7bb90..7e1c613d3 100644 --- a/apps/emqx_authn/src/emqx_authn_api.erl +++ b/apps/emqx_authn/src/emqx_authn_api.erl @@ -1232,15 +1232,10 @@ serialize_error({unknown_authn_type, Type}) -> code => <<"BAD_REQUEST">>, 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) -> {400, #{ code => <<"BAD_REQUEST">>, - message => binfmt("~p", [Reason]) + message => binfmt("~0p", [Reason]) }}. parse_position(<<"front">>) -> diff --git a/apps/emqx_authn/src/emqx_authn_password_hashing.erl b/apps/emqx_authn/src/emqx_authn_password_hashing.erl index b3e90e2cd..4954cd66e 100644 --- a/apps/emqx_authn/src/emqx_authn_password_hashing.erl +++ b/apps/emqx_authn/src/emqx_authn_password_hashing.erl @@ -64,7 +64,7 @@ ]). namespace() -> "authn-hash". -roots() -> [pbkdf2, bcrypt, bcrypt_rw, other_algorithms]. +roots() -> [pbkdf2, bcrypt, bcrypt_rw, simple]. fields(bcrypt_rw) -> fields(bcrypt) ++ @@ -96,7 +96,7 @@ fields(pbkdf2) -> )}, {dk_length, fun dk_length/1} ]; -fields(other_algorithms) -> +fields(simple) -> [ {name, sc( @@ -112,8 +112,8 @@ desc(bcrypt) -> "Settings for bcrypt password hashing algorithm."; desc(pbkdf2) -> "Settings for PBKDF2 password hashing algorithm."; -desc(other_algorithms) -> - "Settings for other password hashing algorithms."; +desc(simple) -> + "Settings for simple algorithms."; desc(_) -> undefined. @@ -231,17 +231,31 @@ check_password(#{name := Other, salt_position := SaltPosition}, Salt, PasswordHa %%------------------------------------------------------------------------------ rw_refs() -> - [ + All = [ hoconsc:ref(?MODULE, bcrypt_rw), 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() -> - [ + All = [ hoconsc:ref(?MODULE, bcrypt), 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). 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 f8a278ee5..e32786b04 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl +++ b/apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl @@ -171,7 +171,14 @@ refs() -> refs(#{<<"mechanism">> := <<"jwt">>} = V) -> 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, _) -> {ok, hoconsc:ref(?MODULE, 'jwks')}; @@ -180,7 +187,7 @@ select_ref(false, #{<<"public_key">> := _}) -> select_ref(false, _) -> {ok, hoconsc:ref(?MODULE, 'hmac-based')}; select_ref(_, _) -> - {error, "use_jwks must be set"}. + {error, "use_jwks must be set to true or false"}. 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 cd97a15d9..740e7387a 100644 --- a/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl @@ -52,6 +52,7 @@ end_per_testcase(_Case, 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">>, @@ -61,8 +62,7 @@ t_check_schema(_Config) -> <<"salt_rounds">> => <<"6">> } }, - - hocon_tconf:check_plain(emqx_authn_mnesia, ?CONF(ConfigOk)), + _ = Check(ConfigOk), ConfigNotOk = #{ <<"mechanism">> => <<"password_based">>, @@ -72,11 +72,31 @@ t_check_schema(_Config) -> <<"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( - throw, - {emqx_authn_mnesia, _}, - hocon_tconf:check_plain(emqx_authn_mnesia, ?CONF(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(_) -> diff --git a/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl b/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl index fa0658f6a..65c783298 100644 --- a/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl @@ -105,17 +105,12 @@ t_update_with_invalid_config(_Config) -> AuthConfig = raw_pgsql_auth_config(), BadConfig = maps:without([<<"server">>], AuthConfig), ?assertMatch( - {error, - {bad_authenticator_config, #{ - reason := - {emqx_authn_pgsql, [ - #{ - kind := validation_error, - path := "authentication.server", - reason := required_field - } - ]} - }}}, + {error, #{ + kind := validation_error, + matched_type := "authn-postgresql:authentication", + path := "authentication.1.server", + reason := required_field + }}, emqx:update_config( ?PATH, {create_authenticator, ?GLOBAL, BadConfig} diff --git a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl index 4f25c8709..4f95bef93 100644 --- a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl @@ -160,10 +160,12 @@ t_create_invalid_config(_Config) -> Config0 = raw_redis_auth_config(), Config = maps:without([<<"server">>], Config0), ?assertMatch( - {error, - {bad_authenticator_config, #{ - reason := {emqx_authn_redis, [#{kind := validation_error}]} - }}}, + {error, #{ + kind := validation_error, + matched_type := "authn-redis:standalone", + path := "authentication.1.server", + reason := required_field + }}, emqx:update_config(?PATH, {create_authenticator, ?GLOBAL, Config}) ), ?assertMatch([], emqx_config:get_raw([authentication])),