From ffbf9b0fabd757e22c5d88489e8f69bb75598f9c Mon Sep 17 00:00:00 2001 From: zhanghongtong Date: Fri, 3 Sep 2021 13:54:32 +0800 Subject: [PATCH] feat(authz): check for duplicate source types Signed-off-by: zhanghongtong --- apps/emqx_authz/src/emqx_authz.erl | 62 +++++++++++++++---- .../test/emqx_authz_api_sources_SUITE.erl | 25 +++----- 2 files changed, 61 insertions(+), 26 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index f950ed53c..7fcd80269 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/apps/emqx_authz/src/emqx_authz.erl @@ -39,6 +39,7 @@ -export([post_config_update/4, pre_config_update/2]). -define(CONF_KEY_PATH, [authorization, sources]). +-define(SOURCE_TYPES, [file, http, mongo, mysql, pgsql, redis]). -spec(register_metrics() -> ok). register_metrics() -> @@ -47,7 +48,9 @@ register_metrics() -> init() -> ok = register_metrics(), emqx_config_handler:add_handler(?CONF_KEY_PATH, ?MODULE), - NSources = [init_source(Source) || Source <- emqx:get_config(?CONF_KEY_PATH, [])], + Sources = emqx:get_config(?CONF_KEY_PATH, []), + ok = check_dup_types(Sources), + NSources = [init_source(Source) || Source <- Sources], ok = emqx_hooks:add('client.authorize', {?MODULE, authorize, [NSources]}, -1). lookup() -> @@ -83,12 +86,16 @@ update(Cmd, Sources, Opts) -> pre_config_update({move, Type, <<"top">>}, Conf) when is_list(Conf) -> {Index, _} = find_source_by_type(Type), {List1, List2} = lists:split(Index, Conf), - {ok, [lists:nth(Index, Conf)] ++ lists:droplast(List1) ++ List2}; + NConf = [lists:nth(Index, Conf)] ++ lists:droplast(List1) ++ List2, + ok = check_dup_types(NConf), + {ok, NConf}; pre_config_update({move, Type, <<"bottom">>}, Conf) when is_list(Conf) -> {Index, _} = find_source_by_type(Type), {List1, List2} = lists:split(Index, Conf), - {ok, lists:droplast(List1) ++ List2 ++ [lists:nth(Index, Conf)]}; + NConf = lists:droplast(List1) ++ List2 ++ [lists:nth(Index, Conf)], + ok = check_dup_types(NConf), + {ok, NConf}; pre_config_update({move, Type, #{<<"before">> := Before}}, Conf) when is_list(Conf) -> {Index1, _} = find_source_by_type(Type), @@ -97,9 +104,11 @@ pre_config_update({move, Type, #{<<"before">> := Before}}, Conf) when is_list(Co Conf2 = lists:nth(Index2, Conf), {List1, List2} = lists:split(Index2, Conf), - {ok, lists:delete(Conf1, lists:droplast(List1)) - ++ [Conf1] ++ [Conf2] - ++ lists:delete(Conf1, List2)}; + NConf = lists:delete(Conf1, lists:droplast(List1)) + ++ [Conf1] ++ [Conf2] + ++ lists:delete(Conf1, List2), + ok = check_dup_types(NConf), + {ok, NConf}; pre_config_update({move, Type, #{<<"after">> := After}}, Conf) when is_list(Conf) -> {Index1, _} = find_source_by_type(Type), @@ -107,21 +116,31 @@ pre_config_update({move, Type, #{<<"after">> := After}}, Conf) when is_list(Conf {Index2, _} = find_source_by_type(After), {List1, List2} = lists:split(Index2, Conf), - {ok, lists:delete(Conf1, List1) - ++ [Conf1] - ++ lists:delete(Conf1, List2)}; + NConf = lists:delete(Conf1, List1) + ++ [Conf1] + ++ lists:delete(Conf1, List2), + ok = check_dup_types(NConf), + {ok, NConf}; pre_config_update({head, Sources}, Conf) when is_list(Sources), is_list(Conf) -> + NConf = Sources ++ Conf, + ok = check_dup_types(NConf), {ok, Sources ++ Conf}; pre_config_update({tail, Sources}, Conf) when is_list(Sources), is_list(Conf) -> + NConf = Conf ++ Sources, + ok = check_dup_types(NConf), {ok, Conf ++ Sources}; pre_config_update({{replace_once, Type}, Source}, Conf) when is_map(Source), is_list(Conf) -> {Index, _} = find_source_by_type(Type), {List1, List2} = lists:split(Index, Conf), - {ok, lists:droplast(List1) ++ [Source] ++ List2}; + NConf = lists:droplast(List1) ++ [Source] ++ List2, + ok = check_dup_types(NConf), + {ok, NConf}; pre_config_update({{delete_once, Type}, _Source}, Conf) when is_list(Conf) -> {_, Source} = find_source_by_type(Type), - {ok, lists:delete(Source, Conf)}; + NConf = lists:delete(Source, Conf), + ok = check_dup_types(NConf), + {ok, NConf}; pre_config_update({_, Sources}, _Conf) when is_list(Sources)-> %% overwrite the entire config! {ok, Sources}. @@ -212,6 +231,27 @@ post_config_update(_, NewSources, _OldConf, _AppEnvs) -> %% Initialize source %%-------------------------------------------------------------------- +check_dup_types(Sources) -> + check_dup_types(Sources, ?SOURCE_TYPES). +check_dup_types(_Sources, []) -> ok; +check_dup_types(Sources, [T0 | Tail]) -> + case lists:foldl(fun (#{type := T1}, AccIn) -> + case T0 =:= T1 of + true -> AccIn + 1; + false -> AccIn + end; + (#{<<"type">> := T1}, AccIn) -> + case T0 =:= atom(T1) of + true -> AccIn + 1; + false -> AccIn + end + end, 0, Sources) > 1 of + true -> + ?LOG(error, "The type is duplicated in the Authorization source"), + {error, authz_source_dup}; + false -> check_dup_types(Sources, Tail) + end. + init_source(#{enable := true, type := file, path := Path diff --git a/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl b/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl index 3c054aa7d..4dc21647a 100644 --- a/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl @@ -178,16 +178,11 @@ t_api(_) -> {ok, 200, Result1} = request(get, uri(["authorization", "sources"]), []), ?assertEqual([], get_sources(Result1)), - lists:foreach(fun(_) -> - {ok, 204, _} = request(post, uri(["authorization", "sources"]), ?SOURCE1) - end, lists:seq(1, 20)), + {ok, 204, _} = request(put, uri(["authorization", "sources"]), [?SOURCE2, ?SOURCE3, ?SOURCE4, ?SOURCE5, ?SOURCE6]), + {ok, 204, _} = request(post, uri(["authorization", "sources"]), ?SOURCE1), + {ok, 200, Result2} = request(get, uri(["authorization", "sources"]), []), - ?assertEqual(20, length(get_sources(Result2))), - - {ok, 204, _} = request(put, uri(["authorization", "sources"]), [?SOURCE1, ?SOURCE2, ?SOURCE3, ?SOURCE4, ?SOURCE5, ?SOURCE6]), - - {ok, 200, Result3} = request(get, uri(["authorization", "sources"]), []), - Sources = get_sources(Result3), + Sources = get_sources(Result2), ?assertMatch([ #{<<"type">> := <<"http">>} , #{<<"type">> := <<"mongo">>} , #{<<"type">> := <<"mysql">>} @@ -198,8 +193,8 @@ t_api(_) -> ?assert(filelib:is_file(filename:join([emqx:get_config([node, data_dir]), "authorization_rules.conf"]))), {ok, 204, _} = request(put, uri(["authorization", "sources", "http"]), ?SOURCE1#{<<"enable">> := false}), - {ok, 200, Result4} = request(get, uri(["authorization", "sources", "http"]), []), - ?assertMatch(#{<<"type">> := <<"http">>, <<"enable">> := false}, jsx:decode(Result4)), + {ok, 200, Result3} = request(get, uri(["authorization", "sources", "http"]), []), + ?assertMatch(#{<<"type">> := <<"http">>, <<"enable">> := false}, jsx:decode(Result3)), {ok, 204, _} = request(put, uri(["authorization", "sources", "mongo"]), ?SOURCE2#{<<"ssl">> := #{ @@ -209,7 +204,7 @@ t_api(_) -> <<"keyfile">> => <<"fake key file">>, <<"verify">> => false }}), - {ok, 200, Result5} = request(get, uri(["authorization", "sources", "mongo"]), []), + {ok, 200, Result4} = request(get, uri(["authorization", "sources", "mongo"]), []), ?assertMatch(#{<<"type">> := <<"mongo">>, <<"ssl">> := #{<<"enable">> := true, <<"cacertfile">> := <<"fake cacert file">>, @@ -217,7 +212,7 @@ t_api(_) -> <<"keyfile">> := <<"fake key file">>, <<"verify">> := false } - }, jsx:decode(Result5)), + }, jsx:decode(Result4)), ?assert(filelib:is_file(filename:join([emqx:get_config([node, data_dir]), "certs", "cacert-fake.pem"]))), ?assert(filelib:is_file(filename:join([emqx:get_config([node, data_dir]), "certs", "cert-fake.pem"]))), ?assert(filelib:is_file(filename:join([emqx:get_config([node, data_dir]), "certs", "key-fake.pem"]))), @@ -225,8 +220,8 @@ t_api(_) -> lists:foreach(fun(#{<<"type">> := Type}) -> {ok, 204, _} = request(delete, uri(["authorization", "sources", binary_to_list(Type)]), []) end, Sources), - {ok, 200, Result6} = request(get, uri(["authorization", "sources"]), []), - ?assertEqual([], get_sources(Result6)), + {ok, 200, Result5} = request(get, uri(["authorization", "sources"]), []), + ?assertEqual([], get_sources(Result5)), ok. t_move_source(_) ->