diff --git a/changes/v4.3.22-en.md b/changes/v4.3.22-en.md index 67c62f4bd..c902fbf61 100644 --- a/changes/v4.3.22-en.md +++ b/changes/v4.3.22-en.md @@ -23,10 +23,19 @@ - Added a log censor to avoid logging sensitive data [#9189](https://github.com/emqx/emqx/pull/9189). If the data to be logged is a map or key-value list which contains sensitive key words such as `password`, the value is obfuscated as `******`. -- Enhanced log security in ACL modules, sensitive data will be obscured. [#9242](https://github.com/emqx/emqx/pull/9242). +- Enhanced log security in ACL modules, sensitive data will be obscured [#9242](https://github.com/emqx/emqx/pull/9242). - Add `management.bootstrap_apps_file` configuration to bulk import default app/secret when EMQX initializes the database [#9273](https://github.com/emqx/emqx/pull/9273). +- Added two new configs for deterministic order of authentication and ACL checks [#9283](https://github.com/emqx/emqx/pull/9283). + The two new global config names are `auth_order` and `acl_order`. + When multiple ACL or auth plugins (or modules) are enabled, without this config, the order (in which each backend is queried) + is determined by the start/restart order of the plugin (or module). + Meaning, if a plugin (or module) is restarted after initial boot, it may get ordered to the end of the list. + With this config, you may set the order with a comma-speapated ACL or auth plugin names (or aliases). + For example: `acl_order = jwt,http`, this will make sure `jwt` is always checked before `http`, + meaning if JWT is not found (or no `acl` cliam) for a client, then the ACL check will fallback to use the HTTP backend. + ## Bug fixes - Fix that after uploading a backup file with an UTF8 filename, HTTP API `GET /data/export` fails with status code 500 [#9224](https://github.com/emqx/emqx/pull/9224). diff --git a/changes/v4.3.22-zh.md b/changes/v4.3.22-zh.md index e40fa0737..b969dd4c2 100644 --- a/changes/v4.3.22-zh.md +++ b/changes/v4.3.22-zh.md @@ -20,10 +20,18 @@ - 增强包含敏感数据的日志的安全性 [#9189](https://github.com/emqx/emqx/pull/9189)。 如果日志中包含敏感关键词，例如 `password`，那么关联的数据回被模糊化处理，替换成 `******`。 -- 增强 ACL 模块中的日志安全性，敏感数据将被模糊化。[#9242](https://github.com/emqx/emqx/pull/9242)。 +- 增强 ACL 模块中的日志安全性，敏感数据将被模糊化 [#9242](https://github.com/emqx/emqx/pull/9242)。 - 增加 `management.bootstrap_apps_file` 配置，可以让 EMQX 初始化数据库时，从该文件批量导入一些 APP / Secret [#9273](https://github.com/emqx/emqx/pull/9273)。 +- 增加了固化认证和 ACL 模块调用顺序的配置 [#9283](https://github.com/emqx/emqx/pull/9283)。 + 这两个新的全局配置名称为 `auth_order` 和 `acl_order`。 + 当有多个认证或 ACL 插件（或模块）开启时，没有该配置的话，模块调用的顺序取决于它们的启动顺序。 + 例如，如果一个插件（或模块）在系统启动之后单独重启了，那么它就有可能排到其他插件（或模块）的后面去。 + 有了这个配置之后，用户可以使用用逗号分隔的插件（或模块）的名字（或别名）来固化他们被调用的顺序。 + 例如，`acl_order = jwt,http`，可以用于保证 `jwt` 这个模块总是排在 `http` 的前面， + 也就是说，在对客户端进行 ACL 检查时，如果 JWT 不存在（或者没有定义 ACL），那么回退到使用 HTTP。 + ## 修复 - 修复若上传的备份文件名中包含 UTF8 字符，`GET /data/export` HTTP 接口返回 500 错误 [#9224](https://github.com/emqx/emqx/pull/9224)。 diff --git a/etc/emqx.conf b/etc/emqx.conf index 7ce550c87..c16a11c31 100644 --- a/etc/emqx.conf +++ b/etc/emqx.conf @@ -703,6 +703,45 @@ acl_deny_action = ignore ## Value: Integer,Duration,Duration flapping_detect_policy = 30, 1m, 5m +## When using multiple authentication backends, this config can be used to define the order. +## Default value "none" means no explicit ordering. +## Use comma to separate the names or aliases, for example "jwt,http" means "jwt" authentication should be checked before "http". +## Supported names are: +## 'http': emqx_auth_http +## 'jwt': emqx_auth_jwt +## 'ldap': emqx_auth_ldap +## 'mnesia': emqx_auth_mnesia +## 'mongo' (or 'mongodb'): emqx_auth_mongo +## 'mysql': emqx_auth_mysql +## 'pgsql' (or 'postgres'): emqx_auth_pgsql +## 'redis': emqx_auth_redis +## The specified backends are ordered prior to the unspecified ones, +## for example when "mnesia", "jwt" and "http" are in use, +## if only "jwt,http" is configured here, then "mnesia" is ordered after the other two. +## The name can also be the specific callback module name, e.g. my_auth_plugin_module, +## but it is silently discarded if there is no such module found in the system. +auth_order = none + +## When using multiple ACL backends, this config can be used to define the order. +## Default value "none" means no explicit ordering. +## Use comma to separate the names or aliases, for example "jwt,http" means "jwt" ACL should be checked before "http". +## Supported names are: +## 'http': emqx_acl_http +## 'internal' (or 'file'): emqx_mod_acl_internal +## 'jwt': emqx_auth_jwt +## 'ldap': emqx_acl_ldap +## 'mnesia': emqx_acl_mnesia +## 'mongo' (or 'mongodb'): emqx_acl_mongo +## 'mysql': emqx_acl_mysql +## 'pgsql' (or 'postgres'): emqx_acl_pgsql +## 'redis': emqx_acl_redis +## The specified backends are ordered prior to the unspecified ones, +## for example when "mnesia", "jwt" and "http" are in use, +## if only "jwt,http" is configured here, then "mnesia" is ordered after the other two. +## The name can also be the specific callback module name, e.g. my_auth_plugin_module, +## but it is silently discarded if there is no such module found in the system.' +acl_order = none + ##-------------------------------------------------------------------- ## MQTT Protocol ##-------------------------------------------------------------------- diff --git a/priv/emqx.schema b/priv/emqx.schema index 865dc7bec..dc567bdf4 100644 --- a/priv/emqx.schema +++ b/priv/emqx.schema @@ -839,6 +839,19 @@ end}. %% Authentication/ACL %%-------------------------------------------------------------------- +%% @doc Define a determined authentication plugin/module check order. +%% see detailed doc in emqx.conf +{mapping, "auth_order", "emqx.auth_order", [ + {default, "none"}, + {datatype, string} +]}. + +%% @doc Same as auth_order, but for ACL. +{mapping, "acl_order", "emqx.acl_order", [ + {default, "none"}, + {datatype, string} +]}. + %% @doc Allow anonymous authentication. {mapping, "allow_anonymous", "emqx.allow_anonymous", [ {default, false}, diff --git a/src/emqx.appup.src b/src/emqx.appup.src index fff6ba87e..6c3c1b080 100644 --- a/src/emqx.appup.src +++ b/src/emqx.appup.src @@ -2,14 +2,16 @@ %% Unless you know what you are doing, DO NOT edit manually!! {VSN, [{"4.4.10", - [{load_module,emqx_misc,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_cm,brutal_purge,soft_purge,[]}, {load_module,emqx_app,brutal_purge,soft_purge,[]}]}, {"4.4.9", - [{load_module,emqx_shared_sub,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_shared_sub,brutal_purge,soft_purge,[]}, {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}, {add_module,emqx_secret}, @@ -25,7 +27,8 @@ {load_module,emqx_router,brutal_purge,soft_purge,[]}, {load_module,emqx_app,brutal_purge,soft_purge,[]}]}, {"4.4.8", - [{load_module,emqx_misc,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}, {add_module,emqx_secret}, {load_module,emqx_session,brutal_purge,soft_purge,[]}, @@ -42,7 +45,8 @@ {load_module,emqx_cm,brutal_purge,soft_purge,[]}, {load_module,emqx_message,brutal_purge,soft_purge,[]}]}, {"4.4.7", - [{add_module,emqx_secret}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {add_module,emqx_secret}, {load_module,emqx_session,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_alarm,brutal_purge,soft_purge,[]}, @@ -59,7 +63,8 @@ {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_app,brutal_purge,soft_purge,[]}]}, {"4.4.6", - [{add_module,emqx_secret}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {add_module,emqx_secret}, {load_module,emqx_session,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_alarm,brutal_purge,soft_purge,[]}, @@ -76,7 +81,8 @@ {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}]}, {"4.4.5", - [{add_module,emqx_secret}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {add_module,emqx_secret}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_alarm,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, @@ -95,7 +101,8 @@ {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_session,brutal_purge,soft_purge,[]}]}, {"4.4.4", - [{add_module,emqx_secret}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {add_module,emqx_secret}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_alarm,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, @@ -122,7 +129,8 @@ {load_module,emqx_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_session,brutal_purge,soft_purge,[]}]}, {"4.4.3", - [{add_module,emqx_secret}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {add_module,emqx_secret}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_connection,brutal_purge,soft_purge,[]}, @@ -279,14 +287,16 @@ {load_module,emqx_limiter,brutal_purge,soft_purge,[]}]}, {<<".*">>,[]}], [{"4.4.10", - [{load_module,emqx_misc,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_cm,brutal_purge,soft_purge,[]}, {load_module,emqx_app,brutal_purge,soft_purge,[]}]}, {"4.4.9", - [{load_module,emqx_shared_sub,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_shared_sub,brutal_purge,soft_purge,[]}, {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}, {load_module,emqx_session,brutal_purge,soft_purge,[]}, @@ -301,7 +311,8 @@ {load_module,emqx_router,brutal_purge,soft_purge,[]}, {load_module,emqx_app,brutal_purge,soft_purge,[]}]}, {"4.4.8", - [{load_module,emqx_misc,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_misc,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}, {load_module,emqx_session,brutal_purge,soft_purge,[]}, {load_module,emqx_plugins,brutal_purge,soft_purge,[]}, @@ -317,7 +328,8 @@ {load_module,emqx_cm,brutal_purge,soft_purge,[]}, {load_module,emqx_message,brutal_purge,soft_purge,[]}]}, {"4.4.7", - [{load_module,emqx_session,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_session,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_alarm,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, @@ -333,7 +345,8 @@ {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_app,brutal_purge,soft_purge,[]}]}, {"4.4.6", - [{load_module,emqx_session,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_session,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_alarm,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, @@ -349,7 +362,8 @@ {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}]}, {"4.4.5", - [{load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_alarm,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_connection,brutal_purge,soft_purge,[]}, @@ -367,7 +381,8 @@ {load_module,emqx_shared_sub,brutal_purge,soft_purge,[]}, {load_module,emqx_relup,brutal_purge,soft_purge,[]}]}, {"4.4.4", - [{load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_alarm,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_connection,brutal_purge,soft_purge,[]}, @@ -393,7 +408,8 @@ {load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_metrics,brutal_purge,soft_purge,[]}]}, {"4.4.3", - [{load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_router,brutal_purge,soft_purge,[]}, diff --git a/src/emqx_hooks.erl b/src/emqx_hooks.erl index 28a2899ae..88422cc73 100644 --- a/src/emqx_hooks.erl +++ b/src/emqx_hooks.erl @@ -21,6 +21,10 @@ -include("logger.hrl"). -include("types.hrl"). +-ifdef(TEST). +-include_lib("eunit/include/eunit.hrl"). +-endif. + -logger_header("[Hooks]"). -export([ start_link/0 @@ -38,6 +42,8 @@ , run/2 , run_fold/3 , lookup/1 + , reorder_acl_callbacks/0 + , reorder_auth_callbacks/0 ]). -export([ callback_action/1 @@ -86,6 +92,7 @@ -define(TAB, ?MODULE). -define(SERVER, ?MODULE). +-define(UNKNOWN_ORDER, 999999999). -spec(start_link() -> startlink_ret()). start_link() -> @@ -229,6 +236,16 @@ lookup(HookPoint) -> [] -> [] end. +%% @doc Reorder ACL check callbacks +-spec reorder_acl_callbacks() -> ok. +reorder_acl_callbacks() -> + gen_server:cast(?SERVER, {reorder_callbacks, 'client.check_acl'}). + +%% @doc Reorder Authentication check callbacks +-spec reorder_auth_callbacks() -> ok. +reorder_auth_callbacks() -> + gen_server:cast(?SERVER, {reorder_callbacks, 'client.authenticate'}). + %%-------------------------------------------------------------------- %% gen_server callbacks %%-------------------------------------------------------------------- @@ -238,11 +255,12 @@ init([]) -> {ok, #{}}. handle_call({add, HookPoint, Callback = #callback{action = Action}}, _From, State) -> - Reply = case lists:keymember(Action, #callback.action, Callbacks = lookup(HookPoint)) of + Callbacks = lookup(HookPoint), + Reply = case lists:keymember(Action, #callback.action, Callbacks) of true -> {error, already_exists}; false -> - insert_hook(HookPoint, add_callback(Callback, Callbacks)) + ok = add_and_insert(HookPoint, [Callback], Callbacks) end, {reply, Reply, State}; @@ -250,12 +268,22 @@ handle_call(Req, _From, State) -> ?LOG(error, "Unexpected call: ~p", [Req]), {reply, ignored, State}. +handle_cast({reorder_callbacks, HookPoint}, State) -> + Callbacks = lookup(HookPoint), + case Callbacks =:= [] of + true -> + %% no callbaks, make sure not to insert [] + ok; + false -> + ok = add_and_insert(HookPoint, Callbacks, []) + end, + {noreply, State}; handle_cast({del, HookPoint, Action}, State) -> case del_callback(Action, lookup(HookPoint)) of [] -> ets:delete(?TAB, HookPoint); Callbacks -> - insert_hook(HookPoint, Callbacks) + ok = insert_hook(HookPoint, Callbacks) end, {noreply, State}; @@ -277,19 +305,92 @@ code_change(_OldVsn, State, _Extra) -> %% Internal functions %%------------------------------------------------------------------------------ +add_and_insert(HookPoint, NewCallbacks, Callbacks) -> + HookOrder = get_hook_order(HookPoint), + NewCallbaks = add_callbacks(HookOrder, NewCallbacks, Callbacks), + ok = insert_hook(HookPoint, NewCallbaks). + +get_hook_order('client.authenticate') -> + get_auth_acl_hook_order(auth_order); +get_hook_order('client.check_acl') -> + get_auth_acl_hook_order(acl_order); +get_hook_order(_) -> + []. + +get_auth_acl_hook_order(AppEnvName) -> + case emqx:get_env(AppEnvName) of + [_|_] = CSV -> + %% non-empty string + parse_auth_acl_hook_order(AppEnvName, CSV); + _ -> + [] + end. + +parse_auth_acl_hook_order(auth_order, CSV) -> + parse_auth_acl_hook_order(fun parse_auth_name/1, CSV); +parse_auth_acl_hook_order(acl_order, CSV) -> + parse_auth_acl_hook_order(fun parse_acl_name/1, CSV); +parse_auth_acl_hook_order(NameParser, CSV) when is_function(NameParser) -> + do_parse_auth_acl_hook_order(NameParser, string:tokens(CSV, ", ")). + +do_parse_auth_acl_hook_order(_, []) -> []; +do_parse_auth_acl_hook_order(Parser, ["none" | Names]) -> + %% "none" is the default config value + do_parse_auth_acl_hook_order(Parser, Names); +do_parse_auth_acl_hook_order(Parser, [Name0 | Names]) -> + Name = Parser(Name0), + [Name | do_parse_auth_acl_hook_order(Parser, Names)]. + +%% NOTE: It's ugly to enumerate plugin names here. +%% But it's the most straightforward way. +parse_auth_name("http") -> "emqx_auth_http"; +parse_auth_name("jwt") -> "emqx_auth_jwt"; +parse_auth_name("ldap") -> "emqx_auth_ldap"; +parse_auth_name("mnesia") -> "emqx_auth_mnesia"; +parse_auth_name("mongodb") -> "emqx_auth_mongo"; +parse_auth_name("mongo") -> "emqx_auth_mongo"; +parse_auth_name("mysql") -> "emqx_auth_mysql"; +parse_auth_name("pgsql") -> "emqx_auth_pgsql"; +parse_auth_name("postgres") -> "emqx_auth_pgsql"; +parse_auth_name("redis") -> "emqx_auth_redis"; +parse_auth_name(Other) -> Other. %% maybe a user defined plugin or the module name directly + +parse_acl_name("file") -> "emqx_mod_acl_internal"; +parse_acl_name("internal") -> "emqx_mod_acl_internal"; +parse_acl_name("http") -> "emqx_acl_http"; +parse_acl_name("jwt") -> "emqx_auth_jwt"; %% this is not a typo, there is no emqx_acl_jwt module +parse_acl_name("ldap") -> "emqx_acl_ldap"; +parse_acl_name("mnesia") -> "emqx_acl_mnesia"; +parse_acl_name("mongo") -> "emqx_acl_mongo"; +parse_acl_name("mongodb") -> "emqx_acl_mongo"; +parse_acl_name("mysql") -> "emqx_acl_mysql"; +parse_acl_name("pgsql") -> "emqx_acl_pgsql"; +parse_acl_name("postgres") -> "emqx_acl_pgsql"; +parse_acl_name("redis") -> "emqx_acl_redis"; +parse_acl_name(Other) -> Other. %% maybe a user defined plugin or the module name directly + insert_hook(HookPoint, Callbacks) -> - ets:insert(?TAB, #hook{name = HookPoint, callbacks = Callbacks}), ok. + ets:insert(?TAB, #hook{name = HookPoint, callbacks = Callbacks}), + ok. -add_callback(C, Callbacks) -> - add_callback(C, Callbacks, []). +add_callbacks(_Order, [], Callbacks) -> + Callbacks; +add_callbacks(Order, [C | More], Callbacks) -> + NewCallbacks = add_callback(Order, C, Callbacks), + add_callbacks(Order, More, NewCallbacks). -add_callback(C, [], Acc) -> +add_callback(Order, C, Callbacks) -> + add_callback(Order, C, Callbacks, []). + +add_callback(_Order, C, [], Acc) -> lists:reverse([C|Acc]); -add_callback(C1 = #callback{priority = P1}, [C2 = #callback{priority = P2}|More], Acc) - when P1 =< P2 -> - add_callback(C1, More, [C2|Acc]); -add_callback(C1, More, Acc) -> - lists:append(lists:reverse(Acc), [C1 | More]). +add_callback(Order, C1, [C2|More], Acc) -> + case is_lower_priority(Order, C1, C2) of + true -> + add_callback(Order, C1, More, [C2|Acc]); + false -> + lists:append(lists:reverse(Acc), [C1, C2 | More]) + end. del_callback(Action, Callbacks) -> del_callback(Action, Callbacks, []). @@ -304,3 +405,144 @@ del_callback(Func, [#callback{action = {Func, _A}} | Callbacks], Acc) -> del_callback(Func, Callbacks, Acc); del_callback(Action, [Callback | Callbacks], Acc) -> del_callback(Action, Callbacks, [Callback | Acc]). + + +%% does A have lower priority than B? +is_lower_priority(Order, + #callback{priority = PrA, action = ActA}, + #callback{priority = PrB, action = ActB}) -> + PosA = callback_position(Order, ActA), + PosB = callback_position(Order, ActB), + case PosA =:= PosB of + true -> + %% When priority is equal, the new callback (A) goes after the existing (B) hence '=<' + PrA =< PrB; + false -> + %% When OrdA > OrdB the new callback (A) positioned after the exiting (B) + PosA > PosB + end. + +callback_position(Order, Callback) -> + M = callback_module(Callback), + find_list_item_position(Order, atom_to_list(M)). + +callback_module({M, _F, _A}) -> M; +callback_module({F, _A}) when is_function(F) -> + {module, M} = erlang:fun_info(F, module), + M; +callback_module(F) when is_function(F) -> + {module, M} = erlang:fun_info(F, module), + M. + +find_list_item_position(Order, Name) -> + find_list_item_position(Order, Name, 1). + +find_list_item_position([], _ModuleName, _N) -> + %% Not found, make sure it's ordered behind the found ones + ?UNKNOWN_ORDER; +find_list_item_position([Prefix | Rest], ModuleName, N) -> + case is_prefix(Prefix, ModuleName) of + true -> + N; + false -> + find_list_item_position(Rest, ModuleName, N + 1) + end. + +is_prefix(Prefix, ModuleName) -> + case string:prefix(ModuleName, Prefix) of + nomatch -> + false; + _Sufix -> + true + end. + +-ifdef(TEST). +add_priority_rules_test_() -> + [{ "high prio", + fun() -> + OrderString = "foo, bar", + Existing = [make_hook(0, emqx_acl_pgsql), make_hook(0, emqx_acl_mysql)], + New = make_hook(1, emqx_acl_mnesia), + Expected = [New | Existing], + ?assertEqual(Expected, test_add_acl(OrderString, New, Existing)) + end}, + { "low prio", + fun() -> + OrderString = "foo, bar", + Existing = [make_hook(0, emqx_auth_jwt), make_hook(0, emqx_acl_mongo)], + New = make_hook(-1, emqx_acl_mnesia), + Expected = Existing++ [New], + ?assertEqual(Expected, test_add_acl(OrderString, New, Existing)) + end}, + { "mid prio", + fun() -> + OrderString = "", + Existing = [make_hook(3, emqx_acl_http), make_hook(1, emqx_acl_redis)], + New = make_hook(2, emqx_acl_ldap), + Expected = [hd(Existing), New | tl(Existing)], + ?assertEqual(Expected, test_add_acl(OrderString, New, Existing)) + end} + ]. + +add_order_rules_test_() -> + [{"initial add", + fun() -> + OrderString = "ldap,pgsql,file", + Existing = [], + New = make_hook(2, foo), + ?assertEqual([New], test_add_auth(OrderString, New, Existing)) + end}, + { "before", + fun() -> + OrderString = "mongodb,postgres,internal", + Existing = [make_hook(1, emqx_auth_pgsql), make_hook(3, emqx_auth_mysql)], + New = make_hook(2, emqx_auth_mongo), + Expected = [New | Existing], + ?assertEqual(Expected, test_add_auth(OrderString, New, Existing)) + end}, + { "after", + fun() -> + OrderString = "mysql,postgres,ldap", + Existing = [make_hook(1, emqx_auth_pgsql), make_hook(3, emqx_auth_mysql)], + New = make_hook(2, emqx_auth_ldap), + Expected = Existing ++ [New], + ?assertEqual(Expected, test_add_auth(OrderString, New, Existing)) + end}, + { "unknown goes after knowns", + fun() -> + OrderString = "mongo,mysql,,mnesia", %% ,, is intended to test empty string + Existing = [make_hook(1, emqx_auth_mnesia), make_hook(3, emqx_auth_mysql)], + New1 = make_hook(2, fun() -> foo end), %% fake hook + New2 = make_hook(3, {fun lists:append/1, []}), %% fake hook + Expected1 = Existing ++ [New1], + Expected2 = Existing ++ [New2, New1], %% 2 is before 1 due to higher prio + ?assertEqual(Expected1, test_add_auth(OrderString, New1, Existing)), + ?assertEqual(Expected2, test_add_auth(OrderString, New2, Expected1)) + end}, + { "known goes first", + fun() -> + OrderString = "redis,jwt", + Existing = [make_hook(1, emqx_auth_mnesia), make_hook(3, emqx_auth_mysql)], + Redis = make_hook(2, emqx_auth_redis), + Jwt = make_hook(2, emqx_auth_jwt), + Expected1 = [Redis | Existing], + ?assertEqual(Expected1, test_add_auth(OrderString, Redis, Existing)), + Expected2 = [Redis, Jwt | Existing], + ?assertEqual(Expected2, test_add_auth(OrderString, Jwt, Expected1)) + end} + ]. + +make_hook(Priority, CallbackModule) when is_atom(CallbackModule) -> + #callback{priority = Priority, action = {CallbackModule, dummy, []}}; +make_hook(Priority, F) -> + #callback{priority = Priority, action = F}. + +test_add_acl(OrderString, NewHook, ExistingHooks) -> + Order = parse_auth_acl_hook_order(acl_order, OrderString), + add_callback(Order, NewHook, ExistingHooks). + +test_add_auth(OrderString, NewHook, ExistingHooks) -> + Order = parse_auth_acl_hook_order(auth_order, OrderString), + add_callback(Order, NewHook, ExistingHooks). + +-endif. diff --git a/test/emqx_hooks_SUITE.erl b/test/emqx_hooks_SUITE.erl index 645056fc5..bee0dbefe 100644 --- a/test/emqx_hooks_SUITE.erl +++ b/test/emqx_hooks_SUITE.erl @@ -23,21 +23,13 @@ all() -> emqx_ct:all(?MODULE). -% t_lookup(_) -> -% error('TODO'). +init_per_testcase(_Test, Config) -> + Config. -% t_run_fold(_) -> -% error('TODO'). +end_per_testcase(_Test, _Config) -> + _ = (catch emqx_hooks:stop()), + _ = clear_orders(). -% t_run(_) -> -% error('TODO'). - -% t_del(_) -> -% error('TODO'). - -% t_add(_) -> -% error('TODO'). - t_add_del_hook(_) -> {ok, _} = emqx_hooks:start_link(), ok = emqx:hook(test_hook, fun ?MODULE:hook_fun1/1, []), @@ -107,6 +99,80 @@ t_uncovered_func(_) -> Pid ! test, ok = emqx_hooks:stop(). +t_explicit_order_acl(_) -> + HookPoint = 'client.check_acl', + test_explicit_order(acl_order, HookPoint). + +t_explicit_order_auth(_) -> + HookPoint = 'client.authenticate', + test_explicit_order(auth_order, HookPoint). + +test_explicit_order(ConfigKey, HookPoint) -> + {ok, _} = emqx_hooks:start_link(), + + ok = set_orders(ConfigKey, "mod_a, mod_b"), + + ok = emqx:hook(HookPoint, {mod_c, cb, []}, 5), + ok = emqx:hook(HookPoint, {mod_d, cb, []}, 0), + ok = emqx:hook(HookPoint, {mod_b, cb, []}, 0), + ok = emqx:hook(HookPoint, {mod_a, cb, []}, -1), + ok = emqx:hook(HookPoint, {mod_e, cb, []}, -1), + + ?assertEqual( + [ + {mod_a, cb, []}, + {mod_b, cb, []}, + {mod_c, cb, []}, + {mod_d, cb, []}, + {mod_e, cb, []} + ], + get_hookpoint_callbacks(HookPoint)). + +t_reorder_callbacks_acl(_) -> + F = fun emqx_hooks:reorder_acl_callbacks/0, + ok = emqx_hooks:reorder_auth_callbacks(), + test_reorder_callbacks(acl_order, 'client.check_acl', F). + +t_reorder_callbacks_auth(_) -> + F = fun emqx_hooks:reorder_auth_callbacks/0, + test_reorder_callbacks(auth_order, 'client.authenticate', F). + +test_reorder_callbacks(ConfigKey, HookPoint, ReorderFun) -> + {ok, _} = emqx_hooks:start_link(), + ok = set_orders(ConfigKey, "mod_a,mod_b,mod_c"), + ok = emqx:hook(HookPoint, fun mod_c:foo/1), + ok = emqx:hook(HookPoint, fun mod_a:foo/1), + ok = emqx:hook(HookPoint, fun mod_b:foo/1), + ok = emqx:hook(HookPoint, fun mod_y:foo/1), + ok = emqx:hook(HookPoint, fun mod_x:foo/1), + ?assertEqual( + [fun mod_a:foo/1, fun mod_b:foo/1, fun mod_c:foo/1, + fun mod_y:foo/1, fun mod_x:foo/1 + ], + get_hookpoint_callbacks(HookPoint)), + ok = set_orders(ConfigKey, "mod_x,mod_a,mod_c,mod_b"), + ReorderFun(), + ignored = gen_server:call(emqx_hooks, x), + ?assertEqual( + [fun mod_x:foo/1, fun mod_a:foo/1, fun mod_c:foo/1, + fun mod_b:foo/1, fun mod_y:foo/1 + ], + get_hookpoint_callbacks(HookPoint)), + ok. + +%%-------------------------------------------------------------------- +%% Helpers +%%-------------------------------------------------------------------- + +set_orders(Key, OrderString) -> + application:set_env(emqx, Key, OrderString). + +clear_orders() -> + application:set_env(emqx, acl_order, "none"). + +get_hookpoint_callbacks(HookPoint) -> + [emqx_hooks:callback_action(C) || C <- emqx_hooks:lookup(HookPoint)]. + %%-------------------------------------------------------------------- %% Hook fun %%-------------------------------------------------------------------- @@ -140,4 +206,3 @@ hook_filter2(_, _Acc, _IntArg) -> false. hook_filter2_1(arg, _Acc, init_arg) -> true; hook_filter2_1(arg1, _Acc, init_arg) -> true; hook_filter2_1(_, _Acc, _IntArg) -> false. -