diff --git a/apps/emqx_auth_mnesia/src/emqx_authz_api_mnesia.erl b/apps/emqx_auth_mnesia/src/emqx_authz_api_mnesia.erl index fd073eb84..7e95a6500 100644 --- a/apps/emqx_auth_mnesia/src/emqx_authz_api_mnesia.erl +++ b/apps/emqx_auth_mnesia/src/emqx_authz_api_mnesia.erl @@ -469,8 +469,8 @@ users(get, #{query_string := QueryString}) -> {200, Result} end; users(post, #{body := Body}) when is_list(Body) -> - case ensure_all_not_exists(<<"username">>, username, Body) of - [] -> + case ensure_rules_is_valid(<<"username">>, username, Body) of + ok -> lists:foreach( fun(#{<<"username">> := Username, <<"rules">> := Rules}) -> emqx_authz_mnesia:store_rules({username, Username}, Rules) @@ -478,10 +478,19 @@ users(post, #{body := Body}) when is_list(Body) -> Body ), {204}; - Exists -> + {error, {Username, too_many_rules}} -> + {400, #{ + code => <<"BAD_REQUEST">>, + message => + binfmt( + <<"The rules length of User '~ts' exceeds the maximum limit.">>, + [Username] + ) + }}; + {error, {already_exists, Exists}} -> {409, #{ code => <<"ALREADY_EXISTS">>, - message => binfmt("Users '~ts' already exist", [binjoin(Exists)]) + message => binfmt("User '~ts' already exist", [Exists]) }} end. @@ -507,8 +516,8 @@ clients(get, #{query_string := QueryString}) -> {200, Result} end; clients(post, #{body := Body}) when is_list(Body) -> - case ensure_all_not_exists(<<"clientid">>, clientid, Body) of - [] -> + case ensure_rules_is_valid(<<"clientid">>, clientid, Body) of + ok -> lists:foreach( fun(#{<<"clientid">> := ClientID, <<"rules">> := Rules}) -> emqx_authz_mnesia:store_rules({clientid, ClientID}, Rules) @@ -516,10 +525,19 @@ clients(post, #{body := Body}) when is_list(Body) -> Body ), {204}; - Exists -> + {error, {ClientId, too_many_rules}} -> + {400, #{ + code => <<"BAD_REQUEST">>, + message => + binfmt( + <<"The rules length of Client '~ts' exceeds the maximum limit.">>, + [ClientId] + ) + }}; + {error, {already_exists, Exists}} -> {409, #{ code => <<"ALREADY_EXISTS">>, - message => binfmt("Clients '~ts' already exist", [binjoin(Exists)]) + message => binfmt("Client '~ts' already exist", [Exists]) }} end. @@ -583,8 +601,17 @@ all(get, _) -> }} end; all(post, #{body := #{<<"rules">> := Rules}}) -> - emqx_authz_mnesia:store_rules(all, Rules), - {204}; + case ensure_rules_len(Rules) of + ok -> + emqx_authz_mnesia:store_rules(all, Rules), + {204}; + _ -> + {400, #{ + code => <<"BAD_REQUEST">>, + message => + <<"The length of rules exceeds the maximum limit.">> + }} + end; all(delete, _) -> emqx_authz_mnesia:store_rules(all, []), {204}. @@ -700,28 +727,45 @@ rules_example({ExampleName, ExampleType}) -> } }. -ensure_all_not_exists(Key, Type, Cfgs) -> - lists:foldl( - fun(#{Key := Id}, Acc) -> - case emqx_authz_mnesia:get_rules({Type, Id}) of - not_found -> - Acc; - _ -> - [Id | Acc] - end - end, - [], - Cfgs +ensure_rules_len(Rules) -> + emqx_authz_api_sources:with_source( + ?AUTHZ_TYPE_BIN, + fun(#{<<"max_rules">> := MaxLen}) -> + ensure_rules_len(Rules, MaxLen) + end ). -binjoin([Bin]) -> - Bin; -binjoin(Bins) -> - binjoin(Bins, <<>>). +ensure_rules_len(Rules, MaxLen) -> + case erlang:length(Rules) =< MaxLen of + true -> + ok; + _ -> + {error, too_many_rules} + end. -binjoin([H | T], Acc) -> - binjoin(T, <>); -binjoin([], Acc) -> - Acc. +ensure_rules_is_valid(Key, Type, Cfgs) -> + MaxLen = emqx_authz_api_sources:with_source( + ?AUTHZ_TYPE_BIN, + fun(#{<<"max_rules">> := MaxLen}) -> + MaxLen + end + ), + ensure_rules_is_valid(Key, Type, MaxLen, Cfgs). + +ensure_rules_is_valid(Key, Type, MaxLen, [Cfg | Cfgs]) -> + #{Key := Id, <<"rules">> := Rules} = Cfg, + case emqx_authz_mnesia:get_rules({Type, Id}) of + not_found -> + case ensure_rules_len(Rules, MaxLen) of + ok -> + ensure_rules_is_valid(Key, Type, MaxLen, Cfgs); + {error, Reason} -> + {error, {Id, Reason}} + end; + _ -> + {error, {already_exists, Id}} + end; +ensure_rules_is_valid(_Key, _Type, _MaxLen, []) -> + ok. binfmt(Fmt, Args) -> iolist_to_binary(io_lib:format(Fmt, Args)). diff --git a/apps/emqx_auth_mnesia/src/emqx_authz_mnesia_schema.erl b/apps/emqx_auth_mnesia/src/emqx_authz_mnesia_schema.erl index 1924752f3..1076be2f3 100644 --- a/apps/emqx_auth_mnesia/src/emqx_authz_mnesia_schema.erl +++ b/apps/emqx_auth_mnesia/src/emqx_authz_mnesia_schema.erl @@ -30,12 +30,24 @@ namespace/0 ]). +-define(MAX_RULES, 100). + namespace() -> "authz". type() -> ?AUTHZ_TYPE. fields(builtin_db) -> - emqx_authz_schema:authz_common_fields(?AUTHZ_TYPE). + emqx_authz_schema:authz_common_fields(?AUTHZ_TYPE) ++ + [ + {max_rules, + ?HOCON( + pos_integer(), + #{ + default => ?MAX_RULES, + desc => ?DESC(max_rules) + } + )} + ]. source_refs() -> [?R_REF(builtin_db)]. diff --git a/apps/emqx_auth_mnesia/test/emqx_authz_api_mnesia_SUITE.erl b/apps/emqx_auth_mnesia/test/emqx_authz_api_mnesia_SUITE.erl index cb9875ee2..4ba4c7075 100644 --- a/apps/emqx_auth_mnesia/test/emqx_authz_api_mnesia_SUITE.erl +++ b/apps/emqx_auth_mnesia/test/emqx_authz_api_mnesia_SUITE.erl @@ -36,7 +36,7 @@ init_per_suite(Config) -> {emqx_conf, "authorization.cache { enable = false }," "authorization.no_match = deny," - "authorization.sources = [{type = built_in_database}]"}, + "authorization.sources = [{type = built_in_database, max_rules = 5}]"}, emqx, emqx_auth, emqx_auth_mnesia, @@ -66,6 +66,14 @@ t_api(_) -> [?USERNAME_RULES_EXAMPLE] ), + %% check length limit + {ok, 400, _} = + request( + post, + uri(["authorization", "sources", "built_in_database", "rules", "users"]), + [dup_rules_example(?USERNAME_RULES_EXAMPLE)] + ), + {ok, 409, _} = request( post, @@ -171,6 +179,13 @@ t_api(_) -> [?CLIENTID_RULES_EXAMPLE] ), + {ok, 400, _} = + request( + post, + uri(["authorization", "sources", "built_in_database", "rules", "clients"]), + [dup_rules_example(?CLIENTID_RULES_EXAMPLE)] + ), + {ok, 409, _} = request( post, @@ -238,6 +253,14 @@ t_api(_) -> uri(["authorization", "sources", "built_in_database", "rules", "all"]), ?ALL_RULES_EXAMPLE ), + + {ok, 400, _} = + request( + post, + uri(["authorization", "sources", "built_in_database", "rules", "all"]), + dup_rules_example(?ALL_RULES_EXAMPLE) + ), + {ok, 200, Request7} = request( get, @@ -491,3 +514,10 @@ replace_parts(Parts, Replacements) -> end, Parts ). + +dup_rules_example(#{username := _, rules := Rules}) -> + #{username => user2, rules => Rules ++ Rules}; +dup_rules_example(#{clientid := _, rules := Rules}) -> + #{clientid => client2, rules => Rules ++ Rules}; +dup_rules_example(#{rules := Rules}) -> + #{rules => Rules ++ Rules}. diff --git a/changes/ce/fix-13196.en.md b/changes/ce/fix-13196.en.md new file mode 100644 index 000000000..26950ffc0 --- /dev/null +++ b/changes/ce/fix-13196.en.md @@ -0,0 +1 @@ +In the built-in database of authorization, added a limit for the number of rules per client/user, and the default values is 100. diff --git a/rel/i18n/emqx_authz_mnesia_schema.hocon b/rel/i18n/emqx_authz_mnesia_schema.hocon index 6face07b9..efc80dd34 100644 --- a/rel/i18n/emqx_authz_mnesia_schema.hocon +++ b/rel/i18n/emqx_authz_mnesia_schema.hocon @@ -6,4 +6,7 @@ builtin_db.desc: builtin_db.label: """Builtin Database""" +max_rules.desc: +"""Maximum number of rules per client/user. Note that performance may decrease as number of rules increases.""" + }