From b7bc8b8cacf1cbced204af014cc72eb3ca4ba642 Mon Sep 17 00:00:00 2001 From: zhouzb Date: Thu, 12 Aug 2021 13:34:45 +0800 Subject: [PATCH] feat(authn): improve apis of moving authenticators --- apps/emqx_authn/src/emqx_authn.erl | 113 +++++++++++++----- apps/emqx_authn/src/emqx_authn_api.erl | 62 ++++++---- .../emqx_authn_implied_schema.erl | 9 +- apps/emqx_authn/test/emqx_authn_SUITE.erl | 14 ++- .../test/emqx_authn_mnesia_SUITE.erl | 2 +- 5 files changed, 136 insertions(+), 64 deletions(-) rename apps/emqx_authn/src/{simple_authn => }/emqx_authn_implied_schema.erl (83%) diff --git a/apps/emqx_authn/src/emqx_authn.erl b/apps/emqx_authn/src/emqx_authn.erl index 703b0efcf..90114c269 100644 --- a/apps/emqx_authn/src/emqx_authn.erl +++ b/apps/emqx_authn/src/emqx_authn.erl @@ -49,7 +49,7 @@ , update_or_create_authenticator/3 , lookup_authenticator/2 , list_authenticators/1 - , move_authenticator_to_the_nth/3 + , move_authenticator/3 ]). -export([ import_users/3 @@ -108,6 +108,30 @@ pre_config_update({update_or_create_authenticator, ID, Config}, OldConfig) -> false -> C end end, OldConfig) + end; +pre_config_update({move, ID, Position}, OldConfig) -> + case lookup_authenticator(?CHAIN, ID) of + {error, Reason} -> error(Reason); + {ok, #{name := Name}} -> + {ok, Found, Part1, Part2} = split_by_name(Name, OldConfig), + case Position of + <<"top">> -> + [Found | Part1] ++ Part2; + <<"bottom">> -> + Part1 ++ Part2 ++ [Found]; + Before -> + case binary:split(Before, <<":">>, [global]) of + [<<"before">>, ID0] -> + case lookup_authenticator(?CHAIN, ID0) of + {error, Reason} -> error(Reason); + {ok, #{name := Name1}} -> + {ok, NFound, NPart1, NPart2} = split_by_name(Name1, Part1 + Part2), + NPart1 ++ [Found, NFound | NPart2] + end; + _ -> + error({invalid_parameter, position}) + end + end end. post_config_update({enable, true}, _NewConfig, _OldConfig) -> @@ -157,6 +181,22 @@ post_config_update({update_or_create_authenticator, ID, #{<<"name">> := Name}}, end; [_Config | _] -> error(name_has_be_used) + end; +post_config_update({move, ID, Position}, _NewConfig, _OldConfig) -> + NPosition = case Position of + <<"top">> -> top; + <<"bottom">> -> bottom; + Before -> + case binary:split(Before, <<":">>, [global]) of + [<<"before">>, ID0] -> + {before, ID0}; + _ -> + error({invalid_parameter, position}) + end + end, + case move_authenticator(?CHAIN, ID, NPosition) of + ok -> ok; + {error, Reason} -> throw(Reason) end. update_config(Path, ConfigRequest) -> @@ -255,8 +295,8 @@ list_authenticators(ChainID) -> {ok, serialize_authenticators(Authenticators)} end. -move_authenticator_to_the_nth(ChainID, AuthenticatorID, N) -> - gen_server:call(?MODULE, {move_authenticator, ChainID, AuthenticatorID, N}). +move_authenticator(ChainID, AuthenticatorID, Position) -> + gen_server:call(?MODULE, {move_authenticator, ChainID, AuthenticatorID, Position}). import_users(ChainID, AuthenticatorID, Filename) -> gen_server:call(?MODULE, {import_users, ChainID, AuthenticatorID, Filename}). @@ -364,16 +404,16 @@ handle_call({update_or_create_authenticator, ChainID, AuthenticatorID, Config}, Reply = update_or_create_authenticator(ChainID, AuthenticatorID, Config, true), reply(Reply, State); -handle_call({move_authenticator, ChainID, AuthenticatorID, N}, _From, State) -> +handle_call({move_authenticator, ChainID, AuthenticatorID, Position}, _From, State) -> UpdateFun = fun(#chain{authenticators = Authenticators} = Chain) -> - case move_authenticator_to_the_nth_(AuthenticatorID, Authenticators, N) of - {ok, NAuthenticators} -> - true = ets:insert(?CHAIN_TAB, Chain#chain{authenticators = NAuthenticators}), - ok; - {error, Reason} -> - {error, Reason} - end + case do_move_authenticator(AuthenticatorID, Authenticators, Position) of + {ok, NAuthenticators} -> + true = ets:insert(?CHAIN_TAB, Chain#chain{authenticators = NAuthenticators}), + ok; + {error, Reason} -> + {error, Reason} + end end, Reply = update_chain(ChainID, UpdateFun), reply(Reply, State); @@ -458,6 +498,21 @@ switch_version(State = #{version := ?VER_2}) -> switch_version(State) -> State#{version => ?VER_1}. +split_by_name(Name, Config) -> + {Part1, Part2, true} = lists:foldl( + fun(#{<<"name">> := N} = C, {P1, P2, F0}) -> + F = case N =:= Name of + true -> true; + false -> F0 + end, + case F of + false -> {[C | P1], P2, F}; + true -> {P1, [C | P2], F} + end + end, {[], [], false}, Config), + [Found | NPart2] = lists:reverse(Part2), + {ok, Found, lists:reverse(Part1), NPart2}. + do_create_authenticator(ChainID, AuthenticatorID, #{name := Name} = Config) -> Provider = authenticator_provider(Config), Unique = <>, @@ -546,23 +601,27 @@ update_or_create_authenticator(ChainID, AuthenticatorID, #{name := NewName} = Co replace_authenticator(ID, #authenticator{name = Name} = Authenticator, Authenticators) -> lists:keyreplace(ID, 1, Authenticators, {ID, Name, Authenticator}). -move_authenticator_to_the_nth_(AuthenticatorID, Authenticators, N) - when N =< length(Authenticators) andalso N > 0 -> - move_authenticator_to_the_nth_(AuthenticatorID, Authenticators, N, []); -move_authenticator_to_the_nth_(_, _, _) -> - {error, out_of_range}. +do_move_authenticator(AuthenticatorID, Authenticators, Position) when is_binary(AuthenticatorID) -> + case lists:keytake(AuthenticatorID, 1, Authenticators) of + false -> + {error, {not_found, {authenticator, AuthenticatorID}}}; + {value, Authenticator, NAuthenticators} -> + do_move_authenticator(Authenticator, NAuthenticators, Position) + end; -move_authenticator_to_the_nth_(AuthenticatorID, [], _, _) -> - {error, {not_found, {authenticator, AuthenticatorID}}}; -move_authenticator_to_the_nth_(AuthenticatorID, [{AuthenticatorID, _, _} = Authenticator | More], N, Passed) - when N =< length(Passed) -> - {L1, L2} = lists:split(N - 1, lists:reverse(Passed)), - {ok, L1 ++ [Authenticator] ++ L2 ++ More}; -move_authenticator_to_the_nth_(AuthenticatorID, [{AuthenticatorID, _, _} = Authenticator | More], N, Passed) -> - {L1, L2} = lists:split(N - length(Passed) - 1, More), - {ok, lists:reverse(Passed) ++ L1 ++ [Authenticator] ++ L2}; -move_authenticator_to_the_nth_(AuthenticatorID, [Authenticator | More], N, Passed) -> - move_authenticator_to_the_nth_(AuthenticatorID, More, N, [Authenticator | Passed]). +do_move_authenticator(Authenticator, Authenticators, top) -> + {ok, [Authenticator | Authenticators]}; +do_move_authenticator(Authenticator, Authenticators, bottom) -> + {ok, Authenticators ++ [Authenticator]}; +do_move_authenticator(Authenticator, Authenticators, {before, ID}) -> + insert(Authenticator, Authenticators, ID, []). + +insert(_, [], ID, _) -> + {error, {not_found, {authenticator, ID}}}; +insert(Authenticator, [{ID, _, _} | _] = Authenticators, ID, Acc) -> + {ok, lists:reverse(Acc) ++ [Authenticator | Authenticators]}; +insert(Authenticator, [{_, _, _} = Authenticator0 | More], ID, Acc) -> + insert(Authenticator, More, ID, [Authenticator0 | Acc]). update_chain(ChainID, UpdateFun) -> case ets:lookup(?CHAIN_TAB, ChainID) of diff --git a/apps/emqx_authn/src/emqx_authn_api.erl b/apps/emqx_authn/src/emqx_authn_api.erl index 1e232d553..2503adf10 100644 --- a/apps/emqx_authn/src/emqx_authn_api.erl +++ b/apps/emqx_authn/src/emqx_authn_api.erl @@ -24,7 +24,7 @@ , authentication/2 , authenticators/2 , authenticators2/2 - , position/2 + , move/2 , import_users/2 , users/2 , users2/2 @@ -109,7 +109,7 @@ api_spec() -> {[ authentication_api() , authenticators_api() , authenticators_api2() - , position_api() + , move_api() , import_users_api() , users_api() , users2_api() @@ -405,10 +405,10 @@ authenticators_api2() -> }, {"/authentication/authenticators/:id", Metadata, authenticators2}. -position_api() -> +move_api() -> Metadata = #{ post => #{ - description => "Change the order of authenticators", + description => "Move authenticator", parameters => [ #{ name => id, @@ -423,14 +423,30 @@ position_api() -> content => #{ 'application/json' => #{ schema => #{ - type => object, - required => [position], - properties => #{ - position => #{ - type => integer, - example => 1 + oneOf => [ + #{ + type => object, + required => [position], + properties => #{ + position => #{ + type => string, + enum => [<<"top">>, <<"bottom">>], + example => <<"top">> + } + } + }, + #{ + type => object, + required => [position], + properties => #{ + position => #{ + type => string, + description => <<"before:">>, + example => <<"before:67e4c9d3">> + } + } } - } + ] } } } @@ -444,7 +460,7 @@ position_api() -> } } }, - {"/authentication/authenticators/:id/position", Metadata, position}. + {"/authentication/authenticators/:id/move", Metadata, move}. import_users_api() -> Metadata = #{ @@ -1304,18 +1320,17 @@ authenticators2(delete, Request) -> serialize_error(Reason) end. -position(post, Request) -> +move(post, Request) -> AuthenticatorID = cowboy_req:binding(id, Request), {ok, Body, _} = cowboy_req:read_body(Request), - NBody = emqx_json:decode(Body, [return_maps]), - Config = hocon_schema:check_plain(emqx_authn_implied_schema, #{<<"position">> => NBody}, - #{nullable => true}, ["position"]), - #{position := #{position := Position}} = emqx_map_lib:unsafe_atom_key_map(Config), - case emqx_authn:move_authenticator_to_the_nth(?CHAIN, AuthenticatorID, Position) of - ok -> - {204}; - {error, Reason} -> - serialize_error(Reason) + case emqx_json:decode(Body, [return_maps]) of + #{<<"position">> := Position} -> + case emqx_authn:update_config([authentication, authenticators], {move_authenticator, AuthenticatorID, Position}) of + ok -> {204}; + {error, Reason} -> serialize_error(Reason) + end; + _ -> + serialize_error({missing_parameter, position}) end. import_users(post, Request) -> @@ -1393,9 +1408,6 @@ serialize_error({not_found, {authenticator, ID}}) -> serialize_error(name_has_be_used) -> {409, #{code => <<"ALREADY_EXISTS">>, message => <<"Name has be used">>}}; -serialize_error(out_of_range) -> - {400, #{code => <<"OUT_OF_RANGE">>, - message => <<"Out of range">>}}; serialize_error({missing_parameter, Name}) -> {400, #{code => <<"MISSING_PARAMETER">>, message => list_to_binary( diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_implied_schema.erl b/apps/emqx_authn/src/emqx_authn_implied_schema.erl similarity index 83% rename from apps/emqx_authn/src/simple_authn/emqx_authn_implied_schema.erl rename to apps/emqx_authn/src/emqx_authn_implied_schema.erl index 1a0731e92..65b4bf356 100644 --- a/apps/emqx_authn/src/simple_authn/emqx_authn_implied_schema.erl +++ b/apps/emqx_authn/src/emqx_authn_implied_schema.erl @@ -25,12 +25,10 @@ , fields/1 ]). -structs() -> [ "filename", "position", "user_info", "new_user_info"]. +structs() -> [ "filename", "user_info", "new_user_info"]. fields("filename") -> [ {filename, fun filename/1} ]; -fields("position") -> - [ {position, fun position/1} ]; fields("user_info") -> [ {user_id, fun user_id/1} , {password, fun password/1} @@ -43,11 +41,6 @@ filename(type) -> string(); filename(nullable) -> false; filename(_) -> undefined. -position(type) -> integer(); -position(validate) -> [fun (Position) -> Position > 0 end]; -position(nullable) -> false; -position(_) -> undefined. - user_id(type) -> binary(); user_id(nullable) -> false; user_id(_) -> undefined. diff --git a/apps/emqx_authn/test/emqx_authn_SUITE.erl b/apps/emqx_authn/test/emqx_authn_SUITE.erl index 92e506d51..9c4371838 100644 --- a/apps/emqx_authn/test/emqx_authn_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_SUITE.erl @@ -86,10 +86,18 @@ t_authenticator(_) -> ?assertMatch({ok, #{id := ?CHAIN, authenticators := [#{name := AuthenticatorName1}, #{name := AuthenticatorName2}]}}, ?AUTH:lookup_chain(?CHAIN)), ?assertMatch({ok, [#{name := AuthenticatorName1}, #{name := AuthenticatorName2}]}, ?AUTH:list_authenticators(?CHAIN)), - ?assertEqual(ok, ?AUTH:move_authenticator_to_the_nth(?CHAIN, ID2, 1)), + ?assertEqual(ok, ?AUTH:move_authenticator(?CHAIN, ID2, top)), ?assertMatch({ok, [#{name := AuthenticatorName2}, #{name := AuthenticatorName1}]}, ?AUTH:list_authenticators(?CHAIN)), - ?assertEqual({error, out_of_range}, ?AUTH:move_authenticator_to_the_nth(?CHAIN, ID2, 3)), - ?assertEqual({error, out_of_range}, ?AUTH:move_authenticator_to_the_nth(?CHAIN, ID2, 0)), + + ?assertEqual(ok, ?AUTH:move_authenticator(?CHAIN, ID2, bottom)), + ?assertMatch({ok, [#{name := AuthenticatorName1}, #{name := AuthenticatorName2}]}, ?AUTH:list_authenticators(?CHAIN)), + + ?assertEqual(ok, ?AUTH:move_authenticator(?CHAIN, ID2, {before, ID1})), + + ?assertMatch({ok, [#{name := AuthenticatorName2}, #{name := AuthenticatorName1}]}, ?AUTH:list_authenticators(?CHAIN)), + + ?assertEqual({error, {not_found, {authenticator, <<"nonexistent">>}}}, ?AUTH:move_authenticator(?CHAIN, ID2, {before, <<"nonexistent">>})), + ?assertEqual(ok, ?AUTH:delete_authenticator(?CHAIN, ID1)), ?assertEqual(ok, ?AUTH:delete_authenticator(?CHAIN, ID2)), ?assertEqual({ok, []}, ?AUTH:list_authenticators(?CHAIN)), diff --git a/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl b/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl index 4a5a24844..fdcaf519d 100644 --- a/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_mnesia_SUITE.erl @@ -144,7 +144,7 @@ t_multi_mnesia_authenticator(_) -> clientid => <<"myclient">>, password => <<"mypass1">>}, ?assertEqual({stop, ok}, ?AUTH:authenticate(ClientInfo1, ok)), - ?assertEqual(ok, ?AUTH:move_authenticator_to_the_nth(?CHAIN, ID2, 1)), + ?assertEqual(ok, ?AUTH:move_authenticator(?CHAIN, ID2, top)), ?assertEqual({stop, {error, bad_username_or_password}}, ?AUTH:authenticate(ClientInfo1, ok)), ClientInfo2 = ClientInfo1#{password => <<"mypass2">>},