Horton

Thread

Commits

  1. doc: Clarify OAuth validator authn_id logging

  1. Fix OAuth validator docs for error_detail on internal errors

    Chao Li <li.evan.chao@gmail.com> — 2026-06-04T12:33:39Z

    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/

  2. Re: Fix OAuth validator docs for error_detail on internal errors

    Daniel Gustafsson <daniel@yesql.se> — 2026-06-04T20:19:30Z

    > On 4 Jun 2026, at 14:33, Chao Li <li.evan.chao@gmail.com> 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

  3. Re: Fix OAuth validator docs for error_detail on internal errors

    Chao Li <li.evan.chao@gmail.com> — 2026-06-04T22:21:07Z

    
> On Jun 5, 2026, at 04:19, Daniel Gustafsson <daniel@yesql.se> wrote:
> 
>> On 4 Jun 2026, at 14:33, Chao Li <li.evan.chao@gmail.com> 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/

  4. Re: Fix OAuth validator docs for error_detail on internal errors

    Daniel Gustafsson <daniel@yesql.se> — 2026-06-05T19:31:50Z

    > On 5 Jun 2026, at 00:21, Chao Li <li.evan.chao@gmail.com> wrote:
>> On Jun 5, 2026, at 04:19, Daniel Gustafsson <daniel@yesql.se> wrote:
>> 
>>> On 4 Jun 2026, at 14:33, Chao Li <li.evan.chao@gmail.com> 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

  5. Re: Fix OAuth validator docs for error_detail on internal errors

    Jacob Champion <jacob.champion@enterprisedb.com> — 2026-06-05T21:16:20Z

    On Fri, Jun 5, 2026 at 12:32 PM Daniel Gustafsson <daniel@yesql.se> 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

  6. Re: Fix OAuth validator docs for error_detail on internal errors

    Daniel Gustafsson <daniel@yesql.se> — 2026-06-05T22:22:38Z

    > On 5 Jun 2026, at 23:16, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
> 
> On Fri, Jun 5, 2026 at 12:32 PM Daniel Gustafsson <daniel@yesql.se> 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

  7. Re: Fix OAuth validator docs for error_detail on internal errors

    Chao Li <li.evan.chao@gmail.com> — 2026-06-06T00:16:02Z

    
> On Jun 6, 2026, at 06:22, Daniel Gustafsson <daniel@yesql.se> wrote:
> 
>> On 5 Jun 2026, at 23:16, Jacob Champion <jacob.champion@enterprisedb.com> wrote:
>> 
>> On Fri, Jun 5, 2026 at 12:32 PM Daniel Gustafsson <daniel@yesql.se> 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/