(Thanks for the commit, Peter!) On Wed, Sep 11, 2024 at 6:44 AM Daniel Gustafsson <dan...@yesql.se> wrote: > > > On 11 Sep 2024, at 09:37, Peter Eisentraut <pe...@eisentraut.org> wrote: > > > Is there any sense in dealing with the libpq and backend patches separately > > in sequence, or is this split just for ease of handling? > > I think it's just make reviewing a bit easier. At this point I think they can > be merged together, it's mostly out of historic reasons IIUC since the > patchset > earlier on supported more than one library.
I can definitely do that (and yeah, it was to make the review slightly less daunting). The server side could potentially be committed independently, if you want to parallelize a bit, but it'd have to be torn back out if the libpq stuff didn't land in time. > > (I suppose the 0004 "review comments" patch should be folded into the > > respective other patches?) Yes. I'm using that patch as a holding area while I write tests for the hunks, and then moving them backwards. > I added a warning to autconf in case --with-oauth is used without > --with-python > since this combination will error out in running the tests. Might be > superfluous but I had an embarrassingly long headscratcher myself as to why > the > tests kept failing =) Whoops, sorry. I guess we should just skip them if Python isn't there? > CURL_IGNORE_DEPRECATION(x;) broke pgindent, it needs to keep the semicolon on > the outside like CURL_IGNORE_DEPRECATION(x);. This doesn't really work well > with how the macro is defined, not sure how we should handle that best (the > attached makes the style as per how pgindent want's it with the semicolon > returned). Ugh... maybe a case for a pre_indent rule in pgindent? > The oauth_validator test module need to load Makefile.global before exporting > the symbols from there. Hm. Why was that passing the CI, though...? > There is a first stab at documenting the validator module API, more to come > (it > doesn't compile right now). > > It contains a pgindent and pgperltidy run to keep things as close to in final > sync as we can to catch things like the curl deprecation macro mentioned above > early. Thanks! > > What could be the next steps to keep this moving along, other than stare at > > the remaining patches until we're content with them? ;-) > > I'm in the "stare at things" stage now to try and get this into the tree =) Yeah, and I still owe you all an updated roadmap. While I fix up the tests, I've also been picking away at the JSON encoding problem that was mentioned in [1]; the recent SASLprep fix was fallout from that, since I'm planning to pull in pieces of its UTF-8 validation. I will eventually want to fuzz the heck out of this. > To further pick away at this huge patch I propose to merge the SASL message > length hunk which can be extracted separately. The attached .txt (to keep the > CFBot from poking at it) contains a diff which can be committed ahead of the > rest of this patch to make it a tad smaller and to keep the history of that > change a bit clearer. LGTM! -- Peter asked me if there were plans to provide a "standard" validator module, say as part of contrib. The tricky thing is that Bearer validation is issuer-specific, and many providers give you an opaque token that you're not supposed to introspect at all. We could use token introspection (RFC 7662) for online verification, but last I looked at it, no one had actually implemented those endpoints. For offline verification, I think the best we could do would be to provide a generic JWT Profile (RFC 9068) validator, but again I don't know if anyone is actually providing those token formats in practice. I'm inclined to push that out into the future. Thanks, --Jacob [1] https://www.postgresql.org/message-id/ZjxQnOD1OoCkEeMN%40paquier.xyz
