public inbox for [email protected]
help / color / mirror / Atom feedFix OAuth validator docs for error_detail on internal errors
7+ messages / 3 participants
[nested] [flat]
* Fix OAuth validator docs for error_detail on internal errors
@ 2026-06-04 12:33 Chao Li <[email protected]>
2026-06-04 20:19 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Chao Li @ 2026-06-04 12:33 UTC (permalink / raw)
To: Postgres hackers <[email protected]>; +Cc: Jacob Champion <[email protected]>
Hi,
While testing “[d438a3659] oauth: Let validators provide failure DETAILs”, I noticed a tiny doc issue.
With this feature, when a validator returns false, result->error_detail can carry an error message. However, it seems that the previous paragraph was not updated:
```
<para>
A validator may return <literal>false</literal> to signal an internal error,
in which case any result parameters are ignored and the connection fails.
```
“Any result parameters are ignored” is no longer accurate; it should be something like “any result parameters except result->error_detail are ignored”. This patch just makes that tiny doc fix.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-doc-Clarify-OAuth-validator-error_detail-handling.patch (1.5K, 2-v1-0001-doc-Clarify-OAuth-validator-error_detail-handling.patch)
download | inline diff:
From 7c70a33a07856df88071ac7991ced1e99fad0dea Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Thu, 4 Jun 2026 19:23:59 +0800
Subject: [PATCH v1] doc: Clarify OAuth validator error_detail handling
ValidatorModuleResult.error_detail is still consulted when validate_cb
returns false to signal an internal error. Do not describe all result
parameters as ignored in that case.
Author: Chao Li <[email protected]>
---
doc/src/sgml/oauth-validators.sgml | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml
index 8aad470a464..7015664f26c 100644
--- a/doc/src/sgml/oauth-validators.sgml
+++ b/doc/src/sgml/oauth-validators.sgml
@@ -399,9 +399,10 @@ typedef struct ValidatorModuleResult
</para>
<para>
A validator may return <literal>false</literal> to signal an internal error,
- in which case any result parameters are ignored and the connection fails.
- Otherwise the validator should return <literal>true</literal> to indicate
- that it has processed the token and made an authorization decision.
+ in which case the connection fails. Except for
+ <structfield>result->error_detail</structfield>, all result parameters are
+ ignored. Otherwise the validator should return <literal>true</literal> to
+ indicate that it has processed the token and made an authorization decision.
</para>
<para>
In either failure case (validation error or internal error) the module may
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix OAuth validator docs for error_detail on internal errors
2026-06-04 12:33 Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
@ 2026-06-04 20:19 ` Daniel Gustafsson <[email protected]>
2026-06-04 22:21 ` Re: Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Gustafsson @ 2026-06-04 20:19 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: Postgres hackers <[email protected]>; Jacob Champion <[email protected]>
> On 4 Jun 2026, at 14:33, Chao Li <[email protected]> wrote:
> “Any result parameters are ignored” is no longer accurate; it should be something like “any result parameters except result->error_detail are ignored”. This patch just makes that tiny doc fix.
That's true, but error_detail is explained in detail in the next paragraph so
I'm not sure this change is needed.
Another thing we don't explicitly document which seems more interesting is that
authn_id is used even in case of failure if log_connections is enabled. Maybe
that deserves a mention?
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix OAuth validator docs for error_detail on internal errors
2026-06-04 12:33 Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
2026-06-04 20:19 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
@ 2026-06-04 22:21 ` Chao Li <[email protected]>
2026-06-05 19:31 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Chao Li @ 2026-06-04 22:21 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Postgres hackers <[email protected]>; Jacob Champion <[email protected]>
> On Jun 5, 2026, at 04:19, Daniel Gustafsson <[email protected]> wrote:
>
>> On 4 Jun 2026, at 14:33, Chao Li <[email protected]> wrote:
>
>> “Any result parameters are ignored” is no longer accurate; it should be something like “any result parameters except result->error_detail are ignored”. This patch just makes that tiny doc fix.
>
> That's true, but error_detail is explained in detail in the next paragraph so
> I'm not sure this change is needed.
Agreed. Adding the “exception for result->error_detail” sounds a bit redundant with the next paragraph. But “any result parameters are ignored” also seems to conflict with the next paragraph, so I think we can just delete that part.
ValidatorModuleResult has three fields, so the logic is:
* The first paragraph talks about authorized and authn_id when the validator succeeds.
* The second paragraph talks about the validator’s return values.
* The third paragraph talks about result->error_detail when the validator fails.
>
> Another thing we don't explicitly document which seems more interesting is that
> authn_id is used even in case of failure if log_connections is enabled. Maybe
> that deserves a mention?
>
This is a good point. I added that in v2.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v2-0001-doc-clarify-OAuth-validator-authn_id-logging-on-a.patch (2.5K, 2-v2-0001-doc-clarify-OAuth-validator-authn_id-logging-on-a.patch)
download | inline diff:
From 475fc04a7b9d3d388c089126a95686c867b0768e Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Thu, 4 Jun 2026 19:23:59 +0800
Subject: [PATCH v2] doc: clarify OAuth validator authn_id logging on auth
failure
OAuth validators can return an authenticated identity in
ValidatorModuleResult.authn_id. The server records this value before
checking whether the connection is authorized, so it may appear in
connection-authentication logs even when the connection is later rejected.
Also remove outdated wording saying that all result parameters are ignored
when a validator returns false. Validators may still provide error_detail for
both validation failures and internal errors, as described in the following
paragraph.
Author: Chao Li <[email protected]>
Reported-by: Daniel Gustafsson <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
doc/src/sgml/oauth-validators.sgml | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/oauth-validators.sgml b/doc/src/sgml/oauth-validators.sgml
index 8aad470a464..245f3ebb95e 100644
--- a/doc/src/sgml/oauth-validators.sgml
+++ b/doc/src/sgml/oauth-validators.sgml
@@ -395,13 +395,18 @@ typedef struct ValidatorModuleResult
token) shall be palloc'd and returned in the <structfield>result->authn_id</structfield>
field. Alternatively, <structfield>result->authn_id</structfield> may be set to
NULL if the token is valid but the associated user identity cannot be
- determined.
+ determined. If the validator returns <literal>true</literal> and
+ <structfield>result->authn_id</structfield> is set, the server records it
+ before checking whether the connection is authorized, so it may appear in
+ the server log when <xref linkend="guc-log-connections"/> includes
+ <literal>authentication</literal>, even when the connection is later
+ rejected.
</para>
<para>
A validator may return <literal>false</literal> to signal an internal error,
- in which case any result parameters are ignored and the connection fails.
- Otherwise the validator should return <literal>true</literal> to indicate
- that it has processed the token and made an authorization decision.
+ in which case the connection fails. Otherwise the validator should return
+ <literal>true</literal> to indicate that it has processed the token and made
+ an authorization decision.
</para>
<para>
In either failure case (validation error or internal error) the module may
--
2.50.1 (Apple Git-155)
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix OAuth validator docs for error_detail on internal errors
2026-06-04 12:33 Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
2026-06-04 20:19 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
2026-06-04 22:21 ` Re: Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
@ 2026-06-05 19:31 ` Daniel Gustafsson <[email protected]>
2026-06-05 21:16 ` Re: Fix OAuth validator docs for error_detail on internal errors Jacob Champion <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Gustafsson @ 2026-06-05 19:31 UTC (permalink / raw)
To: Chao Li <[email protected]>; +Cc: Postgres hackers <[email protected]>; Jacob Champion <[email protected]>
> On 5 Jun 2026, at 00:21, Chao Li <[email protected]> wrote:
>> On Jun 5, 2026, at 04:19, Daniel Gustafsson <[email protected]> wrote:
>>
>>> On 4 Jun 2026, at 14:33, Chao Li <[email protected]> wrote:
>>
>>> “Any result parameters are ignored” is no longer accurate; it should be something like “any result parameters except result->error_detail are ignored”. This patch just makes that tiny doc fix.
>>
>> That's true, but error_detail is explained in detail in the next paragraph so
>> I'm not sure this change is needed.
>
> Agreed. Adding the “exception for result->error_detail” sounds a bit redundant with the next paragraph. But “any result parameters are ignored” also seems to conflict with the next paragraph, so I think we can just delete that part.
>
> ValidatorModuleResult has three fields, so the logic is:
>
> * The first paragraph talks about authorized and authn_id when the validator succeeds.
> * The second paragraph talks about the validator’s return values.
> * The third paragraph talks about result->error_detail when the validator fails.
>
>> Another thing we don't explicitly document which seems more interesting is that
>> authn_id is used even in case of failure if log_connections is enabled. Maybe
>> that deserves a mention?
>
> This is a good point. I added that in v2.
This version looks good to me, the authn_id sentence is a bit long so I might
do some careful rewording before pushing.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix OAuth validator docs for error_detail on internal errors
2026-06-04 12:33 Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
2026-06-04 20:19 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
2026-06-04 22:21 ` Re: Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
2026-06-05 19:31 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
@ 2026-06-05 21:16 ` Jacob Champion <[email protected]>
2026-06-05 22:22 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Jacob Champion @ 2026-06-05 21:16 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Chao Li <[email protected]>; Postgres hackers <[email protected]>
On Fri, Jun 5, 2026 at 12:32 PM Daniel Gustafsson <[email protected]> wrote:
> This version looks good to me, the authn_id sentence is a bit long so I might
> do some careful rewording before pushing.
+1. I'll also think about how to better document the (intentional)
recording of authn_id during failed authorization, but it doesn't need
to block this.
--Jacob
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix OAuth validator docs for error_detail on internal errors
2026-06-04 12:33 Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
2026-06-04 20:19 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
2026-06-04 22:21 ` Re: Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
2026-06-05 19:31 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
2026-06-05 21:16 ` Re: Fix OAuth validator docs for error_detail on internal errors Jacob Champion <[email protected]>
@ 2026-06-05 22:22 ` Daniel Gustafsson <[email protected]>
2026-06-06 00:16 ` Re: Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Gustafsson @ 2026-06-05 22:22 UTC (permalink / raw)
To: Jacob Champion <[email protected]>; +Cc: Chao Li <[email protected]>; Postgres hackers <[email protected]>
> On 5 Jun 2026, at 23:16, Jacob Champion <[email protected]> wrote:
>
> On Fri, Jun 5, 2026 at 12:32 PM Daniel Gustafsson <[email protected]> wrote:
>> This version looks good to me, the authn_id sentence is a bit long so I might
>> do some careful rewording before pushing.
>
> +1. I'll also think about how to better document the (intentional)
> recording of authn_id during failed authorization, but it doesn't need
> to block this.
Pushed with a little bit of wordsmithing.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Fix OAuth validator docs for error_detail on internal errors
2026-06-04 12:33 Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
2026-06-04 20:19 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
2026-06-04 22:21 ` Re: Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
2026-06-05 19:31 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
2026-06-05 21:16 ` Re: Fix OAuth validator docs for error_detail on internal errors Jacob Champion <[email protected]>
2026-06-05 22:22 ` Re: Fix OAuth validator docs for error_detail on internal errors Daniel Gustafsson <[email protected]>
@ 2026-06-06 00:16 ` Chao Li <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Chao Li @ 2026-06-06 00:16 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Jacob Champion <[email protected]>; Postgres hackers <[email protected]>
> On Jun 6, 2026, at 06:22, Daniel Gustafsson <[email protected]> wrote:
>
>> On 5 Jun 2026, at 23:16, Jacob Champion <[email protected]> wrote:
>>
>> On Fri, Jun 5, 2026 at 12:32 PM Daniel Gustafsson <[email protected]> wrote:
>>> This version looks good to me, the authn_id sentence is a bit long so I might
>>> do some careful rewording before pushing.
>>
>> +1. I'll also think about how to better document the (intentional)
>> recording of authn_id during failed authorization, but it doesn't need
>> to block this.
>
> Pushed with a little bit of wordsmithing.
>
> --
> Daniel Gustafsson
>
Hi Daniel, thank you very much for taking care of this patch.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-06-06 00:16 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-06-04 12:33 Fix OAuth validator docs for error_detail on internal errors Chao Li <[email protected]>
2026-06-04 20:19 ` Daniel Gustafsson <[email protected]>
2026-06-04 22:21 ` Chao Li <[email protected]>
2026-06-05 19:31 ` Daniel Gustafsson <[email protected]>
2026-06-05 21:16 ` Jacob Champion <[email protected]>
2026-06-05 22:22 ` Daniel Gustafsson <[email protected]>
2026-06-06 00:16 ` Chao Li <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox