Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly pass connection options to Fusco #3426

Merged
merged 1 commit into from
Dec 1, 2021
Merged

Conversation

gustawlippa
Copy link
Contributor

@gustawlippa gustawlippa commented Dec 1, 2021

The options are not used currently, which is critical when configuring TLS for HTTP auth.
Fusco did not complain, as the wpool library did not call the exported start/start_link functions, which verified options (https://github.com/inaka/worker_pool/blob/main/src/wpool_process.erl#L45, https://github.com/esl/fusco/blob/master/src/fusco.erl#L84).

Tested manually, without the patch, TLS is silently not verified and connection (if the MIM client certs are not rejected by the auth service) is established. With the patch, errors appear in logs when connecting to a service which has incorrect certificate.

The options were not used, which was critical when configuring TLS.
Fusco did not complain, as the wpool library called did not call the
exported start/start_link functions, which verified options.
(https://github.com/inaka/worker_pool/blob/main/src/wpool_process.erl#L45).
@codecov
Copy link

codecov bot commented Dec 1, 2021

Codecov Report

Merging #3426 (0466d65) into master (5242809) will decrease coverage by 63.27%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #3426       +/-   ##
===========================================
- Coverage   80.78%   17.51%   -63.28%     
===========================================
  Files         414      414               
  Lines       32336    32285       -51     
===========================================
- Hits        26124     5656    -20468     
- Misses       6212    26629    +20417     
Impacted Files Coverage Δ
src/wpool/mongoose_wpool_http.erl 88.00% <100.00%> (-4.00%) ⬇️
src/mam/mam_jid.erl 0.00% <0.00%> (-100.00%) ⬇️
src/mod_aws_sns.erl 0.00% <0.00%> (-100.00%) ⬇️
src/mam/mam_decoder.erl 0.00% <0.00%> (-100.00%) ⬇️
src/mam/mam_encoder.erl 0.00% <0.00%> (-100.00%) ⬇️
src/mam/mam_jid_rfc.erl 0.00% <0.00%> (-100.00%) ⬇️
src/mam/mam_message.erl 0.00% <0.00%> (-100.00%) ⬇️
src/gen_iq_component.erl 0.00% <0.00%> (-100.00%) ⬇️
src/ejabberd_c2s_state.erl 0.00% <0.00%> (-100.00%) ⬇️
src/sasl/cyrsasl_oauth.erl 0.00% <0.00%> (-100.00%) ⬇️
... and 358 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5242809...0466d65. Read the comment docs.

@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 1, 2021

small_tests_24 / small_tests / 0466d65
Reports root / small


small_tests_23 / small_tests / 0466d65
Reports root / small


dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 0466d65
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / 0466d65
Reports root/ big
OK: 2708 / Failed: 1 / User-skipped: 203 / Auto-skipped: 0

mam_SUITE:rdbms_async_cache_muc_all:muc04:muc_text_search_request
{error,{test_case_failed,"Respond size is 2, 3 is expected."}}

Report log


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 0466d65
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 0466d65
Reports root/ big
OK: 2726 / Failed: 0 / User-skipped: 186 / Auto-skipped: 0


ldap_mnesia_23 / ldap_mnesia / 0466d65
Reports root/ big
OK: 1502 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / 0466d65
Reports root/ big
OK: 1502 / Failed: 0 / User-skipped: 386 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 0466d65
Reports root/ big
OK: 1585 / Failed: 2 / User-skipped: 301 / Auto-skipped: 0

amp_big_SUITE:basic:notify_deliver_to_online_user_recipient_privacy_test
{error,
  {test_case_failed,
    {has_stanzas_but_shouldnt,
      {client,
        <<"bOb_notify_deliver_to_online_user_recipient_privacy_test_61.46551@localhost/res1">>,
        escalus_tcp,<0.3042.0>,
        [{event_manager,<0.2661.0>},
         {server,<<"localhost">>},
         {username,
           <<"bOb_notify_deliver_to_online_user_recipient_privacy_test_61.46551">>},
         {resource,<<"res1">>}],
        [{event_client,
           [{event_manager,<0.2661.0>},
            {server,<<"localhost">>},
            {username,
              <<"bOb_notify_deliver_to_online_user_recipient_privacy_test_61.46551">>},
            {resource,<<"res1">>}]},
         {resource,<<"res1">>},
         {username,
           <<"bOb_notify_deliver_to_online_user_recipient_privacy_test_61.46551">>},
         {server,<<"localhost">>},
         {host,<<"localhost">>},
         {port,5222},
         {auth,{escalus_auth,auth_plain}},
         {wspath,undefined},
         {username,
           <<"bOb_notify_deliver_to_online_user_recipient_privacy_test_61.46551">>},
         {server,<<"localhost">>},
         {password,<<"makrolika">>},
         {stream_id,<<"96e00598129164e2">>}]},
      [{xmlel,<<"stream:error">>,[],
         [{xmlel,<<"conflict">>,
            [{<<"xmlns">>,
            <<"urn:ietf:params:xml:ns:xmpp-streams">>}],
            []},
          {xmlel,<<"text">>,
            [{<<"xml:lang">>,<<"en">>},
             {<<"xmlns">>,
            <<"urn:ietf:params:xml:ns:xmpp-stre...

Report log

amp_big_SUITE:basic:notify_deliver_to_online_user_recipient_privacy_test
{error,
  {test_case_failed,
    {has_stanzas_but_shouldnt,
      {client,
        <<"alicE_notify_deliver_to_online_user_recipient_privacy_test_61.46551@localhost/res1">>,
        escalus_tcp,<0.3017.0>,
        [{event_manager,<0.2676.0>},
         {server,<<"localhost">>},
         {username,
           <<"alicE_notify_deliver_to_online_user_recipient_privacy_test_61.46551">>},
         {resource,<<"res1">>}],
        [{event_client,
           [{event_manager,<0.2676.0>},
            {server,<<"localhost">>},
            {username,
              <<"alicE_notify_deliver_to_online_user_recipient_privacy_test_61.46551">>},
            {resource,<<"res1">>}]},
         {resource,<<"res1">>},
         {username,
           <<"alicE_notify_deliver_to_online_user_recipient_privacy_test_61.46551">>},
         {server,<<"localhost">>},
         {host,<<"localhost">>},
         {port,5222},
         {auth,{escalus_auth,auth_plain}},
         {wspath,undefined},
         {username,
           <<"alicE_notify_deliver_to_online_user_recipient_privacy_test_61.46551">>},
         {server,<<"localhost">>},
         {password,<<"matygrysa">>},
         {stream_id,<<"946115e1e1a91605">>}]},
      [{xmlel,<<"stream:error">>,[],
         [{xmlel,<<"conflict">>,
            [{<<"xmlns">>,
            <<"urn:ietf:params:xml:ns:xmpp-streams">>}],
            []},
          {xmlel,<<"text">>,
            [{<<"xml:lang">>,<<"en">>},
             {<<"xmlns">>,
            <<"urn:ietf:params:xml:ns...

Report log


pgsql_mnesia_24 / pgsql_mnesia / 0466d65
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


pgsql_mnesia_23 / pgsql_mnesia / 0466d65
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


mssql_mnesia_24 / odbc_mssql_mnesia / 0466d65
Reports root/ big
OK: 3113 / Failed: 0 / User-skipped: 195 / Auto-skipped: 0


elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 0466d65
Reports root/ big
OK: 1880 / Failed: 0 / User-skipped: 313 / Auto-skipped: 0


mysql_redis_24 / mysql_redis / 0466d65
Reports root/ big
OK: 3108 / Failed: 0 / User-skipped: 200 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 0466d65
Reports root/ big
OK: 1726 / Failed: 0 / User-skipped: 314 / Auto-skipped: 0


dynamic_domains_mysql_redis_24 / mysql_redis / 0466d65
Reports root/ big
OK: 2709 / Failed: 0 / User-skipped: 203 / Auto-skipped: 0


internal_mnesia_24 / internal_mnesia / 0466d65
Reports root/ big
OK: 1587 / Failed: 0 / User-skipped: 301 / Auto-skipped: 0

@codecov
Copy link

codecov bot commented Dec 1, 2021

Codecov Report

Merging #3426 (0466d65) into master (5242809) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3426   +/-   ##
=======================================
  Coverage   80.78%   80.78%           
=======================================
  Files         414      414           
  Lines       32336    32336           
=======================================
  Hits        26124    26124           
  Misses       6212     6212           
Impacted Files Coverage Δ
src/wpool/mongoose_wpool_http.erl 92.00% <100.00%> (ø)
src/mod_roster_mnesia.erl 72.72% <0.00%> (-21.22%) ⬇️
src/ejabberd_c2s.erl 88.89% <0.00%> (-0.23%) ⬇️
src/mod_muc_room.erl 77.26% <0.00%> (+0.17%) ⬆️
...c/global_distrib/mod_global_distrib_server_mgr.erl 77.40% <0.00%> (+0.56%) ⬆️
src/mod_muc.erl 75.00% <0.00%> (+0.67%) ⬆️
src/logger/mongoose_log_filter.erl 79.45% <0.00%> (+1.36%) ⬆️
...bal_distrib/mod_global_distrib_hosts_refresher.erl 77.35% <0.00%> (+1.88%) ⬆️
src/pubsub/mod_pubsub_cache_rdbms.erl 91.17% <0.00%> (+2.94%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5242809...0466d65. Read the comment docs.

Copy link
Collaborator

@NelsonVides NelsonVides left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thanks!

@NelsonVides NelsonVides merged commit a540cd3 into master Dec 1, 2021
@NelsonVides NelsonVides deleted the fix-http-auth-fusco branch December 1, 2021 16:21
@Premwoik Premwoik modified the milestone: 5.1.0 May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants