v6-0001-A-non-superuser-is-only-allowed-to-specify-the-pa.patch

From 7f588672299586de5f74aa76ef96d1138da5617e Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Mon, 27 Nov 2023 08:46:12 +0530
Subject: [PATCH v6] A non-superuser is only allowed to specify the password
 via the password parameter of the connection string.

When the 'password_required' subscription parameter is true, a
non-superuser is only allowed to specify the password via the password
parameter of the connection string, not using a PGPASS file or PGPASSWORD
environment variable.

Reported-by: Jeff Davis
Author: Vignesh C
Reviewed-by: Peter Smith, Robert Haas, Amit Kapila
Discussion: https://www.postgresql.org/message-id/flat/e5892973ae2a80a1a3e0266806640dae3c428100.camel%40j-davis.com
---
 doc/src/sgml/ref/create_subscription.sgml     | 10 ++-
 .../libpqwalreceiver/libpqwalreceiver.c       |  8 ++
 src/test/subscription/t/027_nosuperuser.pl    | 81 +++++++++++++++++++
 3 files changed, 95 insertions(+), 4 deletions(-)

diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index f1c20b3a46..82ec7c4291 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -358,10 +358,12 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
         <listitem>
          <para>
           Specifies whether connections to the publisher made as a result
-          of this subscription must use password authentication. This setting
-          is ignored when the subscription is owned by a superuser.
-          The default is <literal>true</literal>. Only superusers can set
-          this value to <literal>false</literal>.
+          of this subscription must use password authentication. If
+          <literal>true</literal>, a non-superuser is only allowed to specify
+          the password via the <literal>password</literal> parameter of the
+          connection string. This setting is ignored when the subscription is
+          owned by a superuser. The default is <literal>true</literal>. Only
+          superusers can set this value to <literal>false</literal>.
          </para>
         </listitem>
        </varlistentry>
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 60d5c1fc40..47f7a9ffd9 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -137,6 +137,14 @@ libpqrcv_connect(const char *conninfo, bool logical, bool must_use_password,
 	const char *vals[6];
 	int			i = 0;
 
+	/*
+	 * A non-superuser is only allowed to specify the password via the password
+	 * parameter of the connection string, not using a PGPASS file or
+	 * PGPASSWORD environment variable.
+	 */
+	if (must_use_password)
+		libpqrcv_check_conninfo(conninfo, true);
+
 	/*
 	 * We use the expand_dbname parameter to process the connection string (or
 	 * URI), and pass some extra options.
diff --git a/src/test/subscription/t/027_nosuperuser.pl b/src/test/subscription/t/027_nosuperuser.pl
index 642baa5d7c..2a62e05472 100644
--- a/src/test/subscription/t/027_nosuperuser.pl
+++ b/src/test/subscription/t/027_nosuperuser.pl
@@ -327,4 +327,85 @@ $node_subscriber->wait_for_log(
 	qr/LOG: ( [A-Z0-9]+:)? logical replication worker for subscription \"regression_sub\" will restart because the subscription owner's superuser privileges have been revoked/,
 	$offset);
 
+# If the subscription connection requires a password ('password_required'
+# is true) then a non-superuser must specify that password in the connection
+# string.
+$ENV{"PGPASSWORD"} = 'secret';
+
+my $node_publisher1  = PostgreSQL::Test::Cluster->new('publisher1');
+my $node_subscriber1 = PostgreSQL::Test::Cluster->new('subscriber1');
+$node_publisher1->init(allows_streaming => 'logical');
+$node_subscriber1->init;
+$node_publisher1->start;
+$node_subscriber1->start;
+my $publisher_connstr1 =
+  $node_publisher1->connstr . ' user=regress_test_user dbname=postgres';
+my $publisher_connstr2 =
+  $node_publisher1->connstr
+  . ' user=regress_test_user dbname=postgres password=secret';
+
+for my $node ($node_publisher1, $node_subscriber1)
+{
+	$node->safe_psql(
+		'postgres', qq(
+  CREATE ROLE regress_test_user PASSWORD 'secret' LOGIN REPLICATION;
+  GRANT CREATE ON DATABASE postgres TO regress_test_user;
+  GRANT PG_CREATE_SUBSCRIPTION TO regress_test_user;
+  ));
+}
+
+$node_publisher1->safe_psql(
+	'postgres', qq(
+SET SESSION AUTHORIZATION regress_test_user;
+CREATE PUBLICATION regress_test_pub;
+));
+$node_subscriber1->safe_psql(
+	'postgres', qq(
+CREATE SUBSCRIPTION regress_test_sub CONNECTION '$publisher_connstr1' PUBLICATION regress_test_pub;
+));
+
+# Wait for initial sync to finish
+$node_subscriber1->wait_for_subscription_sync($node_publisher1,
+	'regress_test_sub');
+
+# Setup pg_hba configuration so that logical replication connection without
+# password is not allowed.
+unlink($node_publisher1->data_dir . '/pg_hba.conf');
+$node_publisher1->append_conf('pg_hba.conf',
+	qq{local all 				regress_test_user 	md5});
+$node_publisher1->reload;
+
+# Change the subscription owner to a non-superuser
+$node_subscriber1->safe_psql(
+	'postgres', qq(
+ALTER SUBSCRIPTION regress_test_sub OWNER TO regress_test_user;
+));
+
+# Non-superuser must specify password in the connection string
+my ($ret, $stdout, $stderr) = $node_subscriber1->psql(
+	'postgres', qq(
+SET SESSION AUTHORIZATION regress_test_user;
+ALTER SUBSCRIPTION regress_test_sub REFRESH PUBLICATION;
+));
+isnt($ret, 0,
+	"non zero exit for subscription whose owner is a non-superuser must specify password parameter of the connection string"
+);
+is( $stderr, 'psql:<stdin>:3: ERROR:  password is required
+DETAIL:  Non-superusers must provide a password in the connection string.',
+	'subscription whose owner is a non-superuser must specify password parameter of the connection string'
+);
+
+delete $ENV{"PGPASSWORD"};
+
+# It should succeed after including the password parameter of the connection
+# string.
+($ret, $stdout, $stderr) = $node_subscriber1->psql(
+	'postgres', qq(
+SET SESSION AUTHORIZATION regress_test_user;
+ALTER SUBSCRIPTION regress_test_sub CONNECTION '$publisher_connstr2';
+ALTER SUBSCRIPTION regress_test_sub REFRESH PUBLICATION;
+));
+is($ret, 0,
+	"Non-superuser will be able to refresh the publication after specifying the password parameter of the connection string"
+);
 done_testing();
-- 
2.34.1