diff --git a/.github/workflows/run_fvt_tests.yaml b/.github/workflows/run_fvt_tests.yaml index 0cf530765..de2eea01b 100644 --- a/.github/workflows/run_fvt_tests.yaml +++ b/.github/workflows/run_fvt_tests.yaml @@ -55,6 +55,9 @@ jobs: arch: - amd64 steps: + - uses: erlef/setup-beam@v1 + with: + otp-version: "24.2" - uses: actions/download-artifact@v2 with: name: source @@ -135,13 +138,15 @@ jobs: # - emqx-enterprise # TODO test enterprise steps: + - uses: erlef/setup-beam@v1 + with: + otp-version: "24.2" - uses: actions/download-artifact@v2 with: name: source path: . - name: unzip source code run: unzip -q source.zip - - name: Get deps git refs for cache id: deps-refs run: | diff --git a/.github/workflows/run_jmeter_tests.yaml b/.github/workflows/run_jmeter_tests.yaml index 476ca0ea8..c2b0442ca 100644 --- a/.github/workflows/run_jmeter_tests.yaml +++ b/.github/workflows/run_jmeter_tests.yaml @@ -14,6 +14,9 @@ jobs: outputs: version: ${{ steps.build_docker.outputs.version}} steps: + - uses: erlef/setup-beam@v1 + with: + otp-version: "24.2" - name: download jmeter timeout-minutes: 3 env: @@ -54,6 +57,9 @@ jobs: needs: build_emqx_for_jmeter_tests steps: + - uses: erlef/setup-beam@v1 + with: + otp-version: "24.2" - uses: actions/checkout@v2 - uses: actions/download-artifact@v2 with: @@ -145,6 +151,9 @@ jobs: needs: build_emqx_for_jmeter_tests steps: + - uses: erlef/setup-beam@v1 + with: + otp-version: "24.2" - uses: actions/checkout@v2 - uses: actions/download-artifact@v2 with: @@ -246,6 +255,9 @@ jobs: needs: build_emqx_for_jmeter_tests steps: + - uses: erlef/setup-beam@v1 + with: + otp-version: "24.2" - uses: actions/checkout@v2 - uses: actions/download-artifact@v2 with: @@ -343,6 +355,9 @@ jobs: needs: build_emqx_for_jmeter_tests steps: + - uses: erlef/setup-beam@v1 + with: + otp-version: "24.2" - uses: actions/checkout@v2 - uses: actions/download-artifact@v2 with: @@ -437,6 +452,9 @@ jobs: needs: build_emqx_for_jmeter_tests steps: + - uses: erlef/setup-beam@v1 + with: + otp-version: "24.2" - uses: actions/checkout@v2 - uses: actions/download-artifact@v2 with: diff --git a/apps/emqx/src/emqx_authentication.erl b/apps/emqx/src/emqx_authentication.erl index abfa55c36..b866c35f9 100644 --- a/apps/emqx/src/emqx_authentication.erl +++ b/apps/emqx/src/emqx_authentication.erl @@ -47,7 +47,6 @@ register_providers/1, deregister_provider/1, deregister_providers/1, - create_chain/1, delete_chain/1, lookup_chain/1, list_chains/0, @@ -81,7 +80,7 @@ ]). %% utility functions --export([authenticator_id/1]). +-export([authenticator_id/1, metrics_id/2]). %% proxy callback -export([ @@ -216,22 +215,28 @@ when %%------------------------------------------------------------------------------ authenticate(#{listener := Listener, protocol := Protocol} = Credential, _AuthResult) -> - Authenticators = get_authenticators(Listener, global_chain(Protocol)), - case get_enabled(Authenticators) of - [] -> ignore; - NAuthenticators -> do_authenticate(NAuthenticators, Credential) + case get_authenticators(Listener, global_chain(Protocol)) of + {ok, ChainName, Authenticators} -> + case get_enabled(Authenticators) of + [] -> + ignore; + NAuthenticators -> + do_authenticate(ChainName, NAuthenticators, Credential) + end; + none -> + ignore end. get_authenticators(Listener, Global) -> case ets:lookup(?CHAINS_TAB, Listener) of - [#chain{authenticators = Authenticators}] -> - Authenticators; + [#chain{name = Name, authenticators = Authenticators}] -> + {ok, Name, Authenticators}; _ -> case ets:lookup(?CHAINS_TAB, Global) of - [#chain{authenticators = Authenticators}] -> - Authenticators; + [#chain{name = Name, authenticators = Authenticators}] -> + {ok, Name, Authenticators}; _ -> - [] + none end end. @@ -265,7 +270,6 @@ authenticator_id(Config) -> initialize_authentication(_, []) -> ok; initialize_authentication(ChainName, AuthenticatorsConfig) -> - _ = create_chain(ChainName), CheckedConfig = to_list(AuthenticatorsConfig), lists:foreach( fun(AuthenticatorConfig) -> @@ -316,10 +320,6 @@ deregister_providers(AuthNTypes) when is_list(AuthNTypes) -> deregister_provider(AuthNType) -> deregister_providers([AuthNType]). --spec create_chain(chain_name()) -> {ok, chain()} | {error, term()}. -create_chain(Name) -> - call({create_chain, Name}). - -spec delete_chain(chain_name()) -> ok | {error, term()}. delete_chain(Name) -> call({delete_chain, Name}). @@ -445,50 +445,36 @@ handle_call( end; handle_call({deregister_providers, AuthNTypes}, _From, #{providers := Providers} = State) -> reply(ok, State#{providers := maps:without(AuthNTypes, Providers)}); -handle_call({create_chain, Name}, _From, State) -> - case ets:member(?CHAINS_TAB, Name) of - true -> - reply({error, {already_exists, {chain, Name}}}, State); - false -> - Chain = #chain{ - name = Name, - authenticators = [] - }, - true = ets:insert(?CHAINS_TAB, Chain), - reply({ok, serialize_chain(Chain)}, State) - end; -handle_call({delete_chain, Name}, _From, State) -> - case ets:lookup(?CHAINS_TAB, Name) of - [] -> - reply({error, {not_found, {chain, Name}}}, State); - [#chain{authenticators = Authenticators}] -> - _ = [do_destroy_authenticator(Authenticator) || Authenticator <- Authenticators], - true = ets:delete(?CHAINS_TAB, Name), - reply(ok, maybe_unhook(State)) - end; +handle_call({delete_chain, ChainName}, _From, State) -> + UpdateFun = fun(Chain) -> + {_MatchedIDs, NewChain} = do_delete_authenticators(fun(_) -> true end, Chain), + {ok, ok, NewChain} + end, + Reply = with_chain(ChainName, UpdateFun), + reply(Reply, maybe_unhook(State)); handle_call({create_authenticator, ChainName, Config}, _From, #{providers := Providers} = State) -> UpdateFun = fun(Chain) -> handle_create_authenticator(Chain, Config, Providers) end, - Reply = update_chain(ChainName, UpdateFun), + Reply = with_new_chain(ChainName, UpdateFun), reply(Reply, maybe_hook(State)); handle_call({delete_authenticator, ChainName, AuthenticatorID}, _From, State) -> UpdateFun = fun(Chain) -> handle_delete_authenticator(Chain, AuthenticatorID) end, - Reply = update_chain(ChainName, UpdateFun), + Reply = with_chain(ChainName, UpdateFun), reply(Reply, maybe_unhook(State)); handle_call({update_authenticator, ChainName, AuthenticatorID, Config}, _From, State) -> UpdateFun = fun(Chain) -> handle_update_authenticator(Chain, AuthenticatorID, Config) end, - Reply = update_chain(ChainName, UpdateFun), + Reply = with_chain(ChainName, UpdateFun), reply(Reply, State); handle_call({move_authenticator, ChainName, AuthenticatorID, Position}, _From, State) -> UpdateFun = fun(Chain) -> handle_move_authenticator(Chain, AuthenticatorID, Position) end, - Reply = update_chain(ChainName, UpdateFun), + 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]), @@ -563,11 +549,9 @@ handle_update_authenticator(Chain, AuthenticatorID, Config) -> NewAuthenticator, Authenticators ), - true = ets:insert( - ?CHAINS_TAB, - Chain#chain{authenticators = NewAuthenticators} - ), - {ok, serialize_authenticator(NewAuthenticator)}; + NewChain = Chain#chain{authenticators = NewAuthenticators}, + Result = {ok, serialize_authenticator(NewAuthenticator)}, + {ok, Result, NewChain}; {error, Reason} -> {error, Reason} end; @@ -581,25 +565,24 @@ handle_delete_authenticator(Chain, AuthenticatorID) -> ID =:= AuthenticatorID end, case do_delete_authenticators(MatchFun, Chain) of - [] -> + {[], _NewChain} -> {error, {not_found, {authenticator, AuthenticatorID}}}; - [AuthenticatorID] -> - emqx_metrics_worker:clear_metrics(authn_metrics, AuthenticatorID), - ok + {[AuthenticatorID], NewChain} -> + {ok, ok, NewChain} end. handle_move_authenticator(Chain, AuthenticatorID, Position) -> #chain{authenticators = Authenticators} = Chain, case do_move_authenticator(AuthenticatorID, Authenticators, Position) of {ok, NAuthenticators} -> - true = ets:insert(?CHAINS_TAB, Chain#chain{authenticators = NAuthenticators}), - ok; + NewChain = Chain#chain{authenticators = NAuthenticators}, + {ok, ok, NewChain}; {error, Reason} -> {error, Reason} end. handle_create_authenticator(Chain, Config, Providers) -> - #chain{authenticators = Authenticators} = Chain, + #chain{name = Name, authenticators = Authenticators} = Chain, AuthenticatorID = authenticator_id(Config), case lists:keymember(AuthenticatorID, #authenticator.id, Authenticators) of true -> @@ -611,31 +594,31 @@ handle_create_authenticator(Chain, Config, Providers) -> NAuthenticators = Authenticators ++ [Authenticator#authenticator{enable = maps:get(enable, Config)}], - true = ets:insert( - ?CHAINS_TAB, - Chain#chain{authenticators = NAuthenticators} - ), - ok = emqx_metrics_worker:create_metrics( authn_metrics, - AuthenticatorID, + metrics_id(Name, AuthenticatorID), [total, success, failed, nomatch], [total] ), - {ok, serialize_authenticator(Authenticator)}; + NewChain = Chain#chain{authenticators = NAuthenticators}, + Result = {ok, serialize_authenticator(Authenticator)}, + {ok, Result, NewChain}; {error, Reason} -> {error, Reason} end end. -do_authenticate([], _) -> +do_authenticate(_ChainName, [], _) -> {stop, {error, not_authorized}}; -do_authenticate([#authenticator{id = ID, provider = Provider, state = State} | More], Credential) -> - emqx_metrics_worker:inc(authn_metrics, ID, total), +do_authenticate( + ChainName, [#authenticator{id = ID, provider = Provider, state = State} | More], Credential +) -> + MetricsID = metrics_id(ChainName, ID), + emqx_metrics_worker:inc(authn_metrics, MetricsID, total), try Provider:authenticate(Credential, State) of ignore -> - ok = emqx_metrics_worker:inc(authn_metrics, ID, nomatch), - do_authenticate(More, Credential); + ok = emqx_metrics_worker:inc(authn_metrics, MetricsID, nomatch), + do_authenticate(ChainName, More, Credential); Result -> %% {ok, Extra} %% {ok, Extra, AuthData} @@ -644,9 +627,9 @@ do_authenticate([#authenticator{id = ID, provider = Provider, state = State} | M %% {error, Reason} case Result of {ok, _} -> - emqx_metrics_worker:inc(authn_metrics, ID, success); + emqx_metrics_worker:inc(authn_metrics, MetricsID, success); {error, _} -> - emqx_metrics_worker:inc(authn_metrics, ID, failed); + emqx_metrics_worker:inc(authn_metrics, MetricsID, failed); _ -> ok end, @@ -660,13 +643,21 @@ do_authenticate([#authenticator{id = ID, provider = Provider, state = State} | M stacktrace => Stacktrace, authenticator => ID }), - emqx_metrics_worker:inc(authn_metrics, ID, nomatch), - do_authenticate(More, Credential) + emqx_metrics_worker:inc(authn_metrics, MetricsID, nomatch), + do_authenticate(ChainName, More, Credential) end. reply(Reply, State) -> {reply, Reply, State}. +save_chain(#chain{ + name = Name, + authenticators = [] +}) -> + ets:delete(?CHAINS_TAB, Name); +save_chain(#chain{} = Chain) -> + ets:insert(?CHAINS_TAB, Chain). + create_chain_table() -> try _ = ets:new(?CHAINS_TAB, [ @@ -751,7 +742,7 @@ do_create_authenticator(AuthenticatorID, #{enable := Enable} = Config, Providers end end. -do_delete_authenticators(MatchFun, #chain{authenticators = Authenticators} = Chain) -> +do_delete_authenticators(MatchFun, #chain{name = Name, authenticators = Authenticators} = Chain) -> {Matching, Others} = lists:partition(MatchFun, Authenticators), MatchingIDs = lists:map( @@ -759,9 +750,14 @@ do_delete_authenticators(MatchFun, #chain{authenticators = Authenticators} = Cha Matching ), - ok = lists:foreach(fun do_destroy_authenticator/1, Matching), - true = ets:insert(?CHAINS_TAB, Chain#chain{authenticators = Others}), - MatchingIDs. + ok = lists:foreach( + fun(#authenticator{id = ID} = Authenticator) -> + do_destroy_authenticator(Authenticator), + emqx_metrics_worker:clear_metrics(authn_metrics, metrics_id(Name, ID)) + end, + Matching + ), + {MatchingIDs, Chain#chain{authenticators = Others}}. do_destroy_authenticator(#authenticator{provider = Provider, state = State}) -> _ = Provider:destroy(State), @@ -804,21 +800,41 @@ insert( insert(Authenticator, [Authenticator0 | More], {Relative, RelatedID}, Acc) -> insert(Authenticator, More, {Relative, RelatedID}, [Authenticator0 | Acc]). -update_chain(ChainName, UpdateFun) -> +with_new_chain(ChainName, Fun) -> + case ets:lookup(?CHAINS_TAB, ChainName) of + [] -> + Chain = #chain{name = ChainName, authenticators = []}, + do_with_chain(Fun, Chain); + [Chain] -> + do_with_chain(Fun, Chain) + end. + +with_chain(ChainName, Fun) -> case ets:lookup(?CHAINS_TAB, ChainName) of [] -> {error, {not_found, {chain, ChainName}}}; [Chain] -> - try - UpdateFun(Chain) - catch - Class:Reason:Stk -> - {error, {exception, {Class, Reason, Stk}}} - end + do_with_chain(Fun, Chain) + end. + +do_with_chain(Fun, Chain) -> + try + case Fun(Chain) of + {ok, Result} -> + Result; + {ok, Result, NewChain} -> + save_chain(NewChain), + Result; + {error, _} = Error -> + Error + end + catch + Class:Reason:Stk -> + {error, {exception, {Class, Reason, Stk}}} end. call_authenticator(ChainName, AuthenticatorID, Func, Args) -> - UpdateFun = + Fun = fun(#chain{authenticators = Authenticators}) -> case lists:keyfind(AuthenticatorID, #authenticator.id, Authenticators) of false -> @@ -826,13 +842,13 @@ call_authenticator(ChainName, AuthenticatorID, Func, Args) -> #authenticator{provider = Provider, state = State} -> case erlang:function_exported(Provider, Func, length(Args) + 1) of true -> - erlang:apply(Provider, Func, Args ++ [State]); + {ok, erlang:apply(Provider, Func, Args ++ [State])}; false -> {error, unsupported_operation} end end end, - update_chain(ChainName, UpdateFun). + with_chain(ChainName, Fun). serialize_chain(#chain{ name = Name, @@ -875,6 +891,9 @@ insert_user_group( insert_user_group(_Chain, Config) -> Config. +metrics_id(ChainName, AuthenticatorId) -> + iolist_to_binary([atom_to_binary(ChainName), <<"-">>, AuthenticatorId]). + to_list(undefined) -> []; to_list(M) when M =:= #{} -> []; to_list(M) when is_map(M) -> [M]; diff --git a/apps/emqx/src/emqx_authentication_config.erl b/apps/emqx/src/emqx_authentication_config.erl index c826a0fac..681ed1394 100644 --- a/apps/emqx/src/emqx_authentication_config.erl +++ b/apps/emqx/src/emqx_authentication_config.erl @@ -52,7 +52,6 @@ -type update_request() :: {create_authenticator, chain_name(), map()} | {delete_authenticator, chain_name(), authenticator_id()} - | {delete_authenticators, chain_name()} | {update_authenticator, chain_name(), authenticator_id(), map()} | {move_authenticator, chain_name(), authenticator_id(), position()}. @@ -89,8 +88,6 @@ do_pre_config_update({delete_authenticator, _ChainName, AuthenticatorID}, OldCon OldConfig ), {ok, NewConfig}; -do_pre_config_update({delete_authenticators, _ChainName}, _OldConfig) -> - {ok, []}; do_pre_config_update({update_authenticator, ChainName, AuthenticatorID, Config}, OldConfig) -> CertsDir = certs_dir(ChainName, AuthenticatorID), NewConfig = lists:map( @@ -143,7 +140,6 @@ post_config_update(_, UpdateReq, NewConfig, OldConfig, AppEnvs) -> do_post_config_update({create_authenticator, ChainName, Config}, NewConfig, _OldConfig, _AppEnvs) -> NConfig = get_authenticator_config(authenticator_id(Config), NewConfig), - _ = emqx_authentication:create_chain(ChainName), emqx_authentication:create_authenticator(ChainName, NConfig); do_post_config_update( {delete_authenticator, ChainName, AuthenticatorID}, @@ -159,25 +155,6 @@ do_post_config_update( {error, Reason} -> {error, Reason} end; -do_post_config_update( - {delete_authenticators, ChainName}, - _NewConfig, - OldConfig, - _AppEnvs -) -> - case emqx_authentication:delete_chain(ChainName) of - ok -> - lists:foreach( - fun(Config) -> - AuthenticatorID = authenticator_id(Config), - CertsDir = certs_dir(ChainName, AuthenticatorID), - ok = clear_certs(CertsDir, Config) - end, - to_list(OldConfig) - ); - {error, Reason} -> - {error, Reason} - end; do_post_config_update( {update_authenticator, ChainName, AuthenticatorID, Config}, NewConfig, diff --git a/apps/emqx/test/emqx_authentication_SUITE.erl b/apps/emqx/test/emqx_authentication_SUITE.erl index 63aafb124..61b4b2775 100644 --- a/apps/emqx/test/emqx_authentication_SUITE.erl +++ b/apps/emqx/test/emqx_authentication_SUITE.erl @@ -113,20 +113,32 @@ end_per_testcase(Case, Config) -> _ = ?MODULE:Case({'end', Config}), ok. -t_chain({_, Config}) -> +t_chain({'init', Config}) -> Config; t_chain(Config) when is_list(Config) -> % CRUD of authentication chain ChainName = 'test', ?assertMatch({ok, []}, ?AUTHN:list_chains()), ?assertMatch({ok, []}, ?AUTHN:list_chain_names()), - ?assertMatch({ok, #{name := ChainName, authenticators := []}}, ?AUTHN:create_chain(ChainName)), - ?assertEqual({error, {already_exists, {chain, ChainName}}}, ?AUTHN:create_chain(ChainName)), - ?assertMatch({ok, #{name := ChainName, authenticators := []}}, ?AUTHN:lookup_chain(ChainName)), + + %% to create a chain we need create an authenticator + AuthenticatorConfig = #{ + mechanism => password_based, + backend => built_in_database, + enable => true + }, + register_provider({password_based, built_in_database}, ?MODULE), + ?AUTHN:create_authenticator(ChainName, AuthenticatorConfig), + + ?assertMatch({ok, #{name := ChainName, authenticators := [_]}}, ?AUTHN:lookup_chain(ChainName)), ?assertMatch({ok, [#{name := ChainName}]}, ?AUTHN:list_chains()), ?assertEqual({ok, [ChainName]}, ?AUTHN:list_chain_names()), ?assertEqual(ok, ?AUTHN:delete_chain(ChainName)), ?assertMatch({error, {not_found, {chain, ChainName}}}, ?AUTHN:lookup_chain(ChainName)), + ok; +t_chain({'end', _Config}) -> + ?AUTHN:delete_chain(test), + ?AUTHN:deregister_providers([{password_based, built_in_database}]), ok. t_authenticator({'init', Config}) -> @@ -143,13 +155,6 @@ t_authenticator(Config) when is_list(Config) -> enable => true }, - % Create an authenticator when the authentication chain does not exist - ?assertEqual( - {error, {not_found, {chain, ChainName}}}, - ?AUTHN:create_authenticator(ChainName, AuthenticatorConfig1) - ), - - ?AUTHN:create_chain(ChainName), % Create an authenticator when the provider does not exist ?assertEqual( @@ -183,11 +188,14 @@ t_authenticator(Config) when is_list(Config) -> ?assertEqual(ok, ?AUTHN:delete_authenticator(ChainName, ID1)), ?assertEqual( - {error, {not_found, {authenticator, ID1}}}, + {error, {not_found, {chain, test}}}, ?AUTHN:update_authenticator(ChainName, ID1, AuthenticatorConfig1) ), - ?assertMatch({ok, []}, ?AUTHN:list_authenticators(ChainName)), + ?assertMatch( + {error, {not_found, {chain, ChainName}}}, + ?AUTHN:list_authenticators(ChainName) + ), % Multiple authenticators exist at the same time AuthNType2 = ?config("auth2"), @@ -253,7 +261,6 @@ t_authenticate(Config) when is_list(Config) -> backend => built_in_database, enable => true }, - ?AUTHN:create_chain(ListenerID), ?assertMatch({ok, _}, ?AUTHN:create_authenticator(ListenerID, AuthenticatorConfig)), ?assertEqual( @@ -352,7 +359,7 @@ t_update_config(Config) when is_list(Config) -> ), ?assertEqual( - {error, {not_found, {authenticator, ID2}}}, + {error, {not_found, {chain, Global}}}, ?AUTHN:lookup_authenticator(Global, ID2) ), @@ -427,7 +434,15 @@ t_restart({'init', Config}) -> t_restart(Config) when is_list(Config) -> ?assertEqual({ok, []}, ?AUTHN:list_chain_names()), - ?AUTHN:create_chain(test_chain), + %% to create a chain we need create an authenticator + AuthenticatorConfig = #{ + mechanism => password_based, + backend => built_in_database, + enable => true + }, + register_provider({password_based, built_in_database}, ?MODULE), + ?AUTHN:create_authenticator(test_chain, AuthenticatorConfig), + ?assertEqual({ok, [test_chain]}, ?AUTHN:list_chain_names()), ok = supervisor:terminate_child(emqx_authentication_sup, ?AUTHN), @@ -436,6 +451,7 @@ t_restart(Config) when is_list(Config) -> ?assertEqual({ok, [test_chain]}, ?AUTHN:list_chain_names()); t_restart({'end', _Config}) -> ?AUTHN:delete_chain(test_chain), + ?AUTHN:deregister_providers([{password_based, built_in_database}]), ok. t_convert_certs({_, Config}) -> diff --git a/apps/emqx_authn/i18n/emqx_authn_api_i18n.conf b/apps/emqx_authn/i18n/emqx_authn_api_i18n.conf index a31a335d5..f42ff7417 100644 --- a/apps/emqx_authn/i18n/emqx_authn_api_i18n.conf +++ b/apps/emqx_authn/i18n/emqx_authn_api_i18n.conf @@ -49,13 +49,6 @@ emqx_authn_api { } } - listeners_listener_id_authentication_delete { - desc { - en: """Delete listener-specific authentication.""" - zh: """删除特定于侦听器的身份验证。""" - } - } - listeners_listener_id_authentication_post { desc { en: """Create authenticator for listener authentication.""" diff --git a/apps/emqx_authn/src/emqx_authn_api.erl b/apps/emqx_authn/src/emqx_authn_api.erl index b01327ff8..badf8024d 100644 --- a/apps/emqx_authn/src/emqx_authn_api.erl +++ b/apps/emqx_authn/src/emqx_authn_api.erl @@ -248,15 +248,6 @@ schema("/listeners/:listener_id/authentication") -> ) } }, - delete => #{ - tags => ?API_TAGS_SINGLE, - description => ?DESC(listeners_listener_id_authentication_delete), - parameters => [param_listener_id()], - responses => #{ - 204 => <<"Authentication chain deleted">>, - 404 => error_codes([?NOT_FOUND], <<"Not Found">>) - } - }, post => #{ tags => ?API_TAGS_SINGLE, description => ?DESC(listeners_listener_id_authentication_post), @@ -610,13 +601,6 @@ listener_authenticators(get, #{bindings := #{listener_id := ListenerID}}) -> fun(Type, Name, _) -> list_authenticators([listeners, Type, Name, authentication]) end - ); -listener_authenticators(delete, #{bindings := #{listener_id := ListenerID}}) -> - with_listener( - ListenerID, - fun(Type, Name, ChainName) -> - delete_authenticators([listeners, Type, Name, authentication], ChainName) - end ). listener_authenticator(get, #{bindings := #{listener_id := ListenerID, id := AuthenticatorID}}) -> @@ -879,7 +863,8 @@ lookup_from_local_node(ChainName, AuthenticatorID) -> NodeId = node(self()), case emqx_authentication:lookup_authenticator(ChainName, AuthenticatorID) of {ok, #{provider := Provider, state := State}} -> - Metrics = emqx_metrics_worker:get_metrics(authn_metrics, AuthenticatorID), + MetricsId = emqx_authentication:metrics_id(ChainName, AuthenticatorID), + Metrics = emqx_metrics_worker:get_metrics(authn_metrics, MetricsId), case lists:member(Provider, resource_provider()) of false -> {ok, {NodeId, connected, Metrics, #{}}}; @@ -1036,16 +1021,6 @@ delete_authenticator(ConfKeyPath, ChainName, AuthenticatorID) -> serialize_error(Reason) end. -delete_authenticators(ConfKeyPath, ChainName) -> - case update_config(ConfKeyPath, {delete_authenticators, ChainName}) of - {ok, _} -> - {204}; - {error, {_PrePostConfigUpdate, emqx_authentication, Reason}} -> - serialize_error(Reason); - {error, Reason} -> - serialize_error(Reason) - end. - move_authenticator(ConfKeyPath, ChainName, AuthenticatorID, Position) -> case parse_position(Position) of {ok, NPosition} -> diff --git a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl index 648fb8a0f..9d999c820 100644 --- a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl @@ -674,6 +674,11 @@ t_switch_to_global_chain(_) -> uri([listeners, "tcp:default", ?CONF_NS]), emqx_authn_test_lib:built_in_database_example() ), + {ok, 200, _} = request( + post, + uri([listeners, "tcp:default", ?CONF_NS]), + maps:put(enable, false, emqx_authn_test_lib:http_example()) + ), GlobalUser = #{user_id => <<"global_user">>, password => <<"p1">>}, @@ -716,29 +721,46 @@ t_switch_to_global_chain(_) -> {ok, 204, _} = request( delete, - uri([listeners, "tcp:default", ?CONF_NS]) + uri([listeners, "tcp:default", ?CONF_NS, "password_based:built_in_database"]) ), - %% Listener user should not be OK — local chain removed + %% Now listener has only disabled authenticators, should allow anonymous access {ok, Client2} = emqtt:start_link([ + {username, <<"any_user">>}, + {password, <<"any_password">>} + ]), + ?assertMatch( + {ok, _}, + emqtt:connect(Client2) + ), + ok = emqtt:disconnect(Client2), + + {ok, 204, _} = request( + delete, + uri([listeners, "tcp:default", ?CONF_NS, "password_based:http"]) + ), + + %% Local chain is empty now and should be removed + %% Listener user should not be OK + {ok, Client3} = emqtt:start_link([ {username, <<"listener_user">>}, {password, <<"p1">>} ]), ?assertMatch( {error, {unauthorized_client, _}}, - emqtt:connect(Client2) + emqtt:connect(Client3) ), %% Global user should be now OK, switched back to the global chain - {ok, Client3} = emqtt:start_link([ + {ok, Client4} = emqtt:start_link([ {username, <<"global_user">>}, {password, <<"p1">>} ]), ?assertMatch( {ok, _}, - emqtt:connect(Client3) + emqtt:connect(Client4) ), - ok = emqtt:disconnect(Client3). + ok = emqtt:disconnect(Client4). %%------------------------------------------------------------------------------ %% Helpers diff --git a/apps/emqx_authn/test/emqx_authn_http_SUITE.erl b/apps/emqx_authn/test/emqx_authn_http_SUITE.erl index dce4522f3..a22c7765e 100644 --- a/apps/emqx_authn/test/emqx_authn_http_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_http_SUITE.erl @@ -101,7 +101,10 @@ t_create_invalid(_Config) -> throw:Error -> {error, Error} end, - {ok, []} = emqx_authentication:list_authenticators(?GLOBAL) + ?assertEqual( + {error, {not_found, {chain, ?GLOBAL}}}, + emqx_authentication:list_authenticators(?GLOBAL) + ) end, InvalidConfigs ). diff --git a/apps/emqx_authn/test/emqx_authn_mongo_SUITE.erl b/apps/emqx_authn/test/emqx_authn_mongo_SUITE.erl index 1f3667efb..dfdc0fdd1 100644 --- a/apps/emqx_authn/test/emqx_authn_mongo_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_mongo_SUITE.erl @@ -95,7 +95,10 @@ t_create_invalid(_Config) -> {create_authenticator, ?GLOBAL, Config} ), - {ok, []} = emqx_authentication:list_authenticators(?GLOBAL) + ?assertEqual( + {error, {not_found, {chain, ?GLOBAL}}}, + emqx_authentication:list_authenticators(?GLOBAL) + ) end, InvalidConfigs ). diff --git a/apps/emqx_authn/test/emqx_authn_mysql_SUITE.erl b/apps/emqx_authn/test/emqx_authn_mysql_SUITE.erl index fd87335b4..c8bee9223 100644 --- a/apps/emqx_authn/test/emqx_authn_mysql_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_mysql_SUITE.erl @@ -113,7 +113,10 @@ t_create_invalid(_Config) -> {create_authenticator, ?GLOBAL, Config} ), emqx_authn_test_lib:delete_config(?ResourceID), - {ok, _} = emqx_authentication:list_authenticators(?GLOBAL) + ?assertEqual( + {error, {not_found, {chain, ?GLOBAL}}}, + emqx_authentication:list_authenticators(?GLOBAL) + ) end, InvalidConfigs ). diff --git a/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl b/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl index 60b789bcf..618768e42 100644 --- a/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_pgsql_SUITE.erl @@ -114,7 +114,10 @@ t_create_invalid(_Config) -> {create_authenticator, ?GLOBAL, Config} ), emqx_authn_test_lib:delete_config(?ResourceID), - {ok, []} = emqx_authentication:list_authenticators(?GLOBAL) + ?assertEqual( + {error, {not_found, {chain, ?GLOBAL}}}, + emqx_authentication:list_authenticators(?GLOBAL) + ) end, InvalidConfigs ). diff --git a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl index 163044c21..1ff391729 100644 --- a/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_redis_SUITE.erl @@ -85,8 +85,10 @@ end_per_suite(_Config) -> %%------------------------------------------------------------------------------ t_create(_Config) -> - {ok, []} = emqx_authentication:list_authenticators(?GLOBAL), - + ?assertEqual( + {error, {not_found, {chain, ?GLOBAL}}}, + emqx_authentication:list_authenticators(?GLOBAL) + ), AuthConfig = raw_redis_auth_config(), {ok, _} = emqx:update_config( ?PATH, @@ -119,7 +121,10 @@ t_create_invalid(_Config) -> {create_authenticator, ?GLOBAL, Config} ), - {ok, []} = emqx_authentication:list_authenticators(?GLOBAL) + ?assertEqual( + {error, {not_found, {chain, ?GLOBAL}}}, + emqx_authentication:list_authenticators(?GLOBAL) + ) end, InvalidConfigs ), @@ -139,7 +144,10 @@ t_create_invalid(_Config) -> {create_authenticator, ?GLOBAL, Config} ), emqx_authn_test_lib:delete_config(?ResourceID), - {ok, []} = emqx_authentication:list_authenticators(?GLOBAL) + ?assertEqual( + {error, {not_found, {chain, ?GLOBAL}}}, + emqx_authentication:list_authenticators(?GLOBAL) + ) end, InvalidConfigs1 ). diff --git a/apps/emqx_authn/test/emqx_enhanced_authn_scram_mnesia_SUITE.erl b/apps/emqx_authn/test/emqx_enhanced_authn_scram_mnesia_SUITE.erl index 8e1963791..86558e5ec 100644 --- a/apps/emqx_authn/test/emqx_enhanced_authn_scram_mnesia_SUITE.erl +++ b/apps/emqx_authn/test/emqx_enhanced_authn_scram_mnesia_SUITE.erl @@ -87,7 +87,10 @@ t_create_invalid(_Config) -> {create_authenticator, ?GLOBAL, InvalidConfig} ), - {ok, []} = emqx_authentication:list_authenticators(?GLOBAL). + ?assertEqual( + {error, {not_found, {chain, ?GLOBAL}}}, + emqx_authentication:list_authenticators(?GLOBAL) + ). t_authenticate(_Config) -> Algorithm = sha512, diff --git a/apps/emqx_gateway/src/emqx_gateway_insta_sup.erl b/apps/emqx_gateway/src/emqx_gateway_insta_sup.erl index a065ea8fd..c47b8a050 100644 --- a/apps/emqx_gateway/src/emqx_gateway_insta_sup.erl +++ b/apps/emqx_gateway/src/emqx_gateway_insta_sup.erl @@ -277,39 +277,19 @@ authn_conf(Conf) -> maps:get(authentication, Conf, #{enable => false}). do_create_authn_chain(ChainName, AuthConf) -> - case ensure_chain(ChainName) of - ok -> - case emqx_authentication:create_authenticator(ChainName, AuthConf) of - {ok, _} -> - ok; - {error, Reason} -> - ?SLOG(error, #{ - msg => "failed_to_create_authenticator", - chain_name => ChainName, - reason => Reason, - config => AuthConf - }), - throw({badauth, Reason}) - end; + case emqx_authentication:create_authenticator(ChainName, AuthConf) of + {ok, _} -> + ok; {error, Reason} -> ?SLOG(error, #{ - msg => "failed_to_create_authn_chanin", + msg => "failed_to_create_authenticator", chain_name => ChainName, - reason => Reason + reason => Reason, + config => AuthConf }), throw({badauth, Reason}) end. -ensure_chain(ChainName) -> - case emqx_authentication:create_chain(ChainName) of - {ok, _ChainInfo} -> - ok; - {error, {already_exists, _}} -> - ok; - {error, Reason} -> - {error, Reason} - end. - do_deinit_authn(Names) -> lists:foreach( fun(ChainName) ->