From b28e781c50df79ceeb97c48ba171e1e66461cc7b Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 28 Sep 2023 18:37:11 +0200 Subject: [PATCH 1/9] fix(ldap-sso): do not log error level when invalid user credentials --- apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl | 4 +++- apps/emqx_ldap/src/emqx_ldap.erl | 2 +- apps/emqx_ldap/src/emqx_ldap_bind_worker.erl | 4 +++- 3 files changed, 7 insertions(+), 3 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 dd354a007..5591d4f1e 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl @@ -131,8 +131,10 @@ login( {bind, Entry#eldap_entry.object_name, Sign} ) of - ok -> + {ok, #{result := ok}} -> ensure_user_exists(Username); + {ok, #{result := invalidCredentials} = Reason} -> + {error, Reason}; {error, _} = Error -> Error end; diff --git a/apps/emqx_ldap/src/emqx_ldap.erl b/apps/emqx_ldap/src/emqx_ldap.erl index 66ce11e20..394687bc5 100644 --- a/apps/emqx_ldap/src/emqx_ldap.erl +++ b/apps/emqx_ldap/src/emqx_ldap.erl @@ -249,7 +249,7 @@ do_ldap_query( #{pool_name := PoolName} = State ) -> LogMeta = #{connector => InstId, search => SearchOptions, state => emqx_utils:redact(State)}, - ?TRACE("QUERY", "ldap_connector_received", LogMeta), + ?TRACE("QUERY", "ldap_connector_received_query", LogMeta), case ecpool:pick_and_do( PoolName, diff --git a/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl b/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl index 27310c7ff..e3605f523 100644 --- a/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl +++ b/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl @@ -80,7 +80,9 @@ on_query( ldap_connector_query_return, #{result => ok} ), - ok; + {ok, #{result => ok}}; + {error, invalidCredentials} -> + {ok, #{result => invalidCredentials}}; {error, Reason} -> ?SLOG( error, From d858f8af3988700616115e2b7492dc873692c44b Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 28 Sep 2023 18:38:37 +0200 Subject: [PATCH 2/9] test: fix openldap docker runs --- .ci/docker-compose-file/openldap/slapd.conf | 3 +-- apps/emqx_ldap/test/data/emqx.io.ldif | 26 +++++++++++++++++++++ apps/emqx_ldap/test/data/emqx.schema | 19 +++++++++++---- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/.ci/docker-compose-file/openldap/slapd.conf b/.ci/docker-compose-file/openldap/slapd.conf index d6ba20caa..984cf3b4c 100644 --- a/.ci/docker-compose-file/openldap/slapd.conf +++ b/.ci/docker-compose-file/openldap/slapd.conf @@ -1,14 +1,13 @@ include /usr/local/etc/openldap/schema/core.schema include /usr/local/etc/openldap/schema/cosine.schema include /usr/local/etc/openldap/schema/inetorgperson.schema -include /usr/local/etc/openldap/schema/ppolicy.schema include /usr/local/etc/openldap/schema/emqx.schema TLSCACertificateFile /usr/local/etc/openldap/cacert.pem TLSCertificateFile /usr/local/etc/openldap/cert.pem TLSCertificateKeyFile /usr/local/etc/openldap/key.pem -database bdb +database mdb suffix "dc=emqx,dc=io" rootdn "cn=root,dc=emqx,dc=io" rootpw {SSHA}eoF7NhNrejVYYyGHqnt+MdKNBh4r1w3W diff --git a/apps/emqx_ldap/test/data/emqx.io.ldif b/apps/emqx_ldap/test/data/emqx.io.ldif index 138651958..71a1bb3fc 100644 --- a/apps/emqx_ldap/test/data/emqx.io.ldif +++ b/apps/emqx_ldap/test/data/emqx.io.ldif @@ -13,6 +13,12 @@ objectClass: top objectclass:organizationalUnit ou:testdevice +# create dashboard.emqx.io +dn:ou=dashboard,dc=emqx,dc=io +objectClass: top +objectclass:organizationalUnit +ou:dashboard + # create user admin dn:uid=admin,ou=testdevice,dc=emqx,dc=io objectClass: top @@ -150,3 +156,23 @@ objectClass: mqttSecurity uid: mqttuser0007 isSuperuser: TRUE userPassword: {SHA}axpQGbl00j3jvOG058y313ocnBk= + +## Try to test with base DN 'ou=dashboard,dc=emqx,dc=io' +## with a filter ugroup=group1 +## this should return 2 users in the query and fail the test + +## echo -n "viewer1" | sha1sum | cut -d' ' -f1 | xxd -r -p | base64 +dn:uid=viewer1,ou=dashboard,dc=emqx,dc=io +objectClass: top +objectClass: dashboardUser +uid: viewer1 +ugroup: group1 +userPassword: {SHA}I/LgVpQ6joiHifK7pZEQ1+0AUlg= + +## echo -n "viewer2" | sha1sum | cut -d' ' -f1 | xxd -r -p | base64 +dn:uid=viewer2,ou=dashboard,dc=emqx,dc=io +objectClass: top +objectClass: dashboardUser +uid: viewer2 +ugroup: group1 +userPassword: {SHA}SR0qZpf8pYKKAbn6ILFvX91JuQg= diff --git a/apps/emqx_ldap/test/data/emqx.schema b/apps/emqx_ldap/test/data/emqx.schema index d08548272..4ecc37bb7 100644 --- a/apps/emqx_ldap/test/data/emqx.schema +++ b/apps/emqx_ldap/test/data/emqx.schema @@ -35,10 +35,11 @@ attributetype ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.4.4 NAME ( 'mqttAccountName' 'ma SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 USAGE userApplications ) - -objectclass ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.4 NAME 'mqttUser' - AUXILIARY - MAY ( mqttPublishTopic $ mqttSubscriptionTopic $ mqttPubSubTopic $ mqttAccountName $ isSuperuser) ) +attributetype ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.5.1 NAME 'ugroup' + EQUALITY caseIgnoreMatch + SUBSTR caseIgnoreSubstringsMatch + SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 + USAGE userApplications ) objectclass ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.2 NAME 'mqttDevice' SUP top @@ -50,3 +51,13 @@ objectclass ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.3 NAME 'mqttSecurity' SUP top AUXILIARY MUST ( userPassword ) ) + +objectclass ( 1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.4 NAME 'mqttUser' + AUXILIARY + MAY ( mqttPublishTopic $ mqttSubscriptionTopic $ mqttPubSubTopic $ mqttAccountName $ isSuperuser ) ) + +objectclass (1.3.6.1.4.1.11.2.53.2.2.3.1.2.3.5 NAME 'dashboardUser' + SUP top + STRUCTURAL + MUST ( uid $ userPassword ) + MAY ( ugroup )) From 922d5a9a834a4dfb1ec800fd69dd5c9d80dc9c63 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 28 Sep 2023 21:20:50 +0200 Subject: [PATCH 3/9] fix(ldap): do not allow multi-matches to proceed if ldap query returns more than on match we should reject the auth request instead of picking the first one --- .../src/emqx_dashboard_sso_ldap.erl | 2 +- .../test/emqx_dashboard_sso_ldap_SUITE.erl | 86 +++++++++++++++++-- apps/emqx_ldap/src/emqx_ldap.erl | 20 ++++- apps/emqx_ldap/src/emqx_ldap_authn.erl | 2 +- apps/emqx_ldap/src/emqx_ldap_authn_bind.erl | 2 +- apps/emqx_ldap/src/emqx_ldap_authz.erl | 4 +- .../test/emqx_mgmt_api_test_util.erl | 2 +- 7 files changed, 103 insertions(+), 15 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 5591d4f1e..7153056ae 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl @@ -124,7 +124,7 @@ login( of {ok, []} -> {error, user_not_found}; - {ok, [Entry | _]} -> + {ok, [Entry]} -> case emqx_resource:simple_sync_query( ResourceId, diff --git a/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl b/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl index 048a4923a..8966ffca9 100644 --- a/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl +++ b/apps/emqx_dashboard_sso/test/emqx_dashboard_sso_ldap_SUITE.erl @@ -13,8 +13,12 @@ -define(LDAP_HOST, "ldap"). -define(LDAP_DEFAULT_PORT, 389). --define(LDAP_USER, <<"mqttuser0001">>). --define(LDAP_USER_PASSWORD, <<"mqttuser0001">>). +-define(LDAP_USER, <<"viewer1">>). +-define(LDAP_USER_PASSWORD, <<"viewer1">>). +-define(LDAP_BASE_DN, <<"ou=dashboard,dc=emqx,dc=io">>). +-define(LDAP_FILTER_WITH_UID, <<"(uid=${username})">>). +%% there are more than one users in this group +-define(LDAP_FILTER_WITH_GROUP, <<"(ugroup=group1)">>). -define(MOD_TAB, emqx_dashboard_sso). -define(MOD_KEY_PATH, [dashboard, sso, ldap]). @@ -22,6 +26,7 @@ -import(emqx_mgmt_api_test_util, [request/2, request/3, uri/1, request_api/3]). +%% order matters all() -> [ t_bad_create, @@ -31,6 +36,7 @@ all() -> t_login_with_bad, t_first_login, t_next_login, + t_more_than_one_user_matched, t_bad_update, t_delete ]. @@ -46,12 +52,13 @@ end_per_suite(_Config) -> [emqx_dashboard_admin:remove_user(Name) || #{username := Name} <- All], emqx_mgmt_api_test_util:end_suite([emqx_conf, emqx_dashboard_sso]). -init_per_testcase(_, Config) -> +init_per_testcase(Case, Config) -> {ok, _} = emqx_cluster_rpc:start_link(), + ?MODULE:Case({init, Config}), Config. -end_per_testcase(Case, _) -> - Case =:= t_delete_backend andalso emqx_dashboard_sso_manager:delete(ldap), +end_per_testcase(Case, Config) -> + ?MODULE:Case({'end', Config}), case erlang:whereis(node()) of undefined -> ok; @@ -61,6 +68,10 @@ end_per_testcase(Case, _) -> end, ok. +t_bad_create({init, Config}) -> + Config; +t_bad_create({'end', _}) -> + ok; t_bad_create(_) -> Path = uri(["sso", "ldap"]), ?assertMatch( @@ -90,6 +101,10 @@ t_bad_create(_) -> ), ok. +t_create({init, Config}) -> + Config; +t_create({'end', _Config}) -> + ok; t_create(_) -> check_running([]), Path = uri(["sso", "ldap"]), @@ -109,6 +124,10 @@ t_create(_) -> ?assertNotEqual(undefined, emqx_dashboard_sso_manager:lookup_state(ldap)), ok. +t_update({init, Config}) -> + Config; +t_update({'end', _Config}) -> + ok; t_update(_) -> Path = uri(["sso", "ldap"]), {ok, 200, Result} = request(put, Path, ldap_config(#{<<"enable">> => <<"true">>})), @@ -118,6 +137,10 @@ t_update(_) -> ?assertNotEqual(undefined, emqx_dashboard_sso_manager:lookup_state(ldap)), ok. +t_get({init, Config}) -> + Config; +t_get({'end', _Config}) -> + ok; t_get(_) -> Path = uri(["sso", "ldap"]), {ok, 200, Result} = request(get, Path), @@ -127,6 +150,10 @@ t_get(_) -> {ok, 400, _} = request(get, NotExists), ok. +t_login_with_bad({init, Config}) -> + Config; +t_login_with_bad({'end', _Config}) -> + ok; t_login_with_bad(_) -> Path = uri(["sso", "login", "ldap"]), Req = #{ @@ -138,6 +165,10 @@ t_login_with_bad(_) -> ?assertMatch(#{code := <<"BAD_USERNAME_OR_PWD">>}, decode_json(Result)), ok. +t_first_login({init, Config}) -> + Config; +t_first_login({'end', _Config}) -> + ok; t_first_login(_) -> Path = uri(["sso", "login", "ldap"]), Req = #{ @@ -154,6 +185,10 @@ t_first_login(_) -> ), ok. +t_next_login({init, Config}) -> + Config; +t_next_login({'end', _Config}) -> + ok; t_next_login(_) -> Path = uri(["sso", "login", "ldap"]), Req = #{ @@ -165,6 +200,38 @@ t_next_login(_) -> ?assertMatch(#{license := _, token := _}, decode_json(Result)), ok. +t_more_than_one_user_matched({init, Config}) -> + emqx_logger:set_primary_log_level(error), + Config; +t_more_than_one_user_matched({'end', _Config}) -> + %% restore default config + Path = uri(["sso", "ldap"]), + {ok, 200, _} = request(put, Path, ldap_config(#{<<"enable">> => true})), + ok; +t_more_than_one_user_matched(_) -> + Path = uri(["sso", "ldap"]), + %% change to query with ugroup=group1 + NewConfig = ldap_config(#{ + <<"enable">> => true, + <<"base_dn">> => ?LDAP_BASE_DN, + <<"filter">> => ?LDAP_FILTER_WITH_GROUP + }), + ?assertMatch({ok, 200, _}, request(put, Path, NewConfig)), + check_running([<<"ldap">>]), + Path1 = uri(["sso", "login", "ldap"]), + Req = #{ + <<"backend">> => <<"ldap">>, + <<"username">> => ?LDAP_USER, + <<"password">> => ?LDAP_USER_PASSWORD + }, + {ok, 401, Result} = request(post, Path1, Req), + ?assertMatch(#{code := <<"BAD_USERNAME_OR_PWD">>}, decode_json(Result)), + ok. + +t_bad_update({init, Config}) -> + Config; +t_bad_update({'end', _Config}) -> + ok; t_bad_update(_) -> Path = uri(["sso", "ldap"]), ?assertMatch( @@ -184,9 +251,12 @@ t_bad_update(_) -> ?assertMatch( [#{backend := <<"ldap">>, enable := true, running := false, last_error := _}], get_sso() ), - ok. +t_delete({init, Config}) -> + Config; +t_delete({'end', _Config}) -> + ok; t_delete(_) -> Path = uri(["sso", "ldap"]), ?assertMatch({ok, 204, _}, request(delete, Path)), @@ -214,8 +284,8 @@ ldap_config(Override) -> <<"backend">> => <<"ldap">>, <<"enable">> => <<"false">>, <<"server">> => ldap_server(), - <<"base_dn">> => <<"uid=${username},ou=testdevice,dc=emqx,dc=io">>, - <<"filter">> => <<"(objectClass=mqttUser)">>, + <<"base_dn">> => ?LDAP_BASE_DN, + <<"filter">> => ?LDAP_FILTER_WITH_UID, <<"username">> => <<"cn=root,dc=emqx,dc=io">>, <<"password">> => <<"public">>, <<"pool_size">> => 8 diff --git a/apps/emqx_ldap/src/emqx_ldap.erl b/apps/emqx_ldap/src/emqx_ldap.erl index 394687bc5..e923ee7c3 100644 --- a/apps/emqx_ldap/src/emqx_ldap.erl +++ b/apps/emqx_ldap/src/emqx_ldap.erl @@ -262,7 +262,25 @@ do_ldap_query( ldap_connector_query_return, #{result => Result} ), - {ok, Result#eldap_search_result.entries}; + case Result#eldap_search_result.entries of + [_] = Entry -> + {ok, Entry}; + [_ | _] = L -> + %% Accept only a single exact match. + %% Multiple matches likely indicate: + %% 1. A misconfiguration in EMQX, allowing overly broad query conditions. + %% 2. Indistinguishable entries in the LDAP database. + %% Neither scenario should be allowed to proceed. + Msg = "ldap_query_found_more_than_one_match", + ?SLOG( + error, + LogMeta#{ + msg => "ldap_query_found_more_than_one_match", + count => length(L) + } + ), + {error, {recoverable_error, Msg}} + end; {error, 'noSuchObject'} -> {ok, []}; {error, Reason} -> diff --git a/apps/emqx_ldap/src/emqx_ldap_authn.erl b/apps/emqx_ldap/src/emqx_ldap_authn.erl index cbe62dfb2..16540bec8 100644 --- a/apps/emqx_ldap/src/emqx_ldap_authn.erl +++ b/apps/emqx_ldap/src/emqx_ldap_authn.erl @@ -131,7 +131,7 @@ authenticate( of {ok, []} -> ignore; - {ok, [Entry | _]} -> + {ok, [Entry]} -> is_enabled(Password, Entry, State); {error, Reason} -> ?TRACE_AUTHN_PROVIDER(error, "ldap_query_failed", #{ diff --git a/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl b/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl index fe192bf88..37216ef3e 100644 --- a/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl +++ b/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl @@ -95,7 +95,7 @@ authenticate( of {ok, []} -> ignore; - {ok, [Entry | _]} -> + {ok, [Entry]} -> case emqx_resource:simple_sync_query( ResourceId, diff --git a/apps/emqx_ldap/src/emqx_ldap_authz.erl b/apps/emqx_ldap/src/emqx_ldap_authz.erl index b48b3a48b..bfff9149f 100644 --- a/apps/emqx_ldap/src/emqx_ldap_authz.erl +++ b/apps/emqx_ldap/src/emqx_ldap_authz.erl @@ -111,11 +111,11 @@ authorize( case emqx_resource:simple_sync_query(ResourceID, {query, Client, Attrs, QueryTimeout}) of {ok, []} -> nomatch; - {ok, [Entry | _]} -> + {ok, [Entry]} -> do_authorize(Action, Topic, Attrs, Entry); {error, Reason} -> ?SLOG(error, #{ - msg => "query_ldap_error", + msg => "ldap_query_failed", reason => emqx_utils:redact(Reason), resource_id => ResourceID }), diff --git a/apps/emqx_management/test/emqx_mgmt_api_test_util.erl b/apps/emqx_management/test/emqx_mgmt_api_test_util.erl index e6c3bb3d6..cd768a529 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_test_util.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_test_util.erl @@ -119,7 +119,7 @@ do_request_api(Method, Request, Opts) -> ReturnAll = maps:get(return_all, Opts, false), CompatibleMode = maps:get(compatible_mode, Opts, false), HttpcReqOpts = maps:get(httpc_req_opts, Opts, []), - ct:pal("Method: ~p, Request: ~p, Opts: ~p", [Method, Request, Opts]), + ct:pal("~p: ~p~nOpts: ~p", [Method, Request, Opts]), case httpc:request(Method, Request, [], HttpcReqOpts) of {error, socket_closed_remotely} -> {error, socket_closed_remotely}; From b267fc2588603f8f40771b57e7af2f0081a7106c Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 28 Sep 2023 21:22:33 +0200 Subject: [PATCH 4/9] chore: bump release version to 5.3.0 --- apps/emqx/include/emqx_release.hrl | 4 ++-- deploy/charts/emqx-enterprise/Chart.yaml | 4 ++-- deploy/charts/emqx/Chart.yaml | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/emqx/include/emqx_release.hrl b/apps/emqx/include/emqx_release.hrl index 801773663..381c5fa13 100644 --- a/apps/emqx/include/emqx_release.hrl +++ b/apps/emqx/include/emqx_release.hrl @@ -32,10 +32,10 @@ %% `apps/emqx/src/bpapi/README.md' %% Opensource edition --define(EMQX_RELEASE_CE, "5.2.1"). +-define(EMQX_RELEASE_CE, "5.3.0"). %% Enterprise edition --define(EMQX_RELEASE_EE, "5.3.0-alpha.2"). +-define(EMQX_RELEASE_EE, "5.3.0-rc.1"). %% The HTTP API version -define(EMQX_API_VERSION, "5.0"). diff --git a/deploy/charts/emqx-enterprise/Chart.yaml b/deploy/charts/emqx-enterprise/Chart.yaml index c89a7f90d..e0720bad9 100644 --- a/deploy/charts/emqx-enterprise/Chart.yaml +++ b/deploy/charts/emqx-enterprise/Chart.yaml @@ -14,8 +14,8 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. -version: 5.3.0-alpha.2 +version: 5.3.0-rc.1 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. -appVersion: 5.3.0-alpha.2 +appVersion: 5.3.0-rc.1 diff --git a/deploy/charts/emqx/Chart.yaml b/deploy/charts/emqx/Chart.yaml index d32268ee2..f1c26cc8d 100644 --- a/deploy/charts/emqx/Chart.yaml +++ b/deploy/charts/emqx/Chart.yaml @@ -14,8 +14,8 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. -version: 5.2.1 +version: 5.3.0 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. -appVersion: 5.2.1 +appVersion: 5.3.0 From cc5dab1dc7fdf4df85cf2d43fd75837991ce210a Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 28 Sep 2023 21:29:52 +0200 Subject: [PATCH 5/9] chore: fix code style --- apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl | 2 +- apps/emqx_ldap/src/emqx_ldap_bind_worker.erl | 4 ++-- 2 files changed, 3 insertions(+), 3 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 7153056ae..de789df0b 100644 --- a/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl +++ b/apps/emqx_dashboard_sso/src/emqx_dashboard_sso_ldap.erl @@ -133,7 +133,7 @@ login( of {ok, #{result := ok}} -> ensure_user_exists(Username); - {ok, #{result := invalidCredentials} = Reason} -> + {ok, #{result := 'invalidCredentials'} = Reason} -> {error, Reason}; {error, _} = Error -> Error diff --git a/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl b/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl index e3605f523..1b1bd3ce9 100644 --- a/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl +++ b/apps/emqx_ldap/src/emqx_ldap_bind_worker.erl @@ -81,8 +81,8 @@ on_query( #{result => ok} ), {ok, #{result => ok}}; - {error, invalidCredentials} -> - {ok, #{result => invalidCredentials}}; + {error, 'invalidCredentials'} -> + {ok, #{result => 'invalidCredentials'}}; {error, Reason} -> ?SLOG( error, From 6edfdf16d3f3cfa665ddce85331fd430f795b5e2 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 28 Sep 2023 23:16:06 +0200 Subject: [PATCH 6/9] test: update base docker image for ldap --- .ci/docker-compose-file/openldap/Dockerfile | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.ci/docker-compose-file/openldap/Dockerfile b/.ci/docker-compose-file/openldap/Dockerfile index dd0114b64..e6c319bca 100644 --- a/.ci/docker-compose-file/openldap/Dockerfile +++ b/.ci/docker-compose-file/openldap/Dockerfile @@ -1,9 +1,12 @@ -FROM buildpack-deps:bookworm +FROM ubuntu:20.04 ARG LDAP_TAG=2.5.16 -RUN apt-get update && apt-get install -y groff groff-base -RUN wget https://www.openldap.org/software/download/OpenLDAP/openldap-release/openldap-${LDAP_TAG}.tgz \ +ENV DEBIAN_FRONTEND noninteractive + +RUN apt-get update -y +RUN apt-get install -y groff groff-base curl build-essential +RUN curl -o openldap-${LDAP_TAG}.tgz https://www.openldap.org/software/download/OpenLDAP/openldap-release/openldap-${LDAP_TAG}.tgz \ && tar xvzf openldap-${LDAP_TAG}.tgz \ && cd openldap-${LDAP_TAG} \ && ./configure && make depend && make && make install \ From 0ff28afc3dca87f56c1586b3b57520cde3109480 Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 28 Sep 2023 23:37:02 +0200 Subject: [PATCH 7/9] test: use pre-build openldap base image --- .ci/docker-compose-file/docker-compose-ldap.yaml | 2 -- .ci/docker-compose-file/openldap/Dockerfile | 14 +------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/.ci/docker-compose-file/docker-compose-ldap.yaml b/.ci/docker-compose-file/docker-compose-ldap.yaml index f92df47a0..03627c763 100644 --- a/.ci/docker-compose-file/docker-compose-ldap.yaml +++ b/.ci/docker-compose-file/docker-compose-ldap.yaml @@ -6,8 +6,6 @@ services: build: context: ../.. dockerfile: .ci/docker-compose-file/openldap/Dockerfile - args: - LDAP_TAG: ${LDAP_TAG} image: openldap #ports: # - 389:389 diff --git a/.ci/docker-compose-file/openldap/Dockerfile b/.ci/docker-compose-file/openldap/Dockerfile index e6c319bca..9eba7b3a5 100644 --- a/.ci/docker-compose-file/openldap/Dockerfile +++ b/.ci/docker-compose-file/openldap/Dockerfile @@ -1,16 +1,4 @@ -FROM ubuntu:20.04 - -ARG LDAP_TAG=2.5.16 - -ENV DEBIAN_FRONTEND noninteractive - -RUN apt-get update -y -RUN apt-get install -y groff groff-base curl build-essential -RUN curl -o openldap-${LDAP_TAG}.tgz https://www.openldap.org/software/download/OpenLDAP/openldap-release/openldap-${LDAP_TAG}.tgz \ - && tar xvzf openldap-${LDAP_TAG}.tgz \ - && cd openldap-${LDAP_TAG} \ - && ./configure && make depend && make && make install \ - && cd .. && rm -rf openldap-${LDAP_TAG} +FROM docker.io/zmstone/openldap:2.5.16 COPY .ci/docker-compose-file/openldap/slapd.conf /usr/local/etc/openldap/slapd.conf COPY apps/emqx_ldap/test/data/emqx.io.ldif /usr/local/etc/openldap/schema/emqx.io.ldif From 9ee2cb9c79f7d1dfa6c9e6e5d4bb864563ff581a Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Thu, 28 Sep 2023 23:58:34 +0200 Subject: [PATCH 8/9] fix(ldap): return unrecoverable_error if more than on match found --- apps/emqx_ldap/src/emqx_ldap.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx_ldap/src/emqx_ldap.erl b/apps/emqx_ldap/src/emqx_ldap.erl index e923ee7c3..628397ff3 100644 --- a/apps/emqx_ldap/src/emqx_ldap.erl +++ b/apps/emqx_ldap/src/emqx_ldap.erl @@ -279,7 +279,7 @@ do_ldap_query( count => length(L) } ), - {error, {recoverable_error, Msg}} + {error, {unrecoverable_error, Msg}} end; {error, 'noSuchObject'} -> {ok, []}; From 4a4730ad46d55b1fb3365c5aa4593fdfb24bb5cd Mon Sep 17 00:00:00 2001 From: "Zaiming (Stone) Shi" Date: Fri, 29 Sep 2023 00:51:05 +0200 Subject: [PATCH 9/9] fix(ldap): handle invalidCredentials in ldap authn --- apps/emqx_ldap/src/emqx_ldap_authn_bind.erl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl b/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl index 37216ef3e..fce78312c 100644 --- a/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl +++ b/apps/emqx_ldap/src/emqx_ldap_authn_bind.erl @@ -102,8 +102,14 @@ authenticate( {bind, Entry#eldap_entry.object_name, Credential} ) of - ok -> + {ok, #{result := ok}} -> {ok, #{is_superuser => false}}; + {ok, #{result := 'invalidCredentials'}} -> + ?TRACE_AUTHN_PROVIDER(error, "ldap_bind_failed", #{ + resource => ResourceId, + reason => 'invalidCredentials' + }), + {error, bad_username_or_password}; {error, Reason} -> ?TRACE_AUTHN_PROVIDER(error, "ldap_bind_failed", #{ resource => ResourceId,