diff --git a/apps/emqx_auth/src/emqx_authz/emqx_authz_utils.erl b/apps/emqx_auth/src/emqx_authz/emqx_authz_utils.erl index 26b8d000f..5c7b0965c 100644 --- a/apps/emqx_auth/src/emqx_authz/emqx_authz_utils.erl +++ b/apps/emqx_auth/src/emqx_authz/emqx_authz_utils.erl @@ -188,7 +188,7 @@ render_sql_params(ParamList, Values) -> ), Row. --spec parse_http_resp_body(binary(), binary()) -> allow | deny | ignore | error. +-spec parse_http_resp_body(binary(), binary()) -> allow | deny | ignore | error | {error, term()}. parse_http_resp_body(<<"application/x-www-form-urlencoded", _/binary>>, Body) -> try result(maps:from_list(cow_qs:parse_qs(Body))) @@ -200,7 +200,9 @@ parse_http_resp_body(<<"application/json", _/binary>>, Body) -> result(emqx_utils_json:decode(Body, [return_maps])) catch _:_ -> error - end. + end; +parse_http_resp_body(ContentType = <<_/binary>>, _Body) -> + {error, <<"unsupported content-type: ", ContentType/binary>>}. result(#{<<"result">> := <<"allow">>}) -> allow; result(#{<<"result">> := <<"deny">>}) -> deny; diff --git a/apps/emqx_auth_http/src/emqx_authz_http.erl b/apps/emqx_auth_http/src/emqx_authz_http.erl index 49296f690..e4d4326ff 100644 --- a/apps/emqx_auth_http/src/emqx_authz_http.erl +++ b/apps/emqx_auth_http/src/emqx_authz_http.erl @@ -105,6 +105,9 @@ authorize( body => Body }), nomatch; + {error, Reason} -> + ?tp(error, bad_authz_http_response, #{reason => Reason}), + nomatch; Result -> {matched, Result} end; diff --git a/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl b/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl index 93990791d..3fb2c0572 100644 --- a/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl +++ b/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl @@ -463,6 +463,72 @@ t_placeholder_and_body(_Config) -> emqx_access_control:authorize(ClientInfo, ?AUTHZ_PUBLISH, <<"t">>) ). +%% Checks that we don't crash when receiving an unsupported content-type back. +t_bad_response_content_type(_Config) -> + ok = setup_handler_and_config( + fun(Req0, State) -> + ?assertEqual( + <<"/authz/users/">>, + cowboy_req:path(Req0) + ), + + {ok, _PostVars, Req1} = cowboy_req:read_urlencoded_body(Req0), + + Req = cowboy_req:reply( + 200, + #{<<"content-type">> => <<"text/csv">>}, + "hi", + Req1 + ), + {ok, Req, State} + end, + #{ + <<"method">> => <<"post">>, + <<"body">> => #{ + <<"username">> => <<"${username}">>, + <<"clientid">> => <<"${clientid}">>, + <<"peerhost">> => <<"${peerhost}">>, + <<"proto_name">> => <<"${proto_name}">>, + <<"mountpoint">> => <<"${mountpoint}">>, + <<"topic">> => <<"${topic}">>, + <<"action">> => <<"${action}">>, + <<"access">> => <<"${access}">>, + <<"CN">> => ?PH_CERT_CN_NAME, + <<"CS">> => ?PH_CERT_SUBJECT + }, + <<"headers">> => #{ + <<"accept">> => <<"text/plain">>, + <<"content-type">> => <<"application/json">> + } + } + ), + + ClientInfo = #{ + clientid => <<"client id">>, + username => <<"user name">>, + peerhost => {127, 0, 0, 1}, + protocol => <<"MQTT">>, + mountpoint => <<"MOUNTPOINT">>, + zone => default, + listener => {tcp, default}, + cn => ?PH_CERT_CN_NAME, + dn => ?PH_CERT_SUBJECT + }, + + ?check_trace( + ?assertEqual( + deny, + emqx_access_control:authorize(ClientInfo, ?AUTHZ_PUBLISH, <<"t">>) + ), + fun(Trace) -> + ?assertMatch( + [#{reason := <<"unsupported content-type", _/binary>>}], + ?of_kind(bad_authz_http_response, Trace) + ), + ok + end + ). + t_no_value_for_placeholder(_Config) -> ok = setup_handler_and_config( fun(Req0, State) -> diff --git a/changes/ce/fix-13238.en.md b/changes/ce/fix-13238.en.md new file mode 100644 index 000000000..b9f039c33 --- /dev/null +++ b/changes/ce/fix-13238.en.md @@ -0,0 +1 @@ +Improved the logged error messages when an HTTP authorization request with an unsupported content-type header is returned.