public inbox for [email protected]  
help / color / mirror / Atom feed
From: Jacob Champion <[email protected]>
To: Jonathan Gonzalez V. <[email protected]>
Cc: Zsolt Parragi <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Chao Li <[email protected]>
Subject: Re: [oauth] Stabilize the libpq-oauth ABI (and allow alternative implementations?)
Date: Fri, 13 Mar 2026 10:59:23 -0700
Message-ID: <CAOYmi+=cd0SdEm4Pw3RE4eD=rbXex9Rkc3DsNiaDAqQti9taQQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAOYmi+mrGg+n_X2MOLgeWcj3v_M00gR8uz_D7mM8z=dX1JYVbg@mail.gmail.com>
	<[email protected]>
	<CAOYmi+mwzS4xfgomL1Lte+W1hRk+zahhZLnRMDJ8nYxoHUWt_w@mail.gmail.com>
	<CAOYmi+mEU_q9sr1PMmE-4rLwFN=OjyndDwFZvpsMU3RNJLrM9g@mail.gmail.com>
	<CAN4CZFPwdH_GMrappThju=t4m5n95Dse6ecTNwN=h4GpZLOMng@mail.gmail.com>
	<CAOYmi+m-hphbx=CRfGSdcrHi-jL3OrZGJMTAPOYp=OK253hBTA@mail.gmail.com>
	<CAOYmi+nnYr_Oe=KH2ZW6Ld88eZWv5P-s5ZtWOEh2KsPtMqOzbg@mail.gmail.com>
	<CAOYmi+nCg5upBVOo_UCSjMfO=YMkZXcSEsgaADKXqerr5wahZQ@mail.gmail.com>
	<CAOYmi+nRmt+K+ZVNztKFv=LuasDCCSsXLihZB6BEkVeroW8EvA@mail.gmail.com>
	<CAN4CZFOMv=0Y1PZ8uw3xgJmEqt4D1dr1-yOq9jjZYTARZp7H9Q@mail.gmail.com>
	<CAOYmi+mLaohk3FLbH9fKGmzN7yFGHs2Zefdyp31wbL1130puiA@mail.gmail.com>
	<CAOYmi+=Pr7AAdkcKXyLw3ycxcrjGKsOV2CTYVV2PKYQw9ecG0Q@mail.gmail.com>
	<[email protected]>

On Fri, Mar 13, 2026 at 12:24 AM Jonathan Gonzalez V.
<[email protected]> wrote:
> While rebasing this patch[1] I notice that the test where wailing, that
> was due to the following missing dependency in the test, small patch
> here:

Fixed in v8, thanks.

v7-0001 and -0002 are committed, plus the removal of some additional
typedefs, plus a fix for a silly bug in the Makefile that indri
caught.

--Jacob

1:  f5b861f7efd < -:  ----------- libpq-oauth: Use the PGoauthBearerRequestV2 API
2:  f59d6431b04 < -:  ----------- libpq-oauth: Never link against libpq's encoding functions
3:  75ac7ddc120 ! 1:  16fa033f184 libpq: Poison the v2 part of a v1 Bearer request
    @@ Commit message
         The struct is unpoisoned after the call, so we can switch back to the v2
         internal implementation when necessary.
     
    +    Reviewed-by: Zsolt Parragi <[email protected]>
         Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
     
      ## src/interfaces/libpq/fe-auth-oauth.h ##
    @@ src/interfaces/libpq/fe-auth-oauth.c: oauth_unsafe_debugging_enabled(void)
     +static void
     +poison_req_v2(PGoauthBearerRequestV2 *request, bool poison)
     +{
    ++#ifdef USE_VALGRIND
     +	void	   *const base = (char *) request + sizeof(request->v1);
     +	const size_t len = sizeof(*request) - sizeof(request->v1);
    ++#endif
     +
     +	if (poison)
     +	{
4:  7a43398c90a ! 2:  6f886914c83 libpq: Allow developers to reimplement libpq-oauth
    @@ Commit message
         provide feedback for PG20, by telling the runtime linker to find a
         different libpq-oauth. It remains undocumented for end users.
     
    +    Reviewed-by: Zsolt Parragi <[email protected]>
         Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
     
      ## src/interfaces/libpq/fe-auth-oauth.h ##
5:  b3daf3140f8 ! 3:  651d9bdcbc3 WIP: Introduce third-party OAuth flow plugins?
    @@ Commit message
         TODO: how hard would it be to support Windows here?
     
         Reviewed-by: Zsolt Parragi <[email protected]>
    +    Reviewed-by: Jonathan Gonzalez V. <[email protected]>
         Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
     
      ## src/interfaces/libpq/meson.build ##
    @@ src/test/modules/oauth_validator/meson.build: tests += {
            'cert_dir': meson.project_source_root() / 'src/test/ssl/ssl',
     +      'flow_module_path': oauth_flow.full_path(),
          },
    -     'deps': [oauth_hook_client],
    +-    'deps': [oauth_hook_client],
    ++    'deps': [oauth_hook_client, oauth_flow],
        },
    + }
     
      ## src/test/modules/oauth_validator/Makefile ##
     @@ src/test/modules/oauth_validator/Makefile: PGFILEDESC = "validator - test OAuth validator module"


Attachments:

  [text/plain] since-v7.diff.txt (2.5K, 2-since-v7.diff.txt)
  download | inline:
1:  f5b861f7efd < -:  ----------- libpq-oauth: Use the PGoauthBearerRequestV2 API
2:  f59d6431b04 < -:  ----------- libpq-oauth: Never link against libpq's encoding functions
3:  75ac7ddc120 ! 1:  16fa033f184 libpq: Poison the v2 part of a v1 Bearer request
    @@ Commit message
         The struct is unpoisoned after the call, so we can switch back to the v2
         internal implementation when necessary.
     
    +    Reviewed-by: Zsolt Parragi <[email protected]>
         Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
     
      ## src/interfaces/libpq/fe-auth-oauth.h ##
    @@ src/interfaces/libpq/fe-auth-oauth.c: oauth_unsafe_debugging_enabled(void)
     +static void
     +poison_req_v2(PGoauthBearerRequestV2 *request, bool poison)
     +{
    ++#ifdef USE_VALGRIND
     +	void	   *const base = (char *) request + sizeof(request->v1);
     +	const size_t len = sizeof(*request) - sizeof(request->v1);
    ++#endif
     +
     +	if (poison)
     +	{
4:  7a43398c90a ! 2:  6f886914c83 libpq: Allow developers to reimplement libpq-oauth
    @@ Commit message
         provide feedback for PG20, by telling the runtime linker to find a
         different libpq-oauth. It remains undocumented for end users.
     
    +    Reviewed-by: Zsolt Parragi <[email protected]>
         Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
     
      ## src/interfaces/libpq/fe-auth-oauth.h ##
5:  b3daf3140f8 ! 3:  651d9bdcbc3 WIP: Introduce third-party OAuth flow plugins?
    @@ Commit message
         TODO: how hard would it be to support Windows here?
     
         Reviewed-by: Zsolt Parragi <[email protected]>
    +    Reviewed-by: Jonathan Gonzalez V. <[email protected]>
         Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
     
      ## src/interfaces/libpq/meson.build ##
    @@ src/test/modules/oauth_validator/meson.build: tests += {
            'cert_dir': meson.project_source_root() / 'src/test/ssl/ssl',
     +      'flow_module_path': oauth_flow.full_path(),
          },
    -     'deps': [oauth_hook_client],
    +-    'deps': [oauth_hook_client],
    ++    'deps': [oauth_hook_client, oauth_flow],
        },
    + }
     
      ## src/test/modules/oauth_validator/Makefile ##
     @@ src/test/modules/oauth_validator/Makefile: PGFILEDESC = "validator - test OAuth validator module"

  [application/octet-stream] v8-0001-libpq-Poison-the-v2-part-of-a-v1-Bearer-request.patch (7.1K, 3-v8-0001-libpq-Poison-the-v2-part-of-a-v1-Bearer-request.patch)
  download | inline diff:
From 16fa033f1845e34dc2e96f4ca522bd30b6c68b6f Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Tue, 3 Mar 2026 09:45:37 -0800
Subject: [PATCH v8 1/3] libpq: Poison the v2 part of a v1 Bearer request

The new PGoauthBearerRequestV2 API (which has similarities to the
"subclass" pointer architecture in use by the backend, for Nodes)
carries the risk of a developer ignoring the type of hook in use and
just casting directly to the V2 struct. This will appear to work fine in
19, but crash (or worse) when speaking to libpq 18.

However, we're in a unique position to catch this problem, because we
have tight control over the struct. Add poisoning code to the v1 path
which does the following:

- masks the v2 request->issuer pointer, to hopefully point at nonsense
  memory
- abort()s if the v2 request->error is assigned by the hook
- attempts to cover both with VALGRIND_MAKE_MEM_NOACCESS for the
  duration of the callback (a potential AddressSanitizer implementation
  is left for future work)

The struct is unpoisoned after the call, so we can switch back to the v2
internal implementation when necessary.

Reviewed-by: Zsolt Parragi <[email protected]>
Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
---
 src/interfaces/libpq/fe-auth-oauth.h |   1 +
 src/interfaces/libpq/fe-auth-oauth.c | 135 +++++++++++++++++++++++++--
 2 files changed, 127 insertions(+), 9 deletions(-)

diff --git a/src/interfaces/libpq/fe-auth-oauth.h b/src/interfaces/libpq/fe-auth-oauth.h
index 511284614f7..2b22d23ffde 100644
--- a/src/interfaces/libpq/fe-auth-oauth.h
+++ b/src/interfaces/libpq/fe-auth-oauth.h
@@ -34,6 +34,7 @@ typedef struct
 	PGconn	   *conn;
 	void	   *async_ctx;
 
+	bool		v1;
 	bool		builtin;
 	void	   *builtin_flow;
 } fe_oauth_state;
diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c
index f93184f04db..c59b4ce6d30 100644
--- a/src/interfaces/libpq/fe-auth-oauth.c
+++ b/src/interfaces/libpq/fe-auth-oauth.c
@@ -27,6 +27,11 @@
 #include "fe-auth-oauth.h"
 #include "mb/pg_wchar.h"
 #include "pg_config_paths.h"
+#include "utils/memdebug.h"
+
+static int	do_async(fe_oauth_state *state, PGoauthBearerRequestV2 *request);
+static void do_cleanup(fe_oauth_state *state, PGoauthBearerRequestV2 *request);
+static void poison_req_v2(PGoauthBearerRequestV2 *request, bool poison);
 
 /* The exported OAuth callback mechanism. */
 static void *oauth_init(PGconn *conn, const char *password,
@@ -741,9 +746,7 @@ run_oauth_flow(PGconn *conn)
 		return PGRES_POLLING_FAILED;
 	}
 
-	status = request->v1.async(conn,
-							   (PGoauthBearerRequest *) request,
-							   &conn->altsock);
+	status = do_async(state, request);
 
 	if (status == PGRES_POLLING_FAILED)
 	{
@@ -804,8 +807,7 @@ cleanup_oauth_flow(PGconn *conn)
 
 	Assert(request);
 
-	if (request->v1.cleanup)
-		request->v1.cleanup(conn, (PGoauthBearerRequest *) request);
+	do_cleanup(state, request);
 	conn->altsock = PGINVALID_SOCKET;
 
 	free(request);
@@ -1019,7 +1021,14 @@ setup_token_request(PGconn *conn, fe_oauth_state *state)
 	 */
 	res = PQauthDataHook(PQAUTHDATA_OAUTH_BEARER_TOKEN_V2, conn, &request);
 	if (res == 0)
+	{
+		poison_req_v2(&request, true);
+
 		res = PQauthDataHook(PQAUTHDATA_OAUTH_BEARER_TOKEN, conn, &request);
+		state->v1 = (res != 0);
+
+		poison_req_v2(&request, false);
+	}
 	if (res == 0)
 	{
 		state->builtin = true;
@@ -1045,8 +1054,7 @@ setup_token_request(PGconn *conn, fe_oauth_state *state)
 			}
 
 			/* short-circuit */
-			if (request.v1.cleanup)
-				request.v1.cleanup(conn, (PGoauthBearerRequest *) &request);
+			do_cleanup(state, &request);
 			return true;
 		}
 
@@ -1076,8 +1084,7 @@ setup_token_request(PGconn *conn, fe_oauth_state *state)
 		libpq_append_conn_error(conn, "no OAuth flows are available (try installing the libpq-oauth package)");
 
 fail:
-	if (request.v1.cleanup)
-		request.v1.cleanup(conn, (PGoauthBearerRequest *) &request);
+	do_cleanup(state, &request);
 	return false;
 }
 
@@ -1428,3 +1435,113 @@ oauth_unsafe_debugging_enabled(void)
 
 	return (env && strcmp(env, "UNSAFE") == 0);
 }
+
+/*
+ * Hook v1 Poisoning
+ *
+ * Try to catch misuses of the v1 PQAUTHDATA_OAUTH_BEARER_TOKEN hook and its
+ * callbacks, which are not allowed to downcast their request argument to
+ * PGoauthBearerRequestV2. (Such clients may crash or worse when speaking to
+ * libpq 18.)
+ *
+ * This attempts to use Valgrind hooks, if present, to mark the extra members as
+ * inaccessible. For uninstrumented builds, it also munges request->issuer to
+ * try to crash clients that perform string operations, and it aborts if
+ * request->error is set.
+ */
+
+#define MASK_BITS ((uintptr_t) 0x55aa55aa55aa55aa)
+#define POISON_MASK(ptr) ((void *) (((uintptr_t) ptr) ^ MASK_BITS))
+
+/*
+ * Workhorse for v2 request poisoning. This must be called exactly twice: once
+ * to poison, once to unpoison.
+ *
+ * NB: Unpoisoning must restore the request to its original state, because we
+ * might still switch back to a v2 implementation internally. Don't do anything
+ * destructive during the poison operation.
+ */
+static void
+poison_req_v2(PGoauthBearerRequestV2 *request, bool poison)
+{
+#ifdef USE_VALGRIND
+	void	   *const base = (char *) request + sizeof(request->v1);
+	const size_t len = sizeof(*request) - sizeof(request->v1);
+#endif
+
+	if (poison)
+	{
+		/* Poison request->issuer with a mask to help uninstrumented builds. */
+		request->issuer = POISON_MASK(request->issuer);
+
+		VALGRIND_MAKE_MEM_NOACCESS(base, len);
+	}
+	else
+	{
+		/*
+		 * XXX Using DEFINED here is technically too lax; we might catch
+		 * struct padding in the blast radius. But since this API has to
+		 * poison stack addresses, and Valgrind can't track/manage undefined
+		 * stack regions, we can't be any stricter without tracking the
+		 * original state of the memory.
+		 */
+		VALGRIND_MAKE_MEM_DEFINED(base, len);
+
+		/* Undo our mask. */
+		request->issuer = POISON_MASK(request->issuer);
+
+		/*
+		 * For uninstrumented builds, make sure request->error wasn't touched.
+		 */
+		if (request->error)
+		{
+			fprintf(stderr,
+					"abort! out-of-bounds write to PGoauthBearerRequest by PQAUTHDATA_OAUTH_BEARER_TOKEN hook\n");
+			abort();
+		}
+	}
+}
+
+/*
+ * Wrapper around PGoauthBearerRequest.async() which applies poison during the
+ * callback when necessary.
+ */
+static int
+do_async(fe_oauth_state *state, PGoauthBearerRequestV2 *request)
+{
+	PGconn	   *conn = state->conn;
+	int			ret;
+
+	Assert(request->v1.async);
+
+	if (state->v1)
+		poison_req_v2(request, true);
+
+	ret = request->v1.async(conn,
+							(PGoauthBearerRequest *) request,
+							&conn->altsock);
+
+	if (state->v1)
+		poison_req_v2(request, false);
+
+	return ret;
+}
+
+/*
+ * Similar wrapper for the optional PGoauthBearerRequest.cleanup() callback.
+ * Does nothing if one is not defined.
+ */
+static void
+do_cleanup(fe_oauth_state *state, PGoauthBearerRequestV2 *request)
+{
+	if (!request->v1.cleanup)
+		return;
+
+	if (state->v1)
+		poison_req_v2(request, true);
+
+	request->v1.cleanup(state->conn, (PGoauthBearerRequest *) request);
+
+	if (state->v1)
+		poison_req_v2(request, false);
+}
-- 
2.34.1



  [application/octet-stream] v8-0002-libpq-Allow-developers-to-reimplement-libpq-oauth.patch (4.8K, 4-v8-0002-libpq-Allow-developers-to-reimplement-libpq-oauth.patch)
  download | inline diff:
From 6f886914c836c38e0fedf4fc0dcc3a3be5c7369b Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Fri, 6 Mar 2026 14:18:49 -0800
Subject: [PATCH v8 2/3] libpq: Allow developers to reimplement libpq-oauth

For PG19, since we won't have the ability to officially switch out flow
plugins, relax the flow-loading code to not require the internal init
function. Modules that don't have one will be treated as custom user
flows in error messages.

This will let bleeding-edge developers more easily test out the API and
provide feedback for PG20, by telling the runtime linker to find a
different libpq-oauth. It remains undocumented for end users.

Reviewed-by: Zsolt Parragi <[email protected]>
Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
---
 src/interfaces/libpq/fe-auth-oauth.h |  2 +-
 src/interfaces/libpq/fe-auth-oauth.c | 71 ++++++++++++++++------------
 2 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/src/interfaces/libpq/fe-auth-oauth.h b/src/interfaces/libpq/fe-auth-oauth.h
index 2b22d23ffde..872f5df551a 100644
--- a/src/interfaces/libpq/fe-auth-oauth.h
+++ b/src/interfaces/libpq/fe-auth-oauth.h
@@ -36,7 +36,7 @@ typedef struct
 
 	bool		v1;
 	bool		builtin;
-	void	   *builtin_flow;
+	void	   *flow_module;
 } fe_oauth_state;
 
 extern void pqClearOAuthToken(PGconn *conn);
diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c
index c59b4ce6d30..8dd0392b3d0 100644
--- a/src/interfaces/libpq/fe-auth-oauth.c
+++ b/src/interfaces/libpq/fe-auth-oauth.c
@@ -889,8 +889,8 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re
 		"libpq-oauth" DLSUFFIX;
 #endif
 
-	state->builtin_flow = dlopen(module_name, RTLD_NOW | RTLD_LOCAL);
-	if (!state->builtin_flow)
+	state->flow_module = dlopen(module_name, RTLD_NOW | RTLD_LOCAL);
+	if (!state->flow_module)
 	{
 		/*
 		 * For end users, this probably isn't an error condition, it just
@@ -905,8 +905,16 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re
 		return 0;
 	}
 
-	if ((init = dlsym(state->builtin_flow, "libpq_oauth_init")) == NULL
-		|| (start_flow = dlsym(state->builtin_flow, "pg_start_oauthbearer")) == NULL)
+	/*
+	 * Our libpq-oauth.so provides a special initialization function for libpq
+	 * integration. If we don't find this, assume that a custom module is in
+	 * use instead.
+	 */
+	init = dlsym(state->flow_module, "libpq_oauth_init");
+	if (!init)
+		state->builtin = false; /* adjust our error messages */
+
+	if ((start_flow = dlsym(state->flow_module, "pg_start_oauthbearer")) == NULL)
 	{
 		/*
 		 * This is more of an error condition than the one above, but the
@@ -916,8 +924,8 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re
 		if (oauth_unsafe_debugging_enabled())
 			fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror());
 
-		dlclose(state->builtin_flow);
-		state->builtin_flow = NULL;
+		dlclose(state->flow_module);
+		state->flow_module = NULL;
 
 		request->error = libpq_gettext("could not find entry point for libpq-oauth");
 		return -1;
@@ -928,39 +936,42 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re
 	 * permanently.
 	 */
 
-	/*
-	 * We need to inject necessary function pointers into the module. This
-	 * only needs to be done once -- even if the pointers are constant,
-	 * assigning them while another thread is executing the flows feels like
-	 * tempting fate.
-	 */
-	if ((lockerr = pthread_mutex_lock(&init_mutex)) != 0)
+	if (init)
 	{
-		/* Should not happen... but don't continue if it does. */
-		Assert(false);
+		/*
+		 * We need to inject necessary function pointers into the module. This
+		 * only needs to be done once -- even if the pointers are constant,
+		 * assigning them while another thread is executing the flows feels
+		 * like tempting fate.
+		 */
+		if ((lockerr = pthread_mutex_lock(&init_mutex)) != 0)
+		{
+			/* Should not happen... but don't continue if it does. */
+			Assert(false);
 
-		appendPQExpBuffer(&conn->errorMessage,
-						  "use_builtin_flow: failed to lock mutex (%d)\n",
-						  lockerr);
+			appendPQExpBuffer(&conn->errorMessage,
+							  "use_builtin_flow: failed to lock mutex (%d)\n",
+							  lockerr);
 
-		request->error = "";	/* satisfy report_flow_error() */
-		return -1;
-	}
+			request->error = "";	/* satisfy report_flow_error() */
+			return -1;
+		}
 
-	if (!initialized)
-	{
-		init(
+		if (!initialized)
+		{
+			init(
 #ifdef ENABLE_NLS
-			 libpq_gettext
+				 libpq_gettext
 #else
-			 NULL
+				 NULL
 #endif
-			);
+				);
 
-		initialized = true;
-	}
+			initialized = true;
+		}
 
-	pthread_mutex_unlock(&init_mutex);
+		pthread_mutex_unlock(&init_mutex);
+	}
 
 	return (start_flow(conn, request) == 0) ? 1 : -1;
 }
-- 
2.34.1



  [application/octet-stream] v8-0003-WIP-Introduce-third-party-OAuth-flow-plugins.patch (36.2K, 5-v8-0003-WIP-Introduce-third-party-OAuth-flow-plugins.patch)
  download | inline diff:
From 651d9bdcbc3f086bbda9fc1455959d66bfb09403 Mon Sep 17 00:00:00 2001
From: Jacob Champion <[email protected]>
Date: Wed, 3 Dec 2025 15:47:23 -0800
Subject: [PATCH v8 3/3] WIP: Introduce third-party OAuth flow plugins?

[Not intended for PG19.]

This experimental commit promotes the pg_start_oauthbearer API to a
public header (libpq-oauth.h) and adds a PGOAUTHMODULE environment
variable that overrides the load path for the plugin, allowing users to
provide their own. It's probably not a good solution; it provides almost
nothing over LD_LIBRARY_PATH (other than not being LD_LIBRARY_PATH).

This is a relatively small amount of implementation change, but
unfortunately the tests have a large amount of code motion to be able to
share logic between the test executable and plugin. I might need to
split that into multiple squash! commits to make it more easily
reviewable.

TODO: figure out PGDLLEXPORT, which we do not currently provide publicly
TODO: lock down PGOAUTHMODULE as necessary to avoid introducing exciting
      new vulnerabilities
TODO: how hard would it be to support Windows here?

Reviewed-by: Zsolt Parragi <[email protected]>
Reviewed-by: Jonathan Gonzalez V. <[email protected]>
Discussion: https://postgr.es/m/CAOYmi%2BmrGg%2Bn_X2MOLgeWcj3v_M00gR8uz_D7mM8z%3DdX1JYVbg%40mail.gmail.com
---
 src/interfaces/libpq/meson.build              |   1 +
 src/interfaces/libpq/Makefile                 |   2 +
 src/interfaces/libpq-oauth/oauth-curl.h       |  24 --
 src/interfaces/libpq/libpq-oauth.h            |  52 +++
 src/interfaces/libpq-oauth/oauth-curl.c       |   2 +-
 src/interfaces/libpq/fe-auth-oauth.c          |  46 ++-
 src/test/modules/oauth_validator/meson.build  |  17 +-
 src/test/modules/oauth_validator/Makefile     |  12 +-
 .../oauth_validator/oauth_test_common.h       |  26 ++
 src/test/modules/oauth_validator/oauth_flow.c |  72 ++++
 .../oauth_validator/oauth_hook_client.c       | 318 +--------------
 .../oauth_validator/oauth_test_common.c       | 373 ++++++++++++++++++
 .../modules/oauth_validator/t/002_client.pl   |  57 ++-
 13 files changed, 647 insertions(+), 355 deletions(-)
 delete mode 100644 src/interfaces/libpq-oauth/oauth-curl.h
 create mode 100644 src/interfaces/libpq/libpq-oauth.h
 create mode 100644 src/test/modules/oauth_validator/oauth_test_common.h
 create mode 100644 src/test/modules/oauth_validator/oauth_flow.c
 create mode 100644 src/test/modules/oauth_validator/oauth_test_common.c

diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index b0ae72167a1..4eb029cc2a6 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -135,6 +135,7 @@ pkgconfig.generate(
 install_headers(
   'libpq-fe.h',
   'libpq-events.h',
+  'libpq-oauth.h',
 )
 
 install_headers(
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 0963995eed4..a65c36a63fd 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -157,6 +157,7 @@ $(top_builddir)/src/port/pg_config_paths.h:
 install: all installdirs install-lib
 	$(INSTALL_DATA) $(srcdir)/libpq-fe.h '$(DESTDIR)$(includedir)'
 	$(INSTALL_DATA) $(srcdir)/libpq-events.h '$(DESTDIR)$(includedir)'
+	$(INSTALL_DATA) $(srcdir)/libpq-oauth.h '$(DESTDIR)$(includedir)'
 	$(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)'
 	$(INSTALL_DATA) $(srcdir)/fe-auth-sasl.h '$(DESTDIR)$(includedir_internal)'
 	$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)'
@@ -179,6 +180,7 @@ installdirs: installdirs-lib
 uninstall: uninstall-lib
 	rm -f '$(DESTDIR)$(includedir)/libpq-fe.h'
 	rm -f '$(DESTDIR)$(includedir)/libpq-events.h'
+	rm -f '$(DESTDIR)$(includedir)/libpq-oauth.h'
 	rm -f '$(DESTDIR)$(includedir_internal)/libpq-int.h'
 	rm -f '$(DESTDIR)$(includedir_internal)/fe-auth-sasl.h'
 	rm -f '$(DESTDIR)$(includedir_internal)/pqexpbuffer.h'
diff --git a/src/interfaces/libpq-oauth/oauth-curl.h b/src/interfaces/libpq-oauth/oauth-curl.h
deleted file mode 100644
index 1d4dd766217..00000000000
--- a/src/interfaces/libpq-oauth/oauth-curl.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * oauth-curl.h
- *
- *	  Definitions for OAuth Device Authorization module
- *
- * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- * src/interfaces/libpq-oauth/oauth-curl.h
- *
- *-------------------------------------------------------------------------
- */
-
-#ifndef OAUTH_CURL_H
-#define OAUTH_CURL_H
-
-#include "libpq-fe.h"
-
-/* Exported flow callback. */
-extern PGDLLEXPORT int pg_start_oauthbearer(PGconn *conn,
-											PGoauthBearerRequestV2 *request);
-
-#endif							/* OAUTH_CURL_H */
diff --git a/src/interfaces/libpq/libpq-oauth.h b/src/interfaces/libpq/libpq-oauth.h
new file mode 100644
index 00000000000..2a62b330b1c
--- /dev/null
+++ b/src/interfaces/libpq/libpq-oauth.h
@@ -0,0 +1,52 @@
+/*-------------------------------------------------------------------------
+ *
+ * libpq-oauth.h
+ *	  This file contains structs and functions used by custom OAuth plugins.
+ *
+ * Copyright (c) 2025, PostgreSQL Global Development Group
+ *
+ * src/interfaces/libpq/libpq-oauth.h
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#ifndef LIBPQ_OAUTH_H
+#define LIBPQ_OAUTH_H
+
+#include "libpq-fe.h"
+
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
+/* XXX can't rely on c.h, but duplicating this is asking for trouble */
+#ifndef PGDLLEXPORT
+#ifdef _WIN32
+#define PGDLLEXPORT __declspec (dllexport)
+#elif defined(__has_attribute)
+#if __has_attribute(visibility)
+#define PGDLLEXPORT __attribute__((visibility("default")))
+#else
+#define PGDLLEXPORT
+#endif
+#else
+#define PGDLLEXPORT
+#endif
+#endif
+
+/*
+ * V1 API
+ *
+ * Flow plugins must provide an implementation of this callback.
+ *
+ * TODO: provide a magic struct that allows backwards but not forwards compat?
+ */
+extern PGDLLEXPORT int pg_start_oauthbearer(PGconn *conn,
+											PGoauthBearerRequestV2 *request);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif							/* LIBPQ_OAUTH_H */
diff --git a/src/interfaces/libpq-oauth/oauth-curl.c b/src/interfaces/libpq-oauth/oauth-curl.c
index 052ecd32da2..400ca07c297 100644
--- a/src/interfaces/libpq-oauth/oauth-curl.c
+++ b/src/interfaces/libpq-oauth/oauth-curl.c
@@ -29,8 +29,8 @@
 #endif
 
 #include "common/jsonapi.h"
+#include "libpq-oauth.h"
 #include "mb/pg_wchar.h"
-#include "oauth-curl.h"
 
 #ifdef USE_DYNAMIC_OAUTH
 
diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c
index 8dd0392b3d0..b6cb7df9497 100644
--- a/src/interfaces/libpq/fe-auth-oauth.c
+++ b/src/interfaces/libpq/fe-auth-oauth.c
@@ -17,6 +17,8 @@
 
 #ifdef USE_DYNAMIC_OAUTH
 #include <dlfcn.h>
+#else
+#include "libpq-oauth.h"
 #endif
 
 #include "common/base64.h"
@@ -882,33 +884,55 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re
 	 * On the other platforms, load the module using only the basename, to
 	 * rely on the runtime linker's standard search behavior.
 	 */
-	const char *const module_name =
+	const char *module_name =
 #if defined(__darwin__)
 		LIBDIR "/libpq-oauth" DLSUFFIX;
 #else
 		"libpq-oauth" DLSUFFIX;
 #endif
 
+	/*-
+	 * Additionally, the user may override the module path explicitly to be
+	 * able to provide their own module, via PGOAUTHMODULE.
+	 *
+	 * TODO: have to think about _all_ the security ramifications of this. What
+	 * existing protections in LD_LIBRARY_PATH (and/or SIP) are we potentially
+	 * bypassing? Should we check the permissions of the file somehow...?
+	 * TODO: maybe disallow anything not underneath LIBDIR? or PKGLIBDIR?
+	 * Should it have a naming convention?
+	 */
+	const char *env = getenv("PGOAUTHMODULE");
+
+	if (env && env[0])
+		module_name = env;
+	else
+		state->builtin = true;
+
 	state->flow_module = dlopen(module_name, RTLD_NOW | RTLD_LOCAL);
 	if (!state->flow_module)
 	{
 		/*
 		 * For end users, this probably isn't an error condition, it just
 		 * means the flow isn't installed. Developers and package maintainers
-		 * may want to debug this via the PGOAUTHDEBUG envvar, though.
+		 * may want to debug this via the PGOAUTHDEBUG envvar, though, and we
+		 * should be more noisy if users tried to provide a PGOAUTHMODULE.
 		 *
 		 * Note that POSIX dlerror() isn't guaranteed to be threadsafe.
 		 */
 		if (oauth_unsafe_debugging_enabled())
-			fprintf(stderr, "failed dlopen for libpq-oauth: %s\n", dlerror());
+			fprintf(stderr, "failed dlopen for %s: %s\n", module_name, dlerror());
+
+		if (state->builtin)
+			return 0;
 
-		return 0;
+		request->error = libpq_gettext("plugin could not be loaded");
+		return -1;
 	}
 
 	/*
 	 * Our libpq-oauth.so provides a special initialization function for libpq
-	 * integration. If we don't find this, assume that a custom module is in
-	 * use instead.
+	 * integration. It's not a problem if we don't find this; it just means
+	 * that a user-defined module is being used.
 	 */
 	init = dlsym(state->flow_module, "libpq_oauth_init");
 	if (!init)
@@ -922,12 +946,14 @@ use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *re
 		 * threadsafety issue.
 		 */
 		if (oauth_unsafe_debugging_enabled())
-			fprintf(stderr, "failed dlsym for libpq-oauth: %s\n", dlerror());
+			fprintf(stderr, "failed dlsym for %s: %s\n", module_name, dlerror());
 
 		dlclose(state->flow_module);
 		state->flow_module = NULL;
 
-		request->error = libpq_gettext("could not find entry point for libpq-oauth");
+		request->error = state->builtin ?
+			libpq_gettext("could not find entry point for libpq-oauth") :
+			libpq_gettext("could not find entry point for custom plugin");
 		return -1;
 	}
 
@@ -988,6 +1014,7 @@ extern int	pg_start_oauthbearer(PGconn *conn, PGoauthBearerRequestV2 *request);
 static int
 use_builtin_flow(PGconn *conn, fe_oauth_state *state, PGoauthBearerRequestV2 *request)
 {
+	state->builtin = true;
 	return (pg_start_oauthbearer(conn, request) == 0) ? 1 : -1;
 }
 
@@ -1041,10 +1068,7 @@ setup_token_request(PGconn *conn, fe_oauth_state *state)
 		poison_req_v2(&request, false);
 	}
 	if (res == 0)
-	{
-		state->builtin = true;
 		res = use_builtin_flow(conn, state, &request);
-	}
 
 	if (res > 0)
 	{
diff --git a/src/test/modules/oauth_validator/meson.build b/src/test/modules/oauth_validator/meson.build
index 506a9894b8d..bc340c12754 100644
--- a/src/test/modules/oauth_validator/meson.build
+++ b/src/test/modules/oauth_validator/meson.build
@@ -50,6 +50,7 @@ test_install_libs += magic_validator
 
 oauth_hook_client_sources = files(
   'oauth_hook_client.c',
+  'oauth_test_common.c',
 )
 
 if host_system == 'windows'
@@ -67,6 +68,19 @@ oauth_hook_client = executable('oauth_hook_client',
 )
 testprep_targets += oauth_hook_client
 
+oauth_flow = shared_module('oauth_flow',
+  files(
+    'oauth_flow.c',
+    'oauth_test_common.c',
+  ),
+  include_directories: [postgres_inc],
+  dependencies: [frontend_shlib_code, libpq],
+  kwargs: default_lib_args + {
+    'install': false,
+  },
+)
+testprep_targets += oauth_flow
+
 tests += {
   'name': 'oauth_validator',
   'sd': meson.current_source_dir(),
@@ -81,7 +95,8 @@ tests += {
       'with_libcurl': oauth_flow_supported ? 'yes' : 'no',
       'with_python': 'yes',
       'cert_dir': meson.project_source_root() / 'src/test/ssl/ssl',
+      'flow_module_path': oauth_flow.full_path(),
     },
-    'deps': [oauth_hook_client],
+    'deps': [oauth_hook_client, oauth_flow],
   },
 }
diff --git a/src/test/modules/oauth_validator/Makefile b/src/test/modules/oauth_validator/Makefile
index 0b39a88fd9f..48b30aea24f 100644
--- a/src/test/modules/oauth_validator/Makefile
+++ b/src/test/modules/oauth_validator/Makefile
@@ -14,11 +14,13 @@ PGFILEDESC = "validator - test OAuth validator module"
 
 PROGRAM = oauth_hook_client
 PGAPPICON = win32
-OBJS = $(WIN32RES) oauth_hook_client.o
+OBJS = $(WIN32RES) oauth_hook_client.o oauth_test_common.o
 
 PG_CPPFLAGS = -I$(libpq_srcdir)
 PG_LIBS_INTERNAL += $(libpq_pgport)
 
+EXTRA_CLEAN = oauth_flow$(DLSUFFIX) oauth_flow.o
+
 NO_INSTALLCHECK = 1
 
 TAP_TESTS = 1
@@ -33,9 +35,15 @@ top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 
+all: oauth_flow$(DLSUFFIX)
+
+oauth_flow$(DLSUFFIX): oauth_flow.o oauth_test_common.o
+	$(CC) $(CFLAGS) $^ $(LDFLAGS) $(libpq_pgport_shlib) $(LDFLAGS_SL) -shared -o $@
+
 export PYTHON
 export with_libcurl
 export with_python
-export cert_dir=$(top_srcdir)/src/test/ssl/ssl
+export cert_dir := $(top_srcdir)/src/test/ssl/ssl
+export flow_module_path := $(abs_top_builddir)/$(subdir)/oauth_flow$(DLSUFFIX)
 
 endif
diff --git a/src/test/modules/oauth_validator/oauth_test_common.h b/src/test/modules/oauth_validator/oauth_test_common.h
new file mode 100644
index 00000000000..33e72e30440
--- /dev/null
+++ b/src/test/modules/oauth_validator/oauth_test_common.h
@@ -0,0 +1,26 @@
+/*-------------------------------------------------------------------------
+ *
+ * oauth_test_common.h
+ *	  Shared functionality for oauth_hook_client and oauth_flow
+ *
+ * Copyright (c) 2025, PostgreSQL Global Development Group
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#ifndef OAUTH_TEST_COMMON_H
+#define OAUTH_TEST_COMMON_H
+
+/*
+ * Only public headers can be here, since oauth_flow.c is trying to test only
+ * the public API.
+ */
+#include "libpq-fe.h"
+
+extern int	stress_async;		/* for oauth_hook_client */
+
+extern char *oauth_test_parse_argv(int argc, char *argv[], int for_plugin);
+extern int	oauth_test_authdata_hook(PGauthData type, PGconn *conn, void *data);
+extern int	oauth_test_start_flow(PGconn *conn, PGoauthBearerRequestV2 *request);
+
+#endif							/* OAUTH_TEST_COMMON_H */
diff --git a/src/test/modules/oauth_validator/oauth_flow.c b/src/test/modules/oauth_validator/oauth_flow.c
new file mode 100644
index 00000000000..9ef67f03eb3
--- /dev/null
+++ b/src/test/modules/oauth_validator/oauth_flow.c
@@ -0,0 +1,72 @@
+/*-------------------------------------------------------------------------
+ *
+ * oauth_flow.c
+ *	  Test plugin for clientside OAuth flows
+ *
+ * Copyright (c) 2025, PostgreSQL Global Development Group
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include <stdlib.h>
+#include <string.h>
+
+/* Since we want to test the public API, only include public headers here. */
+#include "libpq-fe.h"
+#include "libpq-oauth.h"
+#include "oauth_test_common.h"
+
+static void
+load_test_flags(void)
+{
+	int			argc;
+	char	  **argv;
+	char	   *env = getenv("OAUTH_TEST_FLAGS");
+	int			flag_count;
+	int			i;
+
+	if (!env || !env[0])
+	{
+		fprintf(stderr, "OAUTH_TEST_FLAGS must be set\n");
+		exit(1);
+	}
+
+	flag_count = 1;
+	for (char *c = env; *c; c++)
+	{
+		if (*c == '\x01')
+			flag_count++;
+	}
+
+	/* Slice OAUTH_TEST_FLAGS into a fake argv array. */
+	env = strdup(env);
+	argc = flag_count + 1;
+	argv = malloc(sizeof(*argv) * (argc + 1));
+
+	if (!env || !argv)
+	{
+		fprintf(stderr, "out of memory");
+		exit(1);
+	}
+
+	argv[0] = "[plugin test]";
+	for (i = 1; i < flag_count; i++)
+	{
+		argv[i] = env;
+
+		env = strchr(env, '\x01');
+		*env++ = '\0';
+	}
+	argv[flag_count] = env;
+	argv[argc] = NULL;
+
+	oauth_test_parse_argv(argc, argv, 1 /* plugin */ );
+}
+
+int
+pg_start_oauthbearer(PGconn *conn, PGoauthBearerRequestV2 *request)
+{
+	load_test_flags();
+
+	return oauth_test_start_flow(conn, request);
+}
diff --git a/src/test/modules/oauth_validator/oauth_hook_client.c b/src/test/modules/oauth_validator/oauth_hook_client.c
index 4695d73e8f7..5f932acc571 100644
--- a/src/test/modules/oauth_validator/oauth_hook_client.c
+++ b/src/test/modules/oauth_validator/oauth_hook_client.c
@@ -18,143 +18,18 @@
 
 #include <sys/socket.h>
 
-#include "getopt_long.h"
 #include "libpq-fe.h"
 
-static int	handle_auth_data(PGauthData type, PGconn *conn, void *data);
-static PostgresPollingStatusType async_cb(PGconn *conn,
-										  PGoauthBearerRequest *req,
-										  pgsocket *altsock);
-static PostgresPollingStatusType misbehave_cb(PGconn *conn,
-											  PGoauthBearerRequest *req,
-											  pgsocket *altsock);
-
-static void
-usage(char *argv[])
-{
-	printf("usage: %s [flags] CONNINFO\n\n", argv[0]);
-
-	printf("recognized flags:\n");
-	printf("  -h, --help              show this message\n");
-	printf("  -v VERSION              select the hook API version (default 2)\n");
-	printf("  --expected-scope SCOPE  fail if received scopes do not match SCOPE\n");
-	printf("  --expected-uri URI      fail if received configuration link does not match URI\n");
-	printf("  --expected-issuer ISS   fail if received issuer does not match ISS (v2 only)\n");
-	printf("  --misbehave=MODE        have the hook fail required postconditions\n"
-		   "                          (MODEs: no-hook, fail-async, no-token, no-socket)\n");
-	printf("  --no-hook               don't install OAuth hooks\n");
-	printf("  --hang-forever          don't ever return a token (combine with connect_timeout)\n");
-	printf("  --token TOKEN           use the provided TOKEN value\n");
-	printf("  --error ERRMSG          fail instead, with the given ERRMSG (v2 only)\n");
-	printf("  --stress-async          busy-loop on PQconnectPoll rather than polling\n");
-}
-
-/* --options */
-static bool no_hook = false;
-static bool hang_forever = false;
-static bool stress_async = false;
-static const char *expected_uri = NULL;
-static const char *expected_issuer = NULL;
-static const char *expected_scope = NULL;
-static const char *misbehave_mode = NULL;
-static char *token = NULL;
-static char *errmsg = NULL;
-static int	hook_version = PQAUTHDATA_OAUTH_BEARER_TOKEN_V2;
+#include "oauth_test_common.h"
 
 int
 main(int argc, char *argv[])
 {
-	static const struct option long_options[] = {
-		{"help", no_argument, NULL, 'h'},
-
-		{"expected-scope", required_argument, NULL, 1000},
-		{"expected-uri", required_argument, NULL, 1001},
-		{"no-hook", no_argument, NULL, 1002},
-		{"token", required_argument, NULL, 1003},
-		{"hang-forever", no_argument, NULL, 1004},
-		{"misbehave", required_argument, NULL, 1005},
-		{"stress-async", no_argument, NULL, 1006},
-		{"expected-issuer", required_argument, NULL, 1007},
-		{"error", required_argument, NULL, 1008},
-		{0}
-	};
-
-	const char *conninfo;
+	const char *conninfo = oauth_test_parse_argv(argc, argv, 0 /* hook */ );
 	PGconn	   *conn;
-	int			c;
-
-	while ((c = getopt_long(argc, argv, "hv:", long_options, NULL)) != -1)
-	{
-		switch (c)
-		{
-			case 'h':
-				usage(argv);
-				return 0;
-
-			case 'v':
-				if (strcmp(optarg, "1") == 0)
-					hook_version = PQAUTHDATA_OAUTH_BEARER_TOKEN;
-				else if (strcmp(optarg, "2") == 0)
-					hook_version = PQAUTHDATA_OAUTH_BEARER_TOKEN_V2;
-				else
-				{
-					usage(argv);
-					return 1;
-				}
-				break;
-
-			case 1000:			/* --expected-scope */
-				expected_scope = optarg;
-				break;
-
-			case 1001:			/* --expected-uri */
-				expected_uri = optarg;
-				break;
-
-			case 1002:			/* --no-hook */
-				no_hook = true;
-				break;
-
-			case 1003:			/* --token */
-				token = optarg;
-				break;
-
-			case 1004:			/* --hang-forever */
-				hang_forever = true;
-				break;
-
-			case 1005:			/* --misbehave */
-				misbehave_mode = optarg;
-				break;
-
-			case 1006:			/* --stress-async */
-				stress_async = true;
-				break;
-
-			case 1007:			/* --expected-issuer */
-				expected_issuer = optarg;
-				break;
-
-			case 1008:			/* --error */
-				errmsg = optarg;
-				break;
-
-			default:
-				usage(argv);
-				return 1;
-		}
-	}
-
-	if (argc != optind + 1)
-	{
-		usage(argv);
-		return 1;
-	}
-
-	conninfo = argv[optind];
 
 	/* Set up our OAuth hooks. */
-	PQsetAuthDataHook(handle_auth_data);
+	PQsetAuthDataHook(oauth_test_authdata_hook);
 
 	/* Connect. (All the actual work is in the hook.) */
 	if (stress_async)
@@ -192,190 +67,3 @@ main(int argc, char *argv[])
 	PQfinish(conn);
 	return 0;
 }
-
-/*
- * PQauthDataHook implementation. Replaces the default client flow by handling
- * PQAUTHDATA_OAUTH_BEARER_TOKEN[_V2].
- */
-static int
-handle_auth_data(PGauthData type, PGconn *conn, void *data)
-{
-	PGoauthBearerRequest *req;
-	PGoauthBearerRequestV2 *req2 = NULL;
-
-	Assert(hook_version == PQAUTHDATA_OAUTH_BEARER_TOKEN ||
-		   hook_version == PQAUTHDATA_OAUTH_BEARER_TOKEN_V2);
-
-	if (no_hook || type != hook_version)
-		return 0;
-
-	req = data;
-	if (type == PQAUTHDATA_OAUTH_BEARER_TOKEN_V2)
-		req2 = data;
-
-	if (hang_forever)
-	{
-		/* Start asynchronous processing. */
-		req->async = async_cb;
-		return 1;
-	}
-
-	if (misbehave_mode)
-	{
-		if (strcmp(misbehave_mode, "no-hook") != 0)
-			req->async = misbehave_cb;
-		return 1;
-	}
-
-	if (expected_uri)
-	{
-		if (!req->openid_configuration)
-		{
-			fprintf(stderr, "expected URI \"%s\", got NULL\n", expected_uri);
-			return -1;
-		}
-
-		if (strcmp(expected_uri, req->openid_configuration) != 0)
-		{
-			fprintf(stderr, "expected URI \"%s\", got \"%s\"\n", expected_uri, req->openid_configuration);
-			return -1;
-		}
-	}
-
-	if (expected_scope)
-	{
-		if (!req->scope)
-		{
-			fprintf(stderr, "expected scope \"%s\", got NULL\n", expected_scope);
-			return -1;
-		}
-
-		if (strcmp(expected_scope, req->scope) != 0)
-		{
-			fprintf(stderr, "expected scope \"%s\", got \"%s\"\n", expected_scope, req->scope);
-			return -1;
-		}
-	}
-
-	if (expected_issuer)
-	{
-		if (!req2)
-		{
-			fprintf(stderr, "--expected-issuer cannot be combined with -v1\n");
-			return -1;
-		}
-
-		if (!req2->issuer)
-		{
-			fprintf(stderr, "expected issuer \"%s\", got NULL\n", expected_issuer);
-			return -1;
-		}
-
-		if (strcmp(expected_issuer, req2->issuer) != 0)
-		{
-			fprintf(stderr, "expected issuer \"%s\", got \"%s\"\n", expected_issuer, req2->issuer);
-			return -1;
-		}
-	}
-
-	if (errmsg)
-	{
-		if (token)
-		{
-			fprintf(stderr, "--error cannot be combined with --token\n");
-			return -1;
-		}
-		else if (!req2)
-		{
-			fprintf(stderr, "--error cannot be combined with -v1\n");
-			return -1;
-		}
-
-		req2->error = errmsg;
-		return -1;
-	}
-
-	req->token = token;
-	return 1;
-}
-
-static PostgresPollingStatusType
-async_cb(PGconn *conn, PGoauthBearerRequest *req, pgsocket *altsock)
-{
-	if (hang_forever)
-	{
-		/*
-		 * This code tests that nothing is interfering with libpq's handling
-		 * of connect_timeout.
-		 */
-		static pgsocket sock = PGINVALID_SOCKET;
-
-		if (sock == PGINVALID_SOCKET)
-		{
-			/* First call. Create an unbound socket to wait on. */
-#ifdef WIN32
-			WSADATA		wsaData;
-			int			err;
-
-			err = WSAStartup(MAKEWORD(2, 2), &wsaData);
-			if (err)
-			{
-				perror("WSAStartup failed");
-				return PGRES_POLLING_FAILED;
-			}
-#endif
-			sock = socket(AF_INET, SOCK_DGRAM, 0);
-			if (sock == PGINVALID_SOCKET)
-			{
-				perror("failed to create datagram socket");
-				return PGRES_POLLING_FAILED;
-			}
-		}
-
-		/* Make libpq wait on the (unreadable) socket. */
-		*altsock = sock;
-		return PGRES_POLLING_READING;
-	}
-
-	req->token = token;
-	return PGRES_POLLING_OK;
-}
-
-static PostgresPollingStatusType
-misbehave_cb(PGconn *conn, PGoauthBearerRequest *req, pgsocket *altsock)
-{
-	if (strcmp(misbehave_mode, "fail-async") == 0)
-	{
-		/* Just fail "normally". */
-		if (errmsg)
-		{
-			PGoauthBearerRequestV2 *req2;
-
-			if (hook_version == PQAUTHDATA_OAUTH_BEARER_TOKEN)
-			{
-				fprintf(stderr, "--error cannot be combined with -v1\n");
-				exit(1);
-			}
-
-			req2 = (PGoauthBearerRequestV2 *) req;
-			req2->error = errmsg;
-		}
-
-		return PGRES_POLLING_FAILED;
-	}
-	else if (strcmp(misbehave_mode, "no-token") == 0)
-	{
-		/* Callbacks must assign req->token before returning OK. */
-		return PGRES_POLLING_OK;
-	}
-	else if (strcmp(misbehave_mode, "no-socket") == 0)
-	{
-		/* Callbacks must assign *altsock before asking for polling. */
-		return PGRES_POLLING_READING;
-	}
-	else
-	{
-		fprintf(stderr, "unrecognized --misbehave mode: %s\n", misbehave_mode);
-		exit(1);
-	}
-}
diff --git a/src/test/modules/oauth_validator/oauth_test_common.c b/src/test/modules/oauth_validator/oauth_test_common.c
new file mode 100644
index 00000000000..cfcb4a354a8
--- /dev/null
+++ b/src/test/modules/oauth_validator/oauth_test_common.c
@@ -0,0 +1,373 @@
+/*-------------------------------------------------------------------------
+ *
+ * oauth_test_common.c
+ *	  Shared functionality for oauth_hook_client and oauth_flow
+ *
+ * Copyright (c) 2025, PostgreSQL Global Development Group
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres_fe.h"
+
+#include <sys/socket.h>
+
+#include "getopt_long.h"
+#include "libpq-fe.h"
+
+#include "oauth_test_common.h"
+
+static PostgresPollingStatusType async_cb(PGconn *conn,
+										  PGoauthBearerRequest *req,
+										  pgsocket *altsock);
+static PostgresPollingStatusType misbehave_cb(PGconn *conn,
+											  PGoauthBearerRequest *req,
+											  pgsocket *altsock);
+
+/* --options */
+static bool no_hook = false;
+static bool hang_forever = false;
+static const char *expected_uri = NULL;
+static const char *expected_issuer = NULL;
+static const char *expected_scope = NULL;
+static const char *misbehave_mode = NULL;
+static char *token = NULL;
+static char *errmsg = NULL;
+static int	hook_version = PQAUTHDATA_OAUTH_BEARER_TOKEN_V2;
+
+/*
+ * XXX: stress_async is exported for the benefit of oauth_hook_client. Since
+ * we only use public headers (libpq-fe.h) for oauth_flow, it needs to be an int
+ * rather than a bool.
+ */
+int			stress_async = false;
+
+static void
+usage(char *argv[])
+{
+	printf("usage: %s [flags] CONNINFO\n\n", argv[0]);
+
+	printf("recognized flags:\n");
+	printf("  -h, --help              show this message\n");
+	printf("  -v VERSION              select the hook API version (default 2)\n");
+	printf("  --expected-scope SCOPE  fail if received scopes do not match SCOPE\n");
+	printf("  --expected-uri URI      fail if received configuration link does not match URI\n");
+	printf("  --expected-issuer ISS   fail if received issuer does not match ISS (v2 only)\n");
+	printf("  --misbehave=MODE        have the hook fail required postconditions\n"
+		   "                          (MODEs: no-hook, fail-async, no-token, no-socket)\n");
+	printf("  --no-hook               don't install OAuth hooks\n");
+	printf("  --hang-forever          don't ever return a token (combine with connect_timeout)\n");
+	printf("  --token TOKEN           use the provided TOKEN value\n");
+	printf("  --error ERRMSG          fail instead, with the given ERRMSG (v2 only)\n");
+	printf("  --stress-async          busy-loop on PQconnectPoll rather than polling\n");
+}
+
+char *
+oauth_test_parse_argv(int argc, char *argv[], int for_plugin)
+{
+	static const struct option long_options[] = {
+		{"help", no_argument, NULL, 'h'},
+
+		{"expected-scope", required_argument, NULL, 1000},
+		{"expected-uri", required_argument, NULL, 1001},
+		{"no-hook", no_argument, NULL, 1002},
+		{"token", required_argument, NULL, 1003},
+		{"hang-forever", no_argument, NULL, 1004},
+		{"misbehave", required_argument, NULL, 1005},
+		{"stress-async", no_argument, NULL, 1006},
+		{"expected-issuer", required_argument, NULL, 1007},
+		{"error", required_argument, NULL, 1008},
+		{0}
+	};
+
+	int			c;
+
+	if (for_plugin)
+	{
+		/* The "real" argv has already been parsed. Reset optind. */
+		optind = 1;
+	}
+
+	while ((c = getopt_long(argc, argv, "hv:", long_options, NULL)) != -1)
+	{
+		switch (c)
+		{
+			case 'h':
+				usage(argv);
+				exit(0);
+
+			case 'v':
+				if (strcmp(optarg, "1") == 0)
+					hook_version = PQAUTHDATA_OAUTH_BEARER_TOKEN;
+				else if (strcmp(optarg, "2") == 0)
+					hook_version = PQAUTHDATA_OAUTH_BEARER_TOKEN_V2;
+				else
+				{
+					usage(argv);
+					exit(1);
+				}
+				break;
+
+			case 1000:			/* --expected-scope */
+				expected_scope = optarg;
+				break;
+
+			case 1001:			/* --expected-uri */
+				expected_uri = optarg;
+				break;
+
+			case 1002:			/* --no-hook */
+				no_hook = true;
+				break;
+
+			case 1003:			/* --token */
+				token = optarg;
+				break;
+
+			case 1004:			/* --hang-forever */
+				hang_forever = true;
+				break;
+
+			case 1005:			/* --misbehave */
+				misbehave_mode = optarg;
+				break;
+
+			case 1006:			/* --stress-async */
+				stress_async = true;
+				break;
+
+			case 1007:			/* --expected-issuer */
+				expected_issuer = optarg;
+				break;
+
+			case 1008:			/* --error */
+				errmsg = optarg;
+				break;
+
+			default:
+				usage(argv);
+				exit(1);
+		}
+	}
+
+	if (argc != (for_plugin ? optind : optind + 1))
+	{
+		usage(argv);
+		exit(1);
+	}
+
+	return argv[optind];
+}
+
+/*
+ * PQauthDataHook implementation. Replaces the default client flow by handling
+ * PQAUTHDATA_OAUTH_BEARER_TOKEN[_V2].
+ */
+int
+oauth_test_authdata_hook(PGauthData type, PGconn *conn, void *data)
+{
+	PGoauthBearerRequest *req;
+	PGoauthBearerRequestV2 *req2 = NULL;
+
+	Assert(hook_version == PQAUTHDATA_OAUTH_BEARER_TOKEN ||
+		   hook_version == PQAUTHDATA_OAUTH_BEARER_TOKEN_V2);
+
+	if (no_hook || type != hook_version)
+		return 0;
+
+	req = data;
+	if (type == PQAUTHDATA_OAUTH_BEARER_TOKEN_V2)
+		req2 = data;
+
+	if (hang_forever)
+	{
+		/* Start asynchronous processing. */
+		req->async = async_cb;
+		return 1;
+	}
+
+	if (misbehave_mode)
+	{
+		if (strcmp(misbehave_mode, "no-hook") != 0)
+			req->async = misbehave_cb;
+		return 1;
+	}
+
+	if (expected_uri)
+	{
+		if (!req->openid_configuration)
+		{
+			fprintf(stderr, "expected URI \"%s\", got NULL\n", expected_uri);
+			return -1;
+		}
+
+		if (strcmp(expected_uri, req->openid_configuration) != 0)
+		{
+			fprintf(stderr, "expected URI \"%s\", got \"%s\"\n", expected_uri, req->openid_configuration);
+			return -1;
+		}
+	}
+
+	if (expected_scope)
+	{
+		if (!req->scope)
+		{
+			fprintf(stderr, "expected scope \"%s\", got NULL\n", expected_scope);
+			return -1;
+		}
+
+		if (strcmp(expected_scope, req->scope) != 0)
+		{
+			fprintf(stderr, "expected scope \"%s\", got \"%s\"\n", expected_scope, req->scope);
+			return -1;
+		}
+	}
+
+	if (expected_issuer)
+	{
+		if (!req2)
+		{
+			fprintf(stderr, "--expected-issuer cannot be combined with -v1\n");
+			return -1;
+		}
+
+		if (!req2->issuer)
+		{
+			fprintf(stderr, "expected issuer \"%s\", got NULL\n", expected_issuer);
+			return -1;
+		}
+
+		if (strcmp(expected_issuer, req2->issuer) != 0)
+		{
+			fprintf(stderr, "expected issuer \"%s\", got \"%s\"\n", expected_issuer, req2->issuer);
+			return -1;
+		}
+	}
+
+	if (errmsg)
+	{
+		if (token)
+		{
+			fprintf(stderr, "--error cannot be combined with --token\n");
+			return -1;
+		}
+		else if (!req2)
+		{
+			fprintf(stderr, "--error cannot be combined with -v1\n");
+			return -1;
+		}
+
+		req2->error = errmsg;
+		return -1;
+	}
+
+	req->token = token;
+	return 1;
+}
+
+/*
+ * Sets up a request for a plugin module (pg_start_oauthbearer()) rather than
+ * using the hook.
+ */
+int
+oauth_test_start_flow(PGconn *conn, PGoauthBearerRequestV2 *request)
+{
+	int			ret;
+
+	/*
+	 * We can still defer to the hook above to avoid copying code; we just
+	 * have to translate the return value.
+	 */
+	ret = oauth_test_authdata_hook(PQAUTHDATA_OAUTH_BEARER_TOKEN_V2, conn,
+								   request);
+
+	if (ret == 0)
+	{
+		/* This is a bug in the test. */
+		fprintf(stderr, "plugin tests cannot make use of -v1 or --no-hook\n");
+		exit(1);
+	}
+
+	return (ret == 1) ? 0 : -1;
+}
+
+static PostgresPollingStatusType
+async_cb(PGconn *conn, PGoauthBearerRequest *req, pgsocket *altsock)
+{
+	if (hang_forever)
+	{
+		/*
+		 * This code tests that nothing is interfering with libpq's handling
+		 * of connect_timeout.
+		 */
+		static pgsocket sock = PGINVALID_SOCKET;
+
+		if (sock == PGINVALID_SOCKET)
+		{
+			/* First call. Create an unbound socket to wait on. */
+#ifdef WIN32
+			WSADATA		wsaData;
+			int			err;
+
+			err = WSAStartup(MAKEWORD(2, 2), &wsaData);
+			if (err)
+			{
+				perror("WSAStartup failed");
+				return PGRES_POLLING_FAILED;
+			}
+#endif
+			sock = socket(AF_INET, SOCK_DGRAM, 0);
+			if (sock == PGINVALID_SOCKET)
+			{
+				perror("failed to create datagram socket");
+				return PGRES_POLLING_FAILED;
+			}
+		}
+
+		/* Make libpq wait on the (unreadable) socket. */
+		*altsock = sock;
+		return PGRES_POLLING_READING;
+	}
+
+	req->token = token;
+	return PGRES_POLLING_OK;
+}
+
+static PostgresPollingStatusType
+misbehave_cb(PGconn *conn, PGoauthBearerRequest *req, pgsocket *altsock)
+{
+	if (strcmp(misbehave_mode, "fail-async") == 0)
+	{
+		/* Just fail "normally". */
+		if (errmsg)
+		{
+			PGoauthBearerRequestV2 *req2;
+
+			if (hook_version == PQAUTHDATA_OAUTH_BEARER_TOKEN)
+			{
+				fprintf(stderr, "--error cannot be combined with -v1\n");
+				exit(1);
+			}
+
+			req2 = (PGoauthBearerRequestV2 *) req;
+			req2->error = errmsg;
+		}
+
+		return PGRES_POLLING_FAILED;
+	}
+	else if (strcmp(misbehave_mode, "no-token") == 0)
+	{
+		/* Callbacks must assign req->token before returning OK. */
+		return PGRES_POLLING_OK;
+	}
+	else if (strcmp(misbehave_mode, "no-socket") == 0)
+	{
+		/* Callbacks must assign *altsock before asking for polling. */
+		return PGRES_POLLING_READING;
+	}
+	else
+	{
+		fprintf(stderr, "unrecognized --misbehave mode: %s\n", misbehave_mode);
+		exit(1);
+	}
+}
diff --git a/src/test/modules/oauth_validator/t/002_client.pl b/src/test/modules/oauth_validator/t/002_client.pl
index dac684d7852..2f6f60331a1 100644
--- a/src/test/modules/oauth_validator/t/002_client.pl
+++ b/src/test/modules/oauth_validator/t/002_client.pl
@@ -1,6 +1,6 @@
 #
 # Exercises the API for custom OAuth client flows, using the oauth_hook_client
-# test driver.
+# test driver and the oauth_flow custom plugin.
 #
 # Copyright (c) 2021-2026, PostgreSQL Global Development Group
 #
@@ -20,6 +20,10 @@ if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\boauth\b/)
 	  'Potentially unsafe test oauth not enabled in PG_TEST_EXTRA';
 }
 
+my $plugin_supported = (
+		 check_pg_config("#define HAVE_SYS_EVENT_H 1")
+	  or check_pg_config("#define HAVE_SYS_EPOLL_H 1"));
+
 #
 # Cluster Setup
 #
@@ -72,6 +76,8 @@ sub test
 		$flags = $params{flags};
 	}
 
+	# First run the oauth_hook_client, which uses PQauthDataHook to insert a new
+	# OAuth flow.
 	my @cmd = ("oauth_hook_client", @{$flags}, $common_connstr);
 	note "running '" . join("' '", @cmd) . "'";
 
@@ -103,6 +109,37 @@ sub test
 		$node->log_check("$test_name: log matches",
 			$log_start, log_like => $params{log_like});
 	}
+
+  SKIP:
+	{
+		last SKIP if $params{hook_only};
+		skip "OAuth modules are not supported on this platform"
+		  unless $plugin_supported;
+
+		# Run the same test with psql itself, loading the oauth_flow.so module.
+		local $ENV{PGOAUTHMODULE} = $ENV{flow_module_path};
+
+		# Flags are passed to the module via OAUTH_TEST_FLAGS, with 0x01 as a
+		# separator.
+		local $ENV{OAUTH_TEST_FLAGS} = join("\x01", @{$flags});
+
+		if ($params{expect_success})
+		{
+			$node->connect_ok(
+				$common_connstr,
+				"[plugin flow] $test_name",
+				expected_stderr => $params{expected_stderr},
+				log_like => $params{log_like});
+		}
+		else
+		{
+			$node->connect_fails(
+				$common_connstr,
+				"[plugin flow] $test_name",
+				expected_stderr => $params{expected_stderr},
+				log_like => $params{log_like});
+		}
+	}
 }
 
 test(
@@ -136,6 +173,7 @@ $common_connstr = "$base_connstr oauth_issuer=$issuer oauth_client_id=myID";
 # Make sure the v1 hook continues to work.
 test(
 	"v1 synchronous hook can provide a token",
+	hook_only => 1,    # plugins don't support API v1
 	flags => [
 		"-v1",
 		"--token" => "my-token-v1",
@@ -150,6 +188,7 @@ if ($ENV{with_libcurl} ne 'yes')
 	# libpq should help users out if no OAuth support is built in.
 	test(
 		"fails without custom hook installed",
+		hook_only => 1,    # plugins can't use --no-hook
 		flags => ["--no-hook"],
 		expected_stderr =>
 		  qr/no OAuth flows are available \(try installing the libpq-oauth package\)/
@@ -206,6 +245,7 @@ foreach my $c (@cases)
 	test(
 		"hook misbehavior: $c->{'flag'} (v1)",
 		flags => [ '-v1', $c->{'flag'} ],
+		hook_only => 1,    # plugins can't use -v1
 		expected_stderr => $c->{'expected_error'});
 }
 
@@ -219,4 +259,19 @@ test(
 	expected_stderr =>
 	  qr/user-defined OAuth flow failed: async error message/);
 
+SKIP:
+{
+	skip "OAuth modules are not supported on this platform"
+	  unless $plugin_supported;
+
+	# Make sure a misaimed PGOAUTHMODULE gives the correct error message.
+	local $ENV{PGOAUTHMODULE} = $node->basedir . '/nonexistent.so';
+
+	$node->connect_fails(
+		$common_connstr,
+		"PGOAUTHMODULE error messages",
+		expected_stderr =>
+		  qr/user-defined OAuth flow failed: plugin could not be loaded/);
+}
+
 done_testing();
-- 
2.34.1



view thread (19+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [oauth] Stabilize the libpq-oauth ABI (and allow alternative implementations?)
  In-Reply-To: <CAOYmi+=cd0SdEm4Pw3RE4eD=rbXex9Rkc3DsNiaDAqQti9taQQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox