From 9c414096c70a30d57f63e858f153ef6bc47765d5 Mon Sep 17 00:00:00 2001 From: Zaiming Shi Date: Tue, 19 Oct 2021 17:30:34 +0200 Subject: [PATCH] test(authz): test HTTP apis for built-in-database --- apps/emqx_authz/include/emqx_authz.hrl | 2 +- apps/emqx_authz/src/emqx_authz.erl | 12 ++--- apps/emqx_authz/src/emqx_authz_api_mnesia.erl | 10 +++-- .../emqx_authz/src/emqx_authz_api_sources.erl | 16 +++---- apps/emqx_authz/src/emqx_authz_schema.erl | 25 ++++++++++- apps/emqx_authz/test/emqx_authz_SUITE.erl | 22 +++++----- .../test/emqx_authz_api_mnesia_SUITE.erl | 44 ++++++++++--------- apps/emqx_machine/src/emqx_machine_schema.erl | 2 +- 8 files changed, 83 insertions(+), 50 deletions(-) diff --git a/apps/emqx_authz/include/emqx_authz.hrl b/apps/emqx_authz/include/emqx_authz.hrl index d86f08888..fe4514614 100644 --- a/apps/emqx_authz/include/emqx_authz.hrl +++ b/apps/emqx_authz/include/emqx_authz.hrl @@ -49,7 +49,7 @@ ignore = 'client.authorize.ignore' }). --define(CMD_REPLCAE, replace). +-define(CMD_REPLACE, replace). -define(CMD_DELETE, delete). -define(CMD_PREPEND, prepend). -define(CMD_APPEND, append). diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index 44d4184de..d8aa4e859 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/apps/emqx_authz/src/emqx_authz.erl @@ -73,8 +73,8 @@ move(Type, Position, Opts) -> update(Cmd, Sources) -> update(Cmd, Sources, #{}). -update({?CMD_REPLCAE, Type}, Sources, Opts) -> - emqx:update_config(?CONF_KEY_PATH, {{?CMD_REPLCAE, type(Type)}, Sources}, Opts); +update({?CMD_REPLACE, Type}, Sources, Opts) -> + emqx:update_config(?CONF_KEY_PATH, {{?CMD_REPLACE, type(Type)}, Sources}, Opts); update({?CMD_DELETE, Type}, Sources, Opts) -> emqx:update_config(?CONF_KEY_PATH, {{?CMD_DELETE, type(Type)}, Sources}, Opts); update(Cmd, Sources, Opts) -> @@ -102,7 +102,7 @@ do_update({?CMD_APPEND, Sources}, Conf) when is_list(Sources), is_list(Conf) -> NConf = Conf ++ Sources, ok = check_dup_types(NConf), NConf; -do_update({{?CMD_REPLCAE, Type}, Source}, Conf) when is_map(Source), is_list(Conf) -> +do_update({{?CMD_REPLACE, Type}, Source}, Conf) when is_map(Source), is_list(Conf) -> {_Old, Front, Rear} = take(Type, Conf), NConf = Front ++ [Source | Rear], ok = check_dup_types(NConf), @@ -113,7 +113,9 @@ do_update({{?CMD_DELETE, Type}, _Source}, Conf) when is_list(Conf) -> NConf; do_update({_, Sources}, _Conf) when is_list(Sources)-> %% overwrite the entire config! - Sources. + Sources; +do_update({Op, Sources}, Conf) -> + error({bad_request, #{op => Op, sources => Sources, conf => Conf}}). pre_config_update(Cmd, Conf) -> {ok, do_update(Cmd, Conf)}. @@ -138,7 +140,7 @@ do_post_update({?CMD_APPEND, Sources}, _NewSources) -> InitedSources = init_sources(check_sources(Sources)), emqx_hooks:put('client.authorize', {?MODULE, authorize, [lookup() ++ InitedSources]}, -1), ok = emqx_authz_cache:drain_cache(); -do_post_update({{?CMD_REPLCAE, Type}, Source}, _NewSources) when is_map(Source) -> +do_post_update({{?CMD_REPLACE, Type}, Source}, _NewSources) when is_map(Source) -> OldInitedSources = lookup(), {OldSource, Front, Rear} = take(Type, OldInitedSources), ok = ensure_resource_deleted(OldSource), diff --git a/apps/emqx_authz/src/emqx_authz_api_mnesia.erl b/apps/emqx_authz/src/emqx_authz_api_mnesia.erl index c29f90f43..a6bd0f7b8 100644 --- a/apps/emqx_authz/src/emqx_authz_api_mnesia.erl +++ b/apps/emqx_authz/src/emqx_authz_api_mnesia.erl @@ -632,14 +632,18 @@ all(put, #{body := #{<<"rules">> := Rules}}) -> purge(delete, _) -> case emqx_authz_api_sources:get_raw_source(<<"built-in-database">>) of - [#{enable := false}] -> + [#{<<"enable">> := false}] -> ok = lists:foreach(fun(Key) -> ok = ekka_mnesia:dirty_delete(?ACL_TABLE, Key) end, mnesia:dirty_all_keys(?ACL_TABLE)), {204}; - _ -> + [#{<<"enable">> := true}] -> {400, #{code => <<"BAD_REQUEST">>, - message => <<"'built-in-database' type source must be disabled before purge.">>}} + message => <<"'built-in-database' type source must be disabled before purge.">>}}; + [] -> + {404, #{code => <<"BAD_REQUEST">>, + message => <<"'built-in-database' type source is not found.">> + }} end. format_rules(Rules) when is_list(Rules) -> diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index 784519d54..9c703209a 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -347,17 +347,17 @@ sources(post, #{body := #{<<"type">> := <<"file">>, <<"rules">> := Rules}}) -> {ok, Filename} = write_file(filename:join([emqx:get_config([node, data_dir]), "acl.conf"]), Rules), update_config(?CMD_PREPEND, [#{<<"type">> => <<"file">>, <<"enable">> => true, <<"path">> => Filename}]); sources(post, #{body := Body}) when is_map(Body) -> - update_config(?CMD_PREPEND, [write_cert(Body)]); + update_config(?CMD_PREPEND, [maybe_write_certs(Body)]); sources(put, #{body := Body}) when is_list(Body) -> NBody = [ begin case Source of #{<<"type">> := <<"file">>, <<"rules">> := Rules, <<"enable">> := Enable} -> {ok, Filename} = write_file(filename:join([emqx:get_config([node, data_dir]), "acl.conf"]), Rules), #{<<"type">> => <<"file">>, <<"enable">> => Enable, <<"path">> => Filename}; - _ -> write_cert(Source) + _ -> maybe_write_certs(Source) end end || Source <- Body], - update_config(?CMD_REPLCAE, NBody). + update_config(?CMD_REPLACE, NBody). source(get, #{bindings := #{type := Type}}) -> case get_raw_source(Type) of @@ -379,14 +379,14 @@ source(get, #{bindings := #{type := Type}}) -> end; source(put, #{bindings := #{type := <<"file">>}, body := #{<<"type">> := <<"file">>, <<"rules">> := Rules, <<"enable">> := Enable}}) -> {ok, Filename} = write_file(maps:get(path, emqx_authz:lookup(file), ""), Rules), - case emqx_authz:update({?CMD_REPLCAE, <<"file">>}, #{<<"type">> => <<"file">>, <<"enable">> => Enable, <<"path">> => Filename}) of + case emqx_authz:update({?CMD_REPLACE, <<"file">>}, #{<<"type">> => <<"file">>, <<"enable">> => Enable, <<"path">> => Filename}) of {ok, _} -> {204}; {error, Reason} -> {400, #{code => <<"BAD_REQUEST">>, message => bin(Reason)}} end; source(put, #{bindings := #{type := Type}, body := Body}) when is_map(Body) -> - update_config({?CMD_REPLCAE, Type}, write_cert(Body)); + update_config({?CMD_REPLACE, Type}, maybe_write_certs(Body#{<<"type">> => Type})); source(delete, #{bindings := #{type := Type}}) -> update_config({?CMD_DELETE, Type}, #{}). @@ -402,7 +402,7 @@ move_source(post, #{bindings := #{type := Type}, body := #{<<"position">> := Pos end. get_raw_sources() -> - RawSources = emqx:get_raw_config([authorization, sources]), + RawSources = emqx:get_raw_config([authorization, sources], []), Schema = #{roots => emqx_authz_schema:fields("authorization"), fields => #{}}, Conf = #{<<"sources">> => RawSources}, #{<<"sources">> := Sources} = hocon_schema:check_plain(Schema, Conf, #{only_fill_defaults => true}), @@ -447,7 +447,7 @@ read_cert(#{<<"ssl">> := #{<<"enable">> := true} = SSL} = Source) -> }; read_cert(Source) -> Source. -write_cert(#{<<"ssl">> := #{<<"enable">> := true} = SSL} = Source) -> +maybe_write_certs(#{<<"ssl">> := #{<<"enable">> := true} = SSL} = Source) -> CertPath = filename:join([emqx:get_config([node, data_dir]), "certs"]), CaCert = case maps:is_key(<<"cacertfile">>, SSL) of true -> @@ -475,7 +475,7 @@ write_cert(#{<<"ssl">> := #{<<"enable">> := true} = SSL} = Source) -> <<"keyfile">> => Key } }; -write_cert(Source) -> Source. +maybe_write_certs(Source) -> Source. write_file(Filename, Bytes0) -> ok = filelib:ensure_dir(Filename), diff --git a/apps/emqx_authz/src/emqx_authz_schema.erl b/apps/emqx_authz/src/emqx_authz_schema.erl index 711863628..66171c542 100644 --- a/apps/emqx_authz/src/emqx_authz_schema.erl +++ b/apps/emqx_authz/src/emqx_authz_schema.erl @@ -40,7 +40,30 @@ fields("authorization") -> , hoconsc:ref(?MODULE, redis_single) , hoconsc:ref(?MODULE, redis_sentinel) , hoconsc:ref(?MODULE, redis_cluster) - ])} + ]), + default => [], + desc => +""" +Authorization data sources.
+An array of authorization (ACL) data providers. +It is designed as an array but not a hash-map so the sources can be +ordered to form a chain of access controls.
+ + +When authorizing a publish or subscribe action, the configured +sources are checked in order. When checking an ACL source, +in case the client (identified by username or client ID) is not found, +it moves on to the next source. And it stops immediatly +once an 'allow' or 'deny' decision is returned.
+ +If the client is not found in any of the sources, +the default action configured in 'authorization.no_match' is applied.
+ +NOTE: +The source elements are identified by their 'type'. +It is NOT allowed to configure two or more sources of the same type. +""" + } } ]; fields(file) -> diff --git a/apps/emqx_authz/test/emqx_authz_SUITE.erl b/apps/emqx_authz/test/emqx_authz_SUITE.erl index d91de68a0..fd22bbc7a 100644 --- a/apps/emqx_authz/test/emqx_authz_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_SUITE.erl @@ -50,14 +50,14 @@ init_per_suite(Config) -> Config. end_per_suite(_Config) -> - {ok, _} = emqx_authz:update(?CMD_REPLCAE, []), + {ok, _} = emqx_authz:update(?CMD_REPLACE, []), emqx_common_test_helpers:stop_apps([emqx_authz, emqx_resource]), meck:unload(emqx_resource), meck:unload(emqx_schema), ok. init_per_testcase(_, Config) -> - {ok, _} = emqx_authz:update(?CMD_REPLCAE, []), + {ok, _} = emqx_authz:update(?CMD_REPLACE, []), Config. -define(SOURCE1, #{<<"type">> => <<"http">>, @@ -120,7 +120,7 @@ init_per_testcase(_, Config) -> %%------------------------------------------------------------------------------ t_update_source(_) -> - {ok, _} = emqx_authz:update(?CMD_REPLCAE, [?SOURCE3]), + {ok, _} = emqx_authz:update(?CMD_REPLACE, [?SOURCE3]), {ok, _} = emqx_authz:update(?CMD_PREPEND, [?SOURCE2]), {ok, _} = emqx_authz:update(?CMD_PREPEND, [?SOURCE1]), {ok, _} = emqx_authz:update(?CMD_APPEND, [?SOURCE4]), @@ -135,12 +135,12 @@ t_update_source(_) -> , #{type := file, enable := true} ], emqx:get_config([authorization, sources], [])), - {ok, _} = emqx_authz:update({?CMD_REPLCAE, http}, ?SOURCE1#{<<"enable">> := false}), - {ok, _} = emqx_authz:update({?CMD_REPLCAE, mongodb}, ?SOURCE2#{<<"enable">> := false}), - {ok, _} = emqx_authz:update({?CMD_REPLCAE, mysql}, ?SOURCE3#{<<"enable">> := false}), - {ok, _} = emqx_authz:update({?CMD_REPLCAE, postgresql}, ?SOURCE4#{<<"enable">> := false}), - {ok, _} = emqx_authz:update({?CMD_REPLCAE, redis}, ?SOURCE5#{<<"enable">> := false}), - {ok, _} = emqx_authz:update({?CMD_REPLCAE, file}, ?SOURCE6#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLACE, http}, ?SOURCE1#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLACE, mongodb}, ?SOURCE2#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLACE, mysql}, ?SOURCE3#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLACE, postgresql}, ?SOURCE4#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLACE, redis}, ?SOURCE5#{<<"enable">> := false}), + {ok, _} = emqx_authz:update({?CMD_REPLACE, file}, ?SOURCE6#{<<"enable">> := false}), ?assertMatch([ #{type := http, enable := false} , #{type := mongodb, enable := false} @@ -150,10 +150,10 @@ t_update_source(_) -> , #{type := file, enable := false} ], emqx:get_config([authorization, sources], [])), - {ok, _} = emqx_authz:update(?CMD_REPLCAE, []). + {ok, _} = emqx_authz:update(?CMD_REPLACE, []). t_move_source(_) -> - {ok, _} = emqx_authz:update(?CMD_REPLCAE, [?SOURCE1, ?SOURCE2, ?SOURCE3, ?SOURCE4, ?SOURCE5, ?SOURCE6]), + {ok, _} = emqx_authz:update(?CMD_REPLACE, [?SOURCE1, ?SOURCE2, ?SOURCE3, ?SOURCE4, ?SOURCE5, ?SOURCE6]), ?assertMatch([ #{type := http} , #{type := mongodb} , #{type := mysql} diff --git a/apps/emqx_authz/test/emqx_authz_api_mnesia_SUITE.erl b/apps/emqx_authz/test/emqx_authz_api_mnesia_SUITE.erl index a56d70fae..a49982651 100644 --- a/apps/emqx_authz/test/emqx_authz_api_mnesia_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_api_mnesia_SUITE.erl @@ -22,7 +22,14 @@ -include_lib("eunit/include/eunit.hrl"). -include_lib("common_test/include/ct.hrl"). --define(CONF_DEFAULT, <<"authorization: {sources: []}">>). +-define(CONF_DEFAULT, <<""" +authorization + {sources = [ + { type = \"built-in-database\" + enable = true + } + ]} +""">>). -define(HOST, "http://127.0.0.1:18083/"). -define(API_VERSION, "v5"). @@ -73,6 +80,12 @@ ] }). +roots() -> ["authorization"]. + +fields("authorization") -> + emqx_authz_schema:fields("authorization") ++ + emqx_schema:fields("authorization"). + all() -> emqx_common_test_helpers:all(?MODULE). @@ -80,25 +93,13 @@ groups() -> []. init_per_suite(Config) -> - meck:new(emqx_schema, [non_strict, passthrough, no_history, no_link]), - meck:expect(emqx_schema, fields, fun("authorization") -> - meck:passthrough(["authorization"]) ++ - emqx_authz_schema:fields("authorization"); - (F) -> meck:passthrough([F]) - end), - - ok = emqx_config:init_load(emqx_authz_schema, ?CONF_DEFAULT), - - ok = emqx_common_test_helpers:start_apps([emqx_authz, emqx_dashboard], fun set_special_configs/1), - {ok, _} = emqx:update_config([authorization, cache, enable], false), - {ok, _} = emqx:update_config([authorization, no_match], deny), - + ok = emqx_common_test_helpers:start_apps([emqx_authz, emqx_dashboard], + fun set_special_configs/1), Config. end_per_suite(_Config) -> {ok, _} = emqx_authz:update(replace, []), emqx_common_test_helpers:stop_apps([emqx_authz, emqx_dashboard]), - meck:unload(emqx_schema), ok. set_special_configs(emqx_dashboard) -> @@ -113,9 +114,9 @@ set_special_configs(emqx_dashboard) -> emqx_config:put([emqx_dashboard], Config), ok; set_special_configs(emqx_authz) -> - emqx_config:put([authorization], #{sources => [#{type => 'built-in-database', - enable => true} - ]}), + ok = emqx_config:init_load(?MODULE, ?CONF_DEFAULT), + {ok, _} = emqx:update_config([authorization, cache, enable], false), + {ok, _} = emqx:update_config([authorization, no_match], deny), ok; set_special_configs(_App) -> ok. @@ -174,11 +175,14 @@ t_api(_) -> {ok, 200, Request10} = request(get, uri(["authorization", "sources", "built-in-database", "clientid?limit=5"]), []), ?assertEqual(5, length(jsx:decode(Request10))), - {ok, 400, _} = request(delete, uri(["authorization", "sources", "built-in-database", "purge-all"]), []), + {ok, 400, Msg1} = request(delete, uri(["authorization", "sources", "built-in-database", "purge-all"]), []), + ?assertMatch({match, _}, re:run(Msg1, "must\sbe\sdisabled\sbefore")), + {ok, 204, _} = request(put, uri(["authorization", "sources", "built-in-database"]), #{<<"enable">> => true}), + %% test idempotence + {ok, 204, _} = request(put, uri(["authorization", "sources", "built-in-database"]), #{<<"enable">> => true}), {ok, 204, _} = request(put, uri(["authorization", "sources", "built-in-database"]), #{<<"enable">> => false}), {ok, 204, _} = request(delete, uri(["authorization", "sources", "built-in-database", "purge-all"]), []), ?assertEqual([], mnesia:dirty_all_keys(?ACL_TABLE)), - ok. %%-------------------------------------------------------------------- diff --git a/apps/emqx_machine/src/emqx_machine_schema.erl b/apps/emqx_machine/src/emqx_machine_schema.erl index 08bbeb449..995de1f62 100644 --- a/apps/emqx_machine/src/emqx_machine_schema.erl +++ b/apps/emqx_machine/src/emqx_machine_schema.erl @@ -91,7 +91,7 @@ roots() -> #{ desc => """ Authorization a.k.a ACL.
In EMQ X, MQTT client access control is extremly flexible.
-A an out of the box set of authorization data sources are supported. +An out of the box set of authorization data sources are supported. For example,
'file' source is to support concise and yet generic ACL rules in a file;
'built-in-database' source can be used to store per-client customisable rule sets,