Skip to content

Commit

Permalink
Remove mod_disco extra_domains config key
Browse files Browse the repository at this point in the history
This option was used to make discoverable components that for a user
might not have been discoverable given the domain was unrelated. This
was so decided over a decade ago before MongooseIM 1.0.0, but I'd judge
a better solution is to leave components as either entirely discoverable
or entirely hidden by default, as components are usually dynamic and at
the same time fully controllable at the deployment, and we can now have
an infinite amount of domains for which a component can become a
subdomain and hard-coding "extra_domains" for each static or dynamic
domain is not feasible.
  • Loading branch information
NelsonVides committed Dec 31, 2024
1 parent d10da98 commit 9afc32c
Show file tree
Hide file tree
Showing 6 changed files with 7 additions and 49 deletions.
18 changes: 2 additions & 16 deletions big_tests/tests/disco_and_caps_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ caps_test_cases() ->
user_can_query_server_caps_via_disco].

extra_feature_test_cases() ->
[user_can_query_extra_domains,
user_can_query_server_info].
[user_can_query_server_info].

init_per_suite(C) ->
instrument_helper:start(instrument_helper:declared_events(mod_disco)),
Expand Down Expand Up @@ -155,15 +154,6 @@ user_cannot_query_friend_resources_with_unknown_node(Config) ->
escalus:assert(is_stanza_from, [BobJid], Stanza)
end).

user_can_query_extra_domains(Config) ->
escalus:fresh_story(Config, [{alice, 1}], fun(Alice) ->
Server = escalus_client:server(Alice),
escalus:send(Alice, escalus_stanza:service_discovery(Server)),
Stanza = escalus:wait_for_stanza(Alice),
escalus:assert(has_service, [extra_domain()], Stanza),
escalus:assert(is_stanza_from, [domain()], Stanza)
end).

user_can_query_server_features(Config) ->
escalus:fresh_story(Config, [{alice, 1}], fun(Alice) ->
Server = escalus_client:server(Alice),
Expand Down Expand Up @@ -209,8 +199,7 @@ required_modules(disco_with_extra_features) ->
[{mod_disco, mod_config(mod_disco, extra_disco_opts())}].

extra_disco_opts() ->
#{extra_domains => [extra_domain()],
server_info => [server_info(abuse, #{}),
#{server_info => [server_info(abuse, #{}),
server_info(admin, #{modules => [mod_disco]}),
server_info(sales, #{modules => [mod_pubsub]})]}.

Expand All @@ -219,9 +208,6 @@ get_form_fields(Stanza) ->
{element_with_ns, <<"x">>, ?NS_DATA_FORMS},
{element, <<"field">>}]).

extra_domain() ->
<<"eXtra.example.com">>.

server_info(Type, Extra) ->
maps:merge(#{name => name(Type), urls => urls(Type)}, Extra).

Expand Down
9 changes: 0 additions & 9 deletions doc/modules/mod_disco.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@ Implements [XEP-0030: Service Discovery](http://xmpp.org/extensions/xep-0030.htm
Strategy to handle incoming stanzas. For details, please refer to
[IQ processing policies](../configuration/Modules.md#iq-processing-policies).

### `modules.mod_disco.extra_domains`
* **Syntax:** array of strings, valid domain names
* **Default:** no extra domains
* **Example:** `extra_domains = ["custom_domain"]`

Adds domains that are not registered with other means to a local item announcement (response to `http://jabber.org/protocol/disco#items` IQ get).
Please note that `mod_disco` doesn't verify these domains, so if no handlers are registered later for them, a client will receive a `service-unavailable` error for every stanza sent to one of these hosts.

### `modules.mod_disco.server_info`
* **Syntax:** array of tables described below
* **Default:** no additional server info
Expand Down Expand Up @@ -50,7 +42,6 @@ will still receive full disco results.
```toml
[modules.mod_disco]
iqdisc.type = "one_queue"
extra_domains = ["some_domain", "another_domain"]
server_info = [
{name = "abuse-address", urls = ["[email protected]"]},
{name = "friendly-spirits", urls = ["spirit1@localhost", "spirit2@localhost"], modules = ["mod_muc", "mod_disco"]}
Expand Down
14 changes: 3 additions & 11 deletions src/mod_disco.erl
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,11 @@ iq_handlers() ->
-spec config_spec() -> mongoose_config_spec:config_section().
config_spec() ->
#section{
items = #{<<"extra_domains">> => #list{items = #option{type = binary,
validate = domain}},
<<"server_info">> => #list{items = server_info_spec()},
items = #{<<"server_info">> => #list{items = server_info_spec()},
<<"users_can_see_hidden_services">> => #option{type = boolean},
<<"iqdisc">> => mongoose_config_spec:iqdisc()
},
defaults = #{<<"extra_domains">> => [],
<<"server_info">> => [],
defaults = #{<<"server_info">> => [],
<<"users_can_see_hidden_services">> => true,
<<"iqdisc">> => one_queue}
}.
Expand Down Expand Up @@ -229,8 +226,7 @@ disco_local_items(Acc = #{host_type := HostType, from_jid := From, to_jid := To,
ReturnHidden = should_return_hidden(HostType, From),
Subdomains = get_subdomains(To#jid.lserver),
Components = get_external_components(ReturnHidden),
ExtraDomains = get_extra_domains(HostType),
Domains = Subdomains ++ Components ++ ExtraDomains,
Domains = Subdomains ++ Components,
{ok, mongoose_disco:add_items([#{jid => Domain} || Domain <- Domains], Acc)};
disco_local_items(Acc, _, _) ->
{ok, Acc}.
Expand Down Expand Up @@ -288,10 +284,6 @@ get_subdomains(LServer) ->
get_external_components(ReturnHidden) ->
mongoose_component:dirty_get_all_components(ReturnHidden).

-spec get_extra_domains(mongooseim:host_type()) -> [jid:lserver()].
get_extra_domains(HostType) ->
gen_mod:get_module_opt(HostType, ?MODULE, extra_domains).

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

-spec is_presence_subscribed(jid:jid(), jid:jid()) -> boolean().
Expand Down
6 changes: 2 additions & 4 deletions test/common/config_parser_helper.erl
Original file line number Diff line number Diff line change
Expand Up @@ -501,8 +501,7 @@ all_modules() ->
no_stanzaid_element => true}),
mod_disco =>
mod_config(mod_disco,
#{extra_domains => [<<"some_domain">>, <<"another_domain">>],
server_info =>
#{server_info =>
[#{name => <<"abuse-address">>,
urls => [<<"[email protected]">>]},
#{name => <<"friendly-spirits">>,
Expand Down Expand Up @@ -885,8 +884,7 @@ default_mod_config(mod_csi) ->
default_mod_config(mod_carboncopy) ->
#{iqdisc => no_queue};
default_mod_config(mod_disco) ->
#{extra_domains => [], server_info => [],
users_can_see_hidden_services => true, iqdisc => one_queue};
#{server_info => [], users_can_see_hidden_services => true, iqdisc => one_queue};
default_mod_config(mod_extdisco) ->
#{iqdisc => no_queue, service => []};
default_mod_config(mod_global_distrib) ->
Expand Down
8 changes: 0 additions & 8 deletions test/config_parser_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -1565,11 +1565,6 @@ mod_disco(_Config) ->
T(<<"users_can_see_hidden_services">>, true)),
?cfgh(P ++ [users_can_see_hidden_services], false,
T(<<"users_can_see_hidden_services">>, false)),
%% extra_domains are binaries
?cfgh(P ++ [extra_domains], [<<"localhost">>, <<"erlang-solutions.com">>],
T(<<"extra_domains">>, [<<"localhost">>, <<"erlang-solutions.com">>])),
?cfgh(P ++ [extra_domains], [],
T(<<"extra_domains">>, [])),
Info = #{<<"name">> => <<"abuse-address">>,
<<"urls">> => [<<"[email protected]">>]},
SpiritUrls = [<<"spirit1@localhost">>, <<"spirit2@localhost">>],
Expand All @@ -1581,9 +1576,6 @@ mod_disco(_Config) ->
<<"modules">> => [<<"mod_muc">>, <<"mod_disco">>]}])),
?errh(T(<<"users_can_see_hidden_services">>, 1)),
?errh(T(<<"users_can_see_hidden_services">>, <<"true">>)),
?errh(T(<<"extra_domains">>, [<<"user@localhost">>])),
?errh(T(<<"extra_domains">>, [1])),
?errh(T(<<"extra_domains">>, <<"domains domains domains">>)),
?errh(T(<<"server_info">>, [Info#{<<"name">> => 1}])),
?errh(T(<<"server_info">>, [Info#{<<"name">> => <<"">>}])),
?errh(T(<<"server_info">>, [Info#{<<"modules">> => <<"roll">>}])),
Expand Down
1 change: 0 additions & 1 deletion test/config_parser_SUITE_data/modules.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@

[modules.mod_disco]
iqdisc.type = "one_queue"
extra_domains = ["some_domain", "another_domain"]
server_info = [
{name = "abuse-address", urls = ["[email protected]"]},
{name = "friendly-spirits", urls = ["spirit1@localhost", "spirit2@localhost"], modules = ["mod_muc", "mod_disco"]}
Expand Down

0 comments on commit 9afc32c

Please sign in to comment.