On Mon, Jan 16, 2023 at 11:53:57AM +0100, Jelte Fennema wrote:
>> Perhaps it would be simpler to use copy_auth_token() in this code path
>> and always free the resulting token?
>
> I initially tried that when working on the patch, but copy_auth_token
> (surprisingly) doesn't copy the regex field into the new AuthToken.
> So we'd have to regenerate it conditionally. Making the copy
> conditional seemed just as simple code-wise, with the added
> bonus that it's not doing a useless copy.
Okay, I can live with that.
>> In the code path where system-user is a regexp, could it be better
>> to skip the replacement of \1 in the new AuthToken if pg-user is
>> itself a regexp? The compiled regexp would be the same, but it could
>> be considered as a bit confusing, as it can be thought that the
>> compiled regexp of pg-user happened after the replacement?
>
> I updated 0004 to prioritize membership checks and regexes over
> substitution of \1. I also added tests for this. Prioritizing "all" over
> substitution of \1 is not necessary, since by definition "all" does
> not include \1.
Thanks, 0003 is OK, so applied now.
0004 looks fine as well, be it for the tests (I am hesitating to tweak
things a bit here actually for the role names), the code or the docs,
still I am planning a second lookup.
--
Michael