Merge pull request #8381 from HJianBo/fix-content-type

chore: treat 200 and no body as ACL nomatch

apps/emqx_authz/src/emqx_authz_http.erl

@@ -80,28 +80,26 @@ authorize(
 ) ->
     Request = generate_request(PubSub, Topic, Client, Config),
     case emqx_resource:query(ResourceID, {Method, Request, RequestTimeout}) of
-        {ok, 200, _Headers} ->
-            {matched, allow};
         {ok, 204, _Headers} ->
             {matched, allow};
         {ok, 200, Headers, Body} ->
-            ContentType = content_type(Headers),
+            ContentType = emqx_authz_utils:content_type(Headers),
             case emqx_authz_utils:parse_http_resp_body(ContentType, Body) of
                 error ->
                     ?SLOG(error, #{
                         msg => authz_http_response_incorrect,
-                        content_type => proplists:get_value(
-                            <<"content-type">>, Headers
-                        ),
+                        content_type => ContentType,
                         body => Body
                     }),
                     nomatch;
                 Result ->
                     {matched, Result}
             end;
-        {ok, _Status, _Headers} ->
+        {ok, Status, Headers} ->
+            log_nomtach_msg(Status, Headers, undefined),
             nomatch;
-        {ok, _Status, _Headers, _Body} ->
+        {ok, Status, Headers, Body} ->
+            log_nomtach_msg(Status, Headers, Body),
             nomatch;
         {error, Reason} ->
             ?SLOG(error, #{
@@ -112,6 +110,17 @@ authorize(
             ignore
     end.
 
+log_nomtach_msg(Status, Headers, Body) ->
+    ?SLOG(
+        debug,
+        #{
+            msg => unexpected_authz_http_response,
+            status => Status,
+            content_type => emqx_authz_utils:content_type(Headers),
+            body => Body
+        }
+    ).
+
 parse_config(
     #{
         url := RawUrl,
@@ -218,15 +227,6 @@ serialize_body(<<"application/json">>, Body) ->
 serialize_body(<<"application/x-www-form-urlencoded">>, Body) ->
     query_string(Body).
 
-content_type(Headers) when is_list(Headers) ->
-    content_type(maps:from_list(Headers));
-content_type(#{<<"content-type">> := Type}) ->
-    Type;
-content_type(#{<<"Content-Type">> := Type}) ->
-    Type;
-content_type(Headers) when is_map(Headers) ->
-    <<"application/json">>.
-
 client_vars(Client, PubSub, Topic) ->
     Client#{
         action => PubSub,

apps/emqx_authz/src/emqx_authz_utils.erl

@@ -34,7 +34,10 @@
     render_sql_params/2
 ]).
 
--export([parse_http_resp_body/2]).
+-export([
+    parse_http_resp_body/2,
+    content_type/1
+]).
 
 -define(DEFAULT_RESOURCE_OPTS, #{
     auto_retry_interval => 6000,
@@ -151,6 +154,16 @@ result(#{<<"result">> := <<"deny">>}) -> deny;
 result(#{<<"result">> := <<"ignore">>}) -> ignore;
 result(_) -> error.
 
+-spec content_type(cow_http:headers()) -> binary().
+content_type(Headers) when is_list(Headers) ->
+    %% header name is lower case, see:
+    %% https://github.com/ninenines/cowlib/blob/ce6798c6b2e95b6a34c6a76d2489eaf159827d80/src/cow_http.erl#L192
+    proplists:get_value(
+        <<"content-type">>,
+        Headers,
+        <<"application/json">>
+    ).
+
 %%--------------------------------------------------------------------
 %% Internal functions
 %%--------------------------------------------------------------------

apps/emqx_authz/test/emqx_authz_http_SUITE.erl

@@ -26,6 +26,15 @@
 -define(HTTP_PORT, 33333).
 -define(HTTP_PATH, "/authz/[...]").
 
+-define(AUTHZ_HTTP_RESP(Result, Req),
+    cowboy_req:reply(
+        200,
+        #{<<"content-type">> => <<"application/json">>},
+        "{\"result\": \"" ++ atom_to_list(Result) ++ "\"}",
+        Req
+    )
+).
+
 all() ->
     emqx_common_test_helpers:all(?MODULE).
 
@@ -69,35 +78,29 @@ t_response_handling(_Config) ->
         listener => {tcp, default}
     },
 
-    %% OK, get, no body
+    %% OK, get, body & headers
     ok = setup_handler_and_config(
         fun(Req0, State) ->
-            Req = cowboy_req:reply(200, Req0),
-            {ok, Req, State}
+            {ok, ?AUTHZ_HTTP_RESP(allow, Req0), State}
         end,
         #{}
     ),
 
-    allow = emqx_access_control:authorize(ClientInfo, publish, <<"t">>),
+    ?assertEqual(
+        allow,
+        emqx_access_control:authorize(ClientInfo, publish, <<"t">>)
+    ),
 
-    %% OK, get, body & headers
+    %% Not OK, get, no body
     ok = setup_handler_and_config(
         fun(Req0, State) ->
-            Req = cowboy_req:reply(
-                200,
-                #{<<"content-type">> => <<"application/json">>},
-                "{\"result\": \"allow\"}",
-                Req0
-            ),
+            Req = cowboy_req:reply(200, Req0),
             {ok, Req, State}
         end,
         #{}
     ),
 
-    ?assertEqual(
-        allow,
-        emqx_access_control:authorize(ClientInfo, publish, <<"t">>)
-    ),
+    deny = emqx_access_control:authorize(ClientInfo, publish, <<"t">>),
 
     %% OK, get, 204
     ok = setup_handler_and_config(
@@ -169,8 +172,7 @@ t_query_params(_Config) ->
                 ],
                 Req0
             ),
-            Req = cowboy_req:reply(200, Req0),
-            {ok, Req, State}
+            {ok, ?AUTHZ_HTTP_RESP(allow, Req0), State}
         end,
         #{
             <<"url">> => <<
@@ -217,8 +219,7 @@ t_path(_Config) ->
                 >>,
                 cowboy_req:path(Req0)
             ),
-            Req = cowboy_req:reply(200, Req0),
-            {ok, Req, State}
+            {ok, ?AUTHZ_HTTP_RESP(allow, Req0), State}
         end,
         #{
             <<"url">> => <<
@@ -271,9 +272,7 @@ t_json_body(_Config) ->
                 },
                 jiffy:decode(RawBody, [return_maps])
             ),
-
-            Req = cowboy_req:reply(200, Req1),
-            {ok, Req, State}
+            {ok, ?AUTHZ_HTTP_RESP(allow, Req1), State}
         end,
         #{
             <<"method">> => <<"post">>,
@@ -326,9 +325,7 @@ t_form_body(_Config) ->
                 },
                 jiffy:decode(PostVars, [return_maps])
             ),
-
-            Req = cowboy_req:reply(200, Req1),
-            {ok, Req, State}
+            {ok, ?AUTHZ_HTTP_RESP(allow, Req1), State}
         end,
         #{
             <<"method">> => <<"post">>,
@@ -372,8 +369,7 @@ t_create_replace(_Config) ->
     %% Create with valid URL
     ok = setup_handler_and_config(
         fun(Req0, State) ->
-            Req = cowboy_req:reply(200, Req0),
-            {ok, Req, State}
+            {ok, ?AUTHZ_HTTP_RESP(allow, Req0), State}
         end,
         #{
             <<"url">> =>

apps/emqx_gateway/test/emqx_gateway_auth_ct.erl

@@ -65,6 +65,15 @@
 
 -record(state, {}).
 
+-define(AUTHZ_HTTP_RESP(Result, Req),
+    cowboy_req:reply(
+        200,
+        #{<<"content-type">> => <<"application/json">>},
+        "{\"result\": \"" ++ atom_to_list(Result) ++ "\"}",
+        Req
+    )
+).
+
 %%------------------------------------------------------------------------------
 %% API
 %%------------------------------------------------------------------------------
@@ -171,12 +180,12 @@ on_start_auth(authz_http) ->
     Handler = fun(Req0, State) ->
         case cowboy_req:match_qs([topic, action, username], Req0) of
             #{topic := <<"/publish">>, action := <<"publish">>} ->
-                Req = cowboy_req:reply(200, Req0);
+                Req = ?AUTHZ_HTTP_RESP(allow, Req0);
             #{topic := <<"/subscribe">>, action := <<"subscribe">>} ->
-                Req = cowboy_req:reply(200, Req0);
+                Req = ?AUTHZ_HTTP_RESP(allow, Req0);
             %% for lwm2m
             #{username := <<"lwm2m">>} ->
-                Req = cowboy_req:reply(200, Req0);
+                Req = ?AUTHZ_HTTP_RESP(allow, Req0);
             _ ->
                 Req = cowboy_req:reply(400, Req0)
         end,