Re: [PoC] Federated Authn/z with OAUTHBEARER

> On 14 Jan 2025, at 00:21, Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com> wrote:

> - 0001 moves PG_MAX_AUTH_TOKEN_LENGTH, as discussed upthread
> - 0002 handles the non-OAuth-specific changes to require_auth (0005
> now highlights the OAuth-specific pieces)
> - 0003 adds SASL_ASYNC and its handling code

I was reading these diffs with the aim of trying to get them in sooner rather
than later to get us closer to the full patchset committed. Two small things
came to mind:

+ /*
+ * The mechanism should have set up the necessary callbacks; all we
+ * need to do is signal the caller.
+ */
+ *async = true;
+ return STATUS_OK;
Is it worth adding assertions here to ensure that everything has been set up
properly to help when adding a new mechanism in the future?

+ /* Done. Tear down the async implementation. */
+ conn->cleanup_async_auth(conn);
+ conn->cleanup_async_auth = NULL;
+ Assert(conn->altsock == PGINVALID_SOCKET);
In pqDropConnection() we set ->altsock to NULL just to be sure rather than
assert that cleanup has done so. Shouldn't we be consistent in the
expectation and set to NULL here as well?

--
Daniel Gustafsson