| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Cc: | Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz> |
| Subject: | Re: Improve OAuth discovery logging |
| Date: | 2026-02-24 03:00:23 |
| Message-ID: | 7094F798-8DD1-4974-9A04-10E147B29581@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Feb 13, 2026, at 21:13, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
>
> 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.
> <v3-0001-Improve-OAuth-discovery-logging.patch>
Hi Zsolt,
Thanks for the patch. A few small comments:
1 - commit message
```
SASL/Oauth code, by introducing a new SASL authentication status,
PG_SASL_EXCHANGE_RESTART. The expectation is that authentication
```
Looks like you forgot to update the commit message to change PG_SASL_EXCHANGE_RESTART to PG_SASL_EXCHANGE_ABANDONED.
2 - auth-oauth.c
```
/* 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;
```
"ctx->state = OAUTH_STATE_FINISHED;" is duplicated in the “if” and after the “if”, so it can be pull up to before the “if”.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2026-02-24 03:11:08 | Re: DOCS - pg_walsummary typo? |
| Previous Message | Tatsuo Ishii | 2026-02-24 02:56:25 | Re: Row pattern recognition |