A couple of comments on this patch. I have attached a "fixup" patch on
top of your v4 that should address them.
- I think the bracketing of the LDAP URL synopsis is wrong.
- I have dropped the sentence that LDAP URL extensions are not
supported. That sentence was written mainly to point out that filters
are not supported, which they are now. There is nothing beyond filters
and extensions left in LDAP URLs, AFAIK.
- We have previously used the ldapsearchattribute a bit strangely. We
use it as both the search filter and as the attribute to return from the
search, but we don't actually care about the returned attribute (we only
use the DN (which is not a real attribute)). That hasn't been a real
problem, but now if you specify a filter, as the code stands it will
always request the "uid" attribute, which won't work if there is no such
attribute. I have found that there is an official way to request "no
attribute", so I have fixed the code to do that.
- I was wondering whether we need a way to escape the % (%%?) but it
doesn't seem worth bothering.
I have been looking around the Internet how this functionality compares
to other LDAP authentication systems.
I didn't see the origin of the %u idea in this thread, but perhaps it
came from Dovecot. But there it's a general configuration file syntax,
nothing specific to LDAP. In nginx you use %(username), which again is
general configuration file syntax. I'm OK with using %u.
The original LDAP URL design was adapted from Apache
(https://httpd.apache.org/docs/2.4/mod/mod_authnz_ldap.html#authldapurl).
They combine the attribute filter and the general filter if both are
specified, but I think they got that a bit wrong. The attribute field
in the URL is actually not supposed to be a filter but a return field,
which is also the confusion mentioned above.
The PAM module (https://linux.die.net/man/5/pam_ldap) also has a way to
specify a search attribute and a general filter and combines them.
Neither of these allows substituting the user name into the search filter.
I think there would be a case to be made to allow the searchattribute
and the searchfilter both be specified. One typical use would be to use
the first one to locate the user and the second one to "filter" out
certain things, which I think is the approach the PAM and Apache modules
take. This wouldn't work well, however, if you use the %u placeholder,
because then you'd have to explicitly unset ldapsearchattribute, which
would be annoying. So maybe not.
Please check my patch. I think it's ready to go then.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
