public inbox for [email protected]  
help / color / mirror / Atom feed
From: Tom Lane <[email protected]>
To: Daniel Gustafsson <[email protected]>
Cc: Jacob Champion <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Antonin Houska <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER
Date: Sun, 23 Feb 2025 11:49:21 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAOYmi+kHXMG2Z2KR7y36Cbsop_7t-YugrpfCnOXuXz8BpfaXaw@mail.gmail.com>
	<[email protected]>
	<CAOYmi+=ee8H=GaGNH_gZvBE+YPtmK9eqUdXqemk=zKDnO8YXRw@mail.gmail.com>
	<CAOYmi+kLTJ1wZ6gxRbgtR52E=EyiCpmp6J3mmSvtc1a6i7sZ3Q@mail.gmail.com>
	<CAOYmi+=ceXepXOrQXsxca-u57GxM+x_q34fpyNJ8PLmbUWPwEQ@mail.gmail.com>
	<[email protected]>
	<CAOYmi+=vJQ8EUiVf-iHYBwz4-tLHLrvp8rYDoNicB2yJrBKraw@mail.gmail.com>
	<CAOYmi+=FzVg+C-pQHCwjW0qU-POHmzZaD2z3CdsACj==14H8kQ@mail.gmail.com>
	<[email protected]>
	<CAOYmi+kjPLfT7iFqNMVV44sAUJ9m10TfrSMRPUYU2OgH5j0AYg@mail.gmail.com>
	<[email protected]>
	<CAOYmi+kn0_FioLpDqctn97a_W8n90XLt_OmU1xL6ZdK8DP8ufA@mail.gmail.com>
	<[email protected]>
	<CAOYmi+mEkXu9Jfg9v-5z9DmBCRCMS2dt43Z=TCbgVkbrqJXxRg@mail.gmail.com>
	<06102deb-00ef-431d-a3ea-65afb24! [email protected]>
	<CAOYmi+kZAka0sdxCOBxsQc2ozEZGZKHWU_9nrPXg3sG1NJ-zJw@mail.gmail.com>
	<[email protected]>
	<CAOYmi+kQFrKGmC0=Um-QryU1fvCAOC2bpjcz-4rxgLrp69mSOg@mail.gmail.com>
	<[email protected]>
	<CAOYmi+=K3BBRXwvJpt5D_4nK_EimCJBj7a42maACszsQC3=t9w@mail.gmail.com>
	<[email protected]>
	<CAOYmi+kJqzo6XsR9TEhvVfeVNQ-TyFM5LATypm9yoQVYk=4Wrw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAOYmi+nHG7oy+ybHH72WjiXAQG3tE6v_at-K9ebRy2oqo92V+A@mail.gmail.com>
	<[email protected]>
	<CAOYmi+mFZsExAYWy-bS9=yki_fWC+MC0MufpEZ_SycrbCAJrEg@mail.gmail.com>
	<[email protected]>
	<CAOYmi+=zgJinCoqdQ70fvKmvyrLekd9-supkuj57Poa1FmX8bw@mail.gmail.com>
	<[email protected]>
	<CAOYmi+=1_AqPL0Y7qdqg1ER0-bBjCUKUZfrLcgob5bHjzSsvbA@mail.gmail.com>
	<CAOYmi+nP8AM9xm+xUW5kDz7XDF7M! [email protected]>
	<[email protected]>
	<[email protected]>

Daniel Gustafsson <[email protected]> writes:
> I spent a few more hours staring at this, and ran it through a number of CI and
> local builds, without anything showing up.  Pushed to master with the first set
> of buildfarm animals showing green builds.

Coverity has a nit-pick about this:

/srv/coverity/git/pgsql-git/postgresql/src/interfaces/libpq/fe-auth-oauth.c: 784 in setup_token_request()
778     		if (!request_copy)
779     		{
780     			libpq_append_conn_error(conn, "out of memory");
781     			goto fail;
782     		}
783     
>>>     CID 1643156:  High impact quality  (WRITE_CONST_FIELD)
>>>     A write to an aggregate overwrites a const-qualified field within the aggregate.
784     		memcpy(request_copy, &request, sizeof(request));
785     
786     		conn->async_auth = run_user_oauth_flow;
787     		conn->cleanup_async_auth = cleanup_user_oauth_flow;
788     		state->async_ctx = request_copy;
789     	}

This is evidently because of the fields declared const:

	/* Hook inputs (constant across all calls) */
	const char *const openid_configuration; /* OIDC discovery URI */
	const char *const scope;	/* required scope(s), or NULL */

IMO, the set of cases where it's legitimate to mark individual struct
fields as const is negligibly small, and this doesn't seem to be one
of them.  It's not obvious to me where/how PGoauthBearerRequest
structs are supposed to be constructed, but I find it hard to believe
that they will all spring full-grown from the forehead of Zeus.
Nonetheless, this declaration requires exactly that.

(I'm kind of surprised that we're not getting similar bleats from
any buildfarm animals, but so far I don't see any.)

BTW, as another nitpicky style matter: why do PGoauthBearerRequest
etc. spell their struct tag names differently from their typedef names
(that is, with/without an underscore)?  That is not our project style
anywhere else, and I'm failing to detect a good reason to do it here.

			regards, tom lane






view thread (101+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PoC] Federated Authn/z with OAUTHBEARER
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox