From 544bd6e6a88ff8ced8648d4601b5e76ed2d6298c Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 13 Feb 2017 08:57:45 -0500 Subject: [PATCH 1/6] Refine rules for altering publication owner Previously, the new owner had to be a superuser. The new rules are more refined similar to other objects. --- doc/src/sgml/ref/alter_publication.sgml | 7 ++++-- src/backend/commands/publicationcmds.c | 36 +++++++++++++++++++++---------- src/test/regress/expected/publication.out | 11 +++++++++- src/test/regress/sql/publication.sql | 7 +++++- 4 files changed, 46 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/ref/alter_publication.sgml b/doc/src/sgml/ref/alter_publication.sgml index 47d83b80be..776661bbeb 100644 --- a/doc/src/sgml/ref/alter_publication.sgml +++ b/doc/src/sgml/ref/alter_publication.sgml @@ -48,8 +48,11 @@ Description - To alter the owner, you must also be a direct or indirect member of the - new owning role. The new owner has to be a superuser + To alter the owner, you must also be a direct or indirect member of the new + owning role. The new owner must have CREATE privilege on + the database. Also, the new owner of a FOR ALL TABLES + publication must be a superuser. However, a superuser can change the + ownership of a publication while circumventing these restrictions. diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 04f83e0a2e..0e4eb0726d 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -660,7 +660,7 @@ PublicationDropTables(Oid pubid, List *rels, bool missing_ok) /* * Internal workhorse for changing a publication owner */ - static void + static void AlterPublicationOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId) { Form_pg_publication form; @@ -670,17 +670,31 @@ AlterPublicationOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId) if (form->pubowner == newOwnerId) return; - if (!pg_publication_ownercheck(HeapTupleGetOid(tup), GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PUBLICATION, - NameStr(form->pubname)); + if (!superuser()) + { + AclResult aclresult; - /* New owner must be a superuser */ - if (!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 publication must be a superuser."))); + /* Must be owner */ + if (!pg_publication_ownercheck(HeapTupleGetOid(tup), GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PUBLICATION, + NameStr(form->pubname)); + + /* Must be able to become new owner */ + check_is_member_of_role(GetUserId(), newOwnerId); + + /* New owner must have CREATE privilege on database */ + aclresult = pg_database_aclcheck(MyDatabaseId, newOwnerId, ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, ACL_KIND_DATABASE, + get_database_name(MyDatabaseId)); + + if (form->puballtables && !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 publication must be a superuser."))); + } form->pubowner = newOwnerId; CatalogTupleUpdate(rel, &tup->t_self, tup); diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out index 5784b0fded..448e708c08 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -2,6 +2,7 @@ -- PUBLICATION -- CREATE ROLE regress_publication_user LOGIN SUPERUSER; +CREATE ROLE regress_publication_user2; SET SESSION AUTHORIZATION 'regress_publication_user'; CREATE PUBLICATION testpub_default; CREATE PUBLICATION testpib_ins_trunct WITH (nopublish delete, nopublish update); @@ -148,10 +149,18 @@ DROP TABLE testpub_tbl1; t | t | t (1 row) +ALTER PUBLICATION testpub_default OWNER TO regress_publication_user2; +\dRp+ testpub_default + Publication testpub_default + Inserts | Updates | Deletes +---------+---------+--------- + t | t | t +(1 row) + DROP PUBLICATION testpub_default; DROP PUBLICATION testpib_ins_trunct; DROP PUBLICATION testpub_fortbl; DROP SCHEMA pub_test CASCADE; NOTICE: drop cascades to table pub_test.testpub_nopk RESET SESSION AUTHORIZATION; -DROP ROLE regress_publication_user; +DROP ROLE regress_publication_user, regress_publication_user2; diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql index 87797884d2..4b22053b13 100644 --- a/src/test/regress/sql/publication.sql +++ b/src/test/regress/sql/publication.sql @@ -2,6 +2,7 @@ -- PUBLICATION -- CREATE ROLE regress_publication_user LOGIN SUPERUSER; +CREATE ROLE regress_publication_user2; SET SESSION AUTHORIZATION 'regress_publication_user'; CREATE PUBLICATION testpub_default; @@ -73,6 +74,10 @@ CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1; \dRp+ testpub_default +ALTER PUBLICATION testpub_default OWNER TO regress_publication_user2; + +\dRp+ testpub_default + DROP PUBLICATION testpub_default; DROP PUBLICATION testpib_ins_trunct; DROP PUBLICATION testpub_fortbl; @@ -80,4 +85,4 @@ CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1; DROP SCHEMA pub_test CASCADE; RESET SESSION AUTHORIZATION; -DROP ROLE regress_publication_user; +DROP ROLE regress_publication_user, regress_publication_user2; -- 2.11.1