From 4e0e755b2876361dd7d36c7759a19cc28b320048 Mon Sep 17 00:00:00 2001 From: Stefan Strigler Date: Fri, 20 Oct 2023 12:24:38 +0200 Subject: [PATCH] fix: return 404 if built_in_database not configured as auth source --- .../src/emqx_authz/emqx_authz_api_sources.erl | 2 + .../src/emqx_authz_api_mnesia.erl | 309 ++++++++++-------- .../test/emqx_authz_api_mnesia_SUITE.erl | 155 +++++++++ changes/ce/fix-11797.en.md | 1 + 4 files changed, 337 insertions(+), 130 deletions(-) create mode 100644 changes/ce/fix-11797.en.md diff --git a/apps/emqx_auth/src/emqx_authz/emqx_authz_api_sources.erl b/apps/emqx_auth/src/emqx_authz/emqx_authz_api_sources.erl index 247f3a9ac..00345a108 100644 --- a/apps/emqx_auth/src/emqx_authz/emqx_authz_api_sources.erl +++ b/apps/emqx_auth/src/emqx_authz/emqx_authz_api_sources.erl @@ -49,6 +49,8 @@ aggregate_metrics/1 ]). +-export([with_source/2]). + -define(TAGS, [<<"Authorization">>]). api_spec() -> diff --git a/apps/emqx_auth_mnesia/src/emqx_authz_api_mnesia.erl b/apps/emqx_auth_mnesia/src/emqx_authz_api_mnesia.erl index e71b44add..c0c2322c9 100644 --- a/apps/emqx_auth_mnesia/src/emqx_authz_api_mnesia.erl +++ b/apps/emqx_auth_mnesia/src/emqx_authz_api_mnesia.erl @@ -426,161 +426,210 @@ fields(rules) -> %% HTTP API %%-------------------------------------------------------------------- +-define(IF_CONFIGURED_AUTHZ_SOURCE(EXPR), + emqx_authz_api_sources:with_source( + <<"built_in_database">>, + fun(_Source) -> + EXPR + end + ) +). + users(get, #{query_string := QueryString}) -> - case - emqx_mgmt_api:node_query( - node(), - ?ACL_TABLE, - QueryString, - ?ACL_USERNAME_QSCHEMA, - ?QUERY_USERNAME_FUN, - fun ?MODULE:format_result/1 - ) - of - {error, page_limit_invalid} -> - {400, #{code => <<"INVALID_PARAMETER">>, message => <<"page_limit_invalid">>}}; - {error, Node, Error} -> - Message = list_to_binary(io_lib:format("bad rpc call ~p, Reason ~p", [Node, Error])), - {500, #{code => <<"NODE_DOWN">>, message => Message}}; - Result -> - {200, Result} - end; + ?IF_CONFIGURED_AUTHZ_SOURCE( + case + emqx_mgmt_api:node_query( + node(), + ?ACL_TABLE, + QueryString, + ?ACL_USERNAME_QSCHEMA, + ?QUERY_USERNAME_FUN, + fun ?MODULE:format_result/1 + ) + of + {error, page_limit_invalid} -> + {400, #{code => <<"INVALID_PARAMETER">>, message => <<"page_limit_invalid">>}}; + {error, Node, Error} -> + Message = list_to_binary( + io_lib:format("bad rpc call ~p, Reason ~p", [Node, Error]) + ), + {500, #{code => <<"NODE_DOWN">>, message => Message}}; + Result -> + {200, Result} + end + ); users(post, #{body := Body}) when is_list(Body) -> - case ensure_all_not_exists(<<"username">>, username, Body) of - [] -> - lists:foreach( - fun(#{<<"username">> := Username, <<"rules">> := Rules}) -> - emqx_authz_mnesia:store_rules({username, Username}, Rules) - end, - Body - ), - {204}; - Exists -> - {409, #{ - code => <<"ALREADY_EXISTS">>, - message => binfmt("Users '~ts' already exist", [binjoin(Exists)]) - }} - end. + ?IF_CONFIGURED_AUTHZ_SOURCE( + case ensure_all_not_exists(<<"username">>, username, Body) of + [] -> + lists:foreach( + fun(#{<<"username">> := Username, <<"rules">> := Rules}) -> + emqx_authz_mnesia:store_rules({username, Username}, Rules) + end, + Body + ), + {204}; + Exists -> + {409, #{ + code => <<"ALREADY_EXISTS">>, + message => binfmt("Users '~ts' already exist", [binjoin(Exists)]) + }} + end + ). clients(get, #{query_string := QueryString}) -> - case - emqx_mgmt_api:node_query( - node(), - ?ACL_TABLE, - QueryString, - ?ACL_CLIENTID_QSCHEMA, - ?QUERY_CLIENTID_FUN, - fun ?MODULE:format_result/1 - ) - of - {error, page_limit_invalid} -> - {400, #{code => <<"INVALID_PARAMETER">>, message => <<"page_limit_invalid">>}}; - {error, Node, Error} -> - Message = list_to_binary(io_lib:format("bad rpc call ~p, Reason ~p", [Node, Error])), - {500, #{code => <<"NODE_DOWN">>, message => Message}}; - Result -> - {200, Result} - end; + ?IF_CONFIGURED_AUTHZ_SOURCE( + case + emqx_mgmt_api:node_query( + node(), + ?ACL_TABLE, + QueryString, + ?ACL_CLIENTID_QSCHEMA, + ?QUERY_CLIENTID_FUN, + fun ?MODULE:format_result/1 + ) + of + {error, page_limit_invalid} -> + {400, #{code => <<"INVALID_PARAMETER">>, message => <<"page_limit_invalid">>}}; + {error, Node, Error} -> + Message = list_to_binary( + io_lib:format("bad rpc call ~p, Reason ~p", [Node, Error]) + ), + {500, #{code => <<"NODE_DOWN">>, message => Message}}; + Result -> + {200, Result} + end + ); clients(post, #{body := Body}) when is_list(Body) -> - case ensure_all_not_exists(<<"clientid">>, clientid, Body) of - [] -> - lists:foreach( - fun(#{<<"clientid">> := ClientID, <<"rules">> := Rules}) -> - emqx_authz_mnesia:store_rules({clientid, ClientID}, Rules) - end, - Body - ), - {204}; - Exists -> - {409, #{ - code => <<"ALREADY_EXISTS">>, - message => binfmt("Clients '~ts' already exist", [binjoin(Exists)]) - }} - end. + ?IF_CONFIGURED_AUTHZ_SOURCE( + case ensure_all_not_exists(<<"clientid">>, clientid, Body) of + [] -> + lists:foreach( + fun(#{<<"clientid">> := ClientID, <<"rules">> := Rules}) -> + emqx_authz_mnesia:store_rules({clientid, ClientID}, Rules) + end, + Body + ), + {204}; + Exists -> + {409, #{ + code => <<"ALREADY_EXISTS">>, + message => binfmt("Clients '~ts' already exist", [binjoin(Exists)]) + }} + end + ). user(get, #{bindings := #{username := Username}}) -> - case emqx_authz_mnesia:get_rules({username, Username}) of - not_found -> - {404, #{code => <<"NOT_FOUND">>, message => <<"Not Found">>}}; - {ok, Rules} -> - {200, #{ - username => Username, - rules => format_rules(Rules) - }} - end; + ?IF_CONFIGURED_AUTHZ_SOURCE( + case emqx_authz_mnesia:get_rules({username, Username}) of + not_found -> + {404, #{code => <<"NOT_FOUND">>, message => <<"Not Found">>}}; + {ok, Rules} -> + {200, #{ + username => Username, + rules => format_rules(Rules) + }} + end + ); user(put, #{ bindings := #{username := Username}, body := #{<<"username">> := Username, <<"rules">> := Rules} }) -> - emqx_authz_mnesia:store_rules({username, Username}, Rules), - {204}; -user(delete, #{bindings := #{username := Username}}) -> - case emqx_authz_mnesia:get_rules({username, Username}) of - not_found -> - {404, #{code => <<"NOT_FOUND">>, message => <<"Username Not Found">>}}; - {ok, _Rules} -> - emqx_authz_mnesia:delete_rules({username, Username}), + ?IF_CONFIGURED_AUTHZ_SOURCE( + begin + emqx_authz_mnesia:store_rules({username, Username}, Rules), {204} - end. + end + ); +user(delete, #{bindings := #{username := Username}}) -> + ?IF_CONFIGURED_AUTHZ_SOURCE( + case emqx_authz_mnesia:get_rules({username, Username}) of + not_found -> + {404, #{code => <<"NOT_FOUND">>, message => <<"Username Not Found">>}}; + {ok, _Rules} -> + emqx_authz_mnesia:delete_rules({username, Username}), + {204} + end + ). client(get, #{bindings := #{clientid := ClientID}}) -> - case emqx_authz_mnesia:get_rules({clientid, ClientID}) of - not_found -> - {404, #{code => <<"NOT_FOUND">>, message => <<"Not Found">>}}; - {ok, Rules} -> - {200, #{ - clientid => ClientID, - rules => format_rules(Rules) - }} - end; + ?IF_CONFIGURED_AUTHZ_SOURCE( + case emqx_authz_mnesia:get_rules({clientid, ClientID}) of + not_found -> + {404, #{code => <<"NOT_FOUND">>, message => <<"Not Found">>}}; + {ok, Rules} -> + {200, #{ + clientid => ClientID, + rules => format_rules(Rules) + }} + end + ); client(put, #{ bindings := #{clientid := ClientID}, body := #{<<"clientid">> := ClientID, <<"rules">> := Rules} }) -> - emqx_authz_mnesia:store_rules({clientid, ClientID}, Rules), - {204}; -client(delete, #{bindings := #{clientid := ClientID}}) -> - case emqx_authz_mnesia:get_rules({clientid, ClientID}) of - not_found -> - {404, #{code => <<"NOT_FOUND">>, message => <<"ClientID Not Found">>}}; - {ok, _Rules} -> - emqx_authz_mnesia:delete_rules({clientid, ClientID}), + ?IF_CONFIGURED_AUTHZ_SOURCE( + begin + emqx_authz_mnesia:store_rules({clientid, ClientID}, Rules), {204} - end. + end + ); +client(delete, #{bindings := #{clientid := ClientID}}) -> + ?IF_CONFIGURED_AUTHZ_SOURCE( + case emqx_authz_mnesia:get_rules({clientid, ClientID}) of + not_found -> + {404, #{code => <<"NOT_FOUND">>, message => <<"ClientID Not Found">>}}; + {ok, _Rules} -> + emqx_authz_mnesia:delete_rules({clientid, ClientID}), + {204} + end + ). all(get, _) -> - case emqx_authz_mnesia:get_rules(all) of - not_found -> - {200, #{rules => []}}; - {ok, Rules} -> - {200, #{ - rules => format_rules(Rules) - }} - end; + ?IF_CONFIGURED_AUTHZ_SOURCE( + case emqx_authz_mnesia:get_rules(all) of + not_found -> + {200, #{rules => []}}; + {ok, Rules} -> + {200, #{ + rules => format_rules(Rules) + }} + end + ); all(post, #{body := #{<<"rules">> := Rules}}) -> - emqx_authz_mnesia:store_rules(all, Rules), - {204}; + ?IF_CONFIGURED_AUTHZ_SOURCE( + begin + emqx_authz_mnesia:store_rules(all, Rules), + {204} + end + ); all(delete, _) -> - emqx_authz_mnesia:store_rules(all, []), - {204}. + ?IF_CONFIGURED_AUTHZ_SOURCE( + begin + emqx_authz_mnesia:store_rules(all, []), + {204} + end + ). rules(delete, _) -> - case emqx_authz_api_sources:get_raw_source(<<"built_in_database">>) of - [#{<<"enable">> := false}] -> - ok = emqx_authz_mnesia:purge_rules(), - {204}; - [#{<<"enable">> := true}] -> - {400, #{ - code => <<"BAD_REQUEST">>, - 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. + ?IF_CONFIGURED_AUTHZ_SOURCE( + case emqx_authz_api_sources:get_raw_source(<<"built_in_database">>) of + [#{<<"enable">> := false}] -> + ok = emqx_authz_mnesia:purge_rules(), + {204}; + [#{<<"enable">> := true}] -> + {400, #{ + code => <<"BAD_REQUEST">>, + 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 + ). %%-------------------------------------------------------------------- %% QueryString to MatchSpec diff --git a/apps/emqx_auth_mnesia/test/emqx_authz_api_mnesia_SUITE.erl b/apps/emqx_auth_mnesia/test/emqx_authz_api_mnesia_SUITE.erl index e4b96b08b..50da6e676 100644 --- a/apps/emqx_auth_mnesia/test/emqx_authz_api_mnesia_SUITE.erl +++ b/apps/emqx_auth_mnesia/test/emqx_authz_api_mnesia_SUITE.erl @@ -331,4 +331,159 @@ t_api(_) -> [] ), ?assertEqual(0, emqx_authz_mnesia:record_count()), + + Examples = make_examples(emqx_authz_api_mnesia), + ?assertEqual( + 14, + length(Examples) + ), + + Fixtures1 = fun() -> + {ok, _, _} = + request( + delete, + uri(["authorization", "sources", "built_in_database", "rules", "all"]), + [] + ), + {ok, _, _} = + request( + delete, + uri(["authorization", "sources", "built_in_database", "rules", "users"]), + [] + ), + {ok, _, _} = + request( + delete, + uri(["authorization", "sources", "built_in_database", "rules", "clients"]), + [] + ) + end, + run_examples(Examples, Fixtures1), + + Fixtures2 = fun() -> + %% disable/remove built_in_database + {ok, 204, _} = + request( + delete, + uri(["authorization", "sources", "built_in_database"]), + [] + ) + end, + + run_examples(404, Examples, Fixtures2), + ok. + +%% test helpers +-define(REPLACEMENTS, #{ + ":clientid" => <<"client1">>, + ":username" => <<"user1">> +}). + +run_examples(Examples) -> + %% assume all ok + run_examples( + fun + ({ok, Code, _}) when + Code >= 200, + Code =< 299 + -> + true; + (_Res) -> + ct:pal("check failed: ~p", [_Res]), + false + end, + Examples + ). + +run_examples(Examples, Fixtures) when is_function(Fixtures) -> + Fixtures(), + run_examples(Examples); +run_examples(Check, Examples) when is_function(Check) -> + lists:foreach( + fun({Path, Op, Body} = _Req) -> + ct:pal("req: ~p", [_Req]), + ?assert( + Check( + request(Op, uri(Path), Body) + ) + ) + end, + Examples + ); +run_examples(Code, Examples) when is_number(Code) -> + run_examples( + fun + ({ok, ResCode, _}) when Code =:= ResCode -> true; + (_) -> false + end, + Examples + ). + +run_examples(CodeOrCheck, Examples, Fixtures) when is_function(Fixtures) -> + Fixtures(), + run_examples(CodeOrCheck, Examples). + +make_examples(ApiMod) -> + make_examples(ApiMod, ?REPLACEMENTS). + +-spec make_examples(Mod :: atom()) -> [{Path :: list(), [{Op :: atom(), Body :: term()}]}]. +make_examples(ApiMod, Replacements) -> + Paths = ApiMod:paths(), + lists:flatten( + lists:map( + fun(Path) -> + Schema = ApiMod:schema(Path), + lists:map( + fun({Op, OpSchema}) -> + Body = + case maps:get('requestBody', OpSchema, undefined) of + undefined -> + []; + HoconWithExamples -> + maps:get( + value, + hd( + maps:values( + maps:get( + <<"examples">>, + maps:get(examples, HoconWithExamples) + ) + ) + ) + ) + end, + {replace_parts(to_parts(Path), Replacements), Op, Body} + end, + lists:sort(fun op_sort/2, maps:to_list(maps:remove('operationId', Schema))) + ) + end, + Paths + ) + ). + +op_sort({post, _}, {_, _}) -> + true; +op_sort({put, _}, {_, _}) -> + true; +op_sort({get, _}, {delete, _}) -> + true; +op_sort(_, _) -> + false. + +to_parts(Path) -> + string:tokens(Path, "/"). + +replace_parts(Parts, Replacements) -> + lists:map( + fun(Part) -> + %% that's the fun part + case maps:is_key(Part, Replacements) of + true -> + maps:get(Part, Replacements); + false -> + Part + end + end, + Parts + ). diff --git a/changes/ce/fix-11797.en.md b/changes/ce/fix-11797.en.md new file mode 100644 index 000000000..6227e079c --- /dev/null +++ b/changes/ce/fix-11797.en.md @@ -0,0 +1 @@ +Modified HTTP API behavior for APIs managing the `built_in_database` authorization source: They will now return a `404` status code if `built_in_database` is not set as the authorization source, replacing the former `20X` response.