From 68555ff4666588bba68e372b66d5bdbef3849838 Mon Sep 17 00:00:00 2001 From: Mickael Remond Date: Sat, 23 Jul 2016 17:57:44 +0200 Subject: [PATCH] Add support for checking access rules conformance for commands --- include/ejabberd_commands.hrl | 5 +++ src/ejabberd_admin.erl | 6 ++-- src/ejabberd_commands.erl | 62 +++++++++++++++++++++++------------ src/mod_http_api.erl | 1 + 4 files changed, 51 insertions(+), 23 deletions(-) diff --git a/include/ejabberd_commands.hrl b/include/ejabberd_commands.hrl index 2b4eca581..bafd93a4d 100644 --- a/include/ejabberd_commands.hrl +++ b/include/ejabberd_commands.hrl @@ -38,19 +38,24 @@ function :: atom() | '_', args = [] :: [aterm()] | '_' | '$1' | '$2', policy = restricted :: open | restricted | admin | user, + access_rules = [] :: [atom()], result = {res, rescode} :: rterm() | '_' | '$2', args_desc = none :: none | [string()] | '_', result_desc = none :: none | string() | '_', args_example = none :: none | [any()] | '_', result_example = none :: any()}). +%% TODO Fix me: Type is not up to date -type ejabberd_commands() :: #ejabberd_commands{name :: atom(), tags :: [atom()], desc :: string(), longdesc :: string(), + version :: integer(), module :: atom(), function :: atom(), args :: [aterm()], + policy :: open | restricted | admin | user, + access_rules :: [atom()], result :: rterm()}. %% @type ejabberd_commands() = #ejabberd_commands{ diff --git a/src/ejabberd_admin.erl b/src/ejabberd_admin.erl index 87ac76875..68aaf9be6 100644 --- a/src/ejabberd_admin.erl +++ b/src/ejabberd_admin.erl @@ -129,6 +129,8 @@ get_commands_spec() -> #ejabberd_commands{name = register, tags = [accounts], desc = "Register a user", + policy = admin, + access_rules = [configure], module = ?MODULE, function = register, args = [{user, binary}, {host, binary}, {password, binary}], result = {res, restuple}}, @@ -166,7 +168,7 @@ get_commands_spec() -> #ejabberd_commands{name = list_cluster, tags = [cluster], desc = "List nodes that are part of the cluster handled by Node", module = ?MODULE, function = list_cluster, - args = [], + args = [], result = {nodes, {list, {node, atom}}}}, #ejabberd_commands{name = import_file, tags = [mnesia], @@ -220,7 +222,7 @@ get_commands_spec() -> desc = "Delete offline messages older than DAYS", module = ?MODULE, function = delete_old_messages, args = [{days, integer}], result = {res, rescode}}, - + #ejabberd_commands{name = export2sql, tags = [mnesia], desc = "Export virtual host information from Mnesia tables to SQL files", module = ejd2sql, function = export, diff --git a/src/ejabberd_commands.erl b/src/ejabberd_commands.erl index 075ff35cf..0f8cd2e0a 100644 --- a/src/ejabberd_commands.erl +++ b/src/ejabberd_commands.erl @@ -494,7 +494,7 @@ execute_command(AccessCommands, Auth, Name, Arguments) -> %% %% @doc Execute a command in a given API version %% Can return the following exceptions: -%% command_unknown | account_unprivileged | invalid_account_data | no_auth_provided +%% command_unknown | account_unprivileged | invalid_account_data | no_auth_provided | access_rules_unauthorized execute_command(AccessCommands1, Auth1, Name, Arguments, Version) -> execute_command(AccessCommands1, Auth1, Name, Arguments, Version, #{}). @@ -503,32 +503,45 @@ execute_command(AccessCommands1, Auth1, Name, Arguments, Version, CallerInfo) -> true -> admin; false -> Auth1 end, + TokenJID = oauth_token_user(Auth1), Command = get_command_definition(Name, Version), AccessCommands = get_access_commands(AccessCommands1, Version), case check_access_commands(AccessCommands, Auth, Name, Command, Arguments, CallerInfo) of - ok -> execute_command2(Auth, Command, Arguments) + ok -> execute_check_policy(Auth, TokenJID, Command, Arguments) end. -execute_command2( - _Auth, #ejabberd_commands{policy = open} = Command, Arguments) -> - execute_command2(Command, Arguments); -execute_command2( - _Auth, #ejabberd_commands{policy = restricted} = Command, Arguments) -> - execute_command2(Command, Arguments); -execute_command2( - _Auth, #ejabberd_commands{policy = admin} = Command, Arguments) -> - execute_command2(Command, Arguments); -execute_command2( - admin, #ejabberd_commands{policy = user} = Command, Arguments) -> - execute_command2(Command, Arguments); -execute_command2( - noauth, #ejabberd_commands{policy = user} = Command, Arguments) -> - execute_command2(Command, Arguments); -execute_command2( - {User, Server, _, _}, #ejabberd_commands{policy = user} = Command, Arguments) -> - execute_command2(Command, [User, Server | Arguments]). -execute_command2(Command, Arguments) -> +execute_check_policy( + _Auth, _JID, #ejabberd_commands{policy = open} = Command, Arguments) -> + do_execute_command(Command, Arguments); +execute_check_policy( + _Auth, _JID, #ejabberd_commands{policy = restricted} = Command, Arguments) -> + do_execute_command(Command, Arguments); +execute_check_policy( + _Auth, JID, #ejabberd_commands{policy = admin} = Command, Arguments) -> + execute_check_access(JID, Command, Arguments); +execute_check_policy( + admin, JID, #ejabberd_commands{policy = user} = Command, Arguments) -> + execute_check_access(JID, Command, Arguments); +execute_check_policy( + noauth, _JID, #ejabberd_commands{policy = user} = Command, Arguments) -> + do_execute_command(Command, Arguments); +execute_check_policy( + {User, Server, _, _}, JID, #ejabberd_commands{policy = user} = Command, Arguments) -> + execute_check_access(JID, Command, [User, Server | Arguments]). + +execute_check_access(_FromJID, #ejabberd_commands{access_rules = []} = Command, Arguments) -> + do_execute_command(Command, Arguments); +execute_check_access(FromJID, #ejabberd_commands{access_rules = Rules} = Command, Arguments) -> + %% TODO Review: Do we have smarter / better way to check rule on other Host than global ? + case acl:any_rules_allowed(global, Rules, FromJID) of + true -> + do_execute_command(Command, Arguments); + false -> + {error, access_rules_unauthorized} + end. + +do_execute_command(Command, Arguments) -> Module = Command#ejabberd_commands.module, Function = Command#ejabberd_commands.function, ?DEBUG("Executing command ~p:~p with Args=~p", [Module, Function, Arguments]), @@ -754,6 +767,13 @@ get_commands(Version) -> end, AdminCmds ++ UserCmds, Opts), Cmds. +oauth_token_user(noauth) -> + undefined; +oauth_token_user(admin) -> + undefined; +oauth_token_user({User, Server, _, _}) -> + jid:make(User, Server, <<>>). + is_admin(_Name, admin, _Extra) -> true; is_admin(_Name, {_User, _Server, _, false}, _Extra) -> diff --git a/src/mod_http_api.erl b/src/mod_http_api.erl index 07a1574e9..bc30ee090 100644 --- a/src/mod_http_api.erl +++ b/src/mod_http_api.erl @@ -136,6 +136,7 @@ check_permissions(Request, Command) -> {ok, CommandPolicy, Scope} = ejabberd_commands:get_command_policy_and_scope(Call), check_permissions2(Request, Call, CommandPolicy, Scope); _ -> + %% TODO Should this be a 404 or 400 instead of 401 ? unauthorized_response() end.