Return Assigned Client Identifier in CONNACK

"If the Client connects using a zero length Client Identifier, the Server
MUST respond with a CONNACK containing an Assigned Client Identifier."

deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl

@@ -135,7 +135,7 @@ process_connect(
     Result0 =
     maybe
         ok ?= check_protocol_version(ProtoVer),
-        {ok, ClientId} ?= ensure_client_id(ClientId0, CleanSess),
+        {ok, ClientId} ?= ensure_client_id(ClientId0, CleanSess, ProtoVer),
         {ok, {Username1, Password}} ?= check_credentials(Username0, Password0, SslLoginName, PeerIp),
 
         {VHostPickedUsing, {VHost, Username2}} = get_vhost(Username1, SslLoginName, Port),
@@ -186,19 +186,34 @@ process_connect(
              end,
     case Result of
         {ok, SessPresent, State = #state{}} ->
-            send_conn_ack(?RC_SUCCESS, SessPresent, ProtoVer, SendFun, MaxPacketSize),
+            Props0 = #{'Maximum-QoS' => ?QOS_1,
+                       'Maximum-Packet-Size' => persistent_term:get(
+                                                  ?PERSISTENT_TERM_MAX_PACKET_SIZE_AUTHENTICATED),
+                       'Subscription-Identifier-Available' => 0,
+                       'Shared-Subscription-Available' => 0
+                      },
+            Props = case {ClientId0, ProtoVer} of
+                        {<<>>, 5} ->
+                            %% "If the Client connects using a zero length Client Identifier, the Server
+                            %% MUST respond with a CONNACK containing an Assigned Client Identifier."
+                            maps:put('Assigned-Client-Identifier', State#state.cfg#cfg.client_id, Props0);
+                        _ ->
+                            Props0
+                    end,
+            send_conn_ack(?RC_SUCCESS, SessPresent, ProtoVer, SendFun, MaxPacketSize, Props),
             {ok, State};
         {error, ConnectReasonCode} = Err
           when is_integer(ConnectReasonCode) ->
             %% If a server sends a CONNACK packet containing a non-zero return
             %% code it MUST set Session Present to 0 [MQTT-3.2.2-4].
             SessPresent = false,
-            send_conn_ack(ConnectReasonCode, SessPresent, ProtoVer, SendFun, MaxPacketSize),
+            send_conn_ack(ConnectReasonCode, SessPresent, ProtoVer, SendFun, MaxPacketSize, #{}),
             Err
     end.
 
--spec send_conn_ack(reason_code(), boolean(), protocol_version(), send_fun(), max_packet_size()) -> ok.
+-spec send_conn_ack(reason_code(), boolean(), protocol_version(), send_fun(),
-send_conn_ack(ConnectReasonCode, SessPresent, ProtoVer, SendFun, MaxPacketSize) ->
+                    max_packet_size(), properties()) -> ok.
+send_conn_ack(ConnectReasonCode, SessPresent, ProtoVer, SendFun, MaxPacketSize, Props) ->
     Code = case ProtoVer of
                5 -> ConnectReasonCode;
                _ -> connect_reason_code_to_return_code(ConnectReasonCode)
@@ -206,7 +221,8 @@ send_conn_ack(ConnectReasonCode, SessPresent, ProtoVer, SendFun, MaxPacketSize)
     Packet = #mqtt_packet{fixed = #mqtt_packet_fixed{type = ?CONNACK},
                           variable = #mqtt_packet_connack{
                                         session_present = SessPresent,
-                                        code = Code}},
+                                        code = Code,
+                                        props = Props}},
     _ = send(Packet, ProtoVer, SendFun, MaxPacketSize),
     ok.
 
@@ -435,14 +451,17 @@ check_credentials(Username, Password, SslLoginName, PeerIp) ->
             {ok, {UserBin, PassBin}}
     end.
 
-ensure_client_id(<<>>, _CleanSess = false) ->
+-spec ensure_client_id(binary(), boolean(), protocol_version()) ->
-    ?LOG_ERROR("MQTT client ID must be provided for non-clean session"),
+    {ok, binary()} | {error, reason_code()}.
+ensure_client_id(<<>>, _CleanSess = false, ProtoVer)
+  when ProtoVer < 5 ->
+    ?LOG_ERROR("MQTT client ID must be provided for non-clean session in MQTT v~b", [ProtoVer]),
     {error, ?RC_CLIENT_IDENTIFIER_NOT_VALID};
-ensure_client_id(<<>>, _CleanSess = true) ->
+ensure_client_id(<<>>, _, _) ->
     {ok, rabbit_data_coercion:to_binary(
            rabbit_misc:base64url(
              rabbit_guid:gen_secure()))};
-ensure_client_id(ClientId, _CleanSess)
+ensure_client_id(ClientId, _, _)
   when is_binary(ClientId) ->
     {ok, ClientId}.
 

deps/rabbitmq_mqtt/test/shared_SUITE.erl

@@ -897,15 +897,28 @@ non_clean_sess_reconnect_qos0_and_qos1(Config) ->
     C3 = connect(ClientId, Config, [{clean_start, true}]),
     ok = emqtt:disconnect(C3).
 
-%% "If the Client supplies a zero-byte ClientId with CleanSession set to 0,
-%% the Server MUST respond to the CONNECT Packet with a CONNACK return code 0x02
-%% (Identifier rejected) and then close the Network Connection" [MQTT-3.1.3-8].
 non_clean_sess_empty_client_id(Config) ->
     {C, Connect} = util:start_client(<<>>, Config, 0, [{clean_start, false}]),
+    case ?config(mqtt_version, Config) of
+        V when V =:= v3;
+               V =:= v4 ->
+            %% "If the Client supplies a zero-byte ClientId with CleanSession set to 0,
+            %% the Server MUST respond to the CONNECT Packet with a CONNACK return code 0x02
+            %% (Identifier rejected) and then close the Network Connection" [MQTT-3.1.3-8].
             process_flag(trap_exit, true),
-    ?assertMatch({error, {client_identifier_not_valid, _}},
+            ?assertMatch({error, {client_identifier_not_valid, _}}, Connect(C)),
-                 Connect(C)),
+            ok = await_exit(C);
-    ok = await_exit(C).
+        v5 ->
+            %% "If the Client connects using a zero length Client Identifier, the Server MUST respond with
+            %% a CONNACK containing an Assigned Client Identifier. The Assigned Client Identifier MUST be
+            %% a new Client Identifier not used by any other Session currently in the Server [MQTT-3.2.2-16]."
+            {ok, #{'Assigned-Client-Identifier' := ClientId}} = Connect(C),
+            {C2, Connect2} = util:start_client(<<>>, Config, 0, [{clean_start, true}]),
+            {ok, #{'Assigned-Client-Identifier' := ClientId2}} = Connect2(C2),
+            ?assertNotEqual(ClientId, ClientId2),
+            ok = emqtt:disconnect(C),
+            ok = emqtt:disconnect(C2)
+    end.
 
 subscribe_same_topic_same_qos(Config) ->
     C = connect(?FUNCTION_NAME, Config),
@@ -1434,7 +1447,8 @@ max_packet_size_authenticated(Config) ->
     MaxSize = 500,
     ok = rpc(Config, persistent_term, put, [Key, MaxSize]),
 
-    C = connect(ClientId, Config),
+    {C, Connect} = util:start_client(ClientId, Config, 0, []),
+    {ok, ConnAckProps} = Connect(C),
     process_flag(trap_exit, true),
     ok = emqtt:publish(C, Topic, binary:copy(<<"x">>, MaxSize + 1), qos0),
     await_exit(C),
@@ -1442,6 +1456,7 @@ max_packet_size_authenticated(Config) ->
         V when V =:= v3; V =:= v4 ->
             ok;
         v5 ->
+            ?assertMatch(#{'Maximum-Packet-Size' := MaxSize}, ConnAckProps),
             receive {disconnected, _ReasonCodePacketTooLarge = 149, _Props} -> ok
             after 1000 -> ct:fail("missing DISCONNECT packet from server")
             end

deps/rabbitmq_mqtt/test/v5_SUITE.erl

@@ -15,6 +15,7 @@
 
 -import(util,
         [
+         start_client/4,
          connect/2, connect/3, connect/4
         ]).
 
@@ -124,7 +125,7 @@ client_set_max_packet_size_publish(Config) ->
     ok = emqtt:disconnect(C).
 
 client_set_max_packet_size_connack(Config) ->
-    {C, Connect} = util:start_client(?FUNCTION_NAME, Config, 0,
+    {C, Connect} = start_client(?FUNCTION_NAME, Config, 0,
                                 [{properties, #{'Maximum-Packet-Size' => 2}},
                                  {connect_timeout, 1}]),
     %% We expect the server to drop the CONNACK packet because it's larger than 2 bytes.
@@ -133,7 +134,7 @@ client_set_max_packet_size_connack(Config) ->
 %% "It is a Protocol Error to include the Receive Maximum
 %% value more than once or for it to have the value 0."
 client_set_max_packet_size_invalid(Config) ->
-    {C, Connect} = util:start_client(?FUNCTION_NAME, Config, 0,
+    {C, Connect} = start_client(?FUNCTION_NAME, Config, 0,
                                 [{properties, #{'Maximum-Packet-Size' => 0}}]),
     unlink(C),
     ?assertMatch({error, _}, Connect(C)).