From 0c33df3912d0d7e96dff6b3b5f32819c25458319 Mon Sep 17 00:00:00 2001 From: firest Date: Tue, 26 Sep 2023 23:28:37 +0800 Subject: [PATCH 1/3] fix(ldap): improve the LDAP `parse_config` function --- .../src/emqx_dashboard_sso_ldap.erl | 14 +---------- apps/emqx_ldap/src/emqx_ldap.erl | 24 ++++++++++++++++++- apps/emqx_ldap/src/emqx_ldap_authn.erl | 22 ++++------------- apps/emqx_ldap/src/emqx_ldap_authz.erl | 19 ++++----------- 4 files changed, 33 insertions(+), 46 deletions(-) diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl index bea8ef7c6..dff3b2d31 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl @@ -86,19 +86,7 @@ destroy(#{resource_id := ResourceId}) -> parse_config(Config0) -> Config = ensure_bind_password(Config0), - State = lists:foldl( - fun(Key, Acc) -> - case maps:find(Key, Config) of - {ok, Value} when is_binary(Value) -> - Acc#{Key := erlang:binary_to_list(Value)}; - _ -> - Acc - end - end, - Config, - [query_timeout] - ), - {Config, State}. + {Config, emqx_ldap:parse_config(Config, [query_timeout], [])}. %% In this feature, the `bind_password` is fixed, so it should conceal from the swagger, %% but the connector still needs it, hence we should add it back here diff --git a/apps/emqx_ldap/src/emqx_ldap.erl b/apps/emqx_ldap/src/emqx_ldap.erl index 7b856521c..20e6550df 100644 --- a/apps/emqx_ldap/src/emqx_ldap.erl +++ b/apps/emqx_ldap/src/emqx_ldap.erl @@ -27,7 +27,7 @@ -export([roots/0, fields/1, desc/1]). --export([do_get_status/1]). +-export([do_get_status/1, parse_config/3]). -define(LDAP_HOST_OPTIONS, #{ default_port => 389 @@ -114,6 +114,28 @@ ensure_username(required) -> ensure_username(Field) -> ?ECS:username(Field). +parse_config(Config, ToKeep, ToString) -> + Convert = fun(Value) -> + case lists:member(Value, ToString) of + true -> + erlang:binary_to_list(Value); + _ -> + Value + end + end, + lists:foldl( + fun(Key, Acc) -> + case maps:find(Key, Config) of + {ok, Value} -> + Acc#{Key => Convert(Value)}; + _ -> + Acc + end + end, + #{}, + ToKeep ++ ToString + ). + %% =================================================================== callback_mode() -> always_sync. diff --git a/apps/emqx_ldap/src/emqx_ldap_authn.erl b/apps/emqx_ldap/src/emqx_ldap_authn.erl index d5f2658bb..8f60d57b3 100644 --- a/apps/emqx_ldap/src/emqx_ldap_authn.erl +++ b/apps/emqx_ldap/src/emqx_ldap_authn.erl @@ -91,14 +91,14 @@ refs() -> create(_AuthenticatorID, Config) -> do_create(?MODULE, Config). -do_create(Module, Config0) -> +do_create(Module, Config) -> ResourceId = emqx_authn_utils:make_resource_id(Module), - {Config, State} = parse_config(Config0), + State = parse_config(Config), {ok, _Data} = emqx_authn_utils:create_resource(ResourceId, emqx_ldap, Config), {ok, State#{resource_id => ResourceId}}. -update(Config0, #{resource_id := ResourceId} = _State) -> - {Config, NState} = parse_config(Config0), +update(Config, #{resource_id := ResourceId} = _State) -> + NState = parse_config(Config), case emqx_authn_utils:update_resource(emqx_ldap, Config, ResourceId) of {error, Reason} -> error({load_config_error, Reason}); @@ -143,19 +143,7 @@ authenticate( end. parse_config(Config) -> - State = lists:foldl( - fun(Key, Acc) -> - case maps:find(Key, Config) of - {ok, Value} when is_binary(Value) -> - Acc#{Key := erlang:binary_to_list(Value)}; - _ -> - Acc - end - end, - Config, - [password_attribute, is_superuser_attribute, query_timeout] - ), - {Config, State}. + emqx_ldap:parse_config(Config, [query_timeout], [password_attribute, is_superuser_attribute]). %% To compatible v4.x is_enabled(Password, #eldap_entry{attributes = Attributes} = Entry, State) -> diff --git a/apps/emqx_ldap/src/emqx_ldap_authz.erl b/apps/emqx_ldap/src/emqx_ldap_authz.erl index 13110306c..7a714b87e 100644 --- a/apps/emqx_ldap/src/emqx_ldap_authz.erl +++ b/apps/emqx_ldap/src/emqx_ldap_authz.erl @@ -134,21 +134,10 @@ do_authorize(_Action, _Topic, [], _Entry) -> nomatch. new_annotations(Init, Source) -> - lists:foldl( - fun(Attr, Acc) -> - Acc#{ - Attr => - case maps:get(Attr, Source) of - Value when is_binary(Value) -> - erlang:binary_to_list(Value); - Value -> - Value - end - } - end, - Init, - [publish_attribute, subscribe_attribute, all_attribute] - ). + State = emqx_ldap:parse_config(Source, [query_timeout], [ + publish_attribute, subscribe_attribute, all_attribute + ]), + maps:merge(Init, State). select_attrs(#{action_type := publish}, #{publish_attribute := Pub, all_attribute := All}) -> [Pub, All]; From ffd9a35d694aa36b03eb85e818c3ad1f756369b7 Mon Sep 17 00:00:00 2001 From: firest Date: Tue, 26 Sep 2023 23:33:47 +0800 Subject: [PATCH 2/3] chore: Add missing changelog --- changes/ce/fix-11667.en.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/ce/fix-11667.en.md diff --git a/changes/ce/fix-11667.en.md b/changes/ce/fix-11667.en.md new file mode 100644 index 000000000..5aab26e13 --- /dev/null +++ b/changes/ce/fix-11667.en.md @@ -0,0 +1 @@ +Disable access to the `logout` endpoint by the API key, this endpoint is for the Dashboard only. From 57781d0544a8f01eac86e3c1478243cb21d27a93 Mon Sep 17 00:00:00 2001 From: firest Date: Wed, 27 Sep 2023 22:15:19 +0800 Subject: [PATCH 3/3] fix(ldap): remove the parse_config, it never work --- .../src/emqx_dashboard_sso_ldap.erl | 2 +- apps/emqx_ldap/src/emqx_ldap.erl | 24 +------------------ apps/emqx_ldap/src/emqx_ldap_authn.erl | 2 +- apps/emqx_ldap/src/emqx_ldap_authz.erl | 6 ++--- 4 files changed, 6 insertions(+), 28 deletions(-) diff --git a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl index dff3b2d31..007a7ea88 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl @@ -86,7 +86,7 @@ destroy(#{resource_id := ResourceId}) -> parse_config(Config0) -> Config = ensure_bind_password(Config0), - {Config, emqx_ldap:parse_config(Config, [query_timeout], [])}. + {Config, maps:with([query_timeout], Config0)}. %% In this feature, the `bind_password` is fixed, so it should conceal from the swagger, %% but the connector still needs it, hence we should add it back here diff --git a/apps/emqx_ldap/src/emqx_ldap.erl b/apps/emqx_ldap/src/emqx_ldap.erl index 20e6550df..7b856521c 100644 --- a/apps/emqx_ldap/src/emqx_ldap.erl +++ b/apps/emqx_ldap/src/emqx_ldap.erl @@ -27,7 +27,7 @@ -export([roots/0, fields/1, desc/1]). --export([do_get_status/1, parse_config/3]). +-export([do_get_status/1]). -define(LDAP_HOST_OPTIONS, #{ default_port => 389 @@ -114,28 +114,6 @@ ensure_username(required) -> ensure_username(Field) -> ?ECS:username(Field). -parse_config(Config, ToKeep, ToString) -> - Convert = fun(Value) -> - case lists:member(Value, ToString) of - true -> - erlang:binary_to_list(Value); - _ -> - Value - end - end, - lists:foldl( - fun(Key, Acc) -> - case maps:find(Key, Config) of - {ok, Value} -> - Acc#{Key => Convert(Value)}; - _ -> - Acc - end - end, - #{}, - ToKeep ++ ToString - ). - %% =================================================================== callback_mode() -> always_sync. diff --git a/apps/emqx_ldap/src/emqx_ldap_authn.erl b/apps/emqx_ldap/src/emqx_ldap_authn.erl index 8f60d57b3..cbe62dfb2 100644 --- a/apps/emqx_ldap/src/emqx_ldap_authn.erl +++ b/apps/emqx_ldap/src/emqx_ldap_authn.erl @@ -143,7 +143,7 @@ authenticate( end. parse_config(Config) -> - emqx_ldap:parse_config(Config, [query_timeout], [password_attribute, is_superuser_attribute]). + maps:with([query_timeout, password_attribute, is_superuser_attribute], Config). %% To compatible v4.x is_enabled(Password, #eldap_entry{attributes = Attributes} = Entry, State) -> diff --git a/apps/emqx_ldap/src/emqx_ldap_authz.erl b/apps/emqx_ldap/src/emqx_ldap_authz.erl index 7a714b87e..b48b3a48b 100644 --- a/apps/emqx_ldap/src/emqx_ldap_authz.erl +++ b/apps/emqx_ldap/src/emqx_ldap_authz.erl @@ -134,9 +134,9 @@ do_authorize(_Action, _Topic, [], _Entry) -> nomatch. new_annotations(Init, Source) -> - State = emqx_ldap:parse_config(Source, [query_timeout], [ - publish_attribute, subscribe_attribute, all_attribute - ]), + State = maps:with( + [query_timeout, publish_attribute, subscribe_attribute, all_attribute], Source + ), maps:merge(Init, State). select_attrs(#{action_type := publish}, #{publish_attribute := Pub, all_attribute := All}) ->