Merge pull request #7360 from JimMoen/fix-authz-cluster-update-config

fix authz cluster update config

apps/emqx_authz/src/emqx_authz.erl

@@ -101,7 +101,7 @@ deinit() ->
     emqx_authz_utils:cleanup_resources().
 
 lookup() ->
-    {_M, _F, [A]}= find_action_in_hooks(),
+    {_M, _F, [A]} = find_action_in_hooks(),
     A.
 
 lookup(Type) ->
@@ -125,98 +125,103 @@ update({?CMD_DELETE, Type}, Sources) ->
 update(Cmd, Sources) ->
     emqx_authz_utils:update_config(?CONF_KEY_PATH, {Cmd, Sources}).
 
-do_update({?CMD_MOVE, Type, ?CMD_MOVE_FRONT}, Conf) when is_list(Conf) ->
+pre_config_update(_, Cmd, Sources) ->
-    {Source, Front, Rear} = take(Type, Conf),
+    {ok, do_pre_config_update(Cmd, Sources)}.
-    [Source | Front] ++ Rear;
+
-do_update({?CMD_MOVE, Type, ?CMD_MOVE_REAR}, Conf) when is_list(Conf) ->
+do_pre_config_update({?CMD_MOVE, _, _} = Cmd, Sources) ->
-    {Source, Front, Rear} = take(Type, Conf),
+    do_move(Cmd, Sources);
-    Front ++ Rear ++ [Source];
+do_pre_config_update({?CMD_PREPEND, Source}, Sources) ->
-do_update({?CMD_MOVE, Type, ?CMD_MOVE_BEFORE(Before)}, Conf) when is_list(Conf) ->
+    NSource = maybe_write_files(Source),
-    {S1, Front1, Rear1} = take(Type, Conf),
+    NSources = [NSource] ++ Sources,
-    {S2, Front2, Rear2} = take(Before, Front1 ++ Rear1),
+    ok = check_dup_types(NSources),
-    Front2 ++ [S1, S2] ++ Rear2;
+    NSources;
-do_update({?CMD_MOVE, Type, ?CMD_MOVE_AFTER(After)}, Conf) when is_list(Conf) ->
+do_pre_config_update({?CMD_APPEND, Source}, Sources) ->
-    {S1, Front1, Rear1} = take(Type, Conf),
+    NSource = maybe_write_files(Source),
-    {S2, Front2, Rear2} = take(After, Front1 ++ Rear1),
+    NSources = Sources ++ [NSource],
-    Front2 ++ [S2, S1] ++ Rear2;
+    ok = check_dup_types(NSources),
-do_update({?CMD_PREPEND, Sources}, Conf) when is_list(Sources), is_list(Conf) ->
+    NSources;
-    NConf = Sources ++ Conf,
+do_pre_config_update({{?CMD_REPLACE, Type}, #{<<"enable">> := Enable} = Source}, Sources)
-    ok = check_dup_types(NConf),
+  when ?IS_ENABLED(Enable) ->
-    NConf;
+    NSource = maybe_write_files(Source),
-do_update({?CMD_APPEND, Sources}, Conf) when is_list(Sources), is_list(Conf) ->
+    {_Old, Front, Rear} = take(Type, Sources),
-    NConf = Conf ++ Sources,
+    case create_dry_run(Type, NSource)  of
-    ok = check_dup_types(NConf),
-    NConf;
-do_update({{?CMD_REPLACE, Type}, #{<<"enable">> := Enable} = Source}, Conf)
-  when is_map(Source), is_list(Conf), ?IS_ENABLED(Enable) ->
-    case create_dry_run(Type, Source)  of
         ok ->
-            {_Old, Front, Rear} = take(Type, Conf),
+            NSources = Front ++ [NSource | Rear],
-            NConf = Front ++ [Source | Rear],
+            ok = check_dup_types(NSources),
-            ok = check_dup_types(NConf),
+            NSources;
-            NConf;
+        {error, _} = Error ->
-        {error, _} = Error -> Error
+            throw(Error)
     end;
-do_update({{?CMD_REPLACE, Type}, Source}, Conf)
+do_pre_config_update({{?CMD_REPLACE, Type}, Source}, Sources) ->
-  when is_map(Source), is_list(Conf) ->
+    NSource = maybe_write_files(Source),
-    {_Old, Front, Rear} = take(Type, Conf),
+    {_Old, Front, Rear} = take(Type, Sources),
-    NConf = Front ++ [Source | Rear],
+    NSources = Front ++ [NSource | Rear],
-    ok = check_dup_types(NConf),
+    ok = check_dup_types(NSources),
-    NConf;
+    NSources;
-do_update({{?CMD_DELETE, Type}, _Source}, Conf) when is_list(Conf) ->
+do_pre_config_update({{?CMD_DELETE, Type}, _Source}, Sources) ->
-    {_Old, Front, Rear} = take(Type, Conf),
+    {_Old, Front, Rear} = take(Type, Sources),
-    NConf = Front ++ Rear,
+    NSources = Front ++ Rear,
-    NConf;
+    NSources;
-do_update({_, Sources}, _Conf) when is_list(Sources)->
+do_pre_config_update({?CMD_REPLACE, Sources}, _OldSources) ->
     %% overwrite the entire config!
-    Sources;
+    NSources = lists:map(fun maybe_write_files/1, Sources),
-do_update({Op, Sources}, Conf) ->
+    ok = check_dup_types(NSources),
-    error({bad_request, #{op => Op, sources => Sources, conf => Conf}}).
+    NSources;
+do_pre_config_update({Op, Source}, Sources) ->
+    throw({bad_request, #{op => Op, source => Source, sources => Sources}}).
 
-pre_config_update(_, Cmd, Conf) ->
+post_config_update(_, _, undefined, _OldSource, _AppEnvs) ->
-    {ok, do_update(Cmd, Conf)}.
-
-
-post_config_update(_, _, undefined, _Conf, _AppEnvs) ->
     ok;
 post_config_update(_, Cmd, NewSources, _OldSource, _AppEnvs) ->
-    ok = do_post_update(Cmd, NewSources),
+    Actions = do_post_config_update(Cmd, NewSources),
+    ok = update_authz_chain(Actions),
     ok = emqx_authz_cache:drain_cache().
 
-do_post_update({?CMD_MOVE, _Type, _Where} = Cmd, _NewSources) ->
+do_post_config_update({?CMD_MOVE, _Type, _Where} = Cmd, _Sources) ->
     InitedSources = lookup(),
-    MovedSources = do_update(Cmd, InitedSources),
+    do_move(Cmd, InitedSources);
-    ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [MovedSources]}, -1),
+do_post_config_update({?CMD_PREPEND, RawNewSource}, Sources) ->
-    ok = emqx_authz_cache:drain_cache();
+    InitedNewSource = init_source(get_source_by_type(type(RawNewSource), Sources)),
-do_post_update({?CMD_PREPEND, Sources}, _NewSources) ->
+    [InitedNewSource] ++ lookup();
-    InitedSources = init_sources(check_sources(Sources)),
+do_post_config_update({?CMD_APPEND, RawNewSource}, Sources) ->
-    ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [InitedSources ++ lookup()]}, -1),
+    InitedNewSource = init_source(get_source_by_type(type(RawNewSource), Sources)),
-    ok = emqx_authz_cache:drain_cache();
+    lookup() ++ [InitedNewSource];
-do_post_update({?CMD_APPEND, Sources}, _NewSources) ->
+do_post_config_update({{?CMD_REPLACE, Type}, RawNewSource}, Sources) ->
-    InitedSources = init_sources(check_sources(Sources)),
+    OldSources = lookup(),
-    emqx_hooks:put('client.authorize', {?MODULE, authorize, [lookup() ++ InitedSources]}, -1),
+    {OldSource, Front, Rear} = take(Type, OldSources),
-    ok = emqx_authz_cache:drain_cache();
+    NewSource = get_source_by_type(type(RawNewSource), Sources),
-do_post_update({{?CMD_REPLACE, Type}, Source}, _NewSources) when is_map(Source) ->
+    ok = ensure_resource_deleted(OldSource),
+    clear_certs(OldSource),
+    InitedSources = init_source(NewSource),
+    Front ++ [InitedSources] ++ Rear;
+do_post_config_update({{?CMD_DELETE, Type}, _RawNewSource}, _Sources) ->
     OldInitedSources = lookup(),
     {OldSource, Front, Rear} = take(Type, OldInitedSources),
     ok = ensure_resource_deleted(OldSource),
-    InitedSources = init_sources(check_sources([Source])),
+    clear_certs(OldSource),
-    ok = emqx_hooks:put( 'client.authorize'
+    Front ++ Rear;
-                       , {?MODULE, authorize, [Front ++ InitedSources ++ Rear]}, -1),
+do_post_config_update({?CMD_REPLACE, _RawNewSources}, Sources) ->
-    ok = emqx_authz_cache:drain_cache();
-do_post_update({{?CMD_DELETE, Type}, _Source}, _NewSources) ->
-    OldInitedSources = lookup(),
-    {OldSource, Front, Rear} = take(Type, OldInitedSources),
-    ok = ensure_resource_deleted(OldSource),
-    ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [Front ++ Rear]}, -1),
-    ok = emqx_authz_cache:drain_cache();
-do_post_update({?CMD_REPLACE, Sources}, _NewSources) ->
     %% overwrite the entire config!
     OldInitedSources = lookup(),
-    InitedSources = init_sources(check_sources(Sources)),
-    ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [InitedSources]}, -1),
     lists:foreach(fun ensure_resource_deleted/1, OldInitedSources),
-    ok = emqx_authz_cache:drain_cache().
+    lists:foreach(fun clear_certs/1, OldInitedSources),
+    init_sources(Sources).
+
+%% @doc do source move
+do_move({?CMD_MOVE, Type, ?CMD_MOVE_FRONT}, Sources) ->
+    {Source, Front, Rear} = take(Type, Sources),
+    [Source | Front] ++ Rear;
+do_move({?CMD_MOVE, Type, ?CMD_MOVE_REAR}, Sources) ->
+    {Source, Front, Rear} = take(Type, Sources),
+    Front ++ Rear ++ [Source];
+do_move({?CMD_MOVE, Type, ?CMD_MOVE_BEFORE(Before)}, Sources) ->
+    {S1, Front1, Rear1} = take(Type, Sources),
+    {S2, Front2, Rear2} = take(Before, Front1 ++ Rear1),
+    Front2 ++ [S1, S2] ++ Rear2;
+do_move({?CMD_MOVE, Type, ?CMD_MOVE_AFTER(After)}, Sources) ->
+    {S1, Front1, Rear1} = take(Type, Sources),
+    {S2, Front2, Rear2} = take(After, Front1 ++ Rear1),
+    Front2 ++ [S2, S1] ++ Rear2.
 
 ensure_resource_deleted(#{enable := false}) -> ok;
 ensure_resource_deleted(#{type := Type} = Source) ->
@@ -233,14 +238,14 @@ check_dup_types([Source | Sources], Checked) ->
     Type = case maps:get(<<"type">>, Source, maps:get(type, Source, undefined)) of
                undefined ->
                    %% this should never happen if the value is type checked by honcon schema
-                   error({bad_source_input, Source});
+                   throw({bad_source_input, Source});
                Type0 ->
                    type(Type0)
            end,
     case lists:member(Type, Checked) of
         true ->
             %% we have made it clear not to support more than one authz instance for each type
-            error({duplicated_authz_source_type, Type});
+            throw({duplicated_authz_source_type, Type});
         false ->
             check_dup_types(Sources, [Type | Checked])
     end.
@@ -332,7 +337,7 @@ take(Type, Sources) ->
     {Front, Rear} =  lists:splitwith(fun(T) -> type(T) =/= type(Type) end, Sources),
     case Rear =:= [] of
         true ->
-            error({not_found_source, Type});
+            throw({not_found_source, Type});
         _ ->
             {hd(Rear), Front, tl(Rear)}
     end.
@@ -364,8 +369,62 @@ type(<<"postgresql">>) -> postgresql;
 type('built_in_database') -> 'built_in_database';
 type(<<"built_in_database">>) -> 'built_in_database';
 %% should never happen if the input is type-checked by hocon schema
-type(Unknown) -> error({unknown_authz_source_type, Unknown}).
+type(Unknown) -> throw({unknown_authz_source_type, Unknown}).
+
+maybe_write_files(#{<<"type">> := <<"file">>} = Source) ->
+    write_acl_file(Source);
+maybe_write_files(NewSource) ->
+    maybe_write_certs(NewSource).
+
+write_acl_file(#{<<"rules">> := Rules} = Source) ->
+    NRules = check_acl_file_rules(Rules),
+    Path = acl_conf_file(),
+    {ok, _Filename} = write_file(Path, NRules),
+    maps:without([<<"rules">>], Source#{<<"path">> => Path}).
 
 %% @doc where the acl.conf file is stored.
 acl_conf_file() ->
     filename:join([emqx:data_dir(), "authz", "acl.conf"]).
+
+maybe_write_certs(#{<<"type">> := Type} = Source) ->
+    case emqx_tls_lib:ensure_ssl_files(
+           ssl_file_path(Type), maps:get(<<"ssl">>, Source, undefined)) of
+        {ok, SSL} ->
+            new_ssl_source(Source, SSL);
+        {error, Reason} ->
+            ?SLOG(error, Reason#{msg => "bad_ssl_config"}),
+            throw({bad_ssl_config, Reason})
+    end.
+
+clear_certs(OldSource) ->
+    OldSSL = maps:get(ssl, OldSource, undefined),
+    ok = emqx_tls_lib:delete_ssl_files(ssl_file_path(type(OldSource)), undefined, OldSSL).
+
+write_file(Filename, Bytes) ->
+    ok = filelib:ensure_dir(Filename),
+    case file:write_file(Filename, Bytes) of
+        ok -> {ok, iolist_to_binary(Filename)};
+        {error, Reason} ->
+            ?SLOG(error, #{filename => Filename, msg => "write_file_error", reason => Reason}),
+            throw(Reason)
+    end.
+
+ssl_file_path(Type) ->
+    filename:join(["authz", Type]).
+
+new_ssl_source(Source, undefined) ->
+    Source;
+new_ssl_source(Source, SSL) ->
+    Source#{<<"ssl">> => SSL}.
+
+get_source_by_type(Type, Sources) ->
+    {Source, _Front, _Rear} = take(Type, Sources),
+    Source.
+
+%% @doc put hook with (maybe) initialized new source and old sources
+update_authz_chain(Actions) ->
+    emqx_hooks:put('client.authorize', {?MODULE, authorize, [Actions]}, -1).
+
+check_acl_file_rules(RawRules) ->
+    %% TODO: make sure the bin rules checked
+    RawRules.

apps/emqx_authz/src/emqx_authz_api_schema.erl

@@ -65,13 +65,12 @@ fields(redis_cluster) ->
     ++ emqx_connector_redis:fields(cluster);
 fields(file) ->
     authz_common_fields(file)
-    ++ [ {rules, #{ type => binary()
+    ++ [ { rules, #{ type => binary()
+                   , required => true
                    , example =>
                          <<"{allow,{username,\"^dashboard?\"},","subscribe,[\"$SYS/#\"]}.\n",
                            "{allow,{ipaddr,\"127.0.0.1\"},all,[\"$SYS/#\",\"#\"]}.">>}}
-         %% The path will be deprecated, `acl.conf` will be fixed in subdir of `data`
+    ];
-       , {path, #{ type => binary()
-                 , example => <<"acl.conf">>}}];
 fields(position) ->
     [ { position
       , mk( string()

apps/emqx_authz/src/emqx_authz_api_sources.erl

@@ -27,8 +27,6 @@
 -define(BAD_REQUEST, 'BAD_REQUEST').
 -define(NOT_FOUND, 'NOT_FOUND').
 
--define(IS_TRUE(Val), ((Val =:= true) or (Val =:= <<"true">>))).
-
 -define(API_SCHEMA_MODULE, emqx_authz_api_schema).
 
 -export([ get_raw_sources/0
@@ -95,7 +93,7 @@ schema("/authorization/sources/:type") ->
      , put =>
            #{ description => <<"Update source">>
             , parameters => parameters_field()
-            , 'requestBody' => mk( hoconsc:union(authz_sources_type_refs()))
+            , 'requestBody' => mk(hoconsc:union(authz_sources_type_refs()))
             , responses =>
                   #{ 204 => <<"Authorization source updated successfully">>
                    , 400 => emqx_dashboard_swagger:error_codes([?BAD_REQUEST], <<"Bad Request">>)
@@ -168,18 +166,10 @@ sources(get, _) ->
                                   lists:append(AccIn, [read_certs(Source)])
                           end, [], get_raw_sources()),
     {200, #{sources => Sources}};
-sources(post, #{body := #{<<"type">> := <<"file">>, <<"rules">> := Rules}}) ->
+sources(post, #{body := #{<<"type">> := <<"file">>} = Body}) ->
-    {ok, Filename} = write_file(acl_conf_file(), Rules),
+    create_authz_file(Body);
-    update_config(?CMD_PREPEND, [#{<<"type">> => <<"file">>,
+sources(post, #{body := Body}) ->
-                                   <<"enable">> => true, <<"path">> => Filename}]);
+    update_config(?CMD_PREPEND, Body).
-sources(post, #{body := Body}) when is_map(Body) ->
-    case maybe_write_certs(Body) of
-        Config when is_map(Config) ->
-            update_config(?CMD_PREPEND, [Config]);
-        {error, Reason} ->
-            {400, #{code => <<"BAD_REQUEST">>,
-                    message => bin(Reason)}}
-    end.
 
 source(Method, #{bindings := #{type := Type} = Bindings } = Req)
   when is_atom(Type) ->
@@ -201,13 +191,10 @@ source(get, #{bindings := #{type := Type}}) ->
             end;
         [Source] -> {200, read_certs(Source)}
     end;
-source(put, #{bindings := #{type := <<"file">>}, body := #{<<"type">> := <<"file">>,
+source(put, #{bindings := #{type := <<"file">>}, body := #{<<"type">> := <<"file">>} = Body}) ->
-                                                           <<"rules">> := Rules,
+    update_authz_file(Body);
-                                                           <<"enable">> := Enable}}) ->
+source(put, #{bindings := #{type := Type}, body := Body}) ->
-    {ok, Filename} = write_file(maps:get(path, emqx_authz:lookup(file), ""), Rules),
+    case emqx_authz:update({?CMD_REPLACE, Type}, Body) of
-    case emqx_authz:update({?CMD_REPLACE, <<"file">>}, #{<<"type">> => <<"file">>,
-                                                         <<"enable">> => Enable,
-                                                         <<"path">> => Filename}) of
         {ok, _} -> {204};
         {error, {emqx_conf_schema, _}} ->
             {400, #{code => <<"BAD_REQUEST">>,
@@ -216,14 +203,6 @@ source(put, #{bindings := #{type := <<"file">>}, body := #{<<"type">> := <<"file
             {400, #{code => <<"BAD_REQUEST">>,
                     message => bin(Reason)}}
     end;
-source(put, #{bindings := #{type := Type}, body := Body}) when is_map(Body) ->
-    case maybe_write_certs(Body#{<<"type">> => Type}) of
-        Config when is_map(Config) ->
-            update_config({?CMD_REPLACE, Type}, Config);
-        {error, Reason} ->
-            {400, #{code => <<"BAD_REQUEST">>,
-                    message => bin(Reason)}}
-    end;
 source(delete, #{bindings := #{type := Type}}) ->
     update_config({?CMD_DELETE, Type}, #{}).
 
@@ -423,45 +402,13 @@ update_config(Cmd, Sources) ->
 read_certs(#{<<"ssl">> := SSL} = Source) ->
     case emqx_tls_lib:file_content_as_options(SSL) of
         {error, Reason} ->
-            ?SLOG(error, Reason#{msg => failed_to_readd_ssl_file}),
+            ?SLOG(error, Reason#{msg => failed_to_read_ssl_file}),
-            throw(failed_to_readd_ssl_file);
+            throw(failed_to_read_ssl_file);
         {ok, NewSSL} ->
             Source#{<<"ssl">> => NewSSL}
     end;
 read_certs(Source) -> Source.
 
-maybe_write_certs(#{<<"ssl">> := #{<<"enable">> := True} = SSL} = Source) when ?IS_TRUE(True) ->
-    Type = maps:get(<<"type">>, Source),
-    case emqx_tls_lib:ensure_ssl_files(filename:join(["authz", Type]), SSL) of
-        {ok, Return} ->
-            maps:put(<<"ssl">>, Return, Source);
-        {error, _} ->
-            {error, ensuer_ssl_files_failed}
-    end;
-maybe_write_certs(Source) -> Source.
-
-write_file(Filename, Bytes0) ->
-    ok = filelib:ensure_dir(Filename),
-    case file:read_file(Filename) of
-        {ok, Bytes1} ->
-            case crypto:hash(md5, Bytes1) =:= crypto:hash(md5, Bytes0) of
-                true -> {ok, iolist_to_binary(Filename)};
-                false -> do_write_file(Filename, Bytes0)
-            end;
-        _ -> do_write_file(Filename, Bytes0)
-    end.
-
-do_write_file(Filename, Bytes) ->
-    case file:write_file(Filename, Bytes) of
-       ok -> {ok, iolist_to_binary(Filename)};
-       {error, Reason} ->
-           ?SLOG(error, #{filename => Filename, msg => "write_file_error", reason => Reason}),
-           error(Reason)
-    end.
-
-acl_conf_file() ->
-    emqx_authz:acl_conf_file().
-
 parameters_field() ->
     [ {type, mk( enum(?API_SCHEMA_MODULE:authz_sources_types(simple))
                , #{in => path, desc => <<"Authorization type">>})
@@ -528,3 +475,13 @@ status_metrics_example() ->
                          }
                       ]
      }.
+
+create_authz_file(Body) ->
+    do_update_authz_file(?CMD_PREPEND, Body).
+
+update_authz_file(Body) ->
+    do_update_authz_file({?CMD_REPLACE, <<"file">>}, Body).
+
+do_update_authz_file(Cmd, Body) ->
+    %% API update will placed in `authz` subdirectory inside EMQX's `data_dir`
+    update_config(Cmd,  Body).

apps/emqx_authz/src/emqx_authz_schema.erl

@@ -93,6 +93,7 @@ fields(file) ->
     , {enable, #{type => boolean(),
                  default => true}}
     , {path, #{type => string(),
+               required => true,
                desc => """
 Path to the file which contains the ACL rules.<br>
 If the file provisioned before starting EMQX node,

apps/emqx_authz/test/emqx_authz_SUITE.erl

@@ -34,6 +34,10 @@ init_per_suite(Config) ->
     meck:expect(emqx_resource, create_local, fun(_, _, _, _) -> {ok, meck_data} end),
     meck:expect(emqx_resource, remove_local, fun(_) -> ok end),
     meck:expect(emqx_resource, create_dry_run_local, fun(_, _) -> ok end),
+    meck:expect(emqx_authz, acl_conf_file,
+                fun() ->
+                        emqx_common_test_helpers:deps_path(emqx_authz, "etc/acl.conf")
+                end),
 
     ok = emqx_common_test_helpers:start_apps(
            [emqx_connector, emqx_conf, emqx_authz],
@@ -116,7 +120,9 @@ set_special_configs(_App) ->
                   }).
 -define(SOURCE6, #{<<"type">> => <<"file">>,
                    <<"enable">> => true,
-                   <<"path">> => emqx_common_test_helpers:deps_path(emqx_authz, "etc/acl.conf")
+                   <<"rules">> =>
+<<"{allow,{username,\"^dashboard?\"},subscribe,[\"$SYS/#\"]}."
+  "\n{allow,{ipaddr,\"127.0.0.1\"},all,[\"$SYS/#\",\"#\"]}.">>
                   }).
 
 
@@ -125,12 +131,13 @@ set_special_configs(_App) ->
 %%------------------------------------------------------------------------------
 
 t_update_source(_) ->
+    %% replace all
     {ok, _} = emqx_authz:update(?CMD_REPLACE, [?SOURCE3]),
-    {ok, _} = emqx_authz:update(?CMD_PREPEND, [?SOURCE2]),
+    {ok, _} = emqx_authz:update(?CMD_PREPEND, ?SOURCE2),
-    {ok, _} = emqx_authz:update(?CMD_PREPEND, [?SOURCE1]),
+    {ok, _} = emqx_authz:update(?CMD_PREPEND, ?SOURCE1),
-    {ok, _} = emqx_authz:update(?CMD_APPEND, [?SOURCE4]),
+    {ok, _} = emqx_authz:update(?CMD_APPEND, ?SOURCE4),
-    {ok, _} = emqx_authz:update(?CMD_APPEND, [?SOURCE5]),
+    {ok, _} = emqx_authz:update(?CMD_APPEND, ?SOURCE5),
-    {ok, _} = emqx_authz:update(?CMD_APPEND, [?SOURCE6]),
+    {ok, _} = emqx_authz:update(?CMD_APPEND, ?SOURCE6),
 
     ?assertMatch([ #{type := http,  enable := true}
                  , #{type := mongodb, enable := true}

apps/emqx_authz/test/emqx_authz_api_mnesia_SUITE.erl

@@ -230,16 +230,16 @@ t_api(_) ->
     {ok, 204, _} =
         request( put
                , uri(["authorization", "sources", "built_in_database"])
-               ,  #{<<"enable">> => true}),
+               , #{<<"enable">> => true, <<"type">> => <<"built_in_database">>}),
     %% test idempotence
     {ok, 204, _} =
         request( put
                , uri(["authorization", "sources", "built_in_database"])
-               ,  #{<<"enable">> => true}),
+               , #{<<"enable">> => true, <<"type">> => <<"built_in_database">>}),
     {ok, 204, _} =
         request( put
                , uri(["authorization", "sources", "built_in_database"])
-               ,  #{<<"enable">> => false}),
+               , #{<<"enable">> => false, <<"type">> => <<"built_in_database">>}),
     {ok, 204, _} =
         request( delete
                , uri(["authorization", "sources", "built_in_database", "purge-all"])

apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl

@@ -108,6 +108,10 @@ init_per_suite(Config) ->
                 end),
     meck:expect(emqx_resource, health_check, fun(St) -> {ok, St} end),
     meck:expect(emqx_resource, remove_local, fun(_) -> ok end ),
+    meck:expect(emqx_authz, acl_conf_file,
+                fun() ->
+                        emqx_common_test_helpers:deps_path(emqx_authz, "etc/acl.conf")
+                end),
 
     ok = emqx_common_test_helpers:start_apps(
            [emqx_conf, emqx_authz, emqx_dashboard],

apps/emqx_authz/test/emqx_authz_file_SUITE.erl

@@ -22,6 +22,13 @@
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("common_test/include/ct.hrl").
 
+-define(RAW_SOURCE, #{<<"type">> => <<"file">>,
+                      <<"enable">> => true,
+                      <<"rules">> =>
+<<"{allow,{username,\"^dashboard?\"},subscribe,[\"$SYS/#\"]}."
+  "\n{allow,{ipaddr,\"127.0.0.1\"},all,[\"$SYS/#\",\"#\"]}.">>
+                     }).
+
 all() ->
     emqx_common_test_helpers:all(?MODULE).
 
@@ -32,6 +39,11 @@ init_per_suite(Config) ->
     ok = emqx_common_test_helpers:start_apps(
            [emqx_conf, emqx_authz],
            fun set_special_configs/1),
+    %% meck after authz started
+    meck:expect(emqx_authz, acl_conf_file,
+                fun() ->
+                        emqx_common_test_helpers:deps_path(emqx_authz, "etc/acl.conf")
+                end),
     Config.
 
 end_per_suite(_Config) ->
@@ -61,8 +73,9 @@ t_ok(_Config) ->
                    listener => {tcp, default}
                   },
 
-    ok = setup_rules([{allow, {user, "username"}, publish, ["t"]}]),
+    ok = setup_config(?RAW_SOURCE#{<<"rules">> => <<"{allow, {user, \"username\"}, publish, [\"t\"]}.">>}),
-    ok = setup_config(#{}),
+
+    io:format("~p", [emqx_authz:acl_conf_file()]),
 
     ?assertEqual(
        allow,
@@ -73,61 +86,31 @@ t_ok(_Config) ->
        emqx_access_control:authorize(ClientInfo, subscribe, <<"t">>)).
 
 t_invalid_file(_Config) ->
-    ok = file:write_file(<<"acl.conf">>, <<"{{invalid term">>),
-
     ?assertMatch(
        {error, bad_acl_file_content},
-       emqx_authz:update(?CMD_REPLACE, [raw_file_authz_config()])).
+       emqx_authz:update(?CMD_REPLACE, [?RAW_SOURCE#{<<"rules">> => <<"{{invalid term">>}])).
-
-t_nonexistent_file(_Config) ->
-    ?assertEqual(
-       {error, failed_to_read_acl_file},
-       emqx_authz:update(?CMD_REPLACE,
-                         [maps:merge(raw_file_authz_config(),
-                                     #{<<"path">> => <<"nonexistent.conf">>})
-                         ])).
 
 t_update(_Config) ->
-    ok = setup_rules([{allow, {user, "username"}, publish, ["t"]}]),
+    ok = setup_config(?RAW_SOURCE#{<<"rules">> => <<"{allow, {user, \"username\"}, publish, [\"t\"]}.">>}),
-    ok = setup_config(#{}),
 
     ?assertMatch(
        {error, _},
        emqx_authz:update(
          {?CMD_REPLACE, file},
-         maps:merge(raw_file_authz_config(),
+         ?RAW_SOURCE#{<<"rules">> => <<"{{invalid term">>})),
-                    #{<<"path">> => <<"nonexistent.conf">>}))),
 
     ?assertMatch(
        {ok, _},
        emqx_authz:update(
-         {?CMD_REPLACE, file},
+         {?CMD_REPLACE, file}, ?RAW_SOURCE)).
-         raw_file_authz_config())).
 
 %%------------------------------------------------------------------------------
 %% Helpers
 %%------------------------------------------------------------------------------
 
-raw_file_authz_config() ->
-    #{
-        <<"enable">> => <<"true">>,
-
-        <<"type">> => <<"file">>,
-        <<"path">> => <<"acl.conf">>
-    }.
-
-setup_rules(Rules) ->
-    {ok, F} = file:open(<<"acl.conf">>, [write]),
-    lists:foreach(
-      fun(Rule) ->
-              io:format(F, "~p.~n", [Rule])
-      end,
-      Rules),
-    ok = file:close(F).
-
 setup_config(SpecialParams) ->
     emqx_authz_test_lib:setup_config(
-      raw_file_authz_config(),
+      ?RAW_SOURCE,
       SpecialParams).
 
 stop_apps(Apps) ->