From 188d876b1c52c6b3abaac02703107357f6806025 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Tue, 15 Mar 2022 17:50:59 +0800 Subject: [PATCH 1/5] fix(api): fix unmatched position parsing --- apps/emqx_authz/src/emqx_authz_api_sources.erl | 10 +++++----- apps/emqx_management/src/emqx_mgmt_api_plugins.erl | 8 ++++++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index 0b4b98bd6..5a0dfac1c 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -506,16 +506,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 => diff --git a/apps/emqx_management/src/emqx_mgmt_api_plugins.erl b/apps/emqx_management/src/emqx_mgmt_api_plugins.erl index d806c309e..468d7c931 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_plugins.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_plugins.erl @@ -353,9 +353,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) -> From 0ee3e49db7a8e3e2cd732866f989afcdb9217fc6 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Tue, 15 Mar 2022 17:51:30 +0800 Subject: [PATCH 2/5] chore: rm unused macro and func, fix unchanged product name --- apps/emqx_authz/src/emqx_authz.erl | 5 ----- .../emqx_authz/src/emqx_authz_api_sources.erl | 21 ------------------- .../src/emqx_mgmt_api_plugins.erl | 4 ++-- 3 files changed, 2 insertions(+), 28 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index b270176ec..23f8fde58 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/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). @@ -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}]). diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index 5a0dfac1c..4c0ae5fe5 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/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). diff --git a/apps/emqx_management/src/emqx_mgmt_api_plugins.erl b/apps/emqx_management/src/emqx_mgmt_api_plugins.erl index 468d7c931..eb8999cb1 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_plugins.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_plugins.erl @@ -147,7 +147,7 @@ fields(plugin) -> required => true, example => "emqx_plugin_template-5.0-rc.1"}) }, - {author, hoconsc:mk(list(string()), #{example => [<<"EMQ X Team">>]})}, + {author, hoconsc:mk(list(string()), #{example => [<<"EMQX Team">>]})}, {builder, hoconsc:ref(?MODULE, builder)}, {built_on_otp_release, hoconsc:mk(string(), #{example => "24"})}, {compatibility, hoconsc:mk(map(), #{example => #{<<"emqx">> => <<"~>5.0">>}})}, @@ -195,7 +195,7 @@ fields(name) -> fields(builder) -> [ {contact, hoconsc:mk(string(), #{example => "emqx-support@emqx.io"})}, - {name, hoconsc:mk(string(), #{example => "EMQ X Team"})}, + {name, hoconsc:mk(string(), #{example => "EMQX Team"})}, {website, hoconsc:mk(string(), #{example => "www.emqx.com"})} ]; fields(position) -> From 0b7f1ab69c1ed5b9e4540cfc8581d1f373c74849 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Wed, 16 Mar 2022 10:25:16 +0800 Subject: [PATCH 3/5] refactor(authz_api): avoid copy paste --- apps/emqx_authz/src/emqx_authz.erl | 4 ++-- .../emqx_authz/src/emqx_authz_api_sources.erl | 23 +++++++------------ 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz.erl b/apps/emqx_authz/src/emqx_authz.erl index 23f8fde58..b8abbbed7 100644 --- a/apps/emqx_authz/src/emqx_authz.erl +++ b/apps/emqx_authz/src/emqx_authz.erl @@ -108,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) -> diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index 4c0ae5fe5..b1384b2f2 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -66,16 +66,13 @@ schema("/authorization/sources") -> , get => #{ description => <<"List all authorization sources">> , responses => - #{ 200 => mk( array(hoconsc:union( - [ref(?API_SCHEMA_MODULE, Type) || Type <- authz_sources_types(detailed)])) + #{ 200 => mk( array(hoconsc:union(authz_sources_type_refs())) , #{desc => <<"Authorization source">>}) } } , post => #{ description => <<"Add a new source">> - , 'requestBody' => mk( hoconsc:union( - [ref(?API_SCHEMA_MODULE, Type) - || Type <- authz_sources_types(detailed)]) + , 'requestBody' => mk( hoconsc:union(authz_sources_type_refs()) , #{desc => <<"Source config">>}) , responses => #{ 204 => <<"Authorization source created successfully">> @@ -85,9 +82,7 @@ schema("/authorization/sources") -> } , put => #{ description => <<"Update all sources">> - , 'requestBody' => mk( array(hoconsc:union( - [ref(?API_SCHEMA_MODULE, Type) - || Type <- authz_sources_types(detailed)])) + , 'requestBody' => mk( array(hoconsc:union(authz_sources_type_refs())) , #{desc => <<"Sources">>}) , responses => #{ 204 => <<"Authorization source updated successfully">> @@ -102,9 +97,7 @@ schema("/authorization/sources/:type") -> #{ description => <<"Get a authorization source">> , parameters => parameters_field() , responses => - #{ 200 => mk( hoconsc:union( - [ref(?API_SCHEMA_MODULE, Type) - || Type <- authz_sources_types(detailed)]) + #{ 200 => mk( hoconsc:union(authz_sources_type_refs()) , #{desc => <<"Authorization source">>}) , 404 => emqx_dashboard_swagger:error_codes([?NOT_FOUND], <<"Not Found">>) } @@ -112,8 +105,7 @@ schema("/authorization/sources/:type") -> , put => #{ description => <<"Update source">> , parameters => parameters_field() - , 'requestBody' => mk( hoconsc:union([ref(?API_SCHEMA_MODULE, Type) - || Type <- authz_sources_types(detailed)])) + , 'requestBody' => mk( hoconsc:union(authz_sources_type_refs())) , responses => #{ 204 => <<"Authorization source updated successfully">> , 400 => emqx_dashboard_swagger:error_codes([?BAD_REQUEST], <<"Bad Request">>) @@ -511,8 +503,9 @@ position_example() -> , value => #{<<"position">> => <<"after:file">>}} }. -authz_sources_types(Type) -> - emqx_authz_api_schema:authz_sources_types(Type). +authz_sources_type_refs() -> + [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])). From 1ed8e783f49a58723212bf4751391232526d2d7d Mon Sep 17 00:00:00 2001 From: JimMoen Date: Wed, 16 Mar 2022 16:43:52 +0800 Subject: [PATCH 4/5] fix(authz_api): rm authz sources full update --- .../emqx_authz/src/emqx_authz_api_sources.erl | 22 +------------------ .../test/emqx_authz_api_sources_SUITE.erl | 9 ++++++-- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index b1384b2f2..dca6bf455 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -80,16 +80,6 @@ schema("/authorization/sources") -> <<"Bad Request">>) } } - , put => - #{ description => <<"Update all sources">> - , 'requestBody' => mk( array(hoconsc:union(authz_sources_type_refs())) - , #{desc => <<"Sources">>}) - , responses => - #{ 204 => <<"Authorization source updated successfully">> - , 400 => emqx_dashboard_swagger:error_codes([?BAD_REQUEST], - <<"Bad Request">>) - } - } }; schema("/authorization/sources/:type") -> #{ 'operationId' => source @@ -183,17 +173,7 @@ 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)]); -sources(put, #{body := Body}) when is_list(Body) -> - NBody = [ begin - case Source of - #{<<"type">> := <<"file">>, <<"rules">> := Rules, <<"enable">> := Enable} -> - {ok, Filename} = write_file(acl_conf_file(), Rules), - #{<<"type">> => <<"file">>, <<"enable">> => Enable, <<"path">> => Filename}; - _ -> maybe_write_certs(Source) - end - end || Source <- Body], - update_config(?CMD_REPLACE, NBody). + update_config(?CMD_PREPEND, [maybe_write_certs(Body)]). source(Method, #{bindings := #{type := Type} = Bindings } = Req) when is_atom(Type) -> diff --git a/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl b/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl index 835d8ab48..cd07b2435 100644 --- a/apps/emqx_authz/test/emqx_authz_api_sources_SUITE.erl +++ b/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"]), - [?SOURCE2, ?SOURCE3, ?SOURCE4, ?SOURCE5, ?SOURCE6]), + [ begin {ok, 204, _} = request(post, uri(["authorization", "sources"]), Source) end + || 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). From 3769044a57641598bf8b516a9a6a2adc1ed75d7d Mon Sep 17 00:00:00 2001 From: JimMoen Date: Mon, 21 Mar 2022 15:52:25 +0800 Subject: [PATCH 5/5] fix(authz_api): ssl pem check failed returns 400 --- .../emqx_authz/src/emqx_authz_api_sources.erl | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/apps/emqx_authz/src/emqx_authz_api_sources.erl b/apps/emqx_authz/src/emqx_authz_api_sources.erl index dca6bf455..4834ead66 100644 --- a/apps/emqx_authz/src/emqx_authz_api_sources.erl +++ b/apps/emqx_authz/src/emqx_authz_api_sources.erl @@ -173,7 +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 + 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) -> @@ -211,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}, - maybe_write_certs(Body#{<<"type">> => Type})); + 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}, #{}). @@ -421,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), - maps:put(<<"ssl">>, Return, 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) ->