Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 23 additions & 11 deletions apps/emqx_management/src/emqx_mgmt_auth.erl
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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]} ->
Expand Down Expand Up @@ -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]} ->
Expand Down
9 changes: 9 additions & 0 deletions apps/emqx_management/test/emqx_mgmt_api_api_keys_SUITE.erl
Expand Up @@ -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(_) ->
Expand Down
5 changes: 5 additions & 0 deletions 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