From fb2603d3cdef98542f479764567b15c9c7777411 Mon Sep 17 00:00:00 2001 From: Mickael Remond Date: Sat, 30 Jul 2016 11:50:04 +0200 Subject: Return 409 conflict error code on register if user already exists --- src/mod_http_api.erl | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) (limited to 'src/mod_http_api.erl') diff --git a/src/mod_http_api.erl b/src/mod_http_api.erl index ba3a14cf8..f56a47666 100644 --- a/src/mod_http_api.erl +++ b/src/mod_http_api.erl @@ -444,22 +444,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}; + {{_, restuple}, {V, Text}} when V == true; V == ok -> + {200, iolist_to_binary(Text)}; + {{_, restuple}, {V, Text}} when V == conflict -> + {409, iolist_to_binary(Text)}; + {{_, 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)]}} end. format_result(Atom, {Name, atom}) -> -- cgit v1.2.3 From 39640b67c7bc6c46312879beccc54fa5de4c4d95 Mon Sep 17 00:00:00 2001 From: Mickael Remond Date: Sat, 30 Jul 2016 13:08:30 +0200 Subject: Add support for rich error reporting for API --- src/mod_http_api.erl | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) (limited to 'src/mod_http_api.erl') diff --git a/src/mod_http_api.erl b/src/mod_http_api.erl index f56a47666..6f6d59cda 100644 --- a/src/mod_http_api.erl +++ b/src/mod_http_api.erl @@ -220,12 +220,8 @@ process([Call], #request{method = 'POST', data = Data, ip = {IP, _} = IPPort} = 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 @@ -247,8 +243,8 @@ 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 @@ -302,7 +298,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) -> @@ -448,12 +444,12 @@ format_command_result(Cmd, Auth, Result, Version) -> {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}, {V, Text}} when V == conflict -> - {409, iolist_to_binary(Text)}; - {{_, restuple}, {_, Text2}} -> - {500, iolist_to_binary(Text2)}; + {{_, restuple}, {ErrorAtom, Msg}} -> + format_error_result(ErrorAtom, 0, Msg); {{_, {list, _}}, _V} -> {_, L} = format_result(Result, ResultFormat), {200, L}; @@ -499,6 +495,11 @@ 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.">>). @@ -507,6 +508,11 @@ badrequest_response() -> 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}. -- cgit v1.2.3 From 674a8039ef0da080c9882bbe8ea3a476d78df0f5 Mon Sep 17 00:00:00 2001 From: Mickael Remond Date: Sat, 30 Jul 2016 18:51:54 +0200 Subject: Add support for sending back missing scope error to API ReST command calls --- src/mod_http_api.erl | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'src/mod_http_api.erl') diff --git a/src/mod_http_api.erl b/src/mod_http_api.erl index 6f6d59cda..cda4d6059 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} @@ -495,6 +496,7 @@ 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) -> @@ -503,6 +505,9 @@ format_error_result(_ErrorAtom, Code, 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) -> -- cgit v1.2.3 From 6ea7153e31a8d36fbb34816b4c87ea3eca1ac8fc Mon Sep 17 00:00:00 2001 From: Mickael Remond Date: Sun, 31 Jul 2016 22:48:24 +0200 Subject: Improve error handling --- src/mod_http_api.erl | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) (limited to 'src/mod_http_api.erl') diff --git a/src/mod_http_api.erl b/src/mod_http_api.erl index cda4d6059..d33fb7a7f 100644 --- a/src/mod_http_api.erl +++ b/src/mod_http_api.erl @@ -213,11 +213,7 @@ 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} -> @@ -227,10 +223,14 @@ process([Call], #request{method = 'POST', data = Data, ip = {IP, _} = IPPort} = 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; @@ -250,7 +250,12 @@ process([Call], #request{method = 'GET', q = Data, ip = IP} = Req) -> ErrorResponse -> ErrorResponse end - catch _:_Error -> + catch + %% TODO We need to refactor to remove redundant error return formatting + throw:{error, unknown_command} -> + {404, 40, <<"Command not found.">>}; + _:_Error -> + ?DEBUG("Bad Request: ~p ~p", [_Error, erlang:get_stacktrace()]), badrequest_response() end; @@ -260,6 +265,15 @@ process(_Path, Request) -> ?DEBUG("Bad Request: no handler ~p", [Request]), badrequest_response(). +%% 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}) -> get_api_version(lists:reverse(Path)); -- cgit v1.2.3 From c5c394e929d508a0b04efcae6abc696c142d0802 Mon Sep 17 00:00:00 2001 From: Mickael Remond Date: Mon, 1 Aug 2016 08:58:49 +0200 Subject: Fix HTTP process return formatting --- src/mod_http_api.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/mod_http_api.erl') diff --git a/src/mod_http_api.erl b/src/mod_http_api.erl index d33fb7a7f..a91c3c1a7 100644 --- a/src/mod_http_api.erl +++ b/src/mod_http_api.erl @@ -253,7 +253,7 @@ process([Call], #request{method = 'GET', q = Data, ip = IP} = Req) -> catch %% TODO We need to refactor to remove redundant error return formatting throw:{error, unknown_command} -> - {404, 40, <<"Command not found.">>}; + json_format({404, 40, <<"Command not found.">>}); _:_Error -> ?DEBUG("Bad Request: ~p ~p", [_Error, erlang:get_stacktrace()]), -- cgit v1.2.3 From 90ea3ca3613d7e342ba64e45cefcbe1227ee88c7 Mon Sep 17 00:00:00 2001 From: Mickael Remond Date: Mon, 1 Aug 2016 15:29:47 +0200 Subject: Improve error message when try to call api on api root --- src/mod_http_api.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/mod_http_api.erl') diff --git a/src/mod_http_api.erl b/src/mod_http_api.erl index a91c3c1a7..73e6f7e4e 100644 --- a/src/mod_http_api.erl +++ b/src/mod_http_api.erl @@ -253,7 +253,7 @@ process([Call], #request{method = 'GET', q = Data, ip = IP} = Req) -> catch %% TODO We need to refactor to remove redundant error return formatting throw:{error, unknown_command} -> - json_format({404, 40, <<"Command not found.">>}); + json_format({404, 44, <<"Command not found.">>}); _:_Error -> ?DEBUG("Bad Request: ~p ~p", [_Error, erlang:get_stacktrace()]), @@ -263,7 +263,7 @@ 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">>) -> []; -- cgit v1.2.3