From b56b2545344434e7c4c6bdc6d4c289805db10678 Mon Sep 17 00:00:00 2001 From: Gustaw Lippa Date: Fri, 18 Mar 2022 15:14:43 +0100 Subject: [PATCH] Use maps for mod_keystore config Because the validation of different keys has been moved to parsing the config, the test is also moved to the config_parser_SUITE. --- big_tests/tests/oauth_SUITE.erl | 17 ++++----- src/mod_keystore.erl | 57 +++++++++++----------------- test/common/config_parser_helper.erl | 11 +++--- test/config_parser_SUITE.erl | 19 +++++++--- test/keystore_SUITE.erl | 49 +++--------------------- 5 files changed, 55 insertions(+), 98 deletions(-) diff --git a/big_tests/tests/oauth_SUITE.erl b/big_tests/tests/oauth_SUITE.erl index e9906769b3..2881e05ef8 100644 --- a/big_tests/tests/oauth_SUITE.erl +++ b/big_tests/tests/oauth_SUITE.erl @@ -461,15 +461,14 @@ to_lower(B) when is_binary(B) -> list_to_binary(string:to_lower(binary_to_list(B))). required_modules() -> - KeyStoreOpts = [{keys, [ - {token_secret, ram}, - %% This is a hack for tests! As the name implies, - %% a pre-shared key should be read from a file stored - %% on disk. This way it can be shared with trusted 3rd - %% parties who can use it to sign tokens for users - %% to authenticate with and MongooseIM to verify. - {provision_pre_shared, ram} - ]}], + KeyOpts = #{keys => [{token_secret, ram}, + %% This is a hack for tests! As the name implies, + %% a pre-shared key should be read from a file stored + %% on disk. This way it can be shared with trusted 3rd + %% parties who can use it to sign tokens for users + %% to authenticate with and MongooseIM to verify. + {provision_pre_shared, ram}]}, + KeyStoreOpts = config_parser_helper:mod_config(mod_keystore, KeyOpts), [{mod_last, stopped}, {mod_keystore, KeyStoreOpts}, {mod_auth_token, auth_token_opts()}]. diff --git a/src/mod_keystore.erl b/src/mod_keystore.erl index 5e73c0929a..96b1b572f3 100644 --- a/src/mod_keystore.erl +++ b/src/mod_keystore.erl @@ -12,11 +12,8 @@ %% Hook handlers -export([get_key/2]). -%% Tests only! --export([validate_opts/1]). - -export([config_metrics/1]). --export([process_keys/1]). +-export([process_key/1]). %% Public types -export_type([key/0, @@ -25,9 +22,7 @@ key_name/0, raw_key/0]). --ignore_xref([ - behaviour_info/1, get_key/2, validate_opts/1 -]). +-ignore_xref([get_key/2]). -include("mod_keystore.hrl"). -include("mongoose.hrl"). @@ -54,9 +49,8 @@ %% gen_mod callbacks %% --spec start(mongooseim:host_type(), list()) -> ok. +-spec start(mongooseim:host_type(), gen_mod:module_opts()) -> ok. start(HostType, Opts) -> - validate_opts(Opts), create_keystore_ets(), mod_keystore_backend:init(HostType, Opts), init_keys(HostType, Opts), @@ -84,7 +78,11 @@ config_spec() -> items = #{<<"ram_key_size">> => #option{type = integer, validate = non_negative}, <<"keys">> => #list{items = keys_spec()} - } + }, + defaults = #{<<"ram_key_size">> => ?DEFAULT_RAM_KEY_SIZE, + <<"keys">> => []}, + format_items = map, + process = fun validate_key_ids/1 }. keys_spec() -> @@ -97,15 +95,14 @@ keys_spec() -> validate = filename} }, required = [<<"name">>, <<"type">>], - process = fun ?MODULE:process_keys/1 + format_items = map, + process = fun ?MODULE:process_key/1 }. -process_keys(KVs) -> - {[[{name, Name}], [{type, Type}]], PathOpts} = proplists:split(KVs, [name, type]), - process_key_opts(Name, Type, PathOpts). - -process_key_opts(Name, ram, []) -> {Name, ram}; -process_key_opts(Name, file, [{path, Path}]) -> {Name, {file, Path}}. +process_key(#{name := Name, type := file, path := Path}) -> + {Name, {file, Path}}; +process_key(#{name := Name, type := ram}) -> + {Name, ram}. %% %% Hook handlers @@ -165,16 +162,16 @@ clear_keystore_ets(HostType) -> does_table_exist(NameOrTID) -> ets:info(NameOrTID, name) /= undefined. -init_keys(HostType, Opts) -> - [ init_key(K, HostType, Opts) || K <- proplists:get_value(keys, Opts, []) ]. +init_keys(HostType, Opts = #{keys := Keys}) -> + [ init_key(K, HostType, Opts) || K <- Keys ]. --spec init_key({key_name(), key_type()}, mongooseim:host_type(), list()) -> ok. +-spec init_key({key_name(), key_type()}, mongooseim:host_type(), gen_mod:module_opts()) -> ok. init_key({KeyName, {file, Path}}, HostType, _Opts) -> {ok, Data} = file:read_file(Path), true = ets_store_key({KeyName, HostType}, Data), ok; -init_key({KeyName, ram}, HostType, Opts) -> - ProposedKey = crypto:strong_rand_bytes(get_key_size(Opts)), +init_key({KeyName, ram}, HostType, #{ram_key_size := KeySize}) -> + ProposedKey = crypto:strong_rand_bytes(KeySize), KeyRecord = #key{id = {KeyName, HostType}, key = ProposedKey}, {ok, _ActualKey} = mod_keystore_backend:init_ram_key(HostType, KeyRecord), @@ -187,23 +184,13 @@ ets_get_key(KeyID) -> ets_store_key(KeyID, RawKey) -> ets:insert(keystore, {KeyID, RawKey}). -get_key_size(Opts) -> - case lists:keyfind(ram_key_size, 1, Opts) of - false -> ?DEFAULT_RAM_KEY_SIZE; - {ram_key_size, KeySize} -> KeySize - end. - -validate_opts(Opts) -> - validate_key_ids(proplists:get_value(keys, Opts, [])). - -validate_key_ids(KeySpecs) -> +validate_key_ids(Opts = #{keys := KeySpecs}) -> KeyIDs = [ KeyID || {KeyID, _} <- KeySpecs ], SortedAndUniqueKeyIDs = lists:usort(KeyIDs), case KeyIDs -- SortedAndUniqueKeyIDs of - [] -> ok; + [] -> Opts; [_|_] -> error(non_unique_key_ids, KeySpecs) end. config_metrics(Host) -> - OptsToReport = [{backend, mnesia}], %list of tuples {option, defualt_value} - mongoose_module_metrics:opts_for_module(Host, ?MODULE, OptsToReport). + mongoose_module_metrics:opts_for_module(Host, ?MODULE, [backend]). diff --git a/test/common/config_parser_helper.erl b/test/common/config_parser_helper.erl index 350edbea1e..9d5e9ab66f 100644 --- a/test/common/config_parser_helper.erl +++ b/test/common/config_parser_helper.erl @@ -453,11 +453,10 @@ all_modules() -> mod_adhoc => #{iqdisc => one_queue, report_commands_node => true}, mod_mam_rdbms_arch_async => default_config([modules, mod_mam_meta, async_writer]), mod_keystore => - [{keys, - [{access_secret, ram}, - {access_psk, {file, "priv/access_psk"}}, - {provision_psk, {file, "priv/provision_psk"}}]}, - {ram_key_size, 1000}], + mod_config(mod_keystore, #{keys => [{access_secret, ram}, + {access_psk, {file, "priv/access_psk"}}, + {provision_psk, {file, "priv/provision_psk"}}], + ram_key_size => 1000}), mod_global_distrib => mod_config(mod_global_distrib, #{global_host => <<"example.com">>, @@ -874,6 +873,8 @@ default_mod_config(mod_inbox) -> remove_on_kicked => true, reset_markers => [<<"displayed">>], iqdisc => no_queue}; +default_mod_config(mod_keystore) -> + #{ram_key_size => 2048, keys => []}; default_mod_config(mod_last) -> #{iqdisc => one_queue, backend => mnesia}; default_mod_config(mod_mam) -> diff --git a/test/config_parser_SUITE.erl b/test/config_parser_SUITE.erl index 3e2581f65f..f863d92403 100644 --- a/test/config_parser_SUITE.erl +++ b/test/config_parser_SUITE.erl @@ -2101,9 +2101,10 @@ mod_jingle_sip(_Config) -> ?errh(T(#{<<"sdp_origin">> => <<"abc">>})). mod_keystore(_Config) -> + check_module_defaults(mod_keystore), T = fun(Opts) -> #{<<"modules">> => #{<<"mod_keystore">> => Opts}} end, - M = fun(Cfg) -> modopts(mod_keystore, Cfg) end, - ?cfgh(M([{ram_key_size, 1024}]), + P = [modules, mod_keystore], + ?cfgh(P ++ [ram_key_size], 1024, T(#{<<"ram_key_size">> => 1024})), ?errh(T(#{<<"ram_key_size">> => -1})). @@ -2111,12 +2112,12 @@ mod_keystore_keys(_Config) -> T = fun(Opts) -> #{<<"modules">> => #{<<"mod_keystore">> => #{<<"keys">> => Opts}}} end, - M = fun(Cfg) -> modopts(mod_keystore, [{keys, Cfg}]) end, + P = [modules, mod_keystore, keys], RequiredOpts = #{<<"name">> => <<"access_secret">>, <<"type">> => <<"ram">>}, - ?cfgh(M([{access_secret, ram}]), + ?cfgh(P, [{access_secret, ram}], T([RequiredOpts])), - ?cfgh(M([{access_secret, {file, "priv/access_psk"}}]), + ?cfgh(P, [{access_secret, {file, "priv/access_psk"}}], T([RequiredOpts#{<<"type">> => <<"file">>, <<"path">> => <<"priv/access_psk">>}])), [?errh(T([maps:remove(Key, RequiredOpts)])) || Key <- maps:keys(RequiredOpts)], @@ -2124,7 +2125,13 @@ mod_keystore_keys(_Config) -> ?errh(T([RequiredOpts#{<<"type">> => <<"rampampam">>}])), ?errh(T([RequiredOpts#{<<"type">> => <<"file">>}])), ?errh(T([RequiredOpts#{<<"type">> => <<"file">>, - <<"path">> => <<"does/not/exists">>}])). + <<"path">> => <<"does/not/exists">>}])), + ?errh([#{reason := non_unique_key_ids}], + T([#{<<"name">> => <<"same_name_twice">>, + <<"type">> => <<"ram">>}, + #{<<"name">> => <<"same_name_twice">>, + <<"type">> => <<"file">>, + <<"path">> => <<"priv/access_psk">>}])). mod_last(_Config) -> check_iqdisc_map(mod_last), diff --git a/test/keystore_SUITE.erl b/test/keystore_SUITE.erl index 8d73a0affe..b3fb53042e 100644 --- a/test/keystore_SUITE.erl +++ b/test/keystore_SUITE.erl @@ -1,6 +1,7 @@ -module(keystore_SUITE). -include_lib("eunit/include/eunit.hrl"). -compile([export_all, nowarn_export_all]). +-import(config_parser_helper, [default_mod_config/1, mod_config/2]). -define(ae(Expected, Actual), ?assertEqual(Expected, Actual)). @@ -11,7 +12,6 @@ all() -> module_startup_create_ram_key, module_startup_create_ram_key_of_given_size, module_startup_for_multiple_domains, - module_startup_non_unique_key_ids, multiple_domains_one_stopped ]. @@ -54,7 +54,7 @@ clean_after_testcase(C) -> %% module_startup_no_opts(_) -> - ok = mod_keystore:start(<<"localhost">>, []). + ok = mod_keystore:start(<<"localhost">>, default_mod_config(mod_keystore)). module_startup_read_key_from_file(_) -> %% given @@ -103,18 +103,6 @@ module_startup_for_multiple_domains(_Config) -> ?ae([{{key_from_file, <<"second.com">>}, SecondKey}], get_key(<<"second.com">>, key_from_file)). -module_startup_non_unique_key_ids(_) -> - %% given - NonUniqueKeyIDsOpts = [{keys, [{some_key, ram}, - {some_key, {file, "some_key.dat"}}]}], - %% when - try - mod_keystore:start(<<"localhost">>, NonUniqueKeyIDsOpts) - %% then - catch - error:non_unique_key_ids -> ok - end. - multiple_domains_one_stopped(_Config) -> % given [] = get_key(<<"first.com">>, key_from_file), @@ -135,40 +123,19 @@ multiple_domains_one_stopped(_Config) -> %% Helpers %% -start_async(M, F, A) -> - Self = self(), - P = spawn(fun () -> - erlang:apply(M, F, A), - Self ! started, - helper_loop() - end), - receive - started -> - %ct:pal("started", []), - {ok, P} - after timer:seconds(1) -> - ct:fail("async start timeout") - end. - -helper_loop() -> - receive - stop -> exit(normal); - _ -> helper_loop() - end. - key_at(Path, Data) -> ok = file:write_file(Path, Data), {ok, Path}. key_from_file(KeyFile) -> - [{keys, [{key_from_file, {file, KeyFile}}]}]. + mod_config(mod_keystore, #{keys => [{key_from_file, {file, KeyFile}}]}). ram_key() -> - [{keys, [{ram_key, ram}]}]. + mod_config(mod_keystore, #{keys => [{ram_key, ram}]}). sized_ram_key(Size) -> - [{keys, [{ram_key, ram}]}, - {ram_key_size, Size}]. + mod_config(mod_keystore, #{keys => [{ram_key, ram}], + ram_key_size => Size}). mock_mongoose_metrics() -> meck:new(mongoose_metrics, []), @@ -183,7 +150,3 @@ mock_mongoose_metrics() -> Result :: mod_keystore:key_list(). get_key(HostType, KeyName) -> mongoose_hooks:get_key(HostType, KeyName). - -%%{mod_keystore, [{keys, [{asdqwe_access_secret, ram}, -%% {asdqwe_access_psk, {file, "priv/asdqwe_access_psk"}}, -%% {asdqwe_provision_psk, {file, "priv/asdqwe_access_psk"}}]}]},