public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jacob Champion <[email protected]>
To: Zsolt Parragi <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: Chao Li <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: Improve OAuth discovery logging
Date: Tue, 10 Mar 2026 14:52:38 -0700
Message-ID: <CAOYmi+kEYA0Tp2son-+Ti1wvSAPov87AVFf4qXATTOHRX1F2gg@mail.gmail.com> (raw)
In-Reply-To: <CAN4CZFO7ju7fjjv+qwObP8_V-Tdx463zV8F7u_s6wtg9ANVWVg@mail.gmail.com>
References: <CAN4CZFPim7hUiyb7daNKQPSZ8CvQRBGkVhbvED7yZi8VktSn4Q@mail.gmail.com>
<[email protected]>
<CAN4CZFNNfhFCQdFWui5HWbQR60eM-cyndZ7YgSv7b5SKxB9C2A@mail.gmail.com>
<CAOYmi+mDSmh6RNizHRmMAwg4ZP2W=uai3Fr3-wm186NMypf_Pg@mail.gmail.com>
<CAN4CZFNJftK8NaREYaLi-wqpEz3=crQ=1+3f_XUVji=aOrDSWA@mail.gmail.com>
<[email protected]>
<CAOYmi+kjtmRMBdBU3_bGKGDoRSK2AErXbGtHkAjFRapcQNmjhA@mail.gmail.com>
<CAN4CZFNWBXtF-ML3yzdOvX3QEuUwVo5VrBzyWU3O=y-7SeDstA@mail.gmail.com>
<[email protected]>
<CAN4CZFNscs=hiOkRJYF39r7AD7ef9+MR+O2BQdEtE_2Ajdo5qw@mail.gmail.com>
<CAOYmi+nVzkoLjzNk_58e0NnUPi9uVXwmurK2QP6CzC2WOpqwbg@mail.gmail.com>
<CAN4CZFPjiUQbKo2q+ovs--AHkjvaE8OJyncB9xu5b+1gp=HHPQ@mail.gmail.com>
<CAOYmi+=SR_nJJBh7UXZzK8Zbs21L2RUNkW3d9aPRkQOHj1bBPA@mail.gmail.com>
<CAN4CZFO7ju7fjjv+qwObP8_V-Tdx463zV8F7u_s6wtg9ANVWVg@mail.gmail.com>
On Thu, Mar 5, 2026 at 12:11 PM Zsolt Parragi <[email protected]> wrote:
> Attached v6 with the problematic log expectation removed.
Okay, I was doing some final pre-commit review today and...
unfortunately, using STATUS_EOF like my "TODO" suggested breaks our de
facto SASL profile. The server hasn't completed its side of the
exchange until it sends either
[AuthenticationSASLFinal+]AuthenticationOk or ErrorResponse. Since
STATUS_EOF suppresses not only the log message but the entire
ereport(FATAL), we'll never send that last packet, so a more polite
client can't tell whether the server finished the exchange or just
crashed.
v6 doesn't fail any tests because of a shortcut I took in
PQconnectPoll() in libpq, which skips reading the final message from a
known-doomed OAuth discovery connection. But you can see it if you
apply the attached patch. (It's not a correct patch; it just shows the
problem.)
I'm experimenting with an ereport(FATAL_CLIENT_ONLY) option, in the
same vein as WARNING_CLIENT_ONLY, to try to cover this.
--Jacob
P.S. I would eventually like to record our undocumented SASL profile
in a test suite (he said, staring at pg-pytest)...
Attachments:
[application/octet-stream] nocfbot.polite.diff (1.9K, 2-nocfbot.polite.diff)
download | inline diff:
commit f21c1b8aecc2a7e8baa1b1b004055eabb057403b
Author: Jacob Champion <[email protected]>
Date: Tue Mar 10 12:58:05 2026
XXX polite libpq
diff --git a/src/interfaces/libpq/fe-auth-oauth.c b/src/interfaces/libpq/fe-auth-oauth.c
index 2aef327c68b..562c8595c32 100644
--- a/src/interfaces/libpq/fe-auth-oauth.c
+++ b/src/interfaces/libpq/fe-auth-oauth.c
@@ -1400,7 +1400,7 @@ reconnect:
*/
libpq_append_conn_error(conn, "retrying connection with new bearer token");
conn->oauth_want_retry = true;
- return SASL_FAILED;
+ return SASL_COMPLETE;
}
static bool
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index db9b4c8edbf..151d4096f32 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -4130,6 +4130,17 @@ keep_going: /* We will come back to here until there is
/* Handle errors. */
if (beresp == PqMsg_ErrorResponse)
{
+ /*
+ * OAuth connections may perform two-step discovery, where
+ * the first connection is a dummy. We expect a failure
+ * message to end the SASL exchange from the server side.
+ */
+ if (conn->sasl == &pg_oauth_mech && conn->oauth_want_retry)
+ {
+ need_new_connection = true;
+ goto keep_going;
+ }
+
if (pqGetErrorNotice3(conn, true))
{
libpq_append_conn_error(conn, "received invalid error message");
@@ -4243,19 +4254,7 @@ keep_going: /* We will come back to here until there is
conn->inStart = conn->inCursor;
if (res != STATUS_OK)
- {
- /*
- * OAuth connections may perform two-step discovery, where
- * the first connection is a dummy.
- */
- if (conn->sasl == &pg_oauth_mech && conn->oauth_want_retry)
- {
- need_new_connection = true;
- goto keep_going;
- }
-
goto error_return;
- }
/*
* Just make sure that any data sent by pg_fe_sendauth is
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], [email protected], [email protected], [email protected]
Subject: Re: Improve OAuth discovery logging
In-Reply-To: <CAOYmi+kEYA0Tp2son-+Ti1wvSAPov87AVFf4qXATTOHRX1F2gg@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