v47-0002-fixup-Add-OAUTHBEARER-SASL-mechanism.patch

From 483129c1ca931f60b76ef93bf2f646cea2f00568 Mon Sep 17 00:00:00 2001
From: Jacob Champion <jacob.champion@enterprisedb.com>
Date: Thu, 6 Feb 2025 13:16:04 -0800
Subject: [PATCH v47 02/11] fixup! Add OAUTHBEARER SASL mechanism

---
 src/interfaces/libpq/fe-auth-oauth-curl.c | 35 +++++++++++++++++------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/src/interfaces/libpq/fe-auth-oauth-curl.c b/src/interfaces/libpq/fe-auth-oauth-curl.c
index 2407200ea97..eeddace7060 100644
--- a/src/interfaces/libpq/fe-auth-oauth-curl.c
+++ b/src/interfaces/libpq/fe-auth-oauth-curl.c
@@ -973,11 +973,20 @@ parse_access_token(struct async_ctx *actx, struct token *tok)
 		{"access_token", JSON_TOKEN_STRING, {&tok->access_token}, REQUIRED},
 		{"token_type", JSON_TOKEN_STRING, {&tok->token_type}, REQUIRED},
 
-		/*
-		 * The following fields are technically REQUIRED, but we don't use
-		 * them anywhere yet:
+		/*---
+		 * We currently have no use for the following OPTIONAL fields:
+		 *
+		 * - expires_in: This will be important for maintaining a token cache,
+		 *               but we do not yet implement one.
 		 *
-		 * - scope (only required if different than requested -- TODO check)
+		 * - refresh_token: Ditto.
+		 *
+		 * - scope: This is only sent when the authorization server sees fit to
+		 *          change our scope request. It's not clear what we should do
+		 *          about this; either it's been done as a matter of policy, or
+		 *          the user has explicitly denied part of the authorization,
+		 *          and either way the server-side validator is in a better
+		 *          place to complain if the change isn't acceptable.
 		 */
 
 		{0},
@@ -1252,8 +1261,11 @@ register_timer(CURLM *curlm, long timeout, void *ctx)
 	struct async_ctx *actx = ctx;
 
 	/*
-	 * TODO: maybe just signal drive_request() to immediately call back in the
-	 * (timeout == 0) case?
+	 * There might be an optimization opportunity here: if timeout == 0, we
+	 * could signal drive_request to immediately call
+	 * curl_multi_socket_action, rather than returning all the way up the
+	 * stack only to come right back. But it's not clear that the additional
+	 * code complexity is worth it.
 	 */
 	if (!set_timer(actx, timeout))
 		return -1;				/* actx_error already called */
@@ -1415,7 +1427,14 @@ setup_curl_handles(struct async_ctx *actx)
 		CHECK_SETOPT(actx, popt, protos, return false);
 	}
 
-	/* TODO: would anyone use this in "real" situations, or just testing? */
+	/*
+	 * If we're in debug mode, allow the developer to change the trusted CA
+	 * list. For now, this is not something we expose outside of the UNSAFE
+	 * mode, because it's not clear that it's useful in production: both libpq
+	 * and the user's browser must trust the same authorization servers for
+	 * the flow to work at all, so any changes to the roots are likely to be
+	 * done system-wide.
+	 */
 	if (actx->debugging)
 	{
 		const char *env;
@@ -1824,8 +1843,6 @@ check_issuer(struct async_ctx *actx, PGconn *conn)
 	 *    of the authorization server where the authorization request was
 	 *    sent to. This comparison MUST use simple string comparison as defined
 	 *    in Section 6.2.1 of [RFC3986].
-	 *
-	 * TODO: Encoding support?
 	 */
 	if (strcmp(conn->oauth_issuer_id, provider->issuer) != 0)
 	{
-- 
2.34.1