Horton

Re: [PoC] Federated Authn/z with OAUTHBEARER

Jacob Champion <jacob.champion@enterprisedb.com>

View thread
From: Jacob Champion <jacob.champion@enterprisedb.com>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2024-10-10T23:08:50Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. meson: Fix install-quiet after clean

  2. oauth: Run Autoconf tests with correct compiler flags

  3. Link libpq with libdl if the platform needs that.

  4. Doc: correct spelling of meson switch.

  5. oauth: Correct SSL dependency for libpq-oauth.a

  6. oauth: Fix Autoconf build on macOS

  7. oauth: Move the builtin flow into a separate module

  8. Remove a stray "pgrminclude" annotation

  9. oauth: Simplify copy of PGoauthBearerRequest

  10. oauth: Improve validator docs on interruptibility

  11. oauth: Disallow synchronous DNS in libcurl

  12. oauth: Fix postcondition for set_timer on macOS

  13. oauth: Use IPv4-only issuer in oauth_validator tests

  14. Work around OAuth/EVFILT_TIMER quirk on NetBSD.

  15. oauth: Fix incorrect const markers in struct

  16. Add missing entry to oauth_validator test .gitignore

  17. cirrus: Temporarily fix libcurl link error

  18. Add support for OAUTHBEARER SASL mechanism

  19. libpq: Handle asynchronous actions during SASL

  20. require_auth: prepare for multiple SASL mechanisms

  21. Move PG_MAX_AUTH_TOKEN_LENGTH to libpq/auth.h

  22. Make SASL max message length configurable

  23. jsonapi: fully initialize dummy lexer

  24. common/jsonapi: support libpq as a client

  25. Remove fe_memutils from libpgcommon_shlib

  26. Revert ECPG's use of pnstrdup()

  27. Explicitly require password for SCRAM exchange

  28. Refactor SASL exchange to return tri-state status

Attachments

Hi all,

Here's v30, which aims to fix the infinite-retry problem: you get a
maximum of two OAuth connections to the server, and we won't prompt
the user more than once per transport. (I still need to wrap my head
around the retry behavior during transport negotiation.)

On Wed, Sep 11, 2024 at 3:54 PM Jacob Champion
<jacob.champion@enterprisedb.com> wrote:
> On Wed, Sep 11, 2024 at 6:44 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> >
> > I think it's just make reviewing a bit easier.  At this point I think they can
> > be merged together, it's mostly out of historic reasons IIUC since the patchset
> > earlier on supported more than one library.
>
> I can definitely do that (and yeah, it was to make the review slightly
> less daunting). The server side could potentially be committed
> independently, if you want to parallelize a bit, but it'd have to be
> torn back out if the libpq stuff didn't land in time.

I plan to do the combination in the near future, when I'm not making a
bunch of other changes at the same time.

> > > (I suppose the 0004 "review comments" patch should be folded into the respective other patches?)
>
> Yes. I'm using that patch as a holding area while I write tests for
> the hunks, and then moving them backwards.

This is almost gone; just one piece remaining as of v30. Everything
else has been folded in with new tests.

> > CURL_IGNORE_DEPRECATION(x;) broke pgindent, it needs to keep the semicolon on
> > the outside like CURL_IGNORE_DEPRECATION(x);.  This doesn't really work well
> > with how the macro is defined, not sure how we should handle that best (the
> > attached makes the style as per how pgindent want's it with the semicolon
> > returned).
>
> Ugh... maybe a case for a pre_indent rule in pgindent?

I've taken a stab at a pre_indent rule that seems to work well enough
(though the regex itself is write-once-read-never).

> > There is a first stab at documenting the validator module API, more to come (it
> > doesn't compile right now).
> >
> > It contains a pgindent and pgperltidy run to keep things as close to in final
> > sync as we can to catch things like the curl deprecation macro mentioned above
> > early.

The rest of your second comments patch has been incorporated now, with
the exception of the following hunk:

    - read($read_fh, $port, 7) // die "failed to read port number: $!";
    + read($read_fh, $port, 7) or die "failed to read port number: $!";

read() doesn't set $! unless it returns undef, according to the docs
[1]. A zero read (i.e. immediate EOF) will be handled further down.

> > To further pick away at this huge patch I propose to merge the SASL message
> > length hunk which can be extracted separately.

I've pulled this out into 0001.

Thanks,
--Jacob

[1] https://perldoc.perl.org/functions/read