From e0f860d7d93f1a59fe23ceb465a0b79929072224 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Fri, 17 Dec 2021 23:03:25 +0300 Subject: [PATCH 1/4] chore(authz): fix HTTP authz, cover with tests --- apps/emqx_authz/src/emqx_authz.erl | 6 +- apps/emqx_authz/src/emqx_authz_http.erl | 139 ++++-- apps/emqx_authz/src/emqx_authz_schema.erl | 11 +- apps/emqx_authz/test/emqx_authz_SUITE.erl | 6 +- .../emqx_authz/test/emqx_authz_http_SUITE.erl | 415 +++++++++++++++--- .../test/emqx_authz_http_test_server.erl | 89 ++++ 6 files changed, 564 insertions(+), 102 deletions(-) create mode 100644 apps/emqx_authz/test/emqx_authz_http_test_server.erl diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index 54438793b..510306efe 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/apps/emqx_authz/src/emqx_authz.erl @@ -59,6 +59,8 @@ -define(METRICS, [?METRIC_ALLOW, ?METRIC_DENY, ?METRIC_NOMATCH]). +-define(IS_ENABLED(Enable), ((Enable =:= true) or (Enable =:= <<"true">>))). + %% Initialize authz backend. %% Populate the passed configuration map with necessary data, %% like `ResourceID`s @@ -155,8 +157,8 @@ do_update({?CMD_APPEND, Sources}, Conf) when is_list(Sources), is_list(Conf) -> NConf = Conf ++ Sources, ok = check_dup_types(NConf), NConf; -do_update({{?CMD_REPLACE, Type}, #{<<"enable">> := true} = Source}, Conf) when is_map(Source), - is_list(Conf) -> +do_update({{?CMD_REPLACE, Type}, #{<<"enable">> := Enable} = Source}, Conf) + when is_map(Source), is_list(Conf), ?IS_ENABLED(Enable) -> case create_dry_run(Type, Source) of ok -> {_Old, Front, Rear} = take(Type, Conf), diff --git a/apps/emqx_authz/src/emqx_authz_http.erl b/apps/emqx_authz/src/emqx_authz_http.erl index c2ee96594..d677704c4 100644 --- a/apps/emqx_authz/src/emqx_authz_http.erl +++ b/apps/emqx_authz/src/emqx_authz_http.erl @@ -40,9 +40,8 @@ description() -> "AuthZ with http". -init(#{url := Url} = Source) -> - NSource = maps:put(base_url, maps:remove(query, Url), Source), - case emqx_authz_utils:create_resource(emqx_connector_http, NSource) of +init(Source) -> + case emqx_authz_utils:create_resource(emqx_connector_http, Source) of {error, Reason} -> error({load_config_error, Reason}); {ok, Id} -> Source#{annotations => #{id => Id}} end. @@ -51,39 +50,60 @@ destroy(#{annotations := #{id := Id}}) -> ok = emqx_resource:remove(Id). dry_run(Source) -> - URIMap = maps:get(url, Source), - NSource = maps:put(base_url, maps:remove(query, URIMap), Source), - emqx_resource:create_dry_run(emqx_connector_http, NSource). + emqx_resource:create_dry_run(emqx_connector_http, Source). authorize(Client, PubSub, Topic, #{type := http, - url := #{path := Path} = URL, + query := Query, + path := Path, headers := Headers, method := Method, request_timeout := RequestTimeout, annotations := #{id := ResourceID} } = Source) -> Request = case Method of - get -> - Query = maps:get(query, URL, ""), - Path1 = replvar(Path ++ "?" ++ Query, PubSub, Topic, Client), + get -> + Path1 = replvar( + Path ++ "?" ++ Query, + PubSub, + Topic, + maps:to_list(Client), + fun var_uri_encode/1), + {Path1, maps:to_list(Headers)}; + _ -> - Body0 = serialize_body( - maps:get('Accept', Headers, <<"application/json">>), - maps:get(body, Source, #{}) - ), - Body1 = replvar(Body0, PubSub, Topic, Client), - Path1 = replvar(Path, PubSub, Topic, Client), - {Path1, maps:to_list(Headers), Body1} + Body0 = maps:get(body, Source, #{}), + Body1 = replvar_deep( + Body0, + PubSub, + Topic, + maps:to_list(Client), + fun var_bin_encode/1), + + Body2 = serialize_body( + maps:get(<<"content-type">>, Headers, <<"application/json">>), + Body1), + + Path1 = replvar( + Path, + PubSub, + Topic, + maps:to_list(Client), + fun var_uri_encode/1), + + {Path1, maps:to_list(Headers), Body2} end, - case emqx_resource:query(ResourceID, {Method, Request, RequestTimeout}) of + HttpResult = emqx_resource:query(ResourceID, {Method, Request, RequestTimeout}), + case HttpResult of {ok, 200, _Headers} -> {matched, allow}; {ok, 204, _Headers} -> {matched, allow}; {ok, 200, _Headers, _Body} -> {matched, allow}; + {ok, _Status, _Headers} -> + nomatch; {ok, _Status, _Headers, _Body} -> nomatch; {error, Reason} -> @@ -121,30 +141,67 @@ serialize_body(<<"application/json">>, Body) -> serialize_body(<<"application/x-www-form-urlencoded">>, Body) -> query_string(Body). -replvar(Str0, PubSub, Topic, - #{username := Username, - clientid := Clientid, - peerhost := IpAddress, - protocol := Protocol, - mountpoint := Mountpoint - }) when is_list(Str0); - is_binary(Str0) -> + +replvar_deep(Map, PubSub, Topic, Vars, VarEncode) when is_map(Map) -> + maps:from_list( + lists:map( + fun({Key, Value}) -> + {replvar(Key, PubSub, Topic, Vars, VarEncode), + replvar(Value, PubSub, Topic, Vars, VarEncode)} + end, + maps:to_list(Map))); +replvar_deep(List, PubSub, Topic, Vars, VarEncode) when is_list(List) -> + lists:map( + fun(Value) -> + replvar(Value, PubSub, Topic, Vars, VarEncode) + end, + List); +replvar_deep(Number, _PubSub, _Topic, _Vars, _VarEncode) when is_number(Number) -> + Number; +replvar_deep(Binary, PubSub, Topic, Vars, VarEncode) when is_binary(Binary) -> + replvar(Binary, PubSub, Topic, Vars, VarEncode). + +replvar(Str0, PubSub, Topic, [], VarEncode) -> NTopic = emqx_http_lib:uri_encode(Topic), - Str1 = re:replace( Str0, emqx_authz:ph_to_re(?PH_S_CLIENTID) - , bin(Clientid), [global, {return, binary}]), - Str2 = re:replace( Str1, emqx_authz:ph_to_re(?PH_S_USERNAME) - , bin(Username), [global, {return, binary}]), - Str3 = re:replace( Str2, emqx_authz:ph_to_re(?PH_S_HOST) - , inet_parse:ntoa(IpAddress), [global, {return, binary}]), - Str4 = re:replace( Str3, emqx_authz:ph_to_re(?PH_S_PROTONAME) - , bin(Protocol), [global, {return, binary}]), - Str5 = re:replace( Str4, emqx_authz:ph_to_re(?PH_S_MOUNTPOINT) - , bin(Mountpoint), [global, {return, binary}]), - Str6 = re:replace( Str5, emqx_authz:ph_to_re(?PH_S_TOPIC) - , bin(NTopic), [global, {return, binary}]), - Str7 = re:replace( Str6, emqx_authz:ph_to_re(?PH_S_ACTION) - , bin(PubSub), [global, {return, binary}]), - Str7. + Str1 = re:replace(Str0, emqx_authz:ph_to_re(?PH_S_TOPIC), + VarEncode(NTopic), [global, {return, binary}]), + re:replace(Str1, emqx_authz:ph_to_re(?PH_S_ACTION), + VarEncode(PubSub), [global, {return, binary}]); + + +replvar(Str, PubSub, Topic, [{username, Username} | Rest], VarEncode) -> + Str1 = re:replace(Str, emqx_authz:ph_to_re(?PH_S_USERNAME), + VarEncode(Username), [global, {return, binary}]), + replvar(Str1, PubSub, Topic, Rest, VarEncode); + +replvar(Str, PubSub, Topic, [{clientid, Clientid} | Rest], VarEncode) -> + Str1 = re:replace(Str, emqx_authz:ph_to_re(?PH_S_CLIENTID), + VarEncode(Clientid), [global, {return, binary}]), + replvar(Str1, PubSub, Topic, Rest, VarEncode); + +replvar(Str, PubSub, Topic, [{peerhost, IpAddress} | Rest], VarEncode) -> + Str1 = re:replace(Str, emqx_authz:ph_to_re(?PH_S_PEERHOST), + VarEncode(inet_parse:ntoa(IpAddress)), [global, {return, binary}]), + replvar(Str1, PubSub, Topic, Rest, VarEncode); + +replvar(Str, PubSub, Topic, [{protocol, Protocol} | Rest], VarEncode) -> + Str1 = re:replace(Str, emqx_authz:ph_to_re(?PH_S_PROTONAME), + VarEncode(Protocol), [global, {return, binary}]), + replvar(Str1, PubSub, Topic, Rest, VarEncode); + +replvar(Str, PubSub, Topic, [{mountpoint, Mountpoint} | Rest], VarEncode) -> + Str1 = re:replace(Str, emqx_authz:ph_to_re(?PH_S_MOUNTPOINT), + VarEncode(Mountpoint), [global, {return, binary}]), + replvar(Str1, PubSub, Topic, Rest, VarEncode); + +replvar(Str, PubSub, Topic, [_Unknown | Rest], VarEncode) -> + replvar(Str, PubSub, Topic, Rest, VarEncode). + +var_uri_encode(S) -> + emqx_http_lib:uri_encode(bin(S)). + +var_bin_encode(S) -> + bin(S). bin(A) when is_atom(A) -> atom_to_binary(A, utf8); bin(B) when is_binary(B) -> B; diff --git a/apps/emqx_authz/src/emqx_authz_schema.erl b/apps/emqx_authz/src/emqx_authz_schema.erl index 4f7788849..8415e3710 100644 --- a/apps/emqx_authz/src/emqx_authz_schema.erl +++ b/apps/emqx_authz/src/emqx_authz_schema.erl @@ -20,14 +20,10 @@ -reflect_type([ permission/0 , action/0 - , url/0 ]). --typerefl_from_string({url/0, emqx_http_lib, uri_parse}). - -type action() :: publish | subscribe | all. -type permission() :: allow | deny. --type url() :: emqx_http_lib:uri_map(). -export([ namespace/0 , roots/0 @@ -143,10 +139,11 @@ fields(redis_cluster) -> http_common_fields() -> [ {type, #{type => http}} , {enable, #{type => boolean(), default => true}} - , {url, #{type => url()}} , {request_timeout, mk_duration("request timeout", #{default => "30s"})} , {body, #{type => map(), nullable => true}} - ] ++ proplists:delete(base_url, emqx_connector_http:fields(config)). + , {path, #{type => string(), default => ""}} + , {query, #{type => string(), default => ""}} + ] ++ emqx_connector_http:fields(config). mongo_common_fields() -> [ {collection, #{type => atom()}} @@ -203,7 +200,7 @@ check_ssl_opts(Conf) when Conf =:= #{} -> true; check_ssl_opts(Conf) -> - case emqx_authz_http:parse_url(hocon_schema:get_value("config.url", Conf)) of + case emqx_authz_http:parse_url(hocon_schema:get_value("config.base_url", Conf)) of #{scheme := https} -> case hocon_schema:get_value("config.ssl.enable", Conf) of true -> ok; diff --git a/apps/emqx_authz/test/emqx_authz_SUITE.erl b/apps/emqx_authz/test/emqx_authz_SUITE.erl index e18901fc5..942f74abc 100644 --- a/apps/emqx_authz/test/emqx_authz_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_SUITE.erl @@ -65,7 +65,9 @@ set_special_configs(_App) -> -define(SOURCE1, #{<<"type">> => <<"http">>, <<"enable">> => true, - <<"url">> => <<"https://fake.com:443/">>, + <<"base_url">> => <<"https://example.com:443/">>, + <<"path">> => <<"a/b">>, + <<"query">> => <<"c=d">>, <<"headers">> => #{}, <<"method">> => <<"get">>, <<"request_timeout">> => 5000 @@ -77,7 +79,7 @@ set_special_configs(_App) -> <<"pool_size">> => 1, <<"database">> => <<"mqtt">>, <<"ssl">> => #{<<"enable">> => false}, - <<"collection">> => <<"fake">>, + <<"collection">> => <<"authz">>, <<"selector">> => #{<<"a">> => <<"b">>} }). -define(SOURCE3, #{<<"type">> => <<"mysql">>, diff --git a/apps/emqx_authz/test/emqx_authz_http_SUITE.erl b/apps/emqx_authz/test/emqx_authz_http_SUITE.erl index c438b3f4b..7196d75ff 100644 --- a/apps/emqx_authz/test/emqx_authz_http_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_http_SUITE.erl @@ -4,7 +4,8 @@ %% 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 +%% +%% 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, @@ -22,75 +23,389 @@ -include_lib("eunit/include/eunit.hrl"). -include_lib("common_test/include/ct.hrl"). +-define(HTTP_PORT, 33333). +-define(HTTP_PATH, "/authz/[...]"). + 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, fun(_, _, _) -> {ok, meck_data} end), - meck:expect(emqx_resource, remove, fun(_) -> ok end ), - ok = emqx_common_test_helpers:start_apps( [emqx_conf, emqx_authz], - fun set_special_configs/1), - - Rules = [#{<<"type">> => <<"http">>, - <<"url">> => <<"https://fake.com:443/">>, - <<"headers">> => #{}, - <<"method">> => <<"get">>, - <<"request_timeout">> => 5000 - } - ], - {ok, _} = emqx_authz:update(replace, Rules), + fun set_special_configs/1 + ), + ok = start_apps([emqx_resource, emqx_connector, cowboy]), Config. end_per_suite(_Config) -> - {ok, _} = emqx:update_config( - [authorization], - #{<<"no_match">> => <<"allow">>, - <<"cache">> => #{<<"enable">> => <<"true">>}, - <<"sources">> => []}), - emqx_common_test_helpers:stop_apps([emqx_authz, emqx_conf]), - meck:unload(emqx_resource), - ok. + ok = emqx_authz_test_lib:restore_authorizers(), + ok = stop_apps([emqx_resource, emqx_connector, cowboy]), + ok = emqx_common_test_helpers:stop_apps([emqx_authz]). set_special_configs(emqx_authz) -> - {ok, _} = emqx:update_config([authorization, cache, enable], false), - {ok, _} = emqx:update_config([authorization, no_match], deny), - {ok, _} = emqx:update_config([authorization, sources], []), - ok; -set_special_configs(_App) -> + ok = emqx_authz_test_lib:reset_authorizers(); + +set_special_configs(_) -> ok. +init_per_testcase(_Case, Config) -> + ok = emqx_authz_test_lib:reset_authorizers(), + ok = emqx_authz_http_test_server:start(?HTTP_PORT, ?HTTP_PATH), + Config. + +end_per_testcase(_Case, _Config) -> + ok = emqx_authz_http_test_server:stop(). + %%------------------------------------------------------------------------------ -%% Testcases +%% Tests %%------------------------------------------------------------------------------ -t_authz(_) -> - ClientInfo = #{clientid => <<"my-clientid">>, - username => <<"my-username">>, +t_response_handling(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, peerhost => {127,0,0,1}, - protocol => mqtt, - mountpoint => <<"fake">>, zone => default, listener => {tcp, default} - }, + }, - meck:expect(emqx_resource, query, fun(_, _) -> {ok, 204, fake_headers} end), - ?assertEqual(allow, - emqx_access_control:authorize(ClientInfo, subscribe, <<"#">>)), + %% OK, get, no body + ok = setup_handler_and_config( + fun(Req0, State) -> + Req = cowboy_req:reply(200, Req0), + {ok, Req, State} + end, + #{}), - meck:expect(emqx_resource, query, fun(_, _) -> {ok, 200, fake_headers, fake_body} end), - ?assertEqual(allow, - emqx_access_control:authorize(ClientInfo, publish, <<"#">>)), + 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">> => <<"text/plain">>}, + "Response body", + Req0), + {ok, Req, State} + end, + #{}), + + ?assertEqual( + allow, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)), + + %% OK, get, 204 + ok = setup_handler_and_config( + fun(Req0, State) -> + Req = cowboy_req:reply(204, Req0), + {ok, Req, State} + end, + #{}), + + ?assertEqual( + allow, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)), + + %% Not OK, get, 400 + ok = setup_handler_and_config( + fun(Req0, State) -> + Req = cowboy_req:reply(400, Req0), + {ok, Req, State} + end, + #{}), + + ?assertEqual( + deny, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)), + + %% Not OK, get, 400 + body & headers + ok = setup_handler_and_config( + fun(Req0, State) -> + Req = cowboy_req:reply( + 400, + #{<<"content-type">> => <<"text/plain">>}, + "Response body", + Req0), + {ok, Req, State} + end, + #{}), + + ?assertEqual( + deny, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)). + +t_query_params(_Config) -> + ok = setup_handler_and_config( + fun(Req0, State) -> + #{username := <<"user name">>, + clientid := <<"client id">>, + peerhost := <<"127.0.0.1">>, + proto_name := <<"MQTT">>, + mountpoint := <<"MOUNTPOINT">>, + topic := <<"t">>, + action := <<"publish">> + } = cowboy_req:match_qs( + [username, + clientid, + peerhost, + proto_name, + mountpoint, + topic, + action], + Req0), + Req = cowboy_req:reply(200, Req0), + {ok, Req, State} + end, + #{<<"query">> => <<"username=${username}&" + "clientid=${clientid}&" + "peerhost=${peerhost}&" + "proto_name=${proto_name}&" + "mountpoint=${mountpoint}&" + "topic=${topic}&" + "action=${action}">> + }), + + ClientInfo = #{clientid => <<"client id">>, + username => <<"user name">>, + peerhost => {127,0,0,1}, + protocol => <<"MQTT">>, + mountpoint => <<"MOUNTPOINT">>, + zone => default, + listener => {tcp, default} + }, + + ?assertEqual( + allow, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)). + +t_path_params(_Config) -> + ok = setup_handler_and_config( + fun(Req0, State) -> + <<"/authz/" + "username/user%20name/" + "clientid/client%20id/" + "peerhost/127.0.0.1/" + "proto_name/MQTT/" + "mountpoint/MOUNTPOINT/" + "topic/t/" + "action/publish">> = cowboy_req:path(Req0), + Req = cowboy_req:reply(200, Req0), + {ok, Req, State} + end, + #{<<"path">> => <<"username/${username}/" + "clientid/${clientid}/" + "peerhost/${peerhost}/" + "proto_name/${proto_name}/" + "mountpoint/${mountpoint}/" + "topic/${topic}/" + "action/${action}">> + }), + + ClientInfo = #{clientid => <<"client id">>, + username => <<"user name">>, + peerhost => {127,0,0,1}, + protocol => <<"MQTT">>, + mountpoint => <<"MOUNTPOINT">>, + zone => default, + listener => {tcp, default} + }, + + ?assertEqual( + allow, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)). + +t_json_body(_Config) -> + ok = setup_handler_and_config( + fun(Req0, State) -> + ?assertEqual( + <<"/authz/" + "username/user%20name/" + "clientid/client%20id/" + "peerhost/127.0.0.1/" + "proto_name/MQTT/" + "mountpoint/MOUNTPOINT/" + "topic/t/" + "action/publish">>, + cowboy_req:path(Req0)), + + {ok, RawBody, Req1} = cowboy_req:read_body(Req0), + + ?assertMatch( + #{<<"username">> := <<"user name">>, + <<"CLIENT_client id">> := <<"client id">>, + <<"peerhost">> := <<"127.0.0.1">>, + <<"proto_name">> := <<"MQTT">>, + <<"mountpoint">> := <<"MOUNTPOINT">>, + <<"topic">> := <<"t">>, + <<"action">> := <<"publish">>}, + jiffy:decode(RawBody, [return_maps])), + + Req = cowboy_req:reply(200, Req1), + {ok, Req, State} + end, + #{<<"method">> => <<"post">>, + <<"path">> => <<"username/${username}/" + "clientid/${clientid}/" + "peerhost/${peerhost}/" + "proto_name/${proto_name}/" + "mountpoint/${mountpoint}/" + "topic/${topic}/" + "action/${action}">>, + <<"body">> => #{<<"username">> => <<"${username}">>, + <<"CLIENT_${clientid}">> => <<"${clientid}">>, + <<"peerhost">> => <<"${peerhost}">>, + <<"proto_name">> => <<"${proto_name}">>, + <<"mountpoint">> => <<"${mountpoint}">>, + <<"topic">> => <<"${topic}">>, + <<"action">> => <<"${action}">>} + }), + + ClientInfo = #{clientid => <<"client id">>, + username => <<"user name">>, + peerhost => {127,0,0,1}, + protocol => <<"MQTT">>, + mountpoint => <<"MOUNTPOINT">>, + zone => default, + listener => {tcp, default} + }, + + ?assertEqual( + allow, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)). - meck:expect(emqx_resource, query, fun(_, _) -> {error, other} end), - ?assertEqual(deny, - emqx_access_control:authorize(ClientInfo, subscribe, <<"+">>)), - ?assertEqual(deny, - emqx_access_control:authorize(ClientInfo, publish, <<"+">>)), - ok. +t_form_body(_Config) -> + ok = setup_handler_and_config( + fun(Req0, State) -> + ?assertEqual( + <<"/authz/" + "username/user%20name/" + "clientid/client%20id/" + "peerhost/127.0.0.1/" + "proto_name/MQTT/" + "mountpoint/MOUNTPOINT/" + "topic/t/" + "action/publish">>, + cowboy_req:path(Req0)), + + {ok, PostVars, Req1} = cowboy_req:read_urlencoded_body(Req0), + + ?assertMatch( + #{<<"username">> := <<"user name">>, + <<"clientid">> := <<"client id">>, + <<"peerhost">> := <<"127.0.0.1">>, + <<"proto_name">> := <<"MQTT">>, + <<"mountpoint">> := <<"MOUNTPOINT">>, + <<"topic">> := <<"t">>, + <<"action">> := <<"publish">>}, + maps:from_list(PostVars)), + + Req = cowboy_req:reply(200, Req1), + {ok, Req, State} + end, + #{<<"method">> => <<"post">>, + <<"path">> => <<"username/${username}/" + "clientid/${clientid}/" + "peerhost/${peerhost}/" + "proto_name/${proto_name}/" + "mountpoint/${mountpoint}/" + "topic/${topic}/" + "action/${action}">>, + <<"body">> => #{<<"username">> => <<"${username}">>, + <<"clientid">> => <<"${clientid}">>, + <<"peerhost">> => <<"${peerhost}">>, + <<"proto_name">> => <<"${proto_name}">>, + <<"mountpoint">> => <<"${mountpoint}">>, + <<"topic">> => <<"${topic}">>, + <<"action">> => <<"${action}">>}, + <<"headers">> => #{<<"content-type">> => <<"application/x-www-form-urlencoded">>} + }), + + ClientInfo = #{clientid => <<"client id">>, + username => <<"user name">>, + peerhost => {127,0,0,1}, + protocol => <<"MQTT">>, + mountpoint => <<"MOUNTPOINT">>, + zone => default, + listener => {tcp, default} + }, + + ?assertEqual( + allow, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)). + + +t_create_replace(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + %% Bad URL + ok = setup_handler_and_config( + fun(Req0, State) -> + Req = cowboy_req:reply(200, Req0), + {ok, Req, State} + end, + #{<<"base_url">> => <<"http://127.0.0.1:33331/authz">>}), + + + ?assertEqual( + deny, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)), + + %% Changing to other bad config does not work + BadConfig = maps:merge( + raw_http_authz_config(), + #{<<"base_url">> => <<"http://127.0.0.1:33332/authz">>}), + + ?assertMatch( + {error, _}, + emqx_authz:update({?CMD_REPLACE, http}, BadConfig)), + + ?assertEqual( + deny, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)), + + %% Changing to valid config + OkConfig = maps:merge( + raw_http_authz_config(), + #{<<"base_url">> => <<"http://127.0.0.1:33333/authz">>}), + + ?assertMatch( + {ok, _}, + emqx_authz:update({?CMD_REPLACE, http}, OkConfig)), + + ?assertEqual( + allow, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)). + +%%------------------------------------------------------------------------------ +%% Helpers +%%------------------------------------------------------------------------------ + +raw_http_authz_config() -> + #{ + <<"enable">> => <<"true">>, + + <<"type">> => <<"http">>, + <<"method">> => <<"get">>, + <<"base_url">> => <<"http://127.0.0.1:33333/authz">>, + <<"path">> => <<"users/${username}/">>, + <<"query">> => <<"topic=${topic}&action=${action}">>, + <<"headers">> => #{<<"X-Test-Header">> => <<"Test Value">>} + }. + +setup_handler_and_config(Handler, Config) -> + ok = emqx_authz_http_test_server:set_handler(Handler), + ok = emqx_authz_test_lib:setup_config( + raw_http_authz_config(), + Config). + +start_apps(Apps) -> + lists:foreach(fun application:ensure_all_started/1, Apps). + +stop_apps(Apps) -> + lists:foreach(fun application:stop/1, Apps). diff --git a/apps/emqx_authz/test/emqx_authz_http_test_server.erl b/apps/emqx_authz/test/emqx_authz_http_test_server.erl new file mode 100644 index 000000000..19ead4627 --- /dev/null +++ b/apps/emqx_authz/test/emqx_authz_http_test_server.erl @@ -0,0 +1,89 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2020-2021 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_http_test_server). + +-behaviour(gen_server). +-behaviour(cowboy_handler). + +% cowboy_server callbacks +-export([init/2]). + +% gen_server callbacks +-export([init/1, + handle_call/3, + handle_cast/2 + ]). + +% API +-export([start/2, + stop/0, + set_handler/1 + ]). + +%%------------------------------------------------------------------------------ +%% API +%%------------------------------------------------------------------------------ + +start(Port, Path) -> + Dispatch = cowboy_router:compile([ + {'_', [{Path, ?MODULE, []}]} + ]), + {ok, _} = cowboy:start_clear(?MODULE, + [{port, Port}], + #{env => #{dispatch => Dispatch}} + ), + {ok, _} = gen_server:start_link({local, ?MODULE}, ?MODULE, [], []), + ok. + +stop() -> + gen_server:stop(?MODULE), + cowboy:stop_listener(?MODULE). + +set_handler(F) when is_function(F, 2) -> + gen_server:call(?MODULE, {set_handler, F}). + +%%------------------------------------------------------------------------------ +%% gen_server API +%%------------------------------------------------------------------------------ + +init([]) -> + F = fun(Req0, State) -> + Req = cowboy_req:reply( + 400, + #{<<"content-type">> => <<"text/plain">>}, + <<"">>, + Req0), + {ok, Req, State} + end, + {ok, F}. + +handle_cast(_, F) -> + {noreply, F}. + +handle_call({set_handler, F}, _From, _F) -> + {reply, ok, F}; + +handle_call(get_handler, _From, F) -> + {reply, F, F}. + +%%------------------------------------------------------------------------------ +%% cowboy_server API +%%------------------------------------------------------------------------------ + +init(Req, State) -> + Handler = gen_server:call(?MODULE, get_handler), + Handler(Req, State). From d75e0104cc78f2f1ab21c83ae43158de38f51934 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Mon, 20 Dec 2021 21:12:39 +0300 Subject: [PATCH 2/4] chore(authz): test file authz with real files --- apps/emqx_authz/src/emqx_authz_file.erl | 6 +- .../emqx_authz/test/emqx_authz_file_SUITE.erl | 130 ++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 apps/emqx_authz/test/emqx_authz_file_SUITE.erl diff --git a/apps/emqx_authz/src/emqx_authz_file.erl b/apps/emqx_authz/src/emqx_authz_file.erl index ba4f9c2b7..ad6f39573 100644 --- a/apps/emqx_authz/src/emqx_authz_file.erl +++ b/apps/emqx_authz/src/emqx_authz_file.erl @@ -55,7 +55,11 @@ init(#{path := Path} = Source) -> destroy(_Source) -> ok. -dry_run(_Source) -> ok. +dry_run(#{path := Path}) -> + case file:consult(Path) of + {ok, _} -> ok; + {error, _} = Error -> Error + end. authorize(Client, PubSub, Topic, #{annotations := #{rules := Rules}}) -> emqx_authz_rule:matches(Client, PubSub, Topic, Rules). diff --git a/apps/emqx_authz/test/emqx_authz_file_SUITE.erl b/apps/emqx_authz/test/emqx_authz_file_SUITE.erl new file mode 100644 index 000000000..09c49545c --- /dev/null +++ b/apps/emqx_authz/test/emqx_authz_file_SUITE.erl @@ -0,0 +1,130 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2020-2021 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_file_SUITE). + +-compile(nowarn_export_all). +-compile(export_all). + +-include("emqx_authz.hrl"). +-include_lib("eunit/include/eunit.hrl"). +-include_lib("common_test/include/ct.hrl"). + +all() -> + emqx_common_test_helpers:all(?MODULE). + +groups() -> + []. + +init_per_suite(Config) -> + ok = emqx_common_test_helpers:start_apps( + [emqx_conf, emqx_authz], + fun set_special_configs/1), + Config. + +end_per_suite(_Config) -> + ok = emqx_authz_test_lib:restore_authorizers(), + ok = emqx_common_test_helpers:stop_apps([emqx_authz]). + +init_per_testcase(Config) -> + ok = emqx_authz_test_lib:reset_authorizers(), + Config. + +set_special_configs(emqx_authz) -> + ok = emqx_authz_test_lib:reset_authorizers(); + +set_special_configs(_) -> + ok. + +%%------------------------------------------------------------------------------ +%% Testcases +%%------------------------------------------------------------------------------ + +t_ok(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + ok = setup_rules([{allow, {user, "username"}, publish, ["t"]}]), + ok = setup_config(#{}), + + ?assertEqual( + allow, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)), + + ?assertEqual( + deny, + emqx_access_control:authorize(ClientInfo, subscribe, <<"t">>)). + +t_invalid_file(_Config) -> + ok = file:write_file(<<"acl.conf">>, <<"{{invalid term">>), + + ?assertMatch( + {error, {1, erl_parse, _}}, + emqx_authz:update(?CMD_REPLACE, [raw_file_authz_config()])). + +t_nonexistent_file(_Config) -> + ?assertEqual( + {error, enoent}, + emqx_authz:update(?CMD_REPLACE, + [maps:merge(raw_file_authz_config(), + #{<<"path">> => <<"nonexistent.conf">>}) + ])). + +t_update(_Config) -> + ok = setup_rules([{allow, {user, "username"}, publish, ["t"]}]), + ok = setup_config(#{}), + + ?assertMatch( + {error, _}, + emqx_authz:update( + {?CMD_REPLACE, file}, + maps:merge(raw_file_authz_config(), + #{<<"path">> => <<"nonexistent.conf">>}))), + + ?assertMatch( + {ok, _}, + emqx_authz:update( + {?CMD_REPLACE, file}, + raw_file_authz_config())). + +%%------------------------------------------------------------------------------ +%% Helpers +%%------------------------------------------------------------------------------ + +raw_file_authz_config() -> + #{ + <<"enable">> => <<"true">>, + + <<"type">> => <<"file">>, + <<"path">> => <<"acl.conf">> + }. + +setup_rules(Rules) -> + {ok, F} = file:open(<<"acl.conf">>, [write]), + lists:foreach( + fun(Rule) -> + io:format(F, "~p.~n", [Rule]) + end, + Rules), + ok = file:close(F). + +setup_config(SpecialParams) -> + emqx_authz_test_lib:setup_config( + raw_file_authz_config(), + SpecialParams). From 2bada0bab8dc27b35effa7bd2d51ddfdb33e4ef1 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Tue, 21 Dec 2021 14:08:13 +0300 Subject: [PATCH 3/4] chore(authz): test Mria authz --- apps/emqx_authz/src/emqx_authz_mnesia.erl | 35 +++- .../emqx_authz/test/emqx_authz_file_SUITE.erl | 2 +- .../test/emqx_authz_mnesia_SUITE.erl | 175 +++++++++++------- .../test/emqx_authz_mysql_SUITE.erl | 2 +- .../test/emqx_authz_postgresql_SUITE.erl | 2 +- .../test/emqx_authz_redis_SUITE.erl | 2 +- apps/emqx_authz/test/emqx_authz_test_lib.erl | 1 + 7 files changed, 139 insertions(+), 80 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_mnesia.erl b/apps/emqx_authz/src/emqx_authz_mnesia.erl index 2ce8215cd..851fe1522 100644 --- a/apps/emqx_authz/src/emqx_authz_mnesia.erl +++ b/apps/emqx_authz/src/emqx_authz_mnesia.erl @@ -114,18 +114,19 @@ authorize(#{username := Username, %% Management API %%-------------------------------------------------------------------- +-spec(init_tables() -> ok). init_tables() -> ok = mria_rlog:wait_for_shards([?ACL_SHARDED], infinity). -spec(store_rules(who(), rules()) -> ok). store_rules({username, Username}, Rules) -> - Record = #emqx_acl{who = {?ACL_TABLE_USERNAME, Username}, rules = Rules}, + Record = #emqx_acl{who = {?ACL_TABLE_USERNAME, Username}, rules = normalize_rules(Rules)}, mria:dirty_write(Record); store_rules({clientid, Clientid}, Rules) -> - Record = #emqx_acl{who = {?ACL_TABLE_CLIENTID, Clientid}, rules = Rules}, + Record = #emqx_acl{who = {?ACL_TABLE_CLIENTID, Clientid}, rules = normalize_rules(Rules)}, mria:dirty_write(Record); store_rules(all, Rules) -> - Record = #emqx_acl{who = ?ACL_TABLE_ALL, rules = Rules}, + Record = #emqx_acl{who = ?ACL_TABLE_ALL, rules = normalize_rules(Rules)}, mria:dirty_write(Record). -spec(purge_rules() -> ok). @@ -176,6 +177,29 @@ record_count() -> %% Internal functions %%-------------------------------------------------------------------- +normalize_rules(Rules) -> + lists:map(fun normalize_rule/1, Rules). + +normalize_rule({Permission, Action, Topic}) -> + {normalize_permission(Permission), + normalize_action(Action), + normalize_topic(Topic)}; +normalize_rule(Rule) -> + error({invalid_rule, Rule}). + +normalize_topic(Topic) when is_list(Topic) -> list_to_binary(Topic); +normalize_topic(Topic) when is_binary(Topic) -> Topic; +normalize_topic(Topic) -> error({invalid_rule_topic, Topic}). + +normalize_action(publish) -> publish; +normalize_action(subscribe) -> subscribe; +normalize_action(all) -> all; +normalize_action(Action) -> error({invalid_rule_action, Action}). + +normalize_permission(allow) -> allow; +normalize_permission(deny) -> deny; +normalize_permission(Permission) -> error({invalid_rule_permission, Permission}). + do_get_rules(Key) -> case mnesia:dirty_read(?ACL_TABLE, Key) of [#emqx_acl{rules = Rules}] -> {ok, Rules}; @@ -184,9 +208,8 @@ do_get_rules(Key) -> do_authorize(_Client, _PubSub, _Topic, []) -> nomatch; do_authorize(Client, PubSub, Topic, [ {Permission, Action, TopicFilter} | Tail]) -> - case emqx_authz_rule:match(Client, PubSub, Topic, - emqx_authz_rule:compile({Permission, all, Action, [TopicFilter]}) - ) of + Rule = emqx_authz_rule:compile({Permission, all, Action, [TopicFilter]}), + case emqx_authz_rule:match(Client, PubSub, Topic, Rule) of {matched, Permission} -> {matched, Permission}; nomatch -> do_authorize(Client, PubSub, Topic, Tail) end. diff --git a/apps/emqx_authz/test/emqx_authz_file_SUITE.erl b/apps/emqx_authz/test/emqx_authz_file_SUITE.erl index 09c49545c..8424c0939 100644 --- a/apps/emqx_authz/test/emqx_authz_file_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_file_SUITE.erl @@ -38,7 +38,7 @@ end_per_suite(_Config) -> ok = emqx_authz_test_lib:restore_authorizers(), ok = emqx_common_test_helpers:stop_apps([emqx_authz]). -init_per_testcase(Config) -> +init_per_testcase(_TestCase, Config) -> ok = emqx_authz_test_lib:reset_authorizers(), Config. diff --git a/apps/emqx_authz/test/emqx_authz_mnesia_SUITE.erl b/apps/emqx_authz/test/emqx_authz_mnesia_SUITE.erl index dd98f77d3..4d1dce248 100644 --- a/apps/emqx_authz/test/emqx_authz_mnesia_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_mnesia_SUITE.erl @@ -18,10 +18,8 @@ -compile(nowarn_export_all). -compile(export_all). --include("emqx_authz.hrl"). -include_lib("eunit/include/eunit.hrl"). -include_lib("common_test/include/ct.hrl"). --include_lib("emqx/include/emqx_placeholder.hrl"). all() -> emqx_common_test_helpers:all(?MODULE). @@ -31,86 +29,123 @@ groups() -> init_per_suite(Config) -> ok = emqx_common_test_helpers:start_apps( - [emqx_connector, emqx_conf, emqx_authz], - fun set_special_configs/1 - ), + [emqx_conf, emqx_authz], + fun set_special_configs/1), Config. end_per_suite(_Config) -> - {ok, _} = emqx:update_config( - [authorization], - #{<<"no_match">> => <<"allow">>, - <<"cache">> => #{<<"enable">> => <<"true">>}, - <<"sources">> => []}), - emqx_common_test_helpers:stop_apps([emqx_authz, emqx_conf]), - ok. + ok = emqx_authz_test_lib:restore_authorizers(), + ok = emqx_common_test_helpers:stop_apps([emqx_authz]). + +init_per_testcase(_TestCase, Config) -> + ok = emqx_authz_test_lib:reset_authorizers(), + ok = setup_config(), + Config. + +end_per_testcase(_TestCase, _Config) -> + ok = emqx_authz_mnesia:purge_rules(). set_special_configs(emqx_authz) -> - {ok, _} = emqx:update_config([authorization, cache, enable], false), - {ok, _} = emqx:update_config([authorization, no_match], deny), - {ok, _} = emqx:update_config([authorization, sources], - [#{<<"type">> => <<"built-in-database">>}]), - ok; -set_special_configs(_App) -> + ok = emqx_authz_test_lib:reset_authorizers(); + +set_special_configs(_) -> ok. -init_per_testcase(t_authz, Config) -> - emqx_authz_mnesia:store_rules( - {username, <<"test_username">>}, - [{allow, publish, <<"test/", ?PH_S_USERNAME>>}, - {allow, subscribe, <<"eq #">>}]), - - emqx_authz_mnesia:store_rules( - {clientid, <<"test_clientid">>}, - [{allow, publish, <<"test/", ?PH_S_CLIENTID>>}, - {deny, subscribe, <<"eq #">>}]), - - emqx_authz_mnesia:store_rules( - all, - [{deny, all, <<"#">>}]), - - Config; -init_per_testcase(_, Config) -> Config. - -end_per_testcase(t_authz, Config) -> - ok = emqx_authz_mnesia:purge_rules(), - Config; -end_per_testcase(_, Config) -> Config. - %%------------------------------------------------------------------------------ %% Testcases %%------------------------------------------------------------------------------ +t_username_topic_rules(_Config) -> + ok = test_topic_rules(username). -t_authz(_) -> - ClientInfo1 = #{clientid => <<"test">>, - username => <<"test">>, - peerhost => {127,0,0,1}, - listener => {tcp, default} - }, - ClientInfo2 = #{clientid => <<"fake_clientid">>, - username => <<"test_username">>, - peerhost => {127,0,0,1}, - listener => {tcp, default} - }, - ClientInfo3 = #{clientid => <<"test_clientid">>, - username => <<"fake_username">>, - peerhost => {127,0,0,1}, - listener => {tcp, default} - }, +t_clientid_topic_rules(_Config) -> + ok = test_topic_rules(clientid). - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo1, subscribe, <<"#">>)), - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo1, publish, <<"#">>)), +t_all_topic_rules(_Config) -> + ok = test_topic_rules(all). - ?assertEqual(allow, emqx_access_control:authorize( - ClientInfo2, publish, <<"test/test_username">>)), - ?assertEqual(allow, emqx_access_control:authorize( - ClientInfo2, subscribe, <<"#">>)), +test_topic_rules(Key) -> + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, - ?assertEqual(allow, emqx_access_control:authorize( - ClientInfo3, publish, <<"test/test_clientid">>)), - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo3, subscribe, <<"#">>)), + SetupSamples = fun(CInfo, Samples) -> + setup_client_samples(CInfo, Samples, Key) + end, - ok. + ok = emqx_authz_test_lib:test_no_topic_rules(ClientInfo, SetupSamples), + + ok = emqx_authz_test_lib:test_allow_topic_rules(ClientInfo, SetupSamples), + + ok = emqx_authz_test_lib:test_deny_topic_rules(ClientInfo, SetupSamples). + +t_normalize_rules(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + ok = emqx_authz_mnesia:store_rules( + {username, <<"username">>}, + [{allow, publish, "t"}]), + + ?assertEqual( + allow, + emqx_access_control:authorize(ClientInfo, publish, <<"t">>)), + + ?assertException( + error, + {invalid_rule, _}, + emqx_authz_mnesia:store_rules( + {username, <<"username">>}, + [[allow, publish, <<"t">>]])), + + ?assertException( + error, + {invalid_rule_action, _}, + emqx_authz_mnesia:store_rules( + {username, <<"username">>}, + [{allow, pub, <<"t">>}])), + + ?assertException( + error, + {invalid_rule_permission, _}, + emqx_authz_mnesia:store_rules( + {username, <<"username">>}, + [{accept, publish, <<"t">>}])). + +%%------------------------------------------------------------------------------ +%% Helpers +%%------------------------------------------------------------------------------ + +raw_mnesia_authz_config() -> + #{ + <<"enable">> => <<"true">>, + <<"type">> => <<"built-in-database">> + }. + +setup_client_samples(ClientInfo, Samples, Key) -> + ok = emqx_authz_mnesia:purge_rules(), + Rules = lists:flatmap( + fun(#{topics := Topics, permission := Permission, action := Action}) -> + lists:map( + fun(Topic) -> + {binary_to_atom(Permission), binary_to_atom(Action), Topic} + end, + Topics) + end, + Samples), + #{username := Username, clientid := ClientId} = ClientInfo, + Who = case Key of + username -> {username, Username}; + clientid -> {clientid, ClientId}; + all -> all + end, + ok = emqx_authz_mnesia:store_rules(Who, Rules). + +setup_config() -> + emqx_authz_test_lib:setup_config(raw_mnesia_authz_config(), #{}). diff --git a/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl b/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl index 7db042dd8..853d78fdf 100644 --- a/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl @@ -56,7 +56,7 @@ end_per_suite(_Config) -> ok = stop_apps([emqx_resource, emqx_connector]), ok = emqx_common_test_helpers:stop_apps([emqx_authz]). -init_per_testcase(Config) -> +init_per_testcase(_TestCase, Config) -> ok = emqx_authz_test_lib:reset_authorizers(), Config. diff --git a/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl b/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl index 92c479f92..cda4bb447 100644 --- a/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl @@ -56,7 +56,7 @@ end_per_suite(_Config) -> ok = stop_apps([emqx_resource, emqx_connector]), ok = emqx_common_test_helpers:stop_apps([emqx_authz]). -init_per_testcase(Config) -> +init_per_testcase(_TestCase, Config) -> ok = emqx_authz_test_lib:reset_authorizers(), Config. diff --git a/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl b/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl index 93044e044..59d6e653d 100644 --- a/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl @@ -57,7 +57,7 @@ end_per_suite(_Config) -> ok = stop_apps([emqx_resource, emqx_connector]), ok = emqx_common_test_helpers:stop_apps([emqx_authz]). -init_per_testcase(Config) -> +init_per_testcase(_TestCase, Config) -> ok = emqx_authz_test_lib:reset_authorizers(), Config. diff --git a/apps/emqx_authz/test/emqx_authz_test_lib.erl b/apps/emqx_authz/test/emqx_authz_test_lib.erl index 68686837d..9c186ec4d 100644 --- a/apps/emqx_authz/test/emqx_authz_test_lib.erl +++ b/apps/emqx_authz/test/emqx_authz_test_lib.erl @@ -70,6 +70,7 @@ test_samples(ClientInfo, Samples) -> test_no_topic_rules(ClientInfo, SetupSamples) -> %% No rules + ok = reset_authorizers(deny, false), ok = SetupSamples(ClientInfo, []), ok = test_samples( From 9363b6110ea16e704b167549721c0436a95b2dd7 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Fri, 24 Dec 2021 16:17:49 +0300 Subject: [PATCH 4/4] chore(authz): make test http server more robust --- .../emqx_authz/test/emqx_authz_http_SUITE.erl | 2 +- .../test/emqx_authz_http_test_server.erl | 75 +++++++++---------- 2 files changed, 37 insertions(+), 40 deletions(-) diff --git a/apps/emqx_authz/test/emqx_authz_http_SUITE.erl b/apps/emqx_authz/test/emqx_authz_http_SUITE.erl index 7196d75ff..4e303bed7 100644 --- a/apps/emqx_authz/test/emqx_authz_http_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_http_SUITE.erl @@ -50,7 +50,7 @@ set_special_configs(_) -> init_per_testcase(_Case, Config) -> ok = emqx_authz_test_lib:reset_authorizers(), - ok = emqx_authz_http_test_server:start(?HTTP_PORT, ?HTTP_PATH), + {ok, _} = emqx_authz_http_test_server:start_link(?HTTP_PORT, ?HTTP_PATH), Config. end_per_testcase(_Case, _Config) -> diff --git a/apps/emqx_authz/test/emqx_authz_http_test_server.erl b/apps/emqx_authz/test/emqx_authz_http_test_server.erl index 19ead4627..5cab1397e 100644 --- a/apps/emqx_authz/test/emqx_authz_http_test_server.erl +++ b/apps/emqx_authz/test/emqx_authz_http_test_server.erl @@ -16,20 +16,17 @@ -module(emqx_authz_http_test_server). --behaviour(gen_server). +-behaviour(supervisor). -behaviour(cowboy_handler). % cowboy_server callbacks -export([init/2]). -% gen_server callbacks --export([init/1, - handle_call/3, - handle_cast/2 - ]). +% supervisor callbacks +-export([init/1]). % API --export([start/2, +-export([start_link/2, stop/0, set_handler/1 ]). @@ -38,52 +35,52 @@ %% API %%------------------------------------------------------------------------------ -start(Port, Path) -> - Dispatch = cowboy_router:compile([ - {'_', [{Path, ?MODULE, []}]} - ]), - {ok, _} = cowboy:start_clear(?MODULE, - [{port, Port}], - #{env => #{dispatch => Dispatch}} - ), - {ok, _} = gen_server:start_link({local, ?MODULE}, ?MODULE, [], []), - ok. +start_link(Port, Path) -> + supervisor:start_link({local, ?MODULE}, ?MODULE, [Port, Path]). stop() -> - gen_server:stop(?MODULE), - cowboy:stop_listener(?MODULE). + gen_server:stop(?MODULE). set_handler(F) when is_function(F, 2) -> - gen_server:call(?MODULE, {set_handler, F}). + true = ets:insert(?MODULE, {handler, F}), + ok. %%------------------------------------------------------------------------------ -%% gen_server API +%% supervisor API %%------------------------------------------------------------------------------ -init([]) -> - F = fun(Req0, State) -> - Req = cowboy_req:reply( - 400, - #{<<"content-type">> => <<"text/plain">>}, - <<"">>, - Req0), - {ok, Req, State} - end, - {ok, F}. +init([Port, Path]) -> + Dispatch = cowboy_router:compile( + [ + {'_', [{Path, ?MODULE, []}]} + ]), + TransOpts = #{socket_opts => [{port, Port}], + connection_type => supervisor}, + ProtoOpts = #{env => #{dispatch => Dispatch}}, -handle_cast(_, F) -> - {noreply, F}. + Tab = ets:new(?MODULE, [set, named_table, public]), + ets:insert(Tab, {handler, fun default_handler/2}), -handle_call({set_handler, F}, _From, _F) -> - {reply, ok, F}; - -handle_call(get_handler, _From, F) -> - {reply, F, F}. + ChildSpec = ranch:child_spec(?MODULE, ranch_tcp, TransOpts, cowboy_clear, ProtoOpts), + {ok, {{one_for_one, 10, 10}, [ChildSpec]}}. %%------------------------------------------------------------------------------ %% cowboy_server API %%------------------------------------------------------------------------------ init(Req, State) -> - Handler = gen_server:call(?MODULE, get_handler), + [{handler, Handler}] = ets:lookup(?MODULE, handler), Handler(Req, State). + +%%------------------------------------------------------------------------------ +%% Internal functions +%%------------------------------------------------------------------------------ + +default_handler(Req0, State) -> + Req = cowboy_req:reply( + 400, + #{<<"content-type">> => <<"text/plain">>}, + <<"">>, + Req0), + {ok, Req, State}. +