Merge pull request #8868 from thalesmg/merge-v43-into-v44-f

Merge v43 into v44 f

.github/workflows/run_test_cases.yaml

@@ -39,16 +39,8 @@ jobs:
                 use-self-hosted: false
         steps:
         - uses: actions/checkout@v2
-        - name: set edition
-          id: set_edition
-          run: |
-            if make emqx-ee --dry-run > /dev/null 2>&1; then
-                echo "EDITION=enterprise" >> $GITHUB_ENV
-            else
-                echo "EDITION=opensource" >> $GITHUB_ENV
-            fi
         - name: docker compose up
-          if: env.EDITION == 'opensource'
+          if: endsWith(github.repository, 'emqx')
           env:
             MYSQL_TAG: 8
             REDIS_TAG: 6
@@ -66,7 +58,7 @@ jobs:
                 -f .ci/docker-compose-file/docker-compose-redis-single-tcp.yaml \
                 up -d --build
         - name: docker compose up
-          if: env.EDITION == 'enterprise'
+          if: endsWith(github.repository, 'emqx-enterprise')
           env:
             MYSQL_TAG: 8
             REDIS_TAG: 6

CHANGES-4.3.md

@@ -10,6 +10,12 @@ File format:
 - One list item per change topic
   Change log ends with a list of GitHub PRs
 
+## v4.3.20
+
+### Bug fixes
+
+- Fix rule-engine update behaviour which may initialize actions for disabled rules. [#8849](https://github.com/emqx/emqx/pull/8849)
+
 ## v4.3.19
 
 ### Enhancements

apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src

@@ -1,6 +1,6 @@
 {application, emqx_auth_jwt,
  [{description, "EMQ X Authentication with JWT"},
-  {vsn, "4.4.4"}, % strict semver, bump manually!
+  {vsn, "4.4.5"}, % strict semver, bump manually!
   {modules, []},
   {registered, [emqx_auth_jwt_sup]},
   {applications, [kernel,stdlib,jose]},

apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src

@@ -1,13 +1,25 @@
 %% -*- mode: erlang -*-
 %% Unless you know what you are doing, DO NOT edit manually!!
 {VSN,
-  [{"4.4.3",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]},
+  [{"4.4.4",
-   {"4.4.2",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]},
-             {load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]},
+     {load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]},
+   {"4.4.3",
+    [{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]},
+     {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]},
+   {"4.4.2",
+    [{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]},
+     {load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]},
    {<<"4\\.4\\.[0-1]">>,[{restart_application,emqx_auth_jwt}]},
    {<<".*">>,[]}],
-  [{"4.4.3",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]},
+  [{"4.4.4",
-   {"4.4.2",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]},
+    [{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]},
-             {load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]},
+     {load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]},
+   {"4.4.3",
+    [{load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]},
+     {load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}]},
+   {"4.4.2",
+    [{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]},
+     {load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]},
    {<<"4\\.4\\.[0-1]">>,[{restart_application,emqx_auth_jwt}]},
    {<<".*">>,[]}]}.

apps/emqx_auth_jwt/src/emqx_auth_jwt.erl

@@ -26,6 +26,8 @@
         , description/0
         ]).
 
+-export([string_to_number/1]).
+
 %%--------------------------------------------------------------------
 %% Authentication callbacks
 %%--------------------------------------------------------------------
@@ -56,16 +58,12 @@ check_acl(ClientInfo = #{jwt_claims := Claims},
           #{acl_claim_name := AclClaimName}) ->
     case Claims of
         #{AclClaimName := Acl, <<"exp">> := Exp} ->
-            try is_expired(Exp) of
+            case is_expired(Exp) of
                 true ->
                     ?DEBUG("acl_deny_due_to_jwt_expired", []),
                     deny;
                 false ->
                     verify_acl(ClientInfo, Acl, PubSub, Topic)
-            catch
-                _:_ ->
-                    ?DEBUG("acl_deny_due_to_invalid_jwt_exp", []),
-                    deny
             end;
         #{AclClaimName := Acl} ->
             verify_acl(ClientInfo, Acl, PubSub, Topic);
@@ -81,14 +79,29 @@ check_acl(_ClientInfo,
     ignore.
 
 is_expired(Exp) when is_binary(Exp)  ->
-    ExpInt  = binary_to_integer(Exp),
+    case string_to_number(Exp) of
-    is_expired(ExpInt);
+        {ok, Val} ->
-is_expired(Exp) ->
+            is_expired(Val);
+        _ ->
+            ?DEBUG("acl_deny_due_to_invalid_jwt_exp:~p", [Exp]),
+            true
+    end;
+is_expired(Exp) when is_integer(Exp) ->
     Now = erlang:system_time(second),
-    Now > Exp.
+    Now > Exp;
+is_expired(Exp) ->
+    ?DEBUG("acl_deny_due_to_invalid_jwt_exp:~p", [Exp]),
+    true.
 
 description() -> "Authentication with JWT".
 
+string_to_number(Bin) when is_binary(Bin) ->
+    string_to_number(Bin, fun erlang:binary_to_integer/1, fun erlang:binary_to_float/1);
+string_to_number(Str) when is_list(Str) ->
+    string_to_number(Str, fun erlang:list_to_integer/1, fun erlang:list_to_float/1);
+string_to_number(_) ->
+    false.
+
 %%------------------------------------------------------------------------------
 %% Verify Claims
 %%--------------------------------------------------------------------
@@ -133,3 +146,14 @@ match_topic(ClientInfo, AclTopic, Topic) ->
     TopicWords = emqx_topic:words(Topic),
     AclTopicRendered = emqx_access_rule:feed_var(ClientInfo, AclTopicWords),
     emqx_topic:match(TopicWords, AclTopicRendered).
+
+string_to_number(Str, IntFun, FloatFun) ->
+    try
+        {ok, IntFun(Str)}
+    catch _:_ ->
+        try
+            {ok, FloatFun(Str)}
+        catch _:_ ->
+            false
+        end
+    end.

apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl

@@ -215,13 +215,13 @@ with_int_value(Fun) ->
             case Value of
                 Int when is_integer(Int) -> Fun(Int);
                 Bin when is_binary(Bin) ->
-                    case string:to_integer(Bin) of
+                    case emqx_auth_jwt:string_to_number(Bin) of
-                        {Int, <<>>} -> Fun(Int);
+                        {ok, Num} -> Fun(Num);
                         _ -> false
                     end;
                 Str when is_list(Str) ->
-                    case string:to_integer(Str) of
+                    case emqx_auth_jwt:string_to_number(Str) of
-                        {Int, ""} -> Fun(Int);
+                        {ok, Num} -> Fun(Num);
                         _ -> false
                     end
             end

apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl

@@ -164,7 +164,18 @@ t_check_auth_str_exp(_Config) ->
 
     Result1 = emqx_access_control:authenticate(Plain#{password => Jwt1}),
     ct:pal("Auth result: ~p~n", [Result1]),
-    ?assertMatch({error, _}, Result1).
+    ?assertMatch({error, _}, Result1),
+
+    Exp2 = float_to_binary(os:system_time(seconds) + 3.5),
+
+    Jwt2 = sign([{clientid, <<"client1">>},
+                 {username, <<"plain">>},
+                 {exp, Exp2}], <<"HS256">>, <<"emqxsecret">>),
+    ct:pal("Jwt: ~p~n", [Jwt2]),
+
+    Result2 = emqx_access_control:authenticate(Plain#{password => Jwt2}),
+    ct:pal("Auth result: ~p~n", [Result2]),
+    ?assertMatch({ok, #{auth_result := success, jwt_claims := _}}, Result2).
 
 t_check_claims(init, _Config) ->
     application:set_env(emqx_auth_jwt, verify_claims, [{sub, <<"value">>}]).

apps/emqx_auth_mnesia/src/emqx_auth_mnesia.app.src

@@ -1,6 +1,6 @@
 {application, emqx_auth_mnesia,
  [{description, "EMQ X Authentication with Mnesia"},
-  {vsn, "4.3.8"}, % strict semver, bump manually
+  {vsn, "4.3.9"}, % strict semver, bump manually
   {modules, []},
   {registered, []},
   {applications, [kernel,stdlib,mnesia]},

apps/emqx_rule_engine/src/emqx_rule_engine.erl

@@ -547,11 +547,21 @@ with_resource_params(Args = #{<<"$resource">> := ResId}) ->
     end;
 with_resource_params(Args) -> Args.
 
--dialyzer([{nowarn_function, may_update_rule_params/2}]).
+may_update_rule_params(Rule, Params) ->
-may_update_rule_params(Rule, Params = #{rawsql := SQL}) ->
+    %% NOTE: order matters, e.g. update_actions must be after update_enabled
+    FL = [fun update_raw_sql/2,
+          fun update_enabled/2,
+          fun update_description/2,
+          fun update_on_action_failed/2,
+          fun update_actions/2
+          ],
+    lists:foldl(fun(F, RuleIn) ->
+                        F(RuleIn, Params)
+                end, Rule, FL).
+
+update_raw_sql(Rule, #{rawsql := SQL}) ->
     case emqx_rule_sqlparser:parse_select(SQL) of
         {ok, Select} ->
-            may_update_rule_params(
                 Rule#rule{
                     rawsql = SQL,
                     for = emqx_rule_sqlparser:select_from(Select),
@@ -560,12 +570,14 @@ may_update_rule_params(Rule, Params = #{rawsql := SQL}) ->
                     doeach = emqx_rule_sqlparser:select_doeach(Select),
                     incase = emqx_rule_sqlparser:select_incase(Select),
                     conditions = emqx_rule_sqlparser:select_where(Select)
-                },
+                };
-                maps:remove(rawsql, Params));
+        Reason ->
-        Reason -> throw(Reason)
+            throw(Reason)
     end;
-may_update_rule_params(Rule = #rule{enabled = OldEnb, actions = Actions, state = OldState},
+update_raw_sql(Rule, _) ->
-         Params = #{enabled := NewEnb}) ->
+    Rule.
+
+update_enabled(Rule = #rule{enabled = OldEnb, actions = Actions, state = OldState}, #{enabled := NewEnb}) ->
     State = case {OldEnb, NewEnb} of
         {false, true} ->
             _ = ?CLUSTER_CALL(refresh_rule, [Rule]),
@@ -575,19 +587,27 @@ may_update_rule_params(Rule = #rule{enabled = OldEnb, actions = Actions, state =
             force_changed;
         _NoChange -> OldState
     end,
-    may_update_rule_params(Rule#rule{enabled = NewEnb, state = State}, maps:remove(enabled, Params));
+    Rule#rule{enabled = NewEnb, state = State};
-may_update_rule_params(Rule, Params = #{description := Descr}) ->
+update_enabled(Rule, _) ->
-    may_update_rule_params(Rule#rule{description = Descr}, maps:remove(description, Params));
+    Rule.
-may_update_rule_params(Rule, Params = #{on_action_failed := OnFailed}) ->
+
-    may_update_rule_params(Rule#rule{on_action_failed = OnFailed},
+update_description(Rule, #{description := Descr}) ->
-        maps:remove(on_action_failed, Params));
+    Rule#rule{description = Descr};
-may_update_rule_params(Rule = #rule{actions = OldActions}, Params = #{actions := Actions}) ->
+update_description(Rule, _) ->
+    Rule.
+
+update_on_action_failed(Rule, #{on_action_failed := OnFailed}) ->
+    Rule#rule{on_action_failed = OnFailed};
+update_on_action_failed(Rule, _) ->
+    Rule.
+
+update_actions(Rule = #rule{actions = OldActions, enabled = Enabled}, #{actions := Actions}) ->
     %% prepare new actions before removing old ones
-    NewActions = prepare_actions(Actions, maps:get(enabled, Params, true)),
+    NewActions = prepare_actions(Actions, Enabled),
     _ = ?CLUSTER_CALL(restore_action_metrics, [OldActions, NewActions]),
     _ = ?CLUSTER_CALL(clear_actions, [OldActions]),
-    may_update_rule_params(Rule#rule{actions = NewActions}, maps:remove(actions, Params));
+    Rule#rule{actions = NewActions};
-may_update_rule_params(Rule, _Params) -> %% ignore all the unsupported params
+update_actions(Rule, _) ->
     Rule.
 
 %% NOTE: if the user removed an action, but the action is not the last one in the list,