diff --git a/src/cyrsasl_oauth.erl b/src/cyrsasl_oauth.erl index 16f1e3dfb..80ba315ed 100644 --- a/src/cyrsasl_oauth.erl +++ b/src/cyrsasl_oauth.erl @@ -51,7 +51,7 @@ mech_step(State, ClientIn) -> {ok, [{username, User}, {authzid, AuthzId}, {auth_module, ejabberd_oauth}]}; - false -> + _ -> {error, <<"not-authorized">>, User} end; _ -> {error, <<"bad-protocol">>} diff --git a/src/ejabberd_admin.erl b/src/ejabberd_admin.erl index f20aeebf0..615459f77 100644 --- a/src/ejabberd_admin.erl +++ b/src/ejabberd_admin.erl @@ -87,6 +87,7 @@ get_commands_spec() -> args = [], result = {res, rescode}}, #ejabberd_commands{name = reopen_log, tags = [logs, server], desc = "Reopen the log files", + policy = admin, module = ?MODULE, function = reopen_log, args = [], result = {res, rescode}}, #ejabberd_commands{name = rotate_log, tags = [logs, server], @@ -380,13 +381,12 @@ register(User, Host, Password) -> {atomic, ok} -> {ok, io_lib:format("User ~s@~s successfully registered", [User, Host])}; {atomic, exists} -> - String = io_lib:format("User ~s@~s already registered at node ~p", - [User, Host, node()]), - {exists, String}; + Msg = io_lib:format("User ~s@~s already registered", [User, Host]), + {error, conflict, 10090, Msg}; {error, Reason} -> String = io_lib:format("Can't register user ~s@~s at node ~p: ~p", [User, Host, node(), Reason]), - {cannot_register, String} + {error, cannot_register, 10001, String} end. unregister(User, Host) -> diff --git a/src/ejabberd_commands.erl b/src/ejabberd_commands.erl index 2c095440f..a8b3e25ab 100644 --- a/src/ejabberd_commands.erl +++ b/src/ejabberd_commands.erl @@ -425,7 +425,7 @@ get_command_definition(Name, Version) -> {V, C} end)))) of [{_, Command} | _ ] -> Command; - _E -> throw(unknown_command) + _E -> throw({error, unknown_command}) end. -spec get_commands_definition(integer()) -> [ejabberd_commands()]. @@ -682,7 +682,7 @@ check_auth(Command, {User, Server, {oauth, Token}, _}) -> case ejabberd_oauth:check_token(User, Server, ScopeList, Token) of true -> {ok, User, Server}; - false -> + _ -> throw({error, invalid_account_data}) end; check_auth(_Command, {User, Server, Password, _}) when is_binary(Password) -> diff --git a/src/ejabberd_ctl.erl b/src/ejabberd_ctl.erl index d52d1c0a9..d52b55cf9 100644 --- a/src/ejabberd_ctl.erl +++ b/src/ejabberd_ctl.erl @@ -212,7 +212,7 @@ process(["help" | Mode], Version) -> end; process(["--version", Arg | Args], _) -> - Version = + Version = try list_to_integer(Arg) catch _:_ -> @@ -321,7 +321,7 @@ call_command([CmdString | Args], Auth, AccessCommands, Version) -> {ArgsFormat, ResultFormat} -> case (catch format_args(Args, ArgsFormat)) of ArgsFormatted when is_list(ArgsFormatted) -> - Result = ejabberd_commands:execute_command(AccessCommands, + Result = ejabberd_commands:execute_command(AccessCommands, Auth, Command, ArgsFormatted, Version), @@ -374,6 +374,12 @@ format_arg2(Arg, Parse)-> format_result({error, ErrorAtom}, _) -> {io_lib:format("Error: ~p", [ErrorAtom]), make_status(error)}; +%% An error should always be allowed to return extended error to help with API. +%% Extended error is of the form: +%% {error, type :: atom(), code :: int(), Desc :: string()} +format_result({error, ErrorAtom, Code, _Msg}, _) -> + {io_lib:format("Error: ~p", [ErrorAtom]), make_status(Code)}; + format_result(Atom, {_Name, atom}) -> io_lib:format("~p", [Atom]); @@ -433,6 +439,8 @@ format_result(404, {_Name, _}) -> make_status(ok) -> ?STATUS_SUCCESS; make_status(true) -> ?STATUS_SUCCESS; +make_status(Code) when is_integer(Code), Code > 255 -> ?STATUS_ERROR; +make_status(Code) when is_integer(Code), Code > 0 -> Code; make_status(_Error) -> ?STATUS_ERROR. get_list_commands(Version) -> diff --git a/src/ejabberd_oauth.erl b/src/ejabberd_oauth.erl index c45a69d17..e4396260e 100644 --- a/src/ejabberd_oauth.erl +++ b/src/ejabberd_oauth.erl @@ -302,12 +302,17 @@ check_token(User, Server, ScopeList, Token) -> expire = Expire} -> {MegaSecs, Secs, _} = os:timestamp(), TS = 1000000 * MegaSecs + Secs, - TokenScopeSet = oauth2_priv_set:new(TokenScope), - lists:any(fun(Scope) -> - oauth2_priv_set:is_member(Scope, TokenScopeSet) end, - ScopeList) andalso Expire > TS; + if + Expire > TS -> + TokenScopeSet = oauth2_priv_set:new(TokenScope), + lists:any(fun(Scope) -> + oauth2_priv_set:is_member(Scope, TokenScopeSet) end, + ScopeList); + true -> + {false, expired} + end; _ -> - false + {false, not_found} end. check_token(ScopeList, Token) -> @@ -318,15 +323,20 @@ check_token(ScopeList, Token) -> expire = Expire} -> {MegaSecs, Secs, _} = os:timestamp(), TS = 1000000 * MegaSecs + Secs, - TokenScopeSet = oauth2_priv_set:new(TokenScope), - case lists:any(fun(Scope) -> - oauth2_priv_set:is_member(Scope, TokenScopeSet) end, - ScopeList) andalso Expire > TS of - true -> {ok, user, US}; - false -> false + if + Expire > TS -> + TokenScopeSet = oauth2_priv_set:new(TokenScope), + case lists:any(fun(Scope) -> + oauth2_priv_set:is_member(Scope, TokenScopeSet) end, + ScopeList) of + true -> {ok, user, US}; + false -> {false, no_matching_scope} + end; + true -> + {false, expired} end; _ -> - false + {false, not_found} end. diff --git a/src/ejabberd_s2s.erl b/src/ejabberd_s2s.erl index 19de64adb..2a17c75cb 100644 --- a/src/ejabberd_s2s.erl +++ b/src/ejabberd_s2s.erl @@ -473,28 +473,34 @@ send_element(Pid, El) -> %%% ejabberd commands get_commands_spec() -> - [#ejabberd_commands{name = incoming_s2s_number, - tags = [stats, s2s], - desc = - "Number of incoming s2s connections on " - "the node", - policy = admin, - module = ?MODULE, function = incoming_s2s_number, - args = [], result = {s2s_incoming, integer}}, - #ejabberd_commands{name = outgoing_s2s_number, - tags = [stats, s2s], - desc = - "Number of outgoing s2s connections on " - "the node", - policy = admin, - module = ?MODULE, function = outgoing_s2s_number, - args = [], result = {s2s_outgoing, integer}}]. + [#ejabberd_commands{ + name = incoming_s2s_number, + tags = [stats, s2s], + desc = "Number of incoming s2s connections on the node", + policy = admin, + module = ?MODULE, function = incoming_s2s_number, + args = [], result = {s2s_incoming, integer}}, + #ejabberd_commands{ + name = outgoing_s2s_number, + tags = [stats, s2s], + desc = "Number of outgoing s2s connections on the node", + policy = admin, + module = ?MODULE, function = outgoing_s2s_number, + args = [], result = {s2s_outgoing, integer}}]. +%% TODO Move those stats commands to ejabberd stats command ? incoming_s2s_number() -> - length(supervisor:which_children(ejabberd_s2s_in_sup)). + supervisor_count(ejabberd_s2s_in_sup). outgoing_s2s_number() -> - length(supervisor:which_children(ejabberd_s2s_out_sup)). + supervisor_count(ejabberd_s2s_out_sup). + +supervisor_count(Supervisor) -> + case catch supervisor:which_children(Supervisor) of + {'EXIT', _} -> 0; + Result -> + length(Result) + end. %%%---------------------------------------------------------------------- %%% Update Mnesia tables diff --git a/src/mod_admin_extra.erl b/src/mod_admin_extra.erl index 2ad1cc28e..c23fd7818 100644 --- a/src/mod_admin_extra.erl +++ b/src/mod_admin_extra.erl @@ -535,7 +535,7 @@ get_commands_spec() -> policy = user, module = mod_offline, function = count_offline_messages, args = [], - result = {res, integer}}, + result = {value, integer}}, #ejabberd_commands{name = send_message, tags = [stanza], desc = "Send a message to a local or remote bare of full JID", module = ?MODULE, function = send_message, diff --git a/src/mod_http_api.erl b/src/mod_http_api.erl index ba3a14cf8..73e6f7e4e 100644 --- a/src/mod_http_api.erl +++ b/src/mod_http_api.erl @@ -162,14 +162,15 @@ check_permissions2(#request{auth = HTTPAuth, headers = Headers}, Call, _, ScopeL case oauth_check_token(ScopeList, Token) of {ok, user, {User, Server}} -> {ok, {User, Server, {oauth, Token}, Admin}}; - false -> - false + {false, Reason} -> + {false, Reason} end; _ -> false end, case Auth of {ok, A} -> {allowed, Call, A}; + {false, no_matching_scope} -> outofscope_response(); _ -> unauthorized_response() end; check_permissions2(_Request, Call, open, _Scope) -> @@ -189,7 +190,7 @@ check_permissions2(#request{ip={IP, _Port}}, Call, _Policy, _Scope) -> Commands when is_list(Commands) -> case lists:member(Call, Commands) of true -> {allowed, Call, admin}; - _ -> unauthorized_response() + _ -> outofscope_response() end; _E -> {allowed, Call, noauth} @@ -212,28 +213,24 @@ process(_, #request{method = 'POST', data = <<>>}) -> process([Call], #request{method = 'POST', data = Data, ip = {IP, _} = IPPort} = Req) -> Version = get_api_version(Req), try - Args = case jiffy:decode(Data) of - List when is_list(List) -> List; - {List} when is_list(List) -> List; - Other -> [Other] - end, + Args = extract_args(Data), log(Call, Args, IPPort), case check_permissions(Req, Call) of {allowed, Cmd, Auth} -> - case handle(Cmd, Auth, Args, Version, IP) of - {Code, Result} -> - json_response(Code, jiffy:encode(Result)); - {HTMLCode, JSONErrorCode, Message} -> - json_error(HTMLCode, JSONErrorCode, Message) - end; + Result = handle(Cmd, Auth, Args, Version, IP), + json_format(Result); %% Warning: check_permission direcly formats 401 reply if not authorized ErrorResponse -> ErrorResponse end - catch _:{error,{_,invalid_json}} = _Err -> - ?DEBUG("Bad Request: ~p", [_Err]), - badrequest_response(<<"Invalid JSON input">>); - _:_Error -> + catch + %% TODO We need to refactor to remove redundant error return formatting + throw:{error, unknown_command} -> + {404, 40, <<"Command not found.">>}; + _:{error,{_,invalid_json}} = _Err -> + ?DEBUG("Bad Request: ~p", [_Err]), + badrequest_response(<<"Invalid JSON input">>); + _:_Error -> ?DEBUG("Bad Request: ~p ~p", [_Error, erlang:get_stacktrace()]), badrequest_response() end; @@ -247,13 +244,18 @@ process([Call], #request{method = 'GET', q = Data, ip = IP} = Req) -> log(Call, Args, IP), case check_permissions(Req, Call) of {allowed, Cmd, Auth} -> - {Code, Result} = handle(Cmd, Auth, Args, Version, IP), - json_response(Code, jiffy:encode(Result)); + Result = handle(Cmd, Auth, Args, Version, IP), + json_format(Result); %% Warning: check_permission direcly formats 401 reply if not authorized ErrorResponse -> ErrorResponse end - catch _:_Error -> + catch + %% TODO We need to refactor to remove redundant error return formatting + throw:{error, unknown_command} -> + json_format({404, 44, <<"Command not found.">>}); + _:_Error -> + ?DEBUG("Bad Request: ~p ~p", [_Error, erlang:get_stacktrace()]), badrequest_response() end; @@ -261,7 +263,16 @@ process([], #request{method = 'OPTIONS', data = <<>>}) -> {200, ?OPTIONS_HEADER, []}; process(_Path, Request) -> ?DEBUG("Bad Request: no handler ~p", [Request]), - badrequest_response(). + json_error(400, 40, <<"Missing command name.">>). + +%% Be tolerant to make API more easily usable from command-line pipe. +extract_args(<<"\n">>) -> []; +extract_args(Data) -> + case jiffy:decode(Data) of + List when is_list(List) -> List; + {List} when is_list(List) -> List; + Other -> [Other] + end. % get API version N from last "vN" element in URL path get_api_version(#request{path = Path}) -> @@ -302,7 +313,7 @@ handle(Call, Auth, Args, Version, IP) when is_atom(Call), is_list(Args) -> [{Key, undefined}|Acc] end, [], ArgsSpec), try - handle2(Call, Auth, match(Args2, Spec), Version, IP) + handle2(Call, Auth, match(Args2, Spec), Version, IP) catch throw:not_found -> {404, <<"not_found">>}; throw:{not_found, Why} when is_atom(Why) -> @@ -444,22 +455,24 @@ ejabberd_command(Auth, Cmd, Args, Version, IP) -> format_command_result(Cmd, Auth, Result, Version) -> {_, ResultFormat} = ejabberd_commands:get_command_format(Cmd, Auth, Version), case {ResultFormat, Result} of - {{_, rescode}, V} when V == true; V == ok -> - {200, 0}; - {{_, rescode}, _} -> - {200, 1}; - {{_, restuple}, {V1, Text1}} when V1 == true; V1 == ok -> - {200, iolist_to_binary(Text1)}; - {{_, restuple}, {_, Text2}} -> - {500, iolist_to_binary(Text2)}; - {{_, {list, _}}, _V} -> - {_, L} = format_result(Result, ResultFormat), - {200, L}; - {{_, {tuple, _}}, _V} -> - {_, T} = format_result(Result, ResultFormat), - {200, T}; - _ -> - {200, {[format_result(Result, ResultFormat)]}} + {{_, rescode}, V} when V == true; V == ok -> + {200, 0}; + {{_, rescode}, _} -> + {200, 1}; + {_, {error, ErrorAtom, Code, Msg}} -> + format_error_result(ErrorAtom, Code, Msg); + {{_, restuple}, {V, Text}} when V == true; V == ok -> + {200, iolist_to_binary(Text)}; + {{_, restuple}, {ErrorAtom, Msg}} -> + format_error_result(ErrorAtom, 0, Msg); + {{_, {list, _}}, _V} -> + {_, L} = format_result(Result, ResultFormat), + {200, L}; + {{_, {tuple, _}}, _V} -> + {_, T} = format_result(Result, ResultFormat), + {200, T}; + _ -> + {200, {[format_result(Result, ResultFormat)]}} end. format_result(Atom, {Name, atom}) -> @@ -497,14 +510,28 @@ format_result(Tuple, {Name, {tuple, Def}}) -> format_result(404, {_Name, _}) -> "not_found". + +format_error_result(conflict, Code, Msg) -> + {409, Code, iolist_to_binary(Msg)}; +format_error_result(_ErrorAtom, Code, Msg) -> + {500, Code, iolist_to_binary(Msg)}. + unauthorized_response() -> json_error(401, 10, <<"Oauth Token is invalid or expired.">>). +outofscope_response() -> + json_error(401, 11, <<"Token does not grant usage to command required scope.">>). + badrequest_response() -> badrequest_response(<<"400 Bad Request">>). badrequest_response(Body) -> json_response(400, jiffy:encode(Body)). +json_format({Code, Result}) -> + json_response(Code, jiffy:encode(Result)); +json_format({HTMLCode, JSONErrorCode, Message}) -> + json_error(HTMLCode, JSONErrorCode, Message). + json_response(Code, Body) when is_integer(Code) -> {Code, ?HEADER(?CT_JSON), Body}. diff --git a/test/ejabberd_commands_mock_test.exs b/test/ejabberd_commands_mock_test.exs index 439a3c1d3..9d33d7573 100644 --- a/test/ejabberd_commands_mock_test.exs +++ b/test/ejabberd_commands_mock_test.exs @@ -174,7 +174,7 @@ defmodule EjabberdCommandsMockTest do # default version is latest one assert :result3 == :ejabberd_commands.execute_command(command_name, []) # no such command in APIv0 - assert :unknown_command == + assert {:error, :unknown_command} == catch_throw :ejabberd_commands.execute_command(command_name, [], 0) assert :result1 == :ejabberd_commands.execute_command(command_name, [], 1) assert :result1 == :ejabberd_commands.execute_command(command_name, [], 2)