fix(ldap): improve the filter lex && parse

1. auto escape special chars in the filter
2. fix a bug that the value can't be `dn`

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,

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].

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)).

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'}.

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

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
     ].
 

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

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
 % %%------------------------------------------------------------------------------