diff --git a/apps/emqx_ldap/src/emqx_ldap.app.src b/apps/emqx_ldap/src/emqx_ldap.app.src index e6abb2363..8afe3126c 100644 --- a/apps/emqx_ldap/src/emqx_ldap.app.src +++ b/apps/emqx_ldap/src/emqx_ldap.app.src @@ -1,6 +1,6 @@ {application, emqx_ldap, [ {description, "EMQX LDAP Connector"}, - {vsn, "0.1.3"}, + {vsn, "0.1.4"}, {registered, []}, {applications, [ kernel, diff --git a/apps/emqx_ldap/src/emqx_ldap.erl b/apps/emqx_ldap/src/emqx_ldap.erl index 82f749967..2db801b9b 100644 --- a/apps/emqx_ldap/src/emqx_ldap.erl +++ b/apps/emqx_ldap/src/emqx_ldap.erl @@ -226,7 +226,9 @@ on_query( #{base_tokens := BaseTks, filter_tokens := FilterTks} = State ) -> Base = emqx_placeholder:proc_tmpl(BaseTks, Data), - FilterBin = emqx_placeholder:proc_tmpl(FilterTks, Data), + FilterBin = emqx_placeholder:proc_tmpl(FilterTks, Data, #{ + return => full_binary, var_trans => fun filter_escape/1 + }), case emqx_ldap_filter_parser:scan_and_parse(FilterBin) of {ok, Filter} -> do_ldap_query( @@ -278,7 +280,7 @@ do_ldap_query( error, LogMeta#{ msg => "ldap_query_found_more_than_one_match", - count => length(Entries) + count => Count } ), {error, {unrecoverable_error, Msg}} @@ -314,3 +316,20 @@ do_prepare_template([{filter, V} | T], State) -> do_prepare_template(T, State#{filter_tokens => emqx_placeholder:preproc_tmpl(V)}); do_prepare_template([], State) -> State. + +filter_escape(Binary) when is_binary(Binary) -> + filter_escape(erlang:binary_to_list(Binary)); +filter_escape([$\\ | T]) -> + [$\\, $\\ | filter_escape(T)]; +filter_escape([Char | T]) -> + case lists:member(Char, filter_control_chars()) of + true -> + [$\\, Char | filter_escape(T)]; + _ -> + [Char | filter_escape(T)] + end; +filter_escape([]) -> + []. + +filter_control_chars() -> + [$(, $), $&, $|, $=, $!, $~, $>, $<, $:, $*, $\t, $\n, $\r]. diff --git a/apps/emqx_ldap/src/emqx_ldap_filter_lexer.xrl b/apps/emqx_ldap/src/emqx_ldap_filter_lexer.xrl index a82a3ee3e..7c4d62bc9 100644 --- a/apps/emqx_ldap/src/emqx_ldap_filter_lexer.xrl +++ b/apps/emqx_ldap/src/emqx_ldap_filter_lexer.xrl @@ -2,8 +2,9 @@ Definitions. Control = [()&|!=~><:*] White = [\s\t\n\r]+ -NonString = [^()&|!=~><:*\s\t\n\r] -String = {NonString}+ +StringChars = [^()&|!=~><:*\t\n\r] +Escape = \\{Control}|\\{White} +String = ({Escape}|{StringChars})+ Rules. @@ -17,10 +18,10 @@ Rules. >= : {token, {greaterOrEqual, TokenLine}}. <= : {token, {lessOrEqual, TokenLine}}. \* : {token, {asterisk, TokenLine}}. +\:dn : {token, {dn, TokenLine}}. \: : {token, {colon, TokenLine}}. -dn : {token, {dn, TokenLine}}. {White} : skip_token. -{String} : {token, {string, TokenLine, TokenChars}}. +{String} : {token, {string, TokenLine, to_string(TokenChars)}}. %% Leex will hang if a composite operation is missing a character {Control} : {error, lists:flatten(io_lib:format("Unexpected Tokens:~ts", [TokenChars]))}. @@ -29,3 +30,8 @@ Erlang code. %%-------------------------------------------------------------------- %% Copyright (c) 2023 EMQ Technologies Co., Ltd. All Rights Reserved. %%-------------------------------------------------------------------- +%% eldap does not support neither the '\28value\29' nor '\(value\)' +%% so after the tokenization we should remove all escape character +to_string(TokenChars) -> + String = string:trim(TokenChars), + lists:flatten(string:replace(String, "\\", "", all)). diff --git a/apps/emqx_ldap/src/emqx_ldap_filter_parser.yrl b/apps/emqx_ldap/src/emqx_ldap_filter_parser.yrl index e1b1ed98e..a400132f8 100644 --- a/apps/emqx_ldap/src/emqx_ldap_filter_parser.yrl +++ b/apps/emqx_ldap/src/emqx_ldap_filter_parser.yrl @@ -92,7 +92,7 @@ type -> value: {type, '$1'}. dnattrs -> - colon dn: {dnAttributes, true}. + dn: {dnAttributes, true}. matchingrule -> colon value: {matchingRule, '$2'}. diff --git a/apps/emqx_ldap/test/data/emqx.io.ldif b/apps/emqx_ldap/test/data/emqx.io.ldif index 71a1bb3fc..62ed4c3d5 100644 --- a/apps/emqx_ldap/test/data/emqx.io.ldif +++ b/apps/emqx_ldap/test/data/emqx.io.ldif @@ -157,6 +157,15 @@ uid: mqttuser0007 isSuperuser: TRUE userPassword: {SHA}axpQGbl00j3jvOG058y313ocnBk= +objectClass: top +dn:uid=mqttuser0008 (test),ou=testdevice,dc=emqx,dc=io +objectClass: mqttUser +objectClass: mqttDevice +objectClass: mqttSecurity +uid: mqttuser0008 (test) +isSuperuser: TRUE +userPassword: {SHA}FCzJLOp66OwsZ9DQzXSxdTd9c0U= + ## 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 diff --git a/apps/emqx_ldap/test/emqx_ldap_authn_SUITE.erl b/apps/emqx_ldap/test/emqx_ldap_authn_SUITE.erl index 7b5220b04..94e3ab864 100644 --- a/apps/emqx_ldap/test/emqx_ldap_authn_SUITE.erl +++ b/apps/emqx_ldap/test/emqx_ldap_authn_SUITE.erl @@ -241,7 +241,8 @@ user_seeds() -> %% Disabled New(<<"mqttuser0006">>, <<"mqttuser0006">>, {error, user_disabled}), %% IsSuperuser - New(<<"mqttuser0007">>, <<"mqttuser0007">>, {ok, #{is_superuser => true}}) + New(<<"mqttuser0007">>, <<"mqttuser0007">>, {ok, #{is_superuser => true}}), + New(<<"mqttuser0008 (test)">>, <<"mqttuser0008 (test)">>, {ok, #{is_superuser => true}}) | Valid ]. diff --git a/apps/emqx_ldap/test/emqx_ldap_authn_bind_SUITE.erl b/apps/emqx_ldap/test/emqx_ldap_authn_bind_SUITE.erl index 996416f52..a7b403488 100644 --- a/apps/emqx_ldap/test/emqx_ldap_authn_bind_SUITE.erl +++ b/apps/emqx_ldap/test/emqx_ldap_authn_bind_SUITE.erl @@ -167,7 +167,8 @@ t_update(_Config) -> CorrectConfig = raw_ldap_auth_config(), IncorrectConfig = CorrectConfig#{ - <<"base_dn">> => <<"ou=testdevice,dc=emqx,dc=io">> + <<"base_dn">> => <<"ou=testdevice,dc=emqx,dc=io">>, + <<"filter">> => <<"(objectClass=mqttUser)">> }, {ok, _} = emqx:update_config( @@ -208,7 +209,8 @@ raw_ldap_auth_config() -> <<"mechanism">> => <<"password_based">>, <<"backend">> => <<"ldap_bind">>, <<"server">> => ldap_server(), - <<"base_dn">> => <<"uid=${username},ou=testdevice,dc=emqx,dc=io">>, + <<"base_dn">> => <<"ou=testdevice,dc=emqx,dc=io">>, + <<"filter">> => <<"(uid=${username})">>, <<"username">> => <<"cn=root,dc=emqx,dc=io">>, <<"password">> => <<"public">>, <<"pool_size">> => 8, @@ -226,13 +228,22 @@ user_seeds() -> result => Result } end, + + Normal = lists:map( + fun(Idx) -> + erlang:iolist_to_binary(io_lib:format("mqttuser000~b", [Idx])) + end, + lists:seq(1, 5) + ), + + Specials = [<<"mqttuser0008 (test)">>], + Valid = lists:map( - fun(Idx) -> - Username = erlang:iolist_to_binary(io_lib:format("mqttuser000~b", [Idx])), + fun(Username) -> New(Username, Username, {ok, #{is_superuser => false}}) end, - lists:seq(1, 5) + Normal ++ Specials ), [ %% Not exists diff --git a/apps/emqx_ldap/test/emqx_ldap_filter_SUITE.erl b/apps/emqx_ldap/test/emqx_ldap_filter_SUITE.erl index 1a7e970a8..eb2b6161c 100644 --- a/apps/emqx_ldap/test/emqx_ldap_filter_SUITE.erl +++ b/apps/emqx_ldap/test/emqx_ldap_filter_SUITE.erl @@ -223,6 +223,23 @@ t_error(_Config) -> ?assertMatch({error, _}, scan_and_parse("attr=value")), ?assertMatch({error, _}, scan_and_parse("(a=b)(c=d)")). +t_escape(_Config) -> + ?assertEqual( + 'and'([equalityMatch("a", "(value)")]), + parse("(&(a=\\(value\\)))") + ), + ?assertEqual( + 'or'([equalityMatch("a", "name (1)")]), + parse("(|(a=name \\(1\\)))") + ), + ?assertEqual( + 'or'([equalityMatch("a", "name (1) *")]), + parse("(|(a=name\\ \\(1\\) \\*))") + ). + +t_value_eql_dn(_Config) -> + ?assertEqual('and'([equalityMatch("a", "dn")]), parse("(&(a=dn))")). + % %%------------------------------------------------------------------------------ % %% Helpers % %%------------------------------------------------------------------------------