Re: [PoC] Federated Authn/z with OAUTHBEARER

On Mon, Jan 20, 2025 at 2:10 PM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
> + /*
> + * 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?

Yeah, I think that'd be helpful.

> + /* 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

(I assume you mean PGINVALID_SOCKET?)

> 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?

I'm not opposed; I just figured that the following code might be a bit
confusing:

Assert(conn->altsock == PGINVALID_SOCKET);
conn->altsock = PGINVALID_SOCKET;

But I can add a comment to the assignment to try to explain. I don't
know what the likelihood of landing code that trips that assertion is,
but an explicit assignment would at least stop problems from
cascading.

--Jacob