Merge pull request #8894 from thalesmg/bugfix-will-msg-authn-43

test: add test case for not publishing will msg when not authorized

CHANGES-4.3.md

@@ -24,6 +24,8 @@ File format:
   `SELECT topic =~ 't' as r FROM "$events/client_connected"`.
   The topic is a null value as there's no such field in event `$events/client_connected`, so it
   should return false if match it to a topic.
+- Added a test to prevent a last will testament message to be
+  published when a client is denied connection. [#8894](https://github.com/emqx/emqx/pull/8894)
 
 ## v4.3.19
 

apps/emqx_auth_mnesia/test/emqx_auth_mnesia_SUITE.erl

@@ -21,6 +21,9 @@
 -include("emqx_auth_mnesia.hrl").
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("common_test/include/ct.hrl").
+-include_lib("snabbkaffe/include/snabbkaffe.hrl").
+
+-include_lib("emqx/include/emqx_mqtt.hrl").
 
 -import(emqx_ct_http, [ request_api/3
                       , request_api/5
@@ -74,6 +77,20 @@ set_default(ClientId, UserName, Pwd, HashType) ->
     application:set_env(emqx_auth_mnesia, username_list, [{UserName, Pwd}]),
     application:set_env(emqx_auth_mnesia, password_hash, HashType),
     ok.
+
+init_per_testcase(t_will_message_connection_denied, Config) ->
+    emqx_zone:set_env(external, allow_anonymous, false),
+    Config;
+init_per_testcase(_TestCase, Config) ->
+    Config.
+
+end_per_testcase(t_will_message_connection_denied, _Config) ->
+    emqx_zone:unset_env(external, allow_anonymous),
+    application:stop(emqx_auth_mnesia),
+    ok;
+end_per_testcase(_TestCase, _Config) ->
+    ok.
+
 %%------------------------------------------------------------------------------
 %% Testcases
 %%------------------------------------------------------------------------------
@@ -390,6 +407,48 @@ t_password_hash(_) ->
     application:stop(emqx_auth_mnesia),
     ok = application:start(emqx_auth_mnesia).
 
+t_will_message_connection_denied(Config) when is_list(Config) ->
+    ClientId = Username = <<"subscriber">>,
+    Password = <<"p">>,
+    application:stop(emqx_auth_mnesia),
+    ok = emqx_ct_helpers:start_apps([emqx_auth_mnesia]),
+    ok = emqx_auth_mnesia_cli:add_user({clientid, ClientId}, Password),
+
+    {ok, Subscriber} = emqtt:start_link([
+        {clientid, ClientId},
+        {password, Password}
+    ]),
+    {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(),
+
+    timer:sleep(1000),
+
+    receive
+        {publish, #{
+            topic := <<"lwt">>,
+            payload := <<"should not be published">>
+        }} ->
+            ct:fail("should not publish will message")
+    after 0 ->
+        ok
+    end,
+
+    ok.
+
 %%------------------------------------------------------------------------------
 %% Helpers
 %%------------------------------------------------------------------------------

src/emqx_channel.erl

@@ -1156,20 +1156,34 @@ interval(will_timer, #channel{will_msg = WillMsg}) ->
 %%--------------------------------------------------------------------
 
 -spec(terminate(any(), channel()) -> ok).
-terminate(_, #channel{conn_state = idle}) -> ok;
+terminate(_Reason, #channel{conn_state = idle} = _Channel) ->
+    ?tp(channel_terminated, #{channel => _Channel, reason => _Reason}),
+    ok;
 terminate(normal, Channel) ->
     run_terminate_hook(normal, Channel);
-terminate({shutdown, Reason}, Channel)
-  when Reason =:= kicked; Reason =:= discarded; Reason =:= takeovered ->
-    run_terminate_hook(Reason, Channel);
 terminate(Reason, Channel = #channel{will_msg = WillMsg}) ->
-    (WillMsg =/= undefined) andalso publish_will_msg(WillMsg),
+    should_publish_will_message(Reason, Channel)
+        andalso publish_will_msg(WillMsg),
     run_terminate_hook(Reason, Channel).
 
-run_terminate_hook(_Reason, #channel{session = undefined}) -> ok;
-run_terminate_hook(Reason, #channel{clientinfo = ClientInfo, session = Session}) ->
+run_terminate_hook(_Reason, #channel{session = undefined} = _Channel) ->
+    ?tp(channel_terminated, #{channel => _Channel, reason => _Reason}),
+    ok;
+run_terminate_hook(Reason, #channel{clientinfo = ClientInfo, session = Session} = _Channel) ->
+    ?tp(channel_terminated, #{channel => _Channel, reason => Reason}),
     emqx_session:terminate(ClientInfo, Reason, Session).
 
+should_publish_will_message(TerminateReason, Channel) ->
+    not lists:member(TerminateReason, [ {shutdown, kicked}
+                                      , {shutdown, discarded}
+                                      , {shutdown, takeovered}
+                                      , {shutdown, not_authorized}
+                                      ])
+        andalso not lists:member(info(conn_state, Channel), [ idle
+                                                            , connecting
+                                                            ])
+        andalso info(will_msg, Channel) =/= undefined.
+
 %%--------------------------------------------------------------------
 %% Internal functions
 %%--------------------------------------------------------------------