GraphQL error handling in MUC Light #3881

chrzaszcz · 2022-11-30T12:00:55Z

The main goal is to handle the error cases:

Non-existent users shouldn't be room owners or message senders, and they shouldn't be able to have blocking lists.
'Room not found' should be returned if the room does not exist.

Other functional changes:

Name and subject are not required, because they shouldn't be. They have defaults, and they can be removed by the used in a custom room config schema.
Only room owner should be able to kick room members.
Room JID is always bare in the schema. User JID's are not required to be bare if this doesn't break anything. The resources are mostly ignored.
Update required modules and protected domains in the schema.

Operations are broken into reusable steps, and a fold operation is used to iterate over them.

codecov · 2022-11-30T12:10:28Z

Codecov Report

Base: 83.15% // Head: 83.15% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (9788b67) compared to base (29e8228).
Patch coverage: 99.38% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3881   +/-   ##
=======================================
  Coverage   83.15%   83.15%           
=======================================
  Files         535      535           
  Lines       34202    34220   +18     
=======================================
+ Hits        28440    28456   +16     
- Misses       5762     5764    +2

Impacted Files	Coverage Δ
src/muc_light/mod_muc_light_api.erl	`98.63% <99.12%> (+1.45%)`	⬆️
...dmin/mongoose_graphql_muc_light_admin_mutation.erl	`100.00% <100.00%> (ø)`
...l/admin/mongoose_graphql_muc_light_admin_query.erl	`100.00% <100.00%> (ø)`
src/graphql/mongoose_graphql_muc_light_helper.erl	`100.00% <100.00%> (ø)`
.../user/mongoose_graphql_muc_light_user_mutation.erl	`100.00% <100.00%> (ø)`
...hql/user/mongoose_graphql_muc_light_user_query.erl	`100.00% <100.00%> (ø)`
...ongoose_admin_api/mongoose_admin_api_muc_light.erl	`100.00% <100.00%> (ø)`
.../mongoose_client_api/mongoose_client_api_rooms.erl	`98.50% <100.00%> (+0.02%)`	⬆️
..._client_api/mongoose_client_api_rooms_messages.erl	`100.00% <100.00%> (ø)`
...ose_client_api/mongoose_client_api_rooms_users.erl	`100.00% <100.00%> (ø)`
... and 12 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

The goal is to handle the error cases: - Non-existing users shouldn't be room owners or message senders - 'Room not found' should be returned if the room does not exist Other functional changes: - Name and subject are not required, because they shouldn't be. They have defaults, and they can be removed in a custom config schema. - Only room owner should be able to kick room members. Operations are broken into reusable steps, and a fold operation is used to iterate over them.

- Handle the new error cases from the API module. - Pass name and subject in the config map. - Remove permission checking, which is done in mod_muc_light_api now.

- Update some operation names - Pass room name and subject in a map - Convert printed JIDs to binaries

- Protect 'mucDomain' on room creation. The reason is that a domain admin shouldn't be able to create a room at a different domain. The operation directly calls the MUC Light backend. - Room JID is always bare - Require 'mod_mam_muc' for message archive - Make name and subject optional, as they should be. they have default values, and the user could even remove them from the config schema.

- Run the archive-related tests with and without MAM - Test the new error conditions

Handle new error conditions.

mongoose-im · 2022-12-01T14:55:43Z

small_tests_24 / small_tests / 9788b67
Reports root / small

small_tests_25 / small_tests / 9788b67
Reports root / small

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 9788b67
Reports root/ big
OK: 4124 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

ldap_mnesia_24 / ldap_mnesia / 9788b67
Reports root/ big
OK: 2172 / Failed: 0 / User-skipped: 821 / Auto-skipped: 0

dynamic_domains_mysql_redis_25 / mysql_redis / 9788b67
Reports root/ big
OK: 4098 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 9788b67
Reports root/ big
OK: 4124 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 9788b67
Reports root/ big
OK: 4124 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

internal_mnesia_25 / internal_mnesia / 9788b67
Reports root/ big
OK: 2314 / Failed: 0 / User-skipped: 679 / Auto-skipped: 0

ldap_mnesia_25 / ldap_mnesia / 9788b67
Reports root/ big
OK: 2172 / Failed: 0 / User-skipped: 821 / Auto-skipped: 0

elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 9788b67
Reports root/ big
OK: 2672 / Failed: 0 / User-skipped: 660 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 9788b67
Reports root/ big
OK: 4498 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 9788b67
Reports root/ big
OK: 2510 / Failed: 0 / User-skipped: 652 / Auto-skipped: 0

pgsql_mnesia_25 / pgsql_mnesia / 9788b67
Reports root/ big
OK: 4498 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

mysql_redis_25 / mysql_redis / 9788b67
Reports root/ big
OK: 4484 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0

mssql_mnesia_25 / odbc_mssql_mnesia / 9788b67
Reports root/ big
OK: 4509 / Failed: 1 / User-skipped: 97 / Auto-skipped: 0

pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription

{error,
  {{badmatch,
     [{xmlel,<<"message">>,
        [{<<"from">>,
        <<"alice_unsubscribe_after_presence_unsubscription_2689@localhost">>},
         {<<"to">>,
        <<"bob_unsubscribe_after_presence_unsubscription_2689@localhost/res1">>},
         {<<"type">>,<<"headline">>}],
        [{xmlel,<<"event">>,
           [{<<"xmlns">>,
           <<"http://jabber.org/protocol/pubsub#event">>}],
           [{xmlel,<<"items">>,
            [{<<"node">>,<<"se0m0Ht6TC3DYzDnzrmKSQ==">>}],
            [{xmlel,<<"item">>,
               [{<<"id">>,<<"salmon">>}],
               [{xmlel,<<"entry">>,
                  [{<<"xmlns">>,
                  <<"http://www.w3.org/2005/Atom">>}],
                  []}]}]}]},
         {xmlel,<<"headers">>,
           [{<<"xmlns">>,<<"http://jabber.org/protocol/shim">>}],
           []}]}]},
   [{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
      [{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
       {line,384}]},
    {escalus_story,story,4,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
       {line,72}]},
    {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_server.erl"},{line,1291}]},
    {test_server,run_test_case_eval,9,
      [{file,"test_server.erl"},{line,1223}]}]}}

Report log

JanuszJakubiec

Looks good

jacekwegr

Looks good to me! 👍