public inbox for [email protected]
help / color / mirror / Atom feedFrom: Zsolt Parragi <[email protected]>
To: Jacob Champion <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Michael Paquier <[email protected]>
Subject: Re: Improve OAuth discovery logging
Date: Fri, 13 Feb 2026 13:13:26 +0000
Message-ID: <CAN4CZFNJftK8NaREYaLi-wqpEz3=crQ=1+3f_XUVji=aOrDSWA@mail.gmail.com> (raw)
In-Reply-To: <CAOYmi+mDSmh6RNizHRmMAwg4ZP2W=uai3Fr3-wm186NMypf_Pg@mail.gmail.com>
References: <CAN4CZFPim7hUiyb7daNKQPSZ8CvQRBGkVhbvED7yZi8VktSn4Q@mail.gmail.com>
<[email protected]>
<CAN4CZFNNfhFCQdFWui5HWbQR60eM-cyndZ7YgSv7b5SKxB9C2A@mail.gmail.com>
<CAOYmi+mDSmh6RNizHRmMAwg4ZP2W=uai3Fr3-wm186NMypf_Pg@mail.gmail.com>
These all are good suggestions, attached updated patch.
> Maybe something like PG_SASL_EXCHANGE_ABANDONED?
This is the only one I wasn't sure of, I used RESTART because I was
focusing more on the intention of the server ("please restart
authentication with this additional information"), and a bit also on
the idea that later restart could stay even within the same
connection, both in this case and if we add support for
reauthentication on token expiration.
On the other hand I'm not 100% sure how the other two would work, and
ABANDONED is a better description for the current situation, so I
adjusted the patch to use that.
Attachments:
[application/octet-stream] v3-0001-Improve-OAuth-discovery-logging.patch (6.7K, 2-v3-0001-Improve-OAuth-discovery-logging.patch)
download | inline diff:
From e9b16515ea542cb5ee0e42cf5c5a1c7c246df081 Mon Sep 17 00:00:00 2001
From: Zsolt Parragi <[email protected]>
Date: Wed, 11 Feb 2026 19:28:05 +0100
Subject: [PATCH v3] Improve OAuth discovery logging
Currently when the client sends an empty OAuth token to request the
issuer URL, the server logs the attempt with
FATAL: OAuth bearer authentication failed for user
Which is quite confusing, as this is an expected part of the OAuth
authentication flow and not an error at all.
This in practice results in the server spamming the log with these
messages, which are difficult to separate from real (OAuth)
authentication failures.
This patch improves this by handling the situation properly in the
SASL/Oauth code, by introducing a new SASL authentication status,
PG_SASL_EXCHANGE_RESTART. The expectation is that authentication
mechanisms can set this if they request a restart of the authentication
flow. Restart currently requires starting with a new connection, so this
simply sets STATUS_EOF.
The above prevents logging a fatal error at the end, so instead the
OAuth exchange code outputs a simple log message instead.
---
src/backend/libpq/auth-oauth.c | 39 ++++++++++++-------
src/backend/libpq/auth-sasl.c | 14 ++++---
src/include/libpq/sasl.h | 1 +
.../modules/oauth_validator/t/001_server.pl | 8 +++-
4 files changed, 40 insertions(+), 22 deletions(-)
diff --git a/src/backend/libpq/auth-oauth.c b/src/backend/libpq/auth-oauth.c
index 11365048951..3790d1686ee 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
@@ -193,6 +195,14 @@ oauth_exchange(void *opaq, const char *input, int inputlen,
errdetail("Client did not send a kvsep response."));
/* The (failed) handshake is now complete. */
+ if (ctx->state == OAUTH_STATE_ERROR_DISCOVERY)
+ {
+ ctx->state = OAUTH_STATE_FINISHED;
+ ereport(DEBUG1,
+ errmsg("OAuth issuer discovery requested"));
+ return PG_SASL_EXCHANGE_ABANDONED;
+ }
+
ctx->state = OAUTH_STATE_FINISHED;
return PG_SASL_EXCHANGE_FAILURE;
@@ -279,7 +289,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 +586,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/backend/libpq/auth-sasl.c b/src/backend/libpq/auth-sasl.c
index 36cb748d927..62bb06b579a 100644
--- a/src/backend/libpq/auth-sasl.c
+++ b/src/backend/libpq/auth-sasl.c
@@ -167,7 +167,7 @@ CheckSASLAuth(const pg_be_sasl_mech *mech, Port *port, char *shadow_pass,
* PG_SASL_EXCHANGE_FAILURE with some output is forbidden by SASL.
* Make sure here that the mechanism used got that right.
*/
- if (result == PG_SASL_EXCHANGE_FAILURE)
+ if (result == PG_SASL_EXCHANGE_FAILURE || result == PG_SASL_EXCHANGE_ABANDONED)
elog(ERROR, "output message found after SASL exchange failure");
/*
@@ -184,11 +184,13 @@ CheckSASLAuth(const pg_be_sasl_mech *mech, Port *port, char *shadow_pass,
}
} while (result == PG_SASL_EXCHANGE_CONTINUE);
- /* Oops, Something bad happened */
- if (result != PG_SASL_EXCHANGE_SUCCESS)
+ switch (result)
{
- return STATUS_ERROR;
+ case PG_SASL_EXCHANGE_SUCCESS:
+ return STATUS_OK;
+ case PG_SASL_EXCHANGE_ABANDONED:
+ return STATUS_EOF;
+ default:
+ return STATUS_ERROR;
}
-
- return STATUS_OK;
}
diff --git a/src/include/libpq/sasl.h b/src/include/libpq/sasl.h
index 1e8ec7d6293..31a97747fed 100644
--- a/src/include/libpq/sasl.h
+++ b/src/include/libpq/sasl.h
@@ -25,6 +25,7 @@
#define PG_SASL_EXCHANGE_CONTINUE 0
#define PG_SASL_EXCHANGE_SUCCESS 1
#define PG_SASL_EXCHANGE_FAILURE 2
+#define PG_SASL_EXCHANGE_ABANDONED 3
/*
* Maximum accepted size of SASL messages.
diff --git a/src/test/modules/oauth_validator/t/001_server.pl b/src/test/modules/oauth_validator/t/001_server.pl
index 6b649c0b06f..9f2f9f49077 100644
--- a/src/test/modules/oauth_validator/t/001_server.pl
+++ b/src/test/modules/oauth_validator/t/001_server.pl
@@ -114,11 +114,13 @@ $node->connect_ok(
expected_stderr =>
qr@Visit https://example\.com/ and enter the code: postgresuser@,
log_like => [
+ qr/OAuth issuer discovery requested/,
qr/oauth_validator: token="9243959234", role="$user"/,
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.
@@ -129,11 +131,13 @@ $node->connect_ok(
expected_stderr =>
qr@Visit https://example\.org/ and enter the code: postgresuser@,
log_like => [
+ qr/OAuth issuer discovery requested/,
qr/oauth_validator: token="9243959234-alt", role="$user"/,
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.43.0
view thread (26+ 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: Improve OAuth discovery logging
In-Reply-To: <CAN4CZFNJftK8NaREYaLi-wqpEz3=crQ=1+3f_XUVji=aOrDSWA@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