From 4c17b381025eeb3789dfaeb1c783190e3cc9c7b1 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 1 Jul 2022 20:29:21 +0800 Subject: [PATCH] chore: treat 200/204 as acl nomatch --- apps/emqx_authz/src/emqx_authz_http.erl | 34 +++++------ apps/emqx_authz/src/emqx_authz_utils.erl | 15 ++++- .../emqx_authz/test/emqx_authz_http_SUITE.erl | 56 +++++++++---------- 3 files changed, 57 insertions(+), 48 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_http.erl b/apps/emqx_authz/src/emqx_authz_http.erl index 79e434587..69f21932a 100644 --- a/apps/emqx_authz/src/emqx_authz_http.erl +++ b/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, diff --git a/apps/emqx_authz/src/emqx_authz_utils.erl b/apps/emqx_authz/src/emqx_authz_utils.erl index 5b91b7d57..dd6f66c7f 100644 --- a/apps/emqx_authz/src/emqx_authz_utils.erl +++ b/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 %%-------------------------------------------------------------------- diff --git a/apps/emqx_authz/test/emqx_authz_http_SUITE.erl b/apps/emqx_authz/test/emqx_authz_http_SUITE.erl index 23c57181b..4b5ad7cbf 100644 --- a/apps/emqx_authz/test/emqx_authz_http_SUITE.erl +++ b/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,27 +78,10 @@ t_response_handling(_Config) -> listener => {tcp, default} }, - %% OK, get, no body - ok = setup_handler_and_config( - fun(Req0, State) -> - Req = cowboy_req:reply(200, Req0), - {ok, Req, State} - end, - #{} - ), - - allow = emqx_access_control:authorize(ClientInfo, publish, <<"t">>), - %% OK, get, body & headers ok = setup_handler_and_config( fun(Req0, State) -> - Req = cowboy_req:reply( - 200, - #{<<"content-type">> => <<"application/json">>}, - "{\"result\": \"allow\"}", - Req0 - ), - {ok, Req, State} + {ok, ?AUTHZ_HTTP_RESP(allow, Req0), State} end, #{} ), @@ -99,6 +91,17 @@ t_response_handling(_Config) -> emqx_access_control:authorize(ClientInfo, publish, <<"t">>) ), + %% Not OK, get, no body + ok = setup_handler_and_config( + fun(Req0, State) -> + Req = cowboy_req:reply(200, Req0), + {ok, Req, State} + end, + #{} + ), + + deny = emqx_access_control:authorize(ClientInfo, publish, <<"t">>), + %% OK, get, 204 ok = setup_handler_and_config( fun(Req0, State) -> @@ -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">> =>