From: Drouvot, Bertrand <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [PATCH] Add peer authentication TAP test
Date: Fri, 30 Sep 2022 19:51:29 +0200
Message-ID: <05056cfa-50d9-502d-4c4c-4725c6fb9943@gmail.com> (raw)
In-Reply-To: <YzYxtuTzpoz6x2tD@paquier.xyz>
References: <aa60994b-1c66-ca7a-dab9-9a200dbac3d2@amazon.com>
<YzPhGhI0dyC4/IAG@paquier.xyz>
<2ecffb46-8b0c-83cb-2577-bf83262b5f10@gmail.com>
<YzP2vBC1uBGEDRM6@paquier.xyz>
<YzYxtuTzpoz6x2tD@paquier.xyz>
Hi,
On 9/30/22 2:00 AM, Michael Paquier wrote:
> On Wed, Sep 28, 2022 at 04:24:44PM +0900, Michael Paquier wrote:
>> Hmm, indeed. It would be more reliable to rely on the contents
>> returned by getpeereid()/getpwuid() after one successful peer
>> connection, then use it in the map. I was wondering whether using
>> stuff like getpwuid() in the perl script itself would be better, but
>> it sounds less of a headache in terms of portability to just rely on
>> authn_id via SYSTEM_USER to generate the contents of the correct map.
>
> By the way, on an extra read I have found a few things that can be
> simplified
> - I think that test_role() should be reworked so as the log patterns
> expected are passed down to connect_ok() and connect_fails() rather
> than involving find_in_log(). You still need find_in_log() to skip
> properly the case where peer is not supported by the platform, of
> course.
> - get_log_size() is not necessary. You should be able to get the same
> information with "-s $self->logfile".
> - Nit: a newline should be added at the end of 003_peer.pl.
> --
Agree that it could be simplified, thanks for the hints!
Attached a simplified version.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.comAttachments:v2-0001-peer-authentication-tap-test.patch (text/plain, 3.3K) downloaddiff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.plnew file mode 100644index 0000000000..57bf2343fc--- /dev/null+++ b/src/test/authentication/t/003_peer.pl@@ -0,0 +1,108 @@++# Copyright (c) 2022, PostgreSQL Global Development Group++# Tests for peer authentication and user name map.+# The test is skipped if the platform does not support peer authentication.++use strict;+use warnings;+use PostgreSQL::Test::Cluster;+use PostgreSQL::Test::Utils;+use Test::More;++# Delete pg_hba.conf from the given node, add a new entry to it+# and then execute a reload to refresh it.+sub reset_pg_hba+{+ my $node = shift;+ my $hba_method = shift;++ unlink($node->data_dir . '/pg_hba.conf');+ # just for testing purposes, use a continuation line.+ $node->append_conf('pg_hba.conf', "local all all\\\n $hba_method");+ $node->reload;+ return;+}++# Test access for a single role, useful to wrap all tests into one.+sub test_role+{+ local $Test::Builder::Level = $Test::Builder::Level + 1;++ my ($node, $role, $method, $expected_res, %params) = @_;+ my $status_string = 'failed';+ $status_string = 'success' if ($expected_res eq 0);++ my $connstr = "user=$role";+ my $testname =+ "authentication $status_string for method $method, role $role";++ if ($expected_res eq 0)+ {+ $node->connect_ok($connstr, $testname, %params);+ }+ else+ {+ # No checks of the error message, only the status code.+ $node->connect_fails($connstr, $testname, %params);+ }+}++# find $pat in logfile of $node.+sub find_in_log+{+ my ($node, $pat) = @_;++ my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);+ return 0 if (length($log) <= 0);++ return $log =~ m/$pat/;+}++# Initialize primary node.+my $node = PostgreSQL::Test::Cluster->new('primary');+$node->init;+$node->append_conf('postgresql.conf', "log_connections = on\n");+$node->start;++# Set pg_hba.conf with the peer authentication.+reset_pg_hba($node, 'peer');++# Check that peer authentication is supported on this platform.+$node->psql('postgres');++# If not supported, then skip the rest of the test.+if (find_in_log($node, qr/peer authentication is not supported on this platform/))+{+ plan skip_all => 'peer authentication is not supported on this platform';+}++# Let's create a new user for the user name map test.+$node->safe_psql('postgres',+ qq{CREATE USER testmapuser});++# Get the system_user to define the user name map test.+my $system_user =+ $node->safe_psql('postgres', q(select (string_to_array(SYSTEM_USER, ':'))[2]));++# This connection should succeed.+test_role($node, $system_user, 'peer', 0,+ log_like =>+ [qr/connection authenticated: identity="$system_user" method=peer/]);++# This connection should failed.+test_role($node, qq{testmapuser}, 'peer', 2,+ log_like => [qr/Peer authentication failed for user "testmapuser"/]);++# Define a user name map.+$node->append_conf('pg_ident.conf', qq{mypeermap $system_user testmapuser});++# Set pg_hba.conf with the peer authentication and the user name map.+reset_pg_hba($node, 'peer map=mypeermap');++# This connection should now succeed.+test_role($node, qq{testmapuser}, 'peer', 0,+ log_like =>+ [qr/connection authenticated: identity="$system_user" method=peer/]);++done_testing();
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bertranddrouvot.pg@gmail.com, michael@paquier.xyz, pgsql-hackers@lists.postgresql.org
Subject: Re: [PATCH] Add peer authentication TAP test
In-Reply-To: <05056cfa-50d9-502d-4c4c-4725c6fb9943@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox