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

---
 doc/src/sgml/oauth-validators.sgml | 36 ++++++++++++++++++++++++++++--
 src/backend/libpq/auth-oauth.c     | 25 ++++++++++++++++++---
 2 files changed, 56 insertions(+), 5 deletions(-)

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/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index 05d86bb46a..90e68dbc93 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -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,7 +647,21 @@ 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;
 }
 
 /*
-- 
2.34.1