diff --git a/.github/workflows/run_test_cases.yaml b/.github/workflows/run_test_cases.yaml index 4e9a5fc37..aeb83b3a4 100644 --- a/.github/workflows/run_test_cases.yaml +++ b/.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 diff --git a/CHANGES-4.3.md b/CHANGES-4.3.md index bb44325f0..c13ef4d51 100644 --- a/CHANGES-4.3.md +++ b/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 diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src b/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src index 2d6524000..b2b56e273 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.app.src +++ b/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]}, diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src b/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src index b89b715f2..206c2af1a 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.appup.src +++ b/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.2",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}, - {load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]}, + [{"4.4.4", + [{load_module,emqx_auth_jwt_svr,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.2",[{load_module,emqx_auth_jwt_svr,brutal_purge,soft_purge,[]}, - {load_module,emqx_auth_jwt,brutal_purge,soft_purge,[]}]}, + [{"4.4.4", + [{load_module,emqx_auth_jwt_svr,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}]}, {<<".*">>,[]}]}. diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl b/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl index 725207d9a..30a6ab43a 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt.erl +++ b/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), - is_expired(ExpInt); -is_expired(Exp) -> + case string_to_number(Exp) of + {ok, Val} -> + 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. diff --git a/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl b/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl index ac07a8640..0f09be22e 100644 --- a/apps/emqx_auth_jwt/src/emqx_auth_jwt_svr.erl +++ b/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 - {Int, <<>>} -> Fun(Int); + case emqx_auth_jwt:string_to_number(Bin) of + {ok, Num} -> Fun(Num); _ -> false end; Str when is_list(Str) -> - case string:to_integer(Str) of - {Int, ""} -> Fun(Int); + case emqx_auth_jwt:string_to_number(Str) of + {ok, Num} -> Fun(Num); _ -> false end end diff --git a/apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl b/apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl index c8eed8b41..934d80f41 100644 --- a/apps/emqx_auth_jwt/test/emqx_auth_jwt_SUITE.erl +++ b/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">>}]). diff --git a/apps/emqx_auth_mnesia/src/emqx_auth_mnesia.app.src b/apps/emqx_auth_mnesia/src/emqx_auth_mnesia.app.src index 54d1317d7..6dd1dcdfc 100644 --- a/apps/emqx_auth_mnesia/src/emqx_auth_mnesia.app.src +++ b/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]}, diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine.erl b/apps/emqx_rule_engine/src/emqx_rule_engine.erl index d5c3ba92c..f48b8a02b 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine.erl +++ b/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 = #{rawsql := SQL}) -> +may_update_rule_params(Rule, Params) -> + %% 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 -> throw(Reason) + }; + Reason -> + throw(Reason) end; -may_update_rule_params(Rule = #rule{enabled = OldEnb, actions = Actions, state = OldState}, - Params = #{enabled := NewEnb}) -> +update_raw_sql(Rule, _) -> + 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)); -may_update_rule_params(Rule, Params = #{description := Descr}) -> - may_update_rule_params(Rule#rule{description = Descr}, maps:remove(description, Params)); -may_update_rule_params(Rule, Params = #{on_action_failed := OnFailed}) -> - may_update_rule_params(Rule#rule{on_action_failed = OnFailed}, - maps:remove(on_action_failed, Params)); -may_update_rule_params(Rule = #rule{actions = OldActions}, Params = #{actions := Actions}) -> + Rule#rule{enabled = NewEnb, state = State}; +update_enabled(Rule, _) -> + Rule. + +update_description(Rule, #{description := Descr}) -> + Rule#rule{description = Descr}; +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)); -may_update_rule_params(Rule, _Params) -> %% ignore all the unsupported params + Rule#rule{actions = NewActions}; +update_actions(Rule, _) -> Rule. %% NOTE: if the user removed an action, but the action is not the last one in the list,