v31-0005-v30-review-comments.patch

From e77e769792a3280e759aefc162fbcaa790b98123 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <daniel@yesql.se>
Date: Fri, 18 Oct 2024 12:40:26 +0200
Subject: [PATCH v31 5/5] v30-review-comments

---
 doc/src/sgml/client-auth.sgml                 |  98 +++++++++-
 doc/src/sgml/config.sgml                      |  17 ++
 doc/src/sgml/installation.sgml                |  29 +++
 doc/src/sgml/libpq.sgml                       |  14 +-
 doc/src/sgml/oauth-validators.sgml            |  36 +++-
 doc/src/sgml/postgres.sgml                    |   2 +-
 doc/src/sgml/regress.sgml                     |  10 +
 src/backend/libpq/auth-oauth.c                | 172 +++++++++++-------
 src/backend/libpq/hba.c                       |  26 +++
 src/interfaces/libpq/fe-auth-oauth.c          |   5 +-
 .../modules/oauth_validator/t/001_server.pl   |   9 +-
 src/test/modules/oauth_validator/validator.c  |   7 +-
 12 files changed, 343 insertions(+), 82 deletions(-)

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index fb78b6c886..8d351c2089 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -2336,7 +2336,103 @@ host ... radius radiusservers="server1,server2" radiussecrets="""secret one"",""
    </indexterm>
 
    <para>
-    TODO
+    OAuth 2.0 is an industry-standard framework, defined in
+    <ulink url="https://datatracker.ietf.org/doc/html/rfc6749">RFC 6749</ulink>,
+    to enable third-party applications to obtain limited access to a protected
+    resource.
+
+    OAuth support has to be enabled when <productname>PostgreSQL</productname>
+    is built, see <xref linkend="installation"/> for more information.
+
+    <itemizedlist>
+     <listitem>
+      <para>
+       Resource owner: The user or system who owns protected resources and can
+       grant access to them.
+      </para>
+     </listitem>
+     <listitem>
+      <para>
+       Client: The system which accesses to the protected resources using access
+       tokens.  Applications using libpq are the clients in connecting to a
+       <productname>PostgreSQL</productname> cluster.
+      </para>
+     </listitem>
+     <listitem>
+      <para>
+       Authentication server: The system which recieves requests from, and
+       issues access tokens to, the client upon successful authentication by
+       the resource owner.
+      </para>
+     </listitem>
+
+     <listitem>
+      <para>
+       Resource server: The system which owns the protected resources and can
+       grant access to them. The <productname>PostgreSQL</productname> cluster
+       being connected to is the resource server.
+      </para>
+     </listitem>
+
+    </itemizedlist>
+   </para>
+
+   <para>
+    <productname>PostgreSQL</productname> supports bearer tokens, defined in
+    <ulink url="https://datatracker.ietf.org/doc/html/rfc6750">RFC 6750</ulink>
+    which are a type of access token used with OAuth 2.0 where the token is an
+    opaque string.  The format of the access token is implementation specific
+    and is chosen by each authentication server.
+   </para>
+
+   <para>
+    The following configuration options are supported for OAuth:
+    <variablelist>
+     <varlistentry>
+      <term><literal>issuer</literal></term>
+      <listitem>
+       <para>
+        The <acronym>URL</acronym> of the OAuth issuing party, which the client
+        must contact to receive a bearer token.  This parameter is required.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><literal>scope</literal></term>
+      <listitem>
+       <para>
+        The OAuth scope required for the server to authenticate and/or authorize
+        the user. The value of the scope is dependent on the OAuth validation
+        module used (see <xref linkend="oauth-validators" /> for more information
+        on validators).  This parameter is required.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><literal>map</literal></term>
+      <listitem>
+       <para>
+        Allows for mapping between OAuth identity provider and database user
+        names.  See <xref linkend="auth-username-maps"/> for details.  If a
+        map is not specified the user name returned from the OAuth validator
+        must match the role being requested.  This parameter is optional.
+       </para>
+      </listitem>
+     </varlistentry>
+
+     <varlistentry>
+      <term><literal>trust_validator_authz</literal></term>
+      <listitem>
+       <para>
+        When set to <literal>1</literal> standard user mapping is skipped. If
+        the OAuth token is validated the user can connect under its desired
+        role.
+       </para>
+      </listitem>
+     </varlistentry>
+    </variablelist>
    </para>
   </sect1>
 
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f089a8ff4c..73bf21c599 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1214,6 +1214,23 @@ include_dir 'conf.d'
        </para>
       </listitem>
      </varlistentry>
+
+     <varlistentry id="guc-oauth-validator-library" xreflabel="oauth_validator_library">
+      <term><varname>oauth_validator_library</varname> (<type>string</type>)
+      <indexterm>
+       <primary><varname>oauth_validator_library</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        The library to use for validating OAuth connection tokens. If set to
+        an empty string (the default), OAuth connections will be refused. For
+        more information on implementing OAuth validators see
+        <xref linkend="oauth-validators" />. This parameter can only be set in
+        the <filename>postgresql.conf</filename> file.
+       </para>
+      </listitem>
+     </varlistentry>
      </variablelist>
      </sect2>
 
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 3a491b5989..9a76aac08b 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -1064,6 +1064,20 @@ build-postgresql:
        </listitem>
       </varlistentry>
 
+      <varlistentry id="configure-option-with-oauth">
+       <term><option>--with-oauth=<replaceable>LIBRARY</replaceable></option></term>
+       <listitem>
+        <para>
+         Build with OAuth authentication and authorization support.  The only
+         <replaceable>LIBRARY</replaceable> supported is <option>curl</option>.
+         This requires the <productname>curl</productname> package to be
+         installed.  Building with this will check for the required header files
+         and libraries to make sure that your <productname>curl</productname>
+         installation is sufficient before proceeding.
+        </para>
+       </listitem>
+      </varlistentry>
+
       <varlistentry id="configure-option-with-systemd">
        <term><option>--with-systemd</option></term>
        <listitem>
@@ -2508,6 +2522,21 @@ ninja install
       </listitem>
      </varlistentry>
 
+     <varlistentry id="configure-with-oauth">
+      <term><option>-Doauth={ auto | <replaceable>LIBRARY</replaceable> | disabled }</option></term>
+      <listitem>
+       <para>
+        Build with OAuth authentication and authorization support.  The only
+        <replaceable>LIBRARY</replaceable> supported is <option>curl</option>.
+        This requires the <productname>curl</productname> package to be
+        installed.  Building with this will check for the required header files
+        and libraries to make sure that your <productname>curl</productname>
+        installation is sufficient before proceeding. The default for this
+        option is auto.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="configure-with-systemd-meson">
       <term><option>-Dsystemd={ auto | enabled | disabled }</option></term>
       <listitem>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index ffec0431e3..86fd146af2 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -2341,7 +2341,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       <term><literal>oauth_client_id</literal></term>
       <listitem>
        <para>
-        TODO
+        The client identifier as issued by the authorization server.
        </para>
       </listitem>
      </varlistentry>
@@ -2350,7 +2350,7 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       <term><literal>oauth_client_secret</literal></term>
       <listitem>
        <para>
-        TODO
+        The client password.
        </para>
       </listitem>
      </varlistentry>
@@ -2368,7 +2368,8 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
       <term><literal>oauth_scope</literal></term>
       <listitem>
        <para>
-        TODO
+        The scope of the access request sent to the authorization server.
+        This parameter is optional.
        </para>
       </listitem>
      </varlistentry>
@@ -10017,6 +10018,11 @@ void PQinitSSL(int do_ssl);
 void PQsetAuthDataHook(PQauthDataHook_type hook);
 </synopsis>
       </para>
+
+      <para>
+       If <replaceable>hook</replaceable> is set to a null pointer instead of
+       a function pointer, the default hook will be installed.
+      </para>
      </listitem>
     </varlistentry>
 
@@ -10025,7 +10031,7 @@ void PQsetAuthDataHook(PQauthDataHook_type hook);
 
      <listitem>
       <para>
-       TODO
+       Retrieves the current value of <literal>PGauthDataHook</literal>.
 <synopsis>
 PQauthDataHook_type PQgetAuthDataHook(void);
 </synopsis>
diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml
index c9914519fc..4615159a9f 100644
--- a/doc/src/sgml/oauth-validators.sgml
+++ b/doc/src/sgml/oauth-validators.sgml
@@ -1,13 +1,13 @@
 <!-- doc/src/sgml/oauth-validators.sgml -->
 
 <chapter id="oauth-validators">
- <title>Implementing OAuth Validator Modules</title>
+ <title>OAuth Validator Modules</title>
  <indexterm zone="oauth-validators">
   <primary>OAuth Validators</primary>
  </indexterm>
  <para>
   <productname>PostgreSQL</productname> provides infrastructure for creating
-  custom modules to perform server-side validation of OAuth tokens.
+  custom modules to perform server-side validation of OAuth bearer tokens.
  </para>
  <para>
   OAuth validation modules must at least consist of an initialization function
@@ -74,9 +74,41 @@ typedef void (*ValidatorStartupCB) (ValidatorModuleState *state);
   <sect2 id="oauth-validator-callback-validate">
    <title>Validate Callback</title>
    <para>
+    The <function>validate_cb</function> callback is executed during the OAuth
+    exchange when a user attempts to authenticate using OAuth. The token is
+    parsed to ensure being well-formed syntactically, but no semantical check
+    has been performed. Any state set in previous calls will be available in
+    <structfield>state->privata_data</structfield>.
+
 <programlisting>
 typedef ValidatorModuleResult *(*ValidatorValidateCB) (ValidatorModuleState *state, const char *token, const char *role);
 </programlisting>
+
+    <replaceable>token</replaceable> will contain the bearer token to validate,
+    <replaceable>role</replaceable> will contain the role the user request to
+    log in as. The callback must return a <literal>ValidatorModuleResult</literal>
+    struct which is defined as below:
+
+<programlisting>
+typedef struct ValidatorModuleResult
+{
+    bool        authorized;
+    char       *authn_id;
+} ValidatorModuleResult;
+</programlisting>
+
+    If <structfield>authorized</structfield> is set to <literal>true</literal>
+    the bearer token is defined to be valid.
+    To authenticate the user, the authenticated user name shall be returned in
+    the <structfield>authn_id</structfield> field. When authenticating against
+    a HBA rule with <literal>trust_validator_authz</literal> turned on the
+    <structfield>authn_id</structfield> user name must exactly match the role
+    expected to login as.
+   </para>
+   <para>
+    The caller assumes ownership of the returned memory allocation, the
+    validator module should not in any way access the memory after it has been
+    returned.
    </para>
   </sect2>
 
diff --git a/doc/src/sgml/postgres.sgml b/doc/src/sgml/postgres.sgml
index 321d4590a3..af476c82fc 100644
--- a/doc/src/sgml/postgres.sgml
+++ b/doc/src/sgml/postgres.sgml
@@ -229,6 +229,7 @@ break is not needed in a wider output rendering.
   &logicaldecoding;
   &replication-origins;
   &archive-modules;
+  &oauth-validators;
 
  </part>
 
@@ -264,7 +265,6 @@ break is not needed in a wider output rendering.
   &bki;
   &planstats;
   &backup-manifest;
-  &oauth-validators;
 
  </part>
 
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index f4cef9e80f..ae4732df65 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -336,6 +336,16 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl load_balance libpq_encryption'
       </para>
      </listitem>
     </varlistentry>
+
+    <varlistentry>
+     <term><literal>oauth</literal></term>
+     <listitem>
+      <para>
+       Runs the test suite under <filename>src/test/modules/oauth_validator</filename>.
+       This opens TCP/IP listen sockets for a test-server running HTTPS.
+      </para>
+     </listitem>
+    </varlistentry>
    </variablelist>
 
    Tests for features that are not supported by the current build
diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index dea973247a..cfa5769b10 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -90,10 +90,10 @@ oauth_init(Port *port, const char *selected_mech, const char *shadow_pass)
 {
 	struct oauth_ctx *ctx;
 
-	if (strcmp(selected_mech, OAUTHBEARER_NAME))
+	if (strcmp(selected_mech, OAUTHBEARER_NAME) != 0)
 		ereport(ERROR,
-				(errcode(ERRCODE_PROTOCOL_VIOLATION),
-				 errmsg("client selected an invalid SASL authentication mechanism")));
+				errcode(ERRCODE_PROTOCOL_VIOLATION),
+				errmsg("client selected an invalid SASL authentication mechanism"));
 
 	ctx = palloc0(sizeof(*ctx));
 
@@ -142,14 +142,14 @@ oauth_exchange(void *opaq, const char *input, int inputlen,
 	 */
 	if (inputlen == 0)
 		ereport(ERROR,
-				(errcode(ERRCODE_PROTOCOL_VIOLATION),
-				 errmsg("malformed OAUTHBEARER message"),
-				 errdetail("The message is empty.")));
+				errcode(ERRCODE_PROTOCOL_VIOLATION),
+				errmsg("malformed OAUTHBEARER message"),
+				errdetail("The message is empty."));
 	if (inputlen != strlen(input))
 		ereport(ERROR,
-				(errcode(ERRCODE_PROTOCOL_VIOLATION),
-				 errmsg("malformed OAUTHBEARER message"),
-				 errdetail("Message length does not match input length.")));
+				errcode(ERRCODE_PROTOCOL_VIOLATION),
+				errmsg("malformed OAUTHBEARER message"),
+				errdetail("Message length does not match input length."));
 
 	switch (ctx->state)
 	{
@@ -165,9 +165,9 @@ oauth_exchange(void *opaq, const char *input, int inputlen,
 			 */
 			if (inputlen != 1 || *input != KVSEP)
 				ereport(ERROR,
-						(errcode(ERRCODE_PROTOCOL_VIOLATION),
-						 errmsg("malformed OAUTHBEARER message"),
-						 errdetail("Client did not send a kvsep response.")));
+						errcode(ERRCODE_PROTOCOL_VIOLATION),
+						errmsg("malformed OAUTHBEARER message"),
+						errdetail("Client did not send a kvsep response."));
 
 			/* The (failed) handshake is now complete. */
 			ctx->state = OAUTH_STATE_FINISHED;
@@ -193,9 +193,9 @@ oauth_exchange(void *opaq, const char *input, int inputlen,
 	{
 		case 'p':
 			ereport(ERROR,
-					(errcode(ERRCODE_PROTOCOL_VIOLATION),
-					 errmsg("malformed OAUTHBEARER message"),
-					 errdetail("The server does not support channel binding for OAuth, but the client message includes channel binding data.")));
+					errcode(ERRCODE_PROTOCOL_VIOLATION),
+					errmsg("malformed OAUTHBEARER message"),
+					errdetail("The server does not support channel binding for OAuth, but the client message includes channel binding data."));
 			break;
 
 		case 'y':				/* fall through */
@@ -203,19 +203,19 @@ oauth_exchange(void *opaq, const char *input, int inputlen,
 			p++;
 			if (*p != ',')
 				ereport(ERROR,
-						(errcode(ERRCODE_PROTOCOL_VIOLATION),
-						 errmsg("malformed OAUTHBEARER message"),
-						 errdetail("Comma expected, but found character \"%s\".",
-								   sanitize_char(*p))));
+						errcode(ERRCODE_PROTOCOL_VIOLATION),
+						errmsg("malformed OAUTHBEARER message"),
+						errdetail("Comma expected, but found character \"%s\".",
+								  sanitize_char(*p)));
 			p++;
 			break;
 
 		default:
 			ereport(ERROR,
-					(errcode(ERRCODE_PROTOCOL_VIOLATION),
-					 errmsg("malformed OAUTHBEARER message"),
-					 errdetail("Unexpected channel-binding flag %s.",
-							   sanitize_char(cbind_flag))));
+					errcode(ERRCODE_PROTOCOL_VIOLATION),
+					errmsg("malformed OAUTHBEARER message"),
+					errdetail("Unexpected channel-binding flag \"%s\".",
+							  sanitize_char(cbind_flag)));
 	}
 
 	/*
@@ -223,38 +223,38 @@ oauth_exchange(void *opaq, const char *input, int inputlen,
 	 */
 	if (*p == 'a')
 		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("client uses authorization identity, but it is not supported")));
+				errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				errmsg("client uses authorization identity, but it is not supported"));
 	if (*p != ',')
 		ereport(ERROR,
-				(errcode(ERRCODE_PROTOCOL_VIOLATION),
-				 errmsg("malformed OAUTHBEARER message"),
-				 errdetail("Unexpected attribute %s in client-first-message.",
-						   sanitize_char(*p))));
+				errcode(ERRCODE_PROTOCOL_VIOLATION),
+				errmsg("malformed OAUTHBEARER message"),
+				errdetail("Unexpected attribute \"%s\" in client-first-message.",
+						  sanitize_char(*p)));
 	p++;
 
 	/* All remaining fields are separated by the RFC's kvsep (\x01). */
 	if (*p != KVSEP)
 		ereport(ERROR,
-				(errcode(ERRCODE_PROTOCOL_VIOLATION),
-				 errmsg("malformed OAUTHBEARER message"),
-				 errdetail("Key-value separator expected, but found character %s.",
-						   sanitize_char(*p))));
+				errcode(ERRCODE_PROTOCOL_VIOLATION),
+				errmsg("malformed OAUTHBEARER message"),
+				errdetail("Key-value separator expected, but found character \"%s\".",
+						  sanitize_char(*p)));
 	p++;
 
 	auth = parse_kvpairs_for_auth(&p);
 	if (!auth)
 		ereport(ERROR,
-				(errcode(ERRCODE_PROTOCOL_VIOLATION),
-				 errmsg("malformed OAUTHBEARER message"),
-				 errdetail("Message does not contain an auth value.")));
+				errcode(ERRCODE_PROTOCOL_VIOLATION),
+				errmsg("malformed OAUTHBEARER message"),
+				errdetail("Message does not contain an auth value."));
 
 	/* We should be at the end of our message. */
 	if (*p)
 		ereport(ERROR,
-				(errcode(ERRCODE_PROTOCOL_VIOLATION),
-				 errmsg("malformed OAUTHBEARER message"),
-				 errdetail("Message contains additional data after the final terminator.")));
+				errcode(ERRCODE_PROTOCOL_VIOLATION),
+				errmsg("malformed OAUTHBEARER message"),
+				errdetail("Message contains additional data after the final terminator."));
 
 	if (!validate(ctx->port, auth))
 	{
@@ -308,16 +308,16 @@ validate_kvpair(const char *key, const char *val)
 
 	if (!key[0])
 		ereport(ERROR,
-				(errcode(ERRCODE_PROTOCOL_VIOLATION),
-				 errmsg("malformed OAUTHBEARER message"),
-				 errdetail("Message contains an empty key name.")));
+				errcode(ERRCODE_PROTOCOL_VIOLATION),
+				errmsg("malformed OAUTHBEARER message"),
+				errdetail("Message contains an empty key name."));
 
 	span = strspn(key, key_allowed_set);
 	if (key[span] != '\0')
 		ereport(ERROR,
-				(errcode(ERRCODE_PROTOCOL_VIOLATION),
-				 errmsg("malformed OAUTHBEARER message"),
-				 errdetail("Message contains an invalid key name.")));
+				errcode(ERRCODE_PROTOCOL_VIOLATION),
+				errmsg("malformed OAUTHBEARER message"),
+				errdetail("Message contains an invalid key name."));
 
 	/*-----
 	 * From Sec 3.1:
@@ -341,9 +341,9 @@ validate_kvpair(const char *key, const char *val)
 
 			default:
 				ereport(ERROR,
-						(errcode(ERRCODE_PROTOCOL_VIOLATION),
-						 errmsg("malformed OAUTHBEARER message"),
-						 errdetail("Message contains an invalid value.")));
+						errcode(ERRCODE_PROTOCOL_VIOLATION),
+						errmsg("malformed OAUTHBEARER message"),
+						errdetail("Message contains an invalid value."));
 		}
 	}
 }
@@ -386,9 +386,9 @@ parse_kvpairs_for_auth(char **input)
 		end = strchr(pos, KVSEP);
 		if (!end)
 			ereport(ERROR,
-					(errcode(ERRCODE_PROTOCOL_VIOLATION),
-					 errmsg("malformed OAUTHBEARER message"),
-					 errdetail("Message contains an unterminated key/value pair.")));
+					errcode(ERRCODE_PROTOCOL_VIOLATION),
+					errmsg("malformed OAUTHBEARER message"),
+					errdetail("Message contains an unterminated key/value pair."));
 		*end = '\0';
 
 		if (pos == end)
@@ -404,9 +404,9 @@ parse_kvpairs_for_auth(char **input)
 		sep = strchr(pos, '=');
 		if (!sep)
 			ereport(ERROR,
-					(errcode(ERRCODE_PROTOCOL_VIOLATION),
-					 errmsg("malformed OAUTHBEARER message"),
-					 errdetail("Message contains a key without a value.")));
+					errcode(ERRCODE_PROTOCOL_VIOLATION),
+					errmsg("malformed OAUTHBEARER message"),
+					errdetail("Message contains a key without a value."));
 		*sep = '\0';
 
 		/* Both key and value are now safely terminated. */
@@ -418,9 +418,9 @@ parse_kvpairs_for_auth(char **input)
 		{
 			if (auth)
 				ereport(ERROR,
-						(errcode(ERRCODE_PROTOCOL_VIOLATION),
-						 errmsg("malformed OAUTHBEARER message"),
-						 errdetail("Message contains multiple auth values.")));
+						errcode(ERRCODE_PROTOCOL_VIOLATION),
+						errmsg("malformed OAUTHBEARER message"),
+						errdetail("Message contains multiple auth values."));
 
 			auth = value;
 		}
@@ -438,9 +438,9 @@ parse_kvpairs_for_auth(char **input)
 	}
 
 	ereport(ERROR,
-			(errcode(ERRCODE_PROTOCOL_VIOLATION),
-			 errmsg("malformed OAUTHBEARER message"),
-			 errdetail("Message did not contain a final terminator.")));
+			errcode(ERRCODE_PROTOCOL_VIOLATION),
+			errmsg("malformed OAUTHBEARER message"),
+			errdetail("Message did not contain a final terminator."));
 
 	pg_unreachable();
 	return NULL;
@@ -461,9 +461,9 @@ generate_error_response(struct oauth_ctx *ctx, char **output, int *outputlen)
 	 */
 	if (!ctx->issuer || !ctx->scope)
 		ereport(FATAL,
-				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("OAuth is not properly configured for this user"),
-				 errdetail_log("The issuer and scope parameters must be set in pg_hba.conf.")));
+				errcode(ERRCODE_INTERNAL_ERROR),
+				errmsg("OAuth is not properly configured for this user"),
+				errdetail_log("The issuer and scope parameters must be set in pg_hba.conf."));
 
 	/*------
 	 * Build the .well-known URI based on our issuer.
@@ -603,6 +603,7 @@ validate(Port *port, const char *auth)
 	int			map_status;
 	ValidatorModuleResult *ret;
 	const char *token;
+	bool		status;
 
 	/* Ensure that we have a correct token to validate */
 	if (!(token = validate_token_format(auth)))
@@ -613,7 +614,10 @@ validate(Port *port, const char *auth)
 										  token, port->user_name);
 
 	if (!ret->authorized)
-		return false;
+	{
+		status = false;
+		goto cleanup;
+	}
 
 	if (ret->authn_id)
 		set_authn_id(port, ret->authn_id);
@@ -626,7 +630,8 @@ validate(Port *port, const char *auth)
 		 * validator implementation; all that matters is that the validator
 		 * says the user can log in with the target role.
 		 */
-		return true;
+		status = true;
+		goto cleanup;
 	}
 
 	/* Make sure the validator authenticated the user. */
@@ -642,9 +647,31 @@ validate(Port *port, const char *auth)
 	/* Finally, check the user map. */
 	map_status = check_usermap(port->hba->usermap, port->user_name,
 							   MyClientConnectionInfo.authn_id, false);
-	return (map_status == STATUS_OK);
+	status = (map_status == STATUS_OK);
+
+cleanup:
+	/*
+	 * Clear and free the validation result from the validator module once
+	 * we're done with it to avoid accidental re-use.
+	 */
+	if (ret->authn_id != NULL)
+	{
+		explicit_bzero(ret->authn_id, strlen(ret->authn_id));
+		pfree(ret->authn_id);
+	}
+	pfree(ret);
+
+	return status;
 }
 
+/*
+ * load_validator_library
+ *
+ * Load the configured validator library in order to perform token validation.
+ * There is no built-in fallback since validation is implementation specific. If
+ * no validator library is configured, or of it fails to load, then error out
+ * since token validation won't be possible.
+ */
 static void
 load_validator_library(void)
 {
@@ -652,20 +679,25 @@ load_validator_library(void)
 
 	if (OAuthValidatorLibrary[0] == '\0')
 		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("oauth_validator_library is not set")));
+				errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				errmsg("oauth_validator_library is not set"));
 
 	validator_init = (OAuthValidatorModuleInit)
 		load_external_function(OAuthValidatorLibrary,
 							   "_PG_oauth_validator_module_init", false, NULL);
 
+	/*
+	 * The validator init function is required since it will set the callbacks
+	 * for the validator library.
+	 */
 	if (validator_init == NULL)
 		ereport(ERROR,
-				(errmsg("%s module \"%s\" have to define the symbol %s",
-						"OAuth validator", OAuthValidatorLibrary, "_PG_oauth_validator_module_init")));
+				errmsg("%s modules \"%s\" have to define the symbol %s",
+					   "OAuth validator", OAuthValidatorLibrary, "_PG_oauth_validator_module_init"));
 
 	ValidatorCallbacks = (*validator_init) ();
 
+	/* Allocate memory for validator library private state data */
 	validator_module_state = (ValidatorModuleState *) palloc0(sizeof(ValidatorModuleState));
 	if (ValidatorCallbacks->startup_cb != NULL)
 		ValidatorCallbacks->startup_cb(validator_module_state);
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 735fd05373..dcb1558ad3 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2042,6 +2042,32 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel)
 		parsedline->clientcert = clientCertFull;
 	}
 
+	/*
+	 * Enforce proper configuration of OAuth authentication.
+	 */
+	if (parsedline->auth_method == uaOAuth)
+	{
+		MANDATORY_AUTH_ARG(parsedline->oauth_scope, "scope", "oauth");
+		MANDATORY_AUTH_ARG(parsedline->oauth_issuer, "issuer", "oauth");
+
+		/*
+		 * Supplying a usermap combined with the option to skip usermapping
+		 * is nonsensical and indicates a configuration error.
+		 */
+		if (parsedline->oauth_skip_usermap && parsedline->usermap != NULL)
+		{
+			ereport(elevel,
+					errcode(ERRCODE_CONFIG_FILE_ERROR),
+					/* translator: strings are replaced with hba options */
+					errmsg("%s cannot be used in combination with %s",
+						   "map", "trust_validator_authz"),
+					errcontext("line %d of configuration file \"%s\"",
+							   line_num, file_name));
+			*err_msg = "map cannot be used in combination with trust_validator_authz";
+			return NULL;
+		}
+	}
+
 	return parsedline;
 }
 
diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c
index 73718ac3b1..f5fc6ebc23 100644
--- a/src/interfaces/libpq/fe-auth-oauth.c
+++ b/src/interfaces/libpq/fe-auth-oauth.c
@@ -89,8 +89,11 @@ client_initial_response(PGconn *conn, const char *token)
 
 	if (!PQExpBufferDataBroken(buf))
 		response = strdup(buf.data);
-
 	termPQExpBuffer(&buf);
+
+	if (!response)
+		libpq_append_conn_error(conn, "out of memory");
+
 	return response;
 }
 
diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl
index 3b8f057a26..3f06f5be76 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -11,11 +11,18 @@ use PostgreSQL::Test::Utils;
 use PostgreSQL::Test::OAuthServer;
 use Test::More;
 
+if (!$ENV{PG_TEST_EXTRA} || $ENV{PG_TEST_EXTRA} !~ /\boauth\b/)
+{
+	plan skip_all =>
+	  'Potentially unsafe test oauth not enabled in PG_TEST_EXTRA';
+}
+
 if ($ENV{with_oauth} ne 'curl')
 {
 	plan skip_all => 'client-side OAuth not supported by this build';
 }
-elsif ($ENV{with_python} ne 'yes')
+
+if ($ENV{with_python} ne 'yes')
 {
 	plan skip_all => 'OAuth tests require --with-python to run';
 }
diff --git a/src/test/modules/oauth_validator/validator.c b/src/test/modules/oauth_validator/validator.c
index d12c79e2a2..dbba326bc4 100644
--- a/src/test/modules/oauth_validator/validator.c
+++ b/src/test/modules/oauth_validator/validator.c
@@ -67,7 +67,10 @@ validator_startup(ValidatorModuleState *state)
 static void
 validator_shutdown(ValidatorModuleState *state)
 {
-	/* do nothing */
+	/* Check to make sure our private state still exists. */
+	if (state->private_data != PRIVATE_COOKIE)
+		elog(ERROR, "oauth_validator: private state cookie changed to %p in shutdown",
+			 state->private_data);
 }
 
 static ValidatorModuleResult *
@@ -77,7 +80,7 @@ validate_token(ValidatorModuleState *state, const char *token, const char *role)
 
 	/* Check to make sure our private state still exists. */
 	if (state->private_data != PRIVATE_COOKIE)
-		elog(ERROR, "oauth_validator: private state cookie changed to %p",
+		elog(ERROR, "oauth_validator: private state cookie changed to %p in validate",
 			 state->private_data);
 
 	res = palloc(sizeof(ValidatorModuleResult));
-- 
2.39.3 (Apple Git-146)