From 324040215463eda90cff96460f25bd5ec434aa63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Wed, 15 Dec 2021 08:48:53 +0100 Subject: [PATCH 01/11] Add better error messages formatting --- src/mongoose_graphql.erl | 4 +- .../mongoose_graphql_cowboy_handler.erl | 22 +++++----- .../mongoose_graphql_errors.erl | 40 ++++++++++++++++++- .../mongoose_graphql_permissions.erl | 7 +++- test/mongoose_graphql_SUITE.erl | 4 +- 5 files changed, 58 insertions(+), 19 deletions(-) diff --git a/src/mongoose_graphql.erl b/src/mongoose_graphql.erl index e050e9f10e5..b9f04b01c3a 100644 --- a/src/mongoose_graphql.erl +++ b/src/mongoose_graphql.erl @@ -104,8 +104,8 @@ graphql_parse(Doc) -> case graphql:parse(Doc) of {ok, _} = Ok -> Ok; - {error, _} = Err -> - throw(Err) + {error, Err} -> + graphql_err:abort([], parse, Err) end. admin_mapping_rules() -> diff --git a/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl b/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl index 3a3aca78fa0..c804c4ad9da 100644 --- a/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl +++ b/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl @@ -58,9 +58,15 @@ charsets_provided(Req, State) -> {[<<"utf-8">>], Req, State}. is_authorized(Req, State) -> - Auth = cowboy_req:parse_header(<<"authorization">>, Req), - State2 = check_auth(Auth, State), - {true, Req, State2}. + try cowboy_req:parse_header(<<"authorization">>, Req) of + Auth -> + State2 = check_auth(Auth, State), + {true, Req, State2} + catch + exit:Err -> + Msg = #{phase => authorize, error_term => Err}, + reply_error(400, Msg, Req, State) + end. resource_exists(#{method := <<"GET">>} = Req, State) -> {true, Req, State}; @@ -179,15 +185,7 @@ operation_name([]) -> undefined. reply_error(Code, Msg, Req, State) -> - Errors = - case Msg of - {error, E} -> - graphql_err:format_errors(#{}, [E]); - _ -> - Formatted = iolist_to_binary(io_lib:format("~p", [Msg])), - [#{type => error, message => Formatted}] - end, - + Errors = mongoose_graphql_errors:format_error(Msg), Body = jiffy:encode(#{errors => Errors}), Req2 = cowboy_req:set_resp_body(Body, Req), Reply = cowboy_req:reply(Code, Req2), diff --git a/src/mongoose_graphql/mongoose_graphql_errors.erl b/src/mongoose_graphql/mongoose_graphql_errors.erl index f47ed3b2eeb..1913abfacd3 100644 --- a/src/mongoose_graphql/mongoose_graphql_errors.erl +++ b/src/mongoose_graphql/mongoose_graphql_errors.erl @@ -1,9 +1,9 @@ %% @doc Implements callbacks that format custom errors returned from resolvers or crashes. -module(mongoose_graphql_errors). --export([err/2, crash/2]). +-export([format_error/1, format_errors/1, err/2, crash/2]). --ignore_xref([err/2, crash/2]). +-ignore_xref([format_error/1, format_errors/1, err/2, crash/2]). % callback invoked when resolver returns error tuple err(_Ctx, domain_not_found) -> @@ -16,3 +16,39 @@ err(_Ctx, ErrorTerm) -> crash(_Ctx, #{type := Type}) -> #{message => <<"Unexpected ", Type/binary, " resolver crash">>, extensions => #{code => resolver_crash}}. + +format_errors(Errors) -> + [format_error(E) || E <- Errors]. + +format_error(#{phase := Phase, error_term := Term}) when Phase =:= authorize; + Phase =:= parse -> + #{extensions => #{code => err_code(Phase, Term)}, + message => iolist_to_binary(err_msg(Phase, Term))}; +format_error(#{error_term := _} = Err) -> + graphql:format_errors(#{}, [Err]); +format_error(Err) -> + #{extensions => #{code => uncathegorized_error}, + message => iolist_to_binary(io_lib:format("~p", Err))}. + +%% Internal + +err_code(authorize, _Term) -> + authorization_error; +err_code(parse, Term) -> + element(1, Term). + +err_msg(parse, Result) -> + parse_err_msg(Result); +err_msg(authorize, Result) -> + authorize_err_msg(Result). + +authorize_err_msg({request_error, {header, <<"authorization">>}, _}) -> + "Malformed authorization header.Please consult the relevant specification."; +authorize_err_msg({no_permissions, Op}) -> + io_lib:format("Cannot execute query ~s without permissions", [Op]). + +parse_err_msg({parser_error, {Line, graphql_parser, Msg}}) -> + io_lib:format("Cannot parse line ~B because of ~s", [Line, Msg]); +parse_err_msg({scanner_error, {Line, graphql_scanner, Msg}}) -> + Formatted = lists:flatten(graphql_scanner:format_error(Msg)), + io_lib:format("Cannot scan line ~B because of ~s", [Line, Formatted]). diff --git a/src/mongoose_graphql/mongoose_graphql_permissions.erl b/src/mongoose_graphql/mongoose_graphql_permissions.erl index 117a287205b..9b14f32b376 100644 --- a/src/mongoose_graphql/mongoose_graphql_permissions.erl +++ b/src/mongoose_graphql/mongoose_graphql_permissions.erl @@ -50,7 +50,7 @@ check_permissions(OpName, false, #document{definitions = Definitions}) -> true -> ok; false -> - throw({error, no_permissions}) + graphql_err:abort([], authorize, {no_permissions, op_name(OpName)}) end; false -> ok @@ -63,6 +63,11 @@ check_permissions(_, true, _) -> % Internal +op_name(undefined) -> + <<"ROOT">>; +op_name(Name) -> + Name. + is_req_operation(#op{id = 'ROOT'}, undefined) -> true; is_req_operation(#op{id = {name, _, Name}}, Name) -> diff --git a/test/mongoose_graphql_SUITE.erl b/test/mongoose_graphql_SUITE.erl index 9d8d07a3c49..ce0e7a88ac4 100644 --- a/test/mongoose_graphql_SUITE.erl +++ b/test/mongoose_graphql_SUITE.erl @@ -150,13 +150,13 @@ unauth_cannot_execute_protected_query(Config) -> Ep = ?config(endpoint, Config), Doc = <<"{ field }">>, Res = mongoose_graphql:execute(Ep, request(Doc, false)), - ?assertEqual({error, no_permissions}, Res). + ?assertMatch({error, #{error_term := {no_permissions, <<"ROOT">>}}}, Res). unauth_cannot_execute_protected_mutation(Config) -> Ep = ?config(endpoint, Config), Doc = <<"mutation { field }">>, Res = mongoose_graphql:execute(Ep, request(Doc, false)), - ?assertEqual({error, no_permissions}, Res). + ?assertMatch({error, #{error_term := {no_permissions, <<"ROOT">>}}}, Res). unauth_can_access_introspection(Config) -> Ep = ?config(endpoint, Config), From e1a3e514033432b1e45217e223bb8bed79bba84a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Wed, 15 Dec 2021 14:38:53 +0100 Subject: [PATCH 02/11] Make graphql_cowboy_handler use correct error format, Format error messages --- big_tests/tests/graphql_SUITE.erl | 12 ++++-- .../mongoose_graphql_cowboy_handler.erl | 18 +++++---- .../mongoose_graphql_errors.erl | 37 ++++++++++++++----- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/big_tests/tests/graphql_SUITE.erl b/big_tests/tests/graphql_SUITE.erl index 707946a7fa2..ed8ecb03570 100644 --- a/big_tests/tests/graphql_SUITE.erl +++ b/big_tests/tests/graphql_SUITE.erl @@ -148,27 +148,31 @@ invalid_json_body_error(Config) -> Body = <<"">>, {Status, Data} = execute(Ep, Body, undefined), ?assertEqual({<<"400">>,<<"Bad Request">>}, Status), - ?assertMatch(#{<<"errors">> := [#{<<"message">> := <<"invalid_json_body">>}]}, Data). + assert_code(invalid_json_body, Data). no_query_supplied_error(Config) -> Ep = ?config(schema_endpoint, Config), Body = #{}, {Status, Data} = execute(Ep, Body, undefined), ?assertEqual({<<"400">>,<<"Bad Request">>}, Status), - ?assertMatch(#{<<"errors">> := [#{<<"message">> := <<"no_query_supplied">>}]}, Data). + assert_code(no_query_supplied, Data). variables_invalid_json_error(Config) -> Ep = ?config(schema_endpoint, Config), Body = #{<<"query">> => <<"{ field }">>, <<"variables">> => <<"{1: 2}">>}, {Status, Data} = execute(Ep, Body, undefined), ?assertEqual({<<"400">>,<<"Bad Request">>}, Status), - ?assertMatch(#{<<"errors">> := [#{<<"message">> := <<"variables_invalid_json">>}]}, Data). + assert_code(variables_invalid_json, Data). %% Helpers +assert_code(Code, Data) -> + BinCode = atom_to_binary(Code), + ?assertMatch(#{<<"errors">> := [#{<<"extensions">> := #{<<"code">> := BinCode}}]}, Data). + assert_no_permissions(Status, Data) -> ?assertEqual({<<"400">>,<<"Bad Request">>}, Status), - ?assertMatch(#{<<"errors">> := [#{<<"message">> := <<"no_permissions">>}]}, Data). + ?assertMatch(#{<<"errors">> := [#{<<"extensions">> := #{<<"code">> := <<"no_permissions">>}}]}, Data). assert_access_granted(Status, Data) -> ?assertEqual({<<"200">>,<<"OK">>}, Status), diff --git a/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl b/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl index c804c4ad9da..7d05e2a2251 100644 --- a/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl +++ b/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl @@ -64,8 +64,7 @@ is_authorized(Req, State) -> {true, Req, State2} catch exit:Err -> - Msg = #{phase => authorize, error_term => Err}, - reply_error(400, Msg, Req, State) + reply_error(400, make_error(authorize, Err), Req, State) end. resource_exists(#{method := <<"GET">>} = Req, State) -> @@ -113,7 +112,7 @@ auth_admin(_, State) -> State#{authorized => true}. run_request(#{document := undefined}, Req, State) -> - reply_error(400, no_query_supplied, Req, State); + reply_error(400, make_error(decode, no_query_supplied), Req, State); run_request(#{} = ReqCtx, Req, #{schema_endpoint := EpName, authorized := AuthStatus} = State) -> Ep = mongoose_graphql:get_endpoint(binary_to_existing_atom(EpName)), @@ -137,7 +136,7 @@ gather(Req) -> gather(Req2, JSON, Bindings) catch _:_ -> - {error, invalid_json_body} + {error, make_error(decode, invalid_json_body)} end. gather(Req, Body, Params) -> @@ -162,10 +161,10 @@ variables([#{<<"variables">> := Vars} | _]) -> try jiffy:decode(Vars, [return_maps]) of null -> {ok, #{}}; JSON when is_map(JSON) -> {ok, JSON}; - _ -> {error, variables_invalid_json} + _ -> {error, make_error(decode, variables_invalid_json)} catch _:_ -> - {error, variables_invalid_json} + {error, make_error(decode, variables_invalid_json)} end; is_map(Vars) -> {ok, Vars}; @@ -184,9 +183,12 @@ operation_name([_ | Next]) -> operation_name([]) -> undefined. +make_error(Phase, Term) -> + #{error_term => Term, phase => Phase}. + reply_error(Code, Msg, Req, State) -> - Errors = mongoose_graphql_errors:format_error(Msg), - Body = jiffy:encode(#{errors => Errors}), + Error = mongoose_graphql_errors:format_error(Msg), + Body = jiffy:encode(#{errors => [Error]}), Req2 = cowboy_req:set_resp_body(Body, Req), Reply = cowboy_req:reply(Code, Req2), {stop, Reply, State}. diff --git a/src/mongoose_graphql/mongoose_graphql_errors.erl b/src/mongoose_graphql/mongoose_graphql_errors.erl index 1913abfacd3..560aff8cbf2 100644 --- a/src/mongoose_graphql/mongoose_graphql_errors.erl +++ b/src/mongoose_graphql/mongoose_graphql_errors.erl @@ -5,45 +5,55 @@ -ignore_xref([format_error/1, format_errors/1, err/2, crash/2]). -% callback invoked when resolver returns error tuple +%% callback invoked when resolver returns error tuple err(_Ctx, domain_not_found) -> #{message => <<"Given domain does not exist">>, extensions => #{code => resolver_error}}; err(_Ctx, ErrorTerm) -> #{message => iolist_to_binary(io_lib:format("~p", [ErrorTerm])), extensions => #{code => resolver_error}}. -% callback invoked when resolver crashes +%% callback invoked when resolver crashes crash(_Ctx, #{type := Type}) -> #{message => <<"Unexpected ", Type/binary, " resolver crash">>, extensions => #{code => resolver_crash}}. +%% @doc Format errors that occured in any phase. format_errors(Errors) -> [format_error(E) || E <- Errors]. +%% @doc Format error that occurred in any phase including HTTP request decoding. format_error(#{phase := Phase, error_term := Term}) when Phase =:= authorize; + Phase =:= decode; Phase =:= parse -> #{extensions => #{code => err_code(Phase, Term)}, message => iolist_to_binary(err_msg(Phase, Term))}; -format_error(#{error_term := _} = Err) -> +format_error(#{error_term := _, phase := Phase} = Err) when Phase =:= execute; + Phase =:= type_check; + Phase =:= validate; + Phase =:= uncategorized -> graphql:format_errors(#{}, [Err]); format_error(Err) -> - #{extensions => #{code => uncathegorized_error}, - message => iolist_to_binary(io_lib:format("~p", Err))}. + #{extensions => #{code => uncathegorized}, + message => iolist_to_binary(io_lib:format("~p", [Err]))}. %% Internal -err_code(authorize, _Term) -> - authorization_error; -err_code(parse, Term) -> - element(1, Term). +err_code(_, Term) -> + simplify(Term). + +simplify(A) when is_atom(A) -> A; +simplify(B) when is_binary(B) -> B; +simplify(T) when is_tuple(T) -> element(1, T). err_msg(parse, Result) -> parse_err_msg(Result); +err_msg(decode, Result) -> + decode_err_mgs(Result); err_msg(authorize, Result) -> authorize_err_msg(Result). authorize_err_msg({request_error, {header, <<"authorization">>}, _}) -> - "Malformed authorization header.Please consult the relevant specification."; + "Malformed authorization header. Please consult the relevant specification"; authorize_err_msg({no_permissions, Op}) -> io_lib:format("Cannot execute query ~s without permissions", [Op]). @@ -52,3 +62,10 @@ parse_err_msg({parser_error, {Line, graphql_parser, Msg}}) -> parse_err_msg({scanner_error, {Line, graphql_scanner, Msg}}) -> Formatted = lists:flatten(graphql_scanner:format_error(Msg)), io_lib:format("Cannot scan line ~B because of ~s", [Line, Formatted]). + +decode_err_mgs(no_query_supplied) -> + "The query was not supplied in request body"; +decode_err_mgs(invalid_json_body) -> + "The request json body is invalid"; +decode_err_mgs(variables_invalid_json) -> + "The variables' json is invalid". From a7841f861481768ed22ecf7d7c9d7e9fbe41c06f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Wed, 15 Dec 2021 15:42:15 +0100 Subject: [PATCH 03/11] Add wrong credentials error --- .../mongoose_graphql_cowboy_handler.erl | 23 ++++++++++++------- .../mongoose_graphql_errors.erl | 2 ++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl b/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl index 7d05e2a2251..b883ce1c4a9 100644 --- a/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl +++ b/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl @@ -60,8 +60,13 @@ charsets_provided(Req, State) -> is_authorized(Req, State) -> try cowboy_req:parse_header(<<"authorization">>, Req) of Auth -> - State2 = check_auth(Auth, State), - {true, Req, State2} + case check_auth(Auth, State) of + {ok, State2} -> + {true, Req, State2}; + error -> + Msg = make_error(authorize, wrong_credentials), + reply_error(401, Msg, Req, State) + end catch exit:Err -> reply_error(400, make_error(authorize, Err), Req, State) @@ -97,19 +102,21 @@ check_auth(Auth, #{schema_endpoint := <<"user">>} = State) -> auth_user({basic, User, Password}, State) -> case mongoose_api_common:check_password(jid:from_binary(User), Password) of - {true, _} -> State#{authorized => true, schema_ctx => #{username => User}}; - _ -> State#{authorized => false} + {true, _} -> {ok, State#{authorized => true, schema_ctx => #{username => User}}}; + _ -> error end; auth_user(_, State) -> - State#{authorized => false}. + {ok, State#{authorized => false}}. auth_admin({basic, Username, Password}, #{username := Username, password := Password} = State) -> - State#{authorized => true}; + {ok, State#{authorized => true}}; +auth_admin({basic, _, _}, _) -> + error; auth_admin(_, #{username := _, password := _} = State) -> - State#{authorized => false}; + {ok, State#{authorized => false}}; auth_admin(_, State) -> % auth credentials not provided in config - State#{authorized => true}. + {ok, State#{authorized => true}}. run_request(#{document := undefined}, Req, State) -> reply_error(400, make_error(decode, no_query_supplied), Req, State); diff --git a/src/mongoose_graphql/mongoose_graphql_errors.erl b/src/mongoose_graphql/mongoose_graphql_errors.erl index 560aff8cbf2..c331143be45 100644 --- a/src/mongoose_graphql/mongoose_graphql_errors.erl +++ b/src/mongoose_graphql/mongoose_graphql_errors.erl @@ -54,6 +54,8 @@ err_msg(authorize, Result) -> authorize_err_msg({request_error, {header, <<"authorization">>}, _}) -> "Malformed authorization header. Please consult the relevant specification"; +authorize_err_msg(wrong_credentials) -> + "The provided credentials are wrong"; authorize_err_msg({no_permissions, Op}) -> io_lib:format("Cannot execute query ~s without permissions", [Op]). From 71e895dd2b915889e9206ba82a1b06d04aa28d9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Thu, 16 Dec 2021 12:37:30 +0100 Subject: [PATCH 04/11] Move http code to format_error function --- .../mongoose_graphql_cowboy_handler.erl | 14 +++++------ .../mongoose_graphql_errors.erl | 25 +++++++++++-------- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl b/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl index b883ce1c4a9..280866249e0 100644 --- a/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl +++ b/src/mongoose_graphql/mongoose_graphql_cowboy_handler.erl @@ -65,11 +65,11 @@ is_authorized(Req, State) -> {true, Req, State2}; error -> Msg = make_error(authorize, wrong_credentials), - reply_error(401, Msg, Req, State) + reply_error(Msg, Req, State) end catch exit:Err -> - reply_error(400, make_error(authorize, Err), Req, State) + reply_error(make_error(authorize, Err), Req, State) end. resource_exists(#{method := <<"GET">>} = Req, State) -> @@ -85,7 +85,7 @@ to_html(Req, #{index_location := {priv_file, App, FileLocation}} = State) -> json_request(Req, State) -> case gather(Req) of {error, Reason} -> - reply_error(400, Reason, Req, State); + reply_error(Reason, Req, State); {ok, Req2, Decoded} -> run_request(Decoded, Req2, State) end. @@ -119,7 +119,7 @@ auth_admin(_, State) -> {ok, State#{authorized => true}}. run_request(#{document := undefined}, Req, State) -> - reply_error(400, make_error(decode, no_query_supplied), Req, State); + reply_error(make_error(decode, no_query_supplied), Req, State); run_request(#{} = ReqCtx, Req, #{schema_endpoint := EpName, authorized := AuthStatus} = State) -> Ep = mongoose_graphql:get_endpoint(binary_to_existing_atom(EpName)), @@ -132,7 +132,7 @@ run_request(#{} = ReqCtx, Req, #{schema_endpoint := EpName, Reply = cowboy_req:reply(200, Req2), {stop, Reply, State}; {error, Reason} -> - reply_error(400, Reason, Req, State) + reply_error(Reason, Req, State) end. gather(Req) -> @@ -193,8 +193,8 @@ operation_name([]) -> make_error(Phase, Term) -> #{error_term => Term, phase => Phase}. -reply_error(Code, Msg, Req, State) -> - Error = mongoose_graphql_errors:format_error(Msg), +reply_error(Msg, Req, State) -> + {Code, Error} = mongoose_graphql_errors:format_error(Msg), Body = jiffy:encode(#{errors => [Error]}), Req2 = cowboy_req:set_resp_body(Body, Req), Reply = cowboy_req:reply(Code, Req2), diff --git a/src/mongoose_graphql/mongoose_graphql_errors.erl b/src/mongoose_graphql/mongoose_graphql_errors.erl index c331143be45..3f6f0c050b1 100644 --- a/src/mongoose_graphql/mongoose_graphql_errors.erl +++ b/src/mongoose_graphql/mongoose_graphql_errors.erl @@ -1,9 +1,9 @@ %% @doc Implements callbacks that format custom errors returned from resolvers or crashes. -module(mongoose_graphql_errors). --export([format_error/1, format_errors/1, err/2, crash/2]). +-export([format_error/1, err/2, crash/2]). --ignore_xref([format_error/1, format_errors/1, err/2, crash/2]). +-ignore_xref([format_error/1, err/2, crash/2]). %% callback invoked when resolver returns error tuple err(_Ctx, domain_not_found) -> @@ -17,27 +17,30 @@ crash(_Ctx, #{type := Type}) -> #{message => <<"Unexpected ", Type/binary, " resolver crash">>, extensions => #{code => resolver_crash}}. -%% @doc Format errors that occured in any phase. -format_errors(Errors) -> - [format_error(E) || E <- Errors]. - %% @doc Format error that occurred in any phase including HTTP request decoding. format_error(#{phase := Phase, error_term := Term}) when Phase =:= authorize; Phase =:= decode; Phase =:= parse -> - #{extensions => #{code => err_code(Phase, Term)}, - message => iolist_to_binary(err_msg(Phase, Term))}; + Msg = #{extensions => #{code => err_code(Phase, Term)}, + message => iolist_to_binary(err_msg(Phase, Term))}, + {err_http_code(Phase), Msg}; format_error(#{error_term := _, phase := Phase} = Err) when Phase =:= execute; Phase =:= type_check; Phase =:= validate; Phase =:= uncategorized -> - graphql:format_errors(#{}, [Err]); + {400, graphql:format_errors(#{}, [Err])}; format_error(Err) -> - #{extensions => #{code => uncathegorized}, - message => iolist_to_binary(io_lib:format("~p", [Err]))}. + Msg = #{extensions => #{code => uncathegorized}, + message => iolist_to_binary(io_lib:format("~p", [Err]))}, + {400, Msg}. %% Internal +err_http_code(authorize) -> + 401; +err_http_code(_) -> + 400. + err_code(_, Term) -> simplify(Term). From d4896bd03d7a3d30fbc8c5819de686c78d8a21ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Thu, 16 Dec 2021 12:38:58 +0100 Subject: [PATCH 05/11] Adapt tests to error messages after formatting --- big_tests/tests/graphql_SUITE.erl | 25 +++++++++++++++++++------ big_tests/tests/mongooseimctl_SUITE.erl | 2 +- test/mongoose_graphql_SUITE.erl | 2 +- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/big_tests/tests/graphql_SUITE.erl b/big_tests/tests/graphql_SUITE.erl index ed8ecb03570..cfb68c61512 100644 --- a/big_tests/tests/graphql_SUITE.erl +++ b/big_tests/tests/graphql_SUITE.erl @@ -30,7 +30,8 @@ admin_handler() -> [auth_admin_can_access_protected_types | common_tests()]. common_tests() -> - [wrong_creds_cannot_access_protected_types, + [malformed_auth_header_error, + wrong_creds_cannot_access_protected_types, unauth_cannot_access_protected_types, unauth_can_access_unprotected_types, can_execute_query_with_variables, @@ -86,7 +87,7 @@ unauth_cannot_access_protected_types(Config) -> Ep = ?config(schema_endpoint, Config), Body = #{query => "{ field }"}, {Status, Data} = execute(Ep, Body, undefined), - assert_no_permissions(Status, Data). + assert_no_permissions(no_permissions, Status, Data). unauth_can_access_unprotected_types(Config) -> Ep = ?config(schema_endpoint, Config), @@ -98,7 +99,19 @@ wrong_creds_cannot_access_protected_types(Config) -> Ep = ?config(schema_endpoint, Config), Body = #{query => "{ field }"}, {Status, Data} = execute(Ep, Body, {<<"user">>, <<"wrong_password">>}), - assert_no_permissions(Status, Data). + assert_no_permissions(wrong_credentials, Status, Data). + +malformed_auth_header_error(Config) -> + EpName = ?config(schema_endpoint, Config), + Request = + #{port => get_port(EpName), + role => {graphql, atom_to_binary(EpName)}, + method => <<"POST">>, + headers => [{<<"Authorization">>, <<"Basic YWRtaW46c2VjcmV">>}], + return_maps => true, + path => "/graphql"}, + {Status, Data} = rest_helper:make_request(Request), + assert_no_permissions(request_error, Status, Data). auth_user_can_access_protected_types(Config) -> escalus:fresh_story( @@ -170,9 +183,9 @@ assert_code(Code, Data) -> BinCode = atom_to_binary(Code), ?assertMatch(#{<<"errors">> := [#{<<"extensions">> := #{<<"code">> := BinCode}}]}, Data). -assert_no_permissions(Status, Data) -> - ?assertEqual({<<"400">>,<<"Bad Request">>}, Status), - ?assertMatch(#{<<"errors">> := [#{<<"extensions">> := #{<<"code">> := <<"no_permissions">>}}]}, Data). +assert_no_permissions(ExpectedCode, Status, Data) -> + ?assertEqual({<<"401">>,<<"Unauthorized">>}, Status), + assert_code(ExpectedCode, Data). assert_access_granted(Status, Data) -> ?assertEqual({<<"200">>,<<"OK">>}, Status), diff --git a/big_tests/tests/mongooseimctl_SUITE.erl b/big_tests/tests/mongooseimctl_SUITE.erl index eb0eedbc086..99870dd8e62 100644 --- a/big_tests/tests/mongooseimctl_SUITE.erl +++ b/big_tests/tests/mongooseimctl_SUITE.erl @@ -1133,7 +1133,7 @@ can_handle_execution_error(Config) -> Res = mongooseimctl("graphql", [Query], Config), ?assertMatch({_, 0}, Res), Data = element(1, Res), - ?assertNotEqual(nomatch, string:find(Data, "{error,{parser_error")). + ?assertNotEqual(nomatch, string:find(Data, "parser_error")). graphql_wrong_arguments_number(Config) -> ExpectedFragment = "This command requires", diff --git a/test/mongoose_graphql_SUITE.erl b/test/mongoose_graphql_SUITE.erl index ce0e7a88ac4..b2ed767313e 100644 --- a/test/mongoose_graphql_SUITE.erl +++ b/test/mongoose_graphql_SUITE.erl @@ -7,7 +7,7 @@ -include_lib("graphql/src/graphql_schema.hrl"). -define(assertPermissionsFailed(Config, Doc), - ?assertThrow({error, no_permissions}, + ?assertThrow({error, #{error_term := {no_permissions, _}}}, check_permissions(Config, Doc))). -define(assertPermissionsSuccess(Config, Doc), ?assertMatch(ok, check_permissions(Config, Doc))). From 2d6cb98e24a62c2680e3161b2a5e46ed57a6c758 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Thu, 16 Dec 2021 16:50:34 +0100 Subject: [PATCH 06/11] Catch any unexpected graphql internal crash --- src/mongoose_graphql.erl | 8 +++++++- src/mongoose_graphql/mongoose_graphql_errors.erl | 12 +++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/mongoose_graphql.erl b/src/mongoose_graphql.erl index b9f04b01c3a..c0832736bbb 100644 --- a/src/mongoose_graphql.erl +++ b/src/mongoose_graphql.erl @@ -72,7 +72,13 @@ execute(Ep, #{document := Doc, {ok, graphql:execute(Ep, Ctx2, Ast2)} catch throw:{error, Err} -> - {error, Err} + {error, Err}; + Class:Reason:Stacktrace -> + Err = #{what => graphql_internal_crash, + class => Class, reason => Reason, + stacktrace => Stacktrace}, + ?LOG_ERROR(Err), + {error, internal_crash} end. %% @doc Execute selected operation on a given endpoint with authorization. diff --git a/src/mongoose_graphql/mongoose_graphql_errors.erl b/src/mongoose_graphql/mongoose_graphql_errors.erl index 3f6f0c050b1..7d609320082 100644 --- a/src/mongoose_graphql/mongoose_graphql_errors.erl +++ b/src/mongoose_graphql/mongoose_graphql_errors.erl @@ -5,7 +5,10 @@ -ignore_xref([format_error/1, err/2, crash/2]). +-type err_msg() :: #{message := binary(), extensions => map(), path => list()}. + %% callback invoked when resolver returns error tuple +-spec err(map(), term()) -> err_msg(). err(_Ctx, domain_not_found) -> #{message => <<"Given domain does not exist">>, extensions => #{code => resolver_error}}; err(_Ctx, ErrorTerm) -> @@ -13,11 +16,13 @@ err(_Ctx, ErrorTerm) -> extensions => #{code => resolver_error}}. %% callback invoked when resolver crashes +-spec crash(map(), term()) -> err_msg(). crash(_Ctx, #{type := Type}) -> #{message => <<"Unexpected ", Type/binary, " resolver crash">>, extensions => #{code => resolver_crash}}. %% @doc Format error that occurred in any phase including HTTP request decoding. +-spec format_error(term())-> {integer(), err_msg()}. format_error(#{phase := Phase, error_term := Term}) when Phase =:= authorize; Phase =:= decode; Phase =:= parse -> @@ -28,7 +33,12 @@ format_error(#{error_term := _, phase := Phase} = Err) when Phase =:= execute; Phase =:= type_check; Phase =:= validate; Phase =:= uncategorized -> - {400, graphql:format_errors(#{}, [Err])}; + [ErrMsg] = graphql:format_errors(#{}, [Err]), + {400, ErrMsg}; +format_error(internal_crash) -> + Msg = #{message => <<"GraphQL Internal Server Error">>, + extensions => #{code => internal_server_error}}, + {500, Msg}; format_error(Err) -> Msg = #{extensions => #{code => uncathegorized}, message => iolist_to_binary(io_lib:format("~p", [Err]))}, From 8cbe56ba6ac74cf652b7212510413e89711e830b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Thu, 16 Dec 2021 19:21:07 +0100 Subject: [PATCH 07/11] Test formatting error messages --- .../mongoose_graphql_errors.erl | 5 +- test/mongoose_graphql_SUITE.erl | 96 +++++++++++++++++++ 2 files changed, 99 insertions(+), 2 deletions(-) diff --git a/src/mongoose_graphql/mongoose_graphql_errors.erl b/src/mongoose_graphql/mongoose_graphql_errors.erl index 7d609320082..8e3c74f05d3 100644 --- a/src/mongoose_graphql/mongoose_graphql_errors.erl +++ b/src/mongoose_graphql/mongoose_graphql_errors.erl @@ -33,14 +33,15 @@ format_error(#{error_term := _, phase := Phase} = Err) when Phase =:= execute; Phase =:= type_check; Phase =:= validate; Phase =:= uncategorized -> - [ErrMsg] = graphql:format_errors(#{}, [Err]), + Err2 = maps:merge(#{path => []}, Err), + [ErrMsg] = graphql:format_errors(#{}, [Err2]), {400, ErrMsg}; format_error(internal_crash) -> Msg = #{message => <<"GraphQL Internal Server Error">>, extensions => #{code => internal_server_error}}, {500, Msg}; format_error(Err) -> - Msg = #{extensions => #{code => uncathegorized}, + Msg = #{extensions => #{code => uncategorized}, message => iolist_to_binary(io_lib:format("~p", [Err]))}, {400, Msg}. diff --git a/test/mongoose_graphql_SUITE.erl b/test/mongoose_graphql_SUITE.erl index b2ed767313e..d13fc567e12 100644 --- a/test/mongoose_graphql_SUITE.erl +++ b/test/mongoose_graphql_SUITE.erl @@ -12,6 +12,9 @@ -define(assertPermissionsSuccess(Config, Doc), ?assertMatch(ok, check_permissions(Config, Doc))). +-define(assertErrMsg(Code, MsgContains, ErrorMsg), + assert_err_msg(Code, MsgContains, ErrorMsg)). + all() -> [can_create_endpoint, can_load_split_schema, @@ -19,12 +22,14 @@ all() -> {group, unprotected_graphql}, {group, protected_graphql}, {group, error_handling}, + {group, error_formatting}, {group, permissions}]. groups() -> [{protected_graphql, [parallel], protected_graphql()}, {unprotected_graphql, [parallel], unprotected_graphql()}, {error_handling, [parallel], error_handling()}, + {error_formatting, [parallel], error_formatting()}, {permissions, [parallel], permissions()}]. protected_graphql() -> @@ -46,6 +51,17 @@ error_handling() -> should_catch_type_check_params_errors, should_catch_type_check_errors]. +error_formatting() -> + [format_internal_crash, + format_parse_errors, + format_decode_errors, + format_authorize_error, + format_validate_error, + format_type_check_error, + format_execute_error, + format_uncategorized_error, + format_any_error]. + permissions() -> [check_object_permissions, check_field_permissions, @@ -291,8 +307,88 @@ check_union_permissions(Config) -> ?assertPermissionsFailed(Config, FDoc), ?assertPermissionsFailed(Config, FDoc2). +format_internal_crash(_Config) -> + {Code, Res} = mongoose_graphql_errors:format_error(internal_crash), + ?assertEqual(500, Code), + ?assertMatch(#{extensions := #{code := internal_server_error}}, Res). + +format_parse_errors(_Config) -> + ParserError = make_error(parse, {parser_error, {0, graphql_parser, "parser_error_msg"}}), + ScannerError = make_error(parse, {scanner_error, + {0, graphql_scanner, {illegal, "illegal_characters"}}}), + ScannerError2 = make_error(parse, {scanner_error, + {0, graphql_scanner, {user, "user_scanner_err"}}}), + + {400, ResParser} = mongoose_graphql_errors:format_error(ParserError), + {400, ResScanner} = mongoose_graphql_errors:format_error(ScannerError), + {400, ResScanner2} = mongoose_graphql_errors:format_error(ScannerError2), + ?assertErrMsg(parser_error, <<"parser_error_msg">>, ResParser), + ?assertErrMsg(scanner_error, <<"illegal_characters">>, ResScanner), + ?assertErrMsg(scanner_error, <<"user_scanner_err">>, ResScanner2). + +format_decode_errors(_Config) -> + {400, Msg1} = mongoose_graphql_errors:format_error(make_error(decode, no_query_supplied)), + {400, Msg2} = mongoose_graphql_errors:format_error(make_error(decode, invalid_json_body)), + {400, Msg3} = mongoose_graphql_errors:format_error(make_error(decode, variables_invalid_json)), + + ?assertErrMsg(no_query_supplied, <<"The query was not supplied">>, Msg1), + ?assertErrMsg(invalid_json_body, <<"invalid">>, Msg2), + ?assertErrMsg(variables_invalid_json, <<"invalid">>, Msg3). + +format_authorize_error(_Config) -> + {401, Msg1} = mongoose_graphql_errors:format_error(make_error(authorize, wrong_credentials)), + {401, Msg2} = mongoose_graphql_errors:format_error( + make_error(authorize, {no_permissions, <<"ROOT">>})), + {401, Msg3} = mongoose_graphql_errors:format_error( + make_error(authorize, {request_error, {header, <<"authorization">>}, 'msg'})), + + ?assertErrMsg(wrong_credentials, <<"provided credentials are wrong">>, Msg1), + ?assertErrMsg(no_permissions, <<"without permissions">>, Msg2), + ?assertErrMsg(request_error, <<"Malformed authorization header">>, Msg3). + +format_validate_error(_Config) -> + % Ensure the module can format this phase + {400, Msg} = mongoose_graphql_errors:format_error( + make_error(validate, {not_unique, <<"OpName">>})), + ?assertMatch(#{extensions := #{code := not_unique}}, Msg). + +format_type_check_error(_Config) -> + % Ensure the module can format this phase + {400, Msg} = mongoose_graphql_errors:format_error( + make_error(type_check, non_null)), + ?assertMatch(#{extensions := #{code := non_null}}, Msg). + +format_execute_error(_Config) -> + % Ensure the module can format this phase + {400, Msg} = mongoose_graphql_errors:format_error( + make_error(execute, {resolver_error, any_error})), + ?assertMatch(#{extensions := #{code := resolver_error}}, Msg). + +format_uncategorized_error(_Config) -> + % Ensure the module can format this phase + {400, Msg} = mongoose_graphql_errors:format_error( + make_error(uncategorized, any_error)), + ?assertMatch(#{extensions := #{code := any_error}}, Msg). + +format_any_error(_Config) -> + {400, Msg1} = mongoose_graphql_errors:format_error(any_error), + {400, Msg2} = mongoose_graphql_errors:format_error(<<"any_error">>), + {400, Msg3} = mongoose_graphql_errors:format_error({1, any_error}), + {400, Msg4} = mongoose_graphql_errors:format_error(#{msg => any_error}), + ?assertErrMsg(uncategorized, <<"any_error">>, Msg1), + ?assertErrMsg(uncategorized, <<"any_error">>, Msg2), + ?assertErrMsg(uncategorized, <<"any_error">>, Msg3), + ?assertErrMsg(uncategorized, <<"any_error">>, Msg4). + %% Helpers +assert_err_msg(Code, MsgContains, #{message := Msg} = ErrorMsg) -> + ?assertMatch(#{extensions := #{code := Code}}, ErrorMsg), + ?assertNotEqual(nomatch, binary:match(Msg, MsgContains)). + +make_error(Phase, Term) -> + #{phase => Phase, error_term => Term}. + check_permissions(Config, Doc) -> Ep = ?config(endpoint, Config), Op = proplists:get_value(op, Config, undefined), From 7cb6b7f1732697e3c8564c2f1b09bd6caa07ef4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Fri, 17 Dec 2021 09:42:03 +0100 Subject: [PATCH 08/11] Add path to the no_permissions error --- big_tests/tests/graphql_SUITE.erl | 1 + .../mongoose_graphql_errors.erl | 18 +++++++++++++----- .../mongoose_graphql_permissions.erl | 3 ++- test/mongoose_graphql_SUITE.erl | 6 +++++- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/big_tests/tests/graphql_SUITE.erl b/big_tests/tests/graphql_SUITE.erl index cfb68c61512..303f15f2136 100644 --- a/big_tests/tests/graphql_SUITE.erl +++ b/big_tests/tests/graphql_SUITE.erl @@ -87,6 +87,7 @@ unauth_cannot_access_protected_types(Config) -> Ep = ?config(schema_endpoint, Config), Body = #{query => "{ field }"}, {Status, Data} = execute(Ep, Body, undefined), + ?assertMatch(#{<<"errors">> := [#{<<"path">> := [<<"ROOT">>]}]}, Data), assert_no_permissions(no_permissions, Status, Data). unauth_can_access_unprotected_types(Config) -> diff --git a/src/mongoose_graphql/mongoose_graphql_errors.erl b/src/mongoose_graphql/mongoose_graphql_errors.erl index 8e3c74f05d3..d30503c9d7f 100644 --- a/src/mongoose_graphql/mongoose_graphql_errors.erl +++ b/src/mongoose_graphql/mongoose_graphql_errors.erl @@ -1,4 +1,7 @@ -%% @doc Implements callbacks that format custom errors returned from resolvers or crashes. +%% @doc Implements callbacks that format custom errors returned from resolvers or crashes. +%% In addition, it can format each type of error that occurred in any graphql +%% or mongoose_graphql phase. +%% @end -module(mongoose_graphql_errors). -export([format_error/1, err/2, crash/2]). @@ -23,12 +26,12 @@ crash(_Ctx, #{type := Type}) -> %% @doc Format error that occurred in any phase including HTTP request decoding. -spec format_error(term())-> {integer(), err_msg()}. -format_error(#{phase := Phase, error_term := Term}) when Phase =:= authorize; - Phase =:= decode; - Phase =:= parse -> +format_error(#{phase := Phase, error_term := Term} = Err) when Phase =:= authorize; + Phase =:= decode; + Phase =:= parse -> Msg = #{extensions => #{code => err_code(Phase, Term)}, message => iolist_to_binary(err_msg(Phase, Term))}, - {err_http_code(Phase), Msg}; + {err_http_code(Phase), add_path(Err, Msg)}; format_error(#{error_term := _, phase := Phase} = Err) when Phase =:= execute; Phase =:= type_check; Phase =:= validate; @@ -85,3 +88,8 @@ decode_err_mgs(invalid_json_body) -> "The request json body is invalid"; decode_err_mgs(variables_invalid_json) -> "The variables' json is invalid". + +add_path(#{path := Path}, ErrMsg) -> + ErrMsg#{path => Path}; +add_path(_, ErrMsg) -> + ErrMsg. diff --git a/src/mongoose_graphql/mongoose_graphql_permissions.erl b/src/mongoose_graphql/mongoose_graphql_permissions.erl index 9b14f32b376..d165404b7fb 100644 --- a/src/mongoose_graphql/mongoose_graphql_permissions.erl +++ b/src/mongoose_graphql/mongoose_graphql_permissions.erl @@ -50,7 +50,8 @@ check_permissions(OpName, false, #document{definitions = Definitions}) -> true -> ok; false -> - graphql_err:abort([], authorize, {no_permissions, op_name(OpName)}) + OpName2 = op_name(OpName), + graphql_err:abort([OpName2], authorize, {no_permissions, OpName2}) end; false -> ok diff --git a/test/mongoose_graphql_SUITE.erl b/test/mongoose_graphql_SUITE.erl index d13fc567e12..82fb71c21a2 100644 --- a/test/mongoose_graphql_SUITE.erl +++ b/test/mongoose_graphql_SUITE.erl @@ -338,12 +338,13 @@ format_decode_errors(_Config) -> format_authorize_error(_Config) -> {401, Msg1} = mongoose_graphql_errors:format_error(make_error(authorize, wrong_credentials)), {401, Msg2} = mongoose_graphql_errors:format_error( - make_error(authorize, {no_permissions, <<"ROOT">>})), + make_error([<<"ROOT">>], authorize, {no_permissions, <<"ROOT">>})), {401, Msg3} = mongoose_graphql_errors:format_error( make_error(authorize, {request_error, {header, <<"authorization">>}, 'msg'})), ?assertErrMsg(wrong_credentials, <<"provided credentials are wrong">>, Msg1), ?assertErrMsg(no_permissions, <<"without permissions">>, Msg2), + ?assertMatch(#{path := [<<"ROOT">>]}, Msg2), ?assertErrMsg(request_error, <<"Malformed authorization header">>, Msg3). format_validate_error(_Config) -> @@ -389,6 +390,9 @@ assert_err_msg(Code, MsgContains, #{message := Msg} = ErrorMsg) -> make_error(Phase, Term) -> #{phase => Phase, error_term => Term}. +make_error(Path, Phase, Term) -> + #{path => Path, phase => Phase, error_term => Term}. + check_permissions(Config, Doc) -> Ep = ?config(endpoint, Config), Op = proplists:get_value(op, Config, undefined), From 2d44a280c7ec7ecbaae82288e5907a2471ba9e6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Fri, 17 Dec 2021 11:40:26 +0100 Subject: [PATCH 09/11] Increase test coverage and fix decode_err_msg --- .../mongoose_graphql_errors.erl | 14 +++++++------- test/mongoose_graphql_SUITE.erl | 18 ++++++++++++++---- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/mongoose_graphql/mongoose_graphql_errors.erl b/src/mongoose_graphql/mongoose_graphql_errors.erl index d30503c9d7f..f3c4d0fbb10 100644 --- a/src/mongoose_graphql/mongoose_graphql_errors.erl +++ b/src/mongoose_graphql/mongoose_graphql_errors.erl @@ -65,7 +65,7 @@ simplify(T) when is_tuple(T) -> element(1, T). err_msg(parse, Result) -> parse_err_msg(Result); err_msg(decode, Result) -> - decode_err_mgs(Result); + decode_err_msg(Result); err_msg(authorize, Result) -> authorize_err_msg(Result). @@ -82,12 +82,12 @@ parse_err_msg({scanner_error, {Line, graphql_scanner, Msg}}) -> Formatted = lists:flatten(graphql_scanner:format_error(Msg)), io_lib:format("Cannot scan line ~B because of ~s", [Line, Formatted]). -decode_err_mgs(no_query_supplied) -> - "The query was not supplied in request body"; -decode_err_mgs(invalid_json_body) -> - "The request json body is invalid"; -decode_err_mgs(variables_invalid_json) -> - "The variables' json is invalid". +decode_err_msg(no_query_supplied) -> + "The query was not supplied in the request body"; +decode_err_msg(invalid_json_body) -> + "The request JSON body is invalid"; +decode_err_msg(variables_invalid_json) -> + "The variables' JSON is invalid". add_path(#{path := Path}, ErrMsg) -> ErrMsg#{path => Path}; diff --git a/test/mongoose_graphql_SUITE.erl b/test/mongoose_graphql_SUITE.erl index 82fb71c21a2..2365c869b95 100644 --- a/test/mongoose_graphql_SUITE.erl +++ b/test/mongoose_graphql_SUITE.erl @@ -18,6 +18,7 @@ all() -> [can_create_endpoint, can_load_split_schema, + unexpected_internal_error, admin_and_user_load_global_types, {group, unprotected_graphql}, {group, protected_graphql}, @@ -138,6 +139,12 @@ can_load_split_schema(Config) -> ?assertMatch(#object_type{id = <<"Query">>}, graphql_schema:get(Ep, <<"Query">>)), ?assertMatch(#object_type{id = <<"Mutation">>}, graphql_schema:get(Ep, <<"Mutation">>)). +unexpected_internal_error(Config) -> + Name = ?config(endpoint_name, Config), + Doc = <<"mutation { field }">>, + Res = mongoose_graphql:execute(Name, undefined, Doc), + ?assertEqual({error, internal_crash}, Res). + admin_and_user_load_global_types(_Config) -> mongoose_graphql:init(), AdminEp = mongoose_graphql:get_endpoint(admin), @@ -164,9 +171,9 @@ auth_can_execute_protected_mutation(Config) -> unauth_cannot_execute_protected_query(Config) -> Ep = ?config(endpoint, Config), - Doc = <<"{ field }">>, - Res = mongoose_graphql:execute(Ep, request(Doc, false)), - ?assertMatch({error, #{error_term := {no_permissions, <<"ROOT">>}}}, Res). + Doc = <<"query Q1 { field }">>, + Res = mongoose_graphql:execute(Ep, request(<<"Q1">>, Doc, false)), + ?assertMatch({error, #{error_term := {no_permissions, <<"Q1">>}, path := [<<"Q1">>]}}, Res). unauth_cannot_execute_protected_mutation(Config) -> Ep = ?config(endpoint, Config), @@ -402,8 +409,11 @@ check_permissions(Config, Doc) -> ok = mongoose_graphql_permissions:check_permissions(Op, false, Ast2). request(Doc, Authorized) -> + request(undefined, Doc, Authorized). + +request(Op, Doc, Authorized) -> #{document => Doc, - operation_name => undefined, + operation_name => Op, vars => #{}, authorized => Authorized, ctx => #{}}. From 88102c2b34a50bc5d28431772374b89d1ed7aa73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Mon, 20 Dec 2021 14:16:36 +0100 Subject: [PATCH 10/11] Test validation phase, Upgdate other test cases --- big_tests/tests/graphql_SUITE.erl | 1 + test/mongoose_graphql_SUITE.erl | 38 ++++++++++++++++++++----------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/big_tests/tests/graphql_SUITE.erl b/big_tests/tests/graphql_SUITE.erl index 303f15f2136..78f71c7a9c2 100644 --- a/big_tests/tests/graphql_SUITE.erl +++ b/big_tests/tests/graphql_SUITE.erl @@ -111,6 +111,7 @@ malformed_auth_header_error(Config) -> headers => [{<<"Authorization">>, <<"Basic YWRtaW46c2VjcmV">>}], return_maps => true, path => "/graphql"}, + % The encoded credentials value is malformed and cannot be decoded. {Status, Data} = rest_helper:make_request(Request), assert_no_permissions(request_error, Status, Data). diff --git a/test/mongoose_graphql_SUITE.erl b/test/mongoose_graphql_SUITE.erl index 2365c869b95..51bc1c94b60 100644 --- a/test/mongoose_graphql_SUITE.erl +++ b/test/mongoose_graphql_SUITE.erl @@ -48,9 +48,10 @@ unprotected_graphql() -> unauth_can_execute_mutation]. error_handling() -> - [should_catch_parsing_errors, - should_catch_type_check_params_errors, - should_catch_type_check_errors]. + [should_catch_parsing_error, + should_catch_type_check_params_error, + should_catch_type_check_error, + should_catch_validation_error]. error_formatting() -> [format_internal_crash, @@ -89,9 +90,10 @@ init_per_testcase(C, Config) when C =:= can_execute_query_with_vars; C =:= auth_can_execute_mutation; C =:= unauth_can_execute_query; C =:= unauth_can_execute_mutation; - C =:= should_catch_type_check_params_errors; - C =:= should_catch_type_check_errors; - C =:= should_catch_parsing_errors -> + C =:= should_catch_type_check_params_error; + C =:= should_catch_type_check_error; + C =:= should_catch_parsing_error; + C =:= should_catch_validation_error -> {Mapping, Pattern} = example_schema_data(Config), {ok, _} = mongoose_graphql:create_endpoint(C, Mapping, [Pattern]), Ep = mongoose_graphql:get_endpoint(C), @@ -237,23 +239,33 @@ auth_can_execute_mutation(Config) -> Res = mongoose_graphql:execute(Ep, request(Doc, true)), ?assertEqual({ok, #{data => #{<<"field">> => <<"Test field">>}}}, Res). -should_catch_parsing_errors(Config) -> +should_catch_parsing_error(Config) -> Ep = ?config(endpoint, Config), Doc = <<"query { field ">>, - Res = mongoose_graphql:execute(Ep, request(Doc, false)), - ?assertMatch({error, _}, Res). + DocScan = <<"query { id(value: \"ala) }">>, + ResParseErr = mongoose_graphql:execute(Ep, request(Doc, false)), + ?assertMatch({error, #{phase := parse, error_term := {parser_error, _}}}, ResParseErr), + ResScanErr = mongoose_graphql:execute(Ep, request(DocScan, false)), + ?assertMatch({error, #{phase := parse, error_term := {scanner_error, _}}}, ResScanErr). -should_catch_type_check_errors(Config) -> +should_catch_type_check_error(Config) -> Ep = ?config(endpoint, Config), Doc = <<"query { notExistingField(value: \"Hello\") }">>, Res = mongoose_graphql:execute(Ep, request(Doc, false)), - ?assertMatch({error, _}, Res). + ?assertMatch({error, #{phase := type_check, error_term := unknown_field}}, Res). -should_catch_type_check_params_errors(Config) -> +should_catch_type_check_params_error(Config) -> Ep = ?config(endpoint, Config), Doc = <<"query { id(value: 12) }">>, Res = mongoose_graphql:execute(Ep, request(Doc, false)), - ?assertMatch({error, _}, Res). + ?assertMatch({error, #{phase := type_check, error_term := {input_coercion, _, _, _}}}, Res). + +should_catch_validation_error(Config) -> + Ep = ?config(endpoint, Config), + Doc = <<"query Q1{ id(value: \"ok\") } query Q1{ id(value: \"ok\") }">>, + % Query name must be unique + Res = mongoose_graphql:execute(Ep, request(<<"Q1">>, Doc, false)), + ?assertMatch({error, #{phase := validate, error_term := {not_unique, _}}}, Res). check_object_permissions(Config) -> Doc = <<"query { field }">>, From 6d971fe690402f094e57860dbc51da2ce5edd989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Wojtasik?= Date: Mon, 20 Dec 2021 14:49:41 +0100 Subject: [PATCH 11/11] Test for handling parse, type-check, and validating errors --- big_tests/tests/graphql_SUITE.erl | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/big_tests/tests/graphql_SUITE.erl b/big_tests/tests/graphql_SUITE.erl index 78f71c7a9c2..aa035af641c 100644 --- a/big_tests/tests/graphql_SUITE.erl +++ b/big_tests/tests/graphql_SUITE.erl @@ -31,6 +31,9 @@ admin_handler() -> common_tests() -> [malformed_auth_header_error, + document_parse_error, + document_type_check_error, + document_validate_error, wrong_creds_cannot_access_protected_types, unauth_cannot_access_protected_types, unauth_can_access_unprotected_types, @@ -115,6 +118,32 @@ malformed_auth_header_error(Config) -> {Status, Data} = rest_helper:make_request(Request), assert_no_permissions(request_error, Status, Data). +document_parse_error(Config) -> + Ep = ?config(schema_endpoint, Config), + Body = #{<<"query">> => <<"{ field ">>}, + {Status, Data} = execute(Ep, Body, undefined), + ?assertEqual({<<"400">>,<<"Bad Request">>}, Status), + assert_code(parser_error, Data), + + BodyScanner = #{<<"query">> => <<"mutation { id(value: \"asdfsad) } ">>}, + {StatusScanner, DataScanner} = execute(Ep, BodyScanner, undefined), + ?assertEqual({<<"400">>,<<"Bad Request">>}, StatusScanner), + assert_code(scanner_error, DataScanner). + +document_type_check_error(Config) -> + Ep = ?config(schema_endpoint, Config), + Body = #{<<"query">> => <<"mutation { id(value: 12) }">>}, + {Status, Data} = execute(Ep, Body, undefined), + ?assertEqual({<<"400">>,<<"Bad Request">>}, Status), + assert_code(input_coercion, Data). + +document_validate_error(Config) -> + Ep = ?config(schema_endpoint, Config), + Body = #{<<"query">> => <<"query Q1 { field } query Q1 { field }">>, <<"operationName">> => <<"Q1">>}, + {Status, Data} = execute(Ep, Body, undefined), + ?assertEqual({<<"400">>,<<"Bad Request">>}, Status), + assert_code(not_unique, Data). + auth_user_can_access_protected_types(Config) -> escalus:fresh_story( Config, [{alice, 1}],