Get rid of store_type/1 in auth backends #2254
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chrzaszcz
force-pushed
the
no-store-type
branch
2 times, most recently
from
3a43817
to
e57d1e3
Compare
|
6295.1 / Erlang 19.3 / small_tests / b09e9f3 6295.2 / Erlang 19.3 / internal_mnesia / b09e9f3 sm_SUITE:parallel:subscription_requests_are_buffered_properly{error,{{badmatch,false},
[{escalus_session,stream_management,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_session.erl"},
{line,227}]},
{escalus_connection,connection_step,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,134}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1263}]},
{escalus_connection,start,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,118}]},
{sm_SUITE,'-subscription_requests_are_buffered_properly/1-fun-3-',6,
[{file,"sm_SUITE.erl"},{line,848}]},
{escalus_story,story,4,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1529}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1045}]}]}}sm_SUITE:parallel:subscription_requests_are_buffered_properly{error,{{badmatch,false},
[{escalus_session,stream_management,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_session.erl"},
{line,227}]},
{escalus_connection,connection_step,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,134}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1263}]},
{escalus_connection,start,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,118}]},
{sm_SUITE,'-subscription_requests_are_buffered_properly/1-fun-3-',6,
[{file,"sm_SUITE.erl"},{line,848}]},
{escalus_story,story,4,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1529}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1045}]}]}}6295.6 / Erlang 19.3 / elasticsearch_and_cassandra_mnesia / b09e9f3 6295.5 / Erlang 19.3 / ldap_mnesia / b09e9f3 6295.3 / Erlang 19.3 / mysql_redis / b09e9f3 6295.4 / Erlang 19.3 / odbc_mssql_mnesia / b09e9f3 6295.8 / Erlang 20.0 / pgsql_mnesia / b09e9f3 mod_global_distrib_SUITE:mod_global_distrib:test_pm_with_ungraceful_reconnection_to_different_server{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,<<"eve1.58679@localhost/res1">>,escalus_tcp,
<0.23273.3>,
[{event_manager,<0.23264.3>},
{server,<<"localhost">>},
{username,<<"eve1.58679">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.23264.3>},
{server,<<"localhost">>},
{username,<<"eve1.58679">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"eve1.58679">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"eve1.58679">>},
{server,<<"localhost">>},
{password,<<"password">>},
{port,5222},
{stream_management,true},
{stream_id,<<"9D04CAB3EBC5078A">>}]},
10000],
[{file,
"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,138}]},
{mod_global_distrib_SUITE,
'-test_pm_with_ungraceful_reconnection_to_different_server/1-fun-0-',
4,
[{file,"mod_global_distrib_SUITE.erl"},{line,610}]},
{escalus_story,story,4,
[{file,
"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_ser...6295.9 / Erlang 21.0 / riak_mnesia / b09e9f3 6295.2 / Erlang 19.3 / internal_mnesia / b09e9f3 sm_SUITE:parallel:subscription_requests_are_buffered_properly{error,{{badmatch,false},
[{escalus_session,stream_management,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_session.erl"},
{line,227}]},
{escalus_connection,connection_step,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,134}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1263}]},
{escalus_connection,start,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,118}]},
{sm_SUITE,'-subscription_requests_are_buffered_properly/1-fun-3-',6,
[{file,"sm_SUITE.erl"},{line,848}]},
{escalus_story,story,4,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1529}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1045}]}]}}sm_SUITE:parallel:subscription_requests_are_buffered_properly{error,{{badmatch,false},
[{escalus_session,stream_management,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_session.erl"},
{line,227}]},
{escalus_connection,connection_step,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,134}]},
{lists,foldl,3,[{file,"lists.erl"},{line,1263}]},
{escalus_connection,start,2,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_connection.erl"},
{line,118}]},
{sm_SUITE,'-subscription_requests_are_buffered_properly/1-fun-3-',6,
[{file,"sm_SUITE.erl"},{line,848}]},
{escalus_story,story,4,
[{file,"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
{line,72}]},
{test_server,ts_tc,3,[{file,"test_server.erl"},{line,1529}]},
{test_server,run_test_case_eval1,6,
[{file,"test_server.erl"},{line,1045}]}]}}6295.9 / Erlang 21.0 / riak_mnesia / b09e9f3 6295.2 / Erlang 19.3 / internal_mnesia / b09e9f3 |
Codecov Report
@@ Coverage Diff @@
## master #2254 +/- ##
==========================================
+ Coverage 77.07% 78.84% +1.76%
==========================================
Files 333 333
Lines 28802 28796 -6
==========================================
+ Hits 22200 22703 +503
+ Misses 6602 6093 -509
Continue to review full report at Codecov.
|
fenek
requested changes
Apr 10, 2019
1. Before the change Each auth backend had a store_type/1 callback, which could return 'external', 'plain' or 'scram'. There was a function called store_type/1 in ejabberd_auth as well. It traversed all backends, trying to determine the store type for the whole host with a logic that e.g. favored 'external' over 'plain' (see the implementation for details). Each SASL mechanism had its own password type, which was stored in the sasl_mechanism ETS table on mechanism registration. The possible password types were 'plain', 'digest', 'scram' and 'cert'. To determine supported mechanisms in cyrsasl:listmech/1, the store type for the current host was used to filter the SASL mechanisms with the following rules: Auth store_type Supported password types -------------- ------------------------ external plain scram plain, scram, cert plain plain, digest, scram 2. After the change There is no notion of store types anymore - auth backends export a function called 'supports_password_type/1' which returns a boolean that indicates the support for individual password types. To determine support for a particular password type for the whole host, the corresponding function in ejabberd_auth checks if any backend supports it. To determine supported mechanisms in cyrsasl:listmech/1, for each mechanism it is enough to check if the current host supports its password type. Furthermore, the 'cert' password type is now supported only by the 'pki' auth backend and no longer matches mechanisms supporting SCRAM. Configuration is simplified, e.g. the HTTP auth backend does not disable SASL EXTERNAL for the whole host.
chrzaszcz
force-pushed
the
no-store-type
branch
from
e57d1e3
to
360bac0
Compare
|
6307.1 / Erlang 19.3 / small_tests / 332f032 6307.5 / Erlang 19.3 / ldap_mnesia / 332f032 6307.3 / Erlang 19.3 / mysql_redis / 332f032 6307.2 / Erlang 19.3 / internal_mnesia / 332f032 6307.6 / Erlang 19.3 / elasticsearch_and_cassandra_mnesia / 332f032 6307.4 / Erlang 19.3 / odbc_mssql_mnesia / 332f032 6307.8 / Erlang 20.0 / pgsql_mnesia / 332f032 6307.9 / Erlang 21.0 / riak_mnesia / 332f032 6307.3 / Erlang 19.3 / mysql_redis / 332f032 pubsub_SUITE:dag+collection:discover_top_level_nodes_test{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,<<"alicE62.756642@localhost/res1">>,escalus_tcp,
<0.10472.3>,
[{event_manager,<0.10436.3>},
{server,<<"localhost">>},
{username,<<"alicE62.756642">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.10436.3>},
{server,<<"localhost">>},
{username,<<"alicE62.756642">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"alicE62.756642">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"alicE62.756642">>},
{server,<<"localhost">>},
{password,<<"matygrysa">>},
{stream_id,<<"4B24A5189F076CA5">>}]},
5000],
[{file,
"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,138}]},
{pubsub_tools,receive_response,3,
[{file,"pubsub_tools.erl"},{line,456}]},
{pubsub_tools,receive_and_check_response,4,
[{file,"pubsub_tools.erl"},{line,447}]},
{pubsub_SUITE,'-discover_top_level_nodes_test/1-fun-0-',2,
[{file,"pubsub_SUITE.erl"},{line,1461}]},
{escalus_story,story,4,
[{file,
"/home/travis/build/esl/MongooseIM/big_tests/_build/def...pubsub_SUITE:tree+hometree_specific:disco_node_children_by_path_prefix{error,
{timeout_when_waiting_for_stanza,
[{escalus_client,wait_for_stanza,
[{client,<<"alicE83.621322@localhost/res1">>,escalus_tcp,
<0.12148.3>,
[{event_manager,<0.12139.3>},
{server,<<"localhost">>},
{username,<<"alicE83.621322">>},
{resource,<<"res1">>}],
[{event_client,
[{event_manager,<0.12139.3>},
{server,<<"localhost">>},
{username,<<"alicE83.621322">>},
{resource,<<"res1">>}]},
{resource,<<"res1">>},
{username,<<"alicE83.621322">>},
{server,<<"localhost">>},
{host,<<"localhost">>},
{port,5222},
{auth,{escalus_auth,auth_plain}},
{wspath,undefined},
{username,<<"alicE83.621322">>},
{server,<<"localhost">>},
{password,<<"matygrysa">>},
{stream_id,<<"C71D041324D21219">>}]},
5000],
[{file,
"/home/travis/build/esl/MongooseIM/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
{line,138}]},
{pubsub_tools,receive_response,3,
[{file,"pubsub_tools.erl"},{line,456}]},
{pubsub_tools,receive_and_check_response,4,
[{file,"pubsub_tools.erl"},{line,447}]},
{pubsub_SUITE,'-disco_node_children_by_path_prefix/1-fun-0-',2,
[{file,"pubsub_SUITE.erl"},{line,1792}]},
{escalus_story,story,4,
[{file,
"/home/travis/build/esl/MongooseIM/big_tests/_buil... |
goddammit
approved these changes
Apr 11, 2019
fenek
approved these changes
Apr 11, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Before the change
Each auth backend had a
store_type/1callback, which could return'external', 'plain' or 'scram'.
There was a function called
store_type/1inejabberd_authas well.It traversed all backends, trying to determine the store type
for the whole host with a logic that e.g. favored 'external' over
'plain' (see the implementation for details).
Each SASL mechanism had its own password type, which was stored
in the sasl_mechanism ETS table on mechanism registration.
The possible password types were 'plain', 'digest', 'scram' and 'cert'.
To determine supported mechanisms in
cyrsasl:listmech/1,the store type for the current host was used to filter the SASL
mechanisms with the following rules:
After the change
There is no notion of store types anymore - auth backends have a new
supports_password_type/1callback, which returns a boolean thatindicates the support for individual password types.
To determine support for a particular password type for the whole host,
the corresponding function in
ejabberd_authchecks if any backendsupports it.
To determine supported mechanisms in
cyrsasl:listmech/1,for each mechanism it is enough to check if the current host supports
its password type.
Furthermore, the 'cert' password type is now supported only by the 'pki'
auth backend and no longer matches mechanisms supporting SCRAM.
Configuration is simplified,
e.g. the HTTP auth backend does not disable SASL EXTERNAL for the whole host.