Re: Proposal: Save user's original authenticated identity for logging
Jacob Champion <pchampion@vmware.com>
From: Jacob Champion <pchampion@vmware.com>
To: "michael@paquier.xyz" <michael@paquier.xyz>
Cc: "magnus@hagander.net" <magnus@hagander.net>, "stark@mit.edu" <stark@mit.edu>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, "sfrost@snowman.net" <sfrost@snowman.net>, "tgl@sss.pgh.pa.us" <tgl@sss.pgh.pa.us>
Date: 2021-04-02T18:18:44Z
Lists: pgsql-hackers
Attachments
- since-v18.diff.txt (text/plain)
- v19-0001-Plug-more-TAP-test-suites-with-new-PostgresNode-.patch (text/x-patch) patch v19-0001
- v19-0002-Log-authenticated-identity-from-all-auth-backend.patch (text/x-patch) patch v19-0002
On Fri, 2021-04-02 at 13:45 +0900, Michael Paquier wrote: > Attached is what I have come up with as the first building piece, > which is basically a combination of 0001 and 0002, except that I > modified things so as the number of arguments remains minimal for all > the routines. This avoids the manipulation of the list of parameters > passed down to PostgresNode::psql. The arguments for the optional > query, the expected stdout and stderr are part of the parameter set > (0001 was not doing that). I made a few changes, highlighted in the since-v18 diff: > + # The result is assumed to match "true", or "t", here. > + $node->connect_ok($connstr, $test_name, sql => $query, > + expected_stdout => qr/t/); I've anchored this as qr/^t$/ so we don't accidentally match a stray "t" in some larger string. > - is($res, 0, $test_name); > - like($stdoutres, $expected, $test_name); > - is($stderrres, "", $test_name); > + my ($stdoutres, $stderrres); > + > + $node->connect_ok($connstr, $test_name, $query, $expected); $query and $expected need to be given as named parameters. We also lost the stderr check from the previous version of the test, so I added expected_stderr to connect_ok(). > @@ -446,14 +446,14 @@ TODO: > # correct client cert in encrypted PEM with empty password > $node->connect_fails( > "$common_connstr user=ssltestuser sslcert=ssl/client.crt sslkey=ssl/client-encrypted-pem_tmp.key sslpassword=''", > - qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E!, > + expected_stderr => qr!\Qprivate key file "ssl/client-encrypted-pem_tmp.key": processing error\E!, > "certificate authorization fails with correct client cert and empty password in encrypted PEM format" > ); These tests don't run yet inside the TODO block, but I've put the expected_stderr parameter at the end of the list for them. > For the main patch, this will need to be > extended with two more parameters in each routine: log_like and > log_unlike to match for the log patterns, handled as arrays of > regexes. That's what 0003 is basically doing already. Rebased on top of your patch as v19, attached. (v17 disappeared into the ether somewhere, I think. :D) Now that it's easy to add log_like to existing tests, I fleshed out the LDAP tests with a few more cases. They don't add code coverage, but they pin the desired behavior for a few more types of LDAP auth. --Jacob
Commits
-
Add missing $Test::Builder::Level settings
- 73aa5e0cafd0 15.0 landed
- e536a2683439 14.0 landed
-
Add some information about authenticated identity via log_connections
- 9afffcb833d3 14.0 landed
-
Fix some issues with SSL and Kerberos tests
- 5a71964a832f 14.0 landed
-
Refactor all TAP test suites doing connection checks
- c50624cdd248 14.0 landed