Merge pull request #11797 from sstrigler/EMQX-1057-fix-authz-api-donot-return-200

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

@@ -18,6 +18,7 @@
 
 -behaviour(minirest_api).
 
+-include("emqx_auth_mnesia.hrl").
 -include_lib("emqx_auth/include/emqx_authz.hrl").
 -include_lib("emqx/include/logger.hrl").
 -include_lib("hocon/include/hoconsc.hrl").
@@ -55,6 +56,9 @@
     format_result/1
 ]).
 
+%% minirest filter callback
+-export([is_configured_authz_source/2]).
+
 -define(BAD_REQUEST, 'BAD_REQUEST').
 -define(NOT_FOUND, 'NOT_FOUND').
 -define(ALREADY_EXISTS, 'ALREADY_EXISTS').
@@ -85,6 +89,7 @@ paths() ->
 schema("/authorization/sources/built_in_database/rules/users") ->
     #{
         'operationId' => users,
+        filter => fun ?MODULE:is_configured_authz_source/2,
         get =>
             #{
                 tags => [<<"authorization">>],
@@ -131,6 +136,7 @@ schema("/authorization/sources/built_in_database/rules/users") ->
 schema("/authorization/sources/built_in_database/rules/clients") ->
     #{
         'operationId' => clients,
+        filter => fun ?MODULE:is_configured_authz_source/2,
         get =>
             #{
                 tags => [<<"authorization">>],
@@ -177,6 +183,7 @@ schema("/authorization/sources/built_in_database/rules/clients") ->
 schema("/authorization/sources/built_in_database/rules/users/:username") ->
     #{
         'operationId' => user,
+        filter => fun ?MODULE:is_configured_authz_source/2,
         get =>
             #{
                 tags => [<<"authorization">>],
@@ -230,6 +237,7 @@ schema("/authorization/sources/built_in_database/rules/users/:username") ->
 schema("/authorization/sources/built_in_database/rules/clients/:clientid") ->
     #{
         'operationId' => client,
+        filter => fun ?MODULE:is_configured_authz_source/2,
         get =>
             #{
                 tags => [<<"authorization">>],
@@ -283,6 +291,7 @@ schema("/authorization/sources/built_in_database/rules/clients/:clientid") ->
 schema("/authorization/sources/built_in_database/rules/all") ->
     #{
         'operationId' => all,
+        filter => fun ?MODULE:is_configured_authz_source/2,
         get =>
             #{
                 tags => [<<"authorization">>],
@@ -317,6 +326,7 @@ schema("/authorization/sources/built_in_database/rules/all") ->
 schema("/authorization/sources/built_in_database/rules") ->
     #{
         'operationId' => rules,
+        filter => fun ?MODULE:is_configured_authz_source/2,
         delete =>
             #{
                 tags => [<<"authorization">>],
@@ -426,6 +436,14 @@ fields(rules) ->
 %% HTTP API
 %%--------------------------------------------------------------------
 
+is_configured_authz_source(Params, _Meta) ->
+    emqx_authz_api_sources:with_source(
+        ?AUTHZ_TYPE_BIN,
+        fun(_Source) ->
+            {ok, Params}
+        end
+    ).
+
 users(get, #{query_string := QueryString}) ->
     case
         emqx_mgmt_api:node_query(
@@ -440,7 +458,9 @@ users(get, #{query_string := QueryString}) ->
         {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])),
+            Message = list_to_binary(
+                io_lib:format("bad rpc call ~p, Reason ~p", [Node, Error])
+            ),
             {500, #{code => <<"NODE_DOWN">>, message => Message}};
         Result ->
             {200, Result}
@@ -476,7 +496,9 @@ clients(get, #{query_string := QueryString}) ->
         {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])),
+            Message = list_to_binary(
+                io_lib:format("bad rpc call ~p, Reason ~p", [Node, Error])
+            ),
             {500, #{code => <<"NODE_DOWN">>, message => Message}};
         Result ->
             {200, Result}

apps/emqx_auth_mnesia/test/emqx_authz_api_mnesia_SUITE.erl

@@ -331,4 +331,163 @@ 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;
+            (_Res) ->
+                ct:pal("check failed: ~p", [_Res]),
+                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:with([get, put, post, delete], 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.

dev

@@ -227,8 +227,6 @@ prepare_erl_libs() {
         for app in "_build/${profile}/checkouts"/*; do
             erl_libs="${erl_libs}${sep}${app}"
         done
-    else
-        echo "no checkouts"
     fi
     export ERL_LIBS="$erl_libs"
 }