On Fri, 2024-10-18 at 11:32 +0200, Pavel Stehule wrote: > pá 18. 10. 2024 v 10:20 odesílatel Laurenz Albe <laurenz.albe@cybertec.at> napsal: > > On Fri, 2024-10-18 at 07:47 +0200, Pavel Stehule wrote: > > > Without deeper checks I don't like using GetUserNameFromId for checking the validity of a role. > > > > > > Maybe it is better to use own read of syscache or wrap SetUserIdAndSecContext to do this check. > > > > I agree; it was just the simplest way I could make it happen. It is ugly to allocate and > > return the user name, since we don't really need it. > > > > I could write a dedicated function to check the existence of a user. > > +1
The check is so simple that I didn't write a dedicated function. Instead, I put the catalog search into the code directly.
ok
> > > > The trigger queue exists only in memory, and PostgreSQL tracks dependencies > > only between persisted objects. Do you think that I should add a sentence > > like that to the comment? > > yes, please. I think so it is not too intuitive. Inside a context of this patch > it is ok, but without knowledge of this context, can be strange, why some role used > for trigger can be invalid, although the transaction is not fully finished yet.
I tried to improve the patch along these lines.
Attached is a new version.
The compilation is without any problems, make check-world is ok
I played with this little bit and I didn't find any problems. This patch introduces a possible compatibility issue, but from a security perspective it is a clear improvement.
I did a quick search of documentation and I didn't find any sentence about identity inside trigger execution.
Maybe we should document so the trigger is executed with the identity used by DML command always, when the trigger function has no SECURITY DEFINER flag?
