From e0fcf07cf908cd0fd74b65fa8548e284779048d0 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 2 Sep 2022 16:57:43 -0300 Subject: [PATCH] fix: do not publish last will when authentication failed fixes #8886 --- CHANGES-5.0.md | 1 + apps/emqx/src/emqx_channel.erl | 14 ++- apps/emqx_authn/test/emqx_authn_SUITE.erl | 117 ++++++++++++++++++++++ 3 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 apps/emqx_authn/test/emqx_authn_SUITE.erl diff --git a/CHANGES-5.0.md b/CHANGES-5.0.md index e9a9282db..8b59bd0dc 100644 --- a/CHANGES-5.0.md +++ b/CHANGES-5.0.md @@ -4,6 +4,7 @@ * Fix exhook `client.authorize` never being execauted. [#8780](https://github.com/emqx/emqx/pull/8780) * Fix JWT plugin don't support non-integer timestamp claims. [#8867](https://github.com/emqx/emqx/pull/8867) +* Avoid publishing will message when client fails to auhtenticate. [#8887](https://github.com/emqx/emqx/pull/8887) ## Enhancements diff --git a/apps/emqx/src/emqx_channel.erl b/apps/emqx/src/emqx_channel.erl index eff03e8ed..686833c45 100644 --- a/apps/emqx/src/emqx_channel.erl +++ b/apps/emqx/src/emqx_channel.erl @@ -22,6 +22,8 @@ -include("logger.hrl"). -include("types.hrl"). +-include_lib("snabbkaffe/include/snabbkaffe.hrl"). + -ifdef(TEST). -compile(export_all). -compile(nowarn_export_all). @@ -1423,7 +1425,8 @@ interval(will_timer, #channel{will_msg = WillMsg}) -> %%-------------------------------------------------------------------- -spec terminate(any(), channel()) -> ok. -terminate(_, #channel{conn_state = idle}) -> +terminate(_, #channel{conn_state = idle} = _Channel) -> + ?tp(channel_terminated, #{channel => _Channel}), ok; terminate(normal, Channel) -> run_terminate_hook(normal, Channel); @@ -1431,7 +1434,8 @@ terminate({shutdown, kicked}, Channel) -> run_terminate_hook(kicked, Channel); terminate({shutdown, Reason}, Channel) when Reason =:= discarded; - Reason =:= takenover + Reason =:= takenover; + Reason =:= not_authorized -> run_terminate_hook(Reason, Channel); terminate(Reason, Channel = #channel{will_msg = WillMsg}) -> @@ -1452,9 +1456,11 @@ persist_if_session(#channel{session = Session} = Channel) -> ok end. -run_terminate_hook(_Reason, #channel{session = undefined}) -> +run_terminate_hook(_Reason, #channel{session = undefined} = _Channel) -> + ?tp(channel_terminated, #{channel => _Channel}), ok; -run_terminate_hook(Reason, #channel{clientinfo = ClientInfo, session = Session}) -> +run_terminate_hook(Reason, #channel{clientinfo = ClientInfo, session = Session} = _Channel) -> + ?tp(channel_terminated, #{channel => _Channel}), emqx_session:terminate(ClientInfo, Reason, Session). %%-------------------------------------------------------------------- diff --git a/apps/emqx_authn/test/emqx_authn_SUITE.erl b/apps/emqx_authn/test/emqx_authn_SUITE.erl new file mode 100644 index 000000000..4c07489c8 --- /dev/null +++ b/apps/emqx_authn/test/emqx_authn_SUITE.erl @@ -0,0 +1,117 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2022 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_authn_SUITE). + +-compile(export_all). +-compile(nowarn_export_all). + +-include_lib("eunit/include/eunit.hrl"). +-include_lib("common_test/include/ct.hrl"). +-include_lib("snabbkaffe/include/snabbkaffe.hrl"). + +-include_lib("emqx/include/emqx.hrl"). +-include_lib("emqx/include/emqx_mqtt.hrl"). + +%%================================================================================= +%% CT boilerplate +%%================================================================================= + +all() -> + emqx_common_test_helpers:all(?MODULE). + +init_per_suite(Config) -> + Config. + +end_per_suite(_Config) -> + ok. + +init_per_testcase(Case, Config) -> + ?MODULE:Case({init, Config}). + +end_per_testcase(Case, Config) -> + ?MODULE:Case({'end', Config}). + +%%================================================================================= +%% Helpers fns +%%================================================================================= + +%%================================================================================= +%% Testcases +%%================================================================================= + +t_will_message_connection_denied({init, Config}) -> + emqx_common_test_helpers:start_apps([emqx_conf, emqx_authn]), + mria:clear_table(emqx_authn_mnesia), + AuthnConfig = #{ + <<"mechanism">> => <<"password_based">>, + <<"backend">> => <<"built_in_database">>, + <<"user_id_type">> => <<"clientid">> + }, + Chain = 'mqtt:global', + emqx:update_config( + [authentication], + {create_authenticator, Chain, AuthnConfig} + ), + User = #{user_id => <<"subscriber">>, password => <<"p">>}, + AuthenticatorID = <<"password_based:built_in_database">>, + {ok, _} = emqx_authentication:add_user( + Chain, + AuthenticatorID, + User + ), + Config; +t_will_message_connection_denied({'end', _Config}) -> + emqx:update_config( + [authentication], + {delete_authenticator, 'mqtt:global', <<"password_based:built_in_database">>} + ), + emqx_common_test_helpers:stop_apps([emqx_authn, emqx_conf]), + mria:clear_table(emqx_authn_mnesia), + ok; +t_will_message_connection_denied(Config) when is_list(Config) -> + {ok, Subscriber} = emqtt:start_link([ + {clientid, <<"subscriber">>}, + {password, <<"p">>} + ]), + {ok, _} = emqtt:connect(Subscriber), + {ok, _, [?RC_SUCCESS]} = emqtt:subscribe(Subscriber, <<"lwt">>), + + process_flag(trap_exit, true), + + {ok, Publisher} = emqtt:start_link([ + {clientid, <<"publisher">>}, + {will_topic, <<"lwt">>}, + {will_payload, <<"should not be published">>} + ]), + snabbkaffe:start_trace(), + ?wait_async_action( + {error, _} = emqtt:connect(Publisher), + #{?snk_kind := channel_terminated} + ), + snabbkaffe:stop(), + + receive + {publish, #{ + topic := <<"lwt">>, + payload := <<"should not be published">> + }} -> + ct:fail("should not publish will message") + after 0 -> + ok + end, + + ok.