From 95275611257334541575214642e854a6a22ed4ec Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Sat, 14 May 2022 11:21:45 +0200 Subject: [PATCH 1/4] refactor: ensure auth mechanism is binary --- apps/emqx/src/emqx_authentication_config.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx/src/emqx_authentication_config.erl b/apps/emqx/src/emqx_authentication_config.erl index 046068f6a..bed8bbd31 100644 --- a/apps/emqx/src/emqx_authentication_config.erl +++ b/apps/emqx/src/emqx_authentication_config.erl @@ -305,7 +305,7 @@ authenticator_id(#{mechanism := Mechanism}) -> authenticator_id(#{<<"mechanism">> := Mechanism, <<"backend">> := Backend}) -> <>; authenticator_id(#{<<"mechanism">> := Mechanism}) -> - Mechanism; + to_bin(Mechanism); authenticator_id(_C) -> throw({missing_parameter, #{name => mechanism}}). From 00198abfd7ea5f6c5395b7cda5d5cf4728a8c506 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Sat, 14 May 2022 12:18:55 +0200 Subject: [PATCH 2/4] chore: hint how to fix fmt in pre-commit hook --- scripts/git-hook-pre-commit.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/git-hook-pre-commit.sh b/scripts/git-hook-pre-commit.sh index dc4e13e93..a5e441d05 100755 --- a/scripts/git-hook-pre-commit.sh +++ b/scripts/git-hook-pre-commit.sh @@ -11,4 +11,7 @@ if [[ "${files_dirty}" == '' ]] && [[ "${files_cached}" == '' ]]; then fi files="$(echo -e "${files_dirty} \n ${files_cached}" | xargs)" # shellcheck disable=SC2086 -./scripts/erlfmt $OPT $files +if ! (./scripts/erlfmt $OPT $files); then + echo "EXECUTE 'make fmt' to fix" >&2 + exit 1 +fi From 522bd935ba21e92333c53cf89daa64f32f45a567 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Sat, 14 May 2022 12:23:05 +0200 Subject: [PATCH 3/4] fix(authn): check authenticator config existence in pre-update callback prior to this change, the authenticator existence check was done in the post-update callback, this causes confusion as teh list already contains duplication. --- apps/emqx/src/emqx_authentication_config.erl | 21 ++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/apps/emqx/src/emqx_authentication_config.erl b/apps/emqx/src/emqx_authentication_config.erl index bed8bbd31..93fcafc35 100644 --- a/apps/emqx/src/emqx_authentication_config.erl +++ b/apps/emqx/src/emqx_authentication_config.erl @@ -71,9 +71,15 @@ pre_config_update(_, UpdateReq, OldConfig) -> end. do_pre_config_update({create_authenticator, ChainName, Config}, OldConfig) -> - CertsDir = certs_dir(ChainName, Config), - NConfig = convert_certs(CertsDir, Config), - {ok, OldConfig ++ [NConfig]}; + NewId = authenticator_id(Config), + case lists:filter(fun(OldConfig0) -> authenticator_id(OldConfig0) =:= NewId end, OldConfig) of + [] -> + CertsDir = certs_dir(ChainName, Config), + NConfig = convert_certs(CertsDir, Config), + {ok, OldConfig ++ [NConfig]}; + [_] -> + {error, {already_exists, {authenticator, NewId}}} + end; do_pre_config_update({delete_authenticator, _ChainName, AuthenticatorID}, OldConfig) -> NewConfig = lists:filter( fun(OldConfig0) -> @@ -257,9 +263,12 @@ clear_certs(CertsDir, Config) -> ok = emqx_tls_lib:delete_ssl_files(CertsDir, undefined, OldSSL). get_authenticator_config(AuthenticatorID, AuthenticatorsConfig) -> - case [C0 || C0 <- AuthenticatorsConfig, AuthenticatorID == authenticator_id(C0)] of - [C | _] -> C; - [] -> {error, not_found} + case + lists:filter(fun(C) -> AuthenticatorID =:= authenticator_id(C) end, AuthenticatorsConfig) + of + [C] -> C; + [] -> {error, not_found}; + _ -> error({duplicated_authenticator_id, AuthenticatorsConfig}) end. split_by_id(ID, AuthenticatorsConfig) -> From ed0ab70aafaa6d104dd4ac698204748ee6a4eb62 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Sat, 14 May 2022 12:23:33 +0200 Subject: [PATCH 4/4] test: fix flaky-ness of authn api tests if api test suite runs after the JWT suite, it failes as the config is not clean --- apps/emqx_authn/test/emqx_authn_api_SUITE.erl | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl index 38e6c21b7..86fc1f66c 100644 --- a/apps/emqx_authn/test/emqx_authn_api_SUITE.erl +++ b/apps/emqx_authn/test/emqx_authn_api_SUITE.erl @@ -55,6 +55,7 @@ init_per_testcase(_, Config) -> Config. init_per_suite(Config) -> + emqx_config:erase(?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_BINARY), _ = application:load(emqx_conf), ok = emqx_common_test_helpers:start_apps( [emqx_authn, emqx_dashboard],