From 0931a8ce0cbf71efb1d9f359a4de749163081ae9 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Fri, 22 Sep 2023 12:27:34 +0530
Subject: [PATCH v3] Make sure that the password option is provided from
 connection string for non-superusers when password required is true.

Password can be specified from PGPASS file or PGPASSWORD environment,
when password required is true then for non-superusers we should make sure that
the password option is provided from connection string.
---
 .../libpqwalreceiver/libpqwalreceiver.c       |  8 +++
 src/test/subscription/t/027_nosuperuser.pl    | 71 +++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 60d5c1fc40..e0d7bfc1fa 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;
 
+	/*
+	 * Check if the password is specified as part of connection string itself
+	 * for non-superusers. This check is required to prevent password being
+	 * set from PGPASSWORD environment or PGPASS file in case of non-superusers.
+	 */
+	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 d7a7e3ef5b..3a85687fff 100644
--- a/src/test/subscription/t/027_nosuperuser.pl
+++ b/src/test/subscription/t/027_nosuperuser.pl
@@ -303,4 +303,75 @@ GRANT SELECT ON alice.unpartitioned TO regress_alice;
 expect_replication("alice.unpartitioned", 3, 17, 21,
 	"restoring SELECT permission permits replication to continue");
 
+# If the superuser owned subscription which was using a connection string
+# (without password) with the password coming from the PGPASSWORD environment
+# or PGPASS FILE transfers ownership to a non-superuser, then the next
+# subscription command(which connects to the publisher) should fail with
+# password required error.
+$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 dbname=postgres';
+
+for my $node ($node_publisher1, $node_subscriber1)
+{
+	$node->safe_psql(
+		'postgres', qq(
+  CREATE ROLE regress_test PASSWORD 'secret' LOGIN REPLICATION;
+  GRANT CREATE ON DATABASE postgres TO regress_test;
+  GRANT PG_CREATE_SUBSCRIPTION TO regress_test;
+  ));
+}
+
+$node_publisher1->safe_psql(
+	'postgres', qq(
+SET SESSION AUTHORIZATION regress_test;
+CREATE PUBLICATION test;
+));
+$node_subscriber1->safe_psql(
+	'postgres', qq(
+CREATE SUBSCRIPTION test_sub CONNECTION '$publisher_connstr1' PUBLICATION test WITH (enabled=false);
+));
+
+$node_subscriber1->safe_psql('postgres',
+	qq(ALTER SUBSCRIPTION test_sub ENABLE;));
+
+# Wait for initial sync to finish
+$node_subscriber1->wait_for_subscription_sync($node_publisher1, '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 	md5});
+$node_publisher1->reload;
+
+# Change the subscription owner to a non-superuser
+$node_subscriber1->safe_psql(
+	'postgres', qq(
+ALTER SUBSCRIPTION test_sub OWNER TO regress_test;
+));
+
+# Non-superuser must specify password in the connection string
+my ($ret, $stdout, $stderr) = $node_subscriber1->psql(
+	'postgres', qq(
+SET SESSION AUTHORIZATION regress_test;
+ALTER SUBSCRIPTION test_sub REFRESH PUBLICATION;
+));
+isnt($ret, 0,
+	"non zerof exit for subscription whose owner is a non-superuser must specify password through 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 through connection string'
+);
+
+delete $ENV{"PGPASSWORD"};
+
 done_testing();
-- 
2.34.1