ldap_dn_filter option is documented; now fetching only needed attributes in LDAP search requests (EJAB-1204)

2 files changed, 38 insertions, 10 deletions

diff --git a/doc/guide.tex b/doc/guide.tex
index a6c289ed0..20f6b17c0 100644
--- a/doc/guide.tex
+++ b/doc/guide.tex
@@ -2238,7 +2238,22 @@ You can authenticate users against an LDAP directory. Available options are:
   not forget to close brackets and do not use superfluous whitespaces. Also you
   \emph{must not} use \option{ldap\_uidattr} attribute in filter because this
   attribute will be substituted in LDAP filter automatically.
-
+  \titem{\{ldap\_dn\_filter, \{ Filter, FilterAttrs \}\}}\ind{options!ldap\_dn\_filter}
+  This filter is applied on the results returned by the main filter. This filter
+  performs additional LDAP lookup to make the complete result. This is useful
+  when you are unable to define all filter rules in \term{ldap\_filter}. You
+  can define \term{"\%u"}, \term{"\%d"}, \term{"\%s"} and \term{"\%D"} pattern
+  variables in Filter: \term{"\%u"} is replaced by a user's part of a JID,
+  \term{"\%d"} is replaced by the corresponding domain (virtual host),
+  all \term{"\%s"} variables are consecutively replaced by values of FilterAttrs
+  attributes and \term{"\%D"} is replaced by Distinguished Name. By default
+  \term{ldap\_dn\_filter} is undefined.
+  Example:
+\begin{verbatim}
+{ldap_dn_filter, {"(&(name=%s)(owner=%D)(user=%u@%d))", ["sn"]}}.
+\end{verbatim}
+  Since this filter makes additional LDAP lookups, use it only in the
+  last resort: try to define all filter rules in \term{ldap\_filter} if possible.
   \titem{\{ldap\_local\_filter, Filter\}}\ind{options!ldap\_local\_filter}
   If you can't use \term{ldap\_filter} due to performance reasons
   (the LDAP server has many users registered),
diff --git a/src/ejabberd_auth_ldap.erl b/src/ejabberd_auth_ldap.erl
index 13d1172bc..8be61f65b 100644
--- a/src/ejabberd_auth_ldap.erl
+++ b/src/ejabberd_auth_ldap.erl
@@ -217,13 +217,13 @@ get_vh_registered_users_ldap(Server) ->
     UIDs = State#state.uids,
     Eldap_ID = State#state.eldap_id,
     Server = State#state.host,
-    SortedDNAttrs = eldap_utils:usort_attrs(State#state.dn_filter_attrs),
+    ResAttrs = result_attrs(State),
     case eldap_filter:parse(State#state.sfilter) of
 		{ok, EldapFilter} ->
 		    case eldap_pool:search(Eldap_ID, [{base, State#state.base},
 						 {filter, EldapFilter},
 						 {timeout, ?LDAP_SEARCH_TIMEOUT},
-						 {attributes, SortedDNAttrs}]) of
+						 {attributes, ResAttrs}]) of
 			#eldap_search_result{entries = Entries} ->
 			    lists:flatmap(
 			      fun(#eldap_entry{attributes = Attrs,
@@ -269,15 +269,16 @@ handle_call(_Request, _From, State) ->
     {reply, bad_request, State}.
 
 find_user_dn(User, State) ->
-    DNAttrs = eldap_utils:usort_attrs(State#state.dn_filter_attrs),
+    ResAttrs = result_attrs(State),
     case eldap_filter:parse(State#state.ufilter, [{"%u", User}]) of
 	{ok, Filter} ->
-	    case eldap_pool:search(State#state.eldap_id, [{base, State#state.base},
-						     {filter, Filter},
-						     {attributes, DNAttrs}]) of
+	    case eldap_pool:search(State#state.eldap_id,
+				   [{base, State#state.base},
+				    {filter, Filter},
+				    {attributes, ResAttrs}]) of
 		#eldap_search_result{entries = [#eldap_entry{attributes = Attrs,
 							     object_name = DN} | _]} ->
-			dn_filter(DN, Attrs, State);
+		    dn_filter(DN, Attrs, State);
 		_ ->
 		    false
 	    end;
@@ -346,6 +347,14 @@ local_filter(equal, Attrs, FilterMatch) ->
 local_filter(notequal, Attrs, FilterMatch) ->
     not local_filter(equal, Attrs, FilterMatch).
 
+result_attrs(#state{uids = UIDs, dn_filter_attrs = DNFilterAttrs}) ->
+    lists:foldl(
+      fun({UID}, Acc) ->
+	      [UID | Acc];
+	 ({UID, _}, Acc) ->
+	      [UID | Acc]
+      end, DNFilterAttrs, UIDs).
+
 %%%----------------------------------------------------------------------
 %%% Auxiliary functions
 %%%----------------------------------------------------------------------
@@ -388,8 +397,12 @@ parse_options(Host) ->
     LDAPBase = ejabberd_config:get_local_option({ldap_base, Host}),
     {DNFilter, DNFilterAttrs} =
 	case ejabberd_config:get_local_option({ldap_dn_filter, Host}) of
-	    undefined -> {undefined, undefined};
-	    {DNF, DNFA} -> {DNF, DNFA}
+	    undefined ->
+		{undefined, []};
+	    {DNF, undefined} ->
+		{DNF, []};
+	    {DNF, DNFA} ->
+		{DNF, DNFA}
 	end,
 	LocalFilter = ejabberd_config:get_local_option({ldap_local_filter, Host}),
     #state{host = Host,