Subject: Re: Custom oauth validator options

From: Jacob Champion <[email-masked]>
Date: Fri, 20 Mar 2026 10:46:46 -0700

Hi all,

First, an apology for the state of this thread -- I thought I had
already responded to the prior message last month. Turns out, I had
not... Instead, I'm dropping a surprise alternative patchset on top of
a thread that I let go cold, which is rude, and I'm sorry.

Here are my overall thoughts on the approaches shared so far. And
thank you, Zsolt, for doing so much legwork here; that effort was not
wasted at all.

1) A GUC-centric solution -- option (d) -- is the Ideal Solution here,
IMO. We shouldn't have to reinvent the wheel.

2) A GUC-centric solution is not going to land for 19, and I'd be
surprised if it landed for 20, given the coalition that I'll need to
build to say either "yes" or "no". There are too many questions about
session_preload_libraries and prefixing and etc., and I'm honestly not
a fan of the guc_prefix_enforcement approach.

3) I'm not convinced yet that GUCs and relation options are similar
enough to share a framework. (This is not a rejection of the reloption
refactoring work, just a statement that I don't want to rely on it to
solve this problem.)

4) I really don't like the hba_parse_option_hook. I prefer APIs over
hooks as a general architectural point, and more practically, I don't
want to hand control to extensions during HBA parsing. I don't think
they're going to coordinate with each other well, and I think they're
likely to couple against internals in ways we don't want to support
(which is my general problem with hooks).

= Option (b) =

I don't want this problem to go unfixed for 2+ years, so I think it
would be best to reinvent a very small wheel that doesn't take up a
lot of maintenance effort once it's in, and then simply replace it
with the Ideal Solution eventually. This is my take on option (b),
which is what Vasuki M advocated for upthread. It's just a
setter/getter API for string keys and values:

  static const char* opts[] = { "my_parameter" };

  void startup_cb(ValidatorModuleState *state)
  {
      RegisterOAuthHBAOptions(state, lengthof(opts), opts);
  }

  bool validate_cb(const ValidatorModuleState *state, ...)
  {
      const char *param;
      if ((param = GetOAuthHBAOption(state, "my_parameter")) != NULL)
          do_something_with(param);
      ...
  }

And then in your HBA:

  host all all ::1/128 oauth validator.my_parameter=foo ...

The core implementation is conceptually simple. Most of the lines in
the patch are guardrails, to reduce the probability of regret over
this temporary solution:
- Parameter names aren't freeform; they're restricted to
almost-alphanumeric identifiers. (We've wanted to steal symbols for
HBA features in the past.)
- Name syntax is checked on reload, but cross-referencing the
registered list must be deferred to connection time. That's unusual
for users, so the WARNING that gets printed in this case is extremely
verbose; that way hopefully no one will be confused about what is
happening.
- The compiler won't let you register names during validate_cb, and we
won't let you retrieve options during startup_cb, so we retain control
over the internal order of operations. (This is discussed more deeply
in the patch.)

I was worried that we'd need a third API call to report option parsing
failures from the validator. Instead of doing that, I've made it
easier to link an authentication failure to a validator internal error
that caused it, in v3-0001, which serendipitously fixes a separate
sharp edge [1]. I feel good about that patch even if -0002 doesn't
make it.

v3-0002 still lacks user documentation, which I need to write a lot of
-- but if everyone dislikes this approach then I'd rather not spend
the time there.

= WDYT? =

I realize this puts you all in a difficult position; the effect is
kind of "take it or leave it", which again was not my intent. I
considered letting this lapse for 19 instead. But since I believe the
ideal solution is one we can't have for a while, and there's good
research and discussion of alternatives in this thread, waiting may
not produce a more committable short-term solution in the end.

Let me know if any of you disagree, though -- especially if you think
the status quo is preferable! -- and I can shelve -0002 for now. (I'll
continue with -0001 at [1] either way.)

Thanks,
--Jacob

[1] https://postgr.es/m/202601241015.y5uvxd7oxnfs%40alvherre.pgsql

---

From: Zsolt Parragi <[email-masked]>
Date: Mon, 23 Mar 2026 21:45:38 +0000

> I considered letting this lapse for 19 instead

That was also my conclusion. After the discussion in the SNI thread I
started working on a PoC for a more modern syntax for hba/ident/hosts,
hoping that a generic extensibility/guc patch could be based on that.
I also didn't want to start a thread about this before the feature
freeze, so I'm still waiting/prototyping for a few weeks.

I'm also not against adding an oauth-only feature for 19, that was my
original intention before getting completely distracted by the guc
direction :)

+ else if (strncmp(name, "validator.", strlen("validator.")) == 0)
+ {
+ const char *key = name + strlen("validator.");

This is my only concern with this patch: since we have a list
separated validatr names as a GUC already, couldn't we require a
<validator_name>. prefix instead of the fixed "validator.", to keep
the hba configuration consistent with gucs?

Validators would still have to handle these options differently, but
at least it would look consistent from the user perspective - global
setting in postgresql.conf, same hba-line specific override in
pg_hba.conf. (also, validators already added global GUCs in pg18, and
this would also keep it consistent with that)

+ REQUIRE_AUTH_OPTION(uaOAuth, name, "oauth");

Shouldn't this check go before the name validation?

---

From: Jacob Champion <[email-masked]>
Date: Fri, 27 Mar 2026 16:03:00 -0700

> the hba configuration consistent with gucs?

Well, the `validator.` prefix lets us end-run the namespace issue [1].
It's one thing if I claim that single prefix in parse_hba_auth_opt();
it's another thing if I camp out on literally every identifier
containing a dot.

I'm also not convinced that it's worth spending additional code here
to decide _which_ of the blessed validators is in force for the
current line. (Deferring the check of the option names is bad enough,
but there appears to be no way around that.)

> this would also keep it consistent with that)

After the wild goose chase I sent you on, I think
consistency-in-form-but-not-function is more likely to be a liability
than a benefit. Sure, validator authors will be able to pretend that
users can override particular GUCs per-line, but that's not what's
actually happening, so that could increase user confusion and support
burden for very little practical upside. (As one example, `SHOW
my_validator.setting` isn't going to behave intuitively.)

Since my pitch here is "this is an architectural dead end, but it'll
get us moving while we pursue the better route," I prefer something
that's very obviously bespoke. Especially since validators will have
to migrate from the old way to the new way, if we get our wish. I
don't really want anyone to spend time resolving the collision of the
two behaviors; I'd rather just let the old ugly configuration solution
wither (or die), and encourage everyone to switch as rapidly as
possible.

> Shouldn't this check go before the name validation?

Yeah, I agree. (My original code had a more generic error message when
the name check failed, but now that the message is OAuth-specific, I
don't think it makes sense to pretend that it could belong to any
other auth method.)

Thanks!
--Jacob

[1] https://postgr.es/m/CAOYmi%2Bn9%2BVDNayxsZuG30YLxOXrVB2Wu%3DjBR4WrEdJvxjTATKw%40mail.gmail.com

---

From: Jacob Champion <[email-masked]>
Date: Fri, 27 Mar 2026 16:03:16 -0700

On Tue, Mar 24, 2026 at 5:29 AM Daniel Gustafsson <[email-masked]> wrote:
> FWIW I support this line of investigation and look forward to seeing what it
> could look like.

+many

--Jacob

---

From: Zsolt Parragi <[email-masked]>
Date: Mon, 30 Mar 2026 22:46:34 +0100

> (As one example, `SHOW

Right, I didn't think about that scenario, it might be better to keep
this intentionally different.

That won't be the most user friendly option, but it is still
definitely better than not having the ability to configure this.

---

From: Jacob Champion <[email-masked]>
Date: Mon, 30 Mar 2026 16:54:18 -0700

> definitely better than not having the ability to configure this.

Sounds good. Barring other objections, then, I'll plan to move forward
with this approach for PG19.

Thanks!
--Jacob

---

From: Jacob Champion <[email-masked]>
Date: Thu, 2 Apr 2026 14:26:54 -0700

> with this approach for PG19.

v4-0002 (based on top of [1]) addresses Zsolt's feedback and adds the
missing user documentation. I believe this is now a complete patch
proposal; tear it apart. :D

Thanks,
--Jacob

[1] https://postgr.es/m/CAOYmi%2BmvFS7Ukmacb1z%3DxWO7M%2BDPF41GZsJiJ6sh4U1Qm_yWOA%40mail.gmail.com

---

From: Jacob Champion <[email-masked]>
Date: Fri, 3 Apr 2026 16:33:15 -0700

> proposal; tear it apart. :D

v5 is a quick rebase to get rid of v4-0001; no other changes.

--Jacob

---

From: Zsolt Parragi <[email-masked]>
Date: Mon, 6 Apr 2026 23:09:27 +0100

> I believe this is now a complete patch

The patch looks good to me as is, I don't see any real issues with it,
maybe one cosmetic question in the test.

+ if (GetOAuthHBAOption(state, "log"))
+ elog(LOG, "%s", GetOAuthHBAOption(state, "log"));
+

If we treat the test code as an example for real implementations, this
and the other use could cache the option in a local variable instead
of making duplicate calls, following the pattern of the documentation.

---

From: Jacob Champion <[email-masked]>
Date: Mon, 6 Apr 2026 15:16:55 -0700

> The patch looks good to me as is, I don't see any real issues with it,

Great! I am prepping for a commit Sometime Very Soon.

> If we treat the test code as an example for real implementations,

(It's not, though -- my test code is frequently abusive on purpose and
should not be used as best-practice. If we want compilable sample
code, that looks different from tests IMHO.)

> this

In this case, I want the test to pin the behavior that multiple calls
should work as you'd expect.

Thanks,
--Jacob