From 10f1ea1bac513c44fa2f0d4c73b25808ee679087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20G=C3=B6m=C3=B6ri?= Date: Thu, 10 Jul 2025 00:37:26 +0200 Subject: [PATCH 1/2] Don't use Mnesia in rabbitmq_mqtt/test/processor_SUITE Soon Mnesia will be gone from RabbitMQ, so better make the test suite metadata store agnostic. --- deps/rabbitmq_mqtt/test/processor_SUITE.erl | 91 ++++++++++----------- 1 file changed, 42 insertions(+), 49 deletions(-) diff --git a/deps/rabbitmq_mqtt/test/processor_SUITE.erl b/deps/rabbitmq_mqtt/test/processor_SUITE.erl index 7e967325e0..8a679b764b 100644 --- a/deps/rabbitmq_mqtt/test/processor_SUITE.erl +++ b/deps/rabbitmq_mqtt/test/processor_SUITE.erl @@ -31,30 +31,15 @@ suite() -> init_per_suite(Config) -> ok = application:load(rabbitmq_mqtt), + meck:new(rabbit_runtime_parameters, [passthrough, no_link]), Config. end_per_suite(Config) -> ok = application:unload(rabbitmq_mqtt), + meck:unload(rabbit_runtime_parameters), Config. init_per_group(_, Config) -> Config. end_per_group(_, Config) -> Config. -init_per_testcase(get_vhost, Config) -> - mnesia:start(), - mnesia:create_table(rabbit_runtime_parameters, [ - {attributes, record_info(fields, runtime_parameters)}, - {record_name, runtime_parameters}]), - meck:new(rabbit_feature_flags, [passthrough, no_link]), - meck:expect( - rabbit_feature_flags, is_enabled, - fun - (khepri_db, _) -> false; - (FeatureNames, _) -> meck:passthrough([FeatureNames]) - end), - Config; init_per_testcase(_, Config) -> Config. -end_per_testcase(get_vhost, Config) -> - meck:unload(rabbit_feature_flags), - mnesia:stop(), - Config; end_per_testcase(_, Config) -> Config. ignore_colons(B) -> application:set_env(rabbitmq_mqtt, ignore_colons_in_username, B). @@ -150,26 +135,32 @@ get_vhost(_Config) -> %% certificate user, port/vhost parameter but no mapping, cert/vhost mapping %% should use cert/vhost mapping - set_global_parameter(mqtt_default_vhosts, [ - {<<"O=client,CN=dummy">>, <<"somevhost">>}, - {<<"O=client,CN=otheruser">>, <<"othervhost">>} - ]), - set_global_parameter(mqtt_port_to_vhost_mapping, [ - {<<"1884">>, <<"othervhost">>} - ]), + set_global_parameters( + [{mqtt_default_vhosts, + [ + {<<"O=client,CN=dummy">>, <<"somevhost">>}, + {<<"O=client,CN=otheruser">>, <<"othervhost">>} + ]}, + {mqtt_port_to_vhost_mapping, + [ + {<<"1884">>, <<"othervhost">>} + ]}]), {_, {<<"somevhost">>, <<"guest">>}} = rabbit_mqtt_processor:get_vhost(<<"guest">>, <<"O=client,CN=dummy">>, 1883), clear_vhost_global_parameters(), %% certificate user, port/vhost parameter, cert/vhost parameter %% cert/vhost parameter takes precedence - set_global_parameter(mqtt_default_vhosts, [ - {<<"O=client,CN=dummy">>, <<"cert-somevhost">>}, - {<<"O=client,CN=otheruser">>, <<"othervhost">>} - ]), - set_global_parameter(mqtt_port_to_vhost_mapping, [ - {<<"1883">>, <<"port-vhost">>}, - {<<"1884">>, <<"othervhost">>} - ]), + set_global_parameters( + [{mqtt_default_vhosts, + [ + {<<"O=client,CN=dummy">>, <<"cert-somevhost">>}, + {<<"O=client,CN=otheruser">>, <<"othervhost">>} + ]}, + {mqtt_port_to_vhost_mapping, + [ + {<<"1883">>, <<"port-vhost">>}, + {<<"1884">>, <<"othervhost">>} + ]}]), {_, {<<"cert-somevhost">>, <<"guest">>}} = rabbit_mqtt_processor:get_vhost(<<"guest">>, <<"O=client,CN=dummy">>, 1883), clear_vhost_global_parameters(), @@ -179,28 +170,30 @@ get_vhost(_Config) -> %% not a certificate user, port/vhost parameter, cert/vhost parameter %% port/vhost mapping is used, as cert/vhost should not be used - set_global_parameter(mqtt_default_vhosts, [ - {<<"O=cert">>, <<"cert-somevhost">>}, - {<<"O=client,CN=otheruser">>, <<"othervhost">>} - ]), - set_global_parameter(mqtt_port_to_vhost_mapping, [ - {<<"1883">>, <<"port-vhost">>}, - {<<"1884">>, <<"othervhost">>} - ]), + set_global_parameters( + [{mqtt_default_vhosts, + [ + {<<"O=cert">>, <<"cert-somevhost">>}, + {<<"O=client,CN=otheruser">>, <<"othervhost">>} + ]}, + {mqtt_port_to_vhost_mapping, + [ + {<<"1883">>, <<"port-vhost">>}, + {<<"1884">>, <<"othervhost">>} + ]}]), {_, {<<"port-vhost">>, <<"guest">>}} = rabbit_mqtt_processor:get_vhost(<<"guest">>, none, 1883), clear_vhost_global_parameters(), ok. set_global_parameter(Key, Term) -> - InsertParameterFun = fun () -> - mnesia:write(rabbit_runtime_parameters, #runtime_parameters{key = Key, value = Term}, write) - end, + set_global_parameters([{Key, Term}]). - {atomic, ok} = mnesia:transaction(InsertParameterFun). +set_global_parameters(KVList) -> + meck:expect( + rabbit_runtime_parameters, value_global, + fun(Key) -> proplists:get_value(Key, KVList, not_found) end). clear_vhost_global_parameters() -> - DeleteParameterFun = fun () -> - ok = mnesia:delete(rabbit_runtime_parameters, mqtt_default_vhosts, write), - ok = mnesia:delete(rabbit_runtime_parameters, mqtt_port_to_vhost_mapping, write) - end, - {atomic, ok} = mnesia:transaction(DeleteParameterFun). + meck:expect( + rabbit_runtime_parameters, value_global, + fun(_) -> not_found end). From 7d4ecb5e8223d8c3a5f1ef87769ee4c778186195 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20G=C3=B6m=C3=B6ri?= Date: Fri, 18 Oct 2024 01:35:13 +0200 Subject: [PATCH 2/2] Refactor mqtt_processor get_vhost functions In order to clarify preference of different methods. This commit oes not change functionality. This highlights some inconsistences: - If both User/Password and SslLogin are provided, in `check_credentials` User/Password takes precedence while in `get_vhost` SslLogin - If SslLogin is provided (but no mapping is found) vhost from port mapping has precedence over vhost from UserName, while in case of no ssl it is the other way around. --- .../src/rabbit_mqtt_processor.erl | 71 +++++++++---------- deps/rabbitmq_mqtt/test/processor_SUITE.erl | 10 ++- 2 files changed, 41 insertions(+), 40 deletions(-) diff --git a/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl b/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl index 3c17f07a43..e8ac5e4232 100644 --- a/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl +++ b/deps/rabbitmq_mqtt/src/rabbit_mqtt_processor.erl @@ -1185,33 +1185,31 @@ get_vhost(UserBin, SslLogin, Port) -> get_vhost_ssl(UserBin, SslLogin, Port). get_vhost_no_ssl(UserBin, Port) -> - case vhost_in_username(UserBin) of - true -> - {vhost_in_username_or_default, get_vhost_username(UserBin)}; - false -> - PortVirtualHostMapping = rabbit_runtime_parameters:value_global( - mqtt_port_to_vhost_mapping - ), - case get_vhost_from_port_mapping(Port, PortVirtualHostMapping) of + case get_vhost_username(UserBin) of + undefined -> + case get_vhost_from_port_mapping(Port) of undefined -> - {plugin_configuration_or_default_vhost, {rabbit_mqtt_util:env(vhost), UserBin}}; - VHost -> - {port_to_vhost_mapping, {VHost, UserBin}} - end + VhostFromConfig = rabbit_mqtt_util:env(vhost), + {plugin_configuration_or_default_vhost, {VhostFromConfig, UserBin}}; + VHostFromPortMapping -> + {port_to_vhost_mapping, {VHostFromPortMapping, UserBin}} + end; + VHostUser -> + {vhost_in_username, VHostUser} end. get_vhost_ssl(UserBin, SslLoginName, Port) -> - UserVirtualHostMapping = rabbit_runtime_parameters:value_global( - mqtt_default_vhosts - ), - case get_vhost_from_user_mapping(SslLoginName, UserVirtualHostMapping) of + case get_vhost_from_user_mapping(SslLoginName) of undefined -> - PortVirtualHostMapping = rabbit_runtime_parameters:value_global( - mqtt_port_to_vhost_mapping - ), - case get_vhost_from_port_mapping(Port, PortVirtualHostMapping) of + case get_vhost_from_port_mapping(Port) of undefined -> - {vhost_in_username_or_default, get_vhost_username(UserBin)}; + case get_vhost_username(UserBin) of + undefined -> + VhostFromConfig = rabbit_mqtt_util:env(vhost), + {plugin_configuration_or_default_vhost, {VhostFromConfig, UserBin}}; + VHostUser -> + {vhost_in_username, VHostUser} + end; VHostFromPortMapping -> {port_to_vhost_mapping, {VHostFromPortMapping, UserBin}} end; @@ -1219,31 +1217,24 @@ get_vhost_ssl(UserBin, SslLoginName, Port) -> {client_cert_to_vhost_mapping, {VHostFromCertMapping, UserBin}} end. -vhost_in_username(UserBin) -> - case application:get_env(?APP_NAME, ignore_colons_in_username) of - {ok, true} -> false; - _ -> - %% split at the last colon, disallowing colons in username - case re:split(UserBin, ":(?!.*?:)") of - [_, _] -> true; - [UserBin] -> false; - [] -> false - end - end. - get_vhost_username(UserBin) -> - Default = {rabbit_mqtt_util:env(vhost), UserBin}, case application:get_env(?APP_NAME, ignore_colons_in_username) of - {ok, true} -> Default; + {ok, true} -> undefined; _ -> %% split at the last colon, disallowing colons in username case re:split(UserBin, ":(?!.*?:)") of [Vhost, UserName] -> {Vhost, UserName}; - [UserBin] -> Default; - [] -> Default + [UserBin] -> undefined; + [] -> undefined end end. +get_vhost_from_user_mapping(User) -> + UserVirtualHostMapping = rabbit_runtime_parameters:value_global( + mqtt_default_vhosts + ), + get_vhost_from_user_mapping(User, UserVirtualHostMapping). + get_vhost_from_user_mapping(_User, not_found) -> undefined; get_vhost_from_user_mapping(User, Mapping) -> @@ -1255,6 +1246,12 @@ get_vhost_from_user_mapping(User, Mapping) -> VHost end. +get_vhost_from_port_mapping(Port) -> + PortVirtualHostMapping = rabbit_runtime_parameters:value_global( + mqtt_port_to_vhost_mapping + ), + get_vhost_from_port_mapping(Port, PortVirtualHostMapping). + get_vhost_from_port_mapping(_Port, not_found) -> undefined; get_vhost_from_port_mapping(Port, Mapping) -> diff --git a/deps/rabbitmq_mqtt/test/processor_SUITE.erl b/deps/rabbitmq_mqtt/test/processor_SUITE.erl index 8a679b764b..c7b38e57a7 100644 --- a/deps/rabbitmq_mqtt/test/processor_SUITE.erl +++ b/deps/rabbitmq_mqtt/test/processor_SUITE.erl @@ -45,9 +45,13 @@ end_per_testcase(_, Config) -> Config. ignore_colons(B) -> application:set_env(rabbitmq_mqtt, ignore_colons_in_username, B). ignores_colons_in_username_if_option_set(_Config) -> - ignore_colons(true), - ?assertEqual({rabbit_mqtt_util:env(vhost), <<"a:b:c">>}, - rabbit_mqtt_processor:get_vhost_username(<<"a:b:c">>)). + clear_vhost_global_parameters(), + ignore_colons(true), + ?assertEqual(undefined, + rabbit_mqtt_processor:get_vhost_username(<<"a:b:c">>)), + ?assertEqual({plugin_configuration_or_default_vhost, + {rabbit_mqtt_util:env(vhost), <<"a:b:c">>}}, + rabbit_mqtt_processor:get_vhost(<<"a:b:c">>, none, 1883)). interprets_colons_in_username_if_option_not_set(_Config) -> ignore_colons(false),