From db31e5f0d946071c2cd560c7cdf4de49e7f6200c Mon Sep 17 00:00:00 2001 From: Ilya Averyanov Date: Tue, 8 Aug 2023 11:52:10 +0300 Subject: [PATCH] chore(auth): tidy up the code --- apps/emqx/include/emqx_hooks.hrl | 1 - apps/emqx/src/emqx_access_control.erl | 13 ++++------- apps/emqx/src/emqx_schema_hooks.erl | 22 +++++++++---------- .../include/emqx_authentication.hrl | 10 ++++----- apps/emqx_authn/src/emqx_authentication.erl | 8 +------ .../src/emqx_authentication_config.erl | 5 ++--- .../test/emqx_authentication_SUITE.erl | 2 +- 7 files changed, 24 insertions(+), 37 deletions(-) diff --git a/apps/emqx/include/emqx_hooks.hrl b/apps/emqx/include/emqx_hooks.hrl index dffc09fcd..2373b5928 100644 --- a/apps/emqx/include/emqx_hooks.hrl +++ b/apps/emqx/include/emqx_hooks.hrl @@ -29,7 +29,6 @@ -define(HP_RETAINER, 930). -define(HP_AUTO_SUB, 920). -define(HP_RULE_ENGINE, 900). - %% apps that can work with the republish action -define(HP_SLOW_SUB, 880). -define(HP_BRIDGE, 870). diff --git a/apps/emqx/src/emqx_access_control.erl b/apps/emqx/src/emqx_access_control.erl index a8ffadb44..82604710a 100644 --- a/apps/emqx/src/emqx_access_control.erl +++ b/apps/emqx/src/emqx_access_control.erl @@ -30,8 +30,8 @@ -compile(nowarn_export_all). -endif. --define(TRACE_RESULT(Label, Tag, Result, Reason), begin - ?TRACE(Label, Tag, #{ +-define(TRACE_RESULT(Label, Result, Reason), begin + ?TRACE(Label, ?AUTHN_TRACE_TAG, #{ result => (Result), reason => (Reason) }), @@ -115,18 +115,13 @@ authorize(ClientInfo, Action, Topic) -> -spec pre_hook_authenticate(emqx_types:clientinfo()) -> ok | continue | {error, not_authorized}. pre_hook_authenticate(#{enable_authn := false}) -> - ?TRACE_RESULT("pre_hook_authenticate", ?AUTHN_TRACE_TAG, ok, enable_authn_false); + ?TRACE_RESULT("pre_hook_authenticate", ok, enable_authn_false); pre_hook_authenticate(#{enable_authn := quick_deny_anonymous} = Credential) -> case is_username_defined(Credential) of true -> continue; false -> - ?TRACE_RESULT( - "pre_hook_authenticate", - ?AUTHN_TRACE_TAG, - {error, not_authorized}, - enable_authn_false - ) + ?TRACE_RESULT("pre_hook_authenticate", {error, not_authorized}, enable_authn_false) end; pre_hook_authenticate(_) -> continue. diff --git a/apps/emqx/src/emqx_schema_hooks.erl b/apps/emqx/src/emqx_schema_hooks.erl index bd50a069f..e704af6cc 100644 --- a/apps/emqx/src/emqx_schema_hooks.erl +++ b/apps/emqx/src/emqx_schema_hooks.erl @@ -24,12 +24,12 @@ }. -optional_callbacks([injected_fields/0]). +-export_type([hookpoint/0]). + -define(HOOKPOINT_PT_KEY(POINT_NAME), {?MODULE, fields, POINT_NAME}). -export([ injection_point/1, - any_injections/1, - inject_fields/2, inject_from_modules/1 ]). @@ -46,10 +46,6 @@ injection_point(PointName) -> persistent_term:get(?HOOKPOINT_PT_KEY(PointName), []). -inject_fields(PointName, Fields) -> - Key = ?HOOKPOINT_PT_KEY(PointName), - persistent_term:put(Key, Fields). - erase_injections() -> lists:foreach( fun @@ -72,9 +68,6 @@ any_injections() -> persistent_term:get() ). -any_injections(PointName) -> - persistent_term:get(?HOOKPOINT_PT_KEY(PointName), undefined) =/= undefined. - inject_from_modules(Modules) -> Injections = lists:foldl( @@ -109,10 +102,17 @@ append_module_injections(ModuleInjections, AllInjections) when is_map(ModuleInje inject_fields([]) -> ok; inject_fields([{PointName, Fields} | Rest]) -> - case emqx_schema_hooks:any_injections(PointName) of + case any_injections(PointName) of true -> inject_fields(Rest); false -> - ok = emqx_schema_hooks:inject_fields(PointName, Fields), + ok = inject_fields(PointName, Fields), inject_fields(Rest) end. + +inject_fields(PointName, Fields) -> + Key = ?HOOKPOINT_PT_KEY(PointName), + persistent_term:put(Key, Fields). + +any_injections(PointName) -> + persistent_term:get(?HOOKPOINT_PT_KEY(PointName), undefined) =/= undefined. diff --git a/apps/emqx_authn/include/emqx_authentication.hrl b/apps/emqx_authn/include/emqx_authentication.hrl index a367af291..c294b8d99 100644 --- a/apps/emqx_authn/include/emqx_authentication.hrl +++ b/apps/emqx_authn/include/emqx_authentication.hrl @@ -20,11 +20,6 @@ -include_lib("emqx/include/logger.hrl"). -include_lib("emqx/include/emqx_access_control.hrl"). -%% config root name all auth providers have to agree on. --define(EMQX_AUTHENTICATION_CONFIG_ROOT_NAME, "authentication"). --define(EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_ATOM, authentication). --define(EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY, <<"authentication">>). - -define(GLOBAL, 'mqtt:global'). -define(TRACE_AUTHN_PROVIDER(Msg), ?TRACE_AUTHN_PROVIDER(Msg, #{})). @@ -36,6 +31,11 @@ -define(TRACE_AUTHN(Msg, Meta), ?TRACE_AUTHN(debug, Msg, Meta)). -define(TRACE_AUTHN(Level, Msg, Meta), ?TRACE(Level, ?AUTHN_TRACE_TAG, Msg, Meta)). +%% config root name all auth providers have to agree on. +-define(EMQX_AUTHENTICATION_CONFIG_ROOT_NAME, "authentication"). +-define(EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_ATOM, authentication). +-define(EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY, <<"authentication">>). + %% authentication move cmd -define(CMD_MOVE_FRONT, front). -define(CMD_MOVE_REAR, rear). diff --git a/apps/emqx_authn/src/emqx_authentication.erl b/apps/emqx_authn/src/emqx_authentication.erl index 5b49f5d28..8f055e049 100644 --- a/apps/emqx_authn/src/emqx_authentication.erl +++ b/apps/emqx_authn/src/emqx_authentication.erl @@ -60,7 +60,6 @@ register_providers/1, deregister_provider/1, deregister_providers/1, - providers/0, delete_chain/1, lookup_chain/1, list_chains/0, @@ -266,6 +265,7 @@ get_enabled(Authenticators) -> %%------------------------------------------------------------------------------ %% @doc Get all registered authentication providers. +-spec get_providers() -> #{authn_type() => module()}. get_providers() -> call(get_providers). @@ -332,10 +332,6 @@ deregister_providers(AuthNTypes) when is_list(AuthNTypes) -> deregister_provider(AuthNType) -> deregister_providers([AuthNType]). --spec providers() -> [{authn_type(), module()}]. -providers() -> - call(providers). - -spec delete_chain(chain_name()) -> ok | {error, term()}. delete_chain(Name) -> call({delete_chain, Name}). @@ -468,8 +464,6 @@ handle_call( end; handle_call({deregister_providers, AuthNTypes}, _From, #{providers := Providers} = State) -> reply(ok, State#{providers := maps:without(AuthNTypes, Providers)}); -handle_call(providers, _From, #{providers := Providers} = State) -> - reply(maps:to_list(Providers), State); handle_call({delete_chain, ChainName}, _From, State) -> UpdateFun = fun(Chain) -> {_MatchedIDs, NewChain} = do_delete_authenticators(fun(_) -> true end, Chain), diff --git a/apps/emqx_authn/src/emqx_authentication_config.erl b/apps/emqx_authn/src/emqx_authentication_config.erl index 431a2cf49..97d082da6 100644 --- a/apps/emqx_authn/src/emqx_authentication_config.erl +++ b/apps/emqx_authn/src/emqx_authentication_config.erl @@ -148,7 +148,7 @@ do_pre_config_update(Paths, NewConfig, _OldConfig) -> ]}. -spec propagated_pre_config_update(list(atom()), update_request(), emqx_config:raw_config()) -> - ok | {error, term()}. + {ok, map() | list()} | {error, term()}. propagated_pre_config_update(Paths, NewConfig, OldConfig) -> do_pre_config_update(Paths, NewConfig, OldConfig). @@ -217,8 +217,7 @@ do_post_config_update(Paths, _UpdateReq, NewConfig0, OldConfig0, _AppEnvs) -> emqx_config:raw_config(), emqx_config:app_envs() ) -> - ok | {ok, map()} | {error, term()}. - + ok. propagated_post_config_update(Paths, UpdateReq, NewConfig, OldConfig, AppEnvs) -> ok = post_config_update(Paths, UpdateReq, NewConfig, OldConfig, AppEnvs), ok. diff --git a/apps/emqx_authn/test/emqx_authentication_SUITE.erl b/apps/emqx_authn/test/emqx_authentication_SUITE.erl index dfeeb458e..a15f22c41 100644 --- a/apps/emqx_authn/test/emqx_authentication_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authentication_SUITE.erl @@ -591,5 +591,5 @@ deregister_providers() -> fun({Type, _Module}) -> ok = ?AUTHN:deregister_provider(Type) end, - lists:flatten([?AUTHN:providers()]) + maps:to_list(?AUTHN:get_providers()) ).