feat(authz): add tests for authz extended actions

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

apps/emqx_authz/src/emqx_authz_mongodb.erl

@@ -77,14 +77,7 @@ authorize(
     }
 ) ->
     RenderedFilter = emqx_authz_utils:render_deep(FilterTemplate, Client),
-    Result =
+    case emqx_resource:simple_sync_query(ResourceID, {find, Collection, RenderedFilter, #{}}) of
-        try
-            emqx_resource:simple_sync_query(ResourceID, {find, Collection, RenderedFilter, #{}})
-        catch
-            error:Error -> {error, Error}
-        end,
-
-    case Result 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)

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} ->

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} ->

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.

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

apps/emqx_authz/test/emqx_authz_mysql_SUITE.erl

@@ -333,19 +333,41 @@ cases() ->
         },
         #{
             name => invalid_query,
-            setup => [],
+            setup => [
-            query => "SELECT permission, action, topic FROM acl WHER",
+                "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,
+            name => runtime_error,
-            setup => [],
+            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">>}
             ]
         }
     ].

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
     ].

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">>]
         }
     ].
 

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,

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.

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 = [

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}