From 513d2bcaaaa7bd3b3036cf48d77579e3a656f4d3 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Fri, 22 Mar 2019 10:21:16 +0800 Subject: [PATCH] Fix reload ACL failed (#2344) The original `reload_acl` function only parse the ACL file, not compile and rehook to 'client.check_acl' --- src/emqx_access_control.erl | 6 ++- src/emqx_acl_cache.erl | 26 +++++----- src/emqx_mod_acl_internal.erl | 98 ++++++++++++++++------------------- 3 files changed, 63 insertions(+), 67 deletions(-) diff --git a/src/emqx_access_control.erl b/src/emqx_access_control.erl index 8a73e5dca..39db9f1b5 100644 --- a/src/emqx_access_control.erl +++ b/src/emqx_access_control.erl @@ -57,12 +57,14 @@ do_check_acl(#{zone := Zone} = Credentials, PubSub, Topic) -> _ -> deny end. --spec(reload_acl() -> list(ok | {error, term()})). +-spec(reload_acl() -> ok | {error, term()}). reload_acl() -> + emqx_acl_cache:is_enabled() andalso + emqx_acl_cache:empty_acl_cache(), emqx_mod_acl_internal:reload_acl(). init_auth_result(Credentials) -> case emqx_zone:get_env(maps:get(zone, Credentials, undefined), allow_anonymous, false) of true -> success; false -> not_authorized - end. \ No newline at end of file + end. diff --git a/src/emqx_acl_cache.erl b/src/emqx_acl_cache.erl index fb63e0ad5..92d4d8328 100644 --- a/src/emqx_acl_cache.erl +++ b/src/emqx_acl_cache.erl @@ -16,19 +16,19 @@ -include("emqx.hrl"). --export([ get_acl_cache/2 - , put_acl_cache/3 - , cleanup_acl_cache/0 - , empty_acl_cache/0 - , dump_acl_cache/0 - , get_cache_size/0 - , get_cache_max_size/0 - , get_newest_key/0 - , get_oldest_key/0 - , cache_k/2 - , cache_v/1 - , is_enabled/0 - ]). +-export([ get_acl_cache/2 + , put_acl_cache/3 + , cleanup_acl_cache/0 + , empty_acl_cache/0 + , dump_acl_cache/0 + , get_cache_size/0 + , get_cache_max_size/0 + , get_newest_key/0 + , get_oldest_key/0 + , cache_k/2 + , cache_v/1 + , is_enabled/0 + ]). -type(acl_result() :: allow | deny). diff --git a/src/emqx_mod_acl_internal.erl b/src/emqx_mod_acl_internal.erl index 0c4152c4e..aed961d04 100644 --- a/src/emqx_mod_acl_internal.erl +++ b/src/emqx_mod_acl_internal.erl @@ -25,11 +25,9 @@ -export([check_acl/5, reload_acl/0]). --define(ACL_RULE_TAB, emqx_acl_rule). +-define(MFA(M, F, A), {M, F, A}). --define(FUNC(M, F, A), {M, F, A}). - --type(acl_rules() :: #{publish => [emqx_access_rule:rule()], +-type(acl_rules() :: #{publish => [emqx_access_rule:rule()], subscribe => [emqx_access_rule:rule()]}). %%------------------------------------------------------------------------------ @@ -37,28 +35,64 @@ %%------------------------------------------------------------------------------ load(_Env) -> - Rules = load_rules_from_file(acl_file()), - emqx_hooks:add('client.check_acl', ?FUNC(?MODULE, check_acl, [Rules]), -1). + Rules = rules_from_file(acl_file()), + emqx_hooks:add('client.check_acl', ?MFA(?MODULE, check_acl, [Rules]), -1). unload(_Env) -> - Rules = load_rules_from_file(acl_file()), - emqx_hooks:del('client.check_acl', ?FUNC(?MODULE, check_acl, [Rules])). + Rules = rules_from_file(acl_file()), + emqx_hooks:del('client.check_acl', ?MFA(?MODULE, check_acl, [Rules])). %% @doc Read all rules -spec(all_rules() -> list(emqx_access_rule:rule())). all_rules() -> - load_rules_from_file(acl_file()). + rules_from_file(acl_file()). %%------------------------------------------------------------------------------ %% ACL callbacks %%------------------------------------------------------------------------------ -load_rules_from_file(AclFile) -> +%% @doc Check ACL +-spec(check_acl(emqx_types:credentials(), emqx_types:pubsub(), emqx_topic:topic(), + emqx_access_rule:acl_result(), acl_rules()) + -> {ok, allow} | {ok, deny} | ok). +check_acl(Credentials, PubSub, Topic, _AclResult, Rules) -> + case match(Credentials, Topic, lookup(PubSub, Rules)) of + {matched, allow} -> {ok, allow}; + {matched, deny} -> {ok, deny}; + nomatch -> ok + end. + +-spec(reload_acl() -> ok | {error, term()}). +reload_acl() -> + unload([]), load([]). + +%%------------------------------------------------------------------------------ +%% Internal Functions +%%------------------------------------------------------------------------------ + +acl_file() -> + emqx_config:get_env(acl_file). + +lookup(PubSub, Rules) -> + maps:get(PubSub, Rules, []). + +match(_Credentials, _Topic, []) -> + nomatch; +match(Credentials, Topic, [Rule|Rules]) -> + case emqx_access_rule:match(Credentials, Topic, Rule) of + nomatch -> + match(Credentials, Topic, Rules); + {matched, AllowDeny} -> + {matched, AllowDeny} + end. + +-spec(rules_from_file(file:filename()) -> map()). +rules_from_file(AclFile) -> case file:consult(AclFile) of {ok, Terms} -> Rules = [emqx_access_rule:compile(Term) || Term <- Terms], - #{publish => lists:filter(fun(Rule) -> filter(publish, Rule) end, Rules), - subscribe => lists:filter(fun(Rule) -> filter(subscribe, Rule) end, Rules)}; + #{publish => [Rule || Rule <- Rules, filter(publish, Rule)], + subscribe => [Rule || Rule <- Rules, filter(subscribe, Rule)]}; {error, Reason} -> ?LOG(error, "[ACL_INTERNAL] Failed to read ~s: ~p", [AclFile, Reason]), #{} @@ -77,43 +111,3 @@ filter(subscribe, {_AllowDeny, _Who, subscribe, _Topics}) -> filter(_PubSub, {_AllowDeny, _Who, _, _Topics}) -> false. -%% @doc Check ACL --spec(check_acl(emqx_types:credentials(), emqx_types:pubsub(), emqx_topic:topic(), - emqx_access_rule:acl_result(), acl_rules()) - -> {ok, allow} | {ok, deny} | ok). -check_acl(Credentials, PubSub, Topic, _AclResult, Rules) -> - case match(Credentials, Topic, lookup(PubSub, Rules)) of - {matched, allow} -> {ok, allow}; - {matched, deny} -> {ok, deny}; - nomatch -> ok - end. - -lookup(PubSub, Rules) -> - maps:get(PubSub, Rules, []). - -match(_Credentials, _Topic, []) -> - nomatch; -match(Credentials, Topic, [Rule|Rules]) -> - case emqx_access_rule:match(Credentials, Topic, Rule) of - nomatch -> - match(Credentials, Topic, Rules); - {matched, AllowDeny} -> - {matched, AllowDeny} - end. - --spec(reload_acl() -> ok | {error, term()}). -reload_acl() -> - try load_rules_from_file(acl_file()) of - _ -> - emqx_logger:info("Reload acl_file ~s successfully", [acl_file()]), - ok; - {error, Error} -> - {error, Error} - catch - error:Reason:StackTrace -> - ?LOG(error, "Reload acl failed. StackTrace: ~p", [StackTrace]), - {error, Reason} - end. - -acl_file() -> - emqx_config:get_env(acl_file).