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
signature.asc
Description: PGP signature
