diff --git a/apps/emqx_management/src/emqx_mgmt_auth.erl b/apps/emqx_management/src/emqx_mgmt_auth.erl index 7795e0bbe..4e44f3f1f 100644 --- a/apps/emqx_management/src/emqx_mgmt_auth.erl +++ b/apps/emqx_management/src/emqx_mgmt_auth.erl @@ -299,14 +299,6 @@ maybe_cleanup_api_key(#?APP{name = Name, api_key = ApiKey}) -> info => <<"The last `KEY:SECRET` in bootstrap file will be used.">> }), ok; - [_App1] -> - ?SLOG(info, #{ - msg => "update_apikey_name_from_old_version", - info => - <<"Update ApiKey name with new name rule, see also: ", - "https://github.com/emqx/emqx/pull/11798">> - }), - ok; Existed -> %% Duplicated or upgraded from old version: %% Which `Name` and `ApiKey` are not related in old version. @@ -316,11 +308,16 @@ maybe_cleanup_api_key(#?APP{name = Name, api_key = ApiKey}) -> %% Use `from_bootstrap_file_` as the prefix, and the first 16 digits of the %% sha512 hexadecimal value of the `ApiKey` as the suffix to form the name of the apikey. %% e.g. The name of the apikey: `example-api-key:secret_xxxx` is `from_bootstrap_file_53280fb165b6cd37` + + %% Note for EMQX-11844: + %% emqx.conf has the highest priority + %% if there is a key conflict, delete the old one and keep the key which from the bootstrap filex ?SLOG(info, #{ msg => "duplicated_apikey_detected", - info => <<"Delete duplicated apikeys and write a new one from bootstrap file">> + info => <<"Delete duplicated apikeys and write a new one from bootstrap file">>, + keys => [EName || #?APP{name = EName} <- Existed] }), - _ = lists:map( + lists:foreach( fun(#?APP{name = N}) -> ok = mnesia:delete({?APP, N}) end, Existed ), ok @@ -385,7 +382,7 @@ init_bootstrap_file(File, Dev, MP) -> -define(FROM_BOOTSTRAP_FILE_PREFIX, <<"from_bootstrap_file_">>). add_bootstrap_file(File, Dev, MP, Line) -> - case file:read_line(Dev) of + case read_line(Dev) of {ok, Bin} -> case parse_bootstrap_line(Bin, MP) of {ok, [ApiKey, ApiSecret, Role]} -> @@ -418,12 +415,27 @@ add_bootstrap_file(File, Dev, MP, Line) -> ), throw(#{file => File, line => Line, content => Bin, reason => Reason}) end; + skip -> + add_bootstrap_file(File, Dev, MP, Line + 1); eof -> ok; {error, Reason} -> throw(#{file => File, line => Line, reason => Reason}) end. +read_line(Dev) -> + case file:read_line(Dev) of + {ok, Bin} -> + case string:trim(Bin) of + <<>> -> + skip; + Result -> + {ok, Result} + end; + Result -> + Result + end. + parse_bootstrap_line(Bin, MP) -> case re:run(Bin, MP, [global, {capture, all_but_first, binary}]) of {match, [[_ApiKey, _ApiSecret] = Args]} -> diff --git a/apps/emqx_management/test/emqx_mgmt_api_api_keys_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_api_keys_SUITE.erl index 760ab1732..8873e98e3 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_api_keys_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_api_keys_SUITE.erl @@ -97,6 +97,15 @@ t_bootstrap_file(_) -> ?assertEqual(ok, auth_authorize(TestPath, <<"test-1">>, <<"secret-11">>)), ?assertMatch({error, _}, auth_authorize(TestPath, <<"test-2">>, <<"secret-12">>)), update_file(<<>>), + + %% skip the empty line + Bin2 = <<"test-3:new-secret-1\n\n\n \ntest-4:new-secret-2">>, + ok = file:write_file(File, Bin2), + update_file(File), + ?assertMatch({error, _}, auth_authorize(TestPath, <<"test-3">>, <<"secret-1">>)), + ?assertMatch({error, _}, auth_authorize(TestPath, <<"test-4">>, <<"secret-2">>)), + ?assertEqual(ok, auth_authorize(TestPath, <<"test-3">>, <<"new-secret-1">>)), + ?assertEqual(ok, auth_authorize(TestPath, <<"test-4">>, <<"new-secret-2">>)), ok. t_bootstrap_file_override(_) -> diff --git a/changes/ce/fix-12566.en.md b/changes/ce/fix-12566.en.md new file mode 100644 index 000000000..2d658855f --- /dev/null +++ b/changes/ce/fix-12566.en.md @@ -0,0 +1,5 @@ +Enhanced the bootstrap file for REST API keys: + +- now the empty line will be skipped instead of throw an error + +- keys from bootstrap file now have highest priority, if one of them is conflict with an old key, the old key will be deleted