diff --git a/apps/emqx/src/emqx_banned.erl b/apps/emqx/src/emqx_banned.erl index 53a71736a..091afae8a 100644 --- a/apps/emqx/src/emqx_banned.erl +++ b/apps/emqx/src/emqx_banned.erl @@ -37,7 +37,6 @@ , info/1 , format/1 , parse/1 - , to_timestamp/1 ]). %% gen_server callbacks @@ -53,6 +52,11 @@ -define(BANNED_TAB, ?MODULE). +-ifdef(TEST). +-compile(export_all). +-compile(nowarn_export_all). +-endif. + %%-------------------------------------------------------------------- %% Mnesia bootstrap %%-------------------------------------------------------------------- @@ -106,32 +110,36 @@ format(#banned{who = Who0, }. parse(Params) -> - Who = pares_who(Params), - By = maps:get(<<"by">>, Params, <<"mgmt_api">>), - Reason = maps:get(<<"reason">>, Params, <<"">>), - At = parse_time(maps:get(<<"at">>, Params, undefined), erlang:system_time(second)), - Until = parse_time(maps:get(<<"until">>, Params, undefined), At + 5 * 60), - #banned{ - who = Who, - by = By, - reason = Reason, - at = At, - until = Until - }. - + case pares_who(Params) of + {error, Reason} -> {error, Reason}; + Who -> + By = maps:get(<<"by">>, Params, <<"mgmt_api">>), + Reason = maps:get(<<"reason">>, Params, <<"">>), + At = maps:get(<<"at">>, Params, erlang:system_time(second)), + Until = maps:get(<<"until">>, Params, At + 5 * 60), + case Until > erlang:system_time(second) of + true -> + #banned{ + who = Who, + by = By, + reason = Reason, + at = At, + until = Until + }; + false -> + {error, "already_expired"} + end + end. pares_who(#{as := As, who := Who}) -> pares_who(#{<<"as">> => As, <<"who">> => Who}); pares_who(#{<<"as">> := peerhost, <<"who">> := Peerhost0}) -> - {ok, Peerhost} = inet:parse_address(binary_to_list(Peerhost0)), - {peerhost, Peerhost}; + case inet:parse_address(binary_to_list(Peerhost0)) of + {ok, Peerhost} -> {peerhost, Peerhost}; + {error, einval} -> {error, "bad peerhost"} + end; pares_who(#{<<"as">> := As, <<"who">> := Who}) -> {As, Who}. -parse_time(undefined, Default) -> - Default; -parse_time(Rfc3339, _Default) -> - to_timestamp(Rfc3339). - maybe_format_host({peerhost, Host}) -> AddrBinary = list_to_binary(inet:ntoa(Host)), {peerhost, AddrBinary}; @@ -141,11 +149,6 @@ maybe_format_host({As, Who}) -> to_rfc3339(Timestamp) -> list_to_binary(calendar:system_time_to_rfc3339(Timestamp, [{unit, second}])). -to_timestamp(Rfc3339) when is_binary(Rfc3339) -> - to_timestamp(binary_to_list(Rfc3339)); -to_timestamp(Rfc3339) -> - calendar:rfc3339_to_system_time(Rfc3339, [{unit, second}]). - -spec(create(emqx_types:banned() | map()) -> {ok, emqx_types:banned()} | {error, {already_exist, emqx_types:banned()}}). create(#{who := Who, @@ -168,10 +171,11 @@ create(Banned = #banned{who = Who}) -> mria:dirty_write(?BANNED_TAB, Banned), {ok, Banned}; [OldBanned = #banned{until = Until}] -> - case Until < erlang:system_time(second) of - true -> - {error, {already_exist, OldBanned}}; - false -> + %% Don't support shorten or extend the until time by overwrite. + %% We don't support update api yet, user must delete then create new one. + case Until > erlang:system_time(second) of + true -> {error, {already_exist, OldBanned}}; + false -> %% overwrite expired one is ok. mria:dirty_write(?BANNED_TAB, Banned), {ok, Banned} end diff --git a/apps/emqx/test/emqx_banned_SUITE.erl b/apps/emqx/test/emqx_banned_SUITE.erl index e09d0baae..4ba45c5ad 100644 --- a/apps/emqx/test/emqx_banned_SUITE.erl +++ b/apps/emqx/test/emqx_banned_SUITE.erl @@ -39,9 +39,13 @@ t_add_delete(_) -> by = <<"banned suite">>, reason = <<"test">>, at = erlang:system_time(second), - until = erlang:system_time(second) + 1000 + until = erlang:system_time(second) + 1 }, {ok, _} = emqx_banned:create(Banned), + {error, {already_exist, Banned}} = emqx_banned:create(Banned), + ?assertEqual(1, emqx_banned:info(size)), + {error, {already_exist, Banned}} = + emqx_banned:create(Banned#banned{until = erlang:system_time(second) + 100}), ?assertEqual(1, emqx_banned:info(size)), ok = emqx_banned:delete({clientid, <<"TestClient">>}), @@ -68,10 +72,14 @@ t_check(_) -> username => <<"user">>, peerhost => {127,0,0,1} }, + ClientInfo5 = #{}, + ClientInfo6 = #{clientid => <<"client1">>}, ?assert(emqx_banned:check(ClientInfo1)), ?assert(emqx_banned:check(ClientInfo2)), ?assert(emqx_banned:check(ClientInfo3)), ?assertNot(emqx_banned:check(ClientInfo4)), + ?assertNot(emqx_banned:check(ClientInfo5)), + ?assertNot(emqx_banned:check(ClientInfo6)), ok = emqx_banned:delete({clientid, <<"BannedClient">>}), ok = emqx_banned:delete({username, <<"BannedUser">>}), ok = emqx_banned:delete({peerhost, {192,168,0,1}}), @@ -83,8 +91,10 @@ t_check(_) -> t_unused(_) -> {ok, Banned} = emqx_banned:start_link(), - {ok, _} = emqx_banned:create(#banned{who = {clientid, <<"BannedClient">>}, - until = erlang:system_time(second)}), + {ok, _} = emqx_banned:create(#banned{who = {clientid, <<"BannedClient1">>}, + until = erlang:system_time(second)}), + {ok, _} = emqx_banned:create(#banned{who = {clientid, <<"BannedClient2">>}, + until = erlang:system_time(second) - 1}), ?assertEqual(ignored, gen_server:call(Banned, unexpected_req)), ?assertEqual(ok, gen_server:cast(Banned, unexpected_msg)), ?assertEqual(ok, Banned ! ok), diff --git a/apps/emqx_management/src/emqx_mgmt_api_banned.erl b/apps/emqx_management/src/emqx_mgmt_api_banned.erl index 6521a549c..911b298a1 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_banned.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_banned.erl @@ -105,7 +105,7 @@ fields(ban) -> desc => <<"Client info as banned type">>, nullable => false, example => <<"Badass坏"/utf8>>})}, - {by, hoconsc:mk(binary(), #{ + {by, hoconsc:mk(emqx_schema:unicode_binary(), #{ desc => <<"Commander">>, nullable => true, example => <<"mgmt_api">>})}, @@ -113,15 +113,13 @@ fields(ban) -> desc => <<"Banned reason">>, nullable => true, example => <<"Too many requests">>})}, - {at, hoconsc:mk(binary(), #{ + {at, hoconsc:mk(emqx_schema:rfc3339_system_time(), #{ desc => <<"Create banned time, rfc3339, now if not specified">>, nullable => true, - validator => fun is_rfc3339/1, example => <<"2021-10-25T21:48:47+08:00">>})}, - {until, hoconsc:mk(binary(), #{ + {until, hoconsc:mk(emqx_schema:rfc3339_system_time(), #{ desc => <<"Cancel banned time, rfc3339, now + 5 minute if not specified">>, nullable => true, - validator => fun is_rfc3339/1, example => <<"2021-10-25T21:53:47+08:00">>}) } ]; @@ -130,22 +128,19 @@ fields(meta) -> emqx_dashboard_swagger:fields(limit) ++ [{count, hoconsc:mk(integer(), #{example => 1})}]. -is_rfc3339(Time) -> - try - emqx_banned:to_timestamp(Time), - ok - catch _:_ -> {error, Time} - end. - banned(get, #{query_string := Params}) -> Response = emqx_mgmt_api:paginate(?TAB, Params, ?FORMAT_FUN), {200, Response}; banned(post, #{body := Body}) -> - case emqx_banned:create(emqx_banned:parse(Body)) of - {ok, Banned} -> - {200, format(Banned)}; - {error, {already_exist, Old}} -> - {400, #{code => 'ALREADY_EXISTED', message => format(Old)}} + case emqx_banned:parse(Body) of + {error, Reason} -> + {400, #{code => 'PARAMS_ERROR', message => list_to_binary(Reason)}}; + Ban -> + case emqx_banned:create(Ban) of + {ok, Banned} -> {200, format(Banned)}; + {error, {already_exist, Old}} -> + {400, #{code => 'ALREADY_EXISTED', message => format(Old)}} + end end. delete_banned(delete, #{bindings := Params}) -> diff --git a/apps/emqx_management/test/emqx_mgmt_banned_api_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_banned_api_SUITE.erl new file mode 100644 index 000000000..d7cb87bb8 --- /dev/null +++ b/apps/emqx_management/test/emqx_mgmt_banned_api_SUITE.erl @@ -0,0 +1,144 @@ +%%-------------------------------------------------------------------- +%% 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_mgmt_banned_api_SUITE). + +-compile(export_all). +-compile(nowarn_export_all). + +-include_lib("eunit/include/eunit.hrl"). + +all() -> + emqx_common_test_helpers:all(?MODULE). + +init_per_suite(Config) -> + emqx_mgmt_api_test_util:init_suite(), + Config. + +end_per_suite(_) -> + emqx_mgmt_api_test_util:end_suite(). + +t_create(_Config) -> + Now = erlang:system_time(second), + At = emqx_banned:to_rfc3339(Now), + Until = emqx_banned:to_rfc3339(Now + 3), + ClientId = <<"TestClient测试"/utf8>>, + By = <<"banned suite测试组"/utf8>>, + Reason = <<"test测试"/utf8>>, + As = <<"clientid">>, + ClientIdBanned = #{ + as => As, + who => ClientId, + by => By, + reason => Reason, + at => At, + until => Until + }, + {ok, ClientIdBannedRes} = create_banned(ClientIdBanned), + ?assertEqual(#{<<"as">> => As, + <<"at">> => At, + <<"by">> => By, + <<"reason">> => Reason, + <<"until">> => Until, + <<"who">> => ClientId + }, ClientIdBannedRes), + PeerHost = <<"192.168.2.13">>, + PeerHostBanned = #{ + as => <<"peerhost">>, + who => PeerHost, + by => By, + reason => Reason, + at => At, + until => Until + }, + {ok, PeerHostBannedRes} = create_banned(PeerHostBanned), + ?assertEqual(#{<<"as">> => <<"peerhost">>, + <<"at">> => At, + <<"by">> => By, + <<"reason">> => Reason, + <<"until">> => Until, + <<"who">> => PeerHost + }, PeerHostBannedRes), + {ok, #{<<"data">> := List}} = list_banned(), + Bans = lists:sort(lists:map(fun(#{<<"who">> := W, <<"as">> := A}) -> {A, W} end, List)), + ?assertEqual([{<<"clientid">>, ClientId}, {<<"peerhost">>, PeerHost}], Bans), + ok. + +t_create_failed(_Config) -> + Now = erlang:system_time(second), + At = emqx_banned:to_rfc3339(Now), + Until = emqx_banned:to_rfc3339(Now + 10), + Who = <<"BadHost"/utf8>>, + By = <<"banned suite测试组"/utf8>>, + Reason = <<"test测试"/utf8>>, + As = <<"peerhost">>, + BadPeerHost = #{ + as => As, + who => Who, + by => By, + reason => Reason, + at => At, + until => Until + }, + BadRequest = {error, {"HTTP/1.1", 400, "Bad Request"}}, + ?assertEqual(BadRequest, create_banned(BadPeerHost)), + Expired = BadPeerHost#{until => emqx_banned:to_rfc3339(Now - 1), + who => <<"127.0.0.1">>}, + ?assertEqual(BadRequest, create_banned(Expired)), + ok. + +t_delete(_Config) -> + Now = erlang:system_time(second), + At = emqx_banned:to_rfc3339(Now), + Until = emqx_banned:to_rfc3339(Now + 3), + Who = <<"TestClient-"/utf8>>, + By = <<"banned suite 中"/utf8>>, + Reason = <<"test测试"/utf8>>, + As = <<"clientid">>, + Banned = #{ + as => clientid, + who => Who, + by => By, + reason => Reason, + at => At, + until => Until + }, + {ok, _} = create_banned(Banned), + ?assertMatch({ok, _}, delete_banned(binary_to_list(As), binary_to_list(Who))), + ?assertMatch({error,{"HTTP/1.1",404,"Not Found"}}, + delete_banned(binary_to_list(As), binary_to_list(Who))), + ok. + +list_banned() -> + Path = emqx_mgmt_api_test_util:api_path(["banned"]), + case emqx_mgmt_api_test_util:request_api(get, Path) of + {ok, Apps} -> {ok, emqx_json:decode(Apps, [return_maps])}; + Error -> Error + end. + +create_banned(Banned) -> + AuthHeader = emqx_mgmt_api_test_util:auth_header_(), + Path = emqx_mgmt_api_test_util:api_path(["banned"]), + case emqx_mgmt_api_test_util:request_api(post, Path, "", AuthHeader, Banned) of + {ok, Res} -> {ok, emqx_json:decode(Res, [return_maps])}; + Error -> Error + end. + +delete_banned(As, Who) -> + DeletePath = emqx_mgmt_api_test_util:api_path(["banned", As, Who]), + emqx_mgmt_api_test_util:request_api(delete, DeletePath). + +to_rfc3339(Sec) -> + list_to_binary(calendar:system_time_to_rfc3339(Sec)).