From 6ad71a483edd2fd4ca8bcd504a27fe9c64f65d46 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Mon, 13 Dec 2021 22:07:46 +0300 Subject: [PATCH 1/6] chore(authz): test Redis backend with real Redis --- .ci/docker-compose-file/Makefile.local | 3 - apps/emqx_authz/src/emqx_authz_redis.erl | 9 +- .../test/emqx_authz_redis_SUITE.erl | 340 ++++++++++++++---- apps/emqx_authz/test/emqx_authz_test_lib.erl | 44 +++ 4 files changed, 326 insertions(+), 70 deletions(-) create mode 100644 apps/emqx_authz/test/emqx_authz_test_lib.erl diff --git a/.ci/docker-compose-file/Makefile.local b/.ci/docker-compose-file/Makefile.local index 096da64c5..14e4c95f7 100644 --- a/.ci/docker-compose-file/Makefile.local +++ b/.ci/docker-compose-file/Makefile.local @@ -16,10 +16,8 @@ up: REDIS_TAG=6 \ MONGO_TAG=5 \ PGSQL_TAG=13 \ - LDAP_TAG=2.4.50 \ docker-compose \ -f .ci/docker-compose-file/docker-compose.yaml \ - -f .ci/docker-compose-file/docker-compose-ldap-tcp.yaml \ -f .ci/docker-compose-file/docker-compose-mongo-single-tcp.yaml \ -f .ci/docker-compose-file/docker-compose-mysql-tcp.yaml \ -f .ci/docker-compose-file/docker-compose-pgsql-tcp.yaml \ @@ -29,7 +27,6 @@ up: down: docker-compose \ -f .ci/docker-compose-file/docker-compose.yaml \ - -f .ci/docker-compose-file/docker-compose-ldap-tcp.yaml \ -f .ci/docker-compose-file/docker-compose-mongo-single-tcp.yaml \ -f .ci/docker-compose-file/docker-compose-mysql-tcp.yaml \ -f .ci/docker-compose-file/docker-compose-pgsql-tcp.yaml \ diff --git a/apps/emqx_authz/src/emqx_authz_redis.erl b/apps/emqx_authz/src/emqx_authz_redis.erl index fc60d57ad..1f6abe330 100644 --- a/apps/emqx_authz/src/emqx_authz_redis.erl +++ b/apps/emqx_authz/src/emqx_authz_redis.erl @@ -70,9 +70,10 @@ authorize(Client, PubSub, Topic, do_authorize(_Client, _PubSub, _Topic, []) -> nomatch; do_authorize(Client, PubSub, Topic, [TopicFilter, Action | Tail]) -> - case emqx_authz_rule:match(Client, PubSub, Topic, - emqx_authz_rule:compile({allow, all, Action, [TopicFilter]}) - )of + case emqx_authz_rule:match( + Client, PubSub, Topic, + emqx_authz_rule:compile({allow, all, Action, [TopicFilter]}) + ) of {matched, Permission} -> {matched, Permission}; nomatch -> do_authorize(Client, PubSub, Topic, Tail) end. @@ -81,6 +82,8 @@ replvar(Cmd, Client = #{cn := CN}) -> replvar(repl(Cmd, ?PH_S_CERT_CN_NAME, CN), maps:remove(cn, Client)); replvar(Cmd, Client = #{dn := DN}) -> replvar(repl(Cmd, ?PH_S_CERT_SUBJECT, DN), maps:remove(dn, Client)); +replvar(Cmd, Client = #{peerhost := IpAddr}) -> + replvar(repl(Cmd, ?PH_S_PEERHOST, inet_parse:ntoa(IpAddr)), maps:remove(peerhost, Client)); replvar(Cmd, Client = #{clientid := ClientId}) -> replvar(repl(Cmd, ?PH_S_CLIENTID, ClientId), maps:remove(clientid, Client)); replvar(Cmd, Client = #{username := Username}) -> diff --git a/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl b/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl index 3951ebfb6..af59ddc0d 100644 --- a/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl @@ -4,7 +4,8 @@ %% Licensed under the Apache License, Version 2.0 (the "License"); %% you may not use this file except in compliance with the License. %% You may obtain a copy of the License at -%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% http://www.apache.org/licenses/LICENSE-2.0 %% %% Unless required by applicable law or agreed to in writing, software %% distributed under the License is distributed on an "AS IS" BASIS, @@ -21,8 +22,11 @@ -include("emqx_authz.hrl"). -include_lib("eunit/include/eunit.hrl"). -include_lib("common_test/include/ct.hrl"). --include_lib("emqx/include/emqx_placeholder.hrl"). --define(CONF_DEFAULT, <<"authorization: {sources: []}">>). + + +-define(REDIS_HOST, "redis"). +-define(REDIS_PORT, 6379). +-define(REDIS_RESOURCE, <<"emqx_authz_redis_SUITE">>). all() -> emqx_common_test_helpers:all(?MODULE). @@ -31,86 +35,294 @@ groups() -> []. init_per_suite(Config) -> - meck:new(emqx_resource, [non_strict, passthrough, no_history, no_link]), - meck:expect(emqx_resource, create, fun(_, _, _) -> {ok, meck_data} end ), - meck:expect(emqx_resource, remove, fun(_) -> ok end ), - - ok = emqx_common_test_helpers:start_apps( - [emqx_conf, emqx_authz], - fun set_special_configs/1), - - Rules = [#{<<"type">> => <<"redis">>, - <<"server">> => <<"127.0.0.1:27017">>, - <<"pool_size">> => 1, - <<"database">> => 0, - <<"password">> => <<"ee">>, - <<"auto_reconnect">> => true, - <<"ssl">> => #{<<"enable">> => false}, - <<"cmd">> => <<"HGETALL mqtt_authz:", ?PH_USERNAME/binary>> - }], - {ok, _} = emqx_authz:update(replace, Rules), - Config. + case emqx_authn_test_lib:is_tcp_server_available(?REDIS_HOST, ?REDIS_PORT) of + true -> + ok = emqx_common_test_helpers:start_apps( + [emqx_conf, emqx_authz], + fun set_special_configs/1 + ), + ok = start_apps([emqx_resource, emqx_connector]), + {ok, _} = emqx_resource:create_local( + ?REDIS_RESOURCE, + emqx_connector_redis, + redis_config()), + Config; + false -> + {skip, no_redis} + end. end_per_suite(_Config) -> - {ok, _} = emqx:update_config( - [authorization], - #{<<"no_match">> => <<"allow">>, - <<"cache">> => #{<<"enable">> => <<"true">>}, - <<"sources">> => []}), - emqx_common_test_helpers:stop_apps([emqx_authz, emqx_resource]), - meck:unload(emqx_resource), - ok. + ok = emqx_authz_test_lib:reset_authorizers(), + ok = emqx_resource:remove_local(?REDIS_RESOURCE), + ok = stop_apps([emqx_resource, emqx_connector]), + ok = emqx_common_test_helpers:stop_apps([emqx_authz]). set_special_configs(emqx_authz) -> - {ok, _} = emqx:update_config([authorization, cache, enable], false), - {ok, _} = emqx:update_config([authorization, no_match], deny), - {ok, _} = emqx:update_config([authorization, sources], []), - ok; -set_special_configs(_App) -> + ok = emqx_authz_test_lib:reset_authorizers(); + +set_special_configs(_) -> ok. --define(SOURCE1, [<<"test/", ?PH_USERNAME/binary>>, <<"publish">>]). --define(SOURCE2, [<<"test/", ?PH_CLIENTID/binary>>, <<"publish">>]). --define(SOURCE3, [<<"#">>, <<"subscribe">>]). %%------------------------------------------------------------------------------ -%% Testcases +%% Tests %%------------------------------------------------------------------------------ -t_authz(_) -> +t_topic_rules(_Config) -> ClientInfo = #{clientid => <<"clientid">>, username => <<"username">>, peerhost => {127,0,0,1}, zone => default, listener => {tcp, default} - }, + }, - meck:expect(emqx_resource, query, fun(_, _) -> {ok, []} end), - % nomatch - ?assertEqual(deny, - emqx_access_control:authorize(ClientInfo, subscribe, <<"#">>)), - ?assertEqual(deny, - emqx_access_control:authorize(ClientInfo, publish, <<"#">>)), + %% No rules + + ok = setup_sample(#{}), + ok = setup_config(#{}), + + ok = test_samples( + ClientInfo, + [{deny, subscribe, <<"#">>}, + {deny, subscribe, <<"subs">>}, + {deny, publish, <<"pub">>}]), + + %% Publish rules + + Sample0 = #{<<"mqtt_user:username">> => + #{<<"testpub1/${username}">> => <<"publish">>, + <<"testpub2/${clientid}">> => <<"publish">>, + <<"testpub3/#">> => <<"publish">> + } + }, + ok = setup_sample(Sample0), + ok = setup_config(#{}), + + ok = test_samples( + ClientInfo, + [{allow, publish, <<"testpub1/username">>}, + {allow, publish, <<"testpub2/clientid">>}, + {allow, publish, <<"testpub3/foobar">>}, + + {deny, publish, <<"testpub2/username">>}, + {deny, publish, <<"testpub1/clientid">>}, - meck:expect(emqx_resource, query, fun(_, _) -> {ok, ?SOURCE1 ++ ?SOURCE2} end), - % nomatch - ?assertEqual(deny, - emqx_access_control:authorize(ClientInfo, subscribe, <<"+">>)), - % nomatch - ?assertEqual(deny, - emqx_access_control:authorize(ClientInfo, subscribe, <<"test/username">>)), + {deny, subscribe, <<"testpub1/username">>}, + {deny, subscribe, <<"testpub2/clientid">>}, + {deny, subscribe, <<"testpub3/foobar">>}]), - ?assertEqual(allow, - emqx_access_control:authorize(ClientInfo, publish, <<"test/clientid">>)), - ?assertEqual(allow, - emqx_access_control:authorize(ClientInfo, publish, <<"test/clientid">>)), + %% Subscribe rules - meck:expect(emqx_resource, query, fun(_, _) -> {ok, ?SOURCE3} end), + Sample1 = #{<<"mqtt_user:username">> => + #{<<"testsub1/${username}">> => <<"subscribe">>, + <<"testsub2/${clientid}">> => <<"subscribe">>, + <<"testsub3/#">> => <<"subscribe">> + } + }, + ok = setup_sample(Sample1), + ok = setup_config(#{}), - ?assertEqual(allow, - emqx_access_control:authorize(ClientInfo, subscribe, <<"#">>)), - % nomatch - ?assertEqual(deny, - emqx_access_control:authorize(ClientInfo, publish, <<"#">>)), + ok = test_samples( + ClientInfo, + [{allow, subscribe, <<"testsub1/username">>}, + {allow, subscribe, <<"testsub2/clientid">>}, + {allow, subscribe, <<"testsub3/foobar">>}, + {allow, subscribe, <<"testsub3/+/foobar">>}, + {allow, subscribe, <<"testsub3/#">>}, + + {deny, subscribe, <<"testsub2/username">>}, + {deny, subscribe, <<"testsub1/clientid">>}, + {deny, subscribe, <<"testsub4/foobar">>}, + {deny, publish, <<"testsub1/username">>}, + {deny, publish, <<"testsub2/clientid">>}, + {deny, publish, <<"testsub3/foobar">>}]), + + %% All rules + + Sample2 = #{<<"mqtt_user:username">> => + #{<<"testall1/${username}">> => <<"all">>, + <<"testall2/${clientid}">> => <<"all">>, + <<"testall3/#">> => <<"all">> + } + }, + ok = setup_sample(Sample2), + ok = setup_config(#{}), + + ok = test_samples( + ClientInfo, + [{allow, subscribe, <<"testall1/username">>}, + {allow, subscribe, <<"testall2/clientid">>}, + {allow, subscribe, <<"testall3/foobar">>}, + {allow, subscribe, <<"testall3/+/foobar">>}, + {allow, subscribe, <<"testall3/#">>}, + {allow, publish, <<"testall1/username">>}, + {allow, publish, <<"testall2/clientid">>}, + {allow, publish, <<"testall3/foobar">>}, + + {deny, subscribe, <<"testall2/username">>}, + {deny, subscribe, <<"testall1/clientid">>}, + {deny, subscribe, <<"testall4/foobar">>}, + {deny, publish, <<"testall2/username">>}, + {deny, publish, <<"testall1/clientid">>}, + {deny, publish, <<"testall4/foobar">>}]). + +t_lookups(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + cn => <<"cn">>, + dn => <<"dn">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + ByClientid = #{<<"mqtt_user:clientid">> => + #{<<"a">> => <<"all">>}}, + + ok = setup_sample(ByClientid), + ok = setup_config(#{<<"cmd">> => <<"HGETALL mqtt_user:${clientid}">>}), + + ok = test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]), + + ByPeerhost = #{<<"mqtt_user:127.0.0.1">> => + #{<<"a">> => <<"all">>}}, + + ok = setup_sample(ByPeerhost), + ok = setup_config(#{<<"cmd">> => <<"HGETALL mqtt_user:${peerhost}">>}), + + ok = test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]), + + ByCN = #{<<"mqtt_user:cn">> => + #{<<"a">> => <<"all">>}}, + + ok = setup_sample(ByCN), + ok = setup_config(#{<<"cmd">> => <<"HGETALL mqtt_user:${cert_common_name}">>}), + + ok = test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]), + + + ByDN = #{<<"mqtt_user:dn">> => + #{<<"a">> => <<"all">>}}, + + ok = setup_sample(ByDN), + ok = setup_config(#{<<"cmd">> => <<"HGETALL mqtt_user:${cert_subject}">>}), + + ok = test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]). + +t_create_invalid(_Config) -> + AuthzConfig = raw_redis_authz_config(), + + InvalidConfigs = + [maps:without([<<"server">>], AuthzConfig), + AuthzConfig#{<<"server">> => <<"unknownhost:3333">>}, + AuthzConfig#{<<"password">> => <<"wrongpass">>}, + AuthzConfig#{<<"database">> => <<"5678">>}], + + lists:foreach( + fun(Config) -> + {error, _} = emqx_authz:update(?CMD_REPLACE, [Config]), + [] = emqx_authz:lookup() + + end, + InvalidConfigs). + +t_redis_error(_Config) -> + ok = setup_config(#{<<"cmd">> => <<"INVALID COMMAND">>}), + + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + deny = emqx_access_control:authorize(ClientInfo, subscribe, <<"a">>). + +%%------------------------------------------------------------------------------ +%% Helpers +%%------------------------------------------------------------------------------ + +test_samples(ClientInfo, Samples) -> + lists:foreach( + fun({Expected, Action, Topic}) -> + ct:pal( + "client_info: ~p, action: ~p, topic: ~p, expected: ~p", + [ClientInfo, Action, Topic, Expected]), + ?assertEqual( + Expected, + emqx_access_control:authorize( + ClientInfo, + Action, + Topic)) + end, + Samples). + + +setup_sample(AuthzData) -> + {ok, _} = q(["FLUSHDB"]), + ok = lists:foreach( + fun({Key, Values}) -> + lists:foreach( + fun({TopicFilter, Action}) -> + q(["HSET", Key, TopicFilter, Action]) + end, + maps:to_list(Values)) + end, + maps:to_list(AuthzData)). + +setup_config(SpecialParams) -> + ok = emqx_authz_test_lib:reset_authorizers(deny, false), + Config = maps:merge(raw_redis_authz_config(), SpecialParams), + {ok, _} = emqx_authz:update(?CMD_REPLACE, [Config]), ok. + +raw_redis_authz_config() -> + #{ + <<"enable">> => <<"true">>, + + <<"type">> => <<"redis">>, + <<"cmd">> => <<"HGETALL mqtt_user:${username}">>, + <<"database">> => <<"1">>, + <<"password">> => <<"public">>, + <<"server">> => redis_server() + }. + +redis_server() -> + iolist_to_binary( + io_lib:format( + "~s:~b", + [?REDIS_HOST, ?REDIS_PORT])). + +q(Command) -> + emqx_resource:query( + ?REDIS_RESOURCE, + {cmd, Command}). + +redis_config() -> + #{auto_reconnect => true, + database => 1, + pool_size => 8, + redis_type => single, + password => "public", + server => {?REDIS_HOST, ?REDIS_PORT}, + ssl => #{enable => false} + }. + +start_apps(Apps) -> + lists:foreach(fun application:ensure_all_started/1, Apps). + +stop_apps(Apps) -> + lists:foreach(fun application:stop/1, Apps). diff --git a/apps/emqx_authz/test/emqx_authz_test_lib.erl b/apps/emqx_authz/test/emqx_authz_test_lib.erl new file mode 100644 index 000000000..5e9dfc32f --- /dev/null +++ b/apps/emqx_authz/test/emqx_authz_test_lib.erl @@ -0,0 +1,44 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2021 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +-module(emqx_authz_test_lib). + +-include("emqx_authz.hrl"). + +-compile(nowarn_export_all). +-compile(export_all). + +-define(DEFAULT_CHECK_AVAIL_TIMEOUT, 1000). + +reset_authorizers() -> + reset_authorizers(allow, true). + +reset_authorizers(Nomatch, ChacheEnabled) -> + {ok, _} = emqx:update_config( + [authorization], + #{<<"no_match">> => atom_to_binary(Nomatch), + <<"cache">> => #{<<"enable">> => atom_to_binary(ChacheEnabled)}, + <<"sources">> => []}), + ok. + +is_tcp_server_available(Host, Port) -> + case gen_tcp:connect(Host, Port, [], ?DEFAULT_CHECK_AVAIL_TIMEOUT) of + {ok, Socket} -> + gen_tcp:close(Socket), + true; + {error, _} -> + false + end. From 28c319b6c4dfb2bd85f32ef1c6e49cc3b71f63ce Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Wed, 15 Dec 2021 01:13:02 +0300 Subject: [PATCH 2/6] chore(authz): test Mongo backend with real Mongo --- apps/emqx_authz/src/emqx_authz.erl | 4 +- .../test/emqx_authz_mongodb_SUITE.erl | 300 +++++++++++++----- .../test/emqx_authz_redis_SUITE.erl | 32 +- apps/emqx_authz/test/emqx_authz_test_lib.erl | 22 ++ 4 files changed, 245 insertions(+), 113 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index d80253c4d..54438793b 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/apps/emqx_authz/src/emqx_authz.erl @@ -217,10 +217,10 @@ do_post_update({{?CMD_DELETE, Type}, _Source}, _NewSources) -> ok = ensure_resource_deleted(OldSource), ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [Front ++ Rear]}, -1), ok = emqx_authz_cache:drain_cache(); -do_post_update(_, NewSources) -> +do_post_update({?CMD_REPLACE, Sources}, _NewSources) -> %% overwrite the entire config! OldInitedSources = lookup(), - InitedSources = init_sources(NewSources), + InitedSources = init_sources(check_sources(Sources)), ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [InitedSources]}, -1), lists:foreach(fun ensure_resource_deleted/1, OldInitedSources), ok = emqx_authz_cache:drain_cache(). diff --git a/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl b/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl index d854e680f..8a6aa17c1 100644 --- a/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl @@ -23,112 +23,238 @@ -include_lib("common_test/include/ct.hrl"). -include_lib("emqx/include/emqx_placeholder.hrl"). +-define(MONGO_HOST, "mongo"). +-define(MONGO_PORT, 27017). +-define(MONGO_CLIENT, 'emqx_authz_mongo_SUITE_client'). + all() -> emqx_common_test_helpers:all(?MODULE). groups() -> []. -init_per_suite(Config) -> - meck:new(emqx_resource, [non_strict, passthrough, no_history, no_link]), - meck:expect(emqx_resource, create, fun(_, _, _) -> {ok, meck_data} end), - meck:expect(emqx_resource, remove, fun(_) -> ok end ), - - ok = emqx_common_test_helpers:start_apps( - [emqx_conf, emqx_authz], - fun set_special_configs/1 - ), - - Rules = [#{<<"type">> => <<"mongodb">>, - <<"mongo_type">> => <<"single">>, - <<"server">> => <<"127.0.0.1:27017">>, - <<"pool_size">> => 1, - <<"database">> => <<"mqtt">>, - <<"ssl">> => #{<<"enable">> => false}, - <<"collection">> => <<"fake">>, - <<"selector">> => #{<<"a">> => <<"b">>} - }], - {ok, _} = emqx_authz:update(replace, Rules), +init_per_testcase(_TestCase, Config) -> + {ok, _} = mc_worker_api:connect(mongo_config()), Config. +end_per_testcase(_TestCase, _Config) -> + ok = reset_samples(), + ok = mc_worker_api:disconnect(?MONGO_CLIENT). + +init_per_suite(Config) -> + case emqx_authz_test_lib:is_tcp_server_available(?MONGO_HOST, ?MONGO_PORT) of + true -> + ok = emqx_common_test_helpers:start_apps( + [emqx_conf, emqx_authz], + fun set_special_configs/1 + ), + ok = start_apps([emqx_resource, emqx_connector]), + Config; + false -> + {skip, no_mongo} + end. + end_per_suite(_Config) -> - {ok, _} = emqx:update_config( - [authorization], - #{<<"no_match">> => <<"allow">>, - <<"cache">> => #{<<"enable">> => <<"true">>}, - <<"sources">> => []}), - emqx_common_test_helpers:stop_apps([emqx_authz, emqx_conf]), - meck:unload(emqx_resource), - ok. + ok = emqx_authz_test_lib:reset_authorizers(), + ok = stop_apps([emqx_resource, emqx_connector]), + ok = emqx_common_test_helpers:stop_apps([emqx_authz]). set_special_configs(emqx_authz) -> - {ok, _} = emqx:update_config([authorization, cache, enable], false), - {ok, _} = emqx:update_config([authorization, no_match], deny), - {ok, _} = emqx:update_config([authorization, sources], []), - ok; -set_special_configs(_App) -> - ok. + ok = emqx_authz_test_lib:reset_authorizers(); --define(SOURCE1,[#{<<"topics">> => [<<"#">>], - <<"permission">> => <<"deny">>, - <<"action">> => <<"all">>}]). --define(SOURCE2,[#{<<"topics">> => [<<"eq #">>], - <<"permission">> => <<"allow">>, - <<"action">> => <<"all">>}]). --define(SOURCE3,[#{<<"topics">> => [<<"test/", ?PH_CLIENTID/binary>>], - <<"permission">> => <<"allow">>, - <<"action">> => <<"subscribe">>}]). --define(SOURCE4,[#{<<"topics">> => [<<"test/", ?PH_USERNAME/binary>>], - <<"permission">> => <<"allow">>, - <<"action">> => <<"publish">>}]). +set_special_configs(_) -> + ok. %%------------------------------------------------------------------------------ %% Testcases %%------------------------------------------------------------------------------ -t_authz(_) -> - ClientInfo1 = #{clientid => <<"test">>, - username => <<"test">>, - peerhost => {127,0,0,1}, - zone => default, - listener => {tcp, default} - }, - ClientInfo2 = #{clientid => <<"test_clientid">>, - username => <<"test_username">>, - peerhost => {192,168,0,10}, - zone => default, - listener => {tcp, default} - }, - ClientInfo3 = #{clientid => <<"test_clientid">>, - username => <<"fake_username">>, - peerhost => {127,0,0,1}, - zone => default, - listener => {tcp, default} - }, - meck:expect(emqx_resource, query, fun(_, _) -> [] end), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, subscribe, <<"#">>)), % nomatch - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, publish, <<"#">>)), % nomatch +t_topic_rules(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, - meck:expect(emqx_resource, query, fun(_, _) -> ?SOURCE1 ++ ?SOURCE2 end), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, subscribe, <<"+">>)), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, publish, <<"+">>)), + %% No rules - meck:expect(emqx_resource, query, fun(_, _) -> ?SOURCE2 ++ ?SOURCE1 end), - ?assertEqual(allow, emqx_access_control:authorize(ClientInfo1, subscribe, <<"#">>)), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, subscribe, <<"+">>)), + ok = setup_samples([]), + ok = setup_config(#{}), - meck:expect(emqx_resource, query, fun(_, _) -> ?SOURCE3 ++ ?SOURCE4 end), - ?assertEqual(allow, emqx_access_control:authorize( - ClientInfo2, subscribe, <<"test/test_clientid">>)), - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo2, publish, <<"test/test_clientid">>)), - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo2, subscribe, <<"test/test_username">>)), - ?assertEqual(allow, emqx_access_control:authorize( - ClientInfo2, publish, <<"test/test_username">>)), - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo3, subscribe, <<"test">>)), % nomatch - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo3, publish, <<"test">>)), % nomatch + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{deny, subscribe, <<"#">>}, + {deny, subscribe, <<"subs">>}, + {deny, publish, <<"pub">>}]), + + %% Publish rules + + Samples0 = populate_records( + [#{<<"topics">> => [<<"eq testpub1/${username}">>]}, + #{<<"topics">> => [<<"testpub2/${clientid}">>, <<"testpub3/#">>]}], + #{<<"permission">> => <<"allow">>, + <<"action">> => <<"publish">>, + <<"username">> => <<"username">>}), + + ok = setup_samples(Samples0), + ok = setup_config(#{}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{deny, publish, <<"testpub1/username">>}, + {allow, publish, <<"testpub1/${username}">>}, + {allow, publish, <<"testpub2/clientid">>}, + {allow, publish, <<"testpub3/foobar">>}, + + {deny, publish, <<"testpub2/username">>}, + {deny, publish, <<"testpub1/clientid">>}, + + + {deny, subscribe, <<"testpub1/username">>}, + {deny, subscribe, <<"testpub2/clientid">>}, + {deny, subscribe, <<"testpub3/foobar">>}]), + + %% Subscribe rules + + Samples1 = populate_records( + [#{<<"topics">> => [<<"eq testsub1/${username}">>]}, + #{<<"topics">> => [<<"testsub2/${clientid}">>, <<"testsub3/#">>]}], + #{<<"permission">> => <<"allow">>, + <<"action">> => <<"subscribe">>, + <<"username">> => <<"username">>}), + + ok = setup_samples(Samples1), + ok = setup_config(#{}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{deny, subscribe, <<"testsub1/username">>}, + {allow, subscribe, <<"testsub1/${username}">>}, + {allow, subscribe, <<"testsub2/clientid">>}, + {allow, subscribe, <<"testsub3/foobar">>}, + {allow, subscribe, <<"testsub3/+/foobar">>}, + {allow, subscribe, <<"testsub3/#">>}, + + {deny, subscribe, <<"testsub2/username">>}, + {deny, subscribe, <<"testsub1/clientid">>}, + {deny, subscribe, <<"testsub4/foobar">>}, + {deny, publish, <<"testsub1/username">>}, + {deny, publish, <<"testsub2/clientid">>}, + {deny, publish, <<"testsub3/foobar">>}]), + + %% All rules + + Samples2 = populate_records( + [#{<<"topics">> => [<<"eq testall1/${username}">>]}, + #{<<"topics">> => [<<"testall2/${clientid}">>, <<"testall3/#">>]}], + #{<<"permission">> => <<"allow">>, + <<"action">> => <<"all">>, + <<"username">> => <<"username">>}), + + ok = setup_samples(Samples2), + ok = setup_config(#{}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{deny, subscribe, <<"testall1/username">>}, + {allow, subscribe, <<"testall1/${username}">>}, + {allow, subscribe, <<"testall2/clientid">>}, + {allow, subscribe, <<"testall3/foobar">>}, + {allow, subscribe, <<"testall3/+/foobar">>}, + {allow, subscribe, <<"testall3/#">>}, + {deny, publish, <<"testall1/username">>}, + {allow, publish, <<"testall1/${username}">>}, + {allow, publish, <<"testall2/clientid">>}, + {allow, publish, <<"testall3/foobar">>}, + + {deny, subscribe, <<"testall2/username">>}, + {deny, subscribe, <<"testall1/clientid">>}, + {deny, subscribe, <<"testall4/foobar">>}, + {deny, publish, <<"testall2/username">>}, + {deny, publish, <<"testall1/clientid">>}, + {deny, publish, <<"testall4/foobar">>}]). + + +t_complex_selector(_) -> + ClientInfo = #{clientid => clientid, + username => "username", + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + Samples = [#{<<"x">> => #{<<"u">> => <<"username">>, + <<"c">> => [#{<<"c">> => <<"clientid">>}], + <<"y">> => 1}, + <<"permission">> => <<"allow">>, + <<"action">> => <<"publish">>, + <<"topics">> => [<<"t">>] + }], + + ok = setup_samples(Samples), + ok = setup_config( + #{<<"selector">> => #{<<"x">> => #{<<"u">> => <<"${username}">>, + <<"c">> => [#{<<"c">> => <<"${clientid}">>}], + <<"y">> => 1} + } + }), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, publish, <<"t">>}]). + +%%------------------------------------------------------------------------------ +%% Helpers +%%------------------------------------------------------------------------------ + +populate_records(AclRecords, AdditionalData) -> + [maps:merge(Record, AdditionalData) || Record <- AclRecords]. + +setup_samples(AclRecords) -> + ok = reset_samples(), + {{true, _}, _} = mc_worker_api:insert(?MONGO_CLIENT, <<"acl">>, AclRecords), ok. + +reset_samples() -> + {true, _} = mc_worker_api:delete(?MONGO_CLIENT, <<"acl">>, #{}), + ok. + +setup_config(SpecialParams) -> + emqx_authz_test_lib:setup_config( + raw_mongo_authz_config(), + SpecialParams). + +raw_mongo_authz_config() -> + #{ + <<"type">> => <<"mongodb">>, + <<"enable">> => <<"true">>, + + <<"mongo_type">> => <<"single">>, + <<"database">> => <<"mqtt">>, + <<"collection">> => <<"acl">>, + <<"server">> => mongo_server(), + + <<"selector">> => #{<<"username">> => <<"${username}">>} + }. + +mongo_server() -> + iolist_to_binary( + io_lib:format( + "~s:~b", + [?MONGO_HOST, ?MONGO_PORT])). + +mongo_config() -> + [ + {database, <<"mqtt">>}, + {host, ?MONGO_HOST}, + {port, ?MONGO_PORT}, + {register, ?MONGO_CLIENT} + ]. + +start_apps(Apps) -> + lists:foreach(fun application:ensure_all_started/1, Apps). + +stop_apps(Apps) -> + lists:foreach(fun application:stop/1, Apps). diff --git a/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl b/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl index af59ddc0d..7853564bf 100644 --- a/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl @@ -81,7 +81,7 @@ t_topic_rules(_Config) -> ok = setup_sample(#{}), ok = setup_config(#{}), - ok = test_samples( + ok = emqx_authz_test_lib:test_samples( ClientInfo, [{deny, subscribe, <<"#">>}, {deny, subscribe, <<"subs">>}, @@ -98,7 +98,7 @@ t_topic_rules(_Config) -> ok = setup_sample(Sample0), ok = setup_config(#{}), - ok = test_samples( + ok = emqx_authz_test_lib:test_samples( ClientInfo, [{allow, publish, <<"testpub1/username">>}, {allow, publish, <<"testpub2/clientid">>}, @@ -123,7 +123,7 @@ t_topic_rules(_Config) -> ok = setup_sample(Sample1), ok = setup_config(#{}), - ok = test_samples( + ok = emqx_authz_test_lib:test_samples( ClientInfo, [{allow, subscribe, <<"testsub1/username">>}, {allow, subscribe, <<"testsub2/clientid">>}, @@ -149,7 +149,7 @@ t_topic_rules(_Config) -> ok = setup_sample(Sample2), ok = setup_config(#{}), - ok = test_samples( + ok = emqx_authz_test_lib:test_samples( ClientInfo, [{allow, subscribe, <<"testall1/username">>}, {allow, subscribe, <<"testall2/clientid">>}, @@ -183,7 +183,7 @@ t_lookups(_Config) -> ok = setup_sample(ByClientid), ok = setup_config(#{<<"cmd">> => <<"HGETALL mqtt_user:${clientid}">>}), - ok = test_samples( + ok = emqx_authz_test_lib:test_samples( ClientInfo, [{allow, subscribe, <<"a">>}, {deny, subscribe, <<"b">>}]), @@ -194,7 +194,7 @@ t_lookups(_Config) -> ok = setup_sample(ByPeerhost), ok = setup_config(#{<<"cmd">> => <<"HGETALL mqtt_user:${peerhost}">>}), - ok = test_samples( + ok = emqx_authz_test_lib:test_samples( ClientInfo, [{allow, subscribe, <<"a">>}, {deny, subscribe, <<"b">>}]), @@ -205,7 +205,7 @@ t_lookups(_Config) -> ok = setup_sample(ByCN), ok = setup_config(#{<<"cmd">> => <<"HGETALL mqtt_user:${cert_common_name}">>}), - ok = test_samples( + ok = emqx_authz_test_lib:test_samples( ClientInfo, [{allow, subscribe, <<"a">>}, {deny, subscribe, <<"b">>}]), @@ -217,7 +217,7 @@ t_lookups(_Config) -> ok = setup_sample(ByDN), ok = setup_config(#{<<"cmd">> => <<"HGETALL mqtt_user:${cert_subject}">>}), - ok = test_samples( + ok = emqx_authz_test_lib:test_samples( ClientInfo, [{allow, subscribe, <<"a">>}, {deny, subscribe, <<"b">>}]). @@ -255,22 +255,6 @@ t_redis_error(_Config) -> %% Helpers %%------------------------------------------------------------------------------ -test_samples(ClientInfo, Samples) -> - lists:foreach( - fun({Expected, Action, Topic}) -> - ct:pal( - "client_info: ~p, action: ~p, topic: ~p, expected: ~p", - [ClientInfo, Action, Topic, Expected]), - ?assertEqual( - Expected, - emqx_access_control:authorize( - ClientInfo, - Action, - Topic)) - end, - Samples). - - setup_sample(AuthzData) -> {ok, _} = q(["FLUSHDB"]), ok = lists:foreach( diff --git a/apps/emqx_authz/test/emqx_authz_test_lib.erl b/apps/emqx_authz/test/emqx_authz_test_lib.erl index 5e9dfc32f..6dd00527e 100644 --- a/apps/emqx_authz/test/emqx_authz_test_lib.erl +++ b/apps/emqx_authz/test/emqx_authz_test_lib.erl @@ -17,6 +17,7 @@ -module(emqx_authz_test_lib). -include("emqx_authz.hrl"). +-include_lib("eunit/include/eunit.hrl"). -compile(nowarn_export_all). -compile(export_all). @@ -34,6 +35,27 @@ reset_authorizers(Nomatch, ChacheEnabled) -> <<"sources">> => []}), ok. +setup_config(BaseConfig, SpecialParams) -> + ok = reset_authorizers(deny, false), + Config = maps:merge(BaseConfig, SpecialParams), + {ok, _} = emqx_authz:update(?CMD_REPLACE, [Config]), + ok. + +test_samples(ClientInfo, Samples) -> + lists:foreach( + fun({Expected, Action, Topic}) -> + ct:pal( + "client_info: ~p, action: ~p, topic: ~p, expected: ~p", + [ClientInfo, Action, Topic, Expected]), + ?assertEqual( + Expected, + emqx_access_control:authorize( + ClientInfo, + Action, + Topic)) + end, + Samples). + is_tcp_server_available(Host, Port) -> case gen_tcp:connect(Host, Port, [], ?DEFAULT_CHECK_AVAIL_TIMEOUT) of {ok, Socket} -> From a21f67a67ebe8d32ca6a62c509ab16ad83352f97 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Wed, 15 Dec 2021 23:06:38 +0300 Subject: [PATCH 3/6] chore(authz): test Mongo backend with real Mongo --- apps/emqx_authz/src/emqx_authz_mongodb.erl | 2 +- .../test/emqx_authz_mongodb_SUITE.erl | 172 +++++++-------- .../test/emqx_authz_redis_SUITE.erl | 107 ++-------- apps/emqx_authz/test/emqx_authz_test_lib.erl | 199 +++++++++++++++++- 4 files changed, 286 insertions(+), 194 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_mongodb.erl b/apps/emqx_authz/src/emqx_authz_mongodb.erl index 439c2c853..97fac9627 100644 --- a/apps/emqx_authz/src/emqx_authz_mongodb.erl +++ b/apps/emqx_authz/src/emqx_authz_mongodb.erl @@ -97,7 +97,7 @@ replvar(Selector, #{clientid := Clientid, , bin(Clientid), [global, {return, binary}]), V2 = re:replace( V1, emqx_authz:ph_to_re(?PH_S_USERNAME) , bin(Username), [global, {return, binary}]), - V3 = re:replace( V2, emqx_authz:ph_to_re(?PH_S_HOST) + V3 = re:replace( V2, emqx_authz:ph_to_re(?PH_S_PEERHOST) , inet_parse:ntoa(IpAddress), [global, {return, binary}]), maps:put(K, V3, AccIn); InFun(K, V, AccIn) -> maps:put(K, V, AccIn) diff --git a/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl b/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl index 8a6aa17c1..5b902ec43 100644 --- a/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl @@ -69,7 +69,6 @@ set_special_configs(_) -> %% Testcases %%------------------------------------------------------------------------------ - t_topic_rules(_Config) -> ClientInfo = #{clientid => <<"clientid">>, username => <<"username">>, @@ -78,106 +77,14 @@ t_topic_rules(_Config) -> listener => {tcp, default} }, - %% No rules + ok = emqx_authz_test_lib:test_no_topic_rules(ClientInfo, fun setup_client_samples/2), - ok = setup_samples([]), - ok = setup_config(#{}), - - ok = emqx_authz_test_lib:test_samples( - ClientInfo, - [{deny, subscribe, <<"#">>}, - {deny, subscribe, <<"subs">>}, - {deny, publish, <<"pub">>}]), - - %% Publish rules - - Samples0 = populate_records( - [#{<<"topics">> => [<<"eq testpub1/${username}">>]}, - #{<<"topics">> => [<<"testpub2/${clientid}">>, <<"testpub3/#">>]}], - #{<<"permission">> => <<"allow">>, - <<"action">> => <<"publish">>, - <<"username">> => <<"username">>}), - - ok = setup_samples(Samples0), - ok = setup_config(#{}), - - ok = emqx_authz_test_lib:test_samples( - ClientInfo, - [{deny, publish, <<"testpub1/username">>}, - {allow, publish, <<"testpub1/${username}">>}, - {allow, publish, <<"testpub2/clientid">>}, - {allow, publish, <<"testpub3/foobar">>}, - - {deny, publish, <<"testpub2/username">>}, - {deny, publish, <<"testpub1/clientid">>}, - - - {deny, subscribe, <<"testpub1/username">>}, - {deny, subscribe, <<"testpub2/clientid">>}, - {deny, subscribe, <<"testpub3/foobar">>}]), - - %% Subscribe rules - - Samples1 = populate_records( - [#{<<"topics">> => [<<"eq testsub1/${username}">>]}, - #{<<"topics">> => [<<"testsub2/${clientid}">>, <<"testsub3/#">>]}], - #{<<"permission">> => <<"allow">>, - <<"action">> => <<"subscribe">>, - <<"username">> => <<"username">>}), - - ok = setup_samples(Samples1), - ok = setup_config(#{}), - - ok = emqx_authz_test_lib:test_samples( - ClientInfo, - [{deny, subscribe, <<"testsub1/username">>}, - {allow, subscribe, <<"testsub1/${username}">>}, - {allow, subscribe, <<"testsub2/clientid">>}, - {allow, subscribe, <<"testsub3/foobar">>}, - {allow, subscribe, <<"testsub3/+/foobar">>}, - {allow, subscribe, <<"testsub3/#">>}, - - {deny, subscribe, <<"testsub2/username">>}, - {deny, subscribe, <<"testsub1/clientid">>}, - {deny, subscribe, <<"testsub4/foobar">>}, - {deny, publish, <<"testsub1/username">>}, - {deny, publish, <<"testsub2/clientid">>}, - {deny, publish, <<"testsub3/foobar">>}]), - - %% All rules - - Samples2 = populate_records( - [#{<<"topics">> => [<<"eq testall1/${username}">>]}, - #{<<"topics">> => [<<"testall2/${clientid}">>, <<"testall3/#">>]}], - #{<<"permission">> => <<"allow">>, - <<"action">> => <<"all">>, - <<"username">> => <<"username">>}), - - ok = setup_samples(Samples2), - ok = setup_config(#{}), - - ok = emqx_authz_test_lib:test_samples( - ClientInfo, - [{deny, subscribe, <<"testall1/username">>}, - {allow, subscribe, <<"testall1/${username}">>}, - {allow, subscribe, <<"testall2/clientid">>}, - {allow, subscribe, <<"testall3/foobar">>}, - {allow, subscribe, <<"testall3/+/foobar">>}, - {allow, subscribe, <<"testall3/#">>}, - {deny, publish, <<"testall1/username">>}, - {allow, publish, <<"testall1/${username}">>}, - {allow, publish, <<"testall2/clientid">>}, - {allow, publish, <<"testall3/foobar">>}, - - {deny, subscribe, <<"testall2/username">>}, - {deny, subscribe, <<"testall1/clientid">>}, - {deny, subscribe, <<"testall4/foobar">>}, - {deny, publish, <<"testall2/username">>}, - {deny, publish, <<"testall1/clientid">>}, - {deny, publish, <<"testall4/foobar">>}]). + ok = emqx_authz_test_lib:test_allow_topic_rules(ClientInfo, fun setup_client_samples/2), + ok = emqx_authz_test_lib:test_deny_topic_rules(ClientInfo, fun setup_client_samples/2). t_complex_selector(_) -> + %% atom and string values also supported ClientInfo = #{clientid => clientid, username => "username", peerhost => {127,0,0,1}, @@ -205,6 +112,60 @@ t_complex_selector(_) -> ClientInfo, [{allow, publish, <<"t">>}]). +t_mongo_error(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + ok = setup_samples([]), + ok = setup_config( + #{<<"selector">> => #{<<"$badoperator">> => <<"$badoperator">>}}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{deny, publish, <<"t">>}]). + +t_lookups(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + cn => <<"cn">>, + dn => <<"dn">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + ByClientid = #{<<"clientid">> => <<"clientid">>, + <<"topics">> => [<<"a">>], + <<"action">> => <<"all">>, + <<"permission">> => <<"allow">>}, + + ok = setup_samples([ByClientid]), + ok = setup_config( + #{<<"selector">> => #{<<"clientid">> => <<"${clientid}">>}}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]), + + ByPeerhost = #{<<"peerhost">> => <<"127.0.0.1">>, + <<"topics">> => [<<"a">>], + <<"action">> => <<"all">>, + <<"permission">> => <<"allow">>}, + + ok = setup_samples([ByPeerhost]), + ok = setup_config( + #{<<"selector">> => #{<<"peerhost">> => <<"${peerhost}">>}}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]). + %%------------------------------------------------------------------------------ %% Helpers %%------------------------------------------------------------------------------ @@ -217,6 +178,23 @@ setup_samples(AclRecords) -> {{true, _}, _} = mc_worker_api:insert(?MONGO_CLIENT, <<"acl">>, AclRecords), ok. +setup_client_samples(ClientInfo, Samples) -> + #{username := Username} = ClientInfo, + Records = lists:map( + fun(Sample) -> + #{topics := Topics, + permission := Permission, + action := Action} = Sample, + + #{<<"topics">> => Topics, + <<"permission">> => Permission, + <<"action">> => Action, + <<"username">> => Username} + end, + Samples), + setup_samples(Records), + setup_config(#{<<"selector">> => #{<<"username">> => <<"${username}">>}}). + reset_samples() -> {true, _} = mc_worker_api:delete(?MONGO_CLIENT, <<"acl">>, #{}), ok. diff --git a/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl b/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl index 7853564bf..a3036d498 100644 --- a/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl @@ -76,96 +76,10 @@ t_topic_rules(_Config) -> listener => {tcp, default} }, - %% No rules + ok = emqx_authz_test_lib:test_no_topic_rules(ClientInfo, fun setup_client_samples/2), - ok = setup_sample(#{}), - ok = setup_config(#{}), + ok = emqx_authz_test_lib:test_allow_topic_rules(ClientInfo, fun setup_client_samples/2). - ok = emqx_authz_test_lib:test_samples( - ClientInfo, - [{deny, subscribe, <<"#">>}, - {deny, subscribe, <<"subs">>}, - {deny, publish, <<"pub">>}]), - - %% Publish rules - - Sample0 = #{<<"mqtt_user:username">> => - #{<<"testpub1/${username}">> => <<"publish">>, - <<"testpub2/${clientid}">> => <<"publish">>, - <<"testpub3/#">> => <<"publish">> - } - }, - ok = setup_sample(Sample0), - ok = setup_config(#{}), - - ok = emqx_authz_test_lib:test_samples( - ClientInfo, - [{allow, publish, <<"testpub1/username">>}, - {allow, publish, <<"testpub2/clientid">>}, - {allow, publish, <<"testpub3/foobar">>}, - - {deny, publish, <<"testpub2/username">>}, - {deny, publish, <<"testpub1/clientid">>}, - - - {deny, subscribe, <<"testpub1/username">>}, - {deny, subscribe, <<"testpub2/clientid">>}, - {deny, subscribe, <<"testpub3/foobar">>}]), - - %% Subscribe rules - - Sample1 = #{<<"mqtt_user:username">> => - #{<<"testsub1/${username}">> => <<"subscribe">>, - <<"testsub2/${clientid}">> => <<"subscribe">>, - <<"testsub3/#">> => <<"subscribe">> - } - }, - ok = setup_sample(Sample1), - ok = setup_config(#{}), - - ok = emqx_authz_test_lib:test_samples( - ClientInfo, - [{allow, subscribe, <<"testsub1/username">>}, - {allow, subscribe, <<"testsub2/clientid">>}, - {allow, subscribe, <<"testsub3/foobar">>}, - {allow, subscribe, <<"testsub3/+/foobar">>}, - {allow, subscribe, <<"testsub3/#">>}, - - {deny, subscribe, <<"testsub2/username">>}, - {deny, subscribe, <<"testsub1/clientid">>}, - {deny, subscribe, <<"testsub4/foobar">>}, - {deny, publish, <<"testsub1/username">>}, - {deny, publish, <<"testsub2/clientid">>}, - {deny, publish, <<"testsub3/foobar">>}]), - - %% All rules - - Sample2 = #{<<"mqtt_user:username">> => - #{<<"testall1/${username}">> => <<"all">>, - <<"testall2/${clientid}">> => <<"all">>, - <<"testall3/#">> => <<"all">> - } - }, - ok = setup_sample(Sample2), - ok = setup_config(#{}), - - ok = emqx_authz_test_lib:test_samples( - ClientInfo, - [{allow, subscribe, <<"testall1/username">>}, - {allow, subscribe, <<"testall2/clientid">>}, - {allow, subscribe, <<"testall3/foobar">>}, - {allow, subscribe, <<"testall3/+/foobar">>}, - {allow, subscribe, <<"testall3/#">>}, - {allow, publish, <<"testall1/username">>}, - {allow, publish, <<"testall2/clientid">>}, - {allow, publish, <<"testall3/foobar">>}, - - {deny, subscribe, <<"testall2/username">>}, - {deny, subscribe, <<"testall1/clientid">>}, - {deny, subscribe, <<"testall4/foobar">>}, - {deny, publish, <<"testall2/username">>}, - {deny, publish, <<"testall1/clientid">>}, - {deny, publish, <<"testall4/foobar">>}]). t_lookups(_Config) -> ClientInfo = #{clientid => <<"clientid">>, @@ -267,6 +181,23 @@ setup_sample(AuthzData) -> end, maps:to_list(AuthzData)). +setup_client_samples(ClientInfo, Samples) -> + #{username := Username} = ClientInfo, + Key = <<"mqtt_user:", Username/binary>>, + lists:foreach( + fun(Sample) -> + #{topics := Topics, + permission := <<"allow">>, + action := Action} = Sample, + lists:foreach( + fun(Topic) -> + q(["HSET", Key, Topic, Action]) + end, + Topics) + end, + Samples), + setup_config(#{}). + setup_config(SpecialParams) -> ok = emqx_authz_test_lib:reset_authorizers(deny, false), Config = maps:merge(raw_redis_authz_config(), SpecialParams), diff --git a/apps/emqx_authz/test/emqx_authz_test_lib.erl b/apps/emqx_authz/test/emqx_authz_test_lib.erl index 6dd00527e..0af77c2e6 100644 --- a/apps/emqx_authz/test/emqx_authz_test_lib.erl +++ b/apps/emqx_authz/test/emqx_authz_test_lib.erl @@ -41,6 +41,15 @@ setup_config(BaseConfig, SpecialParams) -> {ok, _} = emqx_authz:update(?CMD_REPLACE, [Config]), ok. +is_tcp_server_available(Host, Port) -> + case gen_tcp:connect(Host, Port, [], ?DEFAULT_CHECK_AVAIL_TIMEOUT) of + {ok, Socket} -> + gen_tcp:close(Socket), + true; + {error, _} -> + false + end. + test_samples(ClientInfo, Samples) -> lists:foreach( fun({Expected, Action, Topic}) -> @@ -56,11 +65,185 @@ test_samples(ClientInfo, Samples) -> end, Samples). -is_tcp_server_available(Host, Port) -> - case gen_tcp:connect(Host, Port, [], ?DEFAULT_CHECK_AVAIL_TIMEOUT) of - {ok, Socket} -> - gen_tcp:close(Socket), - true; - {error, _} -> - false - end. +test_no_topic_rules(ClientInfo, SetupSamples) -> + %% No rules + + ok = SetupSamples(ClientInfo, []), + + ok = test_samples( + ClientInfo, + [{deny, subscribe, <<"#">>}, + {deny, subscribe, <<"subs">>}, + {deny, publish, <<"pub">>}]). + +test_allow_topic_rules(ClientInfo, SetupSamples) -> + Samples = [#{ + topics => [<<"eq testpub1/${username}">>, + <<"testpub2/${clientid}">>, + <<"testpub3/#">>], + permission => <<"allow">>, + action => <<"publish">> + }, + #{ + topics => [<<"eq testsub1/${username}">>, + <<"testsub2/${clientid}">>, + <<"testsub3/#">>], + permission => <<"allow">>, + action => <<"subscribe">> + }, + + #{ + topics => [<<"eq testall1/${username}">>, + <<"testall2/${clientid}">>, + <<"testall3/#">>], + permission => <<"allow">>, + action => <<"all">> + } + ], + + ok = SetupSamples(ClientInfo, Samples), + + ok = test_samples( + ClientInfo, + [ + + %% Publish rules + + {deny, publish, <<"testpub1/username">>}, + {allow, publish, <<"testpub1/${username}">>}, + {allow, publish, <<"testpub2/clientid">>}, + {allow, publish, <<"testpub3/foobar">>}, + + {deny, publish, <<"testpub2/username">>}, + {deny, publish, <<"testpub1/clientid">>}, + + + {deny, subscribe, <<"testpub1/username">>}, + {deny, subscribe, <<"testpub2/clientid">>}, + {deny, subscribe, <<"testpub3/foobar">>}, + + %% Subscribe rules + + {deny, subscribe, <<"testsub1/username">>}, + {allow, subscribe, <<"testsub1/${username}">>}, + {allow, subscribe, <<"testsub2/clientid">>}, + {allow, subscribe, <<"testsub3/foobar">>}, + {allow, subscribe, <<"testsub3/+/foobar">>}, + {allow, subscribe, <<"testsub3/#">>}, + + {deny, subscribe, <<"testsub2/username">>}, + {deny, subscribe, <<"testsub1/clientid">>}, + {deny, subscribe, <<"testsub4/foobar">>}, + {deny, publish, <<"testsub1/username">>}, + {deny, publish, <<"testsub2/clientid">>}, + {deny, publish, <<"testsub3/foobar">>}, + + %% All rules + + {deny, subscribe, <<"testall1/username">>}, + {allow, subscribe, <<"testall1/${username}">>}, + {allow, subscribe, <<"testall2/clientid">>}, + {allow, subscribe, <<"testall3/foobar">>}, + {allow, subscribe, <<"testall3/+/foobar">>}, + {allow, subscribe, <<"testall3/#">>}, + {deny, publish, <<"testall1/username">>}, + {allow, publish, <<"testall1/${username}">>}, + {allow, publish, <<"testall2/clientid">>}, + {allow, publish, <<"testall3/foobar">>}, + + {deny, subscribe, <<"testall2/username">>}, + {deny, subscribe, <<"testall1/clientid">>}, + {deny, subscribe, <<"testall4/foobar">>}, + {deny, publish, <<"testall2/username">>}, + {deny, publish, <<"testall1/clientid">>}, + {deny, publish, <<"testall4/foobar">>} + ]). + +test_deny_topic_rules(ClientInfo, SetupSamples) -> + Samples = [ + #{ + topics => [<<"#">>], + permission => <<"allow">>, + action => <<"all">> + }, + #{ + topics => [<<"eq testpub1/${username}">>, + <<"testpub2/${clientid}">>, + <<"testpub3/#">>], + permission => <<"deny">>, + action => <<"publish">> + }, + #{ + topics => [<<"eq testsub1/${username}">>, + <<"testsub2/${clientid}">>, + <<"testsub3/#">>], + permission => <<"deny">>, + action => <<"subscribe">> + }, + + #{ + topics => [<<"eq testall1/${username}">>, + <<"testall2/${clientid}">>, + <<"testall3/#">>], + permission => <<"deny">>, + action => <<"all">> + } + ], + + ok = SetupSamples(ClientInfo, Samples), + + ok = test_samples( + ClientInfo, + [ + + %% Publish rules + + {allow, publish, <<"testpub1/username">>}, + {deny, publish, <<"testpub1/${username}">>}, + {deny, publish, <<"testpub2/clientid">>}, + {deny, publish, <<"testpub3/foobar">>}, + + {allow, publish, <<"testpub2/username">>}, + {allow, publish, <<"testpub1/clientid">>}, + + + {allow, subscribe, <<"testpub1/username">>}, + {allow, subscribe, <<"testpub2/clientid">>}, + {allow, subscribe, <<"testpub3/foobar">>}, + + %% Subscribe rules + + {allow, subscribe, <<"testsub1/username">>}, + {deny, subscribe, <<"testsub1/${username}">>}, + {deny, subscribe, <<"testsub2/clientid">>}, + {deny, subscribe, <<"testsub3/foobar">>}, + {deny, subscribe, <<"testsub3/+/foobar">>}, + {deny, subscribe, <<"testsub3/#">>}, + + {allow, subscribe, <<"testsub2/username">>}, + {allow, subscribe, <<"testsub1/clientid">>}, + {allow, subscribe, <<"testsub4/foobar">>}, + {allow, publish, <<"testsub1/username">>}, + {allow, publish, <<"testsub2/clientid">>}, + {allow, publish, <<"testsub3/foobar">>}, + + %% All rules + + {allow, subscribe, <<"testall1/username">>}, + {deny, subscribe, <<"testall1/${username}">>}, + {deny, subscribe, <<"testall2/clientid">>}, + {deny, subscribe, <<"testall3/foobar">>}, + {deny, subscribe, <<"testall3/+/foobar">>}, + {deny, subscribe, <<"testall3/#">>}, + {allow, publish, <<"testall1/username">>}, + {deny, publish, <<"testall1/${username}">>}, + {deny, publish, <<"testall2/clientid">>}, + {deny, publish, <<"testall3/foobar">>}, + + {allow, subscribe, <<"testall2/username">>}, + {allow, subscribe, <<"testall1/clientid">>}, + {allow, subscribe, <<"testall4/foobar">>}, + {allow, publish, <<"testall2/username">>}, + {allow, publish, <<"testall1/clientid">>}, + {allow, publish, <<"testall4/foobar">>} + ]). From 0a1a68245df24d71d9991d8a686d03a187161c80 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Thu, 16 Dec 2021 14:02:15 +0300 Subject: [PATCH 4/6] chore(authz): test Mysql backend with real Mysql --- apps/emqx_authz/include/emqx_authz.hrl | 2 +- apps/emqx_authz/src/emqx_authz_mysql.erl | 20 +- .../test/emqx_authz_mongodb_SUITE.erl | 19 +- .../test/emqx_authz_mysql_SUITE.erl | 312 +++++++++++++----- .../test/emqx_authz_redis_SUITE.erl | 7 +- apps/emqx_authz/test/emqx_authz_test_lib.erl | 11 +- 6 files changed, 257 insertions(+), 114 deletions(-) diff --git a/apps/emqx_authz/include/emqx_authz.hrl b/apps/emqx_authz/include/emqx_authz.hrl index ae9249bb3..3e1c119be 100644 --- a/apps/emqx_authz/include/emqx_authz.hrl +++ b/apps/emqx_authz/include/emqx_authz.hrl @@ -21,7 +21,7 @@ -define(CONF_KEY_PATH, [authorization, sources]). --define(RE_PLACEHOLDER, "\\$\\{[a-z0-9\\-]+\\}"). +-define(RE_PLACEHOLDER, "\\$\\{[a-z0-9_]+\\}"). -define(USERNAME_RULES_EXAMPLE, #{username => user1, rules => [ #{topic => <<"test/toopic/1">>, diff --git a/apps/emqx_authz/src/emqx_authz_mysql.erl b/apps/emqx_authz/src/emqx_authz_mysql.erl index 118b00a4f..6d91a1101 100644 --- a/apps/emqx_authz/src/emqx_authz_mysql.erl +++ b/apps/emqx_authz/src/emqx_authz_mysql.erl @@ -53,17 +53,6 @@ dry_run(Source) -> destroy(#{annotations := #{id := Id}}) -> ok = emqx_resource:remove(Id). -parse_query(undefined) -> - undefined; -parse_query(Sql) -> - case re:run(Sql, ?RE_PLACEHOLDER, [global, {capture, all, list}]) of - {match, Variables} -> - Params = [Var || [Var] <- Variables], - {re:replace(Sql, ?RE_PLACEHOLDER, "?", [global, {return, list}]), Params}; - nomatch -> - {Sql, []} - end. - authorize(Client, PubSub, Topic, #{annotations := #{id := ResourceID, query := {Query, Params} @@ -80,6 +69,15 @@ authorize(Client, PubSub, Topic, nomatch end. +parse_query(Sql) -> + case re:run(Sql, ?RE_PLACEHOLDER, [global, {capture, all, list}]) of + {match, Variables} -> + Params = [Var || [Var] <- Variables], + {re:replace(Sql, ?RE_PLACEHOLDER, "?", [global, {return, list}]), Params}; + nomatch -> + {Sql, []} + end. + do_authorize(_Client, _PubSub, _Topic, _Columns, []) -> nomatch; do_authorize(Client, PubSub, Topic, Columns, [Row | Tail]) -> diff --git a/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl b/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl index 5b902ec43..679b58c8e 100644 --- a/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_mongodb_SUITE.erl @@ -33,14 +33,6 @@ all() -> groups() -> []. -init_per_testcase(_TestCase, Config) -> - {ok, _} = mc_worker_api:connect(mongo_config()), - Config. - -end_per_testcase(_TestCase, _Config) -> - ok = reset_samples(), - ok = mc_worker_api:disconnect(?MONGO_CLIENT). - init_per_suite(Config) -> case emqx_authz_test_lib:is_tcp_server_available(?MONGO_HOST, ?MONGO_PORT) of true -> @@ -55,7 +47,7 @@ init_per_suite(Config) -> end. end_per_suite(_Config) -> - ok = emqx_authz_test_lib:reset_authorizers(), + ok = emqx_authz_test_lib:restore_authorizers(), ok = stop_apps([emqx_resource, emqx_connector]), ok = emqx_common_test_helpers:stop_apps([emqx_authz]). @@ -65,6 +57,15 @@ set_special_configs(emqx_authz) -> set_special_configs(_) -> ok. +init_per_testcase(_TestCase, Config) -> + {ok, _} = mc_worker_api:connect(mongo_config()), + ok = emqx_authz_test_lib:reset_authorizers(), + Config. + +end_per_testcase(_TestCase, _Config) -> + ok = reset_samples(), + ok = mc_worker_api:disconnect(?MONGO_CLIENT). + %%------------------------------------------------------------------------------ %% Testcases %%------------------------------------------------------------------------------ diff --git a/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl b/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl index 4aa0df606..09974d5a5 100644 --- a/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl @@ -23,7 +23,10 @@ -include_lib("common_test/include/ct.hrl"). -include_lib("emqx/include/emqx_placeholder.hrl"). --define(CONF_DEFAULT, <<"authorization: {sources: []}">>). + +-define(MYSQL_HOST, "mysql"). +-define(MYSQL_PORT, 3306). +-define(MYSQL_RESOURCE, <<"emqx_authz_mysql_SUITE">>). all() -> emqx_common_test_helpers:all(?MODULE). @@ -32,101 +35,240 @@ groups() -> []. init_per_suite(Config) -> - meck:new(emqx_resource, [non_strict, passthrough, no_history, no_link]), - meck:expect(emqx_resource, create, fun(_, _, _) -> {ok, meck_data} end ), - meck:expect(emqx_resource, remove, fun(_) -> ok end ), - - ok = emqx_common_test_helpers:start_apps( - [emqx_conf, emqx_authz], - fun set_special_configs/1), - - Rules = [#{<<"type">> => <<"mysql">>, - <<"server">> => <<"127.0.0.1:27017">>, - <<"pool_size">> => 1, - <<"database">> => <<"mqtt">>, - <<"username">> => <<"xx">>, - <<"password">> => <<"ee">>, - <<"auto_reconnect">> => true, - <<"ssl">> => #{<<"enable">> => false}, - <<"query">> => <<"abcb">> - }], - {ok, _} = emqx_authz:update(replace, Rules), - Config. + case emqx_authn_test_lib:is_tcp_server_available(?MYSQL_HOST, ?MYSQL_PORT) of + true -> + ok = emqx_common_test_helpers:start_apps( + [emqx_conf, emqx_authz], + fun set_special_configs/1 + ), + ok = start_apps([emqx_resource, emqx_connector]), + {ok, _} = emqx_resource:create_local( + ?MYSQL_RESOURCE, + emqx_connector_mysql, + mysql_config()), + Config; + false -> + {skip, no_mysql} + end. end_per_suite(_Config) -> - {ok, _} = emqx:update_config( - [authorization], - #{<<"no_match">> => <<"allow">>, - <<"cache">> => #{<<"enable">> => <<"true">>}, - <<"sources">> => []}), - emqx_common_test_helpers:stop_apps([emqx_authz, emqx_conf]), - meck:unload(emqx_resource), - ok. + ok = emqx_authz_test_lib:restore_authorizers(), + ok = emqx_resource:remove_local(?MYSQL_RESOURCE), + ok = stop_apps([emqx_resource, emqx_connector]), + ok = emqx_common_test_helpers:stop_apps([emqx_authz]). + +init_per_testcase(Config) -> + ok = emqx_authz_test_lib:reset_authorizers(), + Config. set_special_configs(emqx_authz) -> - {ok, _} = emqx:update_config([authorization, cache, enable], false), - {ok, _} = emqx:update_config([authorization, no_match], deny), - {ok, _} = emqx:update_config([authorization, sources], []), - ok; -set_special_configs(_App) -> - ok. + ok = emqx_authz_test_lib:reset_authorizers(); --define(COLUMNS, [ <<"action">> - , <<"permission">> - , <<"topic">> - ]). --define(SOURCE1, [[<<"all">>, <<"deny">>, <<"#">>]]). --define(SOURCE2, [[<<"all">>, <<"allow">>, <<"eq #">>]]). --define(SOURCE3, [[<<"subscribe">>, <<"allow">>, <<"test/", ?PH_CLIENTID/binary>>]]). --define(SOURCE4, [[<<"publish">>, <<"allow">>, <<"test/", ?PH_USERNAME/binary>>]]). +set_special_configs(_) -> + ok. %%------------------------------------------------------------------------------ %% Testcases %%------------------------------------------------------------------------------ -t_authz(_) -> - ClientInfo1 = #{clientid => <<"test">>, - username => <<"test">>, - peerhost => {127,0,0,1}, - zone => default, - listener => {tcp, default} - }, - ClientInfo2 = #{clientid => <<"test_clientid">>, - username => <<"test_username">>, - peerhost => {192,168,0,10}, - zone => default, - listener => {tcp, default} - }, - ClientInfo3 = #{clientid => <<"test_clientid">>, - username => <<"fake_username">>, - peerhost => {127,0,0,1}, - zone => default, - listener => {tcp, default} - }, +t_topic_rules(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, - meck:expect(emqx_resource, query, fun(_, _) -> {ok, ?COLUMNS, []} end), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, subscribe, <<"#">>)), % nomatch - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, publish, <<"#">>)), % nomatch + ok = emqx_authz_test_lib:test_no_topic_rules(ClientInfo, fun setup_client_samples/2), - meck:expect(emqx_resource, query, fun(_, _) -> {ok, ?COLUMNS, ?SOURCE1 ++ ?SOURCE2} end), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, subscribe, <<"+">>)), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, publish, <<"+">>)), + ok = emqx_authz_test_lib:test_allow_topic_rules(ClientInfo, fun setup_client_samples/2), - meck:expect(emqx_resource, query, fun(_, _) -> {ok, ?COLUMNS, ?SOURCE2 ++ ?SOURCE1} end), - ?assertEqual(allow, emqx_access_control:authorize(ClientInfo1, subscribe, <<"#">>)), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, subscribe, <<"+">>)), + ok = emqx_authz_test_lib:test_deny_topic_rules(ClientInfo, fun setup_client_samples/2). - meck:expect(emqx_resource, query, fun(_, _) -> {ok, ?COLUMNS, ?SOURCE3 ++ ?SOURCE4} end), - ?assertEqual(allow, emqx_access_control:authorize( - ClientInfo2, subscribe, <<"test/test_clientid">>)), - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo2, publish, <<"test/test_clientid">>)), - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo2, subscribe, <<"test/test_username">>)), - ?assertEqual(allow, emqx_access_control:authorize( - ClientInfo2, publish, <<"test/test_username">>)), - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo3, subscribe, <<"test">>)), % nomatch - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo3, publish, <<"test">>)), % nomatch - ok. + +t_lookups(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + cn => <<"cn">>, + dn => <<"dn">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + %% by clientid + + ok = init_table(), + ok = q(<<"INSERT INTO acl(clientid, topic, permission, action)" + "VALUES(?, ?, ?, ?)">>, + [<<"clientid">>, <<"a">>, <<"allow">>, <<"subscribe">>]), + + ok = setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE clientid = ${clientid}">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]), + + %% by peerhost + + ok = init_table(), + ok = q(<<"INSERT INTO acl(peerhost, topic, permission, action)" + "VALUES(?, ?, ?, ?)">>, + [<<"127.0.0.1">>, <<"a">>, <<"allow">>, <<"subscribe">>]), + + ok = setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE peerhost = ${peerhost}">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]), + + %% by cn + + ok = init_table(), + ok = q(<<"INSERT INTO acl(cn, topic, permission, action)" + "VALUES(?, ?, ?, ?)">>, + [<<"cn">>, <<"a">>, <<"allow">>, <<"subscribe">>]), + + ok = setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE cn = ${cert_common_name}">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]), + + %% by dn + + ok = init_table(), + ok = q(<<"INSERT INTO acl(dn, topic, permission, action)" + "VALUES(?, ?, ?, ?)">>, + [<<"dn">>, <<"a">>, <<"allow">>, <<"subscribe">>]), + + ok = setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE dn = ${cert_subject}">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]). + +t_mysql_error(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + ok = setup_config( + #{<<"query">> => <<"SOME INVALID STATEMENT">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{deny, subscribe, <<"a">>}]). + + +t_create_invalid(_Config) -> + BadConfig = maps:merge( + raw_mysql_authz_config(), + #{<<"server">> => <<"255.255.255.255:33333">>}), + {error, _} = emqx_authz:update(?CMD_REPLACE, [BadConfig]), + + [] = emqx_authz:lookup(). + +%%------------------------------------------------------------------------------ +%% Helpers +%%------------------------------------------------------------------------------ + +raw_mysql_authz_config() -> + #{ + <<"enable">> => <<"true">>, + + <<"type">> => <<"mysql">>, + <<"database">> => <<"mqtt">>, + <<"username">> => <<"root">>, + <<"password">> => <<"public">>, + + <<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE username = ${username}">>, + + <<"server">> => mysql_server() + }. + +q(Sql) -> + emqx_resource:query( + ?MYSQL_RESOURCE, + {sql, Sql}). + +q(Sql, Params) -> + emqx_resource:query( + ?MYSQL_RESOURCE, + {sql, Sql, Params}). + +init_table() -> + ok = drop_table(), + ok = q("CREATE TABLE acl( + username VARCHAR(255), + clientid VARCHAR(255), + peerhost VARCHAR(255), + cn VARCHAR(255), + dn VARCHAR(255), + topic VARCHAR(255), + permission VARCHAR(255), + action VARCHAR(255))"). + +drop_table() -> + ok = q("DROP TABLE IF EXISTS acl"). + +setup_client_samples(ClientInfo, Samples) -> + #{username := Username} = ClientInfo, + ok = init_table(), + ok = lists:foreach( + fun(#{topics := Topics, permission := Permission, action := Action}) -> + lists:foreach( + fun(Topic) -> + q(<<"INSERT INTO acl(username, topic, permission, action)" + "VALUES(?, ?, ?, ?)">>, + [Username, Topic, Permission, Action]) + end, + Topics) + end, + Samples), + setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE username = ${username}">>}). + +setup_config(SpecialParams) -> + emqx_authz_test_lib:setup_config( + raw_mysql_authz_config(), + SpecialParams). + +mysql_server() -> + iolist_to_binary( + io_lib:format( + "~s:~b", + [?MYSQL_HOST, ?MYSQL_PORT])). + +mysql_config() -> + #{auto_reconnect => true, + database => <<"mqtt">>, + username => <<"root">>, + password => <<"public">>, + pool_size => 8, + server => {?MYSQL_HOST, ?MYSQL_PORT}, + ssl => #{enable => false} + }. + +start_apps(Apps) -> + lists:foreach(fun application:ensure_all_started/1, Apps). + +stop_apps(Apps) -> + lists:foreach(fun application:stop/1, Apps). diff --git a/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl b/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl index a3036d498..93044e044 100644 --- a/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_redis_SUITE.erl @@ -52,11 +52,15 @@ init_per_suite(Config) -> end. end_per_suite(_Config) -> - ok = emqx_authz_test_lib:reset_authorizers(), + ok = emqx_authz_test_lib:restore_authorizers(), ok = emqx_resource:remove_local(?REDIS_RESOURCE), ok = stop_apps([emqx_resource, emqx_connector]), ok = emqx_common_test_helpers:stop_apps([emqx_authz]). +init_per_testcase(Config) -> + ok = emqx_authz_test_lib:reset_authorizers(), + Config. + set_special_configs(emqx_authz) -> ok = emqx_authz_test_lib:reset_authorizers(); @@ -199,7 +203,6 @@ setup_client_samples(ClientInfo, Samples) -> setup_config(#{}). setup_config(SpecialParams) -> - ok = emqx_authz_test_lib:reset_authorizers(deny, false), Config = maps:merge(raw_redis_authz_config(), SpecialParams), {ok, _} = emqx_authz:update(?CMD_REPLACE, [Config]), ok. diff --git a/apps/emqx_authz/test/emqx_authz_test_lib.erl b/apps/emqx_authz/test/emqx_authz_test_lib.erl index 0af77c2e6..68686837d 100644 --- a/apps/emqx_authz/test/emqx_authz_test_lib.erl +++ b/apps/emqx_authz/test/emqx_authz_test_lib.erl @@ -25,6 +25,9 @@ -define(DEFAULT_CHECK_AVAIL_TIMEOUT, 1000). reset_authorizers() -> + reset_authorizers(deny, false). + +restore_authorizers() -> reset_authorizers(allow, true). reset_authorizers(Nomatch, ChacheEnabled) -> @@ -36,7 +39,6 @@ reset_authorizers(Nomatch, ChacheEnabled) -> ok. setup_config(BaseConfig, SpecialParams) -> - ok = reset_authorizers(deny, false), Config = maps:merge(BaseConfig, SpecialParams), {ok, _} = emqx_authz:update(?CMD_REPLACE, [Config]), ok. @@ -101,6 +103,7 @@ test_allow_topic_rules(ClientInfo, SetupSamples) -> } ], + ok = reset_authorizers(deny, false), ok = SetupSamples(ClientInfo, Samples), ok = test_samples( @@ -161,11 +164,6 @@ test_allow_topic_rules(ClientInfo, SetupSamples) -> test_deny_topic_rules(ClientInfo, SetupSamples) -> Samples = [ - #{ - topics => [<<"#">>], - permission => <<"allow">>, - action => <<"all">> - }, #{ topics => [<<"eq testpub1/${username}">>, <<"testpub2/${clientid}">>, @@ -190,6 +188,7 @@ test_deny_topic_rules(ClientInfo, SetupSamples) -> } ], + ok = reset_authorizers(allow, false), ok = SetupSamples(ClientInfo, Samples), ok = test_samples( From 462ec39cb14d59ace119c5a4a992c761a021aca3 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Thu, 16 Dec 2021 15:01:34 +0300 Subject: [PATCH 5/6] chore(authz): test Postgresql backend with real Postgresql --- apps/emqx_authz/src/emqx_authz_postgresql.erl | 2 - .../test/emqx_authz_mysql_SUITE.erl | 24 +- .../test/emqx_authz_postgresql_SUITE.erl | 337 +++++++++++++----- 3 files changed, 276 insertions(+), 87 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_postgresql.erl b/apps/emqx_authz/src/emqx_authz_postgresql.erl index cab6a9f11..f101841c2 100644 --- a/apps/emqx_authz/src/emqx_authz_postgresql.erl +++ b/apps/emqx_authz/src/emqx_authz_postgresql.erl @@ -53,8 +53,6 @@ destroy(#{annotations := #{id := Id}}) -> dry_run(Source) -> emqx_resource:create_dry_run(emqx_connector_pgsql, Source). -parse_query(undefined) -> - undefined; parse_query(Sql) -> case re:run(Sql, ?RE_PLACEHOLDER, [global, {capture, all, list}]) of {match, Capured} -> diff --git a/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl b/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl index 09974d5a5..7db042dd8 100644 --- a/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl @@ -21,7 +21,6 @@ -include("emqx_authz.hrl"). -include_lib("eunit/include/eunit.hrl"). -include_lib("common_test/include/ct.hrl"). --include_lib("emqx/include/emqx_placeholder.hrl"). -define(MYSQL_HOST, "mysql"). @@ -184,6 +183,29 @@ t_create_invalid(_Config) -> [] = emqx_authz:lookup(). +t_nonbinary_values(_Config) -> + ClientInfo = #{clientid => clientid, + username => "username", + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + + ok = init_table(), + ok = q(<<"INSERT INTO acl(clientid, username, topic, permission, action)" + "VALUES(?, ?, ?, ?, ?)">>, + [<<"clientid">>, <<"username">>, <<"a">>, <<"allow">>, <<"subscribe">>]), + + ok = setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE clientid = ${clientid} AND username = ${username}">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]). + %%------------------------------------------------------------------------------ %% Helpers %%------------------------------------------------------------------------------ diff --git a/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl b/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl index 0a4001757..92c479f92 100644 --- a/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl @@ -21,7 +21,11 @@ -include("emqx_authz.hrl"). -include_lib("eunit/include/eunit.hrl"). -include_lib("common_test/include/ct.hrl"). --include_lib("emqx/include/emqx_placeholder.hrl"). + + +-define(PGSQL_HOST, "pgsql"). +-define(PGSQL_PORT, 5432). +-define(PGSQL_RESOURCE, <<"emqx_authz_pgsql_SUITE">>). all() -> emqx_common_test_helpers:all(?MODULE). @@ -30,101 +34,266 @@ groups() -> []. init_per_suite(Config) -> - meck:new(emqx_resource, [non_strict, passthrough, no_history, no_link]), - meck:expect(emqx_resource, create, fun(_, _, _) -> {ok, meck_data} end ), - meck:expect(emqx_resource, remove, fun(_) -> ok end ), - - ok = emqx_common_test_helpers:start_apps( - [emqx_conf, emqx_authz], - fun set_special_configs/1), - - Rules = [#{<<"type">> => <<"postgresql">>, - <<"server">> => <<"127.0.0.1:27017">>, - <<"pool_size">> => 1, - <<"database">> => <<"mqtt">>, - <<"username">> => <<"xx">>, - <<"password">> => <<"ee">>, - <<"auto_reconnect">> => true, - <<"ssl">> => #{<<"enable">> => false}, - <<"query">> => <<"abcb">> - }], - {ok, _} = emqx_authz:update(replace, Rules), - Config. + case emqx_authn_test_lib:is_tcp_server_available(?PGSQL_HOST, ?PGSQL_PORT) of + true -> + ok = emqx_common_test_helpers:start_apps( + [emqx_conf, emqx_authz], + fun set_special_configs/1 + ), + ok = start_apps([emqx_resource, emqx_connector]), + {ok, _} = emqx_resource:create_local( + ?PGSQL_RESOURCE, + emqx_connector_pgsql, + pgsql_config()), + Config; + false -> + {skip, no_pgsql} + end. end_per_suite(_Config) -> - {ok, _} = emqx:update_config( - [authorization], - #{<<"no_match">> => <<"allow">>, - <<"cache">> => #{<<"enable">> => <<"true">>}, - <<"sources">> => []}), - emqx_common_test_helpers:stop_apps([emqx_authz, emqx_conf]), - meck:unload(emqx_resource), - ok. + ok = emqx_authz_test_lib:restore_authorizers(), + ok = emqx_resource:remove_local(?PGSQL_RESOURCE), + ok = stop_apps([emqx_resource, emqx_connector]), + ok = emqx_common_test_helpers:stop_apps([emqx_authz]). + +init_per_testcase(Config) -> + ok = emqx_authz_test_lib:reset_authorizers(), + Config. set_special_configs(emqx_authz) -> - {ok, _} = emqx:update_config([authorization, cache, enable], false), - {ok, _} = emqx:update_config([authorization, no_match], deny), - {ok, _} = emqx:update_config([authorization, sources], []), - ok; -set_special_configs(_App) -> - ok. + ok = emqx_authz_test_lib:reset_authorizers(); --define(COLUMNS, [ {column, <<"action">>, meck, meck, meck, meck, meck, meck, meck} - , {column, <<"permission">>, meck, meck, meck, meck, meck, meck, meck} - , {column, <<"topic">>, meck, meck, meck, meck, meck, meck, meck} - ]). --define(SOURCE1, [{<<"all">>, <<"deny">>, <<"#">>}]). --define(SOURCE2, [{<<"all">>, <<"allow">>, <<"eq #">>}]). --define(SOURCE3, [{<<"subscribe">>, <<"allow">>, <<"test/", ?PH_CLIENTID/binary>>}]). --define(SOURCE4, [{<<"publish">>, <<"allow">>, <<"test/", ?PH_USERNAME/binary>>}]). +set_special_configs(_) -> + ok. %%------------------------------------------------------------------------------ %% Testcases %%------------------------------------------------------------------------------ -t_authz(_) -> - ClientInfo1 = #{clientid => <<"test">>, - username => <<"test">>, - peerhost => {127,0,0,1}, - zone => default, - listener => {tcp, default} - }, - ClientInfo2 = #{clientid => <<"test_clientid">>, - username => <<"test_username">>, - peerhost => {192,168,0,10}, - zone => default, - listener => {tcp, default} - }, - ClientInfo3 = #{clientid => <<"test_clientid">>, - username => <<"fake_username">>, - peerhost => {127,0,0,1}, - zone => default, - listener => {tcp, default} - }, +t_topic_rules(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, - meck:expect(emqx_resource, query, fun(_, _) -> {ok, ?COLUMNS, []} end), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, subscribe, <<"#">>)), % nomatch - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, publish, <<"#">>)), % nomatch + ok = emqx_authz_test_lib:test_no_topic_rules(ClientInfo, fun setup_client_samples/2), - meck:expect(emqx_resource, query, fun(_, _) -> {ok, ?COLUMNS, ?SOURCE1 ++ ?SOURCE2} end), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, subscribe, <<"+">>)), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo1, publish, <<"+">>)), + ok = emqx_authz_test_lib:test_allow_topic_rules(ClientInfo, fun setup_client_samples/2), - meck:expect(emqx_resource, query, fun(_, _) -> {ok, ?COLUMNS, ?SOURCE2 ++ ?SOURCE1} end), - ?assertEqual(allow, emqx_access_control:authorize(ClientInfo1, subscribe, <<"#">>)), - ?assertEqual(deny, emqx_access_control:authorize(ClientInfo2, subscribe, <<"+">>)), + ok = emqx_authz_test_lib:test_deny_topic_rules(ClientInfo, fun setup_client_samples/2). - meck:expect(emqx_resource, query, fun(_, _) -> {ok, ?COLUMNS, ?SOURCE3 ++ ?SOURCE4} end), - ?assertEqual(allow, emqx_access_control:authorize( - ClientInfo2, subscribe, <<"test/test_clientid">>)), - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo2, publish, <<"test/test_clientid">>)), - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo2, subscribe, <<"test/test_username">>)), - ?assertEqual(allow, emqx_access_control:authorize( - ClientInfo2, publish, <<"test/test_username">>)), - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo3, subscribe, <<"test">>)), % nomatch - ?assertEqual(deny, emqx_access_control:authorize( - ClientInfo3, publish, <<"test">>)), % nomatch + +t_lookups(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + cn => <<"cn">>, + dn => <<"dn">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + %% by clientid + + ok = init_table(), + ok = insert(<<"INSERT INTO acl(clientid, topic, permission, action)" + "VALUES($1, $2, $3, $4)">>, + [<<"clientid">>, <<"a">>, <<"allow">>, <<"subscribe">>]), + + ok = setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE clientid = ${clientid}">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]), + + %% by peerhost + + ok = init_table(), + ok = insert(<<"INSERT INTO acl(peerhost, topic, permission, action)" + "VALUES($1, $2, $3, $4)">>, + [<<"127.0.0.1">>, <<"a">>, <<"allow">>, <<"subscribe">>]), + + ok = setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE peerhost = ${peerhost}">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]), + + %% by cn + + ok = init_table(), + ok = insert(<<"INSERT INTO acl(cn, topic, permission, action)" + "VALUES($1, $2, $3, $4)">>, + [<<"cn">>, <<"a">>, <<"allow">>, <<"subscribe">>]), + + ok = setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE cn = ${cert_common_name}">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]), + + %% by dn + + ok = init_table(), + ok = insert(<<"INSERT INTO acl(dn, topic, permission, action)" + "VALUES($1, $2, $3, $4)">>, + [<<"dn">>, <<"a">>, <<"allow">>, <<"subscribe">>]), + + ok = setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE dn = ${cert_subject}">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]). + +t_pgsql_error(_Config) -> + ClientInfo = #{clientid => <<"clientid">>, + username => <<"username">>, + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + ok = setup_config( + #{<<"query">> => <<"SOME INVALID STATEMENT">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{deny, subscribe, <<"a">>}]). + + +t_create_invalid(_Config) -> + BadConfig = maps:merge( + raw_pgsql_authz_config(), + #{<<"server">> => <<"255.255.255.255:33333">>}), + {error, _} = emqx_authz:update(?CMD_REPLACE, [BadConfig]), + + [] = emqx_authz:lookup(). + +t_nonbinary_values(_Config) -> + ClientInfo = #{clientid => clientid, + username => "username", + peerhost => {127,0,0,1}, + zone => default, + listener => {tcp, default} + }, + + + ok = init_table(), + ok = insert(<<"INSERT INTO acl(clientid, username, topic, permission, action)" + "VALUES($1, $2, $3, $4, $5)">>, + [<<"clientid">>, <<"username">>, <<"a">>, <<"allow">>, <<"subscribe">>]), + + ok = setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE clientid = ${clientid} AND username = ${username}">>}), + + ok = emqx_authz_test_lib:test_samples( + ClientInfo, + [{allow, subscribe, <<"a">>}, + {deny, subscribe, <<"b">>}]). + +%%------------------------------------------------------------------------------ +%% Helpers +%%------------------------------------------------------------------------------ + +raw_pgsql_authz_config() -> + #{ + <<"enable">> => <<"true">>, + + <<"type">> => <<"postgresql">>, + <<"database">> => <<"mqtt">>, + <<"username">> => <<"root">>, + <<"password">> => <<"public">>, + + <<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE username = ${username}">>, + + <<"server">> => pgsql_server() + }. + +q(Sql) -> + emqx_resource:query( + ?PGSQL_RESOURCE, + {sql, Sql}). + +insert(Sql, Params) -> + {ok, _} = emqx_resource:query( + ?PGSQL_RESOURCE, + {sql, Sql, Params}), ok. + +init_table() -> + ok = drop_table(), + {ok, _, _} = q("CREATE TABLE acl( + username VARCHAR(255), + clientid VARCHAR(255), + peerhost VARCHAR(255), + cn VARCHAR(255), + dn VARCHAR(255), + topic VARCHAR(255), + permission VARCHAR(255), + action VARCHAR(255))"), + ok. + +drop_table() -> + {ok, _, _} = q("DROP TABLE IF EXISTS acl"), + ok. + +setup_client_samples(ClientInfo, Samples) -> + #{username := Username} = ClientInfo, + ok = init_table(), + ok = lists:foreach( + fun(#{topics := Topics, permission := Permission, action := Action}) -> + lists:foreach( + fun(Topic) -> + insert(<<"INSERT INTO acl(username, topic, permission, action)" + "VALUES($1, $2, $3, $4)">>, + [Username, Topic, Permission, Action]) + end, + Topics) + end, + Samples), + setup_config( + #{<<"query">> => <<"SELECT permission, action, topic " + "FROM acl WHERE username = ${username}">>}). + +setup_config(SpecialParams) -> + emqx_authz_test_lib:setup_config( + raw_pgsql_authz_config(), + SpecialParams). + +pgsql_server() -> + iolist_to_binary( + io_lib:format( + "~s:~b", + [?PGSQL_HOST, ?PGSQL_PORT])). + +pgsql_config() -> + #{auto_reconnect => true, + database => <<"mqtt">>, + username => <<"root">>, + password => <<"public">>, + pool_size => 8, + server => {?PGSQL_HOST, ?PGSQL_PORT}, + ssl => #{enable => false} + }. + +start_apps(Apps) -> + lists:foreach(fun application:ensure_all_started/1, Apps). + +stop_apps(Apps) -> + lists:foreach(fun application:stop/1, Apps). From e24cdb067c4c7feae5f59ea138a861d19ffe4a14 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Thu, 16 Dec 2021 16:21:14 +0300 Subject: [PATCH 6/6] chore(authz): fix Mysql variable substitution --- apps/emqx_authz/src/emqx_authz_mysql.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_mysql.erl b/apps/emqx_authz/src/emqx_authz_mysql.erl index 6d91a1101..a5b14ec1b 100644 --- a/apps/emqx_authz/src/emqx_authz_mysql.erl +++ b/apps/emqx_authz/src/emqx_authz_mysql.erl @@ -108,8 +108,8 @@ replvar([], _ClientInfo, Acc) -> replvar([?PH_S_USERNAME | Params], ClientInfo, Acc) -> replvar(Params, ClientInfo, [safe_get(username, ClientInfo) | Acc]); -replvar([?PH_S_CLIENTID | Params], ClientInfo = #{clientid := ClientId}, Acc) -> - replvar(Params, ClientInfo, [ClientId | Acc]); +replvar([?PH_S_CLIENTID | Params], ClientInfo = #{clientid := _ClientId}, Acc) -> + replvar(Params, ClientInfo, [safe_get(clientid, ClientInfo) | Acc]); replvar([?PH_S_PEERHOST | Params], ClientInfo = #{peerhost := IpAddr}, Acc) -> replvar(Params, ClientInfo, [inet_parse:ntoa(IpAddr) | Acc]); replvar([?PH_S_CERT_CN_NAME | Params], ClientInfo, Acc) ->