On Mon, Feb 28, 2022 at 04:00:36PM -0500, Stephen Frost wrote:
> * Jacob Champion (pchampion@vmware.com) wrote:
>> On Fri, Feb 25, 2022 at 01:23:49PM -0800, Andres Freund wrote:
>>> Looks to me like authn_id isn't synchronized to parallel workers right now. So
>>> the function will return the wrong thing when executed as part of a parallel
>>> query.
>>
>> Thanks for the catch. It looks like MyProcPort is left empty, and other
>> functions that rely on like inet_server_addr() are marked parallel-
>> restricted, so I've done the same in v4.
>
> That's probably alright.
I'd say as well that this is right as-is. If it happens that there is
a use-case for making this parallel aware in the future, it could be
done. Now, it may be a bit weird to make parallel workers inherit the
authn ID of the parent as these did not go through authentication, no?
Letting this function being run only by the leader feels intuitive.
> Yeah, we really should make this available to trigger-based auditing
> systems too and not just through log_connections which involves a great
> deal more log parsing and work external to the database to figure out
> who did what.
Okay, I won't fight hard on that if all of you think that this is
useful for a given session.
>> Subject: [PATCH v4] Add API to retrieve authn_id from SQL
>>
>> The authn_id field in MyProcPort is currently only accessible to the
>> backend itself. Add a SQL function, pg_session_authn_id(), to expose
>> the field to triggers that may want to make use of it.
>
> Only did a quick look but generally looks reasonable to me.
The function and the test are fine, pgperltidy complains a bit about
the format of the tests.
Ayway, this function needs to be documented. I think that you should
just add that in "Session Information Functions" in func.sgml, same
area as current_user(). The last time we talked about the authn ID,
one thing we discussed about was how to describe that in a good way to
the user, which is why the section of log_connections was reworked a
bit. And we don't have yet any references to what an authenticated
identity is in the docs.
There is no need to update catversion.h in the patch, committers
usually take care of that and that's an area of the code that
conflicts a lot.
--
Michael
