Merge pull request #13584 from savonarola/0802-fix-http-authz-58

fix(http_authz): fix creation with empty headers, fix empty header format

apps/emqx_auth/src/emqx_authn/emqx_authn_api.erl

@@ -1213,9 +1213,9 @@ merge_default_headers(Config) ->
             NewHeaders =
                 case Config of
                     #{<<"method">> := <<"get">>} ->
-                        emqx_authn_utils:convert_headers_no_content_type(Headers);
+                        emqx_auth_http_utils:convert_headers_no_content_type(Headers);
                     #{<<"method">> := <<"post">>} ->
-                        emqx_authn_utils:convert_headers(Headers);
+                        emqx_auth_http_utils:convert_headers(Headers);
                     _ ->
                         Headers
                 end,

apps/emqx_auth/src/emqx_authn/emqx_authn_utils.erl

@@ -34,11 +34,7 @@
     cleanup_resources/0,
     make_resource_id/1,
     without_password/1,
-    to_bool/1,
-    convert_headers/1,
-    convert_headers_no_content_type/1,
-    default_headers/0,
-    default_headers_no_content_type/0
+    to_bool/1
 ]).
 
 -define(DEFAULT_RESOURCE_OPTS, #{
@@ -184,30 +180,6 @@ to_bool(MaybeBinInt) when is_binary(MaybeBinInt) ->
 to_bool(_) ->
     false.
 
-convert_headers(Headers) ->
-    transform_header_name(Headers).
-
-convert_headers_no_content_type(Headers) ->
-    maps:without(
-        [<<"content-type">>],
-        transform_header_name(Headers)
-    ).
-
-default_headers() ->
-    maps:put(
-        <<"content-type">>,
-        <<"application/json">>,
-        default_headers_no_content_type()
-    ).
-
-default_headers_no_content_type() ->
-    #{
-        <<"accept">> => <<"application/json">>,
-        <<"cache-control">> => <<"no-cache">>,
-        <<"connection">> => <<"keep-alive">>,
-        <<"keep-alive">> => <<"timeout=30, max=1000">>
-    }.
-
 %%--------------------------------------------------------------------
 %% Internal functions
 %%--------------------------------------------------------------------
@@ -221,20 +193,3 @@ without_password(Credential, [Name | Rest]) ->
         false ->
             without_password(Credential, Rest)
     end.
-
-transform_header_name(Headers) ->
-    maps:fold(
-        fun(K0, V, Acc) ->
-            K = list_to_binary(string:to_lower(to_list(K0))),
-            maps:put(K, V, Acc)
-        end,
-        #{},
-        Headers
-    ).
-
-to_list(A) when is_atom(A) ->
-    atom_to_list(A);
-to_list(B) when is_binary(B) ->
-    binary_to_list(B);
-to_list(L) when is_list(L) ->
-    L.

apps/emqx_auth/src/emqx_authz/emqx_authz.erl

@@ -32,7 +32,7 @@
     register_metrics/0,
     init/0,
     deinit/0,
-    merge_defaults/1,
+    format_for_api/1,
     lookup/0,
     lookup/1,
     move/2,
@@ -701,11 +701,11 @@ type(#{<<"type">> := Type}) ->
 type(Type) when is_atom(Type) orelse is_binary(Type) ->
     emqx_authz_source_registry:get(Type).
 
-merge_defaults(Source) ->
+format_for_api(Source) ->
     Type = type(Source),
     Mod = authz_module(Type),
     try
-        Mod:merge_defaults(Source)
+        Mod:format_for_api(Source)
     catch
         error:undef ->
             Source

apps/emqx_auth/src/emqx_authz/emqx_authz_api_sources.erl

@@ -526,10 +526,10 @@ get_raw_sources() ->
     Schema = emqx_hocon:make_schema(emqx_authz_schema:authz_fields()),
     Conf = #{<<"sources">> => RawSources},
     #{<<"sources">> := Sources} = hocon_tconf:make_serializable(Schema, Conf, #{}),
-    merge_defaults(Sources).
+    format_for_api(Sources).
 
-merge_defaults(Sources) ->
-    lists:map(fun emqx_authz:merge_defaults/1, Sources).
+format_for_api(Sources) ->
+    lists:map(fun emqx_authz:format_for_api/1, Sources).
 
 get_raw_source(Type) ->
     lists:filter(

apps/emqx_auth/src/emqx_authz/emqx_authz_source.erl

@@ -59,11 +59,11 @@
 -callback read_files(raw_source()) -> raw_source() | no_return().
 
 %% Merge default values to the source, for example, for exposing via API
--callback merge_defaults(raw_source()) -> raw_source().
+-callback format_for_api(raw_source()) -> raw_source().
 
 -optional_callbacks([
     update/1,
     write_files/1,
     read_files/1,
-    merge_defaults/1
+    format_for_api/1
 ]).

apps/emqx_auth/test/emqx_authz/emqx_authz_api_source_http_SUITE.erl

@@ -0,0 +1,149 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2020-2024 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+
+-module(emqx_authz_api_source_http_SUITE).
+
+-compile(nowarn_export_all).
+-compile(export_all).
+
+-import(emqx_mgmt_api_test_util, [request/3, uri/1]).
+
+-include_lib("eunit/include/eunit.hrl").
+-include_lib("common_test/include/ct.hrl").
+-include_lib("emqx/include/emqx_placeholder.hrl").
+
+-define(SOURCE_HTTP, #{
+    <<"type">> => <<"http">>,
+    <<"enable">> => true,
+    <<"url">> => <<"https://fake.com:443/acl?username=", ?PH_USERNAME/binary>>,
+    <<"ssl">> => #{<<"enable">> => true},
+    <<"headers">> => #{},
+    <<"method">> => <<"get">>,
+    <<"request_timeout">> => <<"5s">>
+}).
+
+all() ->
+    emqx_common_test_helpers:all(?MODULE).
+
+groups() ->
+    [].
+
+init_per_suite(Config) ->
+    meck:new(emqx_resource, [non_strict, passthrough, no_history, no_link]),
+    meck:expect(emqx_resource, create_local, fun(_, _, _, _) -> {ok, meck_data} end),
+    meck:expect(emqx_resource, health_check, fun(St) -> {ok, St} end),
+    meck:expect(emqx_resource, remove_local, fun(_) -> ok end),
+    meck:expect(
+        emqx_authz_file,
+        acl_conf_file,
+        fun() ->
+            emqx_common_test_helpers:deps_path(emqx_auth, "etc/acl.conf")
+        end
+    ),
+
+    Apps = emqx_cth_suite:start(
+        [
+            emqx,
+            {emqx_conf,
+                "authorization { cache { enable = false }, no_match = deny, sources = [] }"},
+            emqx_auth,
+            emqx_auth_http,
+            emqx_management,
+            emqx_mgmt_api_test_util:emqx_dashboard()
+        ],
+        #{
+            work_dir => emqx_cth_suite:work_dir(Config)
+        }
+    ),
+    [{suite_apps, Apps} | Config].
+
+end_per_suite(Config) ->
+    emqx_cth_suite:stop(?config(suite_apps, Config)),
+    meck:unload(emqx_resource),
+    ok.
+
+init_per_testcase(t_api, Config) ->
+    meck:new(emqx_utils, [non_strict, passthrough, no_history, no_link]),
+    meck:expect(emqx_utils, gen_id, fun() -> "fake" end),
+
+    meck:new(emqx, [non_strict, passthrough, no_history, no_link]),
+    meck:expect(
+        emqx,
+        data_dir,
+        fun() ->
+            {data_dir, Data} = lists:keyfind(data_dir, 1, Config),
+            Data
+        end
+    ),
+    Config;
+init_per_testcase(_, Config) ->
+    Config.
+
+end_per_testcase(t_api, _Config) ->
+    meck:unload(emqx_utils),
+    meck:unload(emqx),
+    ok;
+end_per_testcase(_, _Config) ->
+    ok.
+
+%%------------------------------------------------------------------------------
+%% Testcases
+%%------------------------------------------------------------------------------
+
+t_http_headers_api(_) ->
+    {ok, 204, _} = request(post, uri(["authorization", "sources"]), ?SOURCE_HTTP),
+
+    {ok, 200, Result1} = request(get, uri(["authorization", "sources", "http"]), []),
+    ?assertMatch(
+        #{
+            <<"type">> := <<"http">>,
+            <<"headers">> := M
+        } when map_size(M) =:= 0,
+        emqx_utils_json:decode(Result1, [return_maps])
+    ),
+
+    {ok, 204, _} = request(put, uri(["authorization", "sources", "http"]), ?SOURCE_HTTP#{
+        <<"headers">> => #{<<"a">> => <<"b">>}
+    }),
+
+    {ok, 200, Result2} = request(get, uri(["authorization", "sources", "http"]), []),
+    ?assertMatch(
+        #{
+            <<"type">> := <<"http">>,
+            <<"headers">> := #{<<"a">> := <<"b">>}
+        },
+        emqx_utils_json:decode(Result2, [return_maps])
+    ),
+
+    {ok, 204, _} = request(put, uri(["authorization", "sources", "http"]), ?SOURCE_HTTP#{
+        <<"headers">> => #{}
+    }),
+
+    {ok, 200, Result4} = request(get, uri(["authorization", "sources", "http"]), []),
+    ?assertMatch(
+        #{
+            <<"type">> := <<"http">>,
+            <<"headers">> := M
+        } when map_size(M) =:= 0,
+        emqx_utils_json:decode(Result4, [return_maps])
+    ).
+
+data_dir() -> emqx:data_dir().
+
+start_apps(Apps) ->
+    lists:foreach(fun application:ensure_all_started/1, Apps).
+
+stop_apps(Apps) ->
+    lists:foreach(fun application:stop/1, Apps).

apps/emqx_auth_http/src/emqx_auth_http_utils.erl

@@ -0,0 +1,66 @@
+%%--------------------------------------------------------------------
+%% Copyright (c) 2024 EMQ Technologies Co., Ltd. All Rights Reserved.
+%%
+%% Licensed under the Apache License, Version 2.0 (the "License");
+%% you may not use this file except in compliance with the License.
+%% You may obtain a copy of the License at
+%%
+%%     http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS,
+%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+%% See the License for the specific language governing permissions and
+%% limitations under the License.
+%%--------------------------------------------------------------------
+
+-module(emqx_auth_http_utils).
+
+-export([
+    convert_headers/1,
+    convert_headers_no_content_type/1,
+    default_headers/0,
+    default_headers_no_content_type/0,
+    transform_header_name/1
+]).
+
+convert_headers(Headers) ->
+    transform_header_name(Headers).
+
+convert_headers_no_content_type(Headers) ->
+    maps:without(
+        [<<"content-type">>],
+        transform_header_name(Headers)
+    ).
+
+default_headers() ->
+    maps:put(
+        <<"content-type">>,
+        <<"application/json">>,
+        default_headers_no_content_type()
+    ).
+
+default_headers_no_content_type() ->
+    #{
+        <<"accept">> => <<"application/json">>,
+        <<"cache-control">> => <<"no-cache">>,
+        <<"connection">> => <<"keep-alive">>,
+        <<"keep-alive">> => <<"timeout=30, max=1000">>
+    }.
+
+transform_header_name(Headers) ->
+    maps:fold(
+        fun(K0, V, Acc) ->
+            K = list_to_binary(string:to_lower(to_list(K0))),
+            maps:put(K, V, Acc)
+        end,
+        #{},
+        Headers
+    ).
+
+to_list(A) when is_atom(A) ->
+    atom_to_list(A);
+to_list(B) when is_binary(B) ->
+    binary_to_list(B);
+to_list(L) when is_list(L) ->
+    L.

apps/emqx_auth_http/src/emqx_authn_http_schema.erl

@@ -121,9 +121,9 @@ headers(type) ->
 headers(desc) ->
     ?DESC(?FUNCTION_NAME);
 headers(converter) ->
-    fun emqx_authn_utils:convert_headers/1;
+    fun emqx_auth_http_utils:convert_headers/1;
 headers(default) ->
-    emqx_authn_utils:default_headers();
+    emqx_auth_http_utils:default_headers();
 headers(_) ->
     undefined.
 
@@ -132,9 +132,9 @@ headers_no_content_type(type) ->
 headers_no_content_type(desc) ->
     ?DESC(?FUNCTION_NAME);
 headers_no_content_type(converter) ->
-    fun emqx_authn_utils:convert_headers_no_content_type/1;
+    fun emqx_auth_http_utils:convert_headers_no_content_type/1;
 headers_no_content_type(default) ->
-    emqx_authn_utils:default_headers_no_content_type();
+    emqx_auth_http_utils:default_headers_no_content_type();
 headers_no_content_type(_) ->
     undefined.
 

apps/emqx_auth_http/src/emqx_authz_http.erl

@@ -29,7 +29,7 @@
     update/1,
     destroy/1,
     authorize/4,
-    merge_defaults/1
+    format_for_api/1
 ]).
 
 -ifdef(TEST).
@@ -139,18 +139,18 @@ authorize(
             ignore
     end.
 
-merge_defaults(#{<<"headers">> := Headers} = Source) ->
+format_for_api(#{<<"headers">> := Headers} = Source) ->
     NewHeaders =
         case Source of
             #{<<"method">> := <<"get">>} ->
-                (emqx_authz_http_schema:headers_no_content_type(converter))(Headers);
+                emqx_auth_http_utils:convert_headers_no_content_type(Headers);
             #{<<"method">> := <<"post">>} ->
-                (emqx_authz_http_schema:headers(converter))(Headers);
+                emqx_auth_http_utils:convert_headers(Headers);
             _ ->
                 Headers
         end,
     Source#{<<"headers">> => NewHeaders};
-merge_defaults(Source) ->
+format_for_api(Source) ->
     Source.
 
 log_nomtach_msg(Status, Headers, Body) ->
@@ -176,7 +176,7 @@ parse_config(
     Conf#{
         method => Method,
         request_base => RequestBase,
-        headers => Headers,
+        headers => maps:to_list(emqx_auth_http_utils:transform_header_name(Headers)),
         base_path_template => emqx_auth_utils:parse_str(Path, allowed_vars()),
         base_query_template => emqx_auth_utils:parse_deep(
             cow_qs:parse_qs(Query),

apps/emqx_auth_http/src/emqx_authz_http_schema.erl

@@ -64,9 +64,7 @@ fields(http_post) ->
 desc(http_get) ->
     ?DESC(http_get);
 desc(http_post) ->
-    ?DESC(http_post);
-desc(_) ->
-    undefined.
+    ?DESC(http_post).
 
 select_union_member(#{<<"type">> := ?AUTHZ_TYPE_BIN} = Value, _) ->
     Method = maps:get(<<"method">>, Value, undefined),
@@ -108,38 +106,43 @@ http_common_fields() ->
         ).
 
 headers(type) ->
-    typerefl:alias("map", list({binary(), binary()}), #{}, [binary(), binary()]);
+    map();
 headers(desc) ->
     ?DESC(?FUNCTION_NAME);
 headers(converter) ->
-    fun(Headers) ->
-        maps:to_list(transform_header_name(Headers))
+    fun
+        (Headers) when is_map(Headers) ->
+            emqx_auth_http_utils:convert_headers(Headers);
+        ([]) ->
+            emqx_auth_http_utils:convert_headers(#{});
+        (<<>>) ->
+            emqx_auth_http_utils:convert_headers(#{})
     end;
 headers(default) ->
-    default_headers();
+    emqx_auth_http_utils:default_headers();
 headers(_) ->
     undefined.
 
 headers_no_content_type(type) ->
-    typerefl:alias("map", list({binary(), binary()}), #{}, [binary(), binary()]);
+    map();
 headers_no_content_type(desc) ->
     ?DESC(?FUNCTION_NAME);
 headers_no_content_type(converter) ->
-    fun(Headers) ->
-        maps:to_list(
-            maps:without(
-                [<<"content-type">>],
-                transform_header_name(Headers)
-            )
-        )
+    fun
+        (Headers) when is_map(Headers) ->
+            emqx_auth_http_utils:convert_headers_no_content_type(Headers);
+        ([]) ->
+            emqx_auth_http_utils:convert_headers_no_content_type(#{});
+        (<<>>) ->
+            emqx_auth_http_utils:convert_headers_no_content_type(#{})
     end;
 headers_no_content_type(default) ->
-    default_headers_no_content_type();
+    emqx_auth_http_utils:default_headers_no_content_type();
 headers_no_content_type(validator) ->
     fun(Headers) ->
-        case lists:keyfind(<<"content-type">>, 1, Headers) of
-            false -> ok;
-            _ -> {error, do_not_include_content_type}
+        case Headers of
+            #{<<"content-type">> := _} -> {error, do_not_include_content_type};
+            _ -> ok
         end
     end;
 headers_no_content_type(_) ->
@@ -150,33 +153,3 @@ url(desc) -> ?DESC(?FUNCTION_NAME);
 url(validator) -> [?NOT_EMPTY("the value of the field 'url' cannot be empty")];
 url(required) -> true;
 url(_) -> undefined.
-
-default_headers() ->
-    maps:put(
-        <<"content-type">>,
-        <<"application/json">>,
-        default_headers_no_content_type()
-    ).
-
-default_headers_no_content_type() ->
-    #{
-        <<"accept">> => <<"application/json">>,
-        <<"cache-control">> => <<"no-cache">>,
-        <<"connection">> => <<"keep-alive">>,
-        <<"keep-alive">> => <<"timeout=30, max=1000">>
-    }.
-
-transform_header_name(Headers) ->
-    maps:fold(
-        fun(K0, V, Acc) ->
-            K = list_to_binary(string:to_lower(to_list(K0))),
-            maps:put(K, V, Acc)
-        end,
-        #{},
-        Headers
-    ).
-
-to_list(A) when is_atom(A) ->
-    atom_to_list(A);
-to_list(B) when is_binary(B) ->
-    binary_to_list(B).

apps/emqx_utils/src/emqx_utils_redact.erl

@@ -20,7 +20,7 @@
 -export([deobfuscate/2]).
 
 -define(REDACT_VAL, "******").
--define(IS_KEY_HEADERS(K), K == headers; K == <<"headers">>; K == "headers").
+-define(IS_KEY_HEADERS(K), (K == headers orelse K == <<"headers">> orelse K == "headers")).
 
 %% NOTE: keep alphabetical order
 is_sensitive_key(aws_secret_access_key) -> true;

changes/ce/fix-13584.en.md

@@ -0,0 +1 @@
+Fix HTTP authorization creation when HTTP header list is empty.