mirror of
https://github.com/processone/ejabberd.git
synced 2024-11-24 16:23:40 +01:00
Initial attempt on access on commands
May change and will require more work / test / refactor
This commit is contained in:
parent
b4a430541d
commit
d7ad99f147
@ -28,6 +28,23 @@
|
||||
|
||||
-type oauth_scope() :: atom().
|
||||
|
||||
%% ejabberd_commands OAuth ReST ACL definition:
|
||||
%% Two fields exist that are used to control access on a command from ReST API:
|
||||
%% 1. Policy
|
||||
%% If policy is:
|
||||
%% - restricted: command is not exposed as OAuth Rest API.
|
||||
%% - admin: Command is allowed for user that have Admin Rest command enabled by access rule: commands_admin_access
|
||||
%% - user: Command might be called by any server user.
|
||||
%% - open: Command can be called by anyone.
|
||||
%%
|
||||
%% Policy is just used to control who can call the command. A specific additional access rules can be performed, as
|
||||
%% defined by access option.
|
||||
%% Access option can be a list of:
|
||||
%% - {Module, accessName, DefaultValue}: Reference and existing module access to limit who can use the command.
|
||||
%% - AccessRule name: direct name of the access rule to check in config file.
|
||||
%% TODO: Access option could be atom command (not a list). In the case, User performing the command, will be added as first parameter
|
||||
%% to command, so that the command can perform additional check.
|
||||
|
||||
-record(ejabberd_commands,
|
||||
{name :: atom(),
|
||||
tags = [] :: [atom()] | '_' | '$2',
|
||||
@ -38,7 +55,8 @@
|
||||
function :: atom() | '_',
|
||||
args = [] :: [aterm()] | '_' | '$1' | '$2',
|
||||
policy = restricted :: open | restricted | admin | user,
|
||||
access_rules = [] :: [atom()],
|
||||
%% access is: [accessRuleName] or [{Module, AccessOption, DefaultAccessRuleName}]
|
||||
access = [] :: [{atom(),atom(),atom()}|atom()],
|
||||
result = {res, rescode} :: rterm() | '_' | '$2',
|
||||
args_desc = none :: none | [string()] | '_',
|
||||
result_desc = none :: none | string() | '_',
|
||||
@ -55,7 +73,7 @@
|
||||
function :: atom(),
|
||||
args :: [aterm()],
|
||||
policy :: open | restricted | admin | user,
|
||||
access_rules :: [atom()],
|
||||
access :: [{atom(),atom(),atom()}|atom()],
|
||||
result :: rterm()}.
|
||||
|
||||
%% @type ejabberd_commands() = #ejabberd_commands{
|
||||
|
@ -130,7 +130,7 @@ get_commands_spec() ->
|
||||
#ejabberd_commands{name = register, tags = [accounts],
|
||||
desc = "Register a user",
|
||||
policy = admin,
|
||||
access_rules = [configure],
|
||||
access = [{mod_register, access, configure}],
|
||||
module = ?MODULE, function = register,
|
||||
args = [{user, binary}, {host, binary}, {password, binary}],
|
||||
result = {res, restuple}},
|
||||
|
@ -433,7 +433,7 @@ get_commands_definition(Version) ->
|
||||
%% @doc Execute a command.
|
||||
%% Can return the following exceptions:
|
||||
%% command_unknown | account_unprivileged | invalid_account_data |
|
||||
%% no_auth_provided
|
||||
%% no_auth_provided | access_rules_unauthorized
|
||||
execute_command(Name, Arguments) ->
|
||||
execute_command(Name, Arguments, ?DEFAULT_VERSION).
|
||||
|
||||
@ -505,7 +505,7 @@ execute_command(AccessCommands1, Auth1, Name, Arguments, Version, CallerInfo) ->
|
||||
end,
|
||||
TokenJID = oauth_token_user(Auth1),
|
||||
Command = get_command_definition(Name, Version),
|
||||
AccessCommands = get_access_commands(AccessCommands1, Version),
|
||||
AccessCommands = get_all_access_commands(AccessCommands1),
|
||||
case check_access_commands(AccessCommands, Auth, Name, Command, Arguments, CallerInfo) of
|
||||
ok -> execute_check_policy(Auth, TokenJID, Command, Arguments)
|
||||
end.
|
||||
@ -530,15 +530,22 @@ 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) ->
|
||||
execute_check_access(_FromJID, #ejabberd_commands{access = []} = Command, Arguments) ->
|
||||
do_execute_command(Command, Arguments);
|
||||
execute_check_access(FromJID, #ejabberd_commands{access_rules = Rules} = Command, Arguments) ->
|
||||
execute_check_access(FromJID, #ejabberd_commands{access = AccessRefs} = 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
|
||||
Host = global,
|
||||
Rules = lists:map(fun({Mod, AccessName, Default}) ->
|
||||
gen_mod:get_module_opt(Host, Mod,
|
||||
AccessName, fun(A) -> A end, Default);
|
||||
(Default) ->
|
||||
Default
|
||||
end, AccessRefs),
|
||||
case acl:any_rules_allowed(Host, Rules, FromJID) of
|
||||
true ->
|
||||
do_execute_command(Command, Arguments);
|
||||
false ->
|
||||
{error, access_rules_unauthorized}
|
||||
throw({error, access_rules_unauthorized})
|
||||
end.
|
||||
|
||||
do_execute_command(Command, Arguments) ->
|
||||
@ -724,6 +731,10 @@ tag_arguments(ArgsDefs, Args) ->
|
||||
Args).
|
||||
|
||||
|
||||
%% Get commands for all version
|
||||
get_all_access_commands(AccessCommands) ->
|
||||
get_access_commands(AccessCommands, ?DEFAULT_VERSION).
|
||||
|
||||
get_access_commands(undefined, Version) ->
|
||||
Cmds = get_commands(Version),
|
||||
[{?POLICY_ACCESS, Cmds, []}];
|
||||
@ -746,26 +757,28 @@ get_commands(Version) ->
|
||||
Cmds =
|
||||
lists:foldl(
|
||||
fun([{add_commands, L}], Acc) ->
|
||||
Cmds = case L of
|
||||
open -> OpenCmds;
|
||||
restricted -> RestrictedCmds;
|
||||
admin -> AdminCmds;
|
||||
user -> UserCmds;
|
||||
_ when is_list(L) -> L
|
||||
end,
|
||||
Cmds = expand_commands(L, OpenCmds, UserCmds, AdminCmds, RestrictedCmds),
|
||||
lists:usort(Cmds ++ Acc);
|
||||
([{remove_commands, L}], Acc) ->
|
||||
Cmds = case L of
|
||||
Cmds = expand_commands(L, OpenCmds, UserCmds, AdminCmds, RestrictedCmds),
|
||||
Acc -- Cmds;
|
||||
(_, Acc) -> Acc
|
||||
end, [], Opts),
|
||||
Cmds.
|
||||
|
||||
expand_commands(L, OpenCmds, UserCmds, AdminCmds, RestrictedCmds) when is_list(L) ->
|
||||
lists:foldl(fun(El, Acc) ->
|
||||
expand_commands(El, OpenCmds, UserCmds, AdminCmds, RestrictedCmds) ++ Acc
|
||||
end, [], L);
|
||||
expand_commands(El, OpenCmds, UserCmds, AdminCmds, RestrictedCmds) ->
|
||||
case El of
|
||||
open -> OpenCmds;
|
||||
restricted -> RestrictedCmds;
|
||||
admin -> AdminCmds;
|
||||
user -> UserCmds;
|
||||
_ when is_list(L) -> L
|
||||
end,
|
||||
Acc -- Cmds;
|
||||
(_, Acc) -> Acc
|
||||
end, AdminCmds ++ UserCmds, Opts),
|
||||
Cmds.
|
||||
_ -> [El]
|
||||
end.
|
||||
|
||||
|
||||
oauth_token_user(noauth) ->
|
||||
undefined;
|
||||
|
@ -136,8 +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()
|
||||
json_error(404, 40, <<"Endpoint not found.">>)
|
||||
end.
|
||||
|
||||
check_permissions2(#request{auth = HTTPAuth, headers = Headers}, Call, _, ScopeList)
|
||||
@ -318,6 +317,8 @@ handle(Call, Auth, Args, Version, IP) when is_atom(Call), is_list(Args) ->
|
||||
{401, iolist_to_binary(Msg)};
|
||||
throw:{error, account_unprivileged} ->
|
||||
{403, 31, <<"Command need to be run with admin priviledge.">>};
|
||||
throw:{error, access_rules_unauthorized} ->
|
||||
{403, 32, <<"AccessRules: Account associated to token does not have the right to perform the operation.">>};
|
||||
throw:{invalid_parameter, Msg} ->
|
||||
{400, iolist_to_binary(Msg)};
|
||||
throw:{error, Why} when is_atom(Why) ->
|
||||
|
@ -18,6 +18,8 @@
|
||||
#
|
||||
# ----------------------------------------------------------------------
|
||||
|
||||
## TODO Fix next test error: add admin user ACL
|
||||
|
||||
defmodule EjabberdCommandsMockTest do
|
||||
use ExUnit.Case, async: false
|
||||
|
||||
@ -44,6 +46,9 @@ defmodule EjabberdCommandsMockTest do
|
||||
_ -> :ok
|
||||
end
|
||||
:mnesia.start
|
||||
:ok = :jid.start
|
||||
:ok = :ejabberd_config.start(["domain1", "domain2"], [])
|
||||
:ok = :acl.start
|
||||
EjabberdOauthMock.init
|
||||
:ok
|
||||
end
|
||||
@ -313,7 +318,6 @@ defmodule EjabberdCommandsMockTest do
|
||||
end
|
||||
|
||||
|
||||
|
||||
test "API command with admin policy" do
|
||||
mock_commands_config
|
||||
|
||||
@ -393,6 +397,40 @@ defmodule EjabberdCommandsMockTest do
|
||||
assert :meck.validate @module
|
||||
end
|
||||
|
||||
test "Commands can perform extra check on access" do
|
||||
mock_commands_config
|
||||
|
||||
command_name = :test
|
||||
function = :test_command
|
||||
command = ejabberd_commands(name: command_name,
|
||||
args: [{:user, :binary}, {:host, :binary}],
|
||||
access: [:basic_rule_1],
|
||||
module: @module,
|
||||
function: function,
|
||||
policy: :open)
|
||||
:meck.expect(@module, function,
|
||||
fn(user, domain) when is_binary(user) and is_binary(domain) ->
|
||||
{user, domain}
|
||||
end)
|
||||
assert :ok == :ejabberd_commands.register_commands [command]
|
||||
|
||||
:acl.add(:global, :basic_acl_1, {:user, @user})
|
||||
:acl.add_access(:global, :basic_rule_1, [{:allow, [{:acl, :basic_acl_1}]}])
|
||||
|
||||
assert {@user, @domain} ==
|
||||
:ejabberd_commands.execute_command(:undefined,
|
||||
{@user, @domain,
|
||||
@userpass, false},
|
||||
command_name,
|
||||
[@user, @domain])
|
||||
assert {@user, @domain} ==
|
||||
:ejabberd_commands.execute_command(:undefined,
|
||||
{@admin, @domain,
|
||||
@adminpass, false},
|
||||
command_name,
|
||||
[@user, @domain])
|
||||
|
||||
end
|
||||
|
||||
##########################################################
|
||||
# Utils
|
||||
@ -412,7 +450,7 @@ defmodule EjabberdCommandsMockTest do
|
||||
end)
|
||||
:meck.expect(:ejabberd_config, :get_myhosts,
|
||||
fn() -> [@domain] end)
|
||||
:meck.new :acl
|
||||
:meck.new :acl #, [:passthrough]
|
||||
:meck.expect(:acl, :access_matches,
|
||||
fn(:commands_admin_access, info, _scope) ->
|
||||
case info do
|
||||
|
Loading…
Reference in New Issue
Block a user