Merge pull request #13481 from zhongwencool/match_rule_error

chore: add authz tag to match_rule_error log

apps/emqx_auth/src/emqx_authz/emqx_authz_rule.erl

@@ -163,7 +163,7 @@ qos_from_opts(Opts) ->
                 )
         end
     catch
-        {bad_qos, QoS} ->
+        throw:{bad_qos, QoS} ->
             throw(#{
                 reason => invalid_authorization_qos,
                 qos => QoS

apps/emqx_auth/src/emqx_authz/emqx_authz_utils.erl

@@ -16,6 +16,8 @@
 
 -module(emqx_authz_utils).
 
+-feature(maybe_expr, enable).
+
 -include_lib("emqx/include/emqx_placeholder.hrl").
 -include_lib("emqx_authz.hrl").
 -include_lib("snabbkaffe/include/trace.hrl").
@@ -37,7 +39,7 @@
     render_sql_params/2,
     client_vars/1,
     vars_for_rule_query/2,
-    parse_rule_from_row/2
+    do_authorize/6
 ]).
 
 -export([
@@ -221,14 +223,18 @@ content_type(Headers) when is_list(Headers) ->
 
 -define(RAW_RULE_KEYS, [<<"permission">>, <<"action">>, <<"topic">>, <<"qos">>, <<"retain">>]).
 
-parse_rule_from_row(ColumnNames, Row) ->
-    RuleRaw = maps:with(?RAW_RULE_KEYS, maps:from_list(lists:zip(ColumnNames, to_list(Row)))),
-    case emqx_authz_rule_raw:parse_rule(RuleRaw) of
+-spec parse_rule_from_row([binary()], [binary()] | map()) ->
+    {ok, emqx_authz_rule:rule()} | {error, term()}.
+parse_rule_from_row(_ColumnNames, RuleMap = #{}) ->
+    case emqx_authz_rule_raw:parse_rule(RuleMap) of
         {ok, {Permission, Action, Topics}} ->
-            emqx_authz_rule:compile({Permission, all, Action, Topics});
+            {ok, emqx_authz_rule:compile({Permission, all, Action, Topics})};
         {error, Reason} ->
-            error(Reason)
-    end.
+            {error, Reason}
+    end;
+parse_rule_from_row(ColumnNames, Row) ->
+    RuleMap = maps:with(?RAW_RULE_KEYS, maps:from_list(lists:zip(ColumnNames, to_list(Row)))),
+    parse_rule_from_row(ColumnNames, RuleMap).
 
 vars_for_rule_query(Client, ?authz_action(PubSub, Qos) = Action) ->
     Client#{
@@ -281,3 +287,39 @@ to_list(Tuple) when is_tuple(Tuple) ->
     tuple_to_list(Tuple);
 to_list(List) when is_list(List) ->
     List.
+
+do_authorize(Type, Client, Action, Topic, ColumnNames, Row) ->
+    try
+        maybe
+            {ok, Rule} ?= parse_rule_from_row(ColumnNames, Row),
+            {matched, Permission} ?= emqx_authz_rule:match(Client, Action, Topic, Rule),
+            {matched, Permission}
+        else
+            nomatch ->
+                nomatch;
+            {error, Reason0} ->
+                log_match_rule_error(Type, Row, Reason0),
+                nomatch
+        end
+    catch
+        throw:Reason1 ->
+            log_match_rule_error(Type, Row, Reason1),
+            nomatch
+    end.
+
+log_match_rule_error(Type, Row, Reason0) ->
+    Msg0 = #{
+        msg => "match_rule_error",
+        rule => Row,
+        type => Type
+    },
+    Msg1 =
+        case is_map(Reason0) of
+            true -> maps:merge(Msg0, Reason0);
+            false -> Msg0#{reason => Reason0}
+        end,
+    ?SLOG(
+        error,
+        Msg1,
+        #{tag => "AUTHZ"}
+    ).

apps/emqx_auth_mysql/src/emqx_auth_mysql.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_auth_mysql, [
     {description, "EMQX MySQL Authentication and Authorization"},
-    {vsn, "0.2.0"},
+    {vsn, "0.2.1"},
     {registered, []},
     {mod, {emqx_auth_mysql_app, []}},
     {applications, [

apps/emqx_auth_mysql/src/emqx_authz_mysql.erl

@@ -101,19 +101,9 @@ authorize(
 do_authorize(_Client, _Action, _Topic, _ColumnNames, []) ->
     nomatch;
 do_authorize(Client, Action, Topic, ColumnNames, [Row | Tail]) ->
-    try
-        emqx_authz_rule:match(
-            Client, Action, Topic, emqx_authz_utils:parse_rule_from_row(ColumnNames, Row)
-        )
-    of
-        {matched, Permission} -> {matched, Permission};
-        nomatch -> do_authorize(Client, Action, Topic, ColumnNames, Tail)
-    catch
-        error:Reason ->
-            ?SLOG(error, #{
-                msg => "match_rule_error",
-                reason => Reason,
-                rule => Row
-            }),
-            do_authorize(Client, Action, Topic, ColumnNames, Tail)
+    case emqx_authz_utils:do_authorize(mysql, Client, Action, Topic, ColumnNames, Row) of
+        nomatch ->
+            do_authorize(Client, Action, Topic, ColumnNames, Tail);
+        {matched, Permission} ->
+            {matched, Permission}
     end.

apps/emqx_auth_postgresql/src/emqx_auth_postgresql.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_auth_postgresql, [
     {description, "EMQX PostgreSQL Authentication and Authorization"},
-    {vsn, "0.2.0"},
+    {vsn, "0.2.1"},
     {registered, []},
     {mod, {emqx_auth_postgresql_app, []}},
     {applications, [

apps/emqx_auth_postgresql/src/emqx_authz_postgresql.erl

@@ -107,22 +107,11 @@ authorize(
 do_authorize(_Client, _Action, _Topic, _ColumnNames, []) ->
     nomatch;
 do_authorize(Client, Action, Topic, ColumnNames, [Row | Tail]) ->
-    try
-        emqx_authz_rule:match(
-            Client, Action, Topic, emqx_authz_utils:parse_rule_from_row(ColumnNames, Row)
-        )
-    of
-        {matched, Permission} -> {matched, Permission};
-        nomatch -> do_authorize(Client, Action, Topic, ColumnNames, Tail)
-    catch
-        error:Reason:Stack ->
-            ?SLOG(error, #{
-                msg => "match_rule_error",
-                reason => Reason,
-                rule => Row,
-                stack => Stack
-            }),
-            do_authorize(Client, Action, Topic, ColumnNames, Tail)
+    case emqx_authz_utils:do_authorize(postgresql, Client, Action, Topic, ColumnNames, Row) of
+        nomatch ->
+            do_authorize(Client, Action, Topic, ColumnNames, Tail);
+        {matched, Permission} ->
+            {matched, Permission}
     end.
 
 column_names(Columns) ->

apps/emqx_auth_redis/src/emqx_auth_redis.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_auth_redis, [
     {description, "EMQX Redis Authentication and Authorization"},
-    {vsn, "0.2.0"},
+    {vsn, "0.2.1"},
     {registered, []},
     {mod, {emqx_auth_redis_app, []}},
     {applications, [

apps/emqx_auth_redis/src/emqx_authz_redis.erl

@@ -92,44 +92,30 @@ authorize(
 do_authorize(_Client, _Action, _Topic, []) ->
     nomatch;
 do_authorize(Client, Action, Topic, [TopicFilterRaw, RuleEncoded | Tail]) ->
-    try
-        emqx_authz_rule:match(
-            Client,
-            Action,
-            Topic,
-            compile_rule(RuleEncoded, TopicFilterRaw)
-        )
-    of
-        {matched, Permission} -> {matched, Permission};
-        nomatch -> do_authorize(Client, Action, Topic, Tail)
-    catch
-        error:Reason:Stack ->
-            ?SLOG(error, #{
-                msg => "match_rule_error",
-                reason => Reason,
-                rule_encoded => RuleEncoded,
-                topic_filter_raw => TopicFilterRaw,
-                stacktrace => Stack
+    case parse_rule(RuleEncoded) of
+        {ok, RuleMap0} ->
+            RuleMap =
+                maps:merge(
+                    #{
+                        <<"permission">> => <<"allow">>,
+                        <<"topic">> => TopicFilterRaw
+                    },
+                    RuleMap0
+                ),
+            case emqx_authz_utils:do_authorize(redis, Client, Action, Topic, undefined, RuleMap) of
+                nomatch ->
+                    do_authorize(Client, Action, Topic, Tail);
+                {matched, Permission} ->
+                    {matched, Permission}
+            end;
+        {error, Reason} ->
+            ?SLOG(error, Reason#{
+                msg => "parse_rule_error",
+                rule => RuleEncoded
             }),
             do_authorize(Client, Action, Topic, Tail)
     end.
 
-compile_rule(RuleBin, TopicFilterRaw) ->
-    RuleRaw =
-        maps:merge(
-            #{
-                <<"permission">> => <<"allow">>,
-                <<"topic">> => TopicFilterRaw
-            },
-            parse_rule(RuleBin)
-        ),
-    case emqx_authz_rule_raw:parse_rule(RuleRaw) of
-        {ok, {Permission, Action, Topics}} ->
-            emqx_authz_rule:compile({Permission, all, Action, Topics});
-        {error, Reason} ->
-            error(Reason)
-    end.
-
 parse_cmd(Query) ->
     case emqx_redis_command:split(Query) of
         {ok, Cmd} ->
@@ -154,17 +140,17 @@ validate_cmd(Cmd) ->
     end.
 
 parse_rule(<<"publish">>) ->
-    #{<<"action">> => <<"publish">>};
+    {ok, #{<<"action">> => <<"publish">>}};
 parse_rule(<<"subscribe">>) ->
-    #{<<"action">> => <<"subscribe">>};
+    {ok, #{<<"action">> => <<"subscribe">>}};
 parse_rule(<<"all">>) ->
-    #{<<"action">> => <<"all">>};
+    {ok, #{<<"action">> => <<"all">>}};
 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, maps:with([<<"qos">>, <<"action">>, <<"retain">>], Map)};
         {ok, _} ->
-            error({invalid_topic_rule, Bin, notamap});
-        {error, Error} ->
-            error({invalid_topic_rule, Bin, Error})
+            {error, #{reason => invalid_topic_rule_not_map, value => Bin}};
+        {error, _Error} ->
+            {error, #{reason => invalid_topic_rule_not_json, value => Bin}}
     end.