|From:||Daniel Gustafsson <daniel(at)yesql(dot)se>|
|To:||Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>|
|Cc:||Pg Hackers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: DNS SRV support for LDAP authentication|
|Views:||Raw Message | Whole Thread | Download mbox|
> On 25 Sep 2018, at 04:09, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> Some people like to use DNS SRV records to advertise LDAP servers on
> their network. Microsoft Active Directory is usually (always?) set up
> that way. Here is a patch to allow our LDAP auth module to support
> that kind of discovery. It copies the convention of the OpenLDAP
> command line tools: if you give it a URL that has no hostname, it'll
> try to extract a domain name from the bind DN, and then ask your DNS
> server for a SRV record for LDAP-over-TCP at that domain. The
> OpenLDAP version of libldap.so exports the magic to do that, so the
> patch is very small (but the infrastructure set-up to test it is a bit
> of a schlep, see below). I'll add this to the next Commitfest.
Sounds like a reasonable feature.
> Testing instructions for (paths and commands given for FreeBSD, adjust
> as appropriate):
Trying this quickly on macOS while at a conference didn’t yield much success,
will do another attempt when I’m on a more reliable connection.
> This is a first draft. Not tested much yet. I wonder if
> HAVE_LDAP_INITIALIZE is a reasonable way to detact OpenLDAP. The
> documentation was written in about 7 seconds so probably needs work.
> There is probably a Windowsy way to do this too but I didn't look into
Reading through the patch, and related OpenLDAP code, this seems like a good
approach. A few small comments:
+ If <productname>PostgreSQL</productname> was compiled with OpenLDAP as
Should OpenLDAP be wrapped in <productname> tags as well? If so, there is
another “bare” instance in client-auth.sgml which perhaps can be wrapped into
this patch while at it.
+ (errmsg("could not look up a hostlist for %s",
Should this be \”%s\”?
+ new_uris = psprintf("%s%s%s://%s:%d",
While this construction isn't introduced in this patch, would it not make sense
to convert uris to StringInfo instead to improve readability?
+ /* Step over this hostname and any spaces. */
Nitpicking on a moved hunk, but single-line comments shouldn’t end in a period
|Next Message||Alvaro Herrera||2019-02-01 11:52:49||Re: [HACKERS] [PATCH] Generic type subscripting|
|Previous Message||Alvaro Herrera||2019-02-01 11:45:53||Re: [HACKERS] [PATCH] Generic type subscripting|