Re: [PoC] Federated Authn/z with OAUTHBEARER

Daniel Gustafsson <daniel@yesql.se>

From: Daniel Gustafsson <daniel@yesql.se>
To: Peter Eisentraut <peter@eisentraut.org>, Jacob Champion <jacob.champion@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2024-09-11T13:44:37Z
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

> On 11 Sep 2024, at 09:37, Peter Eisentraut <peter@eisentraut.org> wrote:

> Is there any sense in dealing with the libpq and backend patches separately in sequence, or is this split just for ease of handling?

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 suppose the 0004 "review comments" patch should be folded into the respective other patches?)

Yes (0003 now), along with the 0004 in the attached version (I bumped to v29 as
one commit is now committed, but the attached doesn't change Jacobs commits but
rather add to them) which contains more review comments.  More on that below:

I added a warning to autconf in case --with-oauth is used without --with-python
since this combination will error out in running the tests.  Might be
superfluous but I had an embarrassingly long headscratcher myself as to why the
tests kept failing =)

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).

The oauth_validator test module need to load Makefile.global before exporting
the symbols from there.  I also removed the placeholder regress test which did
nothing and turned diag() calls into note() calls to keep the output from
cluttering.

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.

> What could be the next steps to keep this moving along, other than stare at the remaining patches until we're content with them? ;-)

I'm in the "stare at things" stage now to try and get this into the tree =)

To further pick away at this huge patch I propose to merge the SASL message
length hunk which can be extracted separately.  The attached .txt (to keep the
CFBot from poking at it) contains a diff which can be committed ahead of the
rest of this patch to make it a tad smaller and to keep the history of that
change a bit clearer.

--
Daniel Gustafsson