From 52b77b570ff3024c4a371914cdbd40e3716092f5 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 1 Jul 2022 16:18:19 +0800 Subject: [PATCH 1/2] refactor: authz-http return body to reject pub/sub --- apps/emqx_authz/src/emqx_authz.appup.src | 12 +++++++---- apps/emqx_authz/src/emqx_authz_http.erl | 21 +++++++++++++++++-- apps/emqx_authz/src/emqx_authz_utils.erl | 21 +++++++++++++++++++ .../emqx_authz/test/emqx_authz_http_SUITE.erl | 4 ++-- 4 files changed, 50 insertions(+), 8 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz.appup.src b/apps/emqx_authz/src/emqx_authz.appup.src index 9bf34f5b9..2cefe9f7b 100644 --- a/apps/emqx_authz/src/emqx_authz.appup.src +++ b/apps/emqx_authz/src/emqx_authz.appup.src @@ -1,7 +1,11 @@ %% -*- mode: erlang -*- %% Unless you know what you are doing, DO NOT edit manually!! {VSN, - [{"0.1.0",[{load_module,emqx_authz_utils,brutal_purge,soft_purge,[]}]}, - {"0.1.1",[{load_module,emqx_authz_utils,brutal_purge,soft_purge,[]}]}], - [{"0.1.0",[{load_module,emqx_authz_utils,brutal_purge,soft_purge,[]}]}, - {"0.1.1",[{load_module,emqx_authz_utils,brutal_purge,soft_purge,[]}]}]}. + [{<<"0\\.1\\.[0-1]">>,[ + {load_module,emqx_authz_utils,brutal_purge,soft_purge,[]}, + {load_module,emqx_authz_http,brutal_purge,soft_purge,[]}]} + ], + [{<<"0\\.1\\.[0-1]">>,[ + {load_module,emqx_authz_utils,brutal_purge,soft_purge,[]}, + {load_module,emqx_authz_http,brutal_purge,soft_purge,[]}]} + ]}. diff --git a/apps/emqx_authz/src/emqx_authz_http.erl b/apps/emqx_authz/src/emqx_authz_http.erl index 0f9c4dcce..4579a3729 100644 --- a/apps/emqx_authz/src/emqx_authz_http.erl +++ b/apps/emqx_authz/src/emqx_authz_http.erl @@ -84,8 +84,25 @@ authorize( {matched, allow}; {ok, 204, _Headers} -> {matched, allow}; - {ok, 200, _Headers, _Body} -> - {matched, allow}; + {ok, 200, Headers, Body} -> + ContentType = proplists:get_value( + <<"content-type">>, + Headers, + <<"application/json">> + ), + 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 + ), + body => Body + }), + nomatch; + Result -> + {matched, Result} + end; {ok, _Status, _Headers} -> nomatch; {ok, _Status, _Headers, _Body} -> diff --git a/apps/emqx_authz/src/emqx_authz_utils.erl b/apps/emqx_authz/src/emqx_authz_utils.erl index e0ee41817..e8b3d1e34 100644 --- a/apps/emqx_authz/src/emqx_authz_utils.erl +++ b/apps/emqx_authz/src/emqx_authz_utils.erl @@ -34,6 +34,8 @@ render_sql_params/2 ]). +-export([parse_http_resp_body/2]). + -define(DEFAULT_RESOURCE_OPTS, #{ auto_retry_interval => 6000, start_after_created => false @@ -130,6 +132,25 @@ render_sql_params(ParamList, Values) -> #{return => rawlist, var_trans => fun handle_sql_var/2} ). +-spec parse_http_resp_body(binary(), binary()) -> allow | deny | ignore | error. +parse_http_resp_body(<<"application/x-www-form-urlencoded">>, Body) -> + try + result(maps:from_list(cow_qs:parse_qs(Body))) + catch + _:_ -> error + end; +parse_http_resp_body(<<"application/json">>, Body) -> + try + result(emqx_json:decode(Body, [return_maps])) + catch + _:_ -> error + end. + +result(#{<<"result">> := <<"allow">>}) -> allow; +result(#{<<"result">> := <<"deny">>}) -> deny; +result(#{<<"result">> := <<"ignore">>}) -> ignore; +result(_) -> error. + %%-------------------------------------------------------------------- %% 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 9c00c5966..23c57181b 100644 --- a/apps/emqx_authz/test/emqx_authz_http_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_http_SUITE.erl @@ -85,8 +85,8 @@ t_response_handling(_Config) -> fun(Req0, State) -> Req = cowboy_req:reply( 200, - #{<<"content-type">> => <<"text/plain">>}, - "Response body", + #{<<"content-type">> => <<"application/json">>}, + "{\"result\": \"allow\"}", Req0 ), {ok, Req, State} From 83f5da8f9d39d801b5e216328578f759e43ad222 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 1 Jul 2022 16:46:13 +0800 Subject: [PATCH 2/2] fix(authz-http): fix https://github.com/emqx/emqx/pull/8377#discussion_r911743360 --- apps/emqx_authz/src/emqx_authz_http.erl | 15 ++++++++++----- apps/emqx_authz/src/emqx_authz_utils.erl | 4 ++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_http.erl b/apps/emqx_authz/src/emqx_authz_http.erl index 4579a3729..79e434587 100644 --- a/apps/emqx_authz/src/emqx_authz_http.erl +++ b/apps/emqx_authz/src/emqx_authz_http.erl @@ -85,11 +85,7 @@ authorize( {ok, 204, _Headers} -> {matched, allow}; {ok, 200, Headers, Body} -> - ContentType = proplists:get_value( - <<"content-type">>, - Headers, - <<"application/json">> - ), + ContentType = content_type(Headers), case emqx_authz_utils:parse_http_resp_body(ContentType, Body) of error -> ?SLOG(error, #{ @@ -222,6 +218,15 @@ 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 e8b3d1e34..5b91b7d57 100644 --- a/apps/emqx_authz/src/emqx_authz_utils.erl +++ b/apps/emqx_authz/src/emqx_authz_utils.erl @@ -133,13 +133,13 @@ render_sql_params(ParamList, Values) -> ). -spec parse_http_resp_body(binary(), binary()) -> allow | deny | ignore | error. -parse_http_resp_body(<<"application/x-www-form-urlencoded">>, Body) -> +parse_http_resp_body(<<"application/x-www-form-urlencoded", _/binary>>, Body) -> try result(maps:from_list(cow_qs:parse_qs(Body))) catch _:_ -> error end; -parse_http_resp_body(<<"application/json">>, Body) -> +parse_http_resp_body(<<"application/json", _/binary>>, Body) -> try result(emqx_json:decode(Body, [return_maps])) catch