feat: add authz file rule validator

apps/emqx/test/emqx_common_test_helpers.erl

@@ -237,6 +237,8 @@ render_and_load_app_config(App, Opts) ->
     end.
 
 do_render_app_config(App, Schema, ConfigFile, Opts) ->
+    %% copy acl_conf must run before read_schema_configs
+    copy_acl_conf(),
     Vars = mustache_vars(App, Opts),
     RenderedConfigFile = render_config_file(ConfigFile, Vars),
     read_schema_configs(Schema, RenderedConfigFile),
@@ -520,6 +522,16 @@ copy_certs(emqx_conf, Dest0) ->
 copy_certs(_, _) ->
     ok.
 
+copy_acl_conf() ->
+    Dest = filename:join([code:lib_dir(emqx), "etc/acl.conf"]),
+    case code:lib_dir(emqx_authz) of
+        {error, bad_name} ->
+            (not filelib:is_regular(Dest)) andalso file:write_file(Dest, <<"">>);
+        _ ->
+            {ok, _} = file:copy(deps_path(emqx_authz, "etc/acl.conf"), Dest)
+    end,
+    ok.
+
 load_config(SchemaModule, Config) ->
     ConfigBin =
         case is_map(Config) of

apps/emqx_authz/src/emqx_authz.erl

@@ -482,6 +482,8 @@ maybe_write_files(NewSource) ->
 
 write_acl_file(#{<<"rules">> := Rules} = Source0) ->
     AclPath = ?MODULE:acl_conf_file(),
+    %% Always check if the rules are valid before writing to the file
+    %% If the rules are invalid, the old file will be kept
     ok = check_acl_file_rules(AclPath, Rules),
     ok = write_file(AclPath, Rules),
     Source1 = maps:remove(<<"rules">>, Source0),
@@ -538,8 +540,7 @@ check_acl_file_rules(Path, Rules) ->
     TmpPath = Path ++ ".tmp",
     try
         ok = write_file(Path, Rules),
-        #{annotations := #{rules := _}} = emqx_authz_file:create(#{path => Path}),
+        emqx_authz_schema:validate_file_rules(Path)
-        ok
     catch
         throw:Reason -> throw(Reason)
     after

apps/emqx_authz/src/emqx_authz_schema.erl

@@ -42,7 +42,8 @@
 
 -export([
     headers_no_content_type/1,
-    headers/1
+    headers/1,
+    validate_file_rules/1
 ]).
 
 %%--------------------------------------------------------------------
@@ -78,7 +79,17 @@ fields("authorization") ->
     authz_fields();
 fields(file) ->
     authz_common_fields(file) ++
-        [{path, ?HOCON(string(), #{required => true, desc => ?DESC(path)})}];
+        [
+            {path,
+                ?HOCON(
+                    string(),
+                    #{
+                        required => true,
+                        validator => fun ?MODULE:validate_file_rules/1,
+                        desc => ?DESC(path)
+                    }
+                )}
+        ];
 fields(http_get) ->
     authz_common_fields(http) ++
         http_common_fields() ++
@@ -495,7 +506,7 @@ authz_fields() ->
                     %% doc_lift is force a root level reference instead of nesting sub-structs
                     extra => #{doc_lift => true},
                     %% it is recommended to configure authz sources from dashboard
-                    %% hance the importance level for config is low
+                    %% hence the importance level for config is low
                     importance => ?IMPORTANCE_LOW
                 }
             )}
@@ -507,3 +518,10 @@ default_authz() ->
         <<"enable">> => true,
         <<"path">> => <<"${EMQX_ETC_DIR}/acl.conf">>
     }.
+
+validate_file_rules(Path) ->
+    %% Don't need assert the create result here, all error is thrown
+    %% some test mock the create function
+    %% #{annotations := #{rules := _}}
+    _ = emqx_authz_file:create(#{path => Path}),
+    ok.

apps/emqx_conf/test/emqx_conf_schema_tests.erl

@@ -23,6 +23,7 @@
     """).
 
 array_nodes_test() ->
+    ensure_acl_conf(),
     ExpectNodes = ['emqx1@127.0.0.1', 'emqx2@127.0.0.1'],
     lists:foreach(
         fun(Nodes) ->
@@ -133,6 +134,7 @@ outdated_log_test() ->
     validate_log(?OUTDATED_LOG_CONF).
 
 validate_log(Conf) ->
+    ensure_acl_conf(),
     BaseConf = to_bin(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"]),
     Conf0 = <<BaseConf/binary, (list_to_binary(Conf))/binary>>,
     {ok, ConfMap0} = hocon:binary(Conf0, #{format => richmap}),
@@ -199,6 +201,7 @@ log_test() ->
 
 %% erlfmt-ignore
 log_rotation_count_limit_test() ->
+    ensure_acl_conf(),
     Format =
     """
     log.file {
@@ -271,6 +274,7 @@ log_rotation_count_limit_test() ->
 ).
 
 authn_validations_test() ->
+    ensure_acl_conf(),
     BaseConf = to_bin(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"]),
 
     OKHttps = to_bin(?BASE_AUTHN_ARRAY, [post, true, <<"https://127.0.0.1:8080">>]),
@@ -328,6 +332,7 @@ authn_validations_test() ->
 ).
 
 listeners_test() ->
+    ensure_acl_conf(),
     BaseConf = to_bin(?BASE_CONF, ["emqx1@127.0.0.1", "emqx1@127.0.0.1"]),
 
     Conf = <<BaseConf/binary, ?LISTENERS>>,
@@ -402,6 +407,7 @@ authentication_headers(Conf) ->
     Headers.
 
 doc_gen_test() ->
+    ensure_acl_conf(),
     %% the json file too large to encode.
     {
         timeout,
@@ -424,3 +430,11 @@ doc_gen_test() ->
 
 to_bin(Format, Args) ->
     iolist_to_binary(io_lib:format(Format, Args)).
+
+ensure_acl_conf() ->
+    File = emqx_schema:naive_env_interpolation(<<"${EMQX_ETC_DIR}/acl.conf">>),
+    ok = filelib:ensure_dir(filename:dirname(File)),
+    case filelib:is_regular(File) of
+        true -> ok;
+        false -> file:write_file(File, <<"">>)
+    end.