From c30fd94dbff59df501c00088213b046b5ccea530 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 22 Sep 2022 08:59:21 +0200 Subject: [PATCH 1/6] Fail if creds are provided, but not configured This way we don't give the false impression of a password being checked. It already works like this for mongoose_domain_handler. --- big_tests/tests/rest_SUITE.erl | 15 ++++++++------- src/mongoose_admin_api/mongoose_admin_api.erl | 4 ++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/big_tests/tests/rest_SUITE.erl b/big_tests/tests/rest_SUITE.erl index a0fb556e0d7..811048981a0 100644 --- a/big_tests/tests/rest_SUITE.erl +++ b/big_tests/tests/rest_SUITE.erl @@ -66,7 +66,8 @@ auth_test_cases() -> auth_fails_incorrect_creds]. blank_auth_testcases() -> - [auth_always_passes_blank_creds]. + [auth_passes_without_creds, + auth_fails_with_creds]. test_cases() -> [non_existent_command_returns404, @@ -153,13 +154,13 @@ auth_fails_incorrect_creds(_Config) -> % try to login with different creds {?NOT_AUTHORIZED, _} = gett(admin, path("users", [domain()]), {<<"ola">>, <<"mapsa">>}). -auth_always_passes_blank_creds(_Config) -> - % we set control creds for blank - rest_helper:change_admin_creds(any), - % try with any auth - {?OK, Users} = gett(admin, path("users", [domain()]), {<<"aaaa">>, <<"bbbb">>}), +auth_passes_without_creds(_Config) -> % try with no auth - {?OK, Users} = gett(admin, path("users", [domain()])). + {?OK, _Users} = gett(admin, path("users", [domain()])). + +auth_fails_with_creds(_Config) -> + % try with any auth + {?NOT_AUTHORIZED, _} = gett(admin, path("users", [domain()]), {<<"aaaa">>, <<"bbbb">>}). non_existent_command_returns404(_C) -> {?NOT_FOUND, _} = gett(admin, <<"/isitthereornot">>). diff --git a/src/mongoose_admin_api/mongoose_admin_api.erl b/src/mongoose_admin_api/mongoose_admin_api.erl index 36e103c7eca..6f75a168d56 100644 --- a/src/mongoose_admin_api/mongoose_admin_api.erl +++ b/src/mongoose_admin_api/mongoose_admin_api.erl @@ -101,8 +101,8 @@ authorize(#{username := Username, password := Password}, AuthDetails) -> _ -> false end; -authorize(#{}, _) -> - true. % no credentials required +authorize(#{}, AuthDetails) -> + AuthDetails =:= undefined. % Do not accept basic auth when not configured -spec parse_body(req()) -> #{atom() => jiffy:json_value()}. parse_body(Req) -> From 065dafe7e3c8accd3d118af73e116ad6734436f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 22 Sep 2022 09:01:23 +0200 Subject: [PATCH 2/6] Integrate the domain handler with mongoose_admin_api Error messages are changed to capitalized strings, because it is the convention used for the whole admin API. Error codes are left as they were except ones that are already provided by mongoose_admin_api itself, e.g. "401 Unauthorized". The error messages should be common for REST and GraphQL, but this unification is left for the future. --- src/domain/mongoose_domain_handler.erl | 220 ------------------ src/mongoose_admin_api/mongoose_admin_api.erl | 7 +- .../mongoose_admin_api_domain.erl | 150 ++++++++++++ src/mongoose_http_handler.erl | 1 - .../mongoose_system_metrics_collector.erl | 2 +- 5 files changed, 156 insertions(+), 224 deletions(-) delete mode 100644 src/domain/mongoose_domain_handler.erl create mode 100644 src/mongoose_admin_api/mongoose_admin_api_domain.erl diff --git a/src/domain/mongoose_domain_handler.erl b/src/domain/mongoose_domain_handler.erl deleted file mode 100644 index 9851a668141..00000000000 --- a/src/domain/mongoose_domain_handler.erl +++ /dev/null @@ -1,220 +0,0 @@ -%% REST API for domain actions. --module(mongoose_domain_handler). - --behaviour(mongoose_http_handler). --behaviour(cowboy_rest). - -%% mongoose_http_handler callbacks --export([config_spec/0, routes/1]). - -%% config processing callbacks --export([process_config/1]). - -%% Standard cowboy_rest callbacks. --export([init/2, - allowed_methods/2, - content_types_accepted/2, - content_types_provided/2, - is_authorized/2, - delete_resource/2]). - -%% Custom cowboy_rest callbacks. --export([handle_domain/2, - to_json/2]). - --ignore_xref([cowboy_router_paths/2, handle_domain/2, to_json/2]). - --include("mongoose_logger.hrl"). --include("mongoose_config_spec.hrl"). --type state() :: map(). - --type handler_options() :: #{path := string(), username => binary(), password => binary(), - atom() => any()}. - -%% mongoose_http_handler callbacks - --spec config_spec() -> mongoose_config_spec:config_section(). -config_spec() -> - #section{items = #{<<"username">> => #option{type = binary}, - <<"password">> => #option{type = binary}}, - process = fun ?MODULE:process_config/1}. - -process_config(Opts) -> - case maps:is_key(username, Opts) =:= maps:is_key(password, Opts) of - true -> - Opts; - false -> - error(#{what => both_username_and_password_required, opts => Opts}) - end. - --spec routes(handler_options()) -> mongoose_http_handler:routes(). -routes(Opts = #{path := BasePath}) -> - [{[BasePath, "/domains/:domain"], ?MODULE, Opts}]. - -%% cowboy_rest callbacks - -init(Req, Opts) -> - {cowboy_rest, Req, Opts}. - -allowed_methods(Req, State) -> - {[<<"GET">>, <<"PUT">>, <<"PATCH">>, <<"DELETE">>], Req, State}. - -content_types_accepted(Req, State) -> - {[{{<<"application">>, <<"json">>, '*'}, handle_domain}], - Req, State}. - -content_types_provided(Req, State) -> - {[{{<<"application">>, <<"json">>, '*'}, to_json}], Req, State}. - -is_authorized(Req, State) -> - HeaderDetails = cowboy_req:parse_header(<<"authorization">>, Req), - ConfigDetails = state_to_details(State), - case check_auth(HeaderDetails, ConfigDetails) of - ok -> - {true, Req, State}; - {error, auth_header_passed_but_not_expected} -> - {false, reply_error(403, <<"basic auth provided, but not configured">>, Req), State}; - {error, auth_password_invalid} -> - {false, reply_error(403, <<"basic auth provided, invalid password">>, Req), State}; - {error, no_basic_auth_provided} -> - {false, reply_error(403, <<"basic auth is required">>, Req), State} - end. - -state_to_details(#{username := User, password := Pass}) -> - {basic, User, Pass}; -state_to_details(_) -> - not_configured. - -check_auth({basic, _User, _Pass}, _ConfigDetails = not_configured) -> - {error, auth_header_passed_but_not_expected}; -check_auth(_HeaderDetails, _ConfigDetails = not_configured) -> - ok; -check_auth({basic, User, Pass}, {basic, User, Pass}) -> - ok; -check_auth({basic, _, _}, {basic, _, _}) -> - {error, auth_password_invalid}; -check_auth(_, {basic, _, _}) -> - {error, no_basic_auth_provided}. - -%% Custom cowboy_rest callbacks: --spec to_json(Req, State) -> {Body, Req, State} | {stop, Req, State} - when Req :: cowboy_req:req(), State :: state(), Body :: binary(). -to_json(Req, State) -> - ExtDomain = cowboy_req:binding(domain, Req), - Domain = jid:nameprep(ExtDomain), - case mongoose_domain_sql:select_domain(Domain) of - {ok, Props} -> - {jiffy:encode(Props), Req, State}; - {error, not_found} -> - {stop, reply_error(404, <<"domain not found">>, Req), State} - end. - --spec handle_domain(Req, State) -> {boolean(), Req, State} - when Req :: cowboy_req:req(), State :: state(). -handle_domain(Req, State) -> - Method = cowboy_req:method(Req), - ExtDomain = cowboy_req:binding(domain, Req), - Domain = jid:nameprep(ExtDomain), - {ok, Body, Req2} = cowboy_req:read_body(Req), - MaybeParams = json_decode(Body), - case Method of - <<"PUT">> -> - insert_domain(Domain, MaybeParams, Req2, State); - <<"PATCH">> -> - patch_domain(Domain, MaybeParams, Req2, State) - end. - -%% Private helper functions: -insert_domain(Domain, {ok, #{<<"host_type">> := HostType}}, Req, State) -> - case mongoose_domain_api:insert_domain(Domain, HostType) of - ok -> - {true, Req, State}; - {error, duplicate} -> - {false, reply_error(409, <<"duplicate">>, Req), State}; - {error, static} -> - {false, reply_error(403, <<"domain is static">>, Req), State}; - {error, {db_error, _}} -> - {false, reply_error(500, <<"database error">>, Req), State}; - {error, service_disabled} -> - {false, reply_error(403, <<"service disabled">>, Req), State}; - {error, unknown_host_type} -> - {false, reply_error(403, <<"unknown host type">>, Req), State} - end; -insert_domain(_Domain, {ok, #{}}, Req, State) -> - {false, reply_error(400, <<"'host_type' field is missing">>, Req), State}; -insert_domain(_Domain, {error, empty}, Req, State) -> - {false, reply_error(400, <<"body is empty">>, Req), State}; -insert_domain(_Domain, {error, _}, Req, State) -> - {false, reply_error(400, <<"failed to parse JSON">>, Req), State}. - -patch_domain(Domain, {ok, #{<<"enabled">> := true}}, Req, State) -> - Res = mongoose_domain_api:enable_domain(Domain), - handle_enabled_result(Res, Req, State); -patch_domain(Domain, {ok, #{<<"enabled">> := false}}, Req, State) -> - Res = mongoose_domain_api:disable_domain(Domain), - handle_enabled_result(Res, Req, State); -patch_domain(_Domain, {ok, #{}}, Req, State) -> - {false, reply_error(400, <<"'enabled' field is missing">>, Req), State}; -patch_domain(_Domain, {error, empty}, Req, State) -> - {false, reply_error(400, <<"body is empty">>, Req), State}; -patch_domain(_Domain, {error, _}, Req, State) -> - {false, reply_error(400, <<"failed to parse JSON">>, Req), State}. - -handle_enabled_result(Res, Req, State) -> - case Res of - ok -> - {true, Req, State}; - {error, not_found} -> - {false, reply_error(404, <<"domain not found">>, Req), State}; - {error, static} -> - {false, reply_error(403, <<"domain is static">>, Req), State}; - {error, service_disabled} -> - {false, reply_error(403, <<"service disabled">>, Req), State}; - {error, {db_error, _}} -> - {false, reply_error(500, <<"database error">>, Req), State} - end. - -delete_resource(Req, State) -> - ExtDomain = cowboy_req:binding(domain, Req), - Domain = jid:nameprep(ExtDomain), - {ok, Body, Req2} = cowboy_req:read_body(Req), - MaybeParams = json_decode(Body), - delete_domain(Domain, MaybeParams, Req2, State). - -delete_domain(Domain, {ok, #{<<"host_type">> := HostType}}, Req, State) -> - case mongoose_domain_api:delete_domain(Domain, HostType) of - ok -> - {true, Req, State}; - {error, {db_error, _}} -> - {false, reply_error(500, <<"database error">>, Req), State}; - {error, static} -> - {false, reply_error(403, <<"domain is static">>, Req), State}; - {error, service_disabled} -> - {false, reply_error(403, <<"service disabled">>, Req), State}; - {error, wrong_host_type} -> - {false, reply_error(403, <<"wrong host type">>, Req), State}; - {error, unknown_host_type} -> - {false, reply_error(403, <<"unknown host type">>, Req), State} - end; -delete_domain(_Domain, {ok, #{}}, Req, State) -> - {false, reply_error(400, <<"'host_type' field is missing">>, Req), State}; -delete_domain(_Domain, {error, empty}, Req, State) -> - {false, reply_error(400, <<"body is empty">>, Req), State}; -delete_domain(_Domain, {error, _}, Req, State) -> - {false, reply_error(400, <<"failed to parse JSON">>, Req), State}. - -reply_error(Code, What, Req) -> - ?LOG_ERROR(#{what => rest_domain_failed, reason => What, - code => Code, req => Req}), - Body = jiffy:encode(#{what => What}), - cowboy_req:reply(Code, #{<<"content-type">> => <<"application/json">>}, Body, Req). - -json_decode(<<>>) -> - {error, empty}; -json_decode(Bin) -> - try - {ok, jiffy:decode(Bin, [return_maps])} - catch - Class:Reason -> - {error, {Class, Reason}} - end. diff --git a/src/mongoose_admin_api/mongoose_admin_api.erl b/src/mongoose_admin_api/mongoose_admin_api.erl index 6f75a168d56..d424324b23f 100644 --- a/src/mongoose_admin_api/mongoose_admin_api.erl +++ b/src/mongoose_admin_api/mongoose_admin_api.erl @@ -25,7 +25,7 @@ atom() => any()}. -type req() :: cowboy_req:req(). -type state() :: map(). --type error_type() :: bad_request | denied | not_found | internal. +-type error_type() :: bad_request | denied | not_found | duplicate | internal. %% mongoose_http_handler callbacks @@ -67,7 +67,8 @@ api_paths(Opts) -> {"/mucs/:domain", mongoose_admin_api_muc, Opts}, {"/mucs/:domain/:name/:arg", mongoose_admin_api_muc, Opts}, {"/inbox/:host_type/:days/bin", mongoose_admin_api_inbox, Opts}, - {"/inbox/:domain/:user/:days/bin", mongoose_admin_api_inbox, Opts} + {"/inbox/:domain/:user/:days/bin", mongoose_admin_api_inbox, Opts}, + {"/domains/:domain", mongoose_admin_api_domain, Opts} ]. %% Utilities for the handler modules @@ -166,10 +167,12 @@ error_response(ErrorType, Message, Req, State) -> error_code(bad_request) -> 400; error_code(denied) -> 403; error_code(not_found) -> 404; +error_code(duplicate) -> 409; error_code(internal) -> 500. -spec log_level(error_type()) -> logger:level(). log_level(bad_request) -> warning; log_level(denied) -> warning; log_level(not_found) -> warning; +log_level(duplicate) -> warning; log_level(internal) -> error. diff --git a/src/mongoose_admin_api/mongoose_admin_api_domain.erl b/src/mongoose_admin_api/mongoose_admin_api_domain.erl new file mode 100644 index 00000000000..907f3243f73 --- /dev/null +++ b/src/mongoose_admin_api/mongoose_admin_api_domain.erl @@ -0,0 +1,150 @@ +-module(mongoose_admin_api_domain). +-behaviour(cowboy_rest). + +-export([init/2, + is_authorized/2, + content_types_provided/2, + content_types_accepted/2, + allowed_methods/2, + to_json/2, + from_json/2, + delete_resource/2]). + +-ignore_xref([to_json/2, from_json/2]). + +-import(mongoose_admin_api, [parse_body/1, try_handle_request/3, throw_error/2, resource_created/4]). + +-type req() :: cowboy_req:req(). +-type state() :: map(). + +-spec init(req(), state()) -> {cowboy_rest, req(), state()}. +init(Req, Opts) -> + mongoose_admin_api:init(Req, Opts). + +-spec is_authorized(req(), state()) -> {true | {false, iodata()}, req(), state()}. +is_authorized(Req, State) -> + mongoose_admin_api:is_authorized(Req, State). + +-spec content_types_provided(req(), state()) -> + {[{{binary(), binary(), '*'}, atom()}], req(), state()}. +content_types_provided(Req, State) -> + {[ + {{<<"application">>, <<"json">>, '*'}, to_json} + ], Req, State}. + +-spec content_types_accepted(req(), state()) -> + {[{{binary(), binary(), '*'}, atom()}], req(), state()}. +content_types_accepted(Req, State) -> + {[ + {{<<"application">>, <<"json">>, '*'}, from_json} + ], Req, State}. + +-spec allowed_methods(req(), state()) -> {[binary()], req(), state()}. +allowed_methods(Req, State) -> + {[<<"OPTIONS">>, <<"GET">>, <<"PATCH">>, <<"PUT">>, <<"DELETE">>], Req, State}. + +%% @doc Called for a method of type "GET" +-spec to_json(req(), state()) -> {iodata() | stop, req(), state()}. +to_json(Req, State) -> + try_handle_request(Req, State, fun handle_get/2). + +%% @doc Called for a method of type "PUT" or "PATCH" +-spec from_json(req(), state()) -> {true | stop, req(), state()}. +from_json(Req, State) -> + F = case cowboy_req:method(Req) of + <<"PUT">> -> fun handle_put/2; + <<"PATCH">> -> fun handle_patch/2 + end, + try_handle_request(Req, State, F). + +%% @doc Called for a method of type "DELETE" +-spec delete_resource(req(), state()) -> {true | stop, req(), state()}. +delete_resource(Req, State) -> + try_handle_request(Req, State, fun handle_delete/2). + +%% Internal functions + +handle_get(Req, State) -> + Bindings = cowboy_req:bindings(Req), + Domain = get_domain(Bindings), + case mongoose_domain_sql:select_domain(Domain) of + {ok, Props} -> + {jiffy:encode(Props), Req, State}; + {error, not_found} -> + throw_error(not_found, <<"Domain not found">>) + end. + +handle_put(Req, State) -> + Bindings = cowboy_req:bindings(Req), + Domain = get_domain(Bindings), + Args = parse_body(Req), + HostType = get_host_type(Args), + case mongoose_domain_api:insert_domain(Domain, HostType) of + ok -> + {true, Req, State}; + {error, duplicate} -> + throw_error(duplicate, <<"Duplicate domain">>); + {error, static} -> + throw_error(denied, <<"Domain is static">>); + {error, {db_error, _}} -> + throw_error(internal, <<"Database error">>); + {error, service_disabled} -> + throw_error(denied, <<"Service disabled">>); + {error, unknown_host_type} -> + throw_error(denied, <<"Unknown host type">>) + end. + +handle_patch(Req, State) -> + Bindings = cowboy_req:bindings(Req), + Domain = get_domain(Bindings), + Args = parse_body(Req), + Result = case get_enabled(Args) of + true -> + mongoose_domain_api:enable_domain(Domain); + false -> + mongoose_domain_api:disable_domain(Domain) + end, + case Result of + ok -> + {true, Req, State}; + {error, not_found} -> + throw_error(not_found, <<"Domain not found">>); + {error, static} -> + throw_error(denied, <<"Domain is static">>); + {error, service_disabled} -> + throw_error(denied, <<"Service disabled">>); + {error, {db_error, _}} -> + throw_error(internal, <<"Database error">>) + end. + +handle_delete(Req, State) -> + Bindings = cowboy_req:bindings(Req), + Domain = get_domain(Bindings), + Args = parse_body(Req), + HostType = get_host_type(Args), + case mongoose_domain_api:delete_domain(Domain, HostType) of + ok -> + {true, Req, State}; + {error, {db_error, _}} -> + throw_error(internal, <<"Database error">>); + {error, static} -> + throw_error(denied, <<"Domain is static">>); + {error, service_disabled} -> + throw_error(denied, <<"Service disabled">>); + {error, wrong_host_type} -> + throw_error(denied, <<"Wrong host type">>); + {error, unknown_host_type} -> + throw_error(denied, <<"Unknown host type">>) + end. + +get_domain(#{domain := Domain}) -> + case jid:nameprep(Domain) of + error -> throw_error(bad_request, <<"Invalid domain name">>); + PrepDomain -> PrepDomain + end. + +get_host_type(#{host_type := HostType}) -> HostType; +get_host_type(#{}) -> throw_error(bad_request, <<"'host_type' field is missing">>). + +get_enabled(#{enabled := Enabled}) -> Enabled; +get_enabled(#{}) -> throw_error(bad_request, <<"'enabled' field is missing">>). diff --git a/src/mongoose_http_handler.erl b/src/mongoose_http_handler.erl index eee7434a57a..e1da3d77589 100644 --- a/src/mongoose_http_handler.erl +++ b/src/mongoose_http_handler.erl @@ -85,5 +85,4 @@ configurable_handler_modules() -> mongoose_client_api, mongoose_admin_api, mongoose_api, - mongoose_domain_handler, mongoose_graphql_cowboy_handler]. diff --git a/src/system_metrics/mongoose_system_metrics_collector.erl b/src/system_metrics/mongoose_system_metrics_collector.erl index 2fcca06f7b7..b206be499be 100644 --- a/src/system_metrics/mongoose_system_metrics_collector.erl +++ b/src/system_metrics/mongoose_system_metrics_collector.erl @@ -127,7 +127,7 @@ get_api() -> filter_unknown_api(ApiList) -> AllowedToReport = [mongoose_api, mongoose_client_api, mongoose_admin_api, - mongoose_domain_handler, mod_bosh, mod_websockets], + mod_bosh, mod_websockets], [Api || Api <- ApiList, lists:member(Api, AllowedToReport)]. get_transport_mechanisms() -> From c8be0bea5dd9f330f8c0b590f376635065e1ee8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 22 Sep 2022 09:04:45 +0200 Subject: [PATCH 3/6] Remove tests for the deleted domain handler --- test/config_parser_SUITE.erl | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index d65ca2832d8..5a9dfaa90ec 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -99,7 +99,6 @@ groups() -> listen_http_handlers_client_api, listen_http_handlers_admin_api, listen_http_handlers_api, - listen_http_handlers_domain, listen_http_handlers_graphql]}, {auth, [parallel], [auth_methods, auth_password, @@ -613,10 +612,6 @@ listen_http_handlers_api(_Config) -> T(#{<<"handlers">> => [<<"mongoose_api_metrics">>]})), ?err(T(#{<<"handlers">> => [not_a_module]})). -listen_http_handlers_domain(_Config) -> - {P, T} = test_listen_http_handler(mongoose_domain_handler), - test_listen_http_handler_creds(P, T). - listen_http_handlers_graphql(_Config) -> T = fun graphql_handler_raw/1, {P, _} = test_listen_http_handler(mongoose_graphql_cowboy_handler, T), From cecd6e42a20670059078a886da3917fbe219e182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 22 Sep 2022 09:05:21 +0200 Subject: [PATCH 4/6] Remove the domain handler from the TOML config --- rel/files/mongooseim.toml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rel/files/mongooseim.toml b/rel/files/mongooseim.toml index 79c8c382d03..7e25bc74b2e 100644 --- a/rel/files/mongooseim.toml +++ b/rel/files/mongooseim.toml @@ -60,10 +60,6 @@ host = "localhost" path = "/api" - [[listen.http.handlers.mongoose_domain_handler]] - host = "localhost" - path = "/api" - [[listen.http]] {{#http_api_client_endpoint}} {{{http_api_client_endpoint}}} From 33b5d9c14bd9800eb3f60da60a9905f2cc638094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 22 Sep 2022 09:05:49 +0200 Subject: [PATCH 5/6] Adjust big tests for the domain REST API --- big_tests/tests/domain_rest_helper.erl | 39 ------- big_tests/tests/service_domain_db_SUITE.erl | 119 +++++++------------- 2 files changed, 42 insertions(+), 116 deletions(-) diff --git a/big_tests/tests/domain_rest_helper.erl b/big_tests/tests/domain_rest_helper.erl index c85268ec11a..b41d77e8c1c 100644 --- a/big_tests/tests/domain_rest_helper.erl +++ b/big_tests/tests/domain_rest_helper.erl @@ -14,14 +14,7 @@ delete_custom/4, patch_custom/4]). -%% Handler --export([start_listener/1, - stop_listener/1]). - -import(distributed_helper, [mim/0, mim2/0, require_rpc_nodes/1, rpc/4]). --import(config_parser_helper, [default_config/1, config/2]). - --define(TEST_PORT, 8866). set_invalid_creds(Config) -> [{auth_creds, invalid}|Config]. @@ -49,7 +42,6 @@ make_creds(Config) -> rest_patch_enabled(Config, Domain, Enabled) -> Params = #{enabled => Enabled}, rest_helper:make_request(#{ role => admin, method => <<"PATCH">>, - port => ?TEST_PORT, path => domain_path(Domain), creds => make_creds(Config), body => Params }). @@ -57,14 +49,12 @@ rest_patch_enabled(Config, Domain, Enabled) -> rest_put_domain(Config, Domain, Type) -> Params = #{host_type => Type}, rest_helper:make_request(#{ role => admin, method => <<"PUT">>, - port => ?TEST_PORT, path => domain_path(Domain), creds => make_creds(Config), body => Params }). putt_domain_with_custom_body(Config, Body) -> rest_helper:make_request(#{ role => admin, method => <<"PUT">>, - port => ?TEST_PORT, path => <<"/domains/example.db">>, creds => make_creds(Config), body => Body }). @@ -72,7 +62,6 @@ putt_domain_with_custom_body(Config, Body) -> rest_select_domain(Config, Domain) -> Params = #{}, rest_helper:make_request(#{ role => admin, method => <<"GET">>, - port => ?TEST_PORT, path => domain_path(Domain), creds => make_creds(Config), body => Params }). @@ -80,44 +69,16 @@ rest_select_domain(Config, Domain) -> rest_delete_domain(Config, Domain, HostType) -> Params = #{<<"host_type">> => HostType}, rest_helper:make_request(#{ role => admin, method => <<"DELETE">>, - port => ?TEST_PORT, path => domain_path(Domain), creds => make_creds(Config), body => Params }). delete_custom(Config, Role, Path, Body) -> rest_helper:make_request(#{ role => Role, method => <<"DELETE">>, - port => ?TEST_PORT, path => Path, creds => make_creds(Config), body => Body }). patch_custom(Config, Role, Path, Body) -> rest_helper:make_request(#{ role => Role, method => <<"PATCH">>, - port => ?TEST_PORT, path => Path, creds => make_creds(Config), body => Body }). - -%% REST handler setup -start_listener(Params) -> - rpc(mim(), mongoose_listener, start_listener, [listener_opts(Params)]). - -stop_listener(Params) -> - rpc(mim(), mongoose_listener, stop_listener, [listener_opts(Params)]). - -listener_opts(Params) -> - config([listen, http], - #{port => ?TEST_PORT, - ip_tuple => {127, 0, 0, 1}, - ip_address => "127.0.0.1", - module => ejabberd_cowboy, - handlers => [domain_handler(Params)], - transport => config([listen, http, transport], #{num_acceptors => 10})}). - -domain_handler(Params) -> - maps:merge(#{host => "localhost", path => "/api", module => mongoose_domain_handler}, - handler_opts(Params)). - -handler_opts(#{skip_auth := true}) -> - #{}; -handler_opts(_Params) -> - #{password => <<"secret">>, username => <<"admin">>}. diff --git a/big_tests/tests/service_domain_db_SUITE.erl b/big_tests/tests/service_domain_db_SUITE.erl index 54f667e150a..126f0f7ed80 100644 --- a/big_tests/tests/service_domain_db_SUITE.erl +++ b/big_tests/tests/service_domain_db_SUITE.erl @@ -20,17 +20,9 @@ delete_custom/4, patch_custom/4]). --import(domain_rest_helper, - [start_listener/1, - stop_listener/1]). - -import(domain_helper, [domain/0]). -import(config_parser_helper, [config/2]). --define(INV_PWD, <<"basic auth provided, invalid password">>). --define(NO_PWD, <<"basic auth is required">>). --define(UNWANTED_PWD, <<"basic auth provided, but not configured">>). - suite() -> require_rpc_nodes([mim, mim2, mim3]). @@ -209,11 +201,8 @@ init_per_group(db, Config) -> false -> {skip, require_rdbms} end; init_per_group(rest_with_auth, Config) -> - start_listener(#{}), + rest_helper:change_admin_creds({<<"admin">>, <<"secret">>}), [{auth_creds, valid}|Config]; -init_per_group(rest_without_auth, Config) -> - start_listener(#{skip_auth => true}), - Config; init_per_group(GroupName, Config) -> Config1 = save_service_setup_option(GroupName, Config), case ?config(service_setup, Config) of @@ -223,9 +212,7 @@ init_per_group(GroupName, Config) -> Config1. end_per_group(rest_with_auth, _Config) -> - stop_listener(#{}); -end_per_group(rest_without_auth, _Config) -> - stop_listener(#{skip_auth => true}); + rest_helper:change_admin_creds(any); end_per_group(_GroupName, Config) -> case ?config(service_setup, Config) of per_group -> teardown_service(); @@ -841,8 +828,7 @@ rest_can_delete_domain(Config) -> rest_cannot_delete_domain_without_correct_type(Config) -> rest_put_domain(Config, <<"example.db">>, <<"type1">>), - {{<<"403">>, <<"Forbidden">>}, - {[{<<"what">>, <<"wrong host type">>}]}} = + {{<<"403">>, <<"Forbidden">>}, <<"Wrong host type">>} = rest_delete_domain(Config, <<"example.db">>, <<"type2">>), {ok, _} = select_domain(mim(), <<"example.db">>). @@ -851,90 +837,88 @@ rest_delete_missing_domain(Config) -> rest_delete_domain(Config, <<"example.db">>, <<"type1">>). rest_cannot_enable_missing_domain(Config) -> - {{<<"404">>, <<"Not Found">>}, - {[{<<"what">>, <<"domain not found">>}]}} = + {{<<"404">>, <<"Not Found">>}, <<"Domain not found">>} = rest_patch_enabled(Config, <<"example.db">>, true). rest_cannot_insert_domain_twice_with_another_host_type(Config) -> rest_put_domain(Config, <<"example.db">>, <<"type1">>), - {{<<"409">>, <<"Conflict">>}, {[{<<"what">>, <<"duplicate">>}]}} = + {{<<"409">>, <<"Conflict">>}, <<"Duplicate domain">>} = rest_put_domain(Config, <<"example.db">>, <<"type2">>). rest_cannot_insert_domain_with_unknown_host_type(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, <<"unknown host type">>}]}} = + {{<<"403">>,<<"Forbidden">>}, <<"Unknown host type">>} = rest_put_domain(Config, <<"example.db">>, <<"type6">>). rest_cannot_delete_domain_with_unknown_host_type(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, <<"unknown host type">>}]}} = + {{<<"403">>,<<"Forbidden">>}, <<"Unknown host type">>} = rest_delete_domain(Config, <<"example.db">>, <<"type6">>). %% auth provided, but not configured: rest_cannot_insert_domain_if_auth_provided_but_not_configured(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?UNWANTED_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_put_domain(set_valid_creds(Config), <<"example.db">>, <<"type1">>). rest_cannot_delete_domain_if_auth_provided_but_not_configured(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?UNWANTED_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_delete_domain(set_valid_creds(Config), <<"example.db">>, <<"type1">>). rest_cannot_enable_domain_if_auth_provided_but_not_configured(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?UNWANTED_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_patch_enabled(set_valid_creds(Config), <<"example.db">>, false). rest_cannot_disable_domain_if_auth_provided_but_not_configured(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?UNWANTED_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_patch_enabled(set_valid_creds(Config), <<"example.db">>, false). rest_cannot_select_domain_if_auth_provided_but_not_configured(Config) -> - {{<<"403">>, <<"Forbidden">>}, {[{<<"what">>, ?UNWANTED_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_select_domain(set_valid_creds(Config), <<"example.db">>). %% with wrong pass: rest_cannot_insert_domain_with_wrong_pass(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?INV_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_put_domain(set_invalid_creds(Config), <<"example.db">>, <<"type1">>). rest_cannot_delete_domain_with_wrong_pass(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?INV_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_delete_domain(set_invalid_creds(Config), <<"example.db">>, <<"type1">>). rest_cannot_enable_domain_with_wrong_pass(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?INV_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_patch_enabled(set_invalid_creds(Config), <<"example.db">>, true). rest_cannot_disable_domain_with_wrong_pass(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?INV_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_patch_enabled(set_invalid_creds(Config), <<"example.db">>, false). rest_cannot_select_domain_with_wrong_pass(Config) -> - {{<<"403">>, <<"Forbidden">>}, {[{<<"what">>, ?INV_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_select_domain(set_invalid_creds(Config), <<"example.db">>). %% without auth: rest_cannot_insert_domain_without_auth(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?NO_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_put_domain(set_no_creds(Config), <<"example.db">>, <<"type1">>). rest_cannot_delete_domain_without_auth(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?NO_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_delete_domain(set_no_creds(Config), <<"example.db">>, <<"type1">>). rest_cannot_enable_domain_without_auth(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?NO_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_patch_enabled(set_no_creds(Config), <<"example.db">>, true). rest_cannot_disable_domain_without_auth(Config) -> - {{<<"403">>,<<"Forbidden">>}, {[{<<"what">>, ?NO_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_patch_enabled(set_no_creds(Config), <<"example.db">>, false). rest_cannot_select_domain_without_auth(Config) -> - {{<<"403">>, <<"Forbidden">>}, {[{<<"what">>, ?NO_PWD}]}} = + {{<<"401">>, <<"Unauthorized">>}, _} = rest_select_domain(set_no_creds(Config), <<"example.db">>). rest_cannot_disable_missing_domain(Config) -> - {{<<"404">>, <<"Not Found">>}, - {[{<<"what">>, <<"domain not found">>}]}} = + {{<<"404">>, <<"Not Found">>}, <<"Domain not found">>} = rest_patch_enabled(Config, <<"example.db">>, false). rest_can_enable_domain(Config) -> @@ -951,104 +935,85 @@ rest_can_select_domain(Config) -> rest_select_domain(Config, <<"example.db">>). rest_cannot_select_domain_if_domain_not_found(Config) -> - {{<<"404">>, <<"Not Found">>}, - {[{<<"what">>, <<"domain not found">>}]}} = + {{<<"404">>, <<"Not Found">>}, <<"Domain not found">>} = rest_select_domain(Config, <<"example.db">>). rest_cannot_put_domain_without_host_type(Config) -> - {{<<"400">>, <<"Bad Request">>}, - {[{<<"what">>, <<"'host_type' field is missing">>}]}} = + {{<<"400">>, <<"Bad Request">>}, <<"'host_type' field is missing">>} = putt_domain_with_custom_body(Config, #{}). rest_cannot_put_domain_without_body(Config) -> - {{<<"400">>,<<"Bad Request">>}, - {[{<<"what">>,<<"body is empty">>}]}} = + {{<<"400">>, <<"Bad Request">>}, <<"Invalid request body">>} = putt_domain_with_custom_body(Config, <<>>). rest_cannot_put_domain_with_invalid_json(Config) -> - {{<<"400">>,<<"Bad Request">>}, - {[{<<"what">>,<<"failed to parse JSON">>}]}} = + {{<<"400">>, <<"Bad Request">>}, <<"Invalid request body">>} = putt_domain_with_custom_body(Config, <<"{kek">>). rest_cannot_put_domain_when_it_is_static(Config) -> - {{<<"403">>, <<"Forbidden">>}, - {[{<<"what">>, <<"domain is static">>}]}} = + {{<<"403">>, <<"Forbidden">>}, <<"Domain is static">>} = rest_put_domain(Config, <<"example.cfg">>, <<"type1">>). rest_cannot_delete_domain_without_host_type(Config) -> - {{<<"400">>, <<"Bad Request">>}, - {[{<<"what">>, <<"'host_type' field is missing">>}]}} = + {{<<"400">>, <<"Bad Request">>}, <<"'host_type' field is missing">>} = delete_custom(Config, admin, <<"/domains/example.db">>, #{}). rest_cannot_delete_domain_without_body(Config) -> - {{<<"400">>,<<"Bad Request">>}, - {[{<<"what">>,<<"body is empty">>}]}} = + {{<<"400">>, <<"Bad Request">>}, <<"Invalid request body">>} = delete_custom(Config, admin, <<"/domains/example.db">>, <<>>). rest_cannot_delete_domain_with_invalid_json(Config) -> - {{<<"400">>,<<"Bad Request">>}, - {[{<<"what">>,<<"failed to parse JSON">>}]}} = + {{<<"400">>, <<"Bad Request">>}, <<"Invalid request body">>} = delete_custom(Config, admin, <<"/domains/example.db">>, <<"{kek">>). rest_cannot_delete_domain_when_it_is_static(Config) -> - {{<<"403">>, <<"Forbidden">>}, - {[{<<"what">>, <<"domain is static">>}]}} = + {{<<"403">>, <<"Forbidden">>}, <<"Domain is static">>} = rest_delete_domain(Config, <<"example.cfg">>, <<"type1">>). rest_cannot_patch_domain_without_enabled_field(Config) -> - {{<<"400">>, <<"Bad Request">>}, - {[{<<"what">>, <<"'enabled' field is missing">>}]}} = + {{<<"400">>, <<"Bad Request">>}, <<"'enabled' field is missing">>} = patch_custom(Config, admin, <<"/domains/example.db">>, #{}). rest_cannot_patch_domain_without_body(Config) -> - {{<<"400">>,<<"Bad Request">>}, - {[{<<"what">>,<<"body is empty">>}]}} = + {{<<"400">>,<<"Bad Request">>}, <<"Invalid request body">>} = patch_custom(Config, admin, <<"/domains/example.db">>, <<>>). rest_cannot_patch_domain_with_invalid_json(Config) -> - {{<<"400">>,<<"Bad Request">>}, - {[{<<"what">>,<<"failed to parse JSON">>}]}} = + {{<<"400">>,<<"Bad Request">>}, <<"Invalid request body">>} = patch_custom(Config, admin, <<"/domains/example.db">>, <<"{kek">>). %% SQL query is mocked to fail rest_insert_domain_fails_if_db_fails(Config) -> - {{<<"500">>, <<"Internal Server Error">>}, - {[{<<"what">>, <<"database error">>}]}} = + {{<<"500">>, <<"Internal Server Error">>}, <<"Database error">>} = rest_put_domain(Config, <<"example.db">>, <<"type1">>). rest_insert_domain_fails_if_service_disabled(Config) -> service_disabled(mim()), - {{<<"403">>, <<"Forbidden">>}, - {[{<<"what">>, <<"service disabled">>}]}} = + {{<<"403">>, <<"Forbidden">>}, <<"Service disabled">>} = rest_put_domain(Config, <<"example.db">>, <<"type1">>). %% SQL query is mocked to fail rest_delete_domain_fails_if_db_fails(Config) -> - {{<<"500">>, <<"Internal Server Error">>}, - {[{<<"what">>, <<"database error">>}]}} = + {{<<"500">>, <<"Internal Server Error">>}, <<"Database error">>} = rest_delete_domain(Config, <<"example.db">>, <<"type1">>). rest_delete_domain_fails_if_service_disabled(Config) -> service_disabled(mim()), - {{<<"403">>, <<"Forbidden">>}, - {[{<<"what">>, <<"service disabled">>}]}} = + {{<<"403">>, <<"Forbidden">>}, <<"Service disabled">>} = rest_delete_domain(Config, <<"example.db">>, <<"type1">>). %% SQL query is mocked to fail rest_enable_domain_fails_if_db_fails(Config) -> - {{<<"500">>, <<"Internal Server Error">>}, - {[{<<"what">>, <<"database error">>}]}} = + {{<<"500">>, <<"Internal Server Error">>}, <<"Database error">>} = rest_patch_enabled(Config, <<"example.db">>, true). rest_enable_domain_fails_if_service_disabled(Config) -> service_disabled(mim()), - {{<<"403">>, <<"Forbidden">>}, - {[{<<"what">>, <<"service disabled">>}]}} = + {{<<"403">>, <<"Forbidden">>}, <<"Service disabled">>} = rest_patch_enabled(Config, <<"example.db">>, true). rest_cannot_enable_domain_when_it_is_static(Config) -> - {{<<"403">>, <<"Forbidden">>}, - {[{<<"what">>, <<"domain is static">>}]}} = + {{<<"403">>, <<"Forbidden">>}, <<"Domain is static">>} = rest_patch_enabled(Config, <<"example.cfg">>, true). rest_delete_domain_cleans_data_from_mam(Config) -> From 9082e6e30ba500766800836ae0f2e83e40f0093f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Chrz=C4=85szcz?= Date: Thu, 22 Sep 2022 11:46:11 +0200 Subject: [PATCH 6/6] Improve test coverage Cover the unlikely case of an invalid domain name. --- big_tests/tests/service_domain_db_SUITE.erl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/big_tests/tests/service_domain_db_SUITE.erl b/big_tests/tests/service_domain_db_SUITE.erl index 126f0f7ed80..dad9c03f78e 100644 --- a/big_tests/tests/service_domain_db_SUITE.erl +++ b/big_tests/tests/service_domain_db_SUITE.erl @@ -148,6 +148,7 @@ rest_cases() -> rest_cannot_put_domain_without_host_type, rest_cannot_put_domain_without_body, rest_cannot_put_domain_with_invalid_json, + rest_cannot_put_domain_with_invalid_name, rest_cannot_put_domain_when_it_is_static, rest_cannot_delete_domain_without_host_type, rest_cannot_delete_domain_without_body, @@ -950,6 +951,10 @@ rest_cannot_put_domain_with_invalid_json(Config) -> {{<<"400">>, <<"Bad Request">>}, <<"Invalid request body">>} = putt_domain_with_custom_body(Config, <<"{kek">>). +rest_cannot_put_domain_with_invalid_name(Config) -> + {{<<"400">>, <<"Bad Request">>}, <<"Invalid domain name">>} = + rest_put_domain(Config, <<"%f3">>, <<"type1">>). % nameprep fails for ASCII code 243 + rest_cannot_put_domain_when_it_is_static(Config) -> {{<<"403">>, <<"Forbidden">>}, <<"Domain is static">>} = rest_put_domain(Config, <<"example.cfg">>, <<"type1">>).