Access commands in maps

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.
 

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,

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()] ) -> [{_, _}].

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) ->

test/commands_SUITE.erl

@@ -127,26 +127,28 @@ old_exec(_C) ->
 
 old_access_ctl(_C) ->
     %% with no auth method it is all fine
-    checkauth(true, [], noauth),
+    checkauth(true, #{}, noauth),
     %% noauth fails if first item is not 'all' (users)
-    checkauth(account_unprivileged, [{none, none, []}], noauth),
+    checkauth(account_unprivileged, #{none => command_rules(all)}, noauth),
     %% if here we allow all commands to noauth
-    checkauth(true, [{all, all, []}], noauth),
+    checkauth(true, #{all => command_rules(all)}, noauth),
     %% and here only command_one
-    checkauth(true, [{all, [command_one], []}], noauth),
+    checkauth(true, #{all => command_rules([command_one])}, noauth),
     %% so this'd fail
-    checkauth(account_unprivileged, [{all, [command_two], []}], noauth),
+    checkauth(account_unprivileged, #{all => command_rules([command_two])}, noauth),
     % now we provide a role name, this requires a user and triggers password and acl check
     % this fails because password is bad
-    checkauth(invalid_account_data, [{some_acl_role, [command_one], []}], {<<"zenek">>, <<"localhost">>, <<"bbb">>}),
+    checkauth(invalid_account_data, #{some_acl_role => command_rules([command_one])},
+              {<<"zenek">>, <<"localhost">>, <<"bbb">>}),
     % this, because of acl
-    checkauth(account_unprivileged, [{some_acl_role, [command_one], []}], {<<"zenek">>, <<"localhost">>, <<"">>}),
+    checkauth(account_unprivileged, #{some_acl_role => command_rules([command_one])},
+              {<<"zenek">>, <<"localhost">>, <<"">>}),
     % and this should work, because we define command_one as available to experts only, while acls in config
     % (see ggo/1) state that experts-only funcs are available to coders and managers, and zenek is a coder, gah.
-    checkauth(true, [{experts_only, [command_one], []}], {<<"zenek">>, <<"localhost">>, <<"">>}),
+    checkauth(true, #{experts_only => command_rules([command_one])},
+              {<<"zenek">>, <<"localhost">>, <<"">>}),
     ok.
 
-
 new_type_checker(_C) ->
     true = t_check_type({msg, binary}, <<"zzz">>),
     true = t_check_type({msg, integer}, 127),
@@ -603,6 +605,9 @@ mc_holder() ->
     end,
     erlang:exit(Pid, kill).
 
+command_rules(Commands) ->
+    #{commands => Commands, argument_restrictions => #{}}.
+
 checkauth(true, AccessCommands, Auth) ->
     B = <<"bzzzz">>,
     B = ejabberd_commands:execute_command(AccessCommands, Auth, command_one, [B]);