From 52652f6c9682bb8387f8aa152ca7ae6bd134fe75 Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Thu, 20 Oct 2022 23:57:44 +0300 Subject: [PATCH 1/2] feat(hooks): add config options to set explicit callback order --- priv/emqx.schema | 46 ++++++++++++++++++++++++++++ src/emqx.appup.src | 48 +++++++++++++++++++---------- src/emqx_hooks.erl | 64 +++++++++++++++++++++++++++++++++------ test/emqx_hooks_SUITE.erl | 45 +++++++++++++++++++++++++-- 4 files changed, 175 insertions(+), 28 deletions(-) diff --git a/priv/emqx.schema b/priv/emqx.schema index 1b5232b51..1696b51b3 100644 --- a/priv/emqx.schema +++ b/priv/emqx.schema @@ -2494,6 +2494,52 @@ end}. {datatype, {enum, [true, false]}} ]}. + +{mapping, "broker.hook_order.$name.$hp_or_order", "emqx.hook_order", [ + {datatype, string} +]}. + +%% @doc Set explicit callback order for particular callbacks of particular hook points +%% +%% For example, to make JWT acl checks always preceed HTTP ACL checks, one may add: +%% +%% broker.hook_order.acl.hook_point = client.check_acl +%% broker.hook_order.acl.1 = emqx_auth_jwt +%% broker.hook_order.acl.2 = emqx_auth_http +%% +%% Callbacks with explicit ordering always have higher priority than the others. +{translation, "emqx.hook_order", fun(Conf) -> + Conf0 = cuttlefish_variable:filter_by_prefix("broker.hook_order", Conf), + HookOrders = lists:foldl( + fun({["broker", "hook_order", Name, "hook_point"], HookPoint}, AccHookOrders) -> + HookOrder = maps:get(Name, AccHookOrders, #{}), + NHookOrder = HookOrder#{hook_point => list_to_atom(HookPoint)}, + AccHookOrders#{Name => NHookOrder}; + ({["broker", "hook_order", Name, NStr], HookModule}, AccHookOrders) -> + case string:to_integer(NStr) of + {N, ""} -> + HookOrder = maps:get(Name, AccHookOrders, #{}), + HookOrderMods = maps:get(mods, HookOrder, []), + NHookOrder = HookOrder#{mods => [{list_to_atom(HookModule), N} | HookOrderMods]}, + AccHookOrders#{Name => NHookOrder}; + _ -> + AccHookOrders + end; + (_, AccHookOrders) -> + AccHookOrders + end, + #{}, + Conf0), + SimplifiedHookOrders = lists:map( + fun({Name, HookOrder}) -> + HookPoint = maps:get(hook_point, HookOrder, list_to_atom(Name)), + Mods = maps:get(mods, HookOrder, []), + {HookPoint, maps:from_list(Mods)} + end, + maps:to_list(HookOrders)), + maps:from_list(SimplifiedHookOrders) +end}. + %%-------------------------------------------------------------------- %% System Monitor %%-------------------------------------------------------------------- diff --git a/src/emqx.appup.src b/src/emqx.appup.src index 4be308808..6c016f347 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.3.22", - [{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_cm,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_app,brutal_purge,soft_purge,[]}]}, {"4.3.21", - [{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_session,brutal_purge,soft_purge,[]}, {add_module,emqx_secret}, {load_module,emqx_alarm,brutal_purge,soft_purge,[]}, @@ -26,7 +28,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}]}, {"4.3.20", - [{load_module,emqx_session,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_session,brutal_purge,soft_purge,[]}, {add_module,emqx_secret}, {load_module,emqx_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, @@ -44,7 +47,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}]}, {"4.3.19", - [{load_module,emqx_session,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_session,brutal_purge,soft_purge,[]}, {add_module,emqx_secret}, {load_module,emqx_listeners,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, @@ -63,7 +67,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {load_module,emqx_channel,brutal_purge,soft_purge,[]}]}, {"4.3.18", - [{load_module,emqx_session,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_session,brutal_purge,soft_purge,[]}, {add_module,emqx_secret}, {load_module,emqx_listeners,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, @@ -82,7 +87,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {load_module,emqx_plugins,brutal_purge,soft_purge,[]}]}, {"4.3.17", - [{add_module,emqx_secret}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {add_module,emqx_secret}, {load_module,emqx_listeners,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_router,brutal_purge,soft_purge,[]}, @@ -104,7 +110,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {load_module,emqx_access_control,brutal_purge,soft_purge,[]}]}, {"4.3.16", - [{add_module,emqx_secret}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {add_module,emqx_secret}, {load_module,emqx_listeners,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_router,brutal_purge,soft_purge,[]}, @@ -133,7 +140,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {load_module,emqx_topic,brutal_purge,soft_purge,[]}]}, {"4.3.15", - [{add_module,emqx_secret}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {add_module,emqx_secret}, {load_module,emqx_listeners,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_router,brutal_purge,soft_purge,[]}, @@ -870,14 +878,16 @@ {load_module,emqx_limiter,brutal_purge,soft_purge,[]}]}, {<<".*">>,[]}], [{"4.3.22", - [{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_cm,brutal_purge,soft_purge,[]}, {load_module,emqx_ws_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_connection,brutal_purge,soft_purge,[]}, {load_module,emqx_channel,brutal_purge,soft_purge,[]}, {load_module,emqx_app,brutal_purge,soft_purge,[]}]}, {"4.3.21", - [{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_session,brutal_purge,soft_purge,[]}, {load_module,emqx_alarm,brutal_purge,soft_purge,[]}, {load_module,emqx_app,brutal_purge,soft_purge,[]}, @@ -893,7 +903,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}]}, {"4.3.20", - [{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_plugins,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_router,brutal_purge,soft_purge,[]}, @@ -910,7 +921,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {load_module,emqx,brutal_purge,soft_purge,[]}]}, {"4.3.19", - [{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_listeners,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_router,brutal_purge,soft_purge,[]}, @@ -928,7 +940,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {load_module,emqx_channel,brutal_purge,soft_purge,[]}]}, {"4.3.18", - [{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_listeners,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_router,brutal_purge,soft_purge,[]}, @@ -946,7 +959,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {load_module,emqx_plugins,brutal_purge,soft_purge,[]}]}, {"4.3.17", - [{load_module,emqx_listeners,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_listeners,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_router,brutal_purge,soft_purge,[]}, {load_module,emqx_tracer,brutal_purge,soft_purge,[]}, @@ -967,7 +981,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {load_module,emqx_access_control,brutal_purge,soft_purge,[]}]}, {"4.3.16", - [{load_module,emqx_listeners,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_listeners,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_router,brutal_purge,soft_purge,[]}, {load_module,emqx_tracer,brutal_purge,soft_purge,[]}, @@ -995,7 +1010,8 @@ {load_module,emqx_http_lib,brutal_purge,soft_purge,[]}, {delete_module,emqx_exclusive_subscription}]}, {"4.3.15", - [{load_module,emqx_listeners,brutal_purge,soft_purge,[]}, + [{load_module,emqx_hooks,brutal_purge,soft_purge,[]}, + {load_module,emqx_listeners,brutal_purge,soft_purge,[]}, {load_module,emqx_router_helper,brutal_purge,soft_purge,[]}, {load_module,emqx_router,brutal_purge,soft_purge,[]}, {load_module,emqx_tracer,brutal_purge,soft_purge,[]}, diff --git a/src/emqx_hooks.erl b/src/emqx_hooks.erl index 28a2899ae..e65985e09 100644 --- a/src/emqx_hooks.erl +++ b/src/emqx_hooks.erl @@ -235,14 +235,15 @@ lookup(HookPoint) -> init([]) -> ok = emqx_tables:new(?TAB, [{keypos, #hook.name}, {read_concurrency, true}]), - {ok, #{}}. + HookOrders = emqx:get_env(hook_order, #{}), + {ok, #{hook_orders => HookOrders}}. handle_call({add, HookPoint, Callback = #callback{action = Action}}, _From, State) -> Reply = case lists:keymember(Action, #callback.action, Callbacks = lookup(HookPoint)) of true -> {error, already_exists}; false -> - insert_hook(HookPoint, add_callback(Callback, Callbacks)) + insert_hook(HookPoint, add_callback(callback_orders(HookPoint), Callback, Callbacks)) end, {reply, Reply, State}; @@ -280,16 +281,18 @@ code_change(_OldVsn, State, _Extra) -> insert_hook(HookPoint, Callbacks) -> ets:insert(?TAB, #hook{name = HookPoint, callbacks = Callbacks}), ok. -add_callback(C, Callbacks) -> - add_callback(C, Callbacks, []). +add_callback(Orders, C, Callbacks) -> + add_callback(Orders, C, Callbacks, []). -add_callback(C, [], Acc) -> +add_callback(_Orders, 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(Orders, C1, [C2|More], Acc) -> + case is_lower_priority(Orders, C1, C2) of + true -> + add_callback(Orders, C1, More, [C2|Acc]); + false -> + lists:append(lists:reverse(Acc), [C1, C2 | More]) + end. del_callback(Action, Callbacks) -> del_callback(Action, Callbacks, []). @@ -304,3 +307,44 @@ 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(Orders, + #callback{priority = PrA, action = ActA}, + #callback{priority = PrB, action = ActB}) -> + OrdA = callback_order(Orders, ActA), + OrdB = callback_order(Orders, ActB), + case {OrdA, OrdB} of + %% if both action positions are not specified, use priority + {undefined, undefined} -> + PrA =< PrB; + %% actions with specified positions have higher priority + {_OrdA, undefined} -> + false; + %% actions with specified positions have higher priority + {undefined, _OrdB} -> + true; + %% if both action positions are specified, the last one has the lower priority + _ -> + OrdA >= OrdB + end. + +callback_orders(HookPoint) -> + case hook_orders() of + #{HookPoint := CallbackOrders} -> CallbackOrders; + _ -> #{} + end. + +hook_orders() -> + case emqx:get_env(hook_order, #{}) of + Map when is_map(Map) -> Map; + _ -> #{} + end. + +callback_order(Orders, {M, _F, _A}) -> + case Orders of + #{M := N} -> N; + _ -> undefined + end; +callback_order(_Orders, _Action) -> undefined. diff --git a/test/emqx_hooks_SUITE.erl b/test/emqx_hooks_SUITE.erl index 645056fc5..fba722c96 100644 --- a/test/emqx_hooks_SUITE.erl +++ b/test/emqx_hooks_SUITE.erl @@ -23,6 +23,12 @@ all() -> emqx_ct:all(?MODULE). +init_per_testcase(_Test, Config) -> + Config. + +end_per_testcase(_Test, _Config) -> + _ = clear_orders(). + % t_lookup(_) -> % error('TODO'). @@ -37,7 +43,7 @@ all() -> emqx_ct:all(?MODULE). % 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 +113,42 @@ t_uncovered_func(_) -> Pid ! test, ok = emqx_hooks:stop(). +t_explicit_order(_) -> + {ok, _} = emqx_hooks:start_link(), + + ok = set_orders(hookpoint, [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)). + +%%-------------------------------------------------------------------- +%% Helpers +%%-------------------------------------------------------------------- + +set_orders(HookPoint, Mods) -> + Orders = maps:from_list(lists:zip(Mods, lists:seq(0, length(Mods) - 1))), + application:set_env(emqx, hook_order, #{HookPoint => Orders}). + +clear_orders() -> + application:set_env(emqx, hook_order, #{}). + +get_hookpoint_callbacks(HookPoint) -> + [emqx_hooks:callback_action(C) || C <- emqx_hooks:lookup(HookPoint)]. + + %%-------------------------------------------------------------------- %% Hook fun %%-------------------------------------------------------------------- @@ -140,4 +182,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. - From e0db524a10055f908cde8f0d456e9f31e75548d9 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Tue, 1 Nov 2022 10:17:58 +0100 Subject: [PATCH 2/2] refactor: change the format of auth_order and acl_order configs --- changes/v4.3.22-en.md | 11 +- changes/v4.3.22-zh.md | 10 +- etc/emqx.conf | 39 ++++++ priv/emqx.schema | 59 ++------ src/emqx_hooks.erl | 282 ++++++++++++++++++++++++++++++++------ test/emqx_hooks_SUITE.erl | 80 +++++++---- 6 files changed, 363 insertions(+), 118 deletions(-) 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 7b1bf1ef0..a5dd13cc3 100644 --- a/etc/emqx.conf +++ b/etc/emqx.conf @@ -698,6 +698,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 1696b51b3..6c12125cb 100644 --- a/priv/emqx.schema +++ b/priv/emqx.schema @@ -804,6 +804,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}, @@ -2494,52 +2507,6 @@ end}. {datatype, {enum, [true, false]}} ]}. - -{mapping, "broker.hook_order.$name.$hp_or_order", "emqx.hook_order", [ - {datatype, string} -]}. - -%% @doc Set explicit callback order for particular callbacks of particular hook points -%% -%% For example, to make JWT acl checks always preceed HTTP ACL checks, one may add: -%% -%% broker.hook_order.acl.hook_point = client.check_acl -%% broker.hook_order.acl.1 = emqx_auth_jwt -%% broker.hook_order.acl.2 = emqx_auth_http -%% -%% Callbacks with explicit ordering always have higher priority than the others. -{translation, "emqx.hook_order", fun(Conf) -> - Conf0 = cuttlefish_variable:filter_by_prefix("broker.hook_order", Conf), - HookOrders = lists:foldl( - fun({["broker", "hook_order", Name, "hook_point"], HookPoint}, AccHookOrders) -> - HookOrder = maps:get(Name, AccHookOrders, #{}), - NHookOrder = HookOrder#{hook_point => list_to_atom(HookPoint)}, - AccHookOrders#{Name => NHookOrder}; - ({["broker", "hook_order", Name, NStr], HookModule}, AccHookOrders) -> - case string:to_integer(NStr) of - {N, ""} -> - HookOrder = maps:get(Name, AccHookOrders, #{}), - HookOrderMods = maps:get(mods, HookOrder, []), - NHookOrder = HookOrder#{mods => [{list_to_atom(HookModule), N} | HookOrderMods]}, - AccHookOrders#{Name => NHookOrder}; - _ -> - AccHookOrders - end; - (_, AccHookOrders) -> - AccHookOrders - end, - #{}, - Conf0), - SimplifiedHookOrders = lists:map( - fun({Name, HookOrder}) -> - HookPoint = maps:get(hook_point, HookOrder, list_to_atom(Name)), - Mods = maps:get(mods, HookOrder, []), - {HookPoint, maps:from_list(Mods)} - end, - maps:to_list(HookOrders)), - maps:from_list(SimplifiedHookOrders) -end}. - %%-------------------------------------------------------------------- %% System Monitor %%-------------------------------------------------------------------- diff --git a/src/emqx_hooks.erl b/src/emqx_hooks.erl index e65985e09..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,21 +236,31 @@ 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 %%-------------------------------------------------------------------- init([]) -> ok = emqx_tables:new(?TAB, [{keypos, #hook.name}, {read_concurrency, true}]), - HookOrders = emqx:get_env(hook_order, #{}), - {ok, #{hook_orders => HookOrders}}. + {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_orders(HookPoint), Callback, Callbacks)) + ok = add_and_insert(HookPoint, [Callback], Callbacks) end, {reply, Reply, State}; @@ -251,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}; @@ -278,18 +305,89 @@ 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(Orders, C, Callbacks) -> - add_callback(Orders, 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(_Orders, C, [], Acc) -> +add_callback(Order, C, Callbacks) -> + add_callback(Order, C, Callbacks, []). + +add_callback(_Order, C, [], Acc) -> lists:reverse([C|Acc]); -add_callback(Orders, C1, [C2|More], Acc) -> - case is_lower_priority(Orders, C1, C2) of +add_callback(Order, C1, [C2|More], Acc) -> + case is_lower_priority(Order, C1, C2) of true -> - add_callback(Orders, C1, More, [C2|Acc]); + add_callback(Order, C1, More, [C2|Acc]); false -> lists:append(lists:reverse(Acc), [C1, C2 | More]) end. @@ -310,41 +408,141 @@ del_callback(Action, [Callback | Callbacks], Acc) -> %% does A have lower priority than B? -is_lower_priority(Orders, +is_lower_priority(Order, #callback{priority = PrA, action = ActA}, #callback{priority = PrB, action = ActB}) -> - OrdA = callback_order(Orders, ActA), - OrdB = callback_order(Orders, ActB), - case {OrdA, OrdB} of - %% if both action positions are not specified, use priority - {undefined, undefined} -> + 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; - %% actions with specified positions have higher priority - {_OrdA, undefined} -> + 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; - %% actions with specified positions have higher priority - {undefined, _OrdB} -> - true; - %% if both action positions are specified, the last one has the lower priority - _ -> - OrdA >= OrdB + _Sufix -> + true end. -callback_orders(HookPoint) -> - case hook_orders() of - #{HookPoint := CallbackOrders} -> CallbackOrders; - _ -> #{} - 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} + ]. -hook_orders() -> - case emqx:get_env(hook_order, #{}) of - Map when is_map(Map) -> Map; - _ -> #{} - 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} + ]. -callback_order(Orders, {M, _F, _A}) -> - case Orders of - #{M := N} -> N; - _ -> undefined - end; -callback_order(_Orders, _Action) -> undefined. +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 fba722c96..bee0dbefe 100644 --- a/test/emqx_hooks_SUITE.erl +++ b/test/emqx_hooks_SUITE.erl @@ -27,23 +27,9 @@ init_per_testcase(_Test, Config) -> Config. end_per_testcase(_Test, _Config) -> + _ = (catch emqx_hooks:stop()), _ = clear_orders(). -% t_lookup(_) -> -% error('TODO'). - -% t_run_fold(_) -> -% error('TODO'). - -% 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, []), @@ -113,16 +99,24 @@ t_uncovered_func(_) -> Pid ! test, ok = emqx_hooks:stop(). -t_explicit_order(_) -> +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(hookpoint, [mod_a, mod_b]), + 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), + 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( [ @@ -132,23 +126,53 @@ t_explicit_order(_) -> {mod_d, cb, []}, {mod_e, cb, []} ], - get_hookpoint_callbacks(hookpoint)). + 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(HookPoint, Mods) -> - Orders = maps:from_list(lists:zip(Mods, lists:seq(0, length(Mods) - 1))), - application:set_env(emqx, hook_order, #{HookPoint => Orders}). +set_orders(Key, OrderString) -> + application:set_env(emqx, Key, OrderString). clear_orders() -> - application:set_env(emqx, hook_order, #{}). + application:set_env(emqx, acl_order, "none"). get_hookpoint_callbacks(HookPoint) -> [emqx_hooks:callback_action(C) || C <- emqx_hooks:lookup(HookPoint)]. - %%-------------------------------------------------------------------- %% Hook fun %%--------------------------------------------------------------------