Remove mqtt.default_user and mqtt.default_pass

This commit is a breaking change in RabbitMQ 4.0. ## What? Remove mqtt.default_user and mqtt.default_pass Instead, rabbit.anonymous_login_user and rabbit.anonymous_login_pass should be used. ## Why? RabbitMQ 4.0 simplifies anonymous logins. There should be a single configuration place ``` rabbit.anonymous_login_user rabbit.anonymous_login_pass ``` that is used for anonymous logins for any protocol. Anonymous login is orthogonal to the protocol the client uses. Hence, there should be a single configuration place which can then be used for MQTT, AMQP 1.0, AMQP 0.9.1, and RabbitMQ Stream protocol. This will also simplify switching to SASL for MQTT 5.0 in the future.
rabbitmq · Aug 15, 2024 · 01788bb · 01788bb
1 parent 5b23d41
commit 01788bb
Show file tree

Hide file tree

Showing 10 changed files with 58 additions and 100 deletions.
diff --git a/deps/rabbit/src/rabbit_auth_mechanism_anonymous.erl b/deps/rabbit/src/rabbit_auth_mechanism_anonymous.erl
@@ -9,6 +9,7 @@
 -behaviour(rabbit_auth_mechanism).
 
 -export([description/0, should_offer/1, init/1, handle_response/2]).
+-export([credentials/0]).
 
 -define(STATE, []).
 

diff --git a/deps/rabbitmq_mqtt/BUILD.bazel b/deps/rabbitmq_mqtt/BUILD.bazel
@@ -26,10 +26,7 @@ APP_DESCRIPTION = "RabbitMQ MQTT Adapter"
 APP_MODULE = "rabbit_mqtt"
 
 APP_ENV = """[
-	    {default_user, <<"guest">>},
-	    {default_pass, <<"guest">>},
 	    {ssl_cert_login,false},
-	    %% To satisfy an unfortunate expectation from popular MQTT clients.
 	    {allow_anonymous, true},
 	    {vhost, <<"/">>},
 	    {exchange, <<"amq.topic">>},

diff --git a/deps/rabbitmq_mqtt/Makefile b/deps/rabbitmq_mqtt/Makefile
@@ -4,10 +4,7 @@ PROJECT_MOD = rabbit_mqtt
 
 define PROJECT_ENV
 [
-	    {default_user, <<"guest">>},
-	    {default_pass, <<"guest">>},
 	    {ssl_cert_login,false},
-	    %% To satisfy an unfortunate expectation from popular MQTT clients.
 	    {allow_anonymous, true},
 	    {vhost, <<"/">>},
 	    {exchange, <<"amq.topic">>},

diff --git a/deps/rabbitmq_mqtt/priv/schema/rabbitmq_mqtt.schema b/deps/rabbitmq_mqtt/priv/schema/rabbitmq_mqtt.schema
@@ -6,35 +6,8 @@
 %% ----------------------------------------------------------------------------
 
 %  {rabbitmq_mqtt,
-%   [%% Set the default user name and password. Will be used as the default login
-%% if a connecting client provides no other login details.
-%%
-%% Please note that setting this will allow clients to connect without
-%% authenticating!
-%%
-%% {default_user, <<"guest">>},
-%% {default_pass, <<"guest">>},
-
-{mapping, "mqtt.default_user", "rabbitmq_mqtt.default_user", [
-    {datatype, string}
-]}.
-
-{mapping, "mqtt.default_pass", "rabbitmq_mqtt.default_pass", [
-    {datatype, string}
-]}.
-
-{translation, "rabbitmq_mqtt.default_user",
-fun(Conf) ->
-    list_to_binary(cuttlefish:conf_get("mqtt.default_user", Conf))
-end}.
-
-{translation, "rabbitmq_mqtt.default_pass",
-fun(Conf) ->
-    list_to_binary(cuttlefish:conf_get("mqtt.default_pass", Conf))
-end}.
-
-%% Enable anonymous access. If this is set to false, clients MUST provide
-%% login information in order to connect. See the default_user/default_pass
+%   [%% Enable anonymous access. If this is set to false, clients MUST provide
+%% login information in order to connect. See the anonymous_login_user/anonymous_login_pass
 %% configuration elements for managing logins without authentication.
 %%
 %% {allow_anonymous, true},

diff --git a/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl b/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl
@@ -183,7 +183,7 @@ process_connect(
     maybe
         ok ?= check_extended_auth(ConnectProps),
         {ok, ClientId} ?= ensure_client_id(ClientId0, CleanStart, ProtoVer),
-        {ok, {Username1, Password}} ?= check_credentials(Username0, Password0, SslLoginName, PeerIp),
+        {ok, Username1, Password} ?= check_credentials(Username0, Password0, SslLoginName, PeerIp),
 
         {VHostPickedUsing, {VHost, Username2}} = get_vhost(Username1, SslLoginName, Port),
         ?LOG_DEBUG("MQTT connection ~s picked vhost using ~s", [ConnName0, VHostPickedUsing]),
@@ -626,6 +626,8 @@ check_extended_auth(_) ->
 
 check_credentials(Username, Password, SslLoginName, PeerIp) ->
     case creds(Username, Password, SslLoginName) of
+        {ok, _, _} = Ok ->
+            Ok;
         nocreds ->
             ?LOG_ERROR("MQTT login failed: no credentials provided"),
             auth_attempt_failed(PeerIp, <<>>),
@@ -637,9 +639,7 @@ check_credentials(Username, Password, SslLoginName, PeerIp) ->
         {invalid_creds, {User, _Pass}} when is_binary(User) ->
             ?LOG_ERROR("MQTT login failed for user '~s': no password provided", [User]),
             auth_attempt_failed(PeerIp, User),
-            {error, ?RC_BAD_USER_NAME_OR_PASSWORD};
-        {UserBin, PassBin} ->
-            {ok, {UserBin, PassBin}}
+            {error, ?RC_BAD_USER_NAME_OR_PASSWORD}
     end.
 
 -spec ensure_client_id(client_id(), boolean(), protocol_version()) ->
@@ -1201,29 +1201,37 @@ get_vhost_from_port_mapping(Port, Mapping) ->
     Res.
 
 creds(User, Pass, SSLLoginName) ->
-    DefaultUser   = rabbit_mqtt_util:env(default_user),
-    DefaultPass   = rabbit_mqtt_util:env(default_pass),
-    {ok, Anon}    = application:get_env(?APP_NAME, allow_anonymous),
-    {ok, TLSAuth} = application:get_env(?APP_NAME, ssl_cert_login),
-    HaveDefaultCreds = Anon =:= true andalso
-        is_binary(DefaultUser) andalso
-        is_binary(DefaultPass),
-
     CredentialsProvided = User =/= undefined orelse Pass =/= undefined,
-    CorrectCredentials = is_binary(User) andalso is_binary(Pass) andalso Pass =/= <<>>,
+    ValidCredentials = is_binary(User) andalso is_binary(Pass) andalso Pass =/= <<>>,
+    {ok, TLSAuth} = application:get_env(?APP_NAME, ssl_cert_login),
     SSLLoginProvided = TLSAuth =:= true andalso SSLLoginName =/= none,
 
-    case {CredentialsProvided, CorrectCredentials, SSLLoginProvided, HaveDefaultCreds} of
-        %% Username and password take priority
-        {true, true, _, _}          -> {User, Pass};
-        %% Either username or password is provided
-        {true, false, _, _}         -> {invalid_creds, {User, Pass}};
-        %% rabbitmq_mqtt.ssl_cert_login is true. SSL user name provided.
-        %% Authenticating using username only.
-        {false, false, true, _}     -> {SSLLoginName, none};
-        %% Anonymous connection uses default credentials
-        {false, false, false, true} -> {DefaultUser, DefaultPass};
-        _                           -> nocreds
+    case {CredentialsProvided, ValidCredentials, SSLLoginProvided} of
+        {true, true, _} ->
+            %% Username and password take priority
+            {ok, User, Pass};
+        {true, false, _} ->
+            %% Either username or password is provided
+            {invalid_creds, {User, Pass}};
+        {false, false, true} ->
+            %% rabbitmq_mqtt.ssl_cert_login is true. SSL user name provided.
+            %% Authenticating using username only.
+            {ok, SSLLoginName, none};
+        {false, false, false} ->
+            {ok, AllowAnon} = application:get_env(?APP_NAME, allow_anonymous),
+            case AllowAnon of
+                true ->
+                    case rabbit_auth_mechanism_anonymous:credentials() of
+                        {ok, _, _} = Ok ->
+                            Ok;
+                        error ->
+                            nocreds
+                    end;
+                false ->
+                    nocreds
+            end;
+        _ ->
+            nocreds
     end.
 
 -spec auth_attempt_failed(inet:ip_address(), binary()) -> ok.

diff --git a/deps/rabbitmq_mqtt/src/rabbit_mqtt_util.erl b/deps/rabbitmq_mqtt/src/rabbit_mqtt_util.erl
@@ -141,10 +141,10 @@ env(Key) ->
         undefined -> undefined
     end.
 
-coerce_env_value(default_pass, Val) -> rabbit_data_coercion:to_binary(Val);
-coerce_env_value(default_user, Val) -> rabbit_data_coercion:to_binary(Val);
-coerce_env_value(vhost, Val)        -> rabbit_data_coercion:to_binary(Val);
-coerce_env_value(_, Val)            -> Val.
+coerce_env_value(vhost, Val) ->
+    rabbit_data_coercion:to_binary(Val);
+coerce_env_value(_, Val) ->
+    Val.
 
 -spec table_lookup(rabbit_framing:amqp_table() | undefined,  binary()) ->
     tuple() | undefined.

diff --git a/deps/rabbitmq_mqtt/test/auth_SUITE.erl b/deps/rabbitmq_mqtt/test/auth_SUITE.erl
@@ -123,15 +123,20 @@ init_per_group(authz, Config0) ->
     User = <<"mqtt-user">>,
     Password = <<"mqtt-password">>,
     VHost = <<"mqtt-vhost">>,
-    MqttConfig = {rabbitmq_mqtt, [{default_user, User}
-                                 ,{default_pass, Password}
-                                 ,{allow_anonymous, true}
-                                 ,{vhost, VHost}
-                                 ,{exchange, <<"amq.topic">>}
-                                 ]},
-    Config = rabbit_ct_helpers:run_setup_steps(rabbit_ct_helpers:merge_app_env(Config0, MqttConfig),
-                                               rabbit_ct_broker_helpers:setup_steps() ++
-                                               rabbit_ct_client_helpers:setup_steps()),
+    Env = [{rabbitmq_mqtt,
+            [{allow_anonymous, true},
+             {vhost, VHost},
+             {exchange, <<"amq.topic">>}
+            ]},
+           {rabbit,
+            [{anonymous_login_user, User},
+             {anonymous_login_pass, Password}
+            ]}],
+    Config1 = rabbit_ct_helpers:merge_app_env(Config0, Env),
+    Config = rabbit_ct_helpers:run_setup_steps(
+               Config1,
+               rabbit_ct_broker_helpers:setup_steps() ++
+               rabbit_ct_client_helpers:setup_steps()),
     rabbit_ct_broker_helpers:add_user(Config, User, Password),
     rabbit_ct_broker_helpers:add_vhost(Config, VHost),
     [Log|_] = rpc(Config, 0, rabbit, log_locations, []),
@@ -412,7 +417,6 @@ anonymous_auth_success(Config) ->
 anonymous_auth_failure(Config) ->
     expect_authentication_failure(fun connect_anonymous/1, Config).
 
-
 ssl_user_auth_success(Config) ->
     expect_successful_connection(fun connect_ssl/1, Config).
 

diff --git a/deps/rabbitmq_mqtt/test/config_schema_SUITE_data/rabbitmq_mqtt.snippets b/deps/rabbitmq_mqtt/test/config_schema_SUITE_data/rabbitmq_mqtt.snippets
@@ -1,7 +1,5 @@
 [{defaults,
   "listeners.tcp.default = 5672
-   mqtt.default_user     = guest
-   mqtt.default_pass     = guest
    mqtt.allow_anonymous  = true
    mqtt.vhost            = /
    mqtt.exchange         = amq.topic
@@ -20,9 +18,7 @@
    mqtt.topic_alias_maximum = 16",
   [{rabbit,[{tcp_listeners,[5672]}]},
    {rabbitmq_mqtt,
-       [{default_user,<<"guest">>},
-        {default_pass,<<"guest">>},
-        {allow_anonymous,true},
+       [{allow_anonymous,true},
         {vhost,<<"/">>},
         {exchange,<<"amq.topic">>},
         {max_session_expiry_interval_seconds,86400},
@@ -101,8 +97,6 @@
   [rabbitmq_mqtt]},
  {proxy_protocol,
   "listeners.tcp.default = 5672
-   mqtt.default_user     = guest
-   mqtt.default_pass     = guest
    mqtt.allow_anonymous  = true
    mqtt.vhost            = /
    mqtt.exchange         = amq.topic
@@ -111,19 +105,15 @@
    mqtt.proxy_protocol = true",
   [{rabbit,[{tcp_listeners,[5672]}]},
    {rabbitmq_mqtt,
-       [{default_user,<<"guest">>},
-        {default_pass,<<"guest">>},
-        {allow_anonymous,true},
+       [{allow_anonymous,true},
         {vhost,<<"/">>},
         {exchange,<<"amq.topic">>},
         {max_session_expiry_interval_seconds,infinity},
         {prefetch,10},
         {proxy_protocol,true}]}],
   [rabbitmq_mqtt]},
  {prefetch_retained_msg_store,
-  "mqtt.default_user     = guest
-   mqtt.default_pass     = guest
-   mqtt.allow_anonymous  = true
+  "mqtt.allow_anonymous  = true
    mqtt.vhost            = /
    mqtt.exchange         = amq.topic
    mqtt.max_session_expiry_interval_seconds = 1800
@@ -136,9 +126,7 @@
    mqtt.listeners.ssl = none
    mqtt.listeners.tcp.default = 1883",
   [{rabbitmq_mqtt,
-       [{default_user,<<"guest">>},
-        {default_pass,<<"guest">>},
-        {allow_anonymous,true},
+       [{allow_anonymous,true},
         {vhost,<<"/">>},
         {exchange,<<"amq.topic">>},
         {max_session_expiry_interval_seconds,1800},

diff --git a/deps/rabbitmq_mqtt/test/rabbitmq_mqtt.app b/deps/rabbitmq_mqtt/test/rabbitmq_mqtt.app
@@ -4,9 +4,7 @@
   {modules, []},
   {registered, []},
   {mod, {rabbit_mqtt, []}},
-  {env, [{default_user, "guest_user"},
-         {default_pass, "guest_pass"},
-         {ssl_cert_login,false},
+  {env, [{ssl_cert_login,false},
          {allow_anonymous, true},
          {vhost, "/"},
          {exchange, "amq.topic"},

diff --git a/deps/rabbitmq_mqtt/test/util_SUITE.erl b/deps/rabbitmq_mqtt/test/util_SUITE.erl
@@ -18,8 +18,6 @@ groups() ->
     [
      {tests, [parallel], [
                           coerce_vhost,
-                          coerce_default_user,
-                          coerce_default_pass,
                           mqtt_amqp_topic_translation
                          ]
      }
@@ -36,12 +34,6 @@ end_per_suite(Config) ->
 coerce_vhost(_) ->
     ?assertEqual(<<"/">>, rabbit_mqtt_util:env(vhost)).
 
-coerce_default_user(_) ->
-    ?assertEqual(<<"guest_user">>, rabbit_mqtt_util:env(default_user)).
-
-coerce_default_pass(_) ->
-    ?assertEqual(<<"guest_pass">>, rabbit_mqtt_util:env(default_pass)).
-
 mqtt_amqp_topic_translation(_) ->
     ok = application:set_env(rabbitmq_mqtt, sparkplug, true),
     ok = rabbit_mqtt_util:init_sparkplug(),