From 1ba8ad4c25d4f66f00a72e190a3bd56f3aa37271 Mon Sep 17 00:00:00 2001
From: Thales Macedo Garitezi <thalesmg@gmail.com>
Date: Fri, 10 Jun 2022 15:00:57 -0300
Subject: [PATCH 1/2] fix(metrics): inc `connack.auth_error` when using MQTT
 3.1

Since MQTT 3.1 uses a different reason code for auth failures, it was
failing to increase the corresponding metric that works for MQTT 5.0.
---
 src/emqx_metrics.erl       |  8 ++++++--
 test/emqx_broker_SUITE.erl | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/src/emqx_metrics.erl b/src/emqx_metrics.erl
index 7bec83465..c919e25eb 100644
--- a/src/emqx_metrics.erl
+++ b/src/emqx_metrics.erl
@@ -429,8 +429,12 @@ inc_sent(Packet) ->
 
 do_inc_sent(?CONNACK_PACKET(ReasonCode)) ->
     (ReasonCode == ?RC_SUCCESS) orelse inc('packets.connack.error'),
-    (ReasonCode == ?RC_NOT_AUTHORIZED) andalso inc('packets.connack.auth_error'),
-    (ReasonCode == ?RC_BAD_USER_NAME_OR_PASSWORD) andalso inc('packets.connack.auth_error'),
+    ((ReasonCode == ?RC_NOT_AUTHORIZED)
+     orelse (ReasonCode == ?CONNACK_AUTH))
+        andalso inc('packets.connack.auth_error'),
+    ((ReasonCode == ?RC_BAD_USER_NAME_OR_PASSWORD)
+     orelse (ReasonCode == ?CONNACK_CREDENTIALS))
+        andalso inc('packets.connack.auth_error'),
     inc('packets.connack.sent');
 
 do_inc_sent(?PUBLISH_PACKET(QoS)) ->
diff --git a/test/emqx_broker_SUITE.erl b/test/emqx_broker_SUITE.erl
index e8d19c5c0..ec46ff8e1 100644
--- a/test/emqx_broker_SUITE.erl
+++ b/test/emqx_broker_SUITE.erl
@@ -277,6 +277,38 @@ t_stats_fun({'end', _Config}) ->
     ok = emqx_broker:unsubscribe(<<"topic">>),
     ok = emqx_broker:unsubscribe(<<"topic2">>).
 
+t_connack_auth_error({init, Config}) ->
+    process_flag(trap_exit, true),
+    emqx_ct_helpers:stop_apps([]),
+    emqx_ct_helpers:boot_modules(all),
+    Handler =
+        fun(emqx) ->
+                application:set_env(emqx, acl_nomatch, deny),
+                application:set_env(emqx, allow_anonymous, false),
+                application:set_env(emqx, enable_acl_cache, false),
+                ok;
+           (_) ->
+                ok
+        end,
+    emqx_ct_helpers:start_apps([], Handler),
+    Config;
+t_connack_auth_error({'end', _Config}) ->
+    emqx_ct_helpers:stop_apps([]),
+    emqx_ct_helpers:boot_modules(all),
+    emqx_ct_helpers:start_apps([]),
+    ok;
+t_connack_auth_error(Config) when is_list(Config) ->
+    %% MQTT 3.1
+    ?assertEqual(0, emqx_metrics:val('packets.connack.auth_error')),
+    {ok, C0} = emqtt:start_link([{proto_ver, v4}]),
+    ?assertEqual({error, {unauthorized_client, undefined}}, emqtt:connect(C0)),
+    ?assertEqual(1, emqx_metrics:val('packets.connack.auth_error')),
+    %% MQTT 5.0
+    {ok, C1} = emqtt:start_link([{proto_ver, v5}]),
+    ?assertEqual({error, {not_authorized, #{}}}, emqtt:connect(C1)),
+    ?assertEqual(2, emqx_metrics:val('packets.connack.auth_error')),
+    ok.
+
 recv_msgs(Count) ->
     recv_msgs(Count, []).
 

From 1733f19608ebcdfd7f59c409f945f3ef987bc9c5 Mon Sep 17 00:00:00 2001
From: Thales Macedo Garitezi <thalesmg@gmail.com>
Date: Fri, 10 Jun 2022 16:15:17 -0300
Subject: [PATCH 2/2] chore: bump version and update changelog

---
 CHANGES-4.3.md     | 5 +++++
 src/emqx.app.src   | 2 +-
 src/emqx.appup.src | 6 ++++--
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/CHANGES-4.3.md b/CHANGES-4.3.md
index 00cb8cf1e..3178d817e 100644
--- a/CHANGES-4.3.md
+++ b/CHANGES-4.3.md
@@ -18,6 +18,11 @@ File format:
   password-protected private key files used for dashboard and
   management HTTPS listeners. [#8129]
 
+### Bug-fixes
+
+- Correctly tally `connack.auth_error` metrics when a client uses MQTT
+  3.1. [#8177]
+
 ## v4.3.15
 
 ### Enhancements
diff --git a/src/emqx.app.src b/src/emqx.app.src
index c57bd635d..5b5017b1a 100644
--- a/src/emqx.app.src
+++ b/src/emqx.app.src
@@ -6,7 +6,7 @@
   %% the emqx `release' version, which in turn is comprised of several
   %% apps, one of which is this.  See `emqx_release.hrl' for more
   %% info.
-  {vsn, "4.3.16"}, % strict semver, bump manually!
+  {vsn, "4.3.17"}, % strict semver, bump manually!
   {modules, []},
   {registered, []},
   {applications, [ kernel
diff --git a/src/emqx.appup.src b/src/emqx.appup.src
index 844c7fb7a..49fc3abb3 100644
--- a/src/emqx.appup.src
+++ b/src/emqx.appup.src
@@ -1,7 +1,8 @@
 %% -*- mode: erlang -*-
 %% Unless you know what you are doing, DO NOT edit manually!!
 {VSN,
-  [{"4.3.15",
+  [{"4.3.16",[{load_module,emqx_metrics,brutal_purge,soft_purge,[]}]},
+   {"4.3.15",
     [{add_module,emqx_calendar},
      {load_module,emqx_logger_textfmt,brutal_purge,soft_purge,[]},
      {load_module,emqx_packet,brutal_purge,soft_purge,[]},
@@ -555,7 +556,8 @@
      {load_module,emqx_message,brutal_purge,soft_purge,[]},
      {load_module,emqx_limiter,brutal_purge,soft_purge,[]}]},
    {<<".*">>,[]}],
-  [{"4.3.15",
+  [{"4.3.16",[{load_module,emqx_metrics,brutal_purge,soft_purge,[]}]},
+   {"4.3.15",
     [{delete_module,emqx_calendar},
      {load_module,emqx_logger_textfmt,brutal_purge,soft_purge,[]},
      {load_module,emqx_packet,brutal_purge,soft_purge,[]},