GraphQL - Support admin per domain #3633
Conversation
Premwoik
force-pushed
the
graphql/support-admin-per-domain
branch
from
4809fd0
to
6127638
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Codecov Report
@@ Coverage Diff @@
## feature/graphql #3633 +/- ##
===================================================
+ Coverage 81.64% 81.70% +0.05%
===================================================
Files 478 479 +1
Lines 33126 33226 +100
===================================================
+ Hits 27047 27146 +99
- Misses 6079 6080 +1
Continue to review full report at Codecov.
|
This comment was marked as outdated.
This comment was marked as outdated.
Premwoik
force-pushed
the
graphql/support-admin-per-domain
branch
from
661ab1f
to
995f125
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Premwoik
force-pushed
the
graphql/support-admin-per-domain
branch
from
d5d4198
to
5d40435
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Premwoik
force-pushed
the
graphql/support-admin-per-domain
branch
from
5d40435
to
7a9c450
Compare
|
small_tests_24 / small_tests / 7a9c450 small_tests_23 / small_tests / 7a9c450 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 7a9c450 bosh_SUITE:essential:accept_higher_hold_value{error,
{{assertEqual,
[{module,bosh_SUITE},
{line,251},
{expression,"get_bosh_sessions ( )"},
{expected,[]},
{value,
[{bosh_session,<<"4028267dfa18447ee7428e067c8d76e06d81aadb">>,
<8641.6029.0>}]}]},
[{bosh_SUITE,accept_higher_hold_value,1,
[{file,"/home/circleci/project/big_tests/tests/bosh_SUITE.erl"},
{line,251}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}}dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 7a9c450 dynamic_domains_mysql_redis_24 / mysql_redis / 7a9c450 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 7a9c450 muc_SUITE:hibernation:hibernated_room_can_be_queried_for_archive{error,{{assertion_failed,assert,is_groupchat_message,
[<<"Restorable message">>],
undefined,"undefined"},
[{escalus_new_assert,assert_true,2,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
{line,84}]},
{muc_SUITE,wait_for_mam_result,3,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4383}]},
{muc_SUITE,'-hibernated_room_can_be_queried_for_archive/1-fun-0-',3,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4124}]},
{escalus_story,story,4,
[{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{muc_SUITE,hibernated_room_can_be_queried_for_archive,1,
[{file,"/home/circleci/project/big_tests/tests/muc_SUITE.erl"},
{line,4120}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}}ldap_mnesia_23 / ldap_mnesia / 7a9c450 ldap_mnesia_24 / ldap_mnesia / 7a9c450 internal_mnesia_24 / internal_mnesia / 7a9c450 pgsql_mnesia_24 / pgsql_mnesia / 7a9c450 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 7a9c450 pgsql_mnesia_23 / pgsql_mnesia / 7a9c450 mysql_redis_24 / mysql_redis / 7a9c450 mssql_mnesia_24 / odbc_mssql_mnesia / 7a9c450 riak_mnesia_24 / riak_mnesia / 7a9c450 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 7a9c450 |
| transaction(fun(Pool) -> | ||
| case select_domain_admin(Domain) of | ||
| {ok, _} -> | ||
| update_domain_admin(Pool, Domain, Password), |
There was a problem hiding this comment.
Please cover with tests as there is a bug here - it would swap domain and password.
| %acc_path(Field, Acc) -> | ||
| %Acc#{path => [Field]}. |
| acc([], Type) when is_atom(Type) -> false; | ||
| acc(Invalid, Type) when is_atom(Type) -> {true, #{type => Type, path => [], invalid => Invalid}}. | ||
|
|
||
| acc_path(_Field, false) -> false; |
There was a problem hiding this comment.
This function is hard to understand because it accepts too many different types: boolean, tuple, map. Function name does not help either.
| @@ -3,11 +3,20 @@ schema{ | |||
| mutation: UserMutation | |||
| } | |||
|
|
|||
| directive @protected on FIELD_DEFINITION | OBJECT | INTERFACE | |||
| #directive @protected on FIELD_DEFINITION | OBJECT | INTERFACE | |||
This upgrade graphql to version with field() type fix.
| -spec check_fields(map(), map(), [field()]) -> [fail_acc()]. | ||
| check_fields(Ctx, Params, Fields) -> | ||
| % Return empty list when permissions are valid. | ||
| lists:flatten(lists:filtermap(fun(F) -> check_field(F, Ctx, Params) end, Fields)). |
There was a problem hiding this comment.
Just an idea: maybe it would be cleaner to use lists:flatmap instead and always return a list form check_field?
There was a problem hiding this comment.
I changed it completely. Now it stops checking at the first error that occurred. Previously only the first error was thrown anyway.
Premwoik
force-pushed
the
graphql/support-admin-per-domain
branch
from
7a9c450
to
d602b0a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
…omain_admin Minor changes: - rename "domain_admin" table to "domain_admins", - fix password update operation.
Premwoik
force-pushed
the
graphql/support-admin-per-domain
branch
from
c54e3c6
to
2bf7c82
Compare
|
small_tests_24 / small_tests / 2bf7c82 small_tests_23 / small_tests / 2bf7c82 dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 2bf7c82 dynamic_domains_pgsql_mnesia_23 / pgsql_mnesia / 2bf7c82 dynamic_domains_mysql_redis_24 / mysql_redis / 2bf7c82 ldap_mnesia_24 / ldap_mnesia / 2bf7c82 ldap_mnesia_23 / ldap_mnesia / 2bf7c82 dynamic_domains_mssql_mnesia_24 / odbc_mssql_mnesia / 2bf7c82 internal_mnesia_24 / internal_mnesia / 2bf7c82 pgsql_mnesia_24 / pgsql_mnesia / 2bf7c82 pgsql_mnesia_23 / pgsql_mnesia / 2bf7c82 mysql_redis_24 / mysql_redis / 2bf7c82 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 2bf7c82 bosh_SUITE:essential_https:accept_higher_hold_value{error,
{{assertEqual,
[{module,bosh_SUITE},
{line,251},
{expression,"get_bosh_sessions ( )"},
{expected,[]},
{value,
[{bosh_session,<<"45a58ea2e708060fc49497662cd24303a9f4e06c">>,
<8653.5525.0>}]}]},
[{bosh_SUITE,accept_higher_hold_value,1,
[{file,"/home/circleci/project/big_tests/tests/bosh_SUITE.erl"},
{line,251}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1292}]},
{test_server,run_test_case_eval,9,
[{file,"test_server.erl"},{line,1224}]}]}}mssql_mnesia_24 / odbc_mssql_mnesia / 2bf7c82 riak_mnesia_24 / riak_mnesia / 2bf7c82 elasticsearch_and_cassandra_24 / elasticsearch_and_cassandra_mnesia / 2bf7c82 |
This PR addresses MIM-1639 and adds support for admin per domain. It introduces the third GraphQL listener that exposes the admin schema, but queries are restricted to the authorized domain admin. That means that the admin authorized for the
localhostdomain cannot execute queries for other domains (e.g.localhost2).The domain admin credentials are stored in the new table
domain_admin. Currently, the password is stored in plain form. The logic to manage domain admin was added to themongoose_domain_apimodule.The permissions, whether the given domain admin can execute queries are checked in the
mongoose_graphql_permissionsmodule. This module ensures that domain-protected field arguments cannot contain unauthorized domains.The domain-protected field has to contain the following directive in the admin schema:
The directive
typeargument valueDOMAINmeans that the field must be domain checked, and theargsargument stores field arguments to check.The domain admin request authorization basic token should be encoded from the pattern
admin@$domain:$password.Fix the field() type
The
field()type from the graphql core library was incorrect and to fix the dialyzer it had to be changed. I change the graphql dependency to the branch with the fix. It should be changed back after we merge the fix.Premwoik/graphql-erlang#2
Next steps
Questions
Answer: There is no such need for now.