Horton

Re: [PoC] Federated Authn/z with OAUTHBEARER

Peter Eisentraut <peter@eisentraut.org>

View thread
From: Peter Eisentraut <peter@eisentraut.org>
To: Jacob Champion <jacob.champion@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Daniel Gustafsson <daniel@yesql.se>
Date: 2024-08-26T08:18:10Z
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

On 13.08.24 23:11, Jacob Champion wrote:
> On Sun, Aug 11, 2024 at 11:37 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>> I have committed 0002 now.
> 
> Thanks Peter! Rebased over both in v26.

I have looked again at the jsonapi memory management patch (v26-0001).
As previously mentioned, I think adding a third or fourth (depending
on how you count) memory management API is maybe something we should
avoid.  Also, the weird layering where src/common/ now (sometimes)
depends on libpq seems not great.

I'm thinking, maybe we leave the use of StringInfo at the source code
level, but #define the symbols to use PQExpBuffer.  Something like

#ifdef JSONAPI_USE_PQEXPBUFFER

#define StringInfo PQExpBuffer
#define appendStringInfo appendPQExpBuffer
#define appendBinaryStringInfo appendBinaryPQExpBuffer
#define palloc malloc
//etc.

#endif

(simplified, the argument lists might differ)

Or, if people find that too scary, something like

#ifdef JSONAPI_USE_PQEXPBUFFER

#define jsonapi_StringInfo PQExpBuffer
#define jsonapi_appendStringInfo appendPQExpBuffer
#define jsonapi_appendBinaryStringInfo appendBinaryPQExpBuffer
#define jsonapi_palloc malloc
//etc.

#else

#define jsonapi_StringInfo StringInfo
#define jsonapi_appendStringInfo appendStringInfo
#define jsonapi_appendBinaryStringInfo appendBinaryStringInfo
#define jsonapi_palloc palloc
//etc.

#endif

That way, it's at least more easy to follow the source code because
you see a mostly-familiar API.

Also, we should make this PQExpBuffer-using mode only used by libpq,
not by frontend programs.  So libpq takes its own copy of jsonapi.c
and compiles it using JSONAPI_USE_PQEXPBUFFER.  That will make the
libpq build descriptions a bit more complicated, but everyone who is
not libpq doesn't need to change.

Once you get past all the function renaming, the logic changes in
jsonapi.c all look pretty reasonable.  Refactoring like
allocate_incremental_state() makes sense.

You could add pg_nodiscard attributes to
makeJsonLexContextCstringLen() and makeJsonLexContextIncremental() so
that callers who are using the libpq mode are forced to check for
errors.  Or maybe there is a clever way to avoid even that: Create a
fixed JsonLexContext like

     static const JsonLexContext failed_oom;

and on OOM you return that one from makeJsonLexContext*().  And then
in pg_parse_json(), when you get handed that context, you return
JSON_OUT_OF_MEMORY immediately.

Other than that detail and the need to use freeJsonLexContext(), it
looks like this new mode doesn't impose any additional burden on
callers, since during parsing they need to check for errors anyway,
and this just adds one more error type for out of memory.  That's a good 
outcome.