Merge pull request #7862 from zhongwencool/username-as-clientid-not-valid-v4.3

fix: don't allow empty username if username_as_clientid is true

CHANGES-4.3.md

@@ -18,6 +18,7 @@ File format:
 * Add support for JWT authorization [#7596]
   Now MQTT clients may be authorized with respect to a specific claim containing publish/subscribe topic whitelists.
 * Better randomisation of app screts (changed from timestamp seeded sha hash (uuid) to crypto:strong_rand_bytes)
+* Return a client_identifier_not_valid error when username is empty and username_as_clientid is set to true [#7862]
 
 ### Bug fixes
 * List subscription topic (/api/v4/subscriptions), the result do not match with multiple conditions.

etc/emqx_cloud/vm.args

@@ -115,4 +115,4 @@
 -shutdown_time 30000
 
 ## patches dir
--pa {{ platform_data_dir }}/patches
+-pa "{{ platform_data_dir }}/patches"

etc/emqx_edge/vm.args

@@ -113,4 +113,4 @@
 -shutdown_time 10000
 
 ## patches dir
--pa {{ platform_data_dir }}/patches
+-pa "{{ platform_data_dir }}/patches"

src/emqx.appup.src

@@ -3,10 +3,12 @@
 {VSN,
   [{"4.3.15",
     [{load_module,emqx_frame,brutal_purge,soft_purge,[]},
+     {load_module,emqx_channel,brutal_purge,soft_purge,[]},
      {load_module,emqx_access_rule,brutal_purge,soft_purge,[]},
      {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
    {"4.3.14",
     [{load_module,emqx_access_rule,brutal_purge,soft_purge,[]},
+    {load_module,emqx_channel,brutal_purge,soft_purge,[]},
      {load_module,emqx,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys,brutal_purge,soft_purge,[]},
      {load_module,emqx_plugins,brutal_purge,soft_purge,[]},
@@ -458,11 +460,13 @@
    {<<".*">>,[]}],
   [{"4.3.15",
     [{load_module,emqx_frame,brutal_purge,soft_purge,[]},
+     {load_module,emqx_channel,brutal_purge,soft_purge,[]},
      {load_module,emqx_access_rule,brutal_purge,soft_purge,[]},
      {load_module,emqx_app,brutal_purge,soft_purge,[]}]},
    {"4.3.14",
     [{load_module,emqx_access_rule,brutal_purge,soft_purge,[]},
      {load_module,emqx,brutal_purge,soft_purge,[]},
+     {load_module,emqx_channel,brutal_purge,soft_purge,[]},
      {load_module,emqx_sys,brutal_purge,soft_purge,[]},
      {load_module,emqx_plugins,brutal_purge,soft_purge,[]},
      {load_module,emqx_shared_sub,brutal_purge,soft_purge,[]},

src/emqx_channel.erl

@@ -1216,13 +1216,18 @@ check_connect(ConnPkt, #channel{clientinfo = #{zone := Zone}}) ->
 %% Enrich Client Info
 
 enrich_client(ConnPkt, Channel = #channel{clientinfo = ClientInfo}) ->
-    {ok, NConnPkt, NClientInfo} = pipeline([fun set_username/2,
+    Pipe = pipeline([fun set_username/2,
                      fun set_bridge_mode/2,
                      fun maybe_username_as_clientid/2,
                      fun maybe_assign_clientid/2,
                      fun fix_mountpoint/2
                     ], ConnPkt, ClientInfo),
-    {ok, NConnPkt, Channel#channel{clientinfo = NClientInfo}}.
+    case Pipe of
+        {ok, NConnPkt, NClientInfo} ->
+            {ok, NConnPkt, Channel#channel{clientinfo = NClientInfo}};
+        {error, ReasonCode, NClientInfo} ->
+            {error, ReasonCode, Channel#channel{clientinfo = NClientInfo}}
+    end.
 
 set_username(#mqtt_packet_connect{username = Username},
              ClientInfo = #{username := undefined}) ->
@@ -1237,7 +1242,8 @@ maybe_username_as_clientid(_ConnPkt, ClientInfo = #{username := undefined}) ->
     {ok, ClientInfo};
 maybe_username_as_clientid(_ConnPkt, ClientInfo = #{zone := Zone, username := Username}) ->
     case emqx_zone:use_username_as_clientid(Zone) of
-        true  -> {ok, ClientInfo#{clientid => Username}};
+        true when Username =/= <<>> -> {ok, ClientInfo#{clientid => Username}};
+        true -> {error, ?RC_CLIENT_IDENTIFIER_NOT_VALID, ClientInfo};
         false -> ok
     end.
 

test/emqx_client_SUITE.erl

@@ -278,9 +278,15 @@ t_username_as_clientid(_) ->
     {ok, C} = emqtt:start_link([{username, Username}]),
     {ok, _} = emqtt:connect(C),
     #{clientinfo := #{clientid := Username}} = emqx_cm:get_chan_info(Username),
-    emqtt:disconnect(C).
-
-
+    emqtt:disconnect(C),
+    erlang:process_flag(trap_exit, true),
+    {ok, C1} = emqtt:start_link([{username, <<>>}]),
+    ?assertEqual({error, {client_identifier_not_valid, undefined}}, emqtt:connect(C1)),
+    receive
+        {'EXIT', _, {shutdown, client_identifier_not_valid}} -> ok
+    after 100 ->
+        throw({error, "expect_client_identifier_not_valid"})
+    end.
 
 t_certcn_as_clientid_default_config_tls(_) ->
     tls_certcn_as_clientid(default).