v8-0003-oauth-Don-t-log-discovery-connections-by-default.patch

application/octet-stream

Filename: v8-0003-oauth-Don-t-log-discovery-connections-by-default.patch
Type: application/octet-stream
Part: 1
Message: Re: Improve OAuth discovery logging

Patch

Format: format-patch
Series: patch v8-0003
Subject: oauth: Don't log discovery connections by default
File+
src/backend/libpq/auth-oauth.c 30 15
src/test/modules/oauth_validator/t/001_server.pl 4 2
From 85d703f5f8ac331c384ad0b54fecd0475e14de47 Mon Sep 17 00:00:00 2001
From: Jacob Champion <jacob.champion@enterprisedb.com>
Date: Mon, 16 Mar 2026 17:01:05 -0700
Subject: [PATCH v8 3/3] oauth: Don't log discovery connections by default

Currently, when the client sends a parameter discovery request within
OAUTHBEARER, the server logs the attempt with

    FATAL:  OAuth bearer authentication failed for user

These log entries are difficult to distinguish from true authentication
failures, and by default, libpq sends a discovery request as part of
every OAuth connection, making them annoyingly noisy. Use the new
PG_SASL_EXCHANGE_ABANDONED status to suppress them.

Author: Zsolt Parragi <zsolt.parragi@percona.com>
---
 src/backend/libpq/auth-oauth.c                | 45 ++++++++++++-------
 .../modules/oauth_validator/t/001_server.pl   |  6 ++-
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index 11365048951..894efe3c904 100644
--- a/src/backend/libpq/auth-oauth.c
+++ b/src/backend/libpq/auth-oauth.c
@@ -58,6 +58,7 @@ enum oauth_state
 {
 	OAUTH_STATE_INIT = 0,
 	OAUTH_STATE_ERROR,
+	OAUTH_STATE_ERROR_DISCOVERY,
 	OAUTH_STATE_FINISHED,
 };
 
@@ -181,6 +182,7 @@ oauth_exchange(void *opaq, const char *input, int inputlen,
 			break;
 
 		case OAUTH_STATE_ERROR:
+		case OAUTH_STATE_ERROR_DISCOVERY:
 
 			/*
 			 * Only one response is valid for the client during authentication
@@ -192,7 +194,19 @@ oauth_exchange(void *opaq, const char *input, int inputlen,
 						errmsg("malformed OAUTHBEARER message"),
 						errdetail("Client did not send a kvsep response."));
 
-			/* The (failed) handshake is now complete. */
+			/*
+			 * The (failed) handshake is now complete. Don't report discovery
+			 * requests in the server log unless the log level is high enough.
+			 */
+			if (ctx->state == OAUTH_STATE_ERROR_DISCOVERY)
+			{
+				ereport(DEBUG1, errmsg("OAuth issuer discovery requested"));
+
+				ctx->state = OAUTH_STATE_FINISHED;
+				return PG_SASL_EXCHANGE_ABANDONED;
+			}
+
+			/* We're not in discovery, so this is just a normal auth failure. */
 			ctx->state = OAUTH_STATE_FINISHED;
 			return PG_SASL_EXCHANGE_FAILURE;
 
@@ -279,7 +293,19 @@ oauth_exchange(void *opaq, const char *input, int inputlen,
 				errmsg("malformed OAUTHBEARER message"),
 				errdetail("Message contains additional data after the final terminator."));
 
-	if (!validate(ctx->port, auth))
+	if (auth[0] == '\0')
+	{
+		/*
+		 * An empty auth value represents a discovery request; the client
+		 * expects it to fail.  Skip validation entirely and move directly to
+		 * the error response.
+		 */
+		generate_error_response(ctx, output, outputlen);
+
+		ctx->state = OAUTH_STATE_ERROR_DISCOVERY;
+		status = PG_SASL_EXCHANGE_CONTINUE;
+	}
+	else if (!validate(ctx->port, auth))
 	{
 		generate_error_response(ctx, output, outputlen);
 
@@ -564,19 +590,8 @@ validate_token_format(const char *header)
 
 	/* Missing auth headers should be handled by the caller. */
 	Assert(header);
-
-	if (header[0] == '\0')
-	{
-		/*
-		 * A completely empty auth header represents a query for
-		 * authentication parameters. The client expects it to fail; there's
-		 * no need to make any extra noise in the logs.
-		 *
-		 * TODO: should we find a way to return STATUS_EOF at the top level,
-		 * to suppress the authentication error entirely?
-		 */
-		return NULL;
-	}
+	/* Empty auth (discovery) should be handled before calling validate(). */
+	Assert(header[0] != '\0');
 
 	if (pg_strncasecmp(header, BEARER_SCHEME, strlen(BEARER_SCHEME)))
 	{
diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl
index cdad2ae8011..f5dc427cdd1 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -151,7 +151,8 @@ $node->connect_ok(
 		qr/oauth_validator: issuer="\Q$issuer\E", scope="openid postgres"/,
 		qr/connection authenticated: identity="test" method=oauth/,
 		qr/connection authorized/,
-	]);
+	],
+	log_unlike => [qr/FATAL.*OAuth bearer authentication failed/]);
 
 # The /alternate issuer uses slightly different parameters, along with an
 # OAuth-style discovery document.
@@ -166,7 +167,8 @@ $node->connect_ok(
 		qr|oauth_validator: issuer="\Q$issuer/.well-known/oauth-authorization-server/alternate\E", scope="openid postgres alt"|,
 		qr/connection authenticated: identity="testalt" method=oauth/,
 		qr/connection authorized/,
-	]);
+	],
+	log_unlike => [qr/FATAL.*OAuth bearer authentication failed/]);
 
 # The issuer linked by the server must match the client's oauth_issuer setting.
 $node->connect_fails(
-- 
2.34.1