From c3ca7282e10f9364c769941b8ad8b6b61d0e16a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 26 May 2022 15:19:58 +0200 Subject: [PATCH] Convert mongooseimctl_access_commands to use maps with defaults The logic can use the maps directly to be more straightforward. This allows to remove the 'prepend_key' wrapper, which was temporary. --- src/config/mongoose_config_parser_toml.erl | 3 - src/config/mongoose_config_spec.erl | 23 +++---- src/ejabberd_commands.erl | 73 +++++++++------------- src/ejabberd_ctl.erl | 13 ++-- 4 files changed, 42 insertions(+), 70 deletions(-) diff --git a/src/config/mongoose_config_parser_toml.erl b/src/config/mongoose_config_parser_toml.erl index be078aaaf6..31f62f2910 100644 --- a/src/config/mongoose_config_parser_toml.erl +++ b/src/config/mongoose_config_parser_toml.erl @@ -253,9 +253,6 @@ wrap(_Path, V, item) -> [V]; wrap(_Path, _V, remove) -> []; -wrap([Key|_], V, prepend_key) -> - L = [b2a(Key) | tuple_to_list(V)], - [list_to_tuple(L)]; wrap(_Path, V, none) when is_list(V) -> V. diff --git a/src/config/mongoose_config_spec.erl b/src/config/mongoose_config_spec.erl index 698b5eae25..90e6c245b3 100644 --- a/src/config/mongoose_config_spec.erl +++ b/src/config/mongoose_config_spec.erl @@ -13,7 +13,6 @@ -export([process_root/1, process_host/1, process_general/1, - process_ctl_access_rule/1, process_listener/2, process_c2s_tls/1, process_fast_tls/1, @@ -60,8 +59,7 @@ | item % [Value] | remove % [] - the item is ignored | none % just Value - injects elements of Value into the parent section/list - | {kv, NewKey :: term()} % [{NewKey, Value}] - replaces the key with NewKey - | prepend_key. % [{Key, V1, ..., Vn}] when Value = {V1, ..., Vn} + | {kv, NewKey :: term()}. % [{NewKey, Value}] - replaces the key with NewKey %% This option allows to put list/section items in a map -type format_items() :: @@ -194,7 +192,6 @@ general() -> wrap = host_config}, <<"mongooseimctl_access_commands">> => #section{ items = #{default => ctl_access_rule()}, - format_items = list, wrap = global_config}, <<"routing_modules">> => #list{items = #option{type = atom, validate = module}, @@ -221,23 +218,20 @@ general_defaults() -> <<"all_metrics_are_global">> => false, <<"sm_backend">> => mnesia, <<"rdbms_server_type">> => generic, - <<"mongooseimctl_access_commands">> => [], + <<"mongooseimctl_access_commands">> => #{}, <<"routing_modules">> => mongoose_router:default_routing_modules(), <<"replaced_wait_timeout">> => 2000, <<"hide_service_name">> => false}. ctl_access_rule() -> #section{ - items = #{<<"commands">> => #list{items = #option{type = string}}, - <<"argument_restrictions">> => #section{ - items = #{default => #option{type = string}}, - format_items = list - } + items = #{<<"commands">> => #list{items = #option{type = atom, + validate = non_empty}}, + <<"argument_restrictions">> => + #section{items = #{default => #option{type = string}}} }, defaults = #{<<"commands">> => all, - <<"argument_restrictions">> => []}, - process = fun ?MODULE:process_ctl_access_rule/1, - wrap = prepend_key + <<"argument_restrictions">> => #{}} }. %% path: general.domain_certfile @@ -975,9 +969,6 @@ is_host_type_item({{_, HostType}, _}, HostTypes) -> is_host_type_item(_, _) -> false. -process_ctl_access_rule(#{commands := Commands, argument_restrictions := ArgRestrictions}) -> - {Commands, ArgRestrictions}. - process_host(Host) -> Node = jid:nodeprep(Host), true = Node =/= error, diff --git a/src/ejabberd_commands.erl b/src/ejabberd_commands.erl index 1b08bf5535..5077a47d9f 100644 --- a/src/ejabberd_commands.erl +++ b/src/ejabberd_commands.erl @@ -231,20 +231,22 @@ -type cmd_error() :: command_unknown | account_unprivileged | invalid_account_data | no_auth_provided. --type access_cmd() :: {Access :: atom(), - CommandNames :: [atom()], - Arguments :: [term()] - }. +-type access_commands() :: #{acl:rule_name() => command_rules()}. +-type command_rules() :: #{commands := all | [atom()], + argument_restrictions := argument_restrictions()}. + +%% Currently only string arguments can have restrictions +-type argument_restrictions() :: #{ArgName :: atom() => Value :: string()}. + -type list_cmd() :: {Name::atom(), Args::[aterm()], Desc::string()}. -export_type([rterm/0, aterm/0, cmd/0, auth/0, - access_cmd/0, + access_commands/0, list_cmd/0]). - init() -> case ets:info(ejabberd_commands) of undefined -> @@ -254,7 +256,6 @@ init() -> ok end. - %% @doc Register ejabberd commands. If a command is already registered, a %% warning is printed and the old command is preserved. -spec register_commands([cmd()]) -> ok. @@ -269,7 +270,6 @@ register_commands(Commands) -> end, Commands). - %% @doc Unregister ejabberd commands. -spec unregister_commands([cmd()]) -> ok. unregister_commands(Commands) -> @@ -279,7 +279,6 @@ unregister_commands(Commands) -> end, Commands). - %% @doc Get a list of all the available commands, arguments and description. -spec list_commands() -> [list_cmd()]. list_commands() -> @@ -290,7 +289,6 @@ list_commands() -> _ = '_'}), [{A, B, C} || [A, B, C] <- Commands]. - %% @doc Get the format of arguments and result of a command. -spec get_command_format(Name::atom()) -> {Args::[aterm()], Result::rterm()} | {error, command_unknown}. @@ -307,7 +305,6 @@ get_command_format(Name) -> {Args, Result} end. - %% @doc Get the definition record of a command. -spec get_command_definition(Name::atom()) -> cmd() | 'command_not_found'. get_command_definition(Name) -> @@ -316,16 +313,14 @@ get_command_definition(Name) -> [] -> command_not_found end. - %% @doc Execute a command. -spec execute_command(Name :: atom(), Arguments :: list() ) -> Result :: term() | {error, command_unknown}. execute_command(Name, Arguments) -> - execute_command([], noauth, Name, Arguments). - + execute_command(#{}, noauth, Name, Arguments). --spec execute_command(AccessCommands :: [access_cmd()], +-spec execute_command(AccessCommands :: access_commands(), Auth :: auth(), Name :: atom(), Arguments :: [term()] @@ -341,7 +336,6 @@ execute_command(AccessCommands, Auth, Name, Arguments) -> [] -> {error, command_unknown} end. - %% @private execute_command2(Command, Arguments) -> Module = Command#ejabberd_commands.module, @@ -352,7 +346,6 @@ execute_command2(Command, Arguments) -> command_args => Arguments}), apply(Module, Function, Arguments). - %% @doc Get all the tags and associated commands. -spec get_tags_commands() -> [{Tag::string(), [CommandName::string()]}]. get_tags_commands() -> @@ -381,42 +374,38 @@ get_tags_commands() -> CommandTags), orddict:to_list(Dict). - %% ----------------------------- %% Access verification %% ----------------------------- - %% @doc Check access is allowed to that command. %% At least one AccessCommand must be satisfied. %% May throw {error, account_unprivileged | invalid_account_data} --spec check_access_commands(AccessCommands :: [ access_cmd() ], +-spec check_access_commands(AccessCommands :: access_commands(), Auth :: auth(), Method :: atom(), Command :: tuple(), Arguments :: [any()] ) -> ok | none(). -check_access_commands([], _Auth, _Method, _Command, _Arguments) -> - ok; +check_access_commands(AccessCommands, _Auth, _Method, _Command, _Arguments) + when AccessCommands =:= #{} -> ok; check_access_commands(AccessCommands, Auth, Method, Command, Arguments) -> AccessCommandsAllowed = - lists:filter( - fun({Access, Commands, ArgumentRestrictions}) -> + maps:filter( + fun(Access, CommandSpec) -> case check_access(Access, Auth) of true -> - check_access_command(Commands, Command, ArgumentRestrictions, - Method, Arguments); + check_access_command(Command, CommandSpec, Method, Arguments); false -> false end end, AccessCommands), - case AccessCommandsAllowed of - [] -> throw({error, account_unprivileged}); - L when is_list(L) -> ok + case AccessCommandsAllowed =:= #{} of + true -> throw({error, account_unprivileged}); + false -> ok end. - %% @private %% May throw {error, invalid_account_data} -spec check_auth(auth()) -> {ok, jid:jid()} | no_return(). @@ -431,13 +420,11 @@ check_auth({User, Server, Password}) -> _ -> throw({error, invalid_account_data}) end. - -spec get_md5(iodata()) -> string(). get_md5(AccountPass) -> lists:flatten([io_lib:format("~.16B", [X]) || X <- binary_to_list(crypto:hash(md5, AccountPass))]). - -spec check_access(Access :: acl:rule_name(), Auth :: auth()) -> boolean(). check_access(all, _) -> true; @@ -453,29 +440,27 @@ check_access(Access, Auth) -> deny -> false end. - --spec check_access_command(_, tuple(), _, _, _) -> boolean(). -check_access_command(Commands, Command, ArgumentRestrictions, Method, Arguments) -> - case Commands==all orelse lists:member(Method, Commands) of +-spec check_access_command(cmd(), command_rules(), atom(), [any()]) -> boolean(). +check_access_command(Command, CommandRules, Method, Arguments) -> + #{commands := Commands, argument_restrictions := ArgumentRestrictions} = CommandRules, + case Commands == all orelse lists:member(Method, Commands) of true -> check_access_arguments(Command, ArgumentRestrictions, Arguments); false -> false end. - -spec check_access_arguments(Command :: cmd(), Restrictions :: [any()], Args :: [any()]) -> boolean(). check_access_arguments(Command, ArgumentRestrictions, Arguments) -> ArgumentsTagged = tag_arguments(Command#ejabberd_commands.args, Arguments), lists:all( - fun({ArgName, ArgAllowedValue}) -> - %% If the call uses the argument, check the value is acceptable - case lists:keysearch(ArgName, 1, ArgumentsTagged) of - {value, {ArgName, ArgValue}} -> ArgValue == ArgAllowedValue; - false -> true + fun({ArgName, ArgValue}) -> + case ArgumentRestrictions of + %% If there is a restriction, check the value is acceptable + #{ArgName := ArgAllowedValue} -> ArgValue =:= ArgAllowedValue; + #{} -> true end - end, ArgumentRestrictions). - + end, ArgumentsTagged). -spec tag_arguments(ArgsDefs :: [{atom(), integer() | string() | {_, _}}], Args :: [any()] ) -> [{_, _}]. diff --git a/src/ejabberd_ctl.erl b/src/ejabberd_ctl.erl index 6946163cf5..1804288f58 100644 --- a/src/ejabberd_ctl.erl +++ b/src/ejabberd_ctl.erl @@ -218,9 +218,8 @@ process(Args) -> Code. --spec process2(Args :: [string()], - AccessCommands :: [ejabberd_commands:access_cmd()] - ) -> {String::string(), Code::integer()}. +-spec process2(Args :: [string()], AccessCommands :: ejabberd_commands:access_commands()) -> + {String::string(), Code::integer()}. process2(["--auth", User, Server, Pass | Args], AccessCommands) -> process2(Args, {list_to_binary(User), list_to_binary(Server), list_to_binary(Pass)}, AccessCommands); @@ -245,7 +244,7 @@ process2(Args, Auth, AccessCommands) -> end. --spec get_accesscommands() -> [char() | tuple()]. +-spec get_accesscommands() -> ejabberd_commands:access_commands(). get_accesscommands() -> mongoose_config:get_opt(mongooseimctl_access_commands). @@ -256,7 +255,7 @@ get_accesscommands() -> -spec try_run_ctp(Args :: [string()], Auth :: ejabberd_commands:auth(), - AccessCommands :: [ejabberd_commands:access_cmd()] + AccessCommands :: ejabberd_commands:access_commands() ) -> string() | integer() | {string(), integer()} | {string(), wrong_command_arguments}. try_run_ctp(Args, Auth, AccessCommands) -> try mongoose_hooks:ejabberd_ctl_process(false, Args) of @@ -281,7 +280,7 @@ try_run_ctp(Args, Auth, AccessCommands) -> -spec try_call_command(Args :: [string()], Auth :: ejabberd_commands:auth(), - AccessCommands :: [ejabberd_commands:access_cmd()] + AccessCommands :: ejabberd_commands:access_commands() ) -> string() | integer() | {string(), integer()} | {string(), wrong_command_arguments}. try_call_command(Args, Auth, AccessCommands) -> try call_command(Args, Auth, AccessCommands) of @@ -297,7 +296,7 @@ try_call_command(Args, Auth, AccessCommands) -> -spec call_command(Args :: [string()], Auth :: ejabberd_commands:auth(), - AccessCommands :: [ejabberd_commands:access_cmd()] + AccessCommands :: ejabberd_commands:access_commands() ) -> string() | integer() | {string(), integer()} | {string(), wrong_command_arguments} | {error, command_unknown}. call_command([CmdString | Args], Auth, AccessCommands) ->