fix: return 404 if built_in_database not configured as auth source

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() ->

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

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
+    ).

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.