feat(authz api): support move rule position

Signed-off-by: zhanghongtong <rory-z@outlook.com>

apps/emqx_authz/src/emqx_authz.erl

@@ -30,6 +30,7 @@
         , init_rule/1
         , lookup/0
         , lookup/1
+        , move/2
         , update/2
         , authorize/5
         , match/4
@@ -53,32 +54,99 @@ lookup() ->
     {_M, _F, [A]}= find_action_in_hooks(),
     A.
 lookup(Id) ->
-    case find_rule_by_id(Id, lookup()) of
+    try find_rule_by_id(Id, lookup()) of
-        {error, Reason} -> {error, Reason};
         {_, Rule} -> Rule
+    catch
+        error:Reason -> {error, Reason}
     end.
 
+move(Id, Position) ->
+    emqx_config:update(emqx_authz_schema, ?CONF_KEY_PATH, {move, Id, Position}).
+
 update(Cmd, Rules) ->
     emqx_config:update(emqx_authz_schema, ?CONF_KEY_PATH, {Cmd, Rules}).
 
-%% For now we only support re-creating the entire rule list
+pre_config_update({move, Id, <<"top">>}, Conf) when is_list(Conf) ->
-pre_config_update({head, Rules}, OldConf) when is_list(Rules), is_list(OldConf) ->
+    {Index, _} = find_rule_by_id(Id),
-    Rules ++ OldConf;
+    {List1, List2} = lists:split(Index, Conf),
-pre_config_update({tail, Rules}, OldConf) when is_list(Rules), is_list(OldConf) ->
+    [lists:nth(Index, Conf)] ++ lists:droplast(List1) ++ List2;
-    OldConf ++ Rules;
+
-pre_config_update({{replace_once, Id}, Rule}, OldConf) when is_map(Rule), is_list(OldConf) ->
+pre_config_update({move, Id, <<"bottom">>}, Conf) when is_list(Conf) ->
-    {Index, _} = case find_rule_by_id(Id, lookup()) of
+    {Index, _} = find_rule_by_id(Id),
-                            {error, Reason} -> error(Reason);
+    {List1, List2} = lists:split(Index, Conf),
-                            R -> R
+    lists:droplast(List1) ++ List2 ++ [lists:nth(Index, Conf)];
-                 end,
+
-    {OldConf1, OldConf2} = lists:split(Index, OldConf),
+pre_config_update({move, Id, #{<<"before">> := BeforeId}}, Conf) when is_list(Conf) ->
-    lists:droplast(OldConf1) ++ [Rule] ++ OldConf2;
+    {Index1, _} = find_rule_by_id(Id),
-pre_config_update({_, Rules}, _OldConf) when is_list(Rules)->
+    Conf1 = lists:nth(Index1, Conf),
+    {Index2, _} = find_rule_by_id(BeforeId),
+    Conf2 = lists:nth(Index2, Conf),
+
+    {List1, List2} = lists:split(Index2, Conf),
+    lists:delete(Conf1, lists:droplast(List1))
+    ++ [Conf1] ++ [Conf2]
+    ++ lists:delete(Conf1, List2);
+
+pre_config_update({move, Id, #{<<"after">> := AfterId}}, Conf) when is_list(Conf) ->
+    {Index1, _} = find_rule_by_id(Id),
+    Conf1 = lists:nth(Index1, Conf),
+    {Index2, _} = find_rule_by_id(AfterId),
+
+    {List1, List2} = lists:split(Index2, Conf),
+    lists:delete(Conf1, List1)
+    ++ [Conf1]
+    ++ lists:delete(Conf1, List2);
+
+pre_config_update({head, Rules}, Conf) when is_list(Rules), is_list(Conf) ->
+    Rules ++ Conf;
+pre_config_update({tail, Rules}, Conf) when is_list(Rules), is_list(Conf) ->
+    Conf ++ Rules;
+pre_config_update({{replace_once, Id}, Rule}, Conf) when is_map(Rule), is_list(Conf) ->
+    {Index, _} = find_rule_by_id(Id),
+    {List1, List2} = lists:split(Index, Conf),
+    lists:droplast(List1) ++ [Rule] ++ List2;
+pre_config_update({_, Rules}, _Conf) when is_list(Rules)->
     %% overwrite the entire config!
     Rules.
 
-post_config_update(_, undefined, _OldConf) ->
+post_config_update(_, undefined, _Conf) ->
     ok;
+post_config_update({move, Id, <<"top">>}, _NewRules, _OldRules) ->
+    InitedRules = lookup(),
+    {Index, Rule} = find_rule_by_id(Id, InitedRules),
+    {Rules1, Rules2 } = lists:split(Index, InitedRules),
+    Rules3 = [Rule] ++ lists:droplast(Rules1) ++ Rules2,
+    ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [Rules3]}, -1),
+    ok = emqx_authz_cache:drain_cache();
+post_config_update({move, Id, <<"bottom">>}, _NewRules, _OldRules) ->
+    InitedRules = lookup(),
+    {Index, Rule} = find_rule_by_id(Id, InitedRules),
+    {Rules1, Rules2 } = lists:split(Index, InitedRules),
+    Rules3 = lists:droplast(Rules1) ++ Rules2 ++ [Rule],
+    ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [Rules3]}, -1),
+    ok = emqx_authz_cache:drain_cache();
+post_config_update({move, Id, #{<<"before">> := BeforeId}}, _NewRules, _OldRules) ->
+    InitedRules = lookup(),
+    {_, Rule0} = find_rule_by_id(Id, InitedRules),
+    {Index, Rule1} = find_rule_by_id(BeforeId, InitedRules),
+    {Rules1, Rules2} = lists:split(Index, InitedRules),
+    Rules3 = lists:delete(Rule0, lists:droplast(Rules1))
+             ++ [Rule0] ++ [Rule1]
+             ++ lists:delete(Rule0, Rules2),
+    ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [Rules3]}, -1),
+    ok = emqx_authz_cache:drain_cache();
+
+post_config_update({move, Id, #{<<"after">> := AfterId}}, _NewRules, _OldRules) ->
+    InitedRules = lookup(),
+    {_, Rule} = find_rule_by_id(Id, InitedRules),
+    {Index, _} = find_rule_by_id(AfterId, InitedRules),
+    {Rules1, Rules2} = lists:split(Index, InitedRules),
+    Rules3 = lists:delete(Rule, Rules1)
+             ++ [Rule]
+             ++ lists:delete(Rule, Rules2),
+    ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [Rules3]}, -1),
+    ok = emqx_authz_cache:drain_cache();
+
 post_config_update({head, Rules}, _NewRules, _OldConf) ->
     InitedRules = [init_rule(R) || R <- check_rules(Rules)],
     ok = emqx_hooks:put('client.authorize', {?MODULE, authorize, [InitedRules ++ lookup()]}, -1),
@@ -91,10 +159,7 @@ post_config_update({tail, Rules}, _NewRules, _OldConf) ->
 
 post_config_update({{replace_once, Id}, Rule}, _NewRules, _OldConf) when is_map(Rule) ->
     OldInitedRules = lookup(),
-    {Index, OldRule} = case find_rule_by_id(Id, OldInitedRules) of
+    {Index, OldRule} = find_rule_by_id(Id, OldInitedRules),
-                           {error, Reason} -> error(Reason);
-                            R -> R
-                       end,
     case maps:get(type, OldRule, undefined) of
        undefined -> ok;
        _ ->
@@ -127,8 +192,9 @@ check_rules(RawRules) ->
     #{authorization := #{rules := Rules}} = hocon_schema:richmap_to_map(CheckConf),
     Rules.
 
+find_rule_by_id(Id) -> find_rule_by_id(Id, lookup()).
 find_rule_by_id(Id, Rules) -> find_rule_by_id(Id, Rules, 1).
-find_rule_by_id(_RuleId, [], _N) -> {error, not_found_rule};
+find_rule_by_id(_RuleId, [], _N) -> error(not_found_rule);
 find_rule_by_id(RuleId, [ Rule = #{annotations := #{id := Id}} | Tail], N) ->
     case RuleId =:= Id of
         true -> {N, Rule};

apps/emqx_authz/src/emqx_authz_api.erl

@@ -40,18 +40,20 @@
                          topics => [<<"#">>]}).
 
 -export([ api_spec/0
-        , authorization/2
+        , rules/2
-        , authorization_once/2
+        , rule/2
+        , move_rule/2
         ]).
 
 api_spec() ->
-    {[ api(),
+    {[ rules_api()
-       once_api()
+     , rule_api()
+     , move_rule_api()
      ], definitions()}.
 
 definitions() -> emqx_authz_api_schema:definitions().
 
-api() ->
+rules_api() ->
     Metadata = #{
         get => #{
             description => "List authorization rules",
@@ -135,6 +137,7 @@ api() ->
             }
         },
         put => #{
+
             description => "Update all rules",
             requestBody => #{
                 content => #{
@@ -174,9 +177,9 @@ api() ->
             }
         }
     },
-    {"/authorization", Metadata, authorization}.
+    {"/authorization", Metadata, rules}.
 
-once_api() ->
+rule_api() ->
     Metadata = #{
         get => #{
             description => "List authorization rules",
@@ -321,9 +324,101 @@ once_api() ->
             }
         }
     },
-    {"/authorization/:id", Metadata, authorization_once}.
+    {"/authorization/:id", Metadata, rule}.
 
-authorization(get, Request) ->
+move_rule_api() ->
+    Metadata = #{
+        post => #{
+            description => "Change the order of rules",
+            parameters => [
+                #{
+                    name => id,
+                    in => path,
+                    schema => #{
+                        type => string
+                    },
+                    required => true
+                }
+            ],
+            requestBody => #{
+                content => #{
+                    'application/json' => #{
+                        schema => #{
+                            type => object,
+                            required => [position],
+                            properties => #{
+                                position => #{
+                                    oneOf => [
+                                        #{type => string,
+                                          enum => [<<"top">>, <<"bottom">>]
+                                        },
+                                        #{type => object,
+                                          required => ['after'],
+                                          properties => #{
+                                            'after' => #{
+                                              type => string
+                                             }
+                                           }
+                                        },
+                                        #{type => object,
+                                          required => ['before'],
+                                          properties => #{
+                                            'before' => #{
+                                              type => string
+                                             }
+                                           }
+                                        }
+                                    ]
+                                }
+                            }
+                        }
+                    }
+                }
+            },
+            responses => #{
+                <<"204">> => #{
+                    description => <<"No Content">>
+                },
+                <<"404">> => #{
+                    description => <<"Bad Request">>,
+                    content => #{
+                        'application/json' => #{
+                            schema => minirest:ref(<<"error">>),
+                            examples => #{
+                                example1 => #{
+                                    summary => <<"Not Found">>,
+                                    value => #{
+                                        code => <<"NOT_FOUND">>,
+                                        message => <<"rule xxx not found">>
+                                    }
+                                }
+                            }
+                        }
+                    }
+                },
+                <<"400">> => #{
+                    description => <<"Bad Request">>,
+                    content => #{
+                        'application/json' => #{
+                            schema => minirest:ref(<<"error">>),
+                            examples => #{
+                                example1 => #{
+                                    summary => <<"Bad Request">>,
+                                    value => #{
+                                        code => <<"BAD_REQUEST">>,
+                                        message => <<"Bad Request">>
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    },
+    {"/authorization/:id/move", Metadata, move_rule}.
+
+rules(get, Request) ->
     Rules = lists:foldl(fun (#{type := _Type, enable := true, annotations := #{id := Id} = Annotations} = Rule, AccIn) ->
                                 NRule = case emqx_resource:health_check(Id) of
                                     ok ->
@@ -350,7 +445,7 @@ authorization(get, Request) ->
             end;
         false -> {200, #{rules => Rules}}
     end;
-authorization(post, Request) ->
+rules(post, Request) ->
     {ok, Body, _} = cowboy_req:read_body(Request),
     RawConfig = jsx:decode(Body, [return_maps]),
     case emqx_authz:update(head, [RawConfig]) of
@@ -359,7 +454,7 @@ authorization(post, Request) ->
             {400, #{code => <<"BAD_REQUEST">>,
                     messgae => atom_to_binary(Reason)}}
     end;
-authorization(put, Request) ->
+rules(put, Request) ->
     {ok, Body, _} = cowboy_req:read_body(Request),
     RawConfig = jsx:decode(Body, [return_maps]),
     case emqx_authz:update(replace, RawConfig) of
@@ -369,7 +464,7 @@ authorization(put, Request) ->
                     messgae => atom_to_binary(Reason)}}
     end.
 
-authorization_once(get, Request) ->
+rule(get, Request) ->
     Id = cowboy_req:binding(id, Request),
     case emqx_authz:lookup(Id) of
         {error, Reason} -> {404, #{messgae => atom_to_binary(Reason)}};
@@ -386,7 +481,7 @@ authorization_once(get, Request) ->
 
             end
     end;
-authorization_once(put, Request) ->
+rule(put, Request) ->
     RuleId = cowboy_req:binding(id, Request),
     {ok, Body, _} = cowboy_req:read_body(Request),
     RawConfig = jsx:decode(Body, [return_maps]),
@@ -399,7 +494,7 @@ authorization_once(put, Request) ->
             {400, #{code => <<"BAD_REQUEST">>,
                     messgae => atom_to_binary(Reason)}}
     end;
-authorization_once(delete, Request) ->
+rule(delete, Request) ->
     RuleId = cowboy_req:binding(id, Request),
     case emqx_authz:update({replace_once, RuleId}, #{}) of
         ok -> {204};
@@ -407,3 +502,16 @@ authorization_once(delete, Request) ->
             {400, #{code => <<"BAD_REQUEST">>,
                     messgae => atom_to_binary(Reason)}}
     end.
+move_rule(post, Request) ->
+    RuleId = cowboy_req:binding(id, Request),
+    {ok, Body, _} = cowboy_req:read_body(Request),
+    #{<<"position">> := Position} = jsx:decode(Body, [return_maps]),
+    case emqx_authz:move(RuleId, Position) of
+        ok -> {204};
+        {error, not_found_rule} ->
+            {404, #{code => <<"NOT_FOUND">>,
+                    messgae => <<"rule ", RuleId/binary, " not found">>}};
+        {error, Reason} ->
+            {400, #{code => <<"BAD_REQUEST">>,
+                    messgae => atom_to_binary(Reason)}}
+    end.

apps/emqx_authz/test/emqx_authz_SUITE.erl

@@ -42,6 +42,10 @@ end_per_suite(_Config) ->
     emqx_ct_helpers:stop_apps([emqx_authz]),
     ok.
 
+init_per_testcase(_, Config) ->
+    ok = emqx_authz:update(replace, []),
+    Config.
+
 -define(RULE1, #{<<"principal">> => <<"all">>,
                  <<"topics">> => [<<"#">>],
                  <<"action">> => <<"all">>,
@@ -130,6 +134,43 @@ t_update_rule(_) ->
 
     ok = emqx_authz:update(replace, []).
 
+t_move_rule(_) ->
+    ok = emqx_authz:update(replace, [?RULE1, ?RULE2, ?RULE3, ?RULE4]),
+    [#{annotations := #{id := Id1}},
+     #{annotations := #{id := Id2}},
+     #{annotations := #{id := Id3}},
+     #{annotations := #{id := Id4}}
+    ] = emqx_authz:lookup(),
+
+    ok = emqx_authz:move(Id4, <<"top">>),
+    ?assertMatch([#{annotations := #{id := Id4}},
+                  #{annotations := #{id := Id1}},
+                  #{annotations := #{id := Id2}},
+                  #{annotations := #{id := Id3}}
+                 ], emqx_authz:lookup()),
+
+    ok = emqx_authz:move(Id1, <<"bottom">>),
+    ?assertMatch([#{annotations := #{id := Id4}},
+                  #{annotations := #{id := Id2}},
+                  #{annotations := #{id := Id3}},
+                  #{annotations := #{id := Id1}}
+                 ], emqx_authz:lookup()),
+
+    ok = emqx_authz:move(Id3, #{<<"before">> => Id4}),
+    ?assertMatch([#{annotations := #{id := Id3}},
+                  #{annotations := #{id := Id4}},
+                  #{annotations := #{id := Id2}},
+                  #{annotations := #{id := Id1}}
+                 ], emqx_authz:lookup()),
+
+    ok = emqx_authz:move(Id2, #{<<"after">> => Id1}),
+    ?assertMatch([#{annotations := #{id := Id3}},
+                  #{annotations := #{id := Id4}},
+                  #{annotations := #{id := Id1}},
+                  #{annotations := #{id := Id2}}
+                 ], emqx_authz:lookup()),
+    ok.
+
 t_authz(_) ->
     ClientInfo1 = #{clientid => <<"test">>,
                     username => <<"test">>,

apps/emqx_authz/test/emqx_authz_api_SUITE.erl

@@ -35,7 +35,36 @@
 -define(API_VERSION, "v5").
 -define(BASE_PATH, "api").
 
--define(CONF_DEFAULT, <<"authorization: {rules: []}">>).
+-define(RULE1, #{<<"principal">> => <<"all">>,
+                 <<"topics">> => [<<"#">>],
+                 <<"action">> => <<"all">>,
+                 <<"permission">> => <<"deny">>}
+       ).
+-define(RULE2, #{<<"principal">> =>
+                    #{<<"ipaddress">> => <<"127.0.0.1">>},
+                 <<"topics">> =>
+                        [#{<<"eq">> => <<"#">>},
+                         #{<<"eq">> => <<"+">>}
+                        ] ,
+                 <<"action">> => <<"all">>,
+                 <<"permission">> => <<"allow">>}
+       ).
+-define(RULE3,#{<<"principal">> =>
+                    #{<<"and">> => [#{<<"username">> => <<"^test?">>},
+                                    #{<<"clientid">> => <<"^test?">>}
+                                   ]},
+                <<"topics">> => [<<"test">>],
+                <<"action">> => <<"publish">>,
+                <<"permission">> => <<"allow">>}
+       ).
+-define(RULE4,#{<<"principal">> =>
+                    #{<<"or">> => [#{<<"username">> => <<"^test">>},
+                                   #{<<"clientid">> => <<"test?">>}
+                                  ]},
+                <<"topics">> => [<<"%u">>,<<"%c">>],
+                <<"action">> => <<"publish">>,
+                <<"permission">> => <<"deny">>}
+       ).
 
 all() ->
     emqx_ct:all(?MODULE).
@@ -71,7 +100,7 @@ set_special_configs(_App) ->
 %% Testcases
 %%------------------------------------------------------------------------------
 
-t_post(_) ->
+t_api(_) ->
     {ok, 200, Result1} = request(get, uri(["authorization"]), []),
     ?assertEqual([], get_rules(Result1)),
 
@@ -125,6 +154,48 @@ t_post(_) ->
     ?assertEqual([], get_rules(Result5)),
     ok.
 
+t_move_rule(_) ->
+    ok = emqx_authz:update(replace, [?RULE1, ?RULE2, ?RULE3, ?RULE4]),
+    [#{annotations := #{id := Id1}},
+     #{annotations := #{id := Id2}},
+     #{annotations := #{id := Id3}},
+     #{annotations := #{id := Id4}}
+    ] = emqx_authz:lookup(),
+
+    {ok, 204, _} = request(post, uri(["authorization", Id4, "move"]),
+                           #{<<"position">> => <<"top">>}),
+    ?assertMatch([#{annotations := #{id := Id4}},
+                  #{annotations := #{id := Id1}},
+                  #{annotations := #{id := Id2}},
+                  #{annotations := #{id := Id3}}
+                 ], emqx_authz:lookup()),
+
+    {ok, 204, _} = request(post, uri(["authorization", Id1, "move"]),
+                           #{<<"position">> => <<"bottom">>}),
+    ?assertMatch([#{annotations := #{id := Id4}},
+                  #{annotations := #{id := Id2}},
+                  #{annotations := #{id := Id3}},
+                  #{annotations := #{id := Id1}}
+                 ], emqx_authz:lookup()),
+
+    {ok, 204, _} = request(post, uri(["authorization", Id3, "move"]),
+                           #{<<"position">> => #{<<"before">> => Id4}}),
+    ?assertMatch([#{annotations := #{id := Id3}},
+                  #{annotations := #{id := Id4}},
+                  #{annotations := #{id := Id2}},
+                  #{annotations := #{id := Id1}}
+                 ], emqx_authz:lookup()),
+
+    {ok, 204, _} = request(post, uri(["authorization", Id2, "move"]),
+                           #{<<"position">> => #{<<"after">> => Id1}}),
+    ?assertMatch([#{annotations := #{id := Id3}},
+                  #{annotations := #{id := Id4}},
+                  #{annotations := #{id := Id1}},
+                  #{annotations := #{id := Id2}}
+                 ], emqx_authz:lookup()),
+
+    ok.
+
 %%--------------------------------------------------------------------
 %% HTTP Request
 %%--------------------------------------------------------------------