Andrey, Thank you for the review.
>I like "ldapservice" approach more. I don't like breaking changes, even for >very small number of users. After thinking about it more it would be pretty easy to make this non breaking for users. The attached patch `pgservice-0002-Allow-LDAP-lookup-from-pgservice-connection-paramete.patch` implements this. Basically if the ldap connection parameter happens to already exist in pg_service, no LDAP lookup is performed, the values are instead taken from the pg_service.conf file. I'm fine with either implementation direction that is the most simple though. Also attaching the patch `ldapserviceurl-0005-Add-ldapservice-connection-parameter.patch` which addresses some issues you found with the previous patch. - renames parameter from ldapservice to ldapserviceurl - Adds a comment above the ldapserviceurl call explaining the reason for ignoring any error info besides zero/non-zero - Changes dispsize to 64. In reality it maybe should be higher but I was not comfortable changing it to something like 1024 because it was an order of magnitude larger than anything else. Just set it to the current max of 64. - Also moved the parameter definition and added comments about how it is defined even on non LDAP builds. This coincides with similar comments for SSL/GSS specific parameters, though there are no such comments left for oauth. Let me know if there is anything else you see missing. Thanks again, Andrew Jackson
From 0020d2027b581e6bdfc5b78785a9b9171b7f7ef2 Mon Sep 17 00:00:00 2001 From: Andrew Jackson <[email protected]> Date: Sat, 28 Mar 2026 23:29:48 -0500 Subject: [PATCH] Allow LDAP lookup from pgservice connection parameter Currently there exists, only in pg_service.conf, the ability to look up connection parameters from a centralized LDAP server. This patch expands the usability of this be allowing it to be specified directly in a connection string instead of only in a pg_service.conf file. This patch adds a check in parseServiceInfo that checks if pgservice is an LDAP scheme address and if so attempts to connect to that LDAP server and grab connection parameters from there. This is a breaking change in that it is possible that people previously named their pgservice after an LDAP address. v0002 patch This patch Removes all backwards compatibility issues. Now if the LDAP URL was specified in pg_service.conf then the values from pg_service.conf will be used and the LDAP lookup will not be performed. Cool --- doc/src/sgml/libpq.sgml | 6 +++ src/interfaces/libpq/fe-connect.c | 5 ++ .../t/003_ldap_connection_param_lookup.pl | 48 +++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 6db823808fc..88a5db2388d 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -2333,6 +2333,12 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname name in <filename>pg_service.conf</filename> that holds additional connection parameters. This allows applications to specify only a service name so connection parameters can be centrally maintained. See <xref linkend="libpq-pgservice"/>. + + You can also specify an LDAP URL here which will look up connection parameters from an + LDAP server. See <xref linkend="libpq-ldap"/>. Please note that if the LDAP URL + specified here exists as a service name in <filename>pg_service.conf</filename>, then + the <filename>pg_service.conf</filename> values will be used instead and no LDAP lookup + will be performed. </para> </listitem> </varlistentry> diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index db9b4c8edbf..b12b9bceb12 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -6048,6 +6048,11 @@ next_file: last_file: if (!group_found) { +#ifdef USE_LDAP + if (strncmp(service, "ldap", 4) == 0) + /* if error ignore ldapServiceLookup return value, just return 3 */ + return ldapServiceLookup(service, options, errorMessage) ? 3 : 0; +#endif libpq_append_error(errorMessage, "definition of service \"%s\" not found", service); return 3; } diff --git a/src/test/ldap/t/003_ldap_connection_param_lookup.pl b/src/test/ldap/t/003_ldap_connection_param_lookup.pl index 359fc7a998a..976c196df8c 100644 --- a/src/test/ldap/t/003_ldap_connection_param_lookup.pl +++ b/src/test/ldap/t/003_ldap_connection_param_lookup.pl @@ -62,6 +62,20 @@ description:port=} . $node->port . qq{ $ldap->ldapadd_file($ldif_valid); +my $ldif_valid_invalid = "$td/connection_params_ldif_valid_invalid.ldif"; +append_to_file( + $ldif_valid_invalid , qq{ +version:1 +dn:cn=mydatabasefoundinpgservice,dc=example,dc=net +changetype:add +objectclass:top +objectclass:device +cn:mydatabasefoundinpgservice +description:host=} . $node->host . qq{ +description:port=} . $node->port . qq{ +}); +$ldap->ldapadd_file($ldif_valid_invalid); + my ($ldap_server, $ldap_port, $ldaps_port, $ldap_url, $ldaps_url, $ldap_basedn, $ldap_rootdn ) = $ldap->prop(qw(server port s_port url s_url basedn rootdn)); @@ -80,6 +94,9 @@ append_to_file( $srvfile_valid, qq{ [my_srv] ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase) + +[ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabasefoundinpgservice)] +port=1234 }); # File defined with no contents, used as default value for @@ -196,6 +213,37 @@ local $ENV{PGSERVICEFILE} = "$srvfile_empty"; expected_stdout => qr/definition of service "undefined-service" not found/); + delete $ENV{PGSERVICE}; + + $dummy_node->connect_ok( + "service=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase)", + 'connection with correct "service" string populated with LDAP address', + sql => "SELECT 'connect2_4'", + expected_stdout => qr/connect2_4/); + + $dummy_node->connect_ok( + "postgres://?service=ldap%3A%2F%2Flocalhost%3A$ldap_port%2Fdc%3Dexample%2Cdc%3Dnet%3Fdescription%3Fone%3F%28cn%3Dmydatabase%29", + 'connection with correct "ldapservice" string populated with LDAP address', + sql => "SELECT 'connect2_5'", + expected_stdout => qr/connect2_5/); + + local $ENV{PGSERVICE} = "ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase)"; + $dummy_node->connect_ok( + "", + 'connection with correct "service" provided by env var populated with LDAP address', + sql => "SELECT 'connect2_6'", + expected_stdout => qr/connect2_6/); + delete $ENV{PGLDAPSERVICE}; + + # Below test should fail because the service value is defined as a literal service in pg_service.conf. + # The pg_service.conf entry should have an incorrect port and LDAP should not be looked up after reading + # the incorrect port + $dummy_node->connect_fails( + 'service=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabasefoundinpgservice)"', + 'connection using cn=mydatabasefoundinpgservice', + expected_stdout => + qr/definition of service "undefined-service" not found/); + # Remove default pg_service.conf. unlink($srvfile_default); } -- 2.51.2
From b56968599037f402b0b4c040483bd67e61bf2429 Mon Sep 17 00:00:00 2001 From: Andrew Jackson <[email protected]> Date: Sat, 28 Mar 2026 23:29:48 -0500 Subject: [PATCH] Add ldapservice connection parameter Currently there exists, only in pg_service.conf, the ability to look up connection parameters from a centralized LDAP server. This patch expands the usability of this be allowing it to be specified directly in a connection string instead of only in a pg_service.conf file. This adds the PGLDAPSERVICE env var that provides an envvar interface to this functionality. Also the functionality has been moved to conninfo_add_defaults after review. Also 2 tests have been added. One to validate the env var functionality and one to validate that this parameter is ignored when used in pg_service.conf files. -- 0005 patch changes This patch changes the relevant naming from ldapservice to ldapserviceurl. I also add a comment above the new ldapservicelookup call that explains the reasoning for ignoring all non zero return codes. Also I increase the value of dispsize for ldapserviceurl from 20 to 64. It could be reasonable to increase it past 64, was originally thinking about bumping it to 1024 but I am not sure what these values are used for in the codebase and thought it was safer to just bump it to the currently existing max for any parameter. Moves the placement of ldapserviceurl to below the GSS options and adds a comment explaining that the parameter is exposed even in non LDAP builds. This was done to reflect similar comments in GSS/SSL, though this comment does not exist for the new OAUTH parameters so this may not have been a good choice. Also Add filename tags to filename reference in docs --- doc/src/sgml/libpq.sgml | 23 +++++++++++++++ src/interfaces/libpq/fe-connect.c | 24 +++++++++++++++ src/interfaces/libpq/libpq-int.h | 1 + .../t/003_ldap_connection_param_lookup.pl | 29 +++++++++++++++++++ 4 files changed, 77 insertions(+) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 6db823808fc..0e4b7f81551 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -2350,6 +2350,19 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname </listitem> </varlistentry> + <varlistentry id="libpq-connect-ldapserviceurl" xreflabel="ldapserviceurl"> + <term><literal>ldapserviceurl</literal></term> + <listitem> + <para> + This option specifies an LDAP query that can be used to reference connection parameters + stored on an LDAP server. Any connection parameter that is looked up in this way is + overridden by explicitly named connection parameters. This parameter is ignored when used + in a <filename>pg_service.conf</filename> file. This functionality is described in more + detail in <xref linkend="libpq-ldap"/>. + </para> + </listitem> + </varlistentry> + <varlistentry id="libpq-connect-target-session-attrs" xreflabel="target_session_attrs"> <term><literal>target_session_attrs</literal></term> <listitem> @@ -9170,6 +9183,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) </para> </listitem> + <listitem> + <para> + <indexterm> + <primary><envar>PGLDAPSERVICEURL</envar></primary> + </indexterm> + <envar>PGLDAPSERVICEURL</envar> behaves the same as the + <xref linkend="libpq-connect-ldapserviceurl"/> connection parameter. + </para> + </listitem> + <listitem> <para> <indexterm> diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index db9b4c8edbf..d076bfe7701 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -376,6 +376,15 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "GSS-delegation", "", 1, offsetof(struct pg_conn, gssdelegation)}, + /* + * As with SSL and GSS options, ldapserviceurl is exposed even in builds + * that do not have support + */ + {"ldapserviceurl", "PGLDAPSERVICEURL", NULL, NULL, + "Database-LDAP-Service", "", 64, + offsetof(struct pg_conn, pgldapserviceurl)}, + + {"replication", NULL, NULL, NULL, "Replication", "D", 5, offsetof(struct pg_conn, replication)}, @@ -6726,6 +6735,21 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage) PQconninfoOption *sslmode_default = NULL, *sslrootcert = NULL; char *tmp; +#ifdef USE_LDAP + const char *ldapserviceurl = conninfo_getval(options, "ldapserviceurl"); + + if (ldapserviceurl == NULL) + ldapserviceurl = getenv("PGLDAPSERVICEURL"); + + if (ldapserviceurl != NULL) + + /* + * ldapServiceLookup has 4 potential return values. We only care here + * if it succeeded, if it failed we dont care why, return failure. + */ + if (ldapServiceLookup(ldapserviceurl, options, errorMessage) != 0) + return false; +#endif /* * If there's a service spec, use it to obtain any not-explicitly-given diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index bd7eb59f5f8..f8b10635d41 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -392,6 +392,7 @@ struct pg_conn char *pgservice; /* Postgres service, if any */ char *pgservicefile; /* path to a service file containing * service(s) */ + char *pgldapserviceurl; /* Postgres LDAP service URL, if any */ char *pguser; /* Postgres username and password, if any */ char *pgpass; char *pgpassfile; /* path to a file containing password(s) */ diff --git a/src/test/ldap/t/003_ldap_connection_param_lookup.pl b/src/test/ldap/t/003_ldap_connection_param_lookup.pl index 359fc7a998a..f1c5cc85eb6 100644 --- a/src/test/ldap/t/003_ldap_connection_param_lookup.pl +++ b/src/test/ldap/t/003_ldap_connection_param_lookup.pl @@ -80,6 +80,9 @@ append_to_file( $srvfile_valid, qq{ [my_srv] ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase) + +[my_srv_2] +ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase) }); # File defined with no contents, used as default value for @@ -196,6 +199,32 @@ local $ENV{PGSERVICEFILE} = "$srvfile_empty"; expected_stdout => qr/definition of service "undefined-service" not found/); + delete $ENV{PGSERVICE}; + + $dummy_node->connect_ok( + "ldapserviceurl=ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase)", + 'connection with correct "ldapservice" string', + sql => "SELECT 'connect2_4'", + expected_stdout => qr/connect2_4/); + + $dummy_node->connect_ok( + "postgres://?ldapserviceurl=ldap%3A%2F%2Flocalhost%3A$ldap_port%2Fdc%3Dexample%2Cdc%3Dnet%3Fdescription%3Fone%3F%28cn%3Dmydatabase%29", + 'connection with correct "ldapserviceurl"', + sql => "SELECT 'connect2_5'", + expected_stdout => qr/connect2_5/); + + local $ENV{PGLDAPSERVICEURL} = "ldap://localhost:$ldap_port/dc=example,dc=net?description?one?(cn=mydatabase)"; + $dummy_node->connect_ok( + "", + 'connection with correct "ldapserviceurl" provided by env var', + sql => "SELECT 'connect2_6'", + expected_stdout => qr/connect2_6/); + delete $ENV{PGLDAPSERVICEURL}; + + $dummy_node->connect_fails( + '', + 'connection fails with ldapserviceurl specified in pg_service.conf file'); + # Remove default pg_service.conf. unlink($srvfile_default); } -- 2.51.2
