Merge pull request #11811 from lafirest/fix/api_rbac

fix(rbac): for compatibility with old data schema, extend the existing field as an extra
This commit is contained in:
lafirest 2023-10-27 17:56:54 +08:00 committed by GitHub
commit e63602c4b0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 154 additions and 78 deletions

View File

@ -34,7 +34,7 @@
-define(DEFAULT_APP_SECRET, <<"default_app_secret">>).
%% from emqx_dashboard/include/emqx_dashboard_rbac.hrl
-define(ROLE_API_SUPERUSER, <<"api_administrator">>).
-define(ROLE_API_SUPERUSER, <<"administrator">>).
request_api(Method, Url, Auth) ->
request_api(Method, Url, [], Auth, []).

View File

@ -25,9 +25,9 @@
-define(ROLE_SUPERUSER, <<"administrator">>).
-define(ROLE_DEFAULT, ?ROLE_SUPERUSER).
-define(ROLE_API_VIEWER, <<"api_viewer">>).
-define(ROLE_API_SUPERUSER, <<"api_administrator">>).
-define(ROLE_API_PUBLISHER, <<"api_publisher">>).
-define(ROLE_API_VIEWER, <<"viewer">>).
-define(ROLE_API_SUPERUSER, <<"administrator">>).
-define(ROLE_API_PUBLISHER, <<"publisher">>).
-define(ROLE_API_DEFAULT, ?ROLE_API_SUPERUSER).
-endif.

View File

@ -59,12 +59,8 @@ valid_role(Type, Role) ->
%% ===================================================================
check_rbac(?ROLE_SUPERUSER, _, _, _) ->
true;
check_rbac(?ROLE_API_SUPERUSER, _, _, _) ->
true;
check_rbac(?ROLE_VIEWER, <<"GET">>, _, _) ->
true;
check_rbac(?ROLE_API_VIEWER, <<"GET">>, _, _) ->
true;
check_rbac(?ROLE_API_PUBLISHER, <<"POST">>, <<"/publish">>, _) ->
true;
check_rbac(?ROLE_API_PUBLISHER, <<"POST">>, <<"/publish/bulk">>, _) ->

View File

@ -38,7 +38,7 @@
-export([authorize/4]).
-export([post_config_update/5]).
-export([backup_tables/0, validate_mnesia_backup/1, migrate_mnesia_backup/1]).
-export([backup_tables/0, validate_mnesia_backup/1]).
%% Internal exports (RPC)
-export([
@ -53,18 +53,17 @@
-endif.
-define(APP, emqx_app).
-type api_user_role() :: binary().
-record(?APP, {
name = <<>> :: binary() | '_',
api_key = <<>> :: binary() | '_',
api_secret_hash = <<>> :: binary() | '_',
enable = true :: boolean() | '_',
desc = <<>> :: binary() | '_',
%% Since v5.4.0 the `desc` has changed to `extra`
%% desc = <<>> :: binary() | '_',
extra = #{} :: binary() | map() | '_',
expired_at = 0 :: integer() | undefined | infinity | '_',
created_at = 0 :: integer() | '_',
role = ?ROLE_DEFAULT :: api_user_role() | '_',
extra = #{} :: map() | '_'
created_at = 0 :: integer() | '_'
}).
mnesia(boot) ->
@ -75,8 +74,7 @@ mnesia(boot) ->
{storage, disc_copies},
{record_name, ?APP},
{attributes, Fields}
]),
maybe_migrate_table(Fields).
]).
%%--------------------------------------------------------------------
%% Data backup
@ -87,11 +85,12 @@ backup_tables() -> [?APP].
validate_mnesia_backup({schema, _Tab, CreateList} = Schema) ->
case emqx_mgmt_data_backup:default_validate_mnesia_backup(Schema) of
ok ->
{ok, over};
ok;
_ ->
case proplists:get_value(attributes, CreateList) of
%% Since v5.4.0 the `desc` has changed to `extra`
[name, api_key, api_secret_hash, enable, desc, expired_at, created_at] ->
{ok, migrate};
ok;
Fields ->
{error, {unknow_fields, Fields}}
end
@ -99,20 +98,6 @@ validate_mnesia_backup({schema, _Tab, CreateList} = Schema) ->
validate_mnesia_backup(_Other) ->
ok.
migrate_mnesia_backup({schema, Tab, CreateList}) ->
case proplists:get_value(attributes, CreateList) of
[name, api_key, api_secret_hash, enable, desc, expired_at, created_at] = Fields ->
NewFields = Fields ++ [role, extra],
CreateList2 = lists:keyreplace(
attributes, 1, CreateList, {attributes, NewFields}
),
{ok, {schema, Tab, CreateList2}};
Fields ->
{error, {unknow_fields, Fields}}
end;
migrate_mnesia_backup(Data) ->
{ok, do_table_migrate(Data)}.
post_config_update([api_key], _Req, NewConf, _OldConf, _AppEnvs) ->
#{bootstrap_file := File} = NewConf,
case init_bootstrap_file(File) of
@ -158,13 +143,13 @@ do_update(Name, Enable, ExpiredAt, Desc, Role) ->
case mnesia:read(?APP, Name, write) of
[] ->
mnesia:abort(not_found);
[App0 = #?APP{enable = Enable0, desc = Desc0}] ->
[App0 = #?APP{enable = Enable0, extra = Extra0}] ->
#{desc := Desc0} = Extra = normalize_extra(Extra0),
App =
App0#?APP{
expired_at = ExpiredAt,
enable = ensure_not_undefined(Enable, Enable0),
desc = ensure_not_undefined(Desc, Desc0),
role = Role
extra = Extra#{desc := ensure_not_undefined(Desc, Desc0), role := Role}
},
ok = mnesia:write(App),
to_map(App)
@ -220,10 +205,10 @@ find_by_api_key(ApiKey) ->
case mria:ro_transaction(?COMMON_SHARD, Fun) of
{atomic, [
#?APP{
api_secret_hash = SecretHash, enable = Enable, expired_at = ExpiredAt, role = Role
api_secret_hash = SecretHash, enable = Enable, expired_at = ExpiredAt, extra = Extra
}
]} ->
{ok, Enable, ExpiredAt, SecretHash, Role};
{ok, Enable, ExpiredAt, SecretHash, get_role(Extra)};
_ ->
{error, "not_found"}
end.
@ -234,15 +219,16 @@ ensure_not_undefined(New, _Old) -> New.
to_map(Apps) when is_list(Apps) ->
[to_map(App) || App <- Apps];
to_map(#?APP{
name = N, api_key = K, enable = E, expired_at = ET, created_at = CT, desc = D, role = Role
name = N, api_key = K, enable = E, expired_at = ET, created_at = CT, extra = Extra0
}) ->
#{role := Role, desc := Desc} = normalize_extra(Extra0),
#{
name => N,
api_key => K,
enable => E,
expired_at => ET,
created_at => CT,
desc => D,
desc => Desc,
expired => is_expired(ET),
role => Role
}.
@ -256,11 +242,10 @@ create_app(Name, ApiSecret, Enable, ExpiredAt, Desc, Role) ->
name = Name,
enable = Enable,
expired_at = ExpiredAt,
desc = Desc,
extra = #{desc => Desc, role => Role},
created_at = erlang:system_time(second),
api_secret_hash = emqx_dashboard_admin:hash(ApiSecret),
api_key = list_to_binary(emqx_utils:gen_id(16)),
role = Role
api_key = list_to_binary(emqx_utils:gen_id(16))
},
case create_app(App) of
{ok, Res} ->
@ -269,7 +254,7 @@ create_app(Name, ApiSecret, Enable, ExpiredAt, Desc, Role) ->
Error
end.
create_app(App = #?APP{api_key = ApiKey, name = Name, role = Role}) ->
create_app(App = #?APP{api_key = ApiKey, name = Name, extra = #{role := Role}}) ->
case valid_role(Role) of
ok ->
trans(fun ?MODULE:do_create_app/3, [App, ApiKey, Name]);
@ -328,7 +313,7 @@ init_bootstrap_file(<<>>) ->
init_bootstrap_file(File) ->
case file:open(File, [read, binary]) of
{ok, Dev} ->
{ok, MP} = re:compile(<<"(\.+):(\.+$)">>, [ungreedy]),
{ok, MP} = re:compile(<<"(\.+):(\.+)(?::(\.+))?$">>, [ungreedy]),
init_bootstrap_file(File, Dev, MP);
{error, Reason0} ->
Reason = emqx_utils:explain_posix(Reason0),
@ -358,13 +343,13 @@ init_bootstrap_file(File, Dev, MP) ->
add_bootstrap_file(File, Dev, MP, Line) ->
case file:read_line(Dev) of
{ok, Bin} ->
case re:run(Bin, MP, [global, {capture, all_but_first, binary}]) of
{match, [[AppKey, ApiSecret]]} ->
case parse_bootstrap_line(Bin, MP) of
{ok, [AppKey, ApiSecret, Role]} ->
App =
#?APP{
enable = true,
expired_at = infinity,
desc = ?BOOTSTRAP_TAG,
extra = #{desc => ?BOOTSTRAP_TAG, role => Role},
created_at = erlang:system_time(second),
api_secret_hash = emqx_dashboard_admin:hash(ApiSecret),
api_key = AppKey
@ -375,8 +360,7 @@ add_bootstrap_file(File, Dev, MP, Line) ->
{error, Reason} ->
throw(#{file => File, line => Line, content => Bin, reason => Reason})
end;
_ ->
Reason = "invalid_format",
{error, Reason} ->
?SLOG(
error,
#{
@ -395,6 +379,33 @@ add_bootstrap_file(File, Dev, MP, Line) ->
throw(#{file => File, line => Line, reason => Reason})
end.
parse_bootstrap_line(Bin, MP) ->
case re:run(Bin, MP, [global, {capture, all_but_first, binary}]) of
{match, [[_AppKey, _ApiSecret] = Args]} ->
{ok, Args ++ [?ROLE_API_DEFAULT]};
{match, [[_AppKey, _ApiSecret, Role] = Args]} ->
case valid_role(Role) of
ok ->
{ok, Args};
_Error ->
{error, {"invalid_role", Role}}
end;
_ ->
{error, "invalid_format"}
end.
get_role(#{role := Role}) ->
Role;
%% Before v5.4.0,
%% the field in the position of the `extra` is `desc` which is a binary for description
get_role(_Desc) ->
?ROLE_API_DEFAULT.
normalize_extra(Map) when is_map(Map) ->
Map;
normalize_extra(Desc) ->
#{desc => Desc, role => ?ROLE_API_DEFAULT}.
-if(?EMQX_RELEASE_EDITION == ee).
check_rbac(Req, ApiKey, Role) ->
case emqx_dashboard_rbac:check_rbac(Req, ApiKey, Role) of
@ -424,28 +435,3 @@ valid_role(_) ->
{error, <<"Role does not exist">>}.
-endif.
maybe_migrate_table(Fields) ->
case mnesia:table_info(?APP, attributes) =:= Fields of
true ->
ok;
false ->
TransFun = fun do_table_migrate/1,
{atomic, ok} = mnesia:transform_table(?APP, TransFun, Fields, ?APP),
ok
end.
do_table_migrate({?APP, Name, Key, Hash, Enable, Desc, ExpiredAt, CreatedAt}) ->
#?APP{
name = Name,
api_key = Key,
api_secret_hash = Hash,
enable = Enable,
desc = Desc,
expired_at = ExpiredAt,
created_at = CreatedAt,
role = ?ROLE_API_DEFAULT,
extra = #{}
};
do_table_migrate(#?APP{} = App) ->
App.

View File

@ -39,7 +39,7 @@ groups() ->
[
{parallel, [parallel], [t_create, t_update, t_delete, t_authorize, t_create_unexpired_app]},
{parallel, [parallel], ?EE_CASES},
{sequence, [], [t_bootstrap_file, t_create_failed]}
{sequence, [], [t_bootstrap_file, t_bootstrap_file_with_role, t_create_failed]}
].
init_per_suite(Config) ->
@ -86,6 +86,92 @@ t_bootstrap_file(_) ->
update_file(<<>>),
ok.
-if(?EMQX_RELEASE_EDITION == ee).
t_bootstrap_file_with_role(_) ->
Search = fun(Name) ->
lists:search(
fun(#{api_key := AppName}) ->
AppName =:= Name
end,
emqx_mgmt_auth:list()
)
end,
Bin = <<"role-1:role-1:viewer\nrole-2:role-2:administrator\nrole-3:role-3">>,
File = "./bootstrap_api_keys.txt",
ok = file:write_file(File, Bin),
update_file(File),
?assertMatch(
{value, #{api_key := <<"role-1">>, role := <<"viewer">>}},
Search(<<"role-1">>)
),
?assertMatch(
{value, #{api_key := <<"role-2">>, role := <<"administrator">>}},
Search(<<"role-2">>)
),
?assertMatch(
{value, #{api_key := <<"role-3">>, role := <<"administrator">>}},
Search(<<"role-3">>)
),
%% bad role
BadBin = <<"role-4:secret-11:bad\n">>,
ok = file:write_file(File, BadBin),
update_file(File),
?assertEqual(
false,
Search(<<"role-4">>)
),
ok.
-else.
t_bootstrap_file_with_role(_) ->
Search = fun(Name) ->
lists:search(
fun(#{api_key := AppName}) ->
AppName =:= Name
end,
emqx_mgmt_auth:list()
)
end,
Bin = <<"role-1:role-1:administrator\nrole-2:role-2">>,
File = "./bootstrap_api_keys.txt",
ok = file:write_file(File, Bin),
update_file(File),
?assertMatch(
{value, #{api_key := <<"role-1">>, role := <<"administrator">>}},
Search(<<"role-1">>)
),
?assertMatch(
{value, #{api_key := <<"role-2">>, role := <<"administrator">>}},
Search(<<"role-2">>)
),
%% only administrator
OtherRoleBin = <<"role-3:role-3:viewer\n">>,
ok = file:write_file(File, OtherRoleBin),
update_file(File),
?assertEqual(
false,
Search(<<"role-3">>)
),
%% bad role
BadBin = <<"role-4:secret-11:bad\n">>,
ok = file:write_file(File, BadBin),
update_file(File),
?assertEqual(
false,
Search(<<"role-4">>)
),
ok.
-endif.
auth_authorize(Path, Key, Secret) ->
FakePath = erlang:list_to_binary(emqx_dashboard_swagger:relative_uri("/fake")),
FakeReq = #{method => <<"GET">>, path => FakePath},

View File

@ -23,7 +23,7 @@
-include_lib("snabbkaffe/include/snabbkaffe.hrl").
-define(ROLE_SUPERUSER, <<"administrator">>).
-define(ROLE_API_SUPERUSER, <<"api_administrator">>).
-define(ROLE_API_SUPERUSER, <<"administrator">>).
-define(BOOTSTRAP_BACKUP, "emqx-export-test-bootstrap-ce.tar.gz").
all() ->

View File

@ -0,0 +1,5 @@
Improve the format for the REST API key bootstrap file to support initialize key with a role.
The new form is:`api_key:api_secret:role`.
`role` is optional and its default value is `administrator`.

View File

@ -9,8 +9,11 @@ api_key.label:
bootstrap_file.desc:
"""The bootstrap file provides API keys for EMQX.
EMQX will load these keys on startup to authorize API requests.
It contains key-value pairs in the format:`api_key:api_secret`.
Each line specifies an API key and its associated secret."""
It contains colon-separated values in the format: `api_key:api_secret:role`.
Each line specifies an API key and its associated secret, and the role of this key.
The 'role' part should be the pre-defined access scope group name,
for example, `administrator` or `viewer`.
The 'role' is introduced in 5.4, to be backward compatible, if it is missing, the key is implicitly granted `administrator` role."""
bootstrap_file.label:
"""Initialize api_key file."""