Merge pull request #12566 from lafirest/fix/same_api

fix(api_key): enhanced bootstrap files for REST API keys

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]} ->

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(_) ->

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