Merge pull request #7351 from JimMoen/fix-authz-api-pem-check

fix authz api flaky status code.

apps/emqx_authz/src/emqx_authz.erl

@@ -39,8 +39,6 @@
 
 -export([acl_conf_file/0]).
 
--export([ph_to_re/1]).
-
 -type(source() :: map()).
 
 -type(match_result() :: {matched, allow} | {matched, deny} | nomatch).
@@ -110,10 +108,10 @@ lookup(Type) ->
     {Source, _Front, _Rear} = take(Type),
     Source.
 
-move(Type, {before, Before}) ->
+move(Type, ?CMD_MOVE_BEFORE(Before)) ->
     emqx_authz_utils:update_config(
       ?CONF_KEY_PATH, {?CMD_MOVE, type(Type), ?CMD_MOVE_BEFORE(type(Before))});
-move(Type, {'after', After}) ->
+move(Type, ?CMD_MOVE_AFTER(After)) ->
     emqx_authz_utils:update_config(
       ?CONF_KEY_PATH, {?CMD_MOVE, type(Type), ?CMD_MOVE_AFTER(type(After))});
 move(Type, Position) ->
@@ -371,6 +369,3 @@ type(Unknown) -> error({unknown_authz_source_type, Unknown}).
 %% @doc where the acl.conf file is stored.
 acl_conf_file() ->
     filename:join([emqx:data_dir(), "authz", "acl.conf"]).
-
-ph_to_re(VarPH) ->
-    re:replace(VarPH, "[\\$\\{\\}]", "\\\\&", [global, {return, list}]).

apps/emqx_authz/src/emqx_authz_api_sources.erl

@@ -27,27 +27,6 @@
 -define(BAD_REQUEST, 'BAD_REQUEST').
 -define(NOT_FOUND, 'NOT_FOUND').
 
--define(EXAMPLE_REDIS,
-        #{type=> redis,
-          enable => true,
-          server => <<"127.0.0.1:3306">>,
-          redis_type => single,
-          pool_size => 1,
-          auto_reconnect => true,
-          cmd => <<"HGETALL mqtt_authz">>}).
--define(EXAMPLE_FILE,
-        #{type=> file,
-          enable => true,
-          rules => <<"{allow,{username,\"^dashboard?\"},subscribe,[\"$SYS/#\"]}.\n",
-                     "{allow,{ipaddr,\"127.0.0.1\"},all,[\"$SYS/#\",\"#\"]}.">>
-                   }).
-
--define(EXAMPLE_RETURNED,
-        #{sources => [ ?EXAMPLE_REDIS
-                     , ?EXAMPLE_FILE
-                     ]
-        }).
-
 -define(IS_TRUE(Val), ((Val =:= true) or (Val =:= <<"true">>))).
 
 -define(API_SCHEMA_MODULE, emqx_authz_api_schema).
@@ -87,16 +66,13 @@ schema("/authorization/sources") ->
      , get =>
            #{ description => <<"List all authorization sources">>
             , responses =>
-                  #{ 200 => mk( array(hoconsc:union(
+                  #{ 200 => mk( array(hoconsc:union(authz_sources_type_refs()))
-                      [ref(?API_SCHEMA_MODULE, Type) || Type <- authz_sources_types(detailed)]))
                               , #{desc => <<"Authorization source">>})
                    }
             }
      , post =>
            #{ description => <<"Add a new source">>
-            , 'requestBody' => mk( hoconsc:union(
+            , 'requestBody' => mk( hoconsc:union(authz_sources_type_refs())
-                                   [ref(?API_SCHEMA_MODULE, Type)
-                                        || Type <- authz_sources_types(detailed)])
                                  , #{desc => <<"Source config">>})
             , responses =>
                   #{ 204 => <<"Authorization source created successfully">>
@@ -104,18 +80,6 @@ schema("/authorization/sources") ->
                                                                <<"Bad Request">>)
                    }
             }
-     , put =>
-           #{ description => <<"Update all sources">>
-            , 'requestBody' => mk( array(hoconsc:union(
-                                  [ref(?API_SCHEMA_MODULE, Type)
-                                       || Type <- authz_sources_types(detailed)]))
-                                 , #{desc => <<"Sources">>})
-            , responses =>
-                  #{ 204 => <<"Authorization source updated successfully">>
-                   , 400 => emqx_dashboard_swagger:error_codes([?BAD_REQUEST],
-                              <<"Bad Request">>)
-                   }
-            }
      };
 schema("/authorization/sources/:type") ->
     #{ 'operationId' => source
@@ -123,9 +87,7 @@ schema("/authorization/sources/:type") ->
            #{ description => <<"Get a authorization source">>
             , parameters => parameters_field()
             , responses =>
-                  #{ 200 => mk( hoconsc:union(
+                  #{ 200 => mk( hoconsc:union(authz_sources_type_refs())
-                               [ref(?API_SCHEMA_MODULE, Type)
-                                   || Type <- authz_sources_types(detailed)])
                               , #{desc => <<"Authorization source">>})
                    , 404 => emqx_dashboard_swagger:error_codes([?NOT_FOUND], <<"Not Found">>)
                    }
@@ -133,8 +95,7 @@ schema("/authorization/sources/:type") ->
      , put =>
            #{ description => <<"Update source">>
             , parameters => parameters_field()
-            , 'requestBody' => mk( hoconsc:union([ref(?API_SCHEMA_MODULE, Type)
+            , 'requestBody' => mk( hoconsc:union(authz_sources_type_refs()))
-                                   || Type <- authz_sources_types(detailed)]))
             , responses =>
                   #{ 204 => <<"Authorization source updated successfully">>
                    , 400 => emqx_dashboard_swagger:error_codes([?BAD_REQUEST], <<"Bad Request">>)
@@ -212,17 +173,13 @@ sources(post, #{body := #{<<"type">> := <<"file">>, <<"rules">> := Rules}}) ->
     update_config(?CMD_PREPEND, [#{<<"type">> => <<"file">>,
                                    <<"enable">> => true, <<"path">> => Filename}]);
 sources(post, #{body := Body}) when is_map(Body) ->
-    update_config(?CMD_PREPEND, [maybe_write_certs(Body)]);
+    case maybe_write_certs(Body) of
-sources(put, #{body := Body}) when is_list(Body) ->
+        Config when is_map(Config) ->
-    NBody = [ begin
+            update_config(?CMD_PREPEND, [Config]);
-                case Source of
+        {error, Reason} ->
-                    #{<<"type">> := <<"file">>, <<"rules">> := Rules, <<"enable">> := Enable} ->
+            {400, #{code => <<"BAD_REQUEST">>,
-                        {ok, Filename} = write_file(acl_conf_file(), Rules),
+                    message => bin(Reason)}}
-                        #{<<"type">> => <<"file">>, <<"enable">> => Enable, <<"path">> => Filename};
+    end.
-                    _ -> maybe_write_certs(Source)
-                end
-              end || Source <- Body],
-    update_config(?CMD_REPLACE, NBody).
 
 source(Method, #{bindings := #{type := Type} = Bindings } = Req)
   when is_atom(Type) ->
@@ -260,8 +217,13 @@ source(put, #{bindings := #{type := <<"file">>}, body := #{<<"type">> := <<"file
                     message => bin(Reason)}}
     end;
 source(put, #{bindings := #{type := Type}, body := Body}) when is_map(Body) ->
-    update_config({?CMD_REPLACE, Type},
+    case maybe_write_certs(Body#{<<"type">> => Type}) of
-                   maybe_write_certs(Body#{<<"type">> => Type}));
+        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}, #{}).
 
@@ -470,8 +432,12 @@ read_certs(Source) -> Source.
 
 maybe_write_certs(#{<<"ssl">> := #{<<"enable">> := True} = SSL} = Source) when ?IS_TRUE(True) ->
     Type = maps:get(<<"type">>, Source),
-    {ok, Return} = emqx_tls_lib:ensure_ssl_files(filename:join(["authz", Type]), SSL),
+    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) ->
@@ -506,16 +472,16 @@ parse_position(<<"front">>) ->
     {ok, ?CMD_MOVE_FRONT};
 parse_position(<<"rear">>) ->
     {ok, ?CMD_MOVE_REAR};
+parse_position(<<"before:">>) ->
+    {error, <<"Invalid parameter. Cannot be placed before an empty target">>};
+parse_position(<<"after:">>) ->
+    {error, <<"Invalid parameter. Cannot be placed after an empty target">>};
 parse_position(<<"before:", Before/binary>>) ->
     {ok, ?CMD_MOVE_BEFORE(Before)};
 parse_position(<<"after:", After/binary>>) ->
     {ok, ?CMD_MOVE_AFTER(After)};
-parse_position(<<"before:">>) ->
-    {error, {invalid_parameter, position}};
-parse_position(<<"after:">>) ->
-    {error, {invalid_parameter, position}};
 parse_position(_) ->
-    {error, {invalid_parameter, position}}.
+    {error, <<"Invalid parameter. Unknow position">>}.
 
 position_example() ->
     #{ front =>
@@ -532,8 +498,9 @@ position_example() ->
            , value => #{<<"position">> => <<"after:file">>}}
      }.
 
-authz_sources_types(Type) ->
+authz_sources_type_refs() ->
-    emqx_authz_api_schema:authz_sources_types(Type).
+    [ref(?API_SCHEMA_MODULE, Type)
+     || Type <- emqx_authz_api_schema:authz_sources_types(detailed)].
 
 bin(Term) -> erlang:iolist_to_binary(io_lib:format("~p", [Term])).
 

apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl

@@ -98,6 +98,7 @@ groups() ->
     [].
 
 init_per_suite(Config) ->
+    ok = stop_apps([emqx_resource, emqx_connector]),
     meck:new(emqx_resource, [non_strict, passthrough, no_history, no_link]),
     meck:expect(emqx_resource, create_local, fun(_, _, _, _) -> {ok, meck_data} end),
     meck:expect(emqx_resource, create_dry_run_local,
@@ -111,6 +112,7 @@ init_per_suite(Config) ->
     ok = emqx_common_test_helpers:start_apps(
            [emqx_conf, emqx_authz, emqx_dashboard],
            fun set_special_configs/1),
+    ok = start_apps([emqx_resource, emqx_connector]),
     Config.
 
 end_per_suite(_Config) ->
@@ -172,8 +174,8 @@ t_api(_) ->
     {ok, 200, Result1} = request(get, uri(["authorization", "sources"]), []),
     ?assertEqual([], get_sources(Result1)),
 
-    {ok, 204, _} = request(put, uri(["authorization", "sources"]),
+    [ begin {ok, 204, _} = request(post, uri(["authorization", "sources"]), Source) end
-                           [?SOURCE2, ?SOURCE3, ?SOURCE4, ?SOURCE5, ?SOURCE6]),
+      || Source <- lists:reverse([?SOURCE2, ?SOURCE3, ?SOURCE4, ?SOURCE5, ?SOURCE6])],
     {ok, 204, _} = request(post, uri(["authorization", "sources"]), ?SOURCE1),
 
     Snd = fun ({_, Val}) -> Val end,
@@ -392,5 +394,8 @@ auth_header_() ->
 
 data_dir() -> emqx:data_dir().
 
+start_apps(Apps) ->
+    lists:foreach(fun application:ensure_all_started/1, Apps).
+
 stop_apps(Apps) ->
     lists:foreach(fun application:stop/1, Apps).

apps/emqx_management/src/emqx_mgmt_api_plugins.erl

@@ -371,9 +371,13 @@ return(_, {error, Reason}) ->
 parse_position(#{<<"position">> := <<"front">>}, _) -> front;
 parse_position(#{<<"position">> := <<"rear">>}, _) -> rear;
 parse_position(#{<<"position">> := <<"before:", Name/binary>>}, Name) ->
-    {error, <<"Can't before:self">>};
+    {error, <<"Invalid parameter. Cannot be placed before itself">>};
 parse_position(#{<<"position">> := <<"after:", Name/binary>>}, Name) ->
-    {error, <<"Can't after:self">>};
+    {error, <<"Invalid parameter. Cannot be placed after itself">>};
+parse_position(#{<<"position">> := <<"before:">>}, _Name) ->
+    {error, <<"Invalid parameter. Cannot be placed before an empty target">>};
+parse_position(#{<<"position">> := <<"after:">>}, _Name) ->
+    {error, <<"Invalid parameter. Cannot be placed after an empty target">>};
 parse_position(#{<<"position">> := <<"before:", Before/binary>>}, _Name) ->
     {before, binary_to_list(Before)};
 parse_position(#{<<"position">> := <<"after:", After/binary>>}, _Name) ->