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

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

  1. Add missing $Test::Builder::Level settings

  2. Add some information about authenticated identity via log_connections

  3. Fix some issues with SSL and Kerberos tests

  4. Refactor all TAP test suites doing connection checks