fix: reorder authn when updating

This commit is contained in:
Zhongwen Deng 2023-06-01 17:24:09 +08:00
parent 96ee38ce15
commit 7eea693422
7 changed files with 201 additions and 30 deletions

View File

@ -60,7 +60,8 @@
update_authenticator/3,
lookup_authenticator/2,
list_authenticators/1,
move_authenticator/3
move_authenticator/3,
reorder_authenticator/2
]).
%% APIs for observer built_in_database
@ -401,6 +402,12 @@ list_authenticators(ChainName) ->
move_authenticator(ChainName, AuthenticatorID, Position) ->
call({move_authenticator, ChainName, AuthenticatorID, Position}).
-spec reorder_authenticator(chain_name(), [authenticator_id()]) -> ok.
reorder_authenticator(_ChainName, []) ->
ok;
reorder_authenticator(ChainName, AuthenticatorIDs) ->
call({reorder_authenticator, ChainName, AuthenticatorIDs}).
-spec import_users(chain_name(), authenticator_id(), {binary(), binary()}) ->
ok | {error, term()}.
import_users(ChainName, AuthenticatorID, Filename) ->
@ -493,6 +500,12 @@ handle_call({move_authenticator, ChainName, AuthenticatorID, Position}, _From, S
end,
Reply = with_chain(ChainName, UpdateFun),
reply(Reply, State);
handle_call({reorder_authenticator, ChainName, AuthenticatorIDs}, _From, State) ->
UpdateFun = fun(Chain) ->
handle_reorder_authenticator(Chain, AuthenticatorIDs)
end,
Reply = with_chain(ChainName, UpdateFun),
reply(Reply, State);
handle_call({import_users, ChainName, AuthenticatorID, Filename}, _From, State) ->
Reply = call_authenticator(ChainName, AuthenticatorID, import_users, [Filename]),
reply(Reply, State);
@ -598,6 +611,24 @@ handle_move_authenticator(Chain, AuthenticatorID, Position) ->
{error, Reason}
end.
handle_reorder_authenticator(Chain, AuthenticatorIDs) ->
#chain{authenticators = Authenticators} = Chain,
NAuthenticators =
lists:filtermap(
fun(ID) ->
case lists:keyfind(ID, #authenticator.id, Authenticators) of
false ->
?SLOG(error, #{msg => "authenticator_not_found", id => ID}),
false;
Authenticator ->
{true, Authenticator}
end
end,
AuthenticatorIDs
),
NewChain = Chain#chain{authenticators = NAuthenticators},
{ok, ok, NewChain}.
handle_create_authenticator(Chain, Config, Providers) ->
#chain{name = Name, authenticators = Authenticators} = Chain,
AuthenticatorID = authenticator_id(Config),

View File

@ -202,7 +202,28 @@ do_post_config_update(Paths, _UpdateReq, NewConfig0, OldConfig0, _AppEnvs) ->
NewConfig = to_list(NewConfig0),
OldIds = lists:map(fun authenticator_id/1, OldConfig),
NewIds = lists:map(fun authenticator_id/1, NewConfig),
%% delete authenticators that are not in the new config
ok = delete_authenticators(NewIds, ChainName, OldConfig),
ok = create_or_update_authenticators(OldIds, ChainName, NewConfig),
ok = emqx_authentication:reorder_authenticator(ChainName, NewIds),
ok.
%% create new authenticators and update existing ones
create_or_update_authenticators(OldIds, ChainName, NewConfig) ->
lists:foreach(
fun(Conf) ->
Id = authenticator_id(Conf),
case lists:member(Id, OldIds) of
true ->
emqx_authentication:update_authenticator(ChainName, Id, Conf);
false ->
emqx_authentication:create_authenticator(ChainName, Conf)
end
end,
NewConfig
).
%% delete authenticators that are not in the new config
delete_authenticators(NewIds, ChainName, OldConfig) ->
lists:foreach(
fun(Conf) ->
Id = authenticator_id(Conf),
@ -216,19 +237,6 @@ do_post_config_update(Paths, _UpdateReq, NewConfig0, OldConfig0, _AppEnvs) ->
end
end,
OldConfig
),
%% create new authenticators and update existing ones
lists:foreach(
fun(Conf) ->
Id = authenticator_id(Conf),
case lists:member(Id, OldIds) of
true ->
emqx_authentication:update_authenticator(ChainName, Id, Conf);
false ->
emqx_authentication:create_authenticator(ChainName, Conf)
end
end,
NewConfig
).
to_list(undefined) -> [];

View File

@ -174,7 +174,7 @@ t_authenticator(Config) when is_list(Config) ->
register_provider(AuthNType1, ?MODULE),
ID1 = <<"password_based:built_in_database">>,
% CRUD of authencaticator
% CRUD of authenticator
?assertMatch(
{ok, #{id := ID1, state := #{mark := 1}}},
?AUTHN:create_authenticator(ChainName, AuthenticatorConfig1)
@ -355,6 +355,10 @@ t_update_config(Config) when is_list(Config) ->
?assertMatch({ok, [#{id := ID2}, #{id := ID1}]}, ?AUTHN:list_authenticators(Global)),
[Raw2, Raw1] = emqx:get_raw_config([?CONF_ROOT]),
?assertMatch({ok, _}, update_config([?CONF_ROOT], [Raw1, Raw2])),
?assertMatch({ok, [#{id := ID1}, #{id := ID2}]}, ?AUTHN:list_authenticators(Global)),
?assertMatch({ok, _}, update_config([?CONF_ROOT], {delete_authenticator, Global, ID1})),
?assertEqual(
{error, {not_found, {authenticator, ID1}}},
@ -417,11 +421,16 @@ t_update_config(Config) when is_list(Config) ->
{ok, _},
update_config(ConfKeyPath, {move_authenticator, ListenerID, ID2, ?CMD_MOVE_FRONT})
),
?assertMatch(
{ok, [#{id := ID2}, #{id := ID1}]},
?AUTHN:list_authenticators(ListenerID)
),
[LRaw2, LRaw1] = emqx:get_raw_config(ConfKeyPath),
?assertMatch({ok, _}, update_config(ConfKeyPath, [LRaw1, LRaw2])),
?assertMatch(
{ok, [#{id := ID1}, #{id := ID2}]},
?AUTHN:list_authenticators(ListenerID)
),
?assertMatch(
{ok, _},

View File

@ -200,6 +200,127 @@ t_union_selector_errors(Config) when is_list(Config) ->
),
ok.
t_update_conf({init, Config}) ->
emqx_common_test_helpers:start_apps([emqx_conf, emqx_authn]),
{ok, _} = emqx:update_config([authentication], []),
Config;
t_update_conf({'end', _Config}) ->
{ok, _} = emqx:update_config([authentication], []),
emqx_common_test_helpers:stop_apps([emqx_authn, emqx_conf]),
ok;
t_update_conf(Config) when is_list(Config) ->
Authn1 = #{
<<"mechanism">> => <<"password_based">>,
<<"backend">> => <<"built_in_database">>,
<<"user_id_type">> => <<"clientid">>,
<<"enable">> => true
},
Authn2 = #{
<<"mechanism">> => <<"password_based">>,
<<"backend">> => <<"http">>,
<<"method">> => <<"post">>,
<<"url">> => <<"http://127.0.0.1:18083">>,
<<"headers">> => #{
<<"content-type">> => <<"application/json">>
},
<<"enable">> => true
},
Authn3 = #{
<<"mechanism">> => <<"jwt">>,
<<"use_jwks">> => false,
<<"algorithm">> => <<"hmac-based">>,
<<"secret">> => <<"mysecret">>,
<<"secret_base64_encoded">> => false,
<<"verify_claims">> => #{<<"username">> => <<"${username}">>},
<<"enable">> => true
},
Chain = 'mqtt:global',
{ok, _} = emqx:update_config([authentication], [Authn1]),
?assertMatch(
{ok, #{
authenticators := [
#{
enable := true,
id := <<"password_based:built_in_database">>,
provider := emqx_authn_mnesia
}
]
}},
emqx_authentication:lookup_chain(Chain)
),
{ok, _} = emqx:update_config([authentication], [Authn1, Authn2, Authn3]),
?assertMatch(
{ok, #{
authenticators := [
#{
enable := true,
id := <<"password_based:built_in_database">>,
provider := emqx_authn_mnesia
},
#{
enable := true,
id := <<"password_based:http">>,
provider := emqx_authn_http
},
#{
enable := true,
id := <<"jwt">>,
provider := emqx_authn_jwt
}
]
}},
emqx_authentication:lookup_chain(Chain)
),
{ok, _} = emqx:update_config([authentication], [Authn2, Authn1]),
?assertMatch(
{ok, #{
authenticators := [
#{
enable := true,
id := <<"password_based:http">>,
provider := emqx_authn_http
},
#{
enable := true,
id := <<"password_based:built_in_database">>,
provider := emqx_authn_mnesia
}
]
}},
emqx_authentication:lookup_chain(Chain)
),
{ok, _} = emqx:update_config([authentication], [Authn3, Authn2, Authn1]),
?assertMatch(
{ok, #{
authenticators := [
#{
enable := true,
id := <<"jwt">>,
provider := emqx_authn_jwt
},
#{
enable := true,
id := <<"password_based:http">>,
provider := emqx_authn_http
},
#{
enable := true,
id := <<"password_based:built_in_database">>,
provider := emqx_authn_mnesia
}
]
}},
emqx_authentication:lookup_chain(Chain)
),
{ok, _} = emqx:update_config([authentication], []),
?assertMatch(
{error, {not_found, {chain, Chain}}},
emqx_authentication:lookup_chain(Chain)
),
ok.
parse(Bytes) ->
{ok, Frame, <<>>, {none, _}} = emqx_frame:parse(Bytes),
Frame.

View File

@ -33,21 +33,23 @@ unload() ->
emqx_ctl:unregister_command(?CLUSTER_CALL),
emqx_ctl:unregister_command(?CONF).
conf(["print", "--only-keys"]) ->
conf(["show", "--keys-only"]) ->
print(emqx_config:get_root_names());
conf(["print"]) ->
conf(["show"]) ->
print_hocon(get_config());
conf(["print", Key]) ->
conf(["show", Key]) ->
print_hocon(get_config(Key));
conf(["load", Path]) ->
load_config(Path);
conf(_) ->
emqx_ctl:usage(
[
%% TODO add reload
%{"conf reload", "reload etc/emqx.conf on local node"},
{"conf print --only-keys", "print all keys"},
{"conf print", "print all running configures"},
{"conf print <key>", "print a specific configuration"}
{"conf show --keys-only", "print all keys"},
{"conf show", "print all running configures"},
{"conf show <key>", "print a specific configuration"},
{"conf load <path>", "load a hocon file to all nodes"}
]
).
@ -137,6 +139,6 @@ load_config(Path) ->
Conf
);
{error, Reason} ->
emqx_ctl:print("load ~ts failed: ~p~n", [Path, Reason]),
{error, Reason}
emqx_ctl:print("load ~ts failed~n~p~n", [Path, Reason]),
{error, bad_hocon_file}
end.

View File

@ -135,7 +135,7 @@ run_command(Cmd, Args) when is_atom(Cmd) ->
-spec lookup_command(cmd()) -> [{module(), atom()}].
lookup_command(Cmd) when is_atom(Cmd) ->
case is_init() of
case is_initialized() of
true ->
case ets:match(?CMD_TAB, {{'_', Cmd}, '$1', '_'}) of
[El] -> El;
@ -150,7 +150,7 @@ get_commands() ->
[{Cmd, M, F} || {{_Seq, Cmd}, {M, F}, _Opts} <- ets:tab2list(?CMD_TAB)].
help() ->
case is_init() of
case is_initialized() of
true ->
case ets:tab2list(?CMD_TAB) of
[] ->
@ -290,5 +290,5 @@ safe_to_existing_atom(Str) ->
undefined
end.
is_init() ->
is_initialized() ->
ets:info(?CMD_TAB) =/= undefined.

View File

@ -49,8 +49,8 @@ t_reg_unreg_command(_) ->
emqx_ctl:unregister_command(cmd1),
emqx_ctl:unregister_command(cmd2),
ct:sleep(100),
?assertEqual([], emqx_ctl:lookup_command(cmd1)),
?assertEqual([], emqx_ctl:lookup_command(cmd2)),
?assertEqual({error, cmd_not_found}, emqx_ctl:lookup_command(cmd1)),
?assertEqual({error, cmd_not_found}, emqx_ctl:lookup_command(cmd2)),
?assertEqual([], emqx_ctl:get_commands())
end
).