diff --git a/Makefile b/Makefile index 8c5eb3048..7362cdac4 100644 --- a/Makefile +++ b/Makefile @@ -130,14 +130,14 @@ $(foreach app,$(APPS),$(eval $(call gen-app-prop-target,$(app)))) ct-suite: $(REBAR) merge-config clean-test-cluster-config ifneq ($(TESTCASE),) ifneq ($(GROUP),) - $(REBAR) ct -v --readable=$(CT_READABLE) --name $(CT_NODE_NAME) --suite $(SUITE) --case $(TESTCASE) --group $(GROUP) + $(REBAR) ct -c -v --readable=$(CT_READABLE) --name $(CT_NODE_NAME) --suite $(SUITE) --case $(TESTCASE) --group $(GROUP) else - $(REBAR) ct -v --readable=$(CT_READABLE) --name $(CT_NODE_NAME) --suite $(SUITE) --case $(TESTCASE) + $(REBAR) ct -c -v --readable=$(CT_READABLE) --name $(CT_NODE_NAME) --suite $(SUITE) --case $(TESTCASE) endif else ifneq ($(GROUP),) - $(REBAR) ct -v --readable=$(CT_READABLE) --name $(CT_NODE_NAME) --suite $(SUITE) --group $(GROUP) + $(REBAR) ct -c -v --readable=$(CT_READABLE) --name $(CT_NODE_NAME) --suite $(SUITE) --group $(GROUP) else - $(REBAR) ct -v --readable=$(CT_READABLE) --name $(CT_NODE_NAME) --suite $(SUITE) + $(REBAR) ct -c -v --readable=$(CT_READABLE) --name $(CT_NODE_NAME) --suite $(SUITE) endif .PHONY: cover diff --git a/apps/emqx_authz/src/emqx_authz_mongodb.erl b/apps/emqx_authz/src/emqx_authz_mongodb.erl index 7adb6d2d9..52a920d3a 100644 --- a/apps/emqx_authz/src/emqx_authz_mongodb.erl +++ b/apps/emqx_authz/src/emqx_authz_mongodb.erl @@ -77,14 +77,7 @@ authorize( } ) -> RenderedFilter = emqx_authz_utils:render_deep(FilterTemplate, Client), - Result = - try - emqx_resource:simple_sync_query(ResourceID, {find, Collection, RenderedFilter, #{}}) - catch - error:Error -> {error, Error} - end, - - case Result of + case emqx_resource:simple_sync_query(ResourceID, {find, Collection, RenderedFilter, #{}}) of {error, Reason} -> ?SLOG(error, #{ msg => "query_mongo_error", @@ -94,8 +87,6 @@ authorize( resource_id => ResourceID }), nomatch; - {ok, []} -> - nomatch; {ok, Rows} -> Rules = lists:flatmap(fun parse_rule/1, Rows), do_authorize(Client, Action, Topic, Rules) diff --git a/apps/emqx_authz/src/emqx_authz_mysql.erl b/apps/emqx_authz/src/emqx_authz_mysql.erl index 01debaea9..a724e451c 100644 --- a/apps/emqx_authz/src/emqx_authz_mysql.erl +++ b/apps/emqx_authz/src/emqx_authz_mysql.erl @@ -86,8 +86,6 @@ authorize( case emqx_resource:simple_sync_query(ResourceID, {prepared_query, ?PREPARE_KEY, RenderParams}) of - {ok, _ColumnNames, []} -> - nomatch; {ok, ColumnNames, Rows} -> do_authorize(Client, Action, Topic, ColumnNames, Rows); {error, Reason} -> diff --git a/apps/emqx_authz/src/emqx_authz_postgresql.erl b/apps/emqx_authz/src/emqx_authz_postgresql.erl index 1b05451cc..f0bdf77be 100644 --- a/apps/emqx_authz/src/emqx_authz_postgresql.erl +++ b/apps/emqx_authz/src/emqx_authz_postgresql.erl @@ -93,8 +93,6 @@ authorize( case emqx_resource:simple_sync_query(ResourceID, {prepared_query, ResourceID, RenderedParams}) of - {ok, _Columns, []} -> - nomatch; {ok, Columns, Rows} -> do_authorize(Client, Action, Topic, column_names(Columns), Rows); {error, Reason} -> diff --git a/apps/emqx_authz/src/emqx_authz_redis.erl b/apps/emqx_authz/src/emqx_authz_redis.erl index 01149b5bb..d163c0d16 100644 --- a/apps/emqx_authz/src/emqx_authz_redis.erl +++ b/apps/emqx_authz/src/emqx_authz_redis.erl @@ -80,8 +80,6 @@ authorize( Vars = emqx_authz_utils:vars_for_rule_query(Client, Action), Cmd = emqx_authz_utils:render_deep(CmdTemplate, Vars), case emqx_resource:simple_sync_query(ResourceID, {cmd, Cmd}) of - {ok, []} -> - nomatch; {ok, Rows} -> do_authorize(Client, Action, Topic, Rows); {error, Reason} -> @@ -108,12 +106,13 @@ do_authorize(Client, Action, Topic, [TopicFilterRaw, RuleEncoded | Tail]) -> {matched, Permission} -> {matched, Permission}; nomatch -> do_authorize(Client, Action, Topic, Tail) catch - error:Reason -> + error:Reason:Stack -> ?SLOG(error, #{ msg => "match_rule_error", reason => Reason, rule_encoded => RuleEncoded, - topic_filter_raw => TopicFilterRaw + topic_filter_raw => TopicFilterRaw, + stacktrace => Stack }), do_authorize(Client, Action, Topic, Tail) end. @@ -148,6 +147,8 @@ parse_rule(Bin) when is_binary(Bin) -> case emqx_utils_json:safe_decode(Bin, [return_maps]) of {ok, Map} when is_map(Map) -> maps:with([<<"qos">>, <<"action">>, <<"retain">>], Map); + {ok, _} -> + error({invalid_topic_rule, Bin, notamap}); {error, Error} -> error({invalid_topic_rule, Bin, Error}) end. diff --git a/apps/emqx_authz/test/emqx_authz_file_SUITE.erl b/apps/emqx_authz/test/emqx_authz_file_SUITE.erl index 0ce788f8d..396679783 100644 --- a/apps/emqx_authz/test/emqx_authz_file_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_file_SUITE.erl @@ -38,21 +38,19 @@ all() -> groups() -> []. -init_per_suite(Config) -> - Config. - -end_per_suite(_Config) -> - ok = emqx_authz_test_lib:restore_authorizers(). - init_per_testcase(TestCase, Config) -> Apps = emqx_cth_suite:start( - [{emqx_conf, "authorization.no_match = deny, authorization.cache.enable = false"}, emqx_authz], + [ + {emqx_conf, "authorization.no_match = deny, authorization.cache.enable = false"}, + emqx_authz + ], #{work_dir => filename:join(?config(priv_dir, Config), TestCase)} ), [{tc_apps, Apps} | Config]. end_per_testcase(_TestCase, Config) -> - emqx_cth_suite:stop(?config(tc_apps, Config)). + emqx_cth_suite:stop(?config(tc_apps, Config)), + _ = emqx_authz:set_feature_available(rich_actions, true). %%------------------------------------------------------------------------------ %% Testcases diff --git a/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl b/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl index f31a6ceab..4304dd505 100644 --- a/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl @@ -333,19 +333,41 @@ cases() -> }, #{ name => invalid_query, - setup => [], - query => "SELECT permission, action, topic FROM acl WHER", + setup => [ + "CREATE TABLE acl(username VARCHAR(255), topic VARCHAR(255), " + "permission VARCHAR(255), action VARCHAR(255))" + ], + query => "SELECT permission, action, topic FRO", checks => [ {deny, ?AUTHZ_PUBLISH, <<"a">>} ] }, #{ - name => pgsql_error, - setup => [], + name => runtime_error, + setup => [ + "CREATE TABLE acl(username VARCHAR(255), topic VARCHAR(255), " + "permission VARCHAR(255), action VARCHAR(255))" + ], query => - "SELECT permission, action, topic FROM table_not_exists WHERE username = ${username}", + "SELECT permission, action, topic FROM acl WHERE username = ${username}", checks => [ - {deny, ?AUTHZ_PUBLISH, <<"t">>} + fun() -> + _ = q("DROP TABLE IF EXISTS acl"), + {deny, ?AUTHZ_PUBLISH, <<"t">>} + end + ] + }, + #{ + name => invalid_rule, + setup => [ + "CREATE TABLE acl(username VARCHAR(255), topic VARCHAR(255), " + "permission VARCHAR(255), action VARCHAR(255))", + %% 'permit' is invalid value for action + "INSERT INTO acl(username, topic, permission, action) VALUES('username', 'a', 'permit', 'publish')" + ], + query => "SELECT permission, action, topic FROM acl WHERE username = ${username}", + checks => [ + {deny, ?AUTHZ_PUBLISH, <<"a">>} ] } ]. diff --git a/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl b/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl index 0c446ee99..a9181879e 100644 --- a/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_postgresql_SUITE.erl @@ -352,6 +352,19 @@ cases() -> checks => [ {deny, ?AUTHZ_PUBLISH, <<"t">>} ] + }, + #{ + name => invalid_rule, + setup => [ + "CREATE TABLE acl(username VARCHAR(255), topic VARCHAR(255), " + "permission VARCHAR(255), action VARCHAR(255))", + %% 'permit' is invalid value for action + "INSERT INTO acl(username, topic, permission, action) VALUES('username', 'a', 'permit', 'publish')" + ], + query => "SELECT permission, action, topic FROM acl WHERE username = ${username}", + checks => [ + {deny, ?AUTHZ_PUBLISH, <<"a">>} + ] } %% TODO: add case for unknown variables after fixing EMQX-10400 ]. diff --git a/apps/emqx_authz/test/emqx_authz_rule_raw_SUITE.erl b/apps/emqx_authz/test/emqx_authz_rule_raw_SUITE.erl index 8b097c3fd..798661b53 100644 --- a/apps/emqx_authz/test/emqx_authz_rule_raw_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_rule_raw_SUITE.erl @@ -181,6 +181,14 @@ ok_cases() -> expected_rule_with_qos_retain([0, 1, 2], all), rule_with_raw_qos_retain(#{}) }, + { + expected_rule_with_qos_retain([0, 1, 2], true), + rule_with_raw_qos_retain(#{<<"retain">> => <<"1">>}) + }, + { + expected_rule_with_qos_retain([0, 1, 2], false), + rule_with_raw_qos_retain(#{<<"retain">> => <<"0">>}) + }, %% Qos { expected_rule_with_qos_retain([2], all), @@ -254,6 +262,12 @@ error_rich_action_cases() -> <<"topics">> => [], <<"action">> => <<"publish">>, <<"retain">> => 3 + }, + #{ + <<"permission">> => <<"allow">>, + <<"topics">> => [], + <<"action">> => <<"publish">>, + <<"qos">> => [<<"3">>] } ]. diff --git a/apps/emqx_authz/test/emqx_authz_test_lib.erl b/apps/emqx_authz/test/emqx_authz_test_lib.erl index ea62c5b71..33035c766 100644 --- a/apps/emqx_authz/test/emqx_authz_test_lib.erl +++ b/apps/emqx_authz/test/emqx_authz_test_lib.erl @@ -107,6 +107,8 @@ run_checks(#{checks := Checks} = Case) -> Checks ). +run_check(ClientInfo, Fun) when is_function(Fun, 0) -> + run_check(ClientInfo, Fun()); run_check(ClientInfo, {ExpectedPermission, Action, Topic}) -> ?assertEqual( ExpectedPermission, diff --git a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl index 52f96bcd2..9f3344e95 100644 --- a/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl +++ b/apps/emqx_gateway/src/bhvrs/emqx_gateway_conn.erl @@ -715,13 +715,13 @@ parse_incoming( NState = State#state{parse_state = NParseState}, parse_incoming(Rest, [Packet | Packets], NState) catch - error:Reason:Stk -> + error:Reason:Stack -> ?SLOG(error, #{ msg => "parse_frame_failed", at_state => ParseState, input_bytes => Data, reason => Reason, - stacktrace => Stk + stacktrace => Stack }), {[{frame_error, Reason} | Packets], State} end. diff --git a/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl b/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl index ce809184e..091978172 100644 --- a/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl +++ b/apps/emqx_gateway_coap/test/emqx_coap_SUITE.erl @@ -59,15 +59,14 @@ init_per_suite(Config) -> application:load(emqx_gateway_coap), ok = emqx_common_test_helpers:load_config(emqx_gateway_schema, ?CONF_DEFAULT), emqx_mgmt_api_test_util:init_suite([emqx_conf, emqx_authn, emqx_gateway]), - ok = meck:new(emqx_access_control, [passthrough, no_history, no_link]), Config. end_per_suite(_) -> - meck:unload(emqx_access_control), {ok, _} = emqx:remove_config([<<"gateway">>, <<"coap">>]), emqx_mgmt_api_test_util:end_suite([emqx_gateway, emqx_authn, emqx_conf]). init_per_testcase(t_connection_with_authn_failed, Config) -> + ok = meck:new(emqx_access_control, [passthrough]), ok = meck:expect( emqx_access_control, authenticate, @@ -75,12 +74,11 @@ init_per_testcase(t_connection_with_authn_failed, Config) -> ), Config; init_per_testcase(_, Config) -> + ok = meck:new(emqx_access_control, [passthrough]), Config. -end_per_testcase(t_connection_with_authn_failed, Config) -> - ok = meck:delete(emqx_access_control, authenticate, 1), - Config; end_per_testcase(_, Config) -> + ok = meck:unload(emqx_access_control), Config. default_config() -> @@ -213,6 +211,38 @@ t_publish(_) -> end, with_connection(Topics, Action). +t_publish_with_retain_qos_expiry(_) -> + _ = meck:expect( + emqx_access_control, + authorize, + fun(_, #{action_type := publish, qos := 1, retain := true}, _) -> + allow + end + ), + + Topics = [<<"abc">>], + Action = fun(Topic, Channel, Token) -> + Payload = <<"123">>, + URI = pubsub_uri(binary_to_list(Topic), Token) ++ "&retain=true&qos=1&expiry=60", + + %% Sub topic first + emqx:subscribe(Topic), + + Req = make_req(post, Payload), + {ok, changed, _} = do_request(Channel, URI, Req), + + receive + {deliver, Topic, Msg} -> + ?assertEqual(Topic, Msg#message.topic), + ?assertEqual(Payload, Msg#message.payload) + after 500 -> + ?assert(false) + end + end, + with_connection(Topics, Action), + + _ = meck:validate(emqx_access_control). + t_subscribe(_) -> %% can subscribe to a normal topic Topics = [ diff --git a/apps/emqx_mysql/src/emqx_mysql.erl b/apps/emqx_mysql/src/emqx_mysql.erl index 2a4db3147..c9273f3f1 100644 --- a/apps/emqx_mysql/src/emqx_mysql.erl +++ b/apps/emqx_mysql/src/emqx_mysql.erl @@ -361,7 +361,7 @@ prepare_sql_to_conn(Conn, [{Key, SQL} | PrepareList]) when is_pid(Conn) -> ?SLOG(error, LogMeta#{result => failed, reason => Reason}), {error, undefined_table}; {error, Reason} -> - % FIXME: we should try to differ on transient failers and + % FIXME: we should try to differ on transient failures and % syntax failures. Retrying syntax failures is not very productive. ?SLOG(error, LogMeta#{result => failed, reason => Reason}), {error, Reason}