Hi,

Currently while changing the owner of ALL TABLES IN SCHEMA
publication, it is not checked if the new owner has superuser
permission or not. Added a check to throw an error if the new owner
does not have superuser permission.
Attached patch has the changes for the same. Thoughts?

Regards,
Vignesh
From d0d0d2d8944a66f28b239da5fb80c15415a016e6 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignes...@gmail.com>
Date: Thu, 2 Dec 2021 19:44:15 +0530
Subject: [PATCH v1] Fix for new owner of ALL TABLES IN SCHEMA publication
 should be superuser.

Currently while changing the owner of ALL TABLES IN SCHEMA publication, it is
not checked if the new owner has superuser permission or not. Added a check to
throw an error if the new owner does not have super user permission.
---
 src/backend/commands/publicationcmds.c    | 38 +++++++++++++++++++++++
 src/test/regress/expected/publication.out | 17 ++++++++++
 src/test/regress/sql/publication.sql      | 17 ++++++++++
 3 files changed, 72 insertions(+)

diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 7d4a0e95f6..7081919ffc 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -234,6 +234,37 @@ CheckObjSchemaNotAlreadyInPublication(List *rels, List *schemaidlist,
 	}
 }
 
+/*
+ * Check if any schema is associated with the publication.
+ */
+static bool
+CheckSchemaPublication(Oid pubid)
+{
+	Relation	pubschsrel;
+	ScanKeyData scankey;
+	SysScanDesc scan;
+	HeapTuple	tup;
+	bool 		result = false;
+
+	pubschsrel = table_open(PublicationNamespaceRelationId, AccessShareLock);
+	ScanKeyInit(&scankey,
+				Anum_pg_publication_namespace_pnpubid,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(pubid));
+
+	scan = systable_beginscan(pubschsrel,
+							  PublicationNamespacePnnspidPnpubidIndexId,
+							  true, NULL, 1, &scankey);
+	tup = systable_getnext(scan);
+	if (HeapTupleIsValid(tup))
+		result = true;
+
+	systable_endscan(scan);
+	table_close(pubschsrel, AccessShareLock);
+
+	return result;
+}
+
 /*
  * Create new publication.
  */
@@ -1192,6 +1223,13 @@ AlterPublicationOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
 					 errmsg("permission denied to change owner of publication \"%s\"",
 							NameStr(form->pubname)),
 					 errhint("The owner of a FOR ALL TABLES publication must be a superuser.")));
+
+		if (CheckSchemaPublication(form->oid) && !superuser_arg(newOwnerId))
+			ereport(ERROR,
+					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					 errmsg("permission denied to change owner of publication \"%s\"",
+							NameStr(form->pubname)),
+					 errhint("The owner of a FOR ALL TABLES IN SCHEMA publication must be a superuser.")));
 	}
 
 	form->pubowner = newOwnerId;
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 1feb558968..4c74488113 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -373,6 +373,23 @@ ALTER PUBLICATION testpub2 ADD TABLE testpub_tbl1;  -- ok
 DROP PUBLICATION testpub2;
 DROP PUBLICATION testpub3;
 SET ROLE regress_publication_user;
+CREATE ROLE regress_publication_user3 LOGIN SUPERUSER;
+GRANT regress_publication_user2 TO regress_publication_user3;
+SET ROLE regress_publication_user3;
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub4 FOR ALL TABLES IN SCHEMA pub_test;
+RESET client_min_messages;
+SET ROLE regress_publication_user;
+ALTER ROLE regress_publication_user3 NOSUPERUSER;
+SET ROLE regress_publication_user3;
+-- fail - new owner must be superuser
+ALTER PUBLICATION testpub4 owner to regress_publication_user2; -- fail
+ERROR:  permission denied to change owner of publication "testpub4"
+HINT:  The owner of a FOR ALL TABLES IN SCHEMA publication must be a superuser.
+ALTER PUBLICATION testpub4 owner to regress_publication_user; -- ok
+SET ROLE regress_publication_user;
+DROP PUBLICATION testpub4;
+DROP ROLE regress_publication_user3;
 REVOKE CREATE ON DATABASE regression FROM regress_publication_user2;
 DROP TABLE testpub_parted;
 DROP TABLE testpub_tbl1;
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 8fa0435c32..880f0caa1d 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -218,6 +218,23 @@ DROP PUBLICATION testpub2;
 DROP PUBLICATION testpub3;
 
 SET ROLE regress_publication_user;
+CREATE ROLE regress_publication_user3 LOGIN SUPERUSER;
+GRANT regress_publication_user2 TO regress_publication_user3;
+SET ROLE regress_publication_user3;
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION testpub4 FOR ALL TABLES IN SCHEMA pub_test;
+RESET client_min_messages;
+SET ROLE regress_publication_user;
+ALTER ROLE regress_publication_user3 NOSUPERUSER;
+SET ROLE regress_publication_user3;
+-- fail - new owner must be superuser
+ALTER PUBLICATION testpub4 owner to regress_publication_user2; -- fail
+ALTER PUBLICATION testpub4 owner to regress_publication_user; -- ok
+
+SET ROLE regress_publication_user;
+DROP PUBLICATION testpub4;
+DROP ROLE regress_publication_user3;
+
 REVOKE CREATE ON DATABASE regression FROM regress_publication_user2;
 
 DROP TABLE testpub_parted;
-- 
2.32.0

Reply via email to